Re: [Xen-devel] [PATCH for xen-4.4] libxl: add option for discard support to xl disk configuration

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Thu, 2014-01-30 at 17:25 +0100, Olaf Hering wrote:
> And just for reference, this is a version for our 4.4:
> 
> ....
> 
> This change does not break ABI. Instead of adding a new member 
> ->discard_enable
> to struct libxl_device_disk the existing ->readwrite member is reused.

Looks like it changes the libxlu ABI though. Or maybe that's totally
internal?

TBH -- if you (==suse I guess?) are contemplating carrying this as a
backport even before 4.4 is out the door we should probably be at least
considering a freeze exception for 4.4. George CCd for input. (I
appreciate that "backport=>freeze exception" is a potentially slippery
slope/ripe for abuse...)

> Signed-off-by: Olaf Hering <olaf@xxxxxxxxx>
> ---
>  docs/misc/xl-disk-configuration.txt | 15 +++++++++++++++
>  tools/libxl/libxl.c                 |  2 ++
>  tools/libxl/libxl.h                 | 11 +++++++++++
>  tools/libxl/libxlu_disk.c           |  3 +++
>  tools/libxl/libxlu_disk_i.h         |  2 +-
>  tools/libxl/libxlu_disk_l.l         |  4 ++++
>  6 files changed, 36 insertions(+), 1 deletion(-)
> 
> diff --git a/docs/misc/xl-disk-configuration.txt 
> b/docs/misc/xl-disk-configuration.txt
> index 5bd456d..c9fd9bd 100644
> --- a/docs/misc/xl-disk-configuration.txt
> +++ b/docs/misc/xl-disk-configuration.txt
> @@ -178,6 +178,21 @@ information to be interpreted by the executable program 
> <script>,
>  These scripts are normally called "block-<script>".
>  
> 
> +discard=<boolean>
> +---------------
> +
> +Description:           Instruct backend to advertise discard support to 
> frontend
> +Supported values:      on, off, 0, 1
> +Mandatory:             No
> +Default value:         on if, available for that backend typ
> +
> +This option is an advisory setting for the backend driver, depending of the
> +value, to advertise discard support (TRIM, UNMAP) to the frontend. The real
> +benefit of this option is to be able to force it off rather than on. It 
> allows
> +to disable "hole punching" for file based backends which were intentionally
> +created non-sparse to avoid fragmentation of the file.
> +
> +
>  
>  ============================================
>  DEPRECATED PARAMETERS, PREFIXES AND SYNTAXES
> diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
> index 2845ca4..9ed5062 100644
> --- a/tools/libxl/libxl.c
> +++ b/tools/libxl/libxl.c
> @@ -2196,6 +2196,8 @@ static void device_disk_add(libxl__egc *egc, uint32_t 
> domid,
>          flexarray_append(back, disk->readwrite ? "w" : "r");
>          flexarray_append(back, "device-type");
>          flexarray_append(back, disk->is_cdrom ? "cdrom" : "disk");
> +        if (disk->readwrite == 
> LIBXL_HAVE_LIBXL_DEVICE_DISK_DISCARD_DISABLE_MAGIC)
> +            flexarray_append_pair(back, "discard-enable", "0");
>  
>          flexarray_append(front, "backend-id");
>          flexarray_append(front, libxl__sprintf(gc, "%d", 
> disk->backend_domid));
> diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
> index 12d6c31..021f7e4 100644
> --- a/tools/libxl/libxl.h
> +++ b/tools/libxl/libxl.h
> @@ -95,6 +95,17 @@
>  #define LIBXL_HAVE_BUILDINFO_EVENT_CHANNELS 1
>  
>  /*
> + * The libxl_device_disk lacks discard_enable field, disabling discard
> + * is supported without breaking the ABI. This is done by overloading
> + * struct libxl_device_disk->readwrite:
> + * readwrite == 0: disk is readonly, no discard
> + * readwrite == 1: disk is readwrite, backend driver may enable discard
> + * readwrite == MAGIC: disk is readwrite, backend driver should not offer
> + * discard to the frontend driver.
> + */
> +#define LIBXL_HAVE_LIBXL_DEVICE_DISK_DISCARD_DISABLE_MAGIC 0xdcadU
> +
> +/*
>   * libxl ABI compatibility
>   *
>   * The only guarantee which libxl makes regarding ABI compatibility
> diff --git a/tools/libxl/libxlu_disk.c b/tools/libxl/libxlu_disk.c
> index 18fe386..e596cb6 100644
> --- a/tools/libxl/libxlu_disk.c
> +++ b/tools/libxl/libxlu_disk.c
> @@ -80,6 +80,9 @@ int xlu_disk_parse(XLU_Config *cfg,
>              disk->format = LIBXL_DISK_FORMAT_EMPTY;
>      }
>  
> +    if (disk->readwrite && dpc.disable_discard)
> +        disk->readwrite = LIBXL_HAVE_LIBXL_DEVICE_DISK_DISCARD_DISABLE_MAGIC;
> +
>      if (!disk->vdev) {
>          xlu__disk_err(&dpc,0, "no vdev specified");
>          goto x_err;
> diff --git a/tools/libxl/libxlu_disk_i.h b/tools/libxl/libxlu_disk_i.h
> index 4fccd4a..9db3002 100644
> --- a/tools/libxl/libxlu_disk_i.h
> +++ b/tools/libxl/libxlu_disk_i.h
> @@ -10,7 +10,7 @@ typedef struct {
>      void *scanner;
>      YY_BUFFER_STATE buf;
>      libxl_device_disk *disk;
> -    int access_set, had_depr_prefix;
> +    int access_set, disable_discard, had_depr_prefix;
>      const char *spec;
>  } DiskParseContext;
>  
> diff --git a/tools/libxl/libxlu_disk_l.l b/tools/libxl/libxlu_disk_l.l
> index 7c4e7f1..ecc30ae 100644
> --- a/tools/libxl/libxlu_disk_l.l
> +++ b/tools/libxl/libxlu_disk_l.l
> @@ -173,6 +173,10 @@ backendtype=[^,]*,? { STRIP(','); 
> setbackendtype(DPC,FROMEQUALS); }
>  
>  vdev=[^,]*,? { STRIP(','); SAVESTRING("vdev", vdev, FROMEQUALS); }
>  script=[^,]*,?       { STRIP(','); SAVESTRING("script", script, FROMEQUALS); 
> }
> +discard=on,? { DPC->disable_discard = 0; }
> +discard=1,?  { DPC->disable_discard = 0; }
> +discard=off,?        { DPC->disable_discard = 1; }
> +discard=0,?  { DPC->disable_discard = 1; }
>  
>   /* the target magic parameter, eats the rest of the string */
>  
> 
> 



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel