[Qemu-devel] [PATCH] Fix hanging user monitor when using balloon command

[Qemu-devel] [PATCH] Fix hanging user monitor when using balloon command

[Adam Litke]

Arghh... Adding missing S-O-B

Hi Anthony.  I wonder if there was a problem when importing my async
command handler patchset.  Since the 'balloon' command completes
immediately, it must call the completion callback before returning.
That call was missing but is added by the patch below.

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

diff --git a/monitor.c b/monitor.c
index ae125b8..f94794d 100644
--- a/monitor.c
+++ b/monitor.c
@@ -2258,6 +2258,7 @@ static int do_balloon(Monitor *mon, const QDict *params,
         return -1;
     }
 
+    cb(opaque, NULL);
     return 0;
 }

-- 
Thanks,
Adam

Re: [Qemu-devel] [PATCH] Fix hanging user monitor when using balloon command

[Daniel P. Berrange]

On Fri, Feb 12, 2010 at 02:55:56PM -0600, Adam Litke wrote:
> Arghh... Adding missing S-O-B
> 
> Hi Anthony.  I wonder if there was a problem when importing my async
> command handler patchset.  Since the 'balloon' command completes
> immediately, it must call the completion callback before returning.
> That call was missing but is added by the patch below.
> 
> Signed-off-by: Adam Litke <agl@us.ibm.com>
> 
> diff --git a/monitor.c b/monitor.c
> index ae125b8..f94794d 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -2258,6 +2258,7 @@ static int do_balloon(Monitor *mon, const QDict *params,
>          return -1;
>      }
>  
> +    cb(opaque, NULL);
>      return 0;
>  }
> 
> -- 

Can we get this patch applied to GIT - without it, QEMU just
hangs under libvirt immediately, since we nearly always issue
a balloon command at start

Daniel
-- 
|: Red Hat, Engineering, London   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

Re: [Qemu-devel] [PATCH] Fix hanging user monitor when using balloon command

[Anthony Liguori]

On 02/12/2010 02:55 PM, Adam Litke wrote:
> Arghh... Adding missing S-O-B
>
> Hi Anthony.  I wonder if there was a problem when importing my async
> command handler patchset.  Since the 'balloon' command completes
> immediately, it must call the completion callback before returning.
> That call was missing but is added by the patch below.
>
> Signed-off-by: Adam Litke<agl@us.ibm.com>
>    

Applied.  Thanks.

Regards,

Anthony Liguori
> diff --git a/monitor.c b/monitor.c
> index ae125b8..f94794d 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -2258,6 +2258,7 @@ static int do_balloon(Monitor *mon, const QDict *params,
>           return -1;
>       }
>
> +    cb(opaque, NULL);
>       return 0;
>   }
>
>    

Re: [Qemu-devel] [PATCH] Fix hanging user monitor when using balloon command

[Adam Litke]

On Fri, 2010-02-19 at 15:47 -0600, Anthony Liguori wrote:
> On 02/12/2010 02:55 PM, Adam Litke wrote:
> > Arghh... Adding missing S-O-B
> >
> > Hi Anthony.  I wonder if there was a problem when importing my async
> > command handler patchset.  Since the 'balloon' command completes
> > immediately, it must call the completion callback before returning.
> > That call was missing but is added by the patch below.
> >
> > Signed-off-by: Adam Litke<agl@us.ibm.com>
> >    
> 
> Applied.  Thanks.

This patch application failed.  My patch adds a cb() call in
do_balloon(), but the change in git has added the cb() call to
do_info_balloon().  That is causing qemu segfaults.  Applying the
following should correct the damage.  Thanks.

Fix for commit: 5c366a8a3d7ac71beda8499caa815cb3ea95eb58

The cb() call is needed in do_balloon(), not do_info_balloon().

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

diff --git a/monitor.c b/monitor.c
index b1a6edc..c7d2117 100644
--- a/monitor.c
+++ b/monitor.c
@@ -2309,7 +2309,6 @@ static int do_info_balloon(Monitor *mon, MonitorCompletion cb, void *opaque)
         return -1;
     }
 
-    cb(opaque, NULL);
     return 0;
 }
 
@@ -2332,6 +2331,7 @@ static int do_balloon(Monitor *mon, const QDict *params,
         return -1;
     }
 
+    cb(opaque, NULL);
     return 0;
 }
 



-- 
Thanks,
Adam

Re: [Qemu-devel] [PATCH] Fix hanging user monitor when using balloon command

[Luiz Capitulino]

On Mon, 22 Feb 2010 10:51:20 -0600
Adam Litke <agl@us.ibm.com> wrote:

> On Fri, 2010-02-19 at 15:47 -0600, Anthony Liguori wrote:
> > On 02/12/2010 02:55 PM, Adam Litke wrote:
> > > Arghh... Adding missing S-O-B
> > >
> > > Hi Anthony.  I wonder if there was a problem when importing my async
> > > command handler patchset.  Since the 'balloon' command completes
> > > immediately, it must call the completion callback before returning.
> > > That call was missing but is added by the patch below.
> > >
> > > Signed-off-by: Adam Litke<agl@us.ibm.com>
> > >    
> > 
> > Applied.  Thanks.
> 
> This patch application failed.  My patch adds a cb() call in
> do_balloon(), but the change in git has added the cb() call to
> do_info_balloon().  That is causing qemu segfaults.  Applying the
> following should correct the damage.  Thanks.
> 
> Fix for commit: 5c366a8a3d7ac71beda8499caa815cb3ea95eb58
> 
> The cb() call is needed in do_balloon(), not do_info_balloon().

 It won't segfault on QMP, but will print a duplicated response on
query-balloon and no success response on balloon.

 This patch fixes both. One question, though:

> @@ -2332,6 +2331,7 @@ static int do_balloon(Monitor *mon, const QDict *params,
>          return -1;
>      }
>  
> +    cb(opaque, NULL);
>      return 0;
>  }

 Can't (or shouldn't) this be called by common code upon handler
completion?

Re: [Qemu-devel] [PATCH] Fix hanging user monitor when using balloon command

[Adam Litke]

On Fri, 2010-02-26 at 17:26 -0300, Luiz Capitulino wrote:
>  This patch fixes both. One question, though:
> 
> > @@ -2332,6 +2331,7 @@ static int do_balloon(Monitor *mon, const QDict *params,
> >          return -1;
> >      }
> >  
> > +    cb(opaque, NULL);
> >      return 0;
> >  }
> 
>  Can't (or shouldn't) this be called by common code upon handler
> completion?

The explicit cb() call is only needed for synchronous commands that want
to use this API.  For asynchronous commands, the top-half (ie.
do_info_balloon) returns right away and the callback is called when the
asynchronous command completes (in hw/virtio-balloon.c).  Since
do_balloon() is completely finished when it returns, it has to manually
call cb().

If we decide to merge all commands into this new API in the future, then
we could have common code call the cb() for all synchronous commands.
For now, I just implemented do_balloon() this way as a proof of concept
to show that synchronous commands can use the API. 

-- 
Thanks,
Adam

Re: [Qemu-devel] [PATCH] Fix hanging user monitor when using balloon command

[Anthony Liguori]

On 02/22/2010 10:51 AM, Adam Litke wrote:
> This patch application failed. My patch adds a cb() call in
> do_balloon(), but the change in git has added the cb() call to
> do_info_balloon().  That is causing qemu segfaults.  Applying the
> following should correct the damage.  Thanks.
>
> Fix for commit: 5c366a8a3d7ac71beda8499caa815cb3ea95eb58
>
> The cb() call is needed in do_balloon(), not do_info_balloon().
>
> Signed-off-by: Adam Litke<agl@us.ibm.com>
>    

Applied.  Thanks.

Regards,

Anthony Liguori

> diff --git a/monitor.c b/monitor.c
> index b1a6edc..c7d2117 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -2309,7 +2309,6 @@ static int do_info_balloon(Monitor *mon, MonitorCompletion cb, void *opaque)
>           return -1;
>       }
>
> -    cb(opaque, NULL);
>       return 0;
>   }
>
> @@ -2332,6 +2331,7 @@ static int do_balloon(Monitor *mon, const QDict *params,
>           return -1;
>       }
>
> +    cb(opaque, NULL);
>       return 0;
>   }
>
>
>
>
>    

[Qemu-devel] [PATCH] Fix hanging user monitor when using balloon command

[Adam Litke]

Hi Anthony.  I wonder if there was a problem when importing my async
command handler patchset.  Since the 'balloon' command completes
immediately, it must call the completion callback before returning.
That call was missing but is added by the patch below.

diff --git a/monitor.c b/monitor.c
index ae125b8..f94794d 100644
--- a/monitor.c
+++ b/monitor.c
@@ -2258,6 +2258,7 @@ static int do_balloon(Monitor *mon, const QDict *params,
         return -1;
     }
 
+    cb(opaque, NULL);
     return 0;
 }

-- 
Thanks,
Adam