diff mbox

Disable virtio-balloon memory stats interface

Message ID 1283195871.2937.26.camel@aglitke
State New
Headers show

Commit Message

Adam Litke Aug. 30, 2010, 7:17 p.m. UTC
Just got back from vacation and saw this thread.  I agree with Anthony
that the best thing to do is disable the memory stats interface for
0.13.  We need to fix the underlying problems in qemu with respect to
asynchronous commands first, then we can look at re-enabling the
feature.

The virtio feature negotiation mechanism allows the feature to be
disabled by commenting out a single line of code.  This late in the 0.13
release cycle, this is the safest option.  We can refactor/remove code
in the development tree as needed.

On Mon, 2010-08-30 at 17:01 +0200, Markus Armbruster wrote:
> Anyone care to submit a patch?

Sure...

[PATCH] Disable virtio-balloon memory stats interface

The addition of memory stats reporting to the virtio balloon causes
the 'info balloon' command to become asynchronous.  This is a regression
because management tools that consume this command were not designed to
handle lost or delayed responses.

To fix this regression, the virtio balloon memory stats feature is being
disabled in qemu-0.13.

Signed-off-by: Adam Litke <agl@us.ibm.com>

Comments

Amit Shah Aug. 31, 2010, 3:42 a.m. UTC | #1
On (Mon) Aug 30 2010 [14:17:51], Adam Litke wrote:
> The addition of memory stats reporting to the virtio balloon causes
> the 'info balloon' command to become asynchronous.  This is a regression
> because management tools that consume this command were not designed to
> handle lost or delayed responses.
> 
> To fix this regression, the virtio balloon memory stats feature is being
> disabled in qemu-0.13.
> 
> Signed-off-by: Adam Litke <agl@us.ibm.com>
> 
> diff --git a/hw/virtio-balloon.c b/hw/virtio-balloon.c
> index 9fe3886..2d80382 100644
> --- a/hw/virtio-balloon.c
> +++ b/hw/virtio-balloon.c
> @@ -190,7 +190,18 @@ static void virtio_balloon_set_config(VirtIODevice *vdev,
>  
>  static uint32_t virtio_balloon_get_features(VirtIODevice *vdev, uint32_t f)
>  {
> -    f |= (1 << VIRTIO_BALLOON_F_STATS_VQ);
> +    /*
> +     * The addition of memory stats reporting to the virtio balloon causes
> +     * the 'info balloon' command to become asynchronous.  This is a regression
> +     * because management tools that consume this command were not designed to
> +     * handle lost or delayed responses.

Rather, the monitor now gets stuck when it originally didn't.

> +     *
> +     * To fix this regression, the virtio balloon memory stats feature is being
> +     * disabled in qemu-0.13.
> +     *
> +     * -aglitke
> +     */
> +    /* f |= (1 << VIRTIO_BALLOON_F_STATS_VQ); */
>      return f;
>  }

Acked-by: Amit Shah <amit.shah@redhat.com>

		Amit
Amit Shah Sept. 8, 2010, 10:57 a.m. UTC | #2
On (Tue) Aug 31 2010 [09:12:39], Amit Shah wrote:
> On (Mon) Aug 30 2010 [14:17:51], Adam Litke wrote:
> > The addition of memory stats reporting to the virtio balloon causes
> > the 'info balloon' command to become asynchronous.  This is a regression
> > because management tools that consume this command were not designed to
> > handle lost or delayed responses.
> > 
> > To fix this regression, the virtio balloon memory stats feature is being
> > disabled in qemu-0.13.
> > 
> > Signed-off-by: Adam Litke <agl@us.ibm.com>
> > 
> > diff --git a/hw/virtio-balloon.c b/hw/virtio-balloon.c
> > index 9fe3886..2d80382 100644
> > --- a/hw/virtio-balloon.c
> > +++ b/hw/virtio-balloon.c
> > @@ -190,7 +190,18 @@ static void virtio_balloon_set_config(VirtIODevice *vdev,
> >  
> >  static uint32_t virtio_balloon_get_features(VirtIODevice *vdev, uint32_t f)
> >  {
> > -    f |= (1 << VIRTIO_BALLOON_F_STATS_VQ);
> > +    /*
> > +     * The addition of memory stats reporting to the virtio balloon causes
> > +     * the 'info balloon' command to become asynchronous.  This is a regression
> > +     * because management tools that consume this command were not designed to
> > +     * handle lost or delayed responses.
> 
> Rather, the monitor now gets stuck when it originally didn't.

Care to update the comment and the commit message and send as a
top-level patch (and also for 0.13)?

Thanks

		Amit
diff mbox

Patch

diff --git a/hw/virtio-balloon.c b/hw/virtio-balloon.c
index 9fe3886..2d80382 100644
--- a/hw/virtio-balloon.c
+++ b/hw/virtio-balloon.c
@@ -190,7 +190,18 @@  static void virtio_balloon_set_config(VirtIODevice *vdev,
 
 static uint32_t virtio_balloon_get_features(VirtIODevice *vdev, uint32_t f)
 {
-    f |= (1 << VIRTIO_BALLOON_F_STATS_VQ);
+    /*
+     * The addition of memory stats reporting to the virtio balloon causes
+     * the 'info balloon' command to become asynchronous.  This is a regression
+     * because management tools that consume this command were not designed to
+     * handle lost or delayed responses.
+     *
+     * To fix this regression, the virtio balloon memory stats feature is being
+     * disabled in qemu-0.13.
+     *
+     * -aglitke
+     */
+    /* f |= (1 << VIRTIO_BALLOON_F_STATS_VQ); */
     return f;
 }