[libvirt] [PATCH 0/5] Add vmport feature (v3)
Hi, The QEMU machine vmport option allows to set the VMWare IO port emulation. This emulation is useful for absolute pointer input when the guest has vmware input drivers, and is enabled by default for kvm. However it is unnecessary for Spice-enabled VM, since the agent already handles absolute pointer and multi-monitors. Furthermore, it prevents Spice from switching to relative input since the regular ps/2 pointer driver is replaced by the vmware driver. It is thus advised to disable vmport when using a Spice VM. This will permit the Spice client to switch from absolute to relative pointer, as it may be required for certain games or applications. The following patch series allows to configure the vmport feature. The first version sent used a domain attribute, Daniel Berrange suggested to use a feature instead. I sent a second version that didn't receive reviews. Hopefully this one will stand more chances! cheers Marc-André Lureau (5): docs: add domain vmport feature domain/conf: add VIR_DOMAIN_FEATURE_VMPORT qemu: add QEMU_CAPS_MACHINE_VMPORT_OPT qemu: add machine vmport argument tests: add machine-vmport-opt qemu test docs/formatdomain.html.in | 6 +++++ docs/schemas/domaincommon.rng | 9 +++++++ src/conf/domain_conf.c | 5 +++- src/conf/domain_conf.h | 1 + src/qemu/qemu_capabilities.c | 6 +++++ src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_command.c | 14 +++++++++++ .../qemuxml2argv-machine-vmport-opt.args | 6 +++++ .../qemuxml2argv-machine-vmport-opt.xml | 28 ++++++++++++++++++++++ tests/qemuxml2argvtest.c | 2 ++ 10 files changed, 77 insertions(+), 1 deletion(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-machine-vmport-opt.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-machine-vmport-opt.xml -- 2.1.0
A new feature that can be turned on or off. The QEMU machine vmport option allows to set the VMWare IO port emulation. This emulation is useful for absolute pointer input when the guest has vmware input drivers, and is enabled by default for kvm. However it is unnecessary for Spice-enabled VM, since the agent already handles absolute pointer and multi-monitors. Furthermore, it prevents Spice from switching to relative input since the regular ps/2 pointer driver is replaced by the vmware driver. It is thus advised to disable vmport when using a Spice VM. This will permit the Spice client to switch from absolute to relative pointer, as it may be required for certain games or applications. --- docs/formatdomain.html.in | 6 ++++++ docs/schemas/domaincommon.rng | 9 +++++++++ 2 files changed, 15 insertions(+) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 1b496c3..be588c8 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -1500,6 +1500,12 @@ performance monitoring unit for the guest. <span class="since">Since 1.2.12</span> </dd> + <dt><code>vmport</code></dt> + <dd>Depending on the <code>state</code> attribute (values <code>on</code>, + <code>off</code>, default <code>on</code>) enable or disable the + the emulation of VMWare IO port, for vmmouse etc. + <span class="since">Since 1.2.15</span> + </dd> </dl> <h3><a name="elementsTime">Time keeping</a></h3> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 03fd541..b8e06b3 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -4149,6 +4149,15 @@ <optional> <ref name="pmu"/> </optional> + <optional> + <element name="vmport"> + <optional> + <attribute name="state"> + <ref name="virOnOff"/> + </attribute> + </optional> + </element> + </optional> </interleave> </element> </optional> -- 2.1.0
On Thu, Apr 02, 2015 at 07:02:31PM +0200, Marc-André Lureau wrote:
A new feature that can be turned on or off.
The QEMU machine vmport option allows to set the VMWare IO port emulation. This emulation is useful for absolute pointer input when the guest has vmware input drivers, and is enabled by default for kvm.
However it is unnecessary for Spice-enabled VM, since the agent already handles absolute pointer and multi-monitors. Furthermore, it prevents Spice from switching to relative input since the regular ps/2 pointer driver is replaced by the vmware driver. It is thus advised to disable vmport when using a Spice VM. This will permit the Spice client to switch from absolute to relative pointer, as it may be required for certain games or applications. --- docs/formatdomain.html.in | 6 ++++++ docs/schemas/domaincommon.rng | 9 +++++++++ 2 files changed, 15 insertions(+)
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 1b496c3..be588c8 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -1500,6 +1500,12 @@ performance monitoring unit for the guest. <span class="since">Since 1.2.12</span> </dd> + <dt><code>vmport</code></dt> + <dd>Depending on the <code>state</code> attribute (values <code>on</code>, + <code>off</code>, default <code>on</code>) enable or disable the + the emulation of VMWare IO port, for vmmouse etc.
This will break syntax-check ("the\n the").
--- src/conf/domain_conf.c | 5 ++++- src/conf/domain_conf.h | 1 + 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 1763305..27f15b9 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -139,7 +139,8 @@ VIR_ENUM_IMPL(virDomainFeature, VIR_DOMAIN_FEATURE_LAST, "kvm", "pvspinlock", "capabilities", - "pmu") + "pmu", + "vmport") VIR_ENUM_IMPL(virDomainCapabilitiesPolicy, VIR_DOMAIN_CAPABILITIES_POLICY_LAST, "default", @@ -14237,6 +14238,7 @@ virDomainDefParseXML(xmlDocPtr xml, case VIR_DOMAIN_FEATURE_PMU: case VIR_DOMAIN_FEATURE_PVSPINLOCK: + case VIR_DOMAIN_FEATURE_VMPORT: node = ctxt->node; ctxt->node = nodes[i]; if ((tmp = virXPathString("string(./@state)", ctxt))) { @@ -20853,6 +20855,7 @@ virDomainDefFormatInternal(virDomainDefPtr def, case VIR_DOMAIN_FEATURE_PMU: case VIR_DOMAIN_FEATURE_PVSPINLOCK: + case VIR_DOMAIN_FEATURE_VMPORT: switch ((virTristateSwitch) def->features[i]) { case VIR_TRISTATE_SWITCH_LAST: case VIR_TRISTATE_SWITCH_ABSENT: diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 0b2f1c9..edd488b 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1634,6 +1634,7 @@ typedef enum { VIR_DOMAIN_FEATURE_PVSPINLOCK, VIR_DOMAIN_FEATURE_CAPABILITIES, VIR_DOMAIN_FEATURE_PMU, + VIR_DOMAIN_FEATURE_VMPORT, VIR_DOMAIN_FEATURE_LAST } virDomainFeature; -- 2.1.0
vmport machine argument is only supported since qemu 2.2.0 --- src/qemu/qemu_capabilities.c | 6 ++++++ src/qemu/qemu_capabilities.h | 1 + 2 files changed, 7 insertions(+) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index ce6767c..8782613 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -279,6 +279,8 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST, "qxl.vgamem_mb", "qxl-vga.vgamem_mb", "pc-dimm", + + "machine-vmport-opt", /* 185 */ ); @@ -3239,6 +3241,10 @@ virQEMUCapsInitQMPMonitor(virQEMUCapsPtr qemuCaps, if (qemuCaps->version >= 1003000) virQEMUCapsSet(qemuCaps, QEMU_CAPS_MACHINE_USB_OPT); + /* vmport option is supported v2.2.0 onwards */ + if (qemuCaps->version >= 2002000) + virQEMUCapsSet(qemuCaps, QEMU_CAPS_MACHINE_VMPORT_OPT); + /* WebSockets were introduced between 1.3.0 and 1.3.1 */ if (qemuCaps->version >= 1003001) virQEMUCapsSet(qemuCaps, QEMU_CAPS_VNC_WEBSOCKET); diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index c7b1ac7..48c8f96 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -224,6 +224,7 @@ typedef enum { QEMU_CAPS_QXL_VGAMEM = 182, /* -device qxl.vgamem_mb */ QEMU_CAPS_QXL_VGA_VGAMEM = 183, /* -device qxl-vga.vgamem_mb */ QEMU_CAPS_DEVICE_PC_DIMM = 184, /* pc-dimm device */ + QEMU_CAPS_MACHINE_VMPORT_OPT = 185, /* -machine xxx,vmport=on/off/auto */ QEMU_CAPS_LAST, /* this must always be the last item */ } virQEMUCapsFlags; -- 2.1.0
On Thu, Apr 02, 2015 at 07:02:33PM +0200, Marc-André Lureau wrote:
vmport machine argument is only supported since qemu 2.2.0 --- src/qemu/qemu_capabilities.c | 6 ++++++ src/qemu/qemu_capabilities.h | 1 + 2 files changed, 7 insertions(+)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index ce6767c..8782613 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -279,6 +279,8 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST, "qxl.vgamem_mb", "qxl-vga.vgamem_mb", "pc-dimm", + + "machine-vmport-opt", /* 185 */ );
@@ -3239,6 +3241,10 @@ virQEMUCapsInitQMPMonitor(virQEMUCapsPtr qemuCaps, if (qemuCaps->version >= 1003000) virQEMUCapsSet(qemuCaps, QEMU_CAPS_MACHINE_USB_OPT);
+ /* vmport option is supported v2.2.0 onwards */ + if (qemuCaps->version >= 2002000) + virQEMUCapsSet(qemuCaps, QEMU_CAPS_MACHINE_VMPORT_OPT); +
Is this not exposed in any way in QEMU? Do we really need to use this (what we're trying to avoid)? If there is not, please at least sort it based on the version being tried (after the check for version >= 1006000).
/* WebSockets were introduced between 1.3.0 and 1.3.1 */ if (qemuCaps->version >= 1003001) virQEMUCapsSet(qemuCaps, QEMU_CAPS_VNC_WEBSOCKET); diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index c7b1ac7..48c8f96 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -224,6 +224,7 @@ typedef enum { QEMU_CAPS_QXL_VGAMEM = 182, /* -device qxl.vgamem_mb */ QEMU_CAPS_QXL_VGA_VGAMEM = 183, /* -device qxl-vga.vgamem_mb */ QEMU_CAPS_DEVICE_PC_DIMM = 184, /* pc-dimm device */ + QEMU_CAPS_MACHINE_VMPORT_OPT = 185, /* -machine xxx,vmport=on/off/auto */
QEMU_CAPS_LAST, /* this must always be the last item */ } virQEMUCapsFlags; -- 2.1.0
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Hi On Tue, Apr 14, 2015 at 4:25 PM, Martin Kletzander <mkletzan@redhat.com> wrote:
Is this not exposed in any way in QEMU? Do we really need to use this (what we're trying to avoid)?
That works with the following change: diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 768cef1..1b20a7f 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -2510,6 +2510,7 @@ struct virQEMUCapsCommandLineProps { static struct virQEMUCapsCommandLineProps virQEMUCapsCommandLine[] = { { "machine", "mem-merge", QEMU_CAPS_MEM_MERGE }, + { "machine", "vmport", QEMU_CAPS_MACHINE_VMPORT_OPT }, { "drive", "discard", QEMU_CAPS_DRIVE_DISCARD }, { "realtime", "mlock", QEMU_CAPS_MLOCK }, { "boot-opts", "strict", QEMU_CAPS_BOOT_STRICT }, @@ -3243,10 +3244,6 @@ virQEMUCapsInitQMPMonitor(virQEMUCapsPtr qemuCaps, if (qemuCaps->version >= 1003000) virQEMUCapsSet(qemuCaps, QEMU_CAPS_MACHINE_USB_OPT); - /* vmport option is supported v2.2.0 onwards */ - if (qemuCaps->version >= 2002000) - virQEMUCapsSet(qemuCaps, QEMU_CAPS_MACHINE_VMPORT_OPT); I can resend this patch if necessary. -- Marc-André Lureau
[adding qemu] On 04/14/2015 09:58 AM, Marc-André Lureau wrote:
Hi
On Tue, Apr 14, 2015 at 4:25 PM, Martin Kletzander <mkletzan@redhat.com> wrote:
Is this not exposed in any way in QEMU? Do we really need to use this (what we're trying to avoid)?
That works with the following change:
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 768cef1..1b20a7f 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -2510,6 +2510,7 @@ struct virQEMUCapsCommandLineProps {
static struct virQEMUCapsCommandLineProps virQEMUCapsCommandLine[] = { { "machine", "mem-merge", QEMU_CAPS_MEM_MERGE }, + { "machine", "vmport", QEMU_CAPS_MACHINE_VMPORT_OPT },
Ouch. qemu commit 0a7cf21 fixes what would have been a regression in 2.3 at exposing "mem-merge" through query-command-line-options, but it does NOT expose "vmport", which is a per-architecture option rather than a generic -machine option. Which means that even though qemu 2.2 (perhaps wrongly) advertised "vmport" for all machines (even when it was not supported), 2.3 will not advertise it, and we are hoping for a better solution in 2.4 for properly advertising vmport on an as-appropriate basis. Yes, we WANT to use QMP probing,...
{ "drive", "discard", QEMU_CAPS_DRIVE_DISCARD }, { "realtime", "mlock", QEMU_CAPS_MLOCK }, { "boot-opts", "strict", QEMU_CAPS_BOOT_STRICT }, @@ -3243,10 +3244,6 @@ virQEMUCapsInitQMPMonitor(virQEMUCapsPtr qemuCaps, if (qemuCaps->version >= 1003000) virQEMUCapsSet(qemuCaps, QEMU_CAPS_MACHINE_USB_OPT);
- /* vmport option is supported v2.2.0 onwards */ - if (qemuCaps->version >= 2002000) - virQEMUCapsSet(qemuCaps, QEMU_CAPS_MACHINE_VMPORT_OPT);
...and not version comparison, but we'll need something better in QMP for 2.3 (which is rather late, since we missed 2.3-rc3) if you can't come up with anything better for learning whether vmport is supported. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On Tue, Apr 14, 2015 at 10:07:00AM -0600, Eric Blake wrote:
[adding qemu]
On 04/14/2015 09:58 AM, Marc-André Lureau wrote:
Hi
On Tue, Apr 14, 2015 at 4:25 PM, Martin Kletzander <mkletzan@redhat.com> wrote:
Is this not exposed in any way in QEMU? Do we really need to use this (what we're trying to avoid)?
That works with the following change:
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 768cef1..1b20a7f 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -2510,6 +2510,7 @@ struct virQEMUCapsCommandLineProps {
static struct virQEMUCapsCommandLineProps virQEMUCapsCommandLine[] = { { "machine", "mem-merge", QEMU_CAPS_MEM_MERGE }, + { "machine", "vmport", QEMU_CAPS_MACHINE_VMPORT_OPT },
Ouch. qemu commit 0a7cf21 fixes what would have been a regression in 2.3 at exposing "mem-merge" through query-command-line-options, but it does NOT expose "vmport", which is a per-architecture option rather than a generic -machine option. Which means that even though qemu 2.2 (perhaps wrongly) advertised "vmport" for all machines (even when it was not supported), 2.3 will not advertise it, and we are hoping for a better solution in 2.4 for properly advertising vmport on an as-appropriate basis.
Yes, we WANT to use QMP probing,...
{ "drive", "discard", QEMU_CAPS_DRIVE_DISCARD }, { "realtime", "mlock", QEMU_CAPS_MLOCK }, { "boot-opts", "strict", QEMU_CAPS_BOOT_STRICT }, @@ -3243,10 +3244,6 @@ virQEMUCapsInitQMPMonitor(virQEMUCapsPtr qemuCaps, if (qemuCaps->version >= 1003000) virQEMUCapsSet(qemuCaps, QEMU_CAPS_MACHINE_USB_OPT);
- /* vmport option is supported v2.2.0 onwards */ - if (qemuCaps->version >= 2002000) - virQEMUCapsSet(qemuCaps, QEMU_CAPS_MACHINE_VMPORT_OPT);
...and not version comparison, but we'll need something better in QMP for 2.3 (which is rather late, since we missed 2.3-rc3) if you can't come up with anything better for learning whether vmport is supported.
Ouch, I missed that. But that's something we need for more than just vmport attribute, but also all other machine-specific ones :( I still think this might go in, though.
Hi On Tue, Apr 14, 2015 at 6:07 PM, Eric Blake <eblake@redhat.com> wrote:
+ { "machine", "vmport", QEMU_CAPS_MACHINE_VMPORT_OPT },
Ouch. qemu commit 0a7cf21 fixes what would have been a regression in 2.3 at exposing "mem-merge" through query-command-line-options, but it does NOT expose "vmport", which is a per-architecture option rather than a generic -machine option. Which means that even though qemu 2.2 (perhaps wrongly) advertised "vmport" for all machines (even when it was not supported), 2.3 will not advertise it, and we are hoping for a better solution in 2.4 for properly advertising vmport on an as-appropriate basis.
Thanks Eric for finding out this regression. Is anybody working on a better solution? I can imagine either a new qmp machine-list-properties, or perhaps more reasonably a new properties array in query-machines reply. -- Marc-André Lureau
On 20 April 2015 at 13:39, Marc-André Lureau <marcandre.lureau@gmail.com> wrote:
Hi
On Tue, Apr 14, 2015 at 6:07 PM, Eric Blake <eblake@redhat.com> wrote:
+ { "machine", "vmport", QEMU_CAPS_MACHINE_VMPORT_OPT },
Ouch. qemu commit 0a7cf21 fixes what would have been a regression in 2.3 at exposing "mem-merge" through query-command-line-options, but it does NOT expose "vmport", which is a per-architecture option rather than a generic -machine option. Which means that even though qemu 2.2 (perhaps wrongly) advertised "vmport" for all machines (even when it was not supported), 2.3 will not advertise it, and we are hoping for a better solution in 2.4 for properly advertising vmport on an as-appropriate basis.
Thanks Eric for finding out this regression.
Is anybody working on a better solution? I can imagine either a new qmp machine-list-properties, or perhaps more reasonably a new properties array in query-machines reply.
Unless this is an absolutely critical "we cannot ship 2.3 without a fix" bug (in which case somebody should yell now and you really need a patch on the list within a day or so), you've probably missed the 2.3 boat at this point. -- PMM
On Tue, Apr 14, 2015 at 05:58:45PM +0200, Marc-André Lureau wrote:
Hi
On Tue, Apr 14, 2015 at 4:25 PM, Martin Kletzander <mkletzan@redhat.com> wrote:
Is this not exposed in any way in QEMU? Do we really need to use this (what we're trying to avoid)?
That works with the following change:
ACK with that squashed in, no need to resend it ;)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 768cef1..1b20a7f 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -2510,6 +2510,7 @@ struct virQEMUCapsCommandLineProps {
static struct virQEMUCapsCommandLineProps virQEMUCapsCommandLine[] = { { "machine", "mem-merge", QEMU_CAPS_MEM_MERGE }, + { "machine", "vmport", QEMU_CAPS_MACHINE_VMPORT_OPT }, { "drive", "discard", QEMU_CAPS_DRIVE_DISCARD }, { "realtime", "mlock", QEMU_CAPS_MLOCK }, { "boot-opts", "strict", QEMU_CAPS_BOOT_STRICT }, @@ -3243,10 +3244,6 @@ virQEMUCapsInitQMPMonitor(virQEMUCapsPtr qemuCaps, if (qemuCaps->version >= 1003000) virQEMUCapsSet(qemuCaps, QEMU_CAPS_MACHINE_USB_OPT);
- /* vmport option is supported v2.2.0 onwards */ - if (qemuCaps->version >= 2002000) - virQEMUCapsSet(qemuCaps, QEMU_CAPS_MACHINE_VMPORT_OPT);
I can resend this patch if necessary.
-- Marc-André Lureau
On 04/14/2015 10:24 AM, Martin Kletzander wrote:
That works with the following change:
ACK with that squashed in, no need to resend it ;)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 768cef1..1b20a7f 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -2510,6 +2510,7 @@ struct virQEMUCapsCommandLineProps {
static struct virQEMUCapsCommandLineProps virQEMUCapsCommandLine[] = { { "machine", "mem-merge", QEMU_CAPS_MEM_MERGE }, + { "machine", "vmport", QEMU_CAPS_MACHINE_VMPORT_OPT },
But this will break on qemu 2.3, unless something changes there. I don't want to do a version check, but I also don't want something that works by accident in qemu 2.2 start failing with qemu 2.3. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On Tue, Apr 14, 2015 at 10:28:23AM -0600, Eric Blake wrote:
On 04/14/2015 10:24 AM, Martin Kletzander wrote:
That works with the following change:
ACK with that squashed in, no need to resend it ;)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 768cef1..1b20a7f 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -2510,6 +2510,7 @@ struct virQEMUCapsCommandLineProps {
static struct virQEMUCapsCommandLineProps virQEMUCapsCommandLine[] = { { "machine", "mem-merge", QEMU_CAPS_MEM_MERGE }, + { "machine", "vmport", QEMU_CAPS_MACHINE_VMPORT_OPT },
But this will break on qemu 2.3, unless something changes there. I don't want to do a version check, but I also don't want something that works by accident in qemu 2.2 start failing with qemu 2.3.
I'm failing to find any compromise here. We would have to wait for QEMU to fix the introspection, but in the meantime we'd be unable to fix it ourselves. And since 2.3 is right around the corner, we'd have to wait for 2.4. Can we at least add all the patches except the hunks that set the capability and decide later on how to enable that, at least for any possible adopters to minimize their need for non-upstream patches?
On 04/15/2015 02:34 AM, Martin Kletzander wrote:
But this will break on qemu 2.3, unless something changes there. I don't want to do a version check, but I also don't want something that works by accident in qemu 2.2 start failing with qemu 2.3.
I'm failing to find any compromise here. We would have to wait for QEMU to fix the introspection, but in the meantime we'd be unable to fix it ourselves. And since 2.3 is right around the corner, we'd have to wait for 2.4.
Can we at least add all the patches except the hunks that set the capability and decide later on how to enable that, at least for any possible adopters to minimize their need for non-upstream patches?
I could live with a compromise patch that merges both approaches - query-command-line-options for the qemu versions that work, and a hard-coded version check for qemu 2.3 where it doesn't work. That way, if someone backports it to 2.2 (and includes as part of their backport the gunk to get QMP command line detection working), libvirt can use it; and when using upstream 2.3, we use it by virtue of version numbers. It's a bit annoying that the feature is machine-specific; we have to be careful that the hard-coded version check only enables the libvirt capability bit on machines where it is likely to work in qemu (that is, the reason that qemu 2.3 no longer advertises the bit is that it is not global to all machines; so if powerpc doesn't support it, we must not try to use the bit when targetting a powerpc qemu). -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
Fill qemu command line vmport argument as required. --- src/qemu/qemu_command.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index e94caea..22df0ad 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -7311,6 +7311,7 @@ qemuBuildMachineArgStr(virCommandPtr cmd, obsoleteAccel = true; } else { virBuffer buf = VIR_BUFFER_INITIALIZER; + virTristateSwitch vmport = def->features[VIR_DOMAIN_FEATURE_VMPORT]; virCommandAddArg(cmd, "-machine"); virBufferAdd(&buf, def->os.machine, -1); @@ -7328,6 +7329,19 @@ qemuBuildMachineArgStr(virCommandPtr cmd, if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_MACHINE_USB_OPT)) virBufferAddLit(&buf, ",usb=off"); + if (vmport) { + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_MACHINE_VMPORT_OPT)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("vmport is not available " + "with this QEMU binary")); + virBufferFreeAndReset(&buf); + return -1; + } + + virBufferAsprintf(&buf, ",vmport=%s", + virTristateSwitchTypeToString(vmport)); + } + if (def->mem.dump_core) { if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DUMP_GUEST_CORE)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", -- 2.1.0
Check that the vmport feature is correctly used in qemu commande line. --- .../qemuxml2argv-machine-vmport-opt.args | 6 +++++ .../qemuxml2argv-machine-vmport-opt.xml | 28 ++++++++++++++++++++++ tests/qemuxml2argvtest.c | 2 ++ 3 files changed, 36 insertions(+) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-machine-vmport-opt.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-machine-vmport-opt.xml diff --git a/tests/qemuxml2argvdata/qemuxml2argv-machine-vmport-opt.args b/tests/qemuxml2argvdata/qemuxml2argv-machine-vmport-opt.args new file mode 100644 index 0000000..ea1a11f --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-machine-vmport-opt.args @@ -0,0 +1,6 @@ +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test QEMU_AUDIO_DRV=none \ +/usr/bin/qemu \ +-S -machine pc,accel=tcg,vmport=off -m 214 -smp 1 -nographic \ +-monitor unix:/tmp/test-monitor,server,nowait -no-acpi -boot c -usb \ +-hda /dev/HostVG/QEMUGuest1 -net none -serial \ +none -parallel none diff --git a/tests/qemuxml2argvdata/qemuxml2argv-machine-vmport-opt.xml b/tests/qemuxml2argvdata/qemuxml2argv-machine-vmport-opt.xml new file mode 100644 index 0000000..671d3a9 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-machine-vmport-opt.xml @@ -0,0 +1,28 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219100</memory> + <currentMemory unit='KiB'>219100</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='i686' machine='pc'>hvm</type> + <boot dev='hd'/> + </os> + <features> + <vmport state='off'/> + </features> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu</emulator> + <disk type='block' device='disk'> + <source dev='/dev/HostVG/QEMUGuest1'/> + <target dev='hda' bus='ide'/> + <address type='drive' controller='0' bus='0' target='0' unit='0'/> + </disk> + <controller type='ide' index='0'/> + <memballoon model='virtio'/> + </devices> +</domain> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index c02555d..45c9015 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -608,6 +608,8 @@ mymain(void) DO_TEST_FAILURE("machine-core-on", QEMU_CAPS_MACHINE_OPT); DO_TEST("machine-usb-opt", QEMU_CAPS_MACHINE_OPT, QEMU_CAPS_MACHINE_USB_OPT); + DO_TEST("machine-vmport-opt", QEMU_CAPS_MACHINE_OPT, + QEMU_CAPS_MACHINE_VMPORT_OPT); DO_TEST("kvm", QEMU_CAPS_MACHINE_OPT); DO_TEST("boot-cdrom", NONE); DO_TEST("boot-network", NONE); -- 2.1.0
ping On Thu, Apr 2, 2015 at 7:02 PM, Marc-André Lureau < marcandre.lureau@gmail.com> wrote:
Hi,
The QEMU machine vmport option allows to set the VMWare IO port emulation. This emulation is useful for absolute pointer input when the guest has vmware input drivers, and is enabled by default for kvm.
However it is unnecessary for Spice-enabled VM, since the agent already handles absolute pointer and multi-monitors. Furthermore, it prevents Spice from switching to relative input since the regular ps/2 pointer driver is replaced by the vmware driver. It is thus advised to disable vmport when using a Spice VM. This will permit the Spice client to switch from absolute to relative pointer, as it may be required for certain games or applications.
The following patch series allows to configure the vmport feature. The first version sent used a domain attribute, Daniel Berrange suggested to use a feature instead. I sent a second version that didn't receive reviews. Hopefully this one will stand more chances!
cheers
Marc-André Lureau (5): docs: add domain vmport feature domain/conf: add VIR_DOMAIN_FEATURE_VMPORT qemu: add QEMU_CAPS_MACHINE_VMPORT_OPT qemu: add machine vmport argument tests: add machine-vmport-opt qemu test
docs/formatdomain.html.in | 6 +++++ docs/schemas/domaincommon.rng | 9 +++++++ src/conf/domain_conf.c | 5 +++- src/conf/domain_conf.h | 1 + src/qemu/qemu_capabilities.c | 6 +++++ src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_command.c | 14 +++++++++++ .../qemuxml2argv-machine-vmport-opt.args | 6 +++++ .../qemuxml2argv-machine-vmport-opt.xml | 28 ++++++++++++++++++++++ tests/qemuxml2argvtest.c | 2 ++ 10 files changed, 77 insertions(+), 1 deletion(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-machine-vmport-opt.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-machine-vmport-opt.xml
-- 2.1.0
-- Marc-André Lureau
On Thu, Apr 02, 2015 at 07:02:30PM +0200, Marc-André Lureau wrote:
Hi,
The QEMU machine vmport option allows to set the VMWare IO port emulation. This emulation is useful for absolute pointer input when the guest has vmware input drivers, and is enabled by default for kvm.
However it is unnecessary for Spice-enabled VM, since the agent already handles absolute pointer and multi-monitors. Furthermore, it prevents Spice from switching to relative input since the regular ps/2 pointer driver is replaced by the vmware driver. It is thus advised to disable vmport when using a Spice VM. This will permit the Spice client to switch from absolute to relative pointer, as it may be required for certain games or applications.
The following patch series allows to configure the vmport feature. The first version sent used a domain attribute, Daniel Berrange suggested to use a feature instead. I sent a second version that didn't receive reviews. Hopefully this one will stand more chances!
If there is *really* no other way how to check whether qemu supports the feature (specifying vmport= as part of the machine), ACK series with two adjustments mentioned per-patch.
participants (4)
-
Eric Blake -
Marc-André Lureau -
Martin Kletzander -
Peter Maydell