On 10/14/22 01:05, Michal Prívozník wrote:
On 10/13/22 22:18, Jim Fehlig wrote:
On 10/13/22 09:46, Michal Prívozník wrote:
On 10/13/22 01:24, Jim Fehlig wrote:
As qemu becomes more modularized, it is important for libvirt to
advertise
availability of the modularized functionality through capabilities. This
change adds channel devices to domain capabilities, allowing clients
such
as virt-install to avoid using spicevmc channel devices when not
supported
by the target qemu.
Signed-off-by: Jim Fehlig <jfehlig@xxxxxxxx>
---
docs/formatdomaincaps.rst | 24 +++++++++++++++++++
src/conf/domain_capabilities.c | 13 ++++++++++
src/conf/domain_capabilities.h | 8 +++++++
src/conf/schemas/domaincaps.rng | 10 ++++++++
src/qemu/qemu_capabilities.c | 16 +++++++++++++
src/qemu/qemu_capabilities.h | 3 +++
.../domaincapsdata/qemu_4.2.0-q35.x86_64.xml | 7 ++++++
.../domaincapsdata/qemu_4.2.0-tcg.x86_64.xml | 7 ++++++
.../qemu_4.2.0-virt.aarch64.xml | 6 +++++
tests/domaincapsdata/qemu_4.2.0.aarch64.xml | 6 +++++
tests/domaincapsdata/qemu_4.2.0.ppc64.xml | 6 +++++
tests/domaincapsdata/qemu_4.2.0.s390x.xml | 6 +++++
tests/domaincapsdata/qemu_4.2.0.x86_64.xml | 7 ++++++
.../domaincapsdata/qemu_5.0.0-q35.x86_64.xml | 7 ++++++
.../domaincapsdata/qemu_5.0.0-tcg.x86_64.xml | 7 ++++++
.../qemu_5.0.0-virt.aarch64.xml | 6 +++++
tests/domaincapsdata/qemu_5.0.0.aarch64.xml | 6 +++++
tests/domaincapsdata/qemu_5.0.0.ppc64.xml | 6 +++++
tests/domaincapsdata/qemu_5.0.0.x86_64.xml | 7 ++++++
.../domaincapsdata/qemu_5.1.0-q35.x86_64.xml | 7 ++++++
.../domaincapsdata/qemu_5.1.0-tcg.x86_64.xml | 7 ++++++
tests/domaincapsdata/qemu_5.1.0.sparc.xml | 7 ++++++
tests/domaincapsdata/qemu_5.1.0.x86_64.xml | 7 ++++++
.../domaincapsdata/qemu_5.2.0-q35.x86_64.xml | 7 ++++++
.../domaincapsdata/qemu_5.2.0-tcg.x86_64.xml | 7 ++++++
.../qemu_5.2.0-virt.aarch64.xml | 6 +++++
tests/domaincapsdata/qemu_5.2.0.aarch64.xml | 6 +++++
tests/domaincapsdata/qemu_5.2.0.ppc64.xml | 6 +++++
tests/domaincapsdata/qemu_5.2.0.s390x.xml | 6 +++++
tests/domaincapsdata/qemu_5.2.0.x86_64.xml | 7 ++++++
.../domaincapsdata/qemu_6.0.0-q35.x86_64.xml | 7 ++++++
.../domaincapsdata/qemu_6.0.0-tcg.x86_64.xml | 7 ++++++
.../qemu_6.0.0-virt.aarch64.xml | 6 +++++
tests/domaincapsdata/qemu_6.0.0.aarch64.xml | 6 +++++
tests/domaincapsdata/qemu_6.0.0.s390x.xml | 6 +++++
tests/domaincapsdata/qemu_6.0.0.x86_64.xml | 7 ++++++
.../domaincapsdata/qemu_6.1.0-q35.x86_64.xml | 7 ++++++
.../domaincapsdata/qemu_6.1.0-tcg.x86_64.xml | 7 ++++++
tests/domaincapsdata/qemu_6.1.0.x86_64.xml | 7 ++++++
.../domaincapsdata/qemu_6.2.0-q35.x86_64.xml | 7 ++++++
.../domaincapsdata/qemu_6.2.0-tcg.x86_64.xml | 7 ++++++
.../qemu_6.2.0-virt.aarch64.xml | 7 ++++++
tests/domaincapsdata/qemu_6.2.0.aarch64.xml | 7 ++++++
tests/domaincapsdata/qemu_6.2.0.ppc64.xml | 6 +++++
tests/domaincapsdata/qemu_6.2.0.x86_64.xml | 7 ++++++
.../domaincapsdata/qemu_7.0.0-q35.x86_64.xml | 7 ++++++
.../domaincapsdata/qemu_7.0.0-tcg.x86_64.xml | 7 ++++++
.../qemu_7.0.0-virt.aarch64.xml | 7 ++++++
tests/domaincapsdata/qemu_7.0.0.aarch64.xml | 7 ++++++
tests/domaincapsdata/qemu_7.0.0.ppc64.xml | 6 +++++
tests/domaincapsdata/qemu_7.0.0.x86_64.xml | 7 ++++++
.../domaincapsdata/qemu_7.1.0-q35.x86_64.xml | 7 ++++++
.../domaincapsdata/qemu_7.1.0-tcg.x86_64.xml | 7 ++++++
tests/domaincapsdata/qemu_7.1.0.x86_64.xml | 7 ++++++
.../caps_4.2.0.x86_64.xml | 1 +
.../caps_5.0.0.riscv64.xml | 1 +
.../caps_5.0.0.x86_64.xml | 1 +
.../qemucapabilitiesdata/caps_5.1.0.sparc.xml | 1 +
.../caps_5.1.0.x86_64.xml | 1 +
.../caps_5.2.0.riscv64.xml | 1 +
.../caps_5.2.0.x86_64.xml | 1 +
.../caps_6.0.0.x86_64.xml | 1 +
.../caps_6.1.0.x86_64.xml | 1 +
.../caps_6.2.0.aarch64.xml | 1 +
.../caps_6.2.0.x86_64.xml | 1 +
.../caps_7.0.0.aarch64.xml | 1 +
.../caps_7.0.0.x86_64.xml | 1 +
.../caps_7.1.0.x86_64.xml | 1 +
68 files changed, 408 insertions(+)
diff --git a/docs/formatdomaincaps.rst b/docs/formatdomaincaps.rst
index 93d36f2702..f95d3a7083 100644
--- a/docs/formatdomaincaps.rst
+++ b/docs/formatdomaincaps.rst
@@ -565,6 +565,30 @@ USB redirdev device capabilities are exposed
under the ``redirdev`` element. For
``bus``
Options for the ``bus`` attribute of the ``<redirdev/>`` element.
+Channel device
+^^^^^^^^^^^^^^
+
+Channel device capabilities are exposed under the ``channel``
element. For instance:
+
+::
+
+ <domainCapabilities>
+ ...
+ <devices>
+ <channel supported='yes'>
+ <enum name='type'>
+ <value>pty</value>
+ <value>unix</value>
+ <value>spicevmc</value>
+ </enum>
+ </channel
+ ...
+ </devices>
+ </domainCapabilities>
+
+``type``
+ Options for the ``type`` attribute of the ``<channel/>`` element.
+
Features
~~~~~~~~
diff --git a/src/conf/domain_capabilities.c
b/src/conf/domain_capabilities.c
index f8b2f88376..a7f256e4ec 100644
--- a/src/conf/domain_capabilities.c
+++ b/src/conf/domain_capabilities.c
@@ -574,6 +574,18 @@ virDomainCapsDeviceRedirdevFormat(virBuffer *buf,
}
+static void
+virDomainCapsDeviceChannelFormat(virBuffer *buf,
+ const virDomainCapsDeviceChannel
*channel)
+{
+ FORMAT_PROLOGUE(channel);
+
+ ENUM_PROCESS(channel, type, virDomainChrTypeToString);
+
+ FORMAT_EPILOGUE(channel);
+}
+
+
/**
* virDomainCapsFeatureGICFormat:
* @buf: target buffer
@@ -688,6 +700,7 @@ virDomainCapsFormat(const virDomainCaps *caps)
virDomainCapsDeviceFilesystemFormat(&buf, &caps->filesystem);
virDomainCapsDeviceTPMFormat(&buf, &caps->tpm);
virDomainCapsDeviceRedirdevFormat(&buf, &caps->redirdev);
+ virDomainCapsDeviceChannelFormat(&buf, &caps->channel);
virBufferAdjustIndent(&buf, -2);
virBufferAddLit(&buf, "</devices>\n");
diff --git a/src/conf/domain_capabilities.h
b/src/conf/domain_capabilities.h
index ba7c2a5e42..e0cfa75531 100644
--- a/src/conf/domain_capabilities.h
+++ b/src/conf/domain_capabilities.h
@@ -137,6 +137,13 @@ struct _virDomainCapsDeviceRedirdev {
virDomainCapsEnum bus; /* virDomainRedirdevBus */
};
+STATIC_ASSERT_ENUM(VIR_DOMAIN_CHR_TYPE_LAST);
+typedef struct _virDomainCapsDeviceChannel virDomainCapsDeviceChannel;
+struct _virDomainCapsDeviceChannel {
+ virTristateBool supported;
+ virDomainCapsEnum type; /* virDomainChrType */
+};
+
STATIC_ASSERT_ENUM(VIR_DOMAIN_FS_DRIVER_TYPE_LAST);
typedef struct _virDomainCapsDeviceFilesystem
virDomainCapsDeviceFilesystem;
struct _virDomainCapsDeviceFilesystem {
@@ -234,6 +241,7 @@ struct _virDomainCaps {
virDomainCapsDeviceFilesystem filesystem;
virDomainCapsDeviceTPM tpm;
virDomainCapsDeviceRedirdev redirdev;
+ virDomainCapsDeviceChannel channel;
/* add new domain devices here */
virDomainCapsFeatureGIC gic;
diff --git a/src/conf/schemas/domaincaps.rng
b/src/conf/schemas/domaincaps.rng
index cf7a1d1d89..a6747b20ef 100644
--- a/src/conf/schemas/domaincaps.rng
+++ b/src/conf/schemas/domaincaps.rng
@@ -201,6 +201,9 @@
<optional>
<ref name="redirdev"/>
</optional>
+ <optional>
+ <ref name="channel"/>
+ </optional>
</element>
</define>
@@ -260,6 +263,13 @@
</element>
</define>
+ <define name="channel">
+ <element name="channel">
+ <ref name="supported"/>
+ <ref name="enum"/>
+ </element>
+ </define>
+
<define name="features">
<element name="features">
<optional>
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 5a664ec628..98849daf4c 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -1392,6 +1392,7 @@ struct virQEMUCapsStringFlags
virQEMUCapsObjectTypes[] = {
{ "s390-pv-guest", QEMU_CAPS_S390_PV_GUEST },
{ "virtio-mem-pci", QEMU_CAPS_DEVICE_VIRTIO_MEM_PCI },
{ "virtio-iommu-pci", QEMU_CAPS_DEVICE_VIRTIO_IOMMU_PCI },
+ { "chardev-spicevmc", X_QEMU_CAPS_CHARDEV_SPICEVMC },
The X_ prefix means that the capability was retired beause we are sure
QEMU has it, always. Or another capability reflects the same. In this
specific case I'd say QEMU_CAPS_SPICE is sufficient.
If you mean s/X_QEMU_CAPS_CHARDEV_SPICEVMC/QEMU_CAPS_SPICE/, it doesn't
work. 'spicevmc' is not shown as a supported channel type in
domcapabilities.
I thought more like:
diff --git i/src/qemu/qemu_capabilities.c w/src/qemu/qemu_capabilities.c
index 98849daf4c..c2f6de9363 100644
--- i/src/qemu/qemu_capabilities.c
+++ w/src/qemu/qemu_capabilities.c
@@ -1392,7 +1392,6 @@ struct virQEMUCapsStringFlags virQEMUCapsObjectTypes[] = {
{ "s390-pv-guest", QEMU_CAPS_S390_PV_GUEST },
{ "virtio-mem-pci", QEMU_CAPS_DEVICE_VIRTIO_MEM_PCI },
{ "virtio-iommu-pci", QEMU_CAPS_DEVICE_VIRTIO_IOMMU_PCI },
- { "chardev-spicevmc", X_QEMU_CAPS_CHARDEV_SPICEVMC },
};
@@ -6357,7 +6356,7 @@ virQEMUCapsFillDomainDeviceChannelCaps(virQEMUCaps *qemuCaps,
VIR_DOMAIN_CAPS_ENUM_SET(channel->type,
VIR_DOMAIN_CHR_TYPE_PTY, VIR_DOMAIN_CHR_TYPE_UNIX);
- if (virQEMUCapsGet(qemuCaps, X_QEMU_CAPS_CHARDEV_SPICEVMC))
+ if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_SPICE))
VIR_DOMAIN_CAPS_ENUM_SET(channel->type, VIR_DOMAIN_CHR_TYPE_SPICEVMC);
}
(diff against this patch, minus qemucapabilitiessdata/ modifications)
This renders the same result as your patch. At least according to our test suite.
Yes, it works, but still reports a spicevmc channel device even when
chardev-spice.so is not installed :-).
I thought it was safe to use the X_ variant since it is
already included in CapsFlags
https://gitlab.com/libvirt/libvirt/-/blob/master/src/qemu/qemu_capabilities.h#L121
Well, define safe :-) It is safe in a sense that we can introduce
capabilities as we please, because we have this cache mechanism that
makes libvirt requery caps when needed. And let's leave migration aside
for a moment. But, it's not desirable when we already have a capability
that reflects the same information. In other words, the commit that
retired QEMU_CAPS_CHARDEV_SPICEVMC (v4.3.0-rc1~322) claims, that
qemu-1.2.0 and newer do support -chardev spicevmc. What's not written in
the commit message (and probably should), is that it's enough for us to
rely on QEMU_CAPS_SPICE. But this can be seen in the code (if you know
where to look, which I admit is not developer friendly):
1) qemuValidateDomainChrSourceDef() makes sure that if there's
VIR_DOMAIN_CHR_TYPE_SPICEVMC in the domain XML there's also <graphics
type='spice'/>, then
2) qemuValidateDomainDeviceDefGraphics() makes sure that if there's
spice graphics in the domain XML, QEMU has QEMU_CAPS_SPICE.
Let me break down step 2 a bit more, because it's obfuscated a bit (in
pursuit of code deduplication). The
virQEMUCapsFillDomainDeviceGraphicsCaps() is called, which fills a
portion of domaincaps with supported graphics types (by looking at
qemuCaps), and then VIR_DOMAIN_CAPS_ENUM_IS_SET() checks whether
VIR_DOMAIN_GRAPHICS_TYPE_SPICE was set in the domaincaps.
And indeed, looking into qemu's code base (chardev/meson.build):
if spice.found()
module_ss = ss.source_set()
module_ss.add(when: [spice], if_true: files('spice.c'))
chardev_modules += { 'spice': module_ss }
endif
So it seems that spicevmc can't be compiled out and is available
whenever spice is. The reason we had that capability was that as
qemu/spice gained new features, there was a time when spice was
available in qemu but spicevmc wasn't. But no qemu we support now (4.2.0
and newer - see QEMU_MIN_MAJOR and friends in qemu_capabilities.c)
allows such situation.
I hope this helps.
Yes, it does. Thanks for the detailed response! I suppose we'll need to rethink
the dependencies in our downstream packages. E.g. currently it's possible to
install qemu-ui-spice-core (which contains ui-spice-core.so), but not
qemu-chardev-spice (which contains chardev-spice.so). That seems to fly in the
face of the upstream logic.
Jim
