Patchwork [v3,8/8] virtio-rng: cleanup: use QOM casts.

login
register
mail settings
Submitter fred.konrad@greensocs.com
Date April 14, 2013, 1:01 p.m.
Message ID <1365944470-13837-9-git-send-email-fred.konrad@greensocs.com>
Download mbox | patch
Permalink /patch/236439/
State New
Headers show

Comments

fred.konrad@greensocs.com - April 14, 2013, 1:01 p.m.
From: KONRAD Frederic <fred.konrad@greensocs.com>

As the virtio-rng-pci and virtio-rng-s390 are switched to the new API,
we can use QOM casts.

Signed-off-by: KONRAD Frederic <fred.konrad@greensocs.com>
---
 hw/virtio/virtio-rng.c         | 31 +++++++++++++++++--------------
 include/hw/virtio/virtio-rng.h |  2 +-
 2 files changed, 18 insertions(+), 15 deletions(-)
Andreas Färber - April 15, 2013, 1:34 p.m.
Am 14.04.2013 15:01, schrieb fred.konrad@greensocs.com:
> From: KONRAD Frederic <fred.konrad@greensocs.com>
> 
> As the virtio-rng-pci and virtio-rng-s390 are switched to the new API,

and virtio-rng-ccw ;)

> we can use QOM casts.
> 
> Signed-off-by: KONRAD Frederic <fred.konrad@greensocs.com>
> ---
>  hw/virtio/virtio-rng.c         | 31 +++++++++++++++++--------------
>  include/hw/virtio/virtio-rng.h |  2 +-
>  2 files changed, 18 insertions(+), 15 deletions(-)

Thanks,

Reviewed-by: Andreas Färber <afaerber@suse.de>

I was surprised to see FOO(opaque) since we usually try to avoid it for
performance reasons, but it's not forbidden either.
Also, leaving the variable name as "s" would've spared a few lines but
so what. :)

Regards,
Andreas
fred.konrad@greensocs.com - April 15, 2013, 2:33 p.m.
On 15/04/2013 15:34, Andreas Färber wrote:
> Am 14.04.2013 15:01, schrieb fred.konrad@greensocs.com:
>> From: KONRAD Frederic <fred.konrad@greensocs.com>
>>
>> As the virtio-rng-pci and virtio-rng-s390 are switched to the new API,
> and virtio-rng-ccw ;)
>
>> we can use QOM casts.
>>
>> Signed-off-by: KONRAD Frederic <fred.konrad@greensocs.com>
>> ---
>>   hw/virtio/virtio-rng.c         | 31 +++++++++++++++++--------------
>>   include/hw/virtio/virtio-rng.h |  2 +-
>>   2 files changed, 18 insertions(+), 15 deletions(-)
> Thanks,
>
> Reviewed-by: Andreas Färber <afaerber@suse.de>
>
> I was surprised to see FOO(opaque) since we usually try to avoid it for
> performance reasons, but it's not forbidden either.
True, I had taken that in account but forget this series :/.

Is it better to change that?

Thanks,
Fred
> Also, leaving the variable name as "s" would've spared a few lines but
> so what. :)
>
> Regards,
> Andreas
>
Andreas Färber - April 15, 2013, 5:06 p.m.
Am 15.04.2013 16:33, schrieb KONRAD Frédéric:
> On 15/04/2013 15:34, Andreas Färber wrote:
>> Am 14.04.2013 15:01, schrieb fred.konrad@greensocs.com:
>>> From: KONRAD Frederic <fred.konrad@greensocs.com>
>>>
>>> As the virtio-rng-pci and virtio-rng-s390 are switched to the new API,
>> and virtio-rng-ccw ;)
>>
>>> we can use QOM casts.
>>>
>>> Signed-off-by: KONRAD Frederic <fred.konrad@greensocs.com>
>>> ---
>>>   hw/virtio/virtio-rng.c         | 31 +++++++++++++++++--------------
>>>   include/hw/virtio/virtio-rng.h |  2 +-
>>>   2 files changed, 18 insertions(+), 15 deletions(-)
>> Thanks,
>>
>> Reviewed-by: Andreas Färber <afaerber@suse.de>
>>
>> I was surprised to see FOO(opaque) since we usually try to avoid it for
>> performance reasons, but it's not forbidden either.
> True, I had taken that in account but forget this series :/.
> 
> Is it better to change that?

I didn't spot an obvious issue, but you know the code better than me and
should take a second look: load/save hooks should be no problem, but
there were some opaques I didn't know where they are coming from.

Andreas

> 
> Thanks,
> Fred
>> Also, leaving the variable name as "s" would've spared a few lines but
>> so what. :)
>>
>> Regards,
>> Andreas
>>
>

Patch

diff --git a/hw/virtio/virtio-rng.c b/hw/virtio/virtio-rng.c
index 088f8fe..1acbe18 100644
--- a/hw/virtio/virtio-rng.c
+++ b/hw/virtio/virtio-rng.c
@@ -18,8 +18,9 @@ 
 
 static bool is_guest_ready(VirtIORNG *vrng)
 {
+    VirtIODevice *vdev = VIRTIO_DEVICE(vrng);
     if (virtio_queue_ready(vrng->vq)
-        && (vrng->vdev.status & VIRTIO_CONFIG_S_DRIVER_OK)) {
+        && (vdev->status & VIRTIO_CONFIG_S_DRIVER_OK)) {
         return true;
     }
     return false;
@@ -38,7 +39,8 @@  static void virtio_rng_process(VirtIORNG *vrng);
 /* Send data from a char device over to the guest */
 static void chr_read(void *opaque, const void *buf, size_t size)
 {
-    VirtIORNG *vrng = opaque;
+    VirtIORNG *vrng = VIRTIO_RNG(opaque);
+    VirtIODevice *vdev = VIRTIO_DEVICE(opaque);
     VirtQueueElement elem;
     size_t len;
     int offset;
@@ -60,7 +62,7 @@  static void chr_read(void *opaque, const void *buf, size_t size)
 
         virtqueue_push(vrng->vq, &elem, len);
     }
-    virtio_notify(&vrng->vdev, vrng->vq);
+    virtio_notify(vdev, vrng->vq);
 }
 
 static void virtio_rng_process(VirtIORNG *vrng)
@@ -86,7 +88,7 @@  static void virtio_rng_process(VirtIORNG *vrng)
 
 static void handle_input(VirtIODevice *vdev, VirtQueue *vq)
 {
-    VirtIORNG *vrng = DO_UPCAST(VirtIORNG, vdev, vdev);
+    VirtIORNG *vrng = VIRTIO_RNG(vdev);
     virtio_rng_process(vrng);
 }
 
@@ -97,19 +99,20 @@  static uint32_t get_features(VirtIODevice *vdev, uint32_t f)
 
 static void virtio_rng_save(QEMUFile *f, void *opaque)
 {
-    VirtIORNG *vrng = opaque;
+    VirtIODevice *vdev = VIRTIO_DEVICE(opaque);
 
-    virtio_save(&vrng->vdev, f);
+    virtio_save(vdev, f);
 }
 
 static int virtio_rng_load(QEMUFile *f, void *opaque, int version_id)
 {
-    VirtIORNG *vrng = opaque;
+    VirtIORNG *vrng = VIRTIO_RNG(opaque);
+    VirtIODevice *vdev = VIRTIO_DEVICE(vrng);
 
     if (version_id != 1) {
         return -EINVAL;
     }
-    virtio_load(&vrng->vdev, f);
+    virtio_load(vdev, f);
 
     /* We may have an element ready but couldn't process it due to a quota
      * limit.  Make sure to try again after live migration when the quota may
@@ -122,12 +125,12 @@  static int virtio_rng_load(QEMUFile *f, void *opaque, int version_id)
 
 static void check_rate_limit(void *opaque)
 {
-    VirtIORNG *s = opaque;
+    VirtIORNG *vrng = VIRTIO_RNG(opaque);
 
-    s->quota_remaining = s->conf.max_bytes;
-    virtio_rng_process(s);
-    qemu_mod_timer(s->rate_limit_timer,
-                   qemu_get_clock_ms(vm_clock) + s->conf.period_ms);
+    vrng->quota_remaining = vrng->conf.max_bytes;
+    virtio_rng_process(vrng);
+    qemu_mod_timer(vrng->rate_limit_timer,
+                   qemu_get_clock_ms(vm_clock) + vrng->conf.period_ms);
 }
 
 static int virtio_rng_device_init(VirtIODevice *vdev)
@@ -166,7 +169,7 @@  static int virtio_rng_device_init(VirtIODevice *vdev)
 
     vrng->vq = virtio_add_queue(vdev, 8, handle_input);
 
-    vrng->vdev.get_features = get_features;
+    vdev->get_features = get_features;
 
     assert(vrng->conf.max_bytes <= INT64_MAX);
     vrng->quota_remaining = vrng->conf.max_bytes;
diff --git a/include/hw/virtio/virtio-rng.h b/include/hw/virtio/virtio-rng.h
index ac04ad5..5e007c6 100644
--- a/include/hw/virtio/virtio-rng.h
+++ b/include/hw/virtio/virtio-rng.h
@@ -30,7 +30,7 @@  struct VirtIORNGConf {
 };
 
 typedef struct VirtIORNG {
-    VirtIODevice vdev;
+    VirtIODevice parent_obj;
 
     /* Only one vq - guest puts buffer(s) on it when it needs entropy */
     VirtQueue *vq;