From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:41036) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Tlb5A-0003ji-JQ for qemu-devel@nongnu.org; Thu, 20 Dec 2012 03:02:23 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Tlb59-0002tq-0U for qemu-devel@nongnu.org; Thu, 20 Dec 2012 03:02:20 -0500 Received: from greensocs.com ([87.106.252.221]:38261 helo=s15328186.onlinehome-server.info) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Tlb58-0002st-Np for qemu-devel@nongnu.org; Thu, 20 Dec 2012 03:02:18 -0500 Message-ID: <50D2C600.5050100@greensocs.com> Date: Thu, 20 Dec 2012 09:02:08 +0100 From: =?UTF-8?B?S09OUkFEIEZyw6lkw6lyaWM=?= MIME-Version: 1.0 References: <1355910821-21302-1-git-send-email-fred.konrad@greensocs.com> <1355910821-21302-14-git-send-email-fred.konrad@greensocs.com> In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [RFC PATCH V8 13/15] virtio : Remove the function pointer. List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Blue Swirl Cc: peter.maydell@linaro.org, aliguori@us.ibm.com, e.voevodin@samsung.com, mark.burton@greensocs.com, qemu-devel@nongnu.org, agraf@suse.de, stefanha@redhat.com, cornelia.huck@de.ibm.com, afaerber@suse.de On 19/12/2012 20:50, Blue Swirl wrote: > On Wed, Dec 19, 2012 at 9:53 AM, wrote: >> From: KONRAD Frederic >> >> This remove the function pointer in VirtIODevice, and use only >> VirtioDeviceClass function pointer. It should be applied after all >> the device have been refactored. >> >> Signed-off-by: KONRAD Frederic >> --- >> hw/virtio-blk.c | 5 ----- >> hw/virtio-pci.c | 2 +- >> hw/virtio.c | 41 ++++++++++++++++++++++++++--------------- >> hw/virtio.h | 12 ------------ >> 4 files changed, 27 insertions(+), 33 deletions(-) >> >> diff --git a/hw/virtio-blk.c b/hw/virtio-blk.c >> index 65932fd..fbb829e 100644 >> --- a/hw/virtio-blk.c >> +++ b/hw/virtio-blk.c >> @@ -628,11 +628,6 @@ static int virtio_blk_device_init(VirtIODevice *vdev) >> virtio_init(vdev, "virtio-blk", VIRTIO_ID_BLOCK, >> sizeof(struct virtio_blk_config)); >> >> - vdev->get_config = virtio_blk_update_config; >> - vdev->set_config = virtio_blk_set_config; >> - vdev->get_features = virtio_blk_get_features; >> - vdev->set_status = virtio_blk_set_status; >> - vdev->reset = virtio_blk_reset; >> s->bs = blk->conf.bs; >> s->conf = &blk->conf; >> virtio_blk_set_conf(qdev, blk); >> diff --git a/hw/virtio-pci.c b/hw/virtio-pci.c >> index e3a8276..cdc3473 100644 >> --- a/hw/virtio-pci.c >> +++ b/hw/virtio-pci.c >> @@ -262,7 +262,7 @@ static void virtio_ioport_write(void *opaque, uint32_t addr, uint32_t val) >> case VIRTIO_PCI_GUEST_FEATURES: >> /* Guest does not negotiate properly? We have to assume nothing. */ >> if (val & (1 << VIRTIO_F_BAD_FEATURE)) { >> - val = vdev->bad_features ? vdev->bad_features(vdev) : 0; >> + val = get_virtio_device_bad_features(proxy->bus); >> } >> virtio_set_features(vdev, val); >> break; >> diff --git a/hw/virtio.c b/hw/virtio.c >> index e40fa12..82bf3dd 100644 >> --- a/hw/virtio.c >> +++ b/hw/virtio.c >> @@ -517,10 +517,11 @@ void virtio_update_irq(VirtIODevice *vdev) >> >> void virtio_set_status(VirtIODevice *vdev, uint8_t val) >> { >> + VirtioDeviceClass *k = VIRTIO_DEVICE_GET_CLASS(vdev); >> trace_virtio_set_status(vdev, val); >> >> - if (vdev->set_status) { >> - vdev->set_status(vdev, val); >> + if (k->set_status) { >> + k->set_status(vdev, val); >> } >> vdev->status = val; >> } >> @@ -528,12 +529,14 @@ void virtio_set_status(VirtIODevice *vdev, uint8_t val) >> void virtio_reset(void *opaque) >> { >> VirtIODevice *vdev = opaque; >> + VirtioDeviceClass *k = VIRTIO_DEVICE_GET_CLASS(vdev); >> int i; >> >> virtio_set_status(vdev, 0); >> >> - if (vdev->reset) >> - vdev->reset(vdev); >> + if (k->reset) { >> + k->reset(vdev); >> + } >> >> vdev->guest_features = 0; >> vdev->queue_sel = 0; >> @@ -557,9 +560,10 @@ void virtio_reset(void *opaque) >> >> uint32_t virtio_config_readb(VirtIODevice *vdev, uint32_t addr) >> { >> + VirtioDeviceClass *k = VIRTIO_DEVICE_GET_CLASS(vdev); >> uint8_t val; >> >> - vdev->get_config(vdev, vdev->config); >> + k->get_config(vdev, vdev->config); >> >> if (addr > (vdev->config_len - sizeof(val))) >> return (uint32_t)-1; >> @@ -570,9 +574,10 @@ uint32_t virtio_config_readb(VirtIODevice *vdev, uint32_t addr) >> >> uint32_t virtio_config_readw(VirtIODevice *vdev, uint32_t addr) >> { >> + VirtioDeviceClass *k = VIRTIO_DEVICE_GET_CLASS(vdev); >> uint16_t val; >> >> - vdev->get_config(vdev, vdev->config); >> + k->get_config(vdev, vdev->config); >> >> if (addr > (vdev->config_len - sizeof(val))) >> return (uint32_t)-1; >> @@ -583,9 +588,10 @@ uint32_t virtio_config_readw(VirtIODevice *vdev, uint32_t addr) >> >> uint32_t virtio_config_readl(VirtIODevice *vdev, uint32_t addr) >> { >> + VirtioDeviceClass *k = VIRTIO_DEVICE_GET_CLASS(vdev); >> uint32_t val; >> >> - vdev->get_config(vdev, vdev->config); >> + k->get_config(vdev, vdev->config); >> >> if (addr > (vdev->config_len - sizeof(val))) >> return (uint32_t)-1; >> @@ -596,6 +602,7 @@ uint32_t virtio_config_readl(VirtIODevice *vdev, uint32_t addr) >> >> void virtio_config_writeb(VirtIODevice *vdev, uint32_t addr, uint32_t data) >> { >> + VirtioDeviceClass *k = VIRTIO_DEVICE_GET_CLASS(vdev); >> uint8_t val = data; >> >> if (addr > (vdev->config_len - sizeof(val))) >> @@ -603,12 +610,13 @@ void virtio_config_writeb(VirtIODevice *vdev, uint32_t addr, uint32_t data) >> >> stb_p(vdev->config + addr, val); >> >> - if (vdev->set_config) >> - vdev->set_config(vdev, vdev->config); >> + if (k->set_config) > Missing braces, please use checkpatch.pl. I don't understand, I used checkpatch.pl before submitting but it didn't find it. > >> + k->set_config(vdev, vdev->config); >> } >> >> void virtio_config_writew(VirtIODevice *vdev, uint32_t addr, uint32_t data) >> { >> + VirtioDeviceClass *k = VIRTIO_DEVICE_GET_CLASS(vdev); >> uint16_t val = data; >> >> if (addr > (vdev->config_len - sizeof(val))) >> @@ -616,12 +624,13 @@ void virtio_config_writew(VirtIODevice *vdev, uint32_t addr, uint32_t data) >> >> stw_p(vdev->config + addr, val); >> >> - if (vdev->set_config) >> - vdev->set_config(vdev, vdev->config); >> + if (k->set_config) > Ditto. > >> + k->set_config(vdev, vdev->config); >> } >> >> void virtio_config_writel(VirtIODevice *vdev, uint32_t addr, uint32_t data) >> { >> + VirtioDeviceClass *k = VIRTIO_DEVICE_GET_CLASS(vdev); >> uint32_t val = data; >> >> if (addr > (vdev->config_len - sizeof(val))) >> @@ -629,8 +638,9 @@ void virtio_config_writel(VirtIODevice *vdev, uint32_t addr, uint32_t data) >> >> stl_p(vdev->config + addr, val); >> >> - if (vdev->set_config) >> - vdev->set_config(vdev, vdev->config); >> + if (k->set_config) { >> + k->set_config(vdev, vdev->config); >> + } But it found this one, that's why I added brace here. >> } >> >> void virtio_queue_set_addr(VirtIODevice *vdev, int n, hwaddr addr) >> @@ -799,13 +809,14 @@ void virtio_save(VirtIODevice *vdev, QEMUFile *f) >> >> int virtio_set_features(VirtIODevice *vdev, uint32_t val) >> { >> + VirtioDeviceClass *k = VIRTIO_DEVICE_GET_CLASS(vdev); >> uint32_t supported_features = >> vdev->binding->get_features(vdev->binding_opaque); >> bool bad = (val & ~supported_features) != 0; >> >> val &= supported_features; >> - if (vdev->set_features) { >> - vdev->set_features(vdev, val); >> + if (k->set_features) { >> + k->set_features(vdev, val); >> } >> vdev->guest_features = val; >> return bad ? -1 : 0; >> diff --git a/hw/virtio.h b/hw/virtio.h >> index 98596a9..857fd78 100644 >> --- a/hw/virtio.h >> +++ b/hw/virtio.h >> @@ -128,18 +128,6 @@ struct VirtIODevice >> void *config; >> uint16_t config_vector; >> int nvectors; >> - /* >> - * Will be removed ( at the end of the series ) as we have it in >> - * VirtioDeviceClass. >> - */ >> - uint32_t (*get_features)(VirtIODevice *vdev, uint32_t requested_features); >> - uint32_t (*bad_features)(VirtIODevice *vdev); >> - void (*set_features)(VirtIODevice *vdev, uint32_t val); >> - void (*get_config)(VirtIODevice *vdev, uint8_t *config); >> - void (*set_config)(VirtIODevice *vdev, const uint8_t *config); >> - void (*reset)(VirtIODevice *vdev); >> - void (*set_status)(VirtIODevice *vdev, uint8_t val); >> - /***/ >> VirtQueue *vq; >> const VirtIOBindings *binding; >> void *binding_opaque; >> -- >> 1.7.11.7 >> >>