[libvirt] [PATCH v3 20/20] Add support for detecting capablities using QMP commands
Jiri Denemark
jdenemar at redhat.com
Thu Sep 27 13:00:14 UTC 2012
On Tue, Sep 25, 2012 at 19:00:13 +0100, Daniel P. Berrange wrote:
> From: "Daniel P. Berrange" <berrange at redhat.com>
>
> Start a QEMU process using
>
> $QEMU -S -no-user-config -nodefconfig -nodefaults \
> -nographic -M none -qmp stdio
-nodefconfig should not ever be used if QEMU supports -no-user-config. The
reason is that -nodefconfig disables loading of all files even those that
reside somewhere in /usr/share and may contain required data, such as CPU
definitions (although they were moved back to the code recently) or machine
type definitions if they ever implement the ideas to separate them from the
code.
> 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 at redhat.com>
> ---
> src/qemu/qemu_capabilities.c | 413 +++++++++++++++++++++++++++++++++++++++----
> src/qemu/qemu_capabilities.h | 2 +-
> 2 files changed, 377 insertions(+), 38 deletions(-)
>
> diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
> index c4d36f9..b4e824a 100644
> --- a/src/qemu/qemu_capabilities.c
> +++ b/src/qemu/qemu_capabilities.c
> @@ -35,6 +35,7 @@
> #include "command.h"
> #include "bitmap.h"
> #include "virnodesuspend.h"
> +#include "qemu_monitor.h"
>
> #include <sys/stat.h>
> #include <unistd.h>
> @@ -188,6 +189,8 @@ VIR_ENUM_IMPL(qemuCaps, QEMU_CAPS_LAST,
> struct _qemuCaps {
> virObject object;
>
> + bool usedQMP;
> +
> char *binary;
> time_t mtime;
>
> @@ -1282,6 +1285,7 @@ static struct qemuCapsStringFlags qemuCapsObjectPropsVirtioNet[] = {
> };
>
> static struct qemuCapsStringFlags qemuCapsObjectPropsPciAssign[] = {
> + { "rombar", QEMU_CAPS_PCI_ROMBAR },
> { "configfd", QEMU_CAPS_PCI_CONFIGFD },
> { "bootindex", QEMU_CAPS_PCI_BOOTINDEX },
> };
> @@ -1857,6 +1861,10 @@ qemuCapsProbeQMPCommands(qemuCapsPtr caps,
> qemuCapsSet(caps, QEMU_CAPS_BLOCKJOB_ASYNC);
> else if (STREQ(name, "dump-guest-memory"))
> qemuCapsSet(caps, QEMU_CAPS_DUMP_GUEST_MEMORY);
> + else if (STREQ(name, "query-spice"))
> + qemuCapsSet(caps, QEMU_CAPS_SPICE);
> + else if (STREQ(name, "query-kvm"))
> + qemuCapsSet(caps, QEMU_CAPS_KVM);
> VIR_FREE(name);
> }
> VIR_FREE(commands);
Hmm, looks like these two hunks should rather go into the previous patch,
shouldn't they?
...
> @@ -1979,18 +2078,252 @@ qemuCapsPtr qemuCapsNewForBinary(const char *binary)
> * understands the 0.13.0+ notion of "-device driver,". */
> if (qemuCapsGet(caps, QEMU_CAPS_DEVICE) &&
> strstr(help, "-device driver,?") &&
> - qemuCapsExtractDeviceStr(binary, caps) < 0)
> - goto error;
> + qemuCapsExtractDeviceStr(caps->binary, caps) < 0)
> + goto cleanup;
>
> if (qemuCapsProbeCPUModels(caps) < 0)
> - goto error;
> + goto cleanup;
>
> if (qemuCapsProbeMachineTypes(caps) < 0)
> - goto error;
> + goto cleanup;
> +
> + ret = 0;
> +cleanup:
> + virCommandFree(cmd);
> + return ret;
> +}
I think you actually didn't want to remove VIR_FREE(help); from the cleanup
section.
> +
> +
> +
I guess two empty lines would be sufficient? :-)
> +static void qemuCapsMonitorEOFNotify(qemuMonitorPtr mon ATTRIBUTE_UNUSED,
> + virDomainObjPtr vm ATTRIBUTE_UNUSED)
Wrong indentation.
> +{
> +}
> +
> +static void qemuCapsMonitorErrorNotify(qemuMonitorPtr mon ATTRIBUTE_UNUSED,
> + virDomainObjPtr vm ATTRIBUTE_UNUSED)
Wrong indentation here as well.
> +{
> +}
> +
> +static qemuMonitorCallbacks callbacks = {
> + .eofNotify = qemuCapsMonitorEOFNotify,
> + .errorNotify = qemuCapsMonitorErrorNotify,
> +};
Anyway, can't we just do
static void
qemuCapsMonitorNotify(qemuMonitorPtr mon ATTRIBUTE_UNUSED,
virDomainObjPtr vm ATTRIBUTE_UNUSED)
{
}
static qemuMonitorCallbacks callbacks = {
.eofNotify = qemuCapsMonitorNotify,
.errorNotify = qemuCapsMonitorNotify,
};
since both notifiers have the same prototype?
> +
> +
> +/* 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);
> + qemuCapsSet(caps, QEMU_CAPS_DRIVE);
> + qemuCapsSet(caps, QEMU_CAPS_NAME);
> + qemuCapsSet(caps, QEMU_CAPS_UUID);
> + qemuCapsSet(caps, QEMU_CAPS_VNET_HDR);
> + qemuCapsSet(caps, QEMU_CAPS_MIGRATE_QEMU_TCP);
> + qemuCapsSet(caps, QEMU_CAPS_MIGRATE_QEMU_EXEC);
> + qemuCapsSet(caps, QEMU_CAPS_DRIVE_CACHE_V2);
> + qemuCapsSet(caps, QEMU_CAPS_DRIVE_FORMAT);
> + qemuCapsSet(caps, QEMU_CAPS_VGA);
> + qemuCapsSet(caps, QEMU_CAPS_0_10);
> + qemuCapsSet(caps, QEMU_CAPS_MEM_PATH);
> + qemuCapsSet(caps, QEMU_CAPS_DRIVE_SERIAL);
> + qemuCapsSet(caps, QEMU_CAPS_MIGRATE_QEMU_UNIX);
> + qemuCapsSet(caps, QEMU_CAPS_CHARDEV);
> + qemuCapsSet(caps, QEMU_CAPS_ENABLE_KVM);
> + qemuCapsSet(caps, QEMU_CAPS_MONITOR_JSON);
> + qemuCapsSet(caps, QEMU_CAPS_BALLOON);
> + qemuCapsSet(caps, QEMU_CAPS_DEVICE);
> + qemuCapsSet(caps, QEMU_CAPS_SDL);
> + qemuCapsSet(caps, QEMU_CAPS_SMP_TOPOLOGY);
> + qemuCapsSet(caps, QEMU_CAPS_NETDEV);
> + qemuCapsSet(caps, QEMU_CAPS_RTC);
> + qemuCapsSet(caps, QEMU_CAPS_VHOST_NET);
> + qemuCapsSet(caps, QEMU_CAPS_NO_HPET);
> + qemuCapsSet(caps, QEMU_CAPS_NODEFCONFIG);
> + qemuCapsSet(caps, QEMU_CAPS_BOOT_MENU);
> + qemuCapsSet(caps, QEMU_CAPS_FSDEV);
> + qemuCapsSet(caps, QEMU_CAPS_NESTING);
> + qemuCapsSet(caps, QEMU_CAPS_NAME_PROCESS);
> + qemuCapsSet(caps, QEMU_CAPS_DRIVE_READONLY);
> + qemuCapsSet(caps, QEMU_CAPS_SMBIOS_TYPE);
> + qemuCapsSet(caps, QEMU_CAPS_VGA_NONE);
> + qemuCapsSet(caps, QEMU_CAPS_MIGRATE_QEMU_FD);
> + qemuCapsSet(caps, QEMU_CAPS_DRIVE_AIO);
> + qemuCapsSet(caps, QEMU_CAPS_CHARDEV_SPICEVMC);
> + qemuCapsSet(caps, QEMU_CAPS_DEVICE_QXL_VGA);
> + qemuCapsSet(caps, QEMU_CAPS_DRIVE_CACHE_DIRECTSYNC);
> + qemuCapsSet(caps, QEMU_CAPS_NO_SHUTDOWN);
> + qemuCapsSet(caps, QEMU_CAPS_DRIVE_CACHE_UNSAFE);
> + qemuCapsSet(caps, QEMU_CAPS_NO_ACPI);
> + qemuCapsSet(caps, QEMU_CAPS_FSDEV_READONLY);
> + qemuCapsSet(caps, QEMU_CAPS_VIRTIO_BLK_SG_IO);
> + qemuCapsSet(caps, QEMU_CAPS_DRIVE_COPY_ON_READ);
> + qemuCapsSet(caps, QEMU_CAPS_CPU_HOST);
> + qemuCapsSet(caps, QEMU_CAPS_FSDEV_WRITEOUT);
> + qemuCapsSet(caps, QEMU_CAPS_DRIVE_IOTUNE);
> + qemuCapsSet(caps, QEMU_CAPS_WAKEUP);
> + qemuCapsSet(caps, QEMU_CAPS_NO_USER_CONFIG);
> + qemuCapsSet(caps, QEMU_CAPS_NETDEV_BRIDGE);
> +}
> +
> +/*
> + * Returns -1 on fatal error, 0 on success
> + */
As almost every function in libvirt does...
> +static int
> +qemuCapsInitQMP(qemuCapsPtr caps)
> +{
> + int ret = -1;
> + virCommandPtr cmd = NULL;
> + virDomainObjPtr vm = NULL;
> + int socks[2] = { -1, -1 };
> + qemuMonitorPtr mon = NULL;
> + int major, minor, micro;
> + char *package;
> +
> + VIR_DEBUG("caps=%p", caps);
> +
> + if (socketpair(PF_UNIX, SOCK_STREAM, 0, socks) < 0) {
> + virReportSystemError(errno, "%s",
> + _("unable to create socket pair"));
> + goto cleanup;
> + }
> +
> + if (!(vm = virDomainObjNew(NULL)))
> + goto cleanup;
Heh, AFAICT, vm in qemuMonitorOpenFD is only used to fill in mon->vm so that
the VM pointer can be passed to various callbacks when something asynchronous
happens in the monitor. Looks like we can just safely pass NULL to
qemuMonitorOpenFD() instead.
> +
> + cmd = virCommandNewArgList(caps->binary,
> + "-S",
> + "-no-user-config",
> + "-nodefconfig",
Remove the -nodefconfig parameter, since this code requires at least qemu-1.2
and thus it will either support -no-user-config or be unusable anyway.
> + "-nodefaults",
> + "-nographic",
> + "-M", "none",
> + "-qmp", "stdio",
> + NULL);
> + virCommandAddEnvPassCommon(cmd);
> + virCommandSetInputFD(cmd, socks[1]);
> + virCommandSetOutputFD(cmd, &socks[1]);
> + virCommandClearCaps(cmd);
> +
> + if (virCommandRunAsync(cmd, NULL) < 0)
> + goto cleanup;
However, if qemu is not new enough to support -no-user-config, virCommand will
fail and this would consider it a fatal error instead of falling back to help
parsing.
...
Jirka
More information about the libvir-list
mailing list