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

Re: [libvirt] [PATCH 2/5] add qemuAgentCommand()



On 08/07/2012 06:04 PM, MATSUDA, Daiki wrote:
>     Add qemuAgentCommand() for general qemu agent command.
> 
>  include/libvirt/libvirt-qemu.h |    5 +++++
>  src/qemu/qemu_agent.c          |   38 ++++++++++++++++++++++++++++++++++++++
>  src/qemu/qemu_agent.h          |    5 +++++
>  3 files changed, 48 insertions(+)
> 

Causes a compiler warning:

  CC     libvirt_driver_qemu_impl_la-qemu_agent.lo
qemu/qemu_agent.c: In function 'qemuAgentQemuAgentCommand':
qemu/qemu_agent.c:1411:9: error: variable 'seconds' set but not used
[-Werror=unused-but-set-variable]

> diff --git a/include/libvirt/libvirt-qemu.h b/include/libvirt/libvirt-qemu.h
> index a37f897..ffc5ae5 100644
> --- a/include/libvirt/libvirt-qemu.h
> +++ b/include/libvirt/libvirt-qemu.h
> @@ -44,6 +44,11 @@ virDomainPtr virDomainQemuAttach(virConnectPtr domain,
>                                   unsigned int pid_value,
>                                   unsigned int flags);
> 
> +typedef enum {
> +    VIR_DOMAIN_QEMU_AGENT_COMMAND_BLOCK = -1,
> +    VIR_DOMAIN_QEMU_AGENT_COMMAND_NOWAIT = 0,
> +} virDomainQemuAgentCommandTimeoutFlags;

These aren't flags, but special timeout values.  Maybe s/Flags/Values/


> +
> +int qemuAgentQemuAgentCommand(qemuAgentPtr mon,

This name sounds funny; maybe qemuAgentArbitraryCommand might be nicer.

> +                              const char *cmd_str,
> +                              char **result,
> +                              int timeout)
> +{
> +    int ret = -1;
> +    int seconds;
> +    virJSONValuePtr cmd;
> +    virJSONValuePtr reply = NULL;
> +
> +    cmd = virJSONValueFromString(cmd_str);
> +    if (!cmd)
> +        return ret;
> +
> +    if (result == NULL) {
> +        seconds = 0;
> +    } else if (timeout == VIR_DOMAIN_QEMU_AGENT_COMMAND_BLOCK) {
> +        seconds = 5;

Why 5 seconds here,

> +    } else {
> +        seconds = timeout;

but the user's timeout here?  If you are looping while waiting for a
response, then 5 seconds is too long between loops; you will feel
unresponsive; and if the user passes a timeout of 10, you end up waiting
10 seconds.

> +    }
> +
> +repeat:
> +    ret = qemuAgentCommand(mon, cmd, &reply, timeout);

Won't work - this starts a new agent command every time repeat is
reached, rather than waiting for the completion of an existing command.

> +
> +    if (ret == 0) {
> +        ret = qemuAgentCheckError(cmd, reply);
> +        *result = virJSONValueToString(reply);
> +    } else {
> +        if (timeout == VIR_DOMAIN_QEMU_AGENT_COMMAND_BLOCK) goto repeat;

Typical formatting would be to split this to two lines:

if (timeout == VIR_DOMAIN_QEMU_AGENT_COMMAND_BLOCK)
    goto repeat;

> +        *result = NULL;
> +    }
> +
> +    virJSONValueFree(cmd);
> +    virJSONValueFree(reply);
> +    return ret;
> +}
> diff --git a/src/qemu/qemu_agent.h b/src/qemu/qemu_agent.h
> index 2fdebb2..fc19c2f 100644
> --- a/src/qemu/qemu_agent.h
> +++ b/src/qemu/qemu_agent.h
> @@ -77,4 +77,9 @@ int qemuAgentFSThaw(qemuAgentPtr mon);
> 
>  int qemuAgentSuspend(qemuAgentPtr mon,
>                       unsigned int target);
> +
> +int qemuAgentQemuAgentCommand(qemuAgentPtr mon,
> +                              const char *cmd,
> +                              char **result,
> +                              int timeout);
>  #endif /* __QEMU_AGENT_H__ */
> 

Definitely needs some work before this function will handle timeout
properly.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature


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