[Qemu-devel] [PATCH 1/2] balloon: Don't allow multiple balloon handler registrations

[Qemu-devel] [PATCH 1/2] balloon: Don't allow multiple balloon handler registrations

[Amit Shah]

Multiple balloon devices don't make sense; disallow more than one
registration attempt to register handlers.

Signed-off-by: Amit Shah <amit.shah@redhat.com>
---
 balloon.c |   11 +++++++++--
 balloon.h |    4 ++--
 2 files changed, 11 insertions(+), 4 deletions(-)

diff --git a/balloon.c b/balloon.c
index a938475..cf9e3b2 100644
--- a/balloon.c
+++ b/balloon.c
@@ -36,12 +36,19 @@ static QEMUBalloonEvent *balloon_event_fn;
 static QEMUBalloonStatus *balloon_stat_fn;
 static void *balloon_opaque;
 
-void qemu_add_balloon_handler(QEMUBalloonEvent *event_func,
-                              QEMUBalloonStatus *stat_func, void *opaque)
+int qemu_add_balloon_handler(QEMUBalloonEvent *event_func,
+                             QEMUBalloonStatus *stat_func, void *opaque)
 {
+    if (balloon_event_fn || balloon_stat_fn || balloon_opaque) {
+        /* We're already registered one balloon handler.  How many can
+         * a guest really have?
+         */
+        return -1;
+    }
     balloon_event_fn = event_func;
     balloon_stat_fn = stat_func;
     balloon_opaque = opaque;
+    return 0;
 }
 
 static int qemu_balloon(ram_addr_t target)
diff --git a/balloon.h b/balloon.h
index a6c31d5..3df14e6 100644
--- a/balloon.h
+++ b/balloon.h
@@ -20,8 +20,8 @@ typedef void (QEMUBalloonEvent)(void *opaque, ram_addr_t target);
 typedef void (QEMUBalloonStatus)(void *opaque, MonitorCompletion cb,
                                  void *cb_data);
 
-void qemu_add_balloon_handler(QEMUBalloonEvent *event_func,
-                              QEMUBalloonStatus *stat_func, void *opaque);
+int qemu_add_balloon_handler(QEMUBalloonEvent *event_func,
+			     QEMUBalloonStatus *stat_func, void *opaque);
 
 void monitor_print_balloon(Monitor *mon, const QObject *data);
 int do_info_balloon(Monitor *mon, MonitorCompletion cb, void *opaque);
-- 
1.7.6

[Qemu-devel] [PATCH 2/2] virtio-balloon: Check if balloon registration failed

[Amit Shah]

Multiple balloon registrations are not allowed; check if the
registration with the qemu balloon api succeeded.  If not, fail the
device init.

Signed-off-by: Amit Shah <amit.shah@redhat.com>
---
 hw/virtio-balloon.c |   10 +++++++++-
 hw/virtio-pci.c     |    3 +++
 2 files changed, 12 insertions(+), 1 deletions(-)

diff --git a/hw/virtio-balloon.c b/hw/virtio-balloon.c
index 2ba7e95..304a31a 100644
--- a/hw/virtio-balloon.c
+++ b/hw/virtio-balloon.c
@@ -269,6 +269,7 @@ static int virtio_balloon_load(QEMUFile *f, void *opaque, int version_id)
 VirtIODevice *virtio_balloon_init(DeviceState *dev)
 {
     VirtIOBalloon *s;
+    int ret;
 
     s = (VirtIOBalloon *)virtio_common_init("virtio-balloon",
                                             VIRTIO_ID_BALLOON,
@@ -278,12 +279,19 @@ VirtIODevice *virtio_balloon_init(DeviceState *dev)
     s->vdev.set_config = virtio_balloon_set_config;
     s->vdev.get_features = virtio_balloon_get_features;
 
+    ret = qemu_add_balloon_handler(virtio_balloon_to_target,
+                                   virtio_balloon_stat, s);
+    if (ret < 0) {
+        error_report("Another balloon device already registered");
+        virtio_cleanup(&s->vdev);
+        return NULL;
+    }
+
     s->ivq = virtio_add_queue(&s->vdev, 128, virtio_balloon_handle_output);
     s->dvq = virtio_add_queue(&s->vdev, 128, virtio_balloon_handle_output);
     s->svq = virtio_add_queue(&s->vdev, 128, virtio_balloon_receive_stats);
 
     reset_stats(s);
-    qemu_add_balloon_handler(virtio_balloon_to_target, virtio_balloon_stat, s);
 
     register_savevm(dev, "virtio-balloon", -1, 1,
                     virtio_balloon_save, virtio_balloon_load, s);
diff --git a/hw/virtio-pci.c b/hw/virtio-pci.c
index d685243..ca5f125 100644
--- a/hw/virtio-pci.c
+++ b/hw/virtio-pci.c
@@ -788,6 +788,9 @@ static int virtio_balloon_init_pci(PCIDevice *pci_dev)
     VirtIODevice *vdev;
 
     vdev = virtio_balloon_init(&pci_dev->qdev);
+    if (!vdev) {
+        return -1;
+    }
     virtio_init_pci(proxy, vdev);
     return 0;
 }
-- 
1.7.6

Re: [Qemu-devel] [PATCH 2/2] virtio-balloon: Check if balloon registration failed

[Michael S. Tsirkin]

On Wed, Jul 27, 2011 at 12:31:08PM +0530, Amit Shah wrote:
> Multiple balloon registrations are not allowed; check if the
> registration with the qemu balloon api succeeded.  If not, fail the
> device init.
> 
> Signed-off-by: Amit Shah <amit.shah@redhat.com>
> ---
>  hw/virtio-balloon.c |   10 +++++++++-
>  hw/virtio-pci.c     |    3 +++
>  2 files changed, 12 insertions(+), 1 deletions(-)
> 
> diff --git a/hw/virtio-balloon.c b/hw/virtio-balloon.c
> index 2ba7e95..304a31a 100644
> --- a/hw/virtio-balloon.c
> +++ b/hw/virtio-balloon.c
> @@ -269,6 +269,7 @@ static int virtio_balloon_load(QEMUFile *f, void *opaque, int version_id)
>  VirtIODevice *virtio_balloon_init(DeviceState *dev)
>  {
>      VirtIOBalloon *s;
> +    int ret;
>  
>      s = (VirtIOBalloon *)virtio_common_init("virtio-balloon",
>                                              VIRTIO_ID_BALLOON,
> @@ -278,12 +279,19 @@ VirtIODevice *virtio_balloon_init(DeviceState *dev)
>      s->vdev.set_config = virtio_balloon_set_config;
>      s->vdev.get_features = virtio_balloon_get_features;
>  
> +    ret = qemu_add_balloon_handler(virtio_balloon_to_target,
> +                                   virtio_balloon_stat, s);
> +    if (ret < 0) {
> +        error_report("Another balloon device already registered");

We don't know why it failed at this point. I think
error report should be triggered by qemu_add_balloon_handler,
and this function would just cleanup and exit.


> +        virtio_cleanup(&s->vdev);
> +        return NULL;
> +    }
> +
>      s->ivq = virtio_add_queue(&s->vdev, 128, virtio_balloon_handle_output);
>      s->dvq = virtio_add_queue(&s->vdev, 128, virtio_balloon_handle_output);
>      s->svq = virtio_add_queue(&s->vdev, 128, virtio_balloon_receive_stats);
>  
>      reset_stats(s);
> -    qemu_add_balloon_handler(virtio_balloon_to_target, virtio_balloon_stat, s);
>  
>      register_savevm(dev, "virtio-balloon", -1, 1,
>                      virtio_balloon_save, virtio_balloon_load, s);
> diff --git a/hw/virtio-pci.c b/hw/virtio-pci.c
> index d685243..ca5f125 100644
> --- a/hw/virtio-pci.c
> +++ b/hw/virtio-pci.c
> @@ -788,6 +788,9 @@ static int virtio_balloon_init_pci(PCIDevice *pci_dev)
>      VirtIODevice *vdev;
>  
>      vdev = virtio_balloon_init(&pci_dev->qdev);
> +    if (!vdev) {
> +        return -1;
> +    }
>      virtio_init_pci(proxy, vdev);
>      return 0;
>  }
> -- 
> 1.7.6
> 

Re: [Qemu-devel] [PATCH 2/2] virtio-balloon: Check if balloon registration failed

[Amit Shah]

On (Wed) 27 Jul 2011 [12:06:32], Michael S. Tsirkin wrote:
> On Wed, Jul 27, 2011 at 12:31:08PM +0530, Amit Shah wrote:
> > Multiple balloon registrations are not allowed; check if the
> > registration with the qemu balloon api succeeded.  If not, fail the
> > device init.
> > 
> > Signed-off-by: Amit Shah <amit.shah@redhat.com>
> > ---
> >  hw/virtio-balloon.c |   10 +++++++++-
> >  hw/virtio-pci.c     |    3 +++
> >  2 files changed, 12 insertions(+), 1 deletions(-)
> > 
> > diff --git a/hw/virtio-balloon.c b/hw/virtio-balloon.c
> > index 2ba7e95..304a31a 100644
> > --- a/hw/virtio-balloon.c
> > +++ b/hw/virtio-balloon.c
> > @@ -269,6 +269,7 @@ static int virtio_balloon_load(QEMUFile *f, void *opaque, int version_id)
> >  VirtIODevice *virtio_balloon_init(DeviceState *dev)
> >  {
> >      VirtIOBalloon *s;
> > +    int ret;
> >  
> >      s = (VirtIOBalloon *)virtio_common_init("virtio-balloon",
> >                                              VIRTIO_ID_BALLOON,
> > @@ -278,12 +279,19 @@ VirtIODevice *virtio_balloon_init(DeviceState *dev)
> >      s->vdev.set_config = virtio_balloon_set_config;
> >      s->vdev.get_features = virtio_balloon_get_features;
> >  
> > +    ret = qemu_add_balloon_handler(virtio_balloon_to_target,
> > +                                   virtio_balloon_stat, s);
> > +    if (ret < 0) {
> > +        error_report("Another balloon device already registered");
> 
> We don't know why it failed at this point. I think
> error report should be triggered by qemu_add_balloon_handler,
> and this function would just cleanup and exit.

OK, makes sense.

		Amit