Redefining the blkif_req struct naming seems an unnecessary pain. Why not
define a separate 'struct blkif_trim_request' that starts with the uint8_t
BLKIF_OP_TRIM and thereafter is defined exactly how you like? Then drivers
can cast a request pointer to your new type when reading/writing a trim
request, and no existing code needs to be changed. If all this does is
introduce new definitions into xen-unstable, rather than changing existing
ones, we could still potentially slip it in for 4.1.0 as an obviously safe
patch.
-- Keir
On 14/01/2011 16:44, "owen.smith@xxxxxxxxxx" <owen.smith@xxxxxxxxxx> wrote:
> From: Owen Smith <owen.smith@xxxxxxxxxx>
>
> # HG
> changeset patch
> # User root@xxxxxxxxxxxxxxxxxxxxxxx
> # Date 1294919321 0
> # Node ID 428420495fd3cfda164e7b355b0ffcc0845af2e4
> # Parent 7b4c82f07281ad9c48b652e2a305a7be607c5283
> blkif: Move read/write/barrier specific fields into a union
>
> Patches xen-unstable.hg
>
> Modifies the blkif ring interface by placing the request specific
> fields into a union in order to support additional operation types.
>
> Signed-off-by: Owen Smith <owen.smith@xxxxxxxxxx>
>
> diff -r 7b4c82f07281 -r 428420495fd3 extras/mini-os/blkfront.c
> --- a/extras/mini-os/blkfront.c Wed Jan 05 23:54:15 2011 +0000
> +++ b/extras/mini-os/blkfront.c Thu Jan 13 11:48:41 2011 +0000
> @@ -361,22 +361,22 @@
> req->nr_segments = n;
> req->handle = dev->handle;
> req->id = (uintptr_t) aiocbp;
> - req->sector_number = aiocbp->aio_offset / 512;
> + req->u.rw.sector_number = aiocbp->aio_offset / 512;
>
> for (j = 0; j < n; j++) {
> - req->seg[j].first_sect = 0;
> - req->seg[j].last_sect = PAGE_SIZE / 512 - 1;
> + req->u.rw.seg[j].first_sect = 0;
> + req->u.rw.seg[j].last_sect = PAGE_SIZE / 512 - 1;
> }
> - req->seg[0].first_sect = ((uintptr_t)aiocbp->aio_buf & ~PAGE_MASK) / 512;
> - req->seg[n-1].last_sect = (((uintptr_t)aiocbp->aio_buf +
> aiocbp->aio_nbytes - 1) & ~PAGE_MASK) / 512;
> + req->u.rw.seg[0].first_sect = ((uintptr_t)aiocbp->aio_buf & ~PAGE_MASK) /
> 512;
> + req->u.rw.seg[n-1].last_sect = (((uintptr_t)aiocbp->aio_buf +
> aiocbp->aio_nbytes - 1) & ~PAGE_MASK) / 512;
> for (j = 0; j < n; j++) {
> uintptr_t data = start + j * PAGE_SIZE;
> if (!write) {
> /* Trigger CoW if needed */
> - *(char*)(data + (req->seg[j].first_sect << 9)) = 0;
> + *(char*)(data + (req->u.rw.seg[j].first_sect << 9)) = 0;
> barrier();
> }
> - aiocbp->gref[j] = req->seg[j].gref =
> + aiocbp->gref[j] = req->u.rw.seg[j].gref =
> gnttab_grant_access(dev->dom, virtual_to_mfn(data), write);
> }
>
> @@ -432,7 +432,7 @@
> req->handle = dev->handle;
> req->id = id;
> /* Not needed anyway, but the backend will check it */
> - req->sector_number = 0;
> + req->u.rw.sector_number = 0;
> dev->ring.req_prod_pvt = i + 1;
> wmb();
> RING_PUSH_REQUESTS_AND_CHECK_NOTIFY(&dev->ring, notify);
> diff -r 7b4c82f07281 -r 428420495fd3 tools/blktap/drivers/tapdisk.c
> --- a/tools/blktap/drivers/tapdisk.c Wed Jan 05 23:54:15 2011 +0000
> +++ b/tools/blktap/drivers/tapdisk.c Thu Jan 13 11:48:41 2011 +0000
> @@ -528,10 +528,10 @@
> segment_start(blkif_request_t *req, int sidx)
> {
> int i;
> - uint64_t start = req->sector_number;
> + uint64_t start = req->u.rw.sector_number;
>
> for (i = 0; i < sidx; i++)
> - start += (req->seg[i].last_sect - req->seg[i].first_sect + 1);
> + start += (req->u.rw.seg[i].last_sect - req->u.rw.seg[i].first_sect + 1);
>
> return start;
> }
> @@ -600,13 +600,13 @@
> struct disk_driver *parent = dd->next;
>
> seg_start = segment_start(req, sidx);
> - seg_end = seg_start + req->seg[sidx].last_sect + 1;
> + seg_end = seg_start + req->u.rw.seg[sidx].last_sect + 1;
>
> ASSERT(sector >= seg_start && sector + nr_secs <= seg_end);
>
> page = (char *)MMAP_VADDR(info->vstart,
> (unsigned long)req->id, sidx);
> - page += (req->seg[sidx].first_sect << SECTOR_SHIFT);
> + page += (req->u.rw.seg[sidx].first_sect << SECTOR_SHIFT);
> page += ((sector - seg_start) << SECTOR_SHIFT);
>
> if (!parent) {
> @@ -667,7 +667,7 @@
> req, sizeof(*req));
> blkif->pending_list[idx].status = BLKIF_RSP_OKAY;
> blkif->pending_list[idx].submitting = 1;
> - sector_nr = req->sector_number;
> + sector_nr = req->u.rw.sector_number;
> }
>
> if ((dd->flags & TD_RDONLY) &&
> @@ -677,16 +677,16 @@
> }
>
> for (i = start_seg; i < req->nr_segments; i++) {
> - nsects = req->seg[i].last_sect -
> - req->seg[i].first_sect + 1;
> + nsects = req->u.rw.seg[i].last_sect -
> + req->u.rw.seg[i].first_sect + 1;
>
> - if ((req->seg[i].last_sect >= page_size >> 9) ||
> + if ((req->u.rw.seg[i].last_sect >= page_size >> 9) ||
> (nsects <= 0))
> continue;
>
> page = (char *)MMAP_VADDR(info->vstart,
> (unsigned long)req->id, i);
> - page += (req->seg[i].first_sect << SECTOR_SHIFT);
> + page += (req->u.rw.seg[i].first_sect << SECTOR_SHIFT);
>
> if (sector_nr >= s->size) {
> DPRINTF("Sector request failed:\n");
> diff -r 7b4c82f07281 -r 428420495fd3 tools/blktap2/drivers/tapdisk-diff.c
> --- a/tools/blktap2/drivers/tapdisk-diff.c Wed Jan 05 23:54:15 2011 +0000
> +++ b/tools/blktap2/drivers/tapdisk-diff.c Thu Jan 13 11:48:41 2011 +0000
> @@ -363,13 +363,13 @@
> breq = &sreq->blkif_req;
> breq->id = idx;
> breq->nr_segments = r->blkif_req.nr_segments;
> - breq->sector_number = r->blkif_req.sector_number;
> + breq->u.rw.sector_number = r->blkif_req.u.rw.sector_number;
> breq->operation = BLKIF_OP_READ;
>
> for (int i = 0; i < r->blkif_req.nr_segments; i++) {
> - struct blkif_request_segment *seg = breq->seg + i;
> - seg->first_sect = r->blkif_req.seg[i].first_sect;
> - seg->last_sect = r->blkif_req.seg[i].last_sect;
> + struct blkif_request_segment *seg = breq->u.rw.seg + i;
> + seg->first_sect = r->blkif_req.u.rw.seg[i].first_sect;
> + seg->last_sect = r->blkif_req.u.rw.seg[i].last_sect;
> }
> s->cur += sreq->secs;
>
> @@ -426,12 +426,12 @@
> breq = &sreq->blkif_req;
> breq->id = idx;
> breq->nr_segments = 0;
> - breq->sector_number = sreq->sec;
> + breq->u.rw.sector_number = sreq->sec;
> breq->operation = BLKIF_OP_READ;
>
> for (i = 0; i < BLKIF_MAX_SEGMENTS_PER_REQUEST; i++) {
> uint32_t secs;
> - struct blkif_request_segment *seg = breq->seg + i;
> + struct blkif_request_segment *seg = breq->u.rw.seg + i;
>
> secs = MIN(s->end - s->cur, psize >> SECTOR_SHIFT);
> secs = MIN(((blk + 1) << SPB_SHIFT) - s->cur, secs);
> diff -r 7b4c82f07281 -r 428420495fd3 tools/blktap2/drivers/tapdisk-image.c
> --- a/tools/blktap2/drivers/tapdisk-image.c Wed Jan 05 23:54:15 2011 +0000
> +++ b/tools/blktap2/drivers/tapdisk-image.c Thu Jan 13 11:48:41 2011 +0000
> @@ -148,15 +148,15 @@
> psize = getpagesize();
>
> for (i = 0; i < req->nr_segments; i++) {
> - nsects = req->seg[i].last_sect - req->seg[i].first_sect + 1;
> + nsects = req->u.rw.seg[i].last_sect - req->u.rw.seg[i].first_sect + 1;
>
> - if (req->seg[i].last_sect >= psize >> 9 || nsects <= 0)
> + if (req->u.rw.seg[i].last_sect >= psize >> 9 || nsects <= 0)
> goto fail;
>
> total += nsects;
> }
>
> - if (req->sector_number + nsects > info->size)
> + if (req->u.rw.sector_number + nsects > info->size)
> goto fail;
>
> return 0;
> @@ -164,6 +164,6 @@
> fail:
> ERR(-EINVAL, "bad request on %s (%s, %"PRIu64"): id: %"PRIu64": %d at
> %"PRIu64,
> image->name, (rdonly ? "ro" : "rw"), info->size, req->id,
> - req->operation, req->sector_number + total);
> + req->operation, req->u.rw.sector_number + total);
> return -EINVAL;
> }
> diff -r 7b4c82f07281 -r 428420495fd3 tools/blktap2/drivers/tapdisk-stream.c
> --- a/tools/blktap2/drivers/tapdisk-stream.c Wed Jan 05 23:54:15 2011 +0000
> +++ b/tools/blktap2/drivers/tapdisk-stream.c Thu Jan 13 11:48:41 2011 +0000
> @@ -293,12 +293,12 @@
> breq = &sreq->blkif_req;
> breq->id = idx;
> breq->nr_segments = 0;
> - breq->sector_number = sreq->sec;
> + breq->u.rw.sector_number = sreq->sec;
> breq->operation = BLKIF_OP_READ;
>
> for (i = 0; i < BLKIF_MAX_SEGMENTS_PER_REQUEST; i++) {
> uint32_t secs = MIN(s->end - s->cur, psize >> SECTOR_SHIFT);
> - struct blkif_request_segment *seg = breq->seg + i;
> + struct blkif_request_segment *seg = breq->u.rw.seg + i;
>
> if (!secs)
> break;
> diff -r 7b4c82f07281 -r 428420495fd3 tools/blktap2/drivers/tapdisk-vbd.c
> --- a/tools/blktap2/drivers/tapdisk-vbd.c Wed Jan 05 23:54:15 2011 +0000
> +++ b/tools/blktap2/drivers/tapdisk-vbd.c Thu Jan 13 11:48:41 2011 +0000
> @@ -1066,7 +1066,7 @@
> rsp->status = vreq->status;
>
> DBG(TLOG_DBG, "writing req %d, sec 0x%08"PRIx64", res %d to ring\n",
> - (int)tmp.id, tmp.sector_number, vreq->status);
> + (int)tmp.id, tmp.u.rw.sector_number, vreq->status);
>
> if (rsp->status != BLKIF_RSP_OKAY)
> ERR(EIO, "returning BLKIF_RSP %d", rsp->status);
> @@ -1181,10 +1181,10 @@
> tapdisk_vbd_breq_get_sector(blkif_request_t *breq, td_request_t treq)
> {
> int seg, nsects;
> - uint64_t sector_nr = breq->sector_number;
> + uint64_t sector_nr = breq->u.rw.sector_number;
>
> for(seg=0; seg < treq.sidx; seg++) {
> - nsects = breq->seg[seg].last_sect - breq->seg[seg].first_sect + 1;
> + nsects = breq->u.rw.seg[seg].last_sect -
> breq->u.rw.seg[seg].first_sect + 1;
> sector_nr += nsects;
> }
>
> @@ -1366,7 +1366,7 @@
> req = &vreq->req;
> id = req->id;
> ring = &vbd->ring;
> - sector_nr = req->sector_number;
> + sector_nr = req->u.rw.sector_number;
> image = tapdisk_vbd_first_image(vbd);
>
> vreq->submitting = 1;
> @@ -1385,10 +1385,10 @@
> goto fail;
>
> for (i = 0; i < req->nr_segments; i++) {
> - nsects = req->seg[i].last_sect - req->seg[i].first_sect + 1;
> + nsects = req->u.rw.seg[i].last_sect - req->u.rw.seg[i].first_sect + 1;
> page = (char *)MMAP_VADDR(ring->vstart,
> (unsigned long)req->id, i);
> - page += (req->seg[i].first_sect << SECTOR_SHIFT);
> + page += (req->u.rw.seg[i].first_sect << SECTOR_SHIFT);
>
> treq.id = id;
> treq.sidx = i;
> @@ -1482,7 +1482,7 @@
> vreq->status = BLKIF_RSP_OKAY;
> DBG(TLOG_DBG, "retry #%d of req %"PRIu64", "
> "sec 0x%08"PRIx64", nr_segs: %d\n", vreq->num_retries,
> - vreq->req.id, vreq->req.sector_number,
> + vreq->req.id, vreq->req.u.rw.sector_number,
> vreq->req.nr_segments);
>
> err = tapdisk_vbd_issue_request(vbd, vreq);
> diff -r 7b4c82f07281 -r 428420495fd3 xen/include/public/io/blkif.h
> --- a/xen/include/public/io/blkif.h Wed Jan 05 23:54:15 2011 +0000
> +++ b/xen/include/public/io/blkif.h Thu Jan 13 11:48:41 2011 +0000
> @@ -103,7 +103,22 @@
> uint8_t first_sect, last_sect;
> };
>
> +struct blkif_request_rw {
> + blkif_sector_t sector_number;/* start sector idx on disk (r/w only) */
> + struct blkif_request_segment seg[BLKIF_MAX_SEGMENTS_PER_REQUEST];
> +};
> +
> struct blkif_request {
> + uint8_t operation; /* BLKIF_OP_??? */
> + uint8_t nr_segments; /* number of segments */
> + blkif_vdev_t handle; /* only for read/write requests */
> + uint64_t id; /* private guest value, echoed in resp */
> + union {
> + struct blkif_request_rw rw;
> + } u;
> +};
> +
> +struct blkif_request_old {
> uint8_t operation; /* BLKIF_OP_??? */
> uint8_t nr_segments; /* number of segments */
> blkif_vdev_t handle; /* only for read/write requests */
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@xxxxxxxxxxxxxxxxxxx
> http://lists.xensource.com/xen-devel
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel
|