| Message ID | 570E39F2.2010904@oracle.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Bob Liu writes ("Re: [RFC PATCH] Data integrity extension support for xen-block"):
> A simpler possible solution:
As a syntax this is certainly simpler and quite probably preferable.
But NB that I regard your email as simply a sketch of the syntax. We
also need (as discussed) a statement of the semantics.
Ian.
On 13/04/16 14:22, Bob Liu wrote: > > On 04/07/2016 06:00 PM, Bob Liu wrote: >> * What's data integrity extension and why? >> Modern filesystems feature checksumming of data and metadata to protect against >> data corruption. However, the detection of the corruption is done at read time >> which could potentially be months after the data was written. At that point the >> original data that the application tried to write is most likely lost. >> >> The solution in Linux is the data integrity framework which enables protection >> information to be pinned to I/Os and sent to/received from controllers that >> support it. struct bio has been extended with a pointer to a struct bip which >> in turn contains the integrity metadata. The bip is essentially a trimmed down >> bio with a bio_vec and some housekeeping. >> >> * Issues when xen-block get involved. >> xen-blkfront only transmits the normal data of struct bio while the integrity >> metadata buffer(struct bio_integrity_payload in each bio) is ignored. >> >> * Proposal of transmitting bio integrity payload. >> Adding an extra request following the normal data request, this extra request >> contains the integrity payload. >> The xen-blkback will reconstruct an new bio with both received normal data and >> integrity metadata. >> >> Welcome any better ideas, thank you! >> > > A simpler possible solution: > > bob@boliuliu:~/xen$ git diff xen/include/public/io/blkif.h > diff --git a/xen/include/public/io/blkif.h b/xen/include/public/io/blkif.h > index 3d8d39f..34581a5 100644 > --- a/xen/include/public/io/blkif.h > +++ b/xen/include/public/io/blkif.h > @@ -689,6 +689,11 @@ struct blkif_request_segment { > struct blkif_request { > uint8_t operation; /* BLKIF_OP_??? */ > uint8_t nr_segments; /* number of segments */ > + /* > + * Recording how many segments are data integrity segments. > + * raw data_segments + dix_segments = nr_segments > + */ > + uint8_t dix_segments; > blkif_vdev_t handle; /* only for read/write requests */ > uint64_t id; /* private guest value, echoed in resp */ > blkif_sector_t sector_number;/* start sector idx on disk (r/w only) */ > @@ -715,6 +720,11 @@ struct blkif_request_indirect { > uint8_t operation; /* BLKIF_OP_INDIRECT */ > uint8_t indirect_op; /* BLKIF_OP_{READ/WRITE} */ > uint16_t nr_segments; /* number of segments */ > + /* > + * Recording how many segments are data integrity segments. > + * raw data_segments + dix_segments = nr_segments > + */ > + uint16_t dix_segments; > uint64_t id; /* private guest value, echoed in resp */ > blkif_sector_t sector_number;/* start sector idx on disk (r/w only) */ > blkif_vdev_t handle; /* same as for read/write requests */ > Without having checked whether there were padding holes where you introduced the new elements: this looks much better. As Ian already pointed out: you should mention somewhere what the new segments are containing (data layout description, possibly just a reference to a hardware spec?). Juergen
diff --git a/xen/include/public/io/blkif.h b/xen/include/public/io/blkif.h index 3d8d39f..34581a5 100644 --- a/xen/include/public/io/blkif.h +++ b/xen/include/public/io/blkif.h @@ -689,6 +689,11 @@ struct blkif_request_segment { struct blkif_request { uint8_t operation; /* BLKIF_OP_??? */ uint8_t nr_segments; /* number of segments */ + /* + * Recording how many segments are data integrity segments. + * raw data_segments + dix_segments = nr_segments + */ + uint8_t dix_segments; blkif_vdev_t handle; /* only for read/write requests */ uint64_t id; /* private guest value, echoed in resp */ blkif_sector_t sector_number;/* start sector idx on disk (r/w only) */ @@ -715,6 +720,11 @@ struct blkif_request_indirect { uint8_t operation; /* BLKIF_OP_INDIRECT */ uint8_t indirect_op; /* BLKIF_OP_{READ/WRITE} */ uint16_t nr_segments; /* number of segments */ + /* + * Recording how many segments are data integrity segments. + * raw data_segments + dix_segments = nr_segments + */ + uint16_t dix_segments; uint64_t id; /* private guest value, echoed in resp */ blkif_sector_t sector_number;/* start sector idx on disk (r/w only) */ blkif_vdev_t handle; /* same as for read/write requests */