diff mbox

virtio-balloon: fix dynamic properties.

Message ID 1365719939-7169-1-git-send-email-fred.konrad@greensocs.com
State New
Headers show

Commit Message

fred.konrad@greensocs.com April 11, 2013, 10:38 p.m. UTC 
From: KONRAD Frederic <fred.konrad@greensocs.com>

To keep compatibility with the old virtio-balloon-x, add the dynamic properties
to virtio-balloon-pci and virtio-balloon-ccw.

Cc: Luiz Capitulino <lcapitulino@redhat.com>
Signed-off-by: KONRAD Frederic <fred.konrad@greensocs.com>
---
 hw/s390x/virtio-ccw.c              |  8 ++++++++
 hw/virtio/virtio-balloon.c         | 16 ++++++++--------
 hw/virtio/virtio-pci.c             |  8 ++++++++
 include/hw/virtio/virtio-balloon.h |  9 +++++++++
 4 files changed, 33 insertions(+), 8 deletions(-)

Comments

Peter Maydell April 12, 2013, 8:29 a.m. UTC | #1 
On 11 April 2013 23:38,  <fred.konrad@greensocs.com> wrote:
> From: KONRAD Frederic <fred.konrad@greensocs.com>
>
> To keep compatibility with the old virtio-balloon-x, add the dynamic properties
> to virtio-balloon-pci and virtio-balloon-ccw.

Does the approach I suggested on IRC where virtio-balloon-pci's
property set/get callbacks just set/get the property on
virtio-balloon via the public interface not work? Having to
expose virtio-balloon's callback functions seems a bit of
an encapsulation violation...

thanks
-- PMM
fred.konrad@greensocs.com April 12, 2013, 8:36 a.m. UTC | #2 
On 12/04/2013 10:29, Peter Maydell wrote:
> On 11 April 2013 23:38,  <fred.konrad@greensocs.com> wrote:
>> From: KONRAD Frederic <fred.konrad@greensocs.com>
>>
>> To keep compatibility with the old virtio-balloon-x, add the dynamic properties
>> to virtio-balloon-pci and virtio-balloon-ccw.
> Does the approach I suggested on IRC where virtio-balloon-pci's
> property set/get callbacks just set/get the property on
> virtio-balloon via the public interface not work? Having to
> expose virtio-balloon's callback functions seems a bit of
> an encapsulation violation...
>
> thanks
> -- PMM
Oh, I didn't understand that like that.

What do you mean by the public interface?

I thought you wanted two set/get callbacks which call directly 
virtio-balloon callbacks.

Thanks,
Fred
Peter Maydell April 12, 2013, 9:14 a.m. UTC | #3 
On 12 April 2013 09:36, KONRAD Frédéric <fred.konrad@greensocs.com> wrote:
> On 12/04/2013 10:29, Peter Maydell wrote:
>> Does the approach I suggested on IRC where virtio-balloon-pci's
>> property set/get callbacks just set/get the property on
>> virtio-balloon via the public interface not work? Having to
>> expose virtio-balloon's callback functions seems a bit of
>> an encapsulation violation...

> Oh, I didn't understand that like that.
>
> What do you mean by the public interface?

I mean the interface that any user of an object should
use to access properties, ie object_property_set()
and object_property_get(). Something like:

static void balloon_pci_fwd_get(Object *obj,
     struct Visitor *v, void *opaque, const char *name, Error **errp)
{
    VirtIOBalloonPCI *dev = VIRTIO_BALLOON_PCI(obj);
    object_property_get(OBJECT(&dev->vdev), v, name, errp);
}

Ditto for fwd_set; note that you can use the same accessors
for any property you need to forward to the underlying
virtio-balloon-device. Untested :-)

-- PMM
fred.konrad@greensocs.com April 12, 2013, 9:19 a.m. UTC | #4 
On 12/04/2013 11:14, Peter Maydell wrote:
> On 12 April 2013 09:36, KONRAD Frédéric <fred.konrad@greensocs.com> wrote:
>> On 12/04/2013 10:29, Peter Maydell wrote:
>>> Does the approach I suggested on IRC where virtio-balloon-pci's
>>> property set/get callbacks just set/get the property on
>>> virtio-balloon via the public interface not work? Having to
>>> expose virtio-balloon's callback functions seems a bit of
>>> an encapsulation violation...
>> Oh, I didn't understand that like that.
>>
>> What do you mean by the public interface?
> I mean the interface that any user of an object should
> use to access properties, ie object_property_set()
> and object_property_get(). Something like:
>
> static void balloon_pci_fwd_get(Object *obj,
>       struct Visitor *v, void *opaque, const char *name, Error **errp)
> {
>      VirtIOBalloonPCI *dev = VIRTIO_BALLOON_PCI(obj);
>      object_property_get(OBJECT(&dev->vdev), v, name, errp);
> }
>
> Ditto for fwd_set; note that you can use the same accessors
> for any property you need to forward to the underlying
> virtio-balloon-device. Untested :-)
>
> -- PMM
Ok understood,
will fix that.

Thanks,
Fred
fred.konrad@greensocs.com April 12, 2013, 9:33 a.m. UTC | #5 
On 12/04/2013 11:14, Peter Maydell wrote:
> On 12 April 2013 09:36, KONRAD Frédéric <fred.konrad@greensocs.com> wrote:
>> On 12/04/2013 10:29, Peter Maydell wrote:
>>> Does the approach I suggested on IRC where virtio-balloon-pci's
>>> property set/get callbacks just set/get the property on
>>> virtio-balloon via the public interface not work? Having to
>>> expose virtio-balloon's callback functions seems a bit of
>>> an encapsulation violation...
>> Oh, I didn't understand that like that.
>>
>> What do you mean by the public interface?
> I mean the interface that any user of an object should
> use to access properties, ie object_property_set()
> and object_property_get(). Something like:
>
> static void balloon_pci_fwd_get(Object *obj,
>       struct Visitor *v, void *opaque, const char *name, Error **errp)
> {
>      VirtIOBalloonPCI *dev = VIRTIO_BALLOON_PCI(obj);
>      object_property_get(OBJECT(&dev->vdev), v, name, errp);
> }
>
> Ditto for fwd_set; note that you can use the same accessors
> for any property you need to forward to the underlying
> virtio-balloon-device. Untested :-)
>
> -- PMM
Note, I'm having a kind of similar issue with virtio-rng:

virtio-rng-pci have actually a child device named "default-backend".

The logic wants that default-backend is a child of virtio-rng-device but 
as we
want to keep compatibility.

And it's not possible to have this default-backend be a child of 
virtio-rng-pci and virtio-rng-device at the same time.
diff mbox

Patch

diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c
index 4dec0cd..ea5ed6d 100644
--- a/hw/s390x/virtio-ccw.c
+++ b/hw/s390x/virtio-ccw.c
@@ -626,6 +626,14 @@  static void virtio_ccw_balloon_instance_init(Object *obj)
     VirtIOBalloonCcw *dev = VIRTIO_BALLOON_CCW(obj);
     object_initialize(OBJECT(&dev->vdev), TYPE_VIRTIO_BALLOON);
     object_property_add_child(obj, "virtio-backend", OBJECT(&dev->vdev), NULL);
+
+    object_property_add(obj, "guest-stats", "guest statistics",
+                        balloon_stats_get_all, NULL, NULL, &dev->vdev, NULL);
+
+    object_property_add(obj, "guest-stats-polling-interval", "int",
+                        balloon_stats_get_poll_interval,
+                        balloon_stats_set_poll_interval,
+                        NULL, &dev->vdev, NULL);
 }
 
 static int virtio_ccw_scsi_init(VirtioCcwDevice *ccw_dev)
diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c
index c2c446e..2e2ea30 100644
--- a/hw/virtio/virtio-balloon.c
+++ b/hw/virtio/virtio-balloon.c
@@ -105,8 +105,8 @@  static void balloon_stats_poll_cb(void *opaque)
     virtio_notify(vdev, s->svq);
 }
 
-static void balloon_stats_get_all(Object *obj, struct Visitor *v,
-                                  void *opaque, const char *name, Error **errp)
+void balloon_stats_get_all(Object *obj, struct Visitor *v, void *opaque,
+                           const char *name, Error **errp)
 {
     VirtIOBalloon *s = opaque;
     int i;
@@ -129,17 +129,17 @@  static void balloon_stats_get_all(Object *obj, struct Visitor *v,
     visit_end_struct(v, errp);
 }
 
-static void balloon_stats_get_poll_interval(Object *obj, struct Visitor *v,
-                                            void *opaque, const char *name,
-                                            Error **errp)
+void balloon_stats_get_poll_interval(Object *obj, struct Visitor *v,
+                                     void *opaque, const char *name,
+                                     Error **errp)
 {
     VirtIOBalloon *s = opaque;
     visit_type_int(v, &s->stats_poll_interval, name, errp);
 }
 
-static void balloon_stats_set_poll_interval(Object *obj, struct Visitor *v,
-                                            void *opaque, const char *name,
-                                            Error **errp)
+void balloon_stats_set_poll_interval(Object *obj, struct Visitor *v,
+                                     void *opaque, const char *name,
+                                     Error **errp)
 {
     VirtIOBalloon *s = opaque;
     int64_t value;
diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
index 2b22588..9a3d291 100644
--- a/hw/virtio/virtio-pci.c
+++ b/hw/virtio/virtio-pci.c
@@ -1450,6 +1450,14 @@  static void virtio_balloon_pci_instance_init(Object *obj)
     VirtIOBalloonPCI *dev = VIRTIO_BALLOON_PCI(obj);
     object_initialize(OBJECT(&dev->vdev), TYPE_VIRTIO_BALLOON);
     object_property_add_child(obj, "virtio-backend", OBJECT(&dev->vdev), NULL);
+
+    object_property_add(obj, "guest-stats", "guest statistics",
+                        balloon_stats_get_all, NULL, NULL, &dev->vdev, NULL);
+
+    object_property_add(obj, "guest-stats-polling-interval", "int",
+                        balloon_stats_get_poll_interval,
+                        balloon_stats_set_poll_interval,
+                        NULL, &dev->vdev, NULL);
 }
 
 static const TypeInfo virtio_balloon_pci_info = {
diff --git a/include/hw/virtio/virtio-balloon.h b/include/hw/virtio/virtio-balloon.h
index 3b459bb..023d0f0 100644
--- a/include/hw/virtio/virtio-balloon.h
+++ b/include/hw/virtio/virtio-balloon.h
@@ -69,4 +69,13 @@  typedef struct VirtIOBalloon {
     int64_t stats_poll_interval;
 } VirtIOBalloon;
 
+void balloon_stats_get_all(Object *obj, struct Visitor *v, void *opaque,
+                           const char *name, Error **errp);
+void balloon_stats_get_poll_interval(Object *obj, struct Visitor *v,
+                                     void *opaque, const char *name,
+                                     Error **errp);
+void balloon_stats_set_poll_interval(Object *obj, struct Visitor *v,
+                                     void *opaque, const char *name,
+                                     Error **errp);
+
 #endif