[libvirt] [PATCH 0/2] qemu: Add support for -no-user-config
Eduardo submitted patches[1] for qemu implementing -no-user-config as a better alternative to all-or-nothing -nodefconfig. With this new option, we are finally able to use modern CPU models defined in qemu's configuration file without allowing user-supplied qemu configuration to mess up with qemu processes started by libvirt. The qemu patches are not committed upstream yet (and this patchset won't be committed either until qemu support is finished) but they are very close and are expected to make it into qemu-1.1. [1] https://www.redhat.com/archives/libvir-list/2012-April/msg01293.html Jiri Denemark (2): qemu: Use common helper when probing qemu capabilities qemu: Add support for -no-user-config src/qemu/qemu_capabilities.c | 64 +++++++++++++++++++++++++++-------------- src/qemu/qemu_capabilities.h | 6 ++++ src/qemu/qemu_command.c | 11 ++++--- src/qemu/qemu_driver.c | 9 +++++- 4 files changed, 62 insertions(+), 28 deletions(-) -- 1.7.8.5
QEMU binary is called several times when we probe different kinds of capabilities the binary supports. This patch introduces new common helper so that all probes use a consistent way of invoking qemu. --- src/qemu/qemu_capabilities.c | 59 ++++++++++++++++++++++++++--------------- src/qemu/qemu_capabilities.h | 5 +++ src/qemu/qemu_driver.c | 9 +++++- 3 files changed, 50 insertions(+), 23 deletions(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 3d1fb43..6e5165b 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -289,6 +289,7 @@ qemuCapsParseMachineTypesStr(const char *output, int qemuCapsProbeMachineTypes(const char *binary, + virBitmapPtr qemuCaps, virCapsGuestMachinePtr **machines, int *nmachines) { @@ -306,10 +307,9 @@ qemuCapsProbeMachineTypes(const char *binary, return -1; } - cmd = virCommandNewArgList(binary, "-M", "?", NULL); - virCommandAddEnvPassCommon(cmd); + cmd = qemuCapsProbeCommand(binary, qemuCaps); + virCommandAddArgList(cmd, "-M", "?", NULL); virCommandSetOutputBuffer(cmd, &output); - virCommandClearCaps(cmd); /* Ignore failure from older qemu that did not understand '-M ?'. */ if (virCommandRun(cmd, &status) < 0) @@ -599,12 +599,9 @@ qemuCapsProbeCPUModels(const char *qemu, return 0; } - cmd = virCommandNewArgList(qemu, "-cpu", "?", NULL); - if (qemuCapsGet(qemuCaps, QEMU_CAPS_NODEFCONFIG)) - virCommandAddArg(cmd, "-nodefconfig"); - virCommandAddEnvPassCommon(cmd); + cmd = qemuCapsProbeCommand(qemu, qemuCaps); + virCommandAddArgList(cmd, "-cpu", "?", NULL); virCommandSetOutputBuffer(cmd, &output); - virCommandClearCaps(cmd); if (virCommandRun(cmd, NULL) < 0) goto cleanup; @@ -730,7 +727,8 @@ qemuCapsInitGuest(virCapsPtr caps, info->wordsize, binary, binary_mtime, old_caps, &machines, &nmachines); if (probe && - qemuCapsProbeMachineTypes(binary, &machines, &nmachines) < 0) + qemuCapsProbeMachineTypes(binary, qemuCaps, + &machines, &nmachines) < 0) goto error; } @@ -798,7 +796,8 @@ qemuCapsInitGuest(virCapsPtr caps, kvmbin, binary_mtime, old_caps, &machines, &nmachines); if (probe && - qemuCapsProbeMachineTypes(kvmbin, &machines, &nmachines) < 0) + qemuCapsProbeMachineTypes(kvmbin, qemuCaps, + &machines, &nmachines) < 0) goto error; } @@ -1366,17 +1365,16 @@ qemuCapsExtractDeviceStr(const char *qemu, * understand '-device name,?', and always exits with status 1 for * the simpler '-device ?', so this function is really only useful * if -help includes "device driver,?". */ - cmd = virCommandNewArgList(qemu, - "-device", "?", - "-device", "pci-assign,?", - "-device", "virtio-blk-pci,?", - "-device", "virtio-net-pci,?", - "-device", "scsi-disk,?", - NULL); - virCommandAddEnvPassCommon(cmd); + cmd = qemuCapsProbeCommand(qemu, flags); + virCommandAddArgList(cmd, + "-device", "?", + "-device", "pci-assign,?", + "-device", "virtio-blk-pci,?", + "-device", "virtio-net-pci,?", + "-device", "scsi-disk,?", + NULL); /* qemu -help goes to stdout, but qemu -device ? goes to stderr. */ virCommandSetErrorBuffer(cmd, &output); - virCommandClearCaps(cmd); if (virCommandRun(cmd, NULL) < 0) goto cleanup; @@ -1485,10 +1483,9 @@ int qemuCapsExtractVersionInfo(const char *qemu, const char *arch, return -1; } - cmd = virCommandNewArgList(qemu, "-help", NULL); - virCommandAddEnvPassCommon(cmd); + cmd = qemuCapsProbeCommand(qemu, NULL); + virCommandAddArgList(cmd, "-help", NULL); virCommandSetOutputBuffer(cmd, &help); - virCommandClearCaps(cmd); if (virCommandRun(cmd, NULL) < 0) goto cleanup; @@ -1628,3 +1625,21 @@ qemuCapsGet(virBitmapPtr caps, else return b; } + + +virCommandPtr +qemuCapsProbeCommand(const char *qemu, + virBitmapPtr qemuCaps) +{ + virCommandPtr cmd = virCommandNew(qemu); + + if (qemuCaps) { + if (qemuCapsGet(qemuCaps, QEMU_CAPS_NODEFCONFIG)) + virCommandAddArg(cmd, "-nodefconfig"); + } + + virCommandAddEnvPassCommon(cmd); + virCommandClearCaps(cmd); + + return cmd; +} diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index 7279cdb..7a6c5a0 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -26,6 +26,7 @@ # include "bitmap.h" # include "capabilities.h" +# include "command.h" /* Internal flags to keep track of qemu command line capabilities */ enum qemuCapsFlags { @@ -150,6 +151,7 @@ bool qemuCapsGet(virBitmapPtr caps, virCapsPtr qemuCapsInit(virCapsPtr old_caps); int qemuCapsProbeMachineTypes(const char *binary, + virBitmapPtr qemuCaps, virCapsGuestMachinePtr **machines, int *nmachines); @@ -175,6 +177,9 @@ int qemuCapsParseHelpStr(const char *qemu, int qemuCapsParseDeviceStr(const char *str, virBitmapPtr qemuCaps); +virCommandPtr qemuCapsProbeCommand(const char *qemu, + virBitmapPtr qemuCaps); + VIR_ENUM_DECL(qemuCaps); #endif /* __QEMU_CAPABILITIES_H__*/ diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index c3555ca..bcc3947 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -4867,7 +4867,14 @@ qemudCanonicalizeMachineDirect(virDomainDefPtr def, char **canonical) virCapsGuestMachinePtr *machines = NULL; int i, nmachines = 0; - if (qemuCapsProbeMachineTypes(def->emulator, &machines, &nmachines) < 0) + /* XXX we should be checking emulator capabilities and pass them instead + * of NULL so that -nodefconfig is properly added when + * probing machine types. Luckily, qemu does not support specifying new + * machine types in its configuration files yet, which means passing this + * additional parameter makes no difference now. + */ + if (qemuCapsProbeMachineTypes(def->emulator, NULL, + &machines, &nmachines) < 0) return -1; for (i = 0; i < nmachines; i++) { -- 1.7.8.5
On 04/26/2012 08:28 AM, Jiri Denemark wrote:
QEMU binary is called several times when we probe different kinds of capabilities the binary supports. This patch introduces new common helper so that all probes use a consistent way of invoking qemu. --- src/qemu/qemu_capabilities.c | 59 ++++++++++++++++++++++++++--------------- src/qemu/qemu_capabilities.h | 5 +++ src/qemu/qemu_driver.c | 9 +++++- 3 files changed, 50 insertions(+), 23 deletions(-)
Nice. Just today, I hit a bug where I wish we were caching qemu capabilities better, and by filtering things into a common entry point, I'm hoping it makes future edits able to take advantage of that one location, rather than chasing down multiple callers. https://bugzilla.redhat.com/show_bug.cgi?id=816674 At any rate, this patch looks correct: ACK. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On Thu, Apr 26, 2012 at 10:42:36 -0600, Eric Blake wrote:
On 04/26/2012 08:28 AM, Jiri Denemark wrote:
QEMU binary is called several times when we probe different kinds of capabilities the binary supports. This patch introduces new common helper so that all probes use a consistent way of invoking qemu. --- src/qemu/qemu_capabilities.c | 59 ++++++++++++++++++++++++++--------------- src/qemu/qemu_capabilities.h | 5 +++ src/qemu/qemu_driver.c | 9 +++++- 3 files changed, 50 insertions(+), 23 deletions(-)
Nice. Just today, I hit a bug where I wish we were caching qemu capabilities better, and by filtering things into a common entry point, I'm hoping it makes future edits able to take advantage of that one location, rather than chasing down multiple callers.
https://bugzilla.redhat.com/show_bug.cgi?id=816674
At any rate, this patch looks correct:
ACK.
OK, I pushed this patch since it doesn't depend on qemu support. I'll wait with pushing 2/2 until -no-user-config support is committed in qemu. Jirka
Thanks to this new option we are now able to use modern CPU models (such as Westmere) defined in external configuration file. --- src/qemu/qemu_capabilities.c | 7 ++++++- src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_command.c | 11 ++++++----- src/qemu/qemu_driver.c | 2 +- 4 files changed, 14 insertions(+), 7 deletions(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 6e5165b..a3c87d1 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -161,6 +161,7 @@ VIR_ENUM_IMPL(qemuCaps, QEMU_CAPS_LAST, "block-job-async", "scsi-cd", "ide-cd", + "no-user-config", ); struct qemu_feature_flags { @@ -1082,6 +1083,8 @@ qemuCapsComputeCmdFlags(const char *help, } if (strstr(help, "-nodefconfig")) qemuCapsSet(flags, QEMU_CAPS_NODEFCONFIG); + if (strstr(help, "-no-user-config")) + qemuCapsSet(flags, QEMU_CAPS_NO_USER_CONFIG); /* The trailing ' ' is important to avoid a bogus match */ if (strstr(help, "-rtc ")) qemuCapsSet(flags, QEMU_CAPS_RTC); @@ -1634,7 +1637,9 @@ qemuCapsProbeCommand(const char *qemu, virCommandPtr cmd = virCommandNew(qemu); if (qemuCaps) { - if (qemuCapsGet(qemuCaps, QEMU_CAPS_NODEFCONFIG)) + if (qemuCapsGet(qemuCaps, QEMU_CAPS_NO_USER_CONFIG)) + virCommandAddArg(cmd, "-no-user-config"); + else if (qemuCapsGet(qemuCaps, QEMU_CAPS_NODEFCONFIG)) virCommandAddArg(cmd, "-nodefconfig"); } diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index 7a6c5a0..0e0899e 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -129,6 +129,7 @@ enum qemuCapsFlags { QEMU_CAPS_BLOCKJOB_ASYNC = 91, /* qemu 1.1 block-job-cancel */ QEMU_CAPS_SCSI_CD = 92, /* -device scsi-cd */ QEMU_CAPS_IDE_CD = 93, /* -device ide-cd */ + QEMU_CAPS_NO_USER_CONFIG = 94, /* -no-user-config */ QEMU_CAPS_LAST, /* this must always be the last item */ }; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 45cd417..e847060 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -4237,11 +4237,12 @@ qemuBuildCommandLine(virConnectPtr conn, virCommandAddArg(cmd, "-nographic"); if (qemuCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) { - if (qemuCapsGet(qemuCaps, QEMU_CAPS_NODEFCONFIG)) - virCommandAddArg(cmd, - "-nodefconfig"); /* Disable global config files */ - virCommandAddArg(cmd, - "-nodefaults"); /* Disable default guest devices */ + /* Disable global config files and default devices */ + if (qemuCapsGet(qemuCaps, QEMU_CAPS_NO_USER_CONFIG)) + virCommandAddArg(cmd, "-no-user-config"); + else if (qemuCapsGet(qemuCaps, QEMU_CAPS_NODEFCONFIG)) + virCommandAddArg(cmd, "-nodefconfig"); + virCommandAddArg(cmd, "-nodefaults"); } /* Serial graphics adapter */ diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index bcc3947..0345d89 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -4868,7 +4868,7 @@ qemudCanonicalizeMachineDirect(virDomainDefPtr def, char **canonical) int i, nmachines = 0; /* XXX we should be checking emulator capabilities and pass them instead - * of NULL so that -nodefconfig is properly added when + * of NULL so that -nodefconfig or -no-user-config is properly added when * probing machine types. Luckily, qemu does not support specifying new * machine types in its configuration files yet, which means passing this * additional parameter makes no difference now. -- 1.7.8.5
On 04/26/2012 08:28 AM, Jiri Denemark wrote:
Thanks to this new option we are now able to use modern CPU models (such as Westmere) defined in external configuration file. ---
I agree with your decision to not push this patch until we have a documented qemu pull request incorporating the qemu side of things, to ensure that we will match qemu 1.1 semantics.
src/qemu/qemu_capabilities.c | 7 ++++++- src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_command.c | 11 ++++++----- src/qemu/qemu_driver.c | 2 +- 4 files changed, 14 insertions(+), 7 deletions(-)
When we _do_ get ready to push this, please add a file to tests/qemuhelpdata corresponding to qemu 1.1, so that we can prove that we properly detect the new flag bit according to -help output. ACK to the code that you do have, though. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On Thu, Apr 26, 2012 at 10:48:15 -0600, Eric Blake wrote:
On 04/26/2012 08:28 AM, Jiri Denemark wrote:
Thanks to this new option we are now able to use modern CPU models (such as Westmere) defined in external configuration file. ---
I agree with your decision to not push this patch until we have a documented qemu pull request incorporating the qemu side of things, to ensure that we will match qemu 1.1 semantics.
src/qemu/qemu_capabilities.c | 7 ++++++- src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_command.c | 11 ++++++----- src/qemu/qemu_driver.c | 2 +- 4 files changed, 14 insertions(+), 7 deletions(-)
When we _do_ get ready to push this, please add a file to tests/qemuhelpdata corresponding to qemu 1.1, so that we can prove that we properly detect the new flag bit according to -help output.
ACK to the code that you do have, though.
Thanks. Since the qemu side of this is now included in qemu-1.1-rc2, I went ahead and pushed this patch with appropriate changes to qemuhelptest: commit 63b4243624b8fdabebaf5e6ec912095b2b5fdf5c Author: Jiri Denemark <jdenemar@redhat.com> Date: Thu Apr 26 12:11:49 2012 +0200 qemu: Add support for -no-user-config Thanks to this new option we are now able to use modern CPU models (such as Westmere) defined in external configuration file. The qemu-1.1{,-device} data files for qemuhelptest are filled in with qemu-1.1-rc2 output for now. I will update those files with real qemu-1.1 output once it is released. cfg.mk | 3 +- src/qemu/qemu_capabilities.c | 7 +- src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_command.c | 11 +- src/qemu/qemu_driver.c | 2 +- tests/qemuhelpdata/qemu-1.1 | 268 ++++++++++++++++++++++++++++++++++++ tests/qemuhelpdata/qemu-1.1-device | 160 +++++++++++++++++++++ tests/qemuhelptest.c | 75 ++++++++++ Jirka
participants (2)
-
Eric Blake -
Jiri Denemark