Re: [Xen-devel] [PATCH 08/11] libxl: add option for discard support to xl disk configuration

[Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx>]

Olaf Hering writes ("[PATCH 08/11] libxl: add option for discard support to xl 
disk configuration"):
> Handle new option discard=on|off for disk configuration. It is supposed
> to disable discard support if file based backing storage was
> intentionally created non-sparse to avoid fragmentation of the file.
> 
> The option is a boolean and intended for the backend driver. A new
> boolean property "discard-enable" is written to the backend node. An
> upcoming patch for qemu will make use of this property. The kernel
> blkback driver may be updated as well to disable discard for phy based
> backing storage.

Thanks.

> +discard=<boolean>
> +---------------
...
> +This option is an advisory setting for the backend driver, depending of the
> +value, to advertise discard support (TRIM, UNMAP) to the frontend.

I think the semantics of this are unclear.  Calling it an "advisory"
setting doesn't help.  The documentation should explicitly explain
under what circumstances it might be honoured.

> diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
> index 730f6e1..a18929b 100644
> --- a/tools/libxl/libxl.c
> +++ b/tools/libxl/libxl.c
> @@ -2192,6 +2192,9 @@ static void device_disk_add(libxl__egc *egc, uint32_t 
> d...
> +        if (!libxl_defbool_is_default(disk->discard_enable))
> +            flexarray_append_pair(back, "discard-enable",
> +                                  libxl_defbool_val(disk->discard_enable) ? 
> "1" : "0");

This approach is quite unusual, and arguably wrong.  I think it would
be better if the default were (essentially) fixed.  This would result
in discard-enable always being written here.

> + * discard-enable
> + *      Values:         0/1 (boolean)
> + *      Default Value:  1
> + *
> + *      This optional property, set by the toolstack, instructs the
> + *      backend to offer discard to the frontend. If the property
> + *      is missing the backend should offer discard if the backing
> + *      storage actually supports it.

What should the backend do if the property is set but the backend
doesn't support the requested value ?

Ian.

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