Re: [Qemu-devel] [RFC PATCH V8 13/15] virtio : Remove the function pointer.

[KONRAD Frédéric]

On 19/12/2012 20:50, Blue Swirl wrote:
> On Wed, Dec 19, 2012 at 9:53 AM,  <address@hidden> wrote:
> > From: KONRAD Frederic <address@hidden>
> >
> > 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 <address@hidden>
> > ---
> >   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