Re: [PATCH 3 of 3] interface: add 'discard-secure' and BLKIF_DISCARD_SECURE

[Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>]

On Thu, Oct 13, 2011 at 08:59:28AM +0100, Ian Campbell wrote:
> On Wed, 2011-10-12 at 23:12 +0100, Konrad Rzeszutek Wilk wrote:
> > # HG changeset patch
> > # User Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> > # Date 1318457231 14400
> > # Node ID 50850daec7f0486ee7ca69b3d4cb58b4d340a5a7
> > # Parent  15c2d70dbac3e31c2d74b6700e1bb5f8a7d8256e
> > interface: add 'discard-secure' and BLKIF_DISCARD_SECURE
> > 
> > Alter the 'reserved' uint8_t to be used a 'flag'. We use only for
> > one flag: BLKIF_DISCARD_SECURE.
> > 
> > That flag can only be set if the backend has set 'discard-secure' to one.
> > If backend has not set 'discard-secure' to one, that flag will have no
> > effect.
> > 
> > Acked-by: Jan Beulich <JBeulich@suse.com>
> > Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> > 
> > diff -r 15c2d70dbac3 -r 50850daec7f0 xen/include/public/io/blkif.h
> > --- a/xen/include/public/io/blkif.h	Wed Oct 12 18:07:07 2011 -0400
> > +++ b/xen/include/public/io/blkif.h	Wed Oct 12 18:07:11 2011 -0400
> > @@ -107,7 +107,7 @@
> >   *     e07154r6-Data_Set_Management_Proposal_for_ATA-ACS2.doc
> >   * http://www.seagate.com/staticfiles/support/disc/manuals/
> >   *     Interface%20manuals/100293068c.pdf
> > - * The backend can optionally provide two extra XenBus attributes to
> > + * The backend can optionally provide three extra XenBus attributes to
>                                          these
> 
> avoids patching (or more likely forgetting to patch) this line every
> time we add an attribute.
> 
> >   * further optimize the discard functionality:
> >   * 'discard-aligment' - Devices that support discard functionality may
> >   * internally allocate space in units that are bigger than the exported
> > @@ -119,6 +119,9 @@
> >   * size. The discard-granularity parameter indicates the size of the internal
> >   * allocation unit in bytes if reported by the device. Otherwise the
> >   * discard-granularity will be set to match the device's physical block size.
> > + * 'discard-secure' - All copies of the discarded sectors (potentially created
> > + * by garbage collection) must also be erased.  To use this feature, the flag
> > + * BLKIF_DISCARD_SECURE must be set in the blkif_request_trim.
> 
> Stray "trim" here.

Duh!
> 
> >   */
> >  #define BLKIF_OP_DISCARD           5
> 
> It just occurred to me that if reusing the reserved field is going to
> prove a problem we could have had BLKIF_OP_DISCARD_SECURE. I think we've
> got things under control though.

Yeah, I think the reserved->flag attribute worked out nicely.
> 
> >  
> > @@ -159,7 +162,8 @@ typedef struct blkif_request blkif_reque
> >   */
> >  struct blkif_request_discard {
> >      uint8_t        operation;    /* BLKIF_OP_DISCARD                     */
> > -    uint8_t        reserved;     /*                                      */
> > +    uint8_t        flag;         /* BLKIF_DISCARD_SECURE or zero         */
> > +#define BLKIF_DISCARD_SECURE (1<<0)  /* ignored if discard-secure=0      */
> >      blkif_vdev_t   handle;       /* same as for read/write requests      */
> >      uint64_t       id;           /* private guest value, echoed in resp  */
> >      blkif_sector_t sector_number;/* start sector idx on disk             */
> > 
> > 
>