9:28 a.m.
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
9:28 a.m.
New subject: [libvirt] [PATCH 1/2] qemu: Use common helper when probing qemu capabilities
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
11:42 a.m.
New subject: [libvirt] [PATCH 1/2] qemu: Use common helper when probing qemu capabilities
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(a)redhat.com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
5:19 a.m.
New subject: [libvirt] [PATCH 1/2] qemu: Use common helper when probing qemu capabilities
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
9:28 a.m.
New subject: [libvirt] [PATCH 2/2] 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.
---
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
11:48 a.m.
New subject: [libvirt] [PATCH 2/2] qemu: Add support for -no-user-config
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(a)redhat.com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
1:37 p.m.
New subject: [libvirt] [PATCH 2/2] qemu: Add support for -no-user-config
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(a)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
4574
days inactive
4593
days old
6 comments
2 participants
participants (2)
-
Eric Blake -
Jiri Denemark