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

[Olaf Hering <olaf@aepfle.de>]

On Thu, Mar 06, Ian Jackson wrote:

> Olaf Hering writes ("[PATCH 08/11] libxl: add option for discard support to xl disk configuration"):
> > +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.
> > +        if (!libxl_defbool_is_default(disk->discard_enable))
> 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.

I'm fine with writing discard-enable="1" always.

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

The backend could advertise feature-discard anyway, so that error paths
will be executed when the frontend actually sends a discard request.
Right now the backend will catch this and both frontend/backend will
internally disable discard.

Olaf