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

Re: [libvirt] [PATCH] qemu: Add shortcut for HMP pass through



On 02/02/2011 08:49 AM, Jiri Denemark wrote:
> Currently users who want to use virDomainQemuMonitorCommand() API or
> it's virsh equivalent has to use the same protocol as libvirt uses for
> communication to qemu. Since the protocol is QMP with current qemu and
> HMP much more usable for humans, one ends up typing something like the
> following:
> 
>     virsh qemu-monitor-command DOM \
> '{"execute":"human-monitor-command","arguments":{"command-line":"info kvm"}}'
> 
> which is not a very convenient way of debugging qemu.

>  
> +enum {
> +    VIR_DOMAIN_QEMU_MONITOR_COMMAND_DEFAULT = 0,
> +    VIR_DOMAIN_QEMU_MONITOR_COMMAND_HMP     = (1 << 0), /* cmd is in HMP */
> +} virDomainQemuMonitorCommandFlags;
> +
>  int virDomainQemuMonitorCommand(virDomainPtr domain, const char *cmd,
>                                  char **result, unsigned int flags);

Aren't you glad we included the flags parameter when first adding the
API?  Makes life a lot easier when you don't have to add a new API.

> -    DEBUG("mon=%p, cmd=%s, reply=%p", mon, cmd, reply);
> +    DEBUG("mon=%p, cmd=%s, reply=%p, hmp=%d", mon, cmd, reply, hmp);
>  
>      if (mon->json)
> -        ret = qemuMonitorJSONArbitraryCommand(mon, cmd, reply);
> +        ret = qemuMonitorJSONArbitraryCommand(mon, cmd, reply, hmp);
>      else
>          ret = qemuMonitorTextArbitraryCommand(mon, cmd, reply);
>      return ret;

Nice - only JSON has to care about the difference.

>  
> -    *reply_str = virJSONValueToString(reply);
> +    if (!hmp) {
> +        *reply_str = virJSONValueToString(reply);
> +    } else if (qemuMonitorJSONCheckError(cmd, reply)) {
> +        goto cleanup;
> +    } else {
> +        const char *data;
> +        if (!(data = virJSONValueObjectGetString(reply, "return"))) {
> +            qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +                            _("human monitor command was missing return data"));
> +            goto cleanup;
> +        }
> +        *reply_str = strdup(data);

Hmm, if this fails, you're missing a call to virReportOOMError()...

> +    }
> +
>      if (!(*reply_str))
>          goto cleanup;

since pre-patch, you had been relying on virJSONValueToString to report
it on your behalf.

ACK with that nit fixed.

-- 
Eric Blake   eblake redhat com    +1-801-349-2682
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]