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

Re: [libvirt] [PATCH 3/4] Add support for detecting capablities using QMP commands



On 09/28/2012 08:58 AM, Daniel P. Berrange wrote:
> From: "Daniel P. Berrange" <berrange redhat com>
> 
> Start a QEMU process using
> 
>    $QEMU -S -no-user-config -nodefaults \
>          -nographic -M none -qmp unix:/some/path,server,nowait
> 
> and talk QMP over stdio to discover what capabilities the
> binary supports. This works for QEMU 1.2.0 or later and
> for older QEMU automatically fallback to the old approach
> of parsing -help and related command line args.
> 
> Signed-off-by: Daniel P. Berrange <berrange redhat com>
> ---
>  src/qemu/qemu_capabilities.c | 394 +++++++++++++++++++++++++++++++++++++++----
>  1 file changed, 358 insertions(+), 36 deletions(-)
> 

> +
> +/* Capabilities that we assume are always enabled
> + * for QEMU >= 1.2.0
> + */
> +static void
> +qemuCapsInitQMPBasic(qemuCapsPtr caps)
> +{
> +    qemuCapsSet(caps, QEMU_CAPS_VNC_COLON);
> +    qemuCapsSet(caps, QEMU_CAPS_NO_REBOOT);

I think qemuCapsSetList() would be nicer than setting one bit at a time,
but that doesn't change semantics.


> +
> +static int
> +qemuCapsInitQMP(qemuCapsPtr caps)
> +{
> +    int ret = -1;
> +    virCommandPtr cmd = NULL;
> +    qemuMonitorPtr mon = NULL;
> +    int major, minor, micro;
> +    char *package;
> +    int status = 0;
> +    virDomainChrSourceDef config;
> +
> +    memset(&config, 0, sizeof(config));
> +    config.type = VIR_DOMAIN_CHR_TYPE_UNIX;
> +    config.data.nix.path = (char*)"/tmp/qemu.sock";

This does not seem like a safe file name (it is easily guessable).
Should we instead be generating something under /var/run?

> +    config.data.nix.listen = false;
> +
> +    VIR_DEBUG("Try to get caps via QMP caps=%p", caps);
> +
> +    cmd = virCommandNewArgList(caps->binary,
> +                               "-S",
> +                               "-no-user-config",
> +                               "-nodefaults",
> +                               "-nographic",
> +                               "-M", "none",
> +                               "-qmp", "unix:/tmp/qemu.sock,server,nowait",

And here, we should match whatever filename we actually use.

Other than that, this looks nice.

-- 
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]