Mailing List Archive: [PATCH 3 of 3] interface: add 'discard-secure' and BLKIF_DISCARD

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

Oct 12, 2011, 3:12 PM

Post #1 of 4 (214 views)

# 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
* 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.
*/
#define BLKIF_OP_DISCARD 5

@@ -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 */

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

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

Ian.Campbell at citrix

Oct 13, 2011, 12:59 AM

Post #2 of 4 (215 views)

Permalink

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.

> */
> #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.

>
> @@ -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 */
>
>

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

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

konrad.wilk at oracle

Oct 13, 2011, 7:12 AM

Post #3 of 4 (212 views)

Permalink

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 */
> >
> >
>

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

[PATCH 3 of 3] interface: add 'discard-secure' and BLKIF_DISCARD_SECURE [ In reply to ]

konrad.wilk at oracle

Oct 26, 2011, 2:02 PM

Post #4 of 4 (201 views)

Permalink

# HG changeset patch
# User Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
# Date 1318457231 14400
# Node ID ef78e170a9008346578458abf541075310cb299b
# Parent 14793d6c4adb38cc57a6d55a8907a18d5ca18634
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 14793d6c4adb -r ef78e170a900 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
@@ -120,6 +120,9 @@
* allocation unit in bytes if reported by the device. Otherwise the
* discard-granularity will be set to match the device's physical block size.
* It is the minimum size you can discard.
+ * '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_discard.
*/
#define BLKIF_OP_DISCARD 5

@@ -160,7 +163,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 */

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