Re: [PATCH] qemu: make sure agent returns error when required data are missing

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 04/03/14 07:58, Martin Kletzander wrote:
> Commit 5b3492fa aimed to fix this and caught one error but exposed
> another one.  When agent command is being executed and the thread
> waiting for the reply is woken up by an event (e.g. EOF in case of
> shutdown), the command finishes with no data (rxObject == NULL), but
> no error is reported, since this might be desired by the caller
> (e.g. suspend through agent).  However, in other situations, when the
> data are required (e.g. getting vCPUs), we proceed to getting desired
> data out of the reply, but none of the virJSON*() functions works well
> with NULLs.  I chose the way of a new parameter for qemuAgentCommand()
> function that specifies whether reply is required and behaves
> according to that.
> 
> Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1058149
> 
> Signed-off-by: Martin Kletzander <mkletzan@xxxxxxxxxx>
> ---
> 
> Notes:
>     This issue probably exists since qemu-ga is supported in libvirt, so
>     this (along with 5b3492fa and e9d09fe1) might be worth back-porting to
>     some maintenance branches, I just haven't gone through them to see
>     which ones are applicable.
> 
>  src/qemu/qemu_agent.c | 19 ++++++++++---------
>  1 file changed, 10 insertions(+), 9 deletions(-)
> 
> diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c
> index 92573bd..4082331 100644
> --- a/src/qemu/qemu_agent.c
> +++ b/src/qemu/qemu_agent.c
> @@ -1080,6 +1080,7 @@ static int
>  qemuAgentCommand(qemuAgentPtr mon,
>                   virJSONValuePtr cmd,
>                   virJSONValuePtr *reply,
> +                 bool needReply,
>                   int seconds)
>  {

Seeing that the "needReply" parameter is true in most cases, I'd rename
this function and add one not requiring the new parameter for normal
usage and use the renamed in the few cases that actually expect the VM
to quit.

But this is just a cosmetic issue.


ACK

Peter


Attachment: signature.asc
Description: OpenPGP digital signature

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]