>>> On 11.10.11 at 20:27, Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx> wrote:
> On Mon, Oct 10, 2011 at 03:58:42PM -0400, Konrad Rzeszutek Wilk wrote:
>> On Mon, Oct 10, 2011 at 01:50:11PM -0400, Konrad Rzeszutek Wilk wrote:
>
> Per Ian and Jan's suggestion (note, the structure is 4-byte aligned
> so we do not need to pad it):
>
> # HG changeset patch
> # Parent 72f339bc600d7a9629d3f9eb8a279fbf8be25b12
> 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. Also update the description
> to mention the other parameters that the frontend can query the
> backend for: discard-aligment, discard-granularity, and
> discard-secure. We also utilize per Jan Beulich keen suggestion,
> the 8-bit reserved field to use as a flag value. Currently the only
> flag that can be passed for a discard operation is secure delete:
> BLKIF_OP_DISCARD_FLAG_SECURE.
>
> CC: lidongyang@xxxxxxxxxx
> CC: owen.smith@xxxxxxxxxx
> CC: Pasi Kärkkäinen <pasik@xxxxxx>
> CC: JBeulich@xxxxxxxxxx
> Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>
>
> diff -r 72f339bc600d xen/include/public/io/blkif.h
> --- a/xen/include/public/io/blkif.h Mon Oct 10 11:21:51 2011 +0100
> +++ b/xen/include/public/io/blkif.h Tue Oct 11 14:10:33 2011 -0400
> @@ -82,26 +82,47 @@
> */
> #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
> + * 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
> + * logical block size. The discard-alignment parameter indicates how many
> bytes
> + * the beginning of the partition is offset from the internal allocation
> unit's
> + * natural alignment.
> + * 'discard-granularity' - Devices that support discard functionality may
> + * internally allocate space using units that are bigger than the logical
> block
> + * 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.
> */
> -#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.
> @@ -134,18 +155,20 @@ struct blkif_request {
> typedef struct blkif_request blkif_request_t;
>
> /*
> - * Cast to this structure when blkif_request.operation == BLKIF_OP_TRIM
> - * sizeof(struct blkif_request_trim) <= sizeof(struct blkif_request)
> + * Cast to this structure when blkif_request.operation == BLKIF_OP_DISCARD
> + * sizeof(struct blkif_request_discard) <= sizeof(struct blkif_request)
> */
> -struct blkif_request_trim {
> - uint8_t operation; /* BLKIF_OP_TRIM */
> - uint8_t reserved; /* */
> +struct blkif_request_discard {
> + uint8_t operation; /* BLKIF_OP_DISCARD */
> + /* ignored if 'discard-secure=0' */
> +#define BLKIF_OP_DISCARD_FLAG_SECURE (1<<0)
I'd like to have the _OP in here dropped - BLKIF_OP_* should be
"reserved" to express actual operations.
With that change,
Acked-by: Jan Beulich <jbeulich@xxxxxxxx>
(I'd also consider the _FLAG part to be sort of redundant.)
Jan
> + uint8_t flag; /* BLKIF_OP_DISCARD_FLAG_SECURE or 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 */
> uint64_t nr_sectors; /* number of contiguous sectors to trim */
> };
> -typedef struct blkif_request_trim blkif_request_trim_t;
> +typedef struct blkif_request_discard blkif_request_discard_t;
>
> struct blkif_response {
> uint64_t id; /* copied from request */
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel
|
Home