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

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Wed, 2014-01-29 at 12:19 +0100, Olaf Hering wrote:
> On Wed, Jan 29, Ian Campbell wrote:
> 
> > > +Default value:         on
> > 
> > I think this default should be "on if, available for that backend type".
> 
> Ok, will make this change.
> 
> > What happens if the backed does not support discard?
> 
> The toolstack just does not know if a phy device supports it, or if file
> backed storage can do hole punching. If feature-discard is set and the
> frontend sends a discard request, the backend would return an error
> (like ENOTSUPPORTED) and the frontend internally disables the discard
> flag. Thats how it is done in pvops and the forward ported xenlinux
> tree.

That sounds good.

Is it worth noting that enable-discard=1 is only advisory and will be
ignored if the underlying storage and/or backend doesn't understand it?
The real benefit of this option is to be able to force it off rather
than on I think.

> Up to now I have not prepared a change for the backend drivers. They
> could either force feature-discard to be true so that the error paths
> will be executed. Or they could ignore the discard-enable if the backing
> storage does not support discard.
> 
> > >      disk->readwrite = 1;
> > > +    disk->discard_enable = 1; /* Doing it twice?! */
> > 
> > Why?
> 
> Thats what I'm asking you. Why is readwrite set here, and later on also
> in the .l file? At least just setting it here did not unconditionally
> enable it if no discard= was specified. I have not traced the code why
> that happens.

One for Ian J I think. Perhaps it is just setting the default?

readwrite is on unless you say "ro" in the config, so I suppose that
makes sense. I don't know about discard_enable -- if this were a defbool
it would probably go away anyway.

Ian.


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