From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:43336) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UHwk5-0005Nl-Ko for qemu-devel@nongnu.org; Tue, 19 Mar 2013 09:38:19 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1UHwk0-0004SC-Ka for qemu-devel@nongnu.org; Tue, 19 Mar 2013 09:38:17 -0400 Received: from greensocs.com ([87.106.252.221]:49173 helo=s15328186.onlinehome-server.info) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UHwk0-0004S1-9z for qemu-devel@nongnu.org; Tue, 19 Mar 2013 09:38:12 -0400 Message-ID: <51486A3F.6050003@greensocs.com> Date: Tue, 19 Mar 2013 14:38:07 +0100 From: =?ISO-8859-1?Q?KONRAD_Fr=E9d=E9ric?= MIME-Version: 1.0 References: <1363373338-5090-1-git-send-email-fred.konrad@greensocs.com> <1363373338-5090-4-git-send-email-fred.konrad@greensocs.com> <20130318085943.GA2476@dhcp-200-207.str.redhat.com> In-Reply-To: <20130318085943.GA2476@dhcp-200-207.str.redhat.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v9 03/10] virtio-blk: don't use pointer for configuration. List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Kevin Wolf Cc: peter.maydell@linaro.org, aliguori@us.ibm.com, mst@redhat.com, mark.burton@greensocs.com, qemu-devel@nongnu.org, Stefan Hajnoczi , cornelia.huck@de.ibm.com, afaerber@suse.de On 18/03/2013 09:59, Kevin Wolf wrote: > Am 15.03.2013 um 19:48 hat fred.konrad@greensocs.com geschrieben: >> From: KONRAD Frederic >> >> 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 >> Reviewed-by: Peter Maydell >> --- >> 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) ? > > Is my understanding of this correct? > > Kevin