[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v9 03/10] virtio-blk: don't use pointer for conf
From: |
Kevin Wolf |
Subject: |
Re: [Qemu-devel] [PATCH v9 03/10] virtio-blk: don't use pointer for configuration. |
Date: |
Tue, 19 Mar 2013 14:55:53 +0100 |
User-agent: |
Mutt/1.5.21 (2010-09-15) |
Am 19.03.2013 um 14:38 hat KONRAD Frédéric geschrieben:
> On 18/03/2013 09:59, Kevin Wolf wrote:
> >Am 15.03.2013 um 19:48 hat address@hidden geschrieben:
> >>From: KONRAD Frederic <address@hidden>
> >>
> >>The configuration field must not be a pointer as it will be used for
> >>virtio-blk
> >>properties. So *blk is replaced by blk in VirtIOBlock structure.
> >>
> >>Signed-off-by: KONRAD Frederic <address@hidden>
> >>Reviewed-by: Peter Maydell <address@hidden>
> >>---
> >> hw/virtio-blk.c | 8 ++++----
> >> hw/virtio-blk.h | 2 +-
> >> 2 files changed, 5 insertions(+), 5 deletions(-)
> >>
> >>diff --git a/hw/virtio-blk.c b/hw/virtio-blk.c
> >>index 6714b01..908c316 100644
> >>--- a/hw/virtio-blk.c
> >>+++ b/hw/virtio-blk.c
> >>@@ -151,7 +151,7 @@ static void virtio_blk_handle_scsi(VirtIOBlockReq *req)
> >> */
> >> req->scsi = (void *)req->elem.in_sg[req->elem.in_num - 2].iov_base;
> >>- if (!req->dev->blk->scsi) {
> >>+ if (!req->dev->blk.scsi) {
> >> status = VIRTIO_BLK_S_UNSUPP;
> >> goto fail;
> >> }
> >>@@ -371,7 +371,7 @@ static void virtio_blk_handle_request(VirtIOBlockReq
> >>*req,
> >> * terminated by '\0' only when shorter than buffer.
> >> */
> >> strncpy(req->elem.in_sg[0].iov_base,
> >>- s->blk->serial ? s->blk->serial : "",
> >>+ s->blk.serial ? s->blk.serial : "",
> >> MIN(req->elem.in_sg[0].iov_len, VIRTIO_BLK_ID_BYTES));
> >> virtio_blk_req_complete(req, VIRTIO_BLK_S_OK);
> >> g_free(req);
> >>@@ -534,7 +534,7 @@ static uint32_t virtio_blk_get_features(VirtIODevice
> >>*vdev, uint32_t features)
> >> features |= (1 << VIRTIO_BLK_F_BLK_SIZE);
> >> features |= (1 << VIRTIO_BLK_F_SCSI);
> >>- if (s->blk->config_wce) {
> >>+ if (s->blk.config_wce) {
> >> features |= (1 << VIRTIO_BLK_F_CONFIG_WCE);
> >> }
> >> if (bdrv_enable_write_cache(s->bs))
> >>@@ -650,7 +650,7 @@ VirtIODevice *virtio_blk_init(DeviceState *dev,
> >>VirtIOBlkConf *blk)
> >> s->vdev.reset = virtio_blk_reset;
> >> s->bs = blk->conf.bs;
> >> s->conf = &blk->conf;
> >>- s->blk = blk;
> >>+ memcpy(&(s->blk), blk, sizeof(struct VirtIOBlkConf));
> >Why not simply s->blk = *blk?
> >
> >The reason why copying this works is that blk is read-only after
> >initialisation. We also get an additional reference to blk->serial, but
> >we know that it can only go away after this device has been destroyed
> >(the same assumption was necessary for the s->blk pointer in the old
> >code).
>
> You mean this copying (s->blk = *blk) ?
Or the memcpy() in your code, which should be equivalent.
Kevin
> >Is my understanding of this correct?
> >
> >Kevin
>
- [Qemu-devel] [PATCH v9 00/10] virtio-blk refactoring., fred . konrad, 2013/03/15
- [Qemu-devel] [PATCH v9 04/10] virtio-blk: add the virtio-blk device., fred . konrad, 2013/03/15
- [Qemu-devel] [PATCH v9 06/10] virtio-blk-s390: switch to the new API., fred . konrad, 2013/03/15
- [Qemu-devel] [PATCH v9 05/10] virtio-blk-pci: switch to new API., fred . konrad, 2013/03/15
- [Qemu-devel] [PATCH v9 09/10] virtio-blk: cleanup: QOM cast, fred . konrad, 2013/03/15
- [Qemu-devel] [PATCH v9 07/10] virtio-blk-ccw switch to new API., fred . konrad, 2013/03/15
- [Qemu-devel] [PATCH v9 08/10] virtio-blk: cleanup: init and exit functions., fred . konrad, 2013/03/15
- [Qemu-devel] [PATCH v9 10/10] virtio-blk: cleanup: remove qdev field., fred . konrad, 2013/03/15