Re: [Xen-devel] [PATCH 2/2] Add trim operation to xen block devices

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Wed, 2010-12-22 at 09:10 +0000, Jan Beulich wrote:
> >>> On 21.12.10 at 17:58, Owen Smith <owen.smith@xxxxxxxxxx> wrote:
> > --- a/xen/include/public/io/blkif.h Tue Dec 21 13:28:48 2010 +0000
> > +++ b/xen/include/public/io/blkif.h Tue Dec 21 13:30:16 2010 +0000
> > @@ -76,6 +76,17 @@
> >   * "feature-flush-cache" node!
> >   */
> >  #define BLKIF_OP_FLUSH_DISKCACHE   3
> > +/*
> > + * Recognised only if "feature-trim" is present in backend xenbus info.
> > + * The "feature-trim" node contains a boolean indicating whether trim
> > + * requests are likely to succeed or fail. Either way, a trim request
> > + * may fail at any time with BLKIF_RSP_EOPNOTSUPP if it is unsupported by
> > + * the underlying block-device hardware. The boolean simply indicates 
> > whether
> > + * or not it is worthwhile for the frontend to attempt trim requests.
> > + * If a backend does not recognise BLKIF_OP_TRIM, it should *not*
> > + * create the "feature-trim" node!
> > + */
> > +#define BLKIF_OP_TRIM              4
> 
> I wonder if it would be possible to skip 4 here. We've been carrying
> a patch to support packet commands (for CD-ROM support) for
> quite a while, using 4 for BLKIF_OP_PACKET. I realize this should
> have been presented on the list long ago, but unfortunately the
> author never did and is no longer with the company. While I could
> submit the kernel side patches, I wouldn't be able to advocate for
> them, partly because I don't know all of the details, partly because
> there are some rough edges (Linux-isms introduced into
> xen/include/public/io/), and partly because backend support exists
> only for blktap1 so far.
> 
> An alternative to submitting the full patch set would be to just
> submit a patch adding the necessary definition here[...]
> would this be acceptable?

Given that the horse has already left the stable and there isn't much we
can do about that I think adding some sort of placeholder for op 4 (even
just marking it as reserved) would be fine.

>  - given that
> BLKIF_OP_FLUSH_DISKCACHE too looks like a half-baked thing
> only (used exclusively in mini-os' blkfront and qemu's block-vbd;
> I wasn't able to locate what backend might actually handle it),

The commit which added it is signed-off-by Sun, so I guess solaris...

Ian.

> 
> Thanks, Jan
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@xxxxxxxxxxxxxxxxxxx
> http://lists.xensource.com/xen-devel



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel