Re: Re: [PATCH 1 of 1] interface: BLKIF_OP_TRIM -> BLKIF_OP_DISCARD

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

> well I think we need to change the comments here, discard doesn't
> guarantee the blocks
> will be erased from the device, it's just a hint to the device
> controller these blocks are no longer
> used, and what to do with these blocks are left to the device
> controller, on an SSD the controller

How about this one below:

# HG changeset patch
# Parent ac9aa65050e9abc8f1c12c8603acf3b99e22cddc
interface: BLKIF_OP_TRIM -> BLKIF_OP_DISCARD

The name 'trim' is specific to the ATA discard implementation.
The name 'scsi unmap' is specific to the SCSI discard implementation.

We should really use a generic name - and the name 'discard'
looks to be the most generic of them all.

CC: lidongyang@novell.com
CC: owen.smith@citrix.com
CC: Pasi Kärkkäinen <pasik@iki.fi>
CC: JBeulich@novell.com
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>

diff -r ac9aa65050e9 xen/include/public/io/blkif.h
--- a/xen/include/public/io/blkif.h	Tue Aug 30 11:46:58 2011 +0100
+++ b/xen/include/public/io/blkif.h	Wed Aug 31 11:22:57 2011 -0400
@@ -82,26 +82,33 @@
  */
 #define BLKIF_OP_RESERVED_1        4
 /*
- * 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
+ * Recognised only if "feature-discard" is present in backend xenbus info.
+ * The "feature-discard" node contains a boolean indicating whether trim
+ * (ATA) or unmap (SCSI) - conviently called discard requests are likely
+ * to succeed or fail. Either way, a discard 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!
- * 
- * Trim operation is a request for the underlying block device to mark
- * extents to be erased. Trim operations are passed with sector_number as the
- * sector index to begin trim operations at and nr_sectors as the number of
- * sectors to be trimmed. The specified sectors should be trimmed if the
- * underlying block device supports trim operations, or a BLKIF_RSP_EOPNOTSUPP
- * should be returned. More information about trim operations at:
+ * or not it is worthwhile for the frontend to attempt discard requests.
+ * If a backend does not recognise BLKIF_OP_DISCARD, it should *not*
+ * create the "feature-discard" node!
+ *
+ * Discard operation is a request for the underlying block device to mark
+ * extents to be erased. However, discard does not guarantee that the blocks
+ * will be erased from the device - it is just a hint to the device
+ * controller that these blocks are no longer in use. What the device
+ * controller does with that information is left to the controller.
+ * Discard operations are passed with sector_number as the
+ * sector index to begin discard operations at and nr_sectors as the number of
+ * sectors to be discarded. The specified sectors should be discarded if the
+ * underlying block device supports trim (ATA) or unmap (SCSI) operations,
+ * or a BLKIF_RSP_EOPNOTSUPP  should be returned.
+ * More information about trim/unmap operations at:
  * http://t13.org/Documents/UploadedDocuments/docs2008/
  *     e07154r6-Data_Set_Management_Proposal_for_ATA-ACS2.doc
+ * http://www.seagate.com/staticfiles/support/disc/manuals/
+ *     Interface%20manuals/100293068c.pdf
  */
-#define BLKIF_OP_TRIM              5
-
+#define BLKIF_OP_DISCARD          5
 /*
  * Maximum scatter/gather segments per request.
  * This is carefully chosen so that sizeof(blkif_ring_t) <= PAGE_SIZE.