[libvirt] [PATCH 0/7] qemu: Trivially add support for newer ivshmem
It's "trivially" because it's just about using different parameters. It is a series because I wanted to make some code paths saner and some output nicer, but I tried keeping every patch self-contained and as small as possible while keeping the readability. Also I wanted to make the series to be looked upon by more people by just using the word "trivial". Sorry for that =) Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1347049 Martin Kletzander (7): qemu: Add capabilities for ivshmem-{plain,doorbell} qemu: Save various defaults for shmem qemu: Make qemuBuildShmemDevStr static qemu: Rename qemuBuildShmemDevStr to qemuBuildShmemDevLegacyStr qemu: Move common checks outside qemuBuildShmemDevLegacyStr qemu: Reorder shmem params nicely qemu: Support newer ivshmem and prefer that over the legacy one src/qemu/qemu_capabilities.c | 4 + src/qemu/qemu_capabilities.h | 2 + src/qemu/qemu_command.c | 144 +++++++++++++++------ src/qemu/qemu_command.h | 4 - src/qemu/qemu_domain.c | 18 +++ .../caps_2.6.0-gicv2.aarch64.xml | 2 + .../caps_2.6.0-gicv3.aarch64.xml | 2 + tests/qemucapabilitiesdata/caps_2.6.0.ppc64le.xml | 2 + tests/qemucapabilitiesdata/caps_2.6.0.x86_64.xml | 2 + tests/qemucapabilitiesdata/caps_2.7.0.x86_64.xml | 2 + .../qemuxml2argv-shmem-plain-doorbell.args | 42 ++++++ .../qemuxml2argv-shmem-plain-doorbell.xml} | 14 +- tests/qemuxml2argvdata/qemuxml2argv-shmem.args | 16 +-- tests/qemuxml2argvtest.c | 3 + tests/qemuxml2xmloutdata/qemuxml2xmlout-shmem.xml | 1 + 15 files changed, 193 insertions(+), 65 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-shmem-plain-doorbell.args copy tests/{qemuxml2xmloutdata/qemuxml2xmlout-shmem.xml => qemuxml2argvdata/qemuxml2argv-shmem-plain-doorbell.xml} (66%) -- 2.9.2
Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- src/qemu/qemu_capabilities.c | 4 ++++ src/qemu/qemu_capabilities.h | 2 ++ tests/qemucapabilitiesdata/caps_2.6.0-gicv2.aarch64.xml | 2 ++ tests/qemucapabilitiesdata/caps_2.6.0-gicv3.aarch64.xml | 2 ++ tests/qemucapabilitiesdata/caps_2.6.0.ppc64le.xml | 2 ++ tests/qemucapabilitiesdata/caps_2.6.0.x86_64.xml | 2 ++ tests/qemucapabilitiesdata/caps_2.7.0.x86_64.xml | 2 ++ 7 files changed, 16 insertions(+) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 43e3ea750a92..b0d1daeefe96 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -340,6 +340,8 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST, "display", /* 230 */ "intel-iommu", "smm", + "ivshmem-plain", + "ivshmem-doorbell", ); @@ -1561,6 +1563,8 @@ struct virQEMUCapsStringFlags virQEMUCapsObjectTypes[] = { { "pxb-pcie", QEMU_CAPS_DEVICE_PXB_PCIE }, { "tls-creds-x509", QEMU_CAPS_OBJECT_TLS_CREDS_X509 }, { "intel-iommu", QEMU_CAPS_DEVICE_INTEL_IOMMU }, + { "ivshmem-plain", QEMU_CAPS_DEVICE_IVSHMEM_PLAIN }, + { "ivshmem-doorbell", QEMU_CAPS_DEVICE_IVSHMEM_DOORBELL }, }; static struct virQEMUCapsStringFlags virQEMUCapsObjectPropsVirtioBalloon[] = { diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index d249e2e1f4ef..7cbd553a47ea 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -373,6 +373,8 @@ typedef enum { QEMU_CAPS_DISPLAY, /* -display */ QEMU_CAPS_DEVICE_INTEL_IOMMU, /* -device intel-iommu */ QEMU_CAPS_MACHINE_SMM_OPT, /* -machine xxx,smm=on/off/auto */ + QEMU_CAPS_DEVICE_IVSHMEM_PLAIN, /* -device ivshmem-plain */ + QEMU_CAPS_DEVICE_IVSHMEM_DOORBELL, /* -device ivshmem-doorbell */ QEMU_CAPS_LAST /* this must always be the last item */ } virQEMUCapsFlags; diff --git a/tests/qemucapabilitiesdata/caps_2.6.0-gicv2.aarch64.xml b/tests/qemucapabilitiesdata/caps_2.6.0-gicv2.aarch64.xml index 85d7d3fbf9ed..6bc9b641403f 100644 --- a/tests/qemucapabilitiesdata/caps_2.6.0-gicv2.aarch64.xml +++ b/tests/qemucapabilitiesdata/caps_2.6.0-gicv2.aarch64.xml @@ -158,6 +158,8 @@ <flag name='tls-creds-x509'/> <flag name='display'/> <flag name='smm'/> + <flag name='ivshmem-plain'/> + <flag name='ivshmem-doorbell'/> <version>2005094</version> <kvmVersion>0</kvmVersion> <package></package> diff --git a/tests/qemucapabilitiesdata/caps_2.6.0-gicv3.aarch64.xml b/tests/qemucapabilitiesdata/caps_2.6.0-gicv3.aarch64.xml index deb12577e341..7c4021c21238 100644 --- a/tests/qemucapabilitiesdata/caps_2.6.0-gicv3.aarch64.xml +++ b/tests/qemucapabilitiesdata/caps_2.6.0-gicv3.aarch64.xml @@ -158,6 +158,8 @@ <flag name='tls-creds-x509'/> <flag name='display'/> <flag name='smm'/> + <flag name='ivshmem-plain'/> + <flag name='ivshmem-doorbell'/> <version>2005094</version> <kvmVersion>0</kvmVersion> <package></package> diff --git a/tests/qemucapabilitiesdata/caps_2.6.0.ppc64le.xml b/tests/qemucapabilitiesdata/caps_2.6.0.ppc64le.xml index 2b7ea0e8cc8c..c9894ab12131 100644 --- a/tests/qemucapabilitiesdata/caps_2.6.0.ppc64le.xml +++ b/tests/qemucapabilitiesdata/caps_2.6.0.ppc64le.xml @@ -152,6 +152,8 @@ <flag name='tls-creds-x509'/> <flag name='display'/> <flag name='smm'/> + <flag name='ivshmem-plain'/> + <flag name='ivshmem-doorbell'/> <version>2005094</version> <kvmVersion>0</kvmVersion> <package></package> diff --git a/tests/qemucapabilitiesdata/caps_2.6.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_2.6.0.x86_64.xml index 495c1140b177..a7e7f752b0ab 100644 --- a/tests/qemucapabilitiesdata/caps_2.6.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_2.6.0.x86_64.xml @@ -195,6 +195,8 @@ <flag name='display'/> <flag name='intel-iommu'/> <flag name='smm'/> + <flag name='ivshmem-plain'/> + <flag name='ivshmem-doorbell'/> <version>2006000</version> <kvmVersion>0</kvmVersion> <package></package> diff --git a/tests/qemucapabilitiesdata/caps_2.7.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_2.7.0.x86_64.xml index fafffa62f707..c1d7c0a30fff 100644 --- a/tests/qemucapabilitiesdata/caps_2.7.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_2.7.0.x86_64.xml @@ -192,6 +192,8 @@ <flag name='display'/> <flag name='intel-iommu'/> <flag name='smm'/> + <flag name='ivshmem-plain'/> + <flag name='ivshmem-doorbell'/> <version>2006091</version> <kvmVersion>0</kvmVersion> <package> (v2.7.0-rc1-52-g42e0d60)</package> -- 2.9.2
When we change the device used for the shared memory, it should not change the settings, so rather save them upfront then hving problems later. Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- src/qemu/qemu_domain.c | 18 ++++++++++++++++++ tests/qemuxml2argvdata/qemuxml2argv-shmem.args | 2 +- tests/qemuxml2xmloutdata/qemuxml2xmlout-shmem.xml | 1 + 3 files changed, 20 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index efc46f991692..78d5acd380ec 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -2617,6 +2617,24 @@ qemuDomainDeviceDefPostParse(virDomainDeviceDefPtr dev, } } + if (dev->type == VIR_DOMAIN_DEVICE_SHMEM) { + if (!dev->data.shmem->server.enabled) { + if (!dev->data.shmem->size) + dev->data.shmem->size = 4 << 20; + } else { + /* Defaults/Requirements for the newer device that we should save */ + if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_IVSHMEM_DOORBELL)) { + /* Size does not make much sense when claiming memory from + * the server and so the newer version doesn't support that */ + dev->data.shmem->size = 0; + + dev->data.shmem->msi.enabled = true; + if (!dev->data.shmem->msi.ioeventfd) + dev->data.shmem->msi.ioeventfd = VIR_TRISTATE_SWITCH_ON; + } + } + } + ret = 0; cleanup: diff --git a/tests/qemuxml2argvdata/qemuxml2argv-shmem.args b/tests/qemuxml2argvdata/qemuxml2argv-shmem.args index 89caf499f8dd..debfb531fc92 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-shmem.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-shmem.args @@ -17,7 +17,7 @@ QEMU_AUDIO_DRV=none \ -no-acpi \ -boot c \ -usb \ --device ivshmem,shm=shmem0,id=shmem0,bus=pci.0,addr=0x3 \ +-device ivshmem,size=4m,shm=shmem0,id=shmem0,bus=pci.0,addr=0x3 \ -device ivshmem,size=128m,shm=shmem1,id=shmem1,bus=pci.0,addr=0x5 \ -device ivshmem,size=256m,shm=shmem2,id=shmem2,bus=pci.0,addr=0x4 \ -device ivshmem,size=512m,chardev=charshmem3,id=shmem3,bus=pci.0,addr=0x6 \ diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-shmem.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-shmem.xml index 1197f361e3c4..5c4a91d850e8 100644 --- a/tests/qemuxml2xmloutdata/qemuxml2xmlout-shmem.xml +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-shmem.xml @@ -22,6 +22,7 @@ <input type='keyboard' bus='ps2'/> <memballoon model='none'/> <shmem name='shmem0'> + <size unit='M'>4</size> <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/> </shmem> <shmem name='shmem1'> -- 2.9.2
It isn't used anywhere else. Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- src/qemu/qemu_command.c | 2 +- src/qemu/qemu_command.h | 4 ---- 2 files changed, 1 insertion(+), 5 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 55df23dd40e6..713c665f2b03 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -8405,7 +8405,7 @@ qemuBuildSmartcardCommandLine(virLogManagerPtr logManager, } -char * +static char * qemuBuildShmemDevStr(virDomainDefPtr def, virDomainShmemDefPtr shmem, virQEMUCapsPtr qemuCaps) diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h index dcf9ba63382a..961454d2185d 100644 --- a/src/qemu/qemu_command.h +++ b/src/qemu/qemu_command.h @@ -142,10 +142,6 @@ int qemuBuildRNGBackendProps(virDomainRNGDefPtr rng, const char **type, virJSONValuePtr *props); -char *qemuBuildShmemDevStr(virDomainDefPtr def, - virDomainShmemDefPtr shmem, - virQEMUCapsPtr qemuCaps); - int qemuOpenPCIConfig(virDomainHostdevDefPtr dev); /* Current, best practice */ -- 2.9.2
This will make sense after adding support for newer device types. Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- src/qemu/qemu_command.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 713c665f2b03..7aaadade6fbc 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -8406,9 +8406,9 @@ qemuBuildSmartcardCommandLine(virLogManagerPtr logManager, static char * -qemuBuildShmemDevStr(virDomainDefPtr def, - virDomainShmemDefPtr shmem, - virQEMUCapsPtr qemuCaps) +qemuBuildShmemDevLegacyStr(virDomainDefPtr def, + virDomainShmemDefPtr shmem, + virQEMUCapsPtr qemuCaps) { virBuffer buf = VIR_BUFFER_INITIALIZER; @@ -8507,7 +8507,7 @@ qemuBuildShmemCommandLine(virLogManagerPtr logManager, { char *devstr = NULL; - if (!(devstr = qemuBuildShmemDevStr(def, shmem, qemuCaps))) + if (!(devstr = qemuBuildShmemDevLegacyStr(def, shmem, qemuCaps))) return -1; virCommandAddArgList(cmd, "-device", devstr, NULL); VIR_FREE(devstr); -- 2.9.2
Some checks will need to be performed for newer device types as well, so let's not duplicate them. Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- src/qemu/qemu_command.c | 66 +++++++++++++++++++++++++------------------------ 1 file changed, 34 insertions(+), 32 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 7aaadade6fbc..28c19b58f840 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -8412,33 +8412,9 @@ qemuBuildShmemDevLegacyStr(virDomainDefPtr def, { virBuffer buf = VIR_BUFFER_INITIALIZER; - if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_IVSHMEM)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("ivshmem device is not supported " - "with this QEMU binary")); - goto error; - } - virBufferAddLit(&buf, "ivshmem"); - if (shmem->size) { - /* - * Thanks to our parsing code, we have a guarantee that the - * size is power of two and is at least a mebibyte in size. - * But because it may change in the future, the checks are - * doubled in here. - */ - if (shmem->size & (shmem->size - 1)) { - virReportError(VIR_ERR_XML_ERROR, "%s", - _("shmem size must be a power of two")); - goto error; - } - if (shmem->size < 1024 * 1024) { - virReportError(VIR_ERR_XML_ERROR, "%s", - _("shmem size must be at least 1 MiB (1024 KiB)")); - goto error; - } + if (shmem->size) virBufferAsprintf(&buf, ",size=%llum", shmem->size >> 20); - } if (!shmem->server.enabled) { virBufferAsprintf(&buf, ",shm=%s,id=%s", shmem->name, shmem->info.alias); @@ -8454,13 +8430,6 @@ qemuBuildShmemDevLegacyStr(virDomainDefPtr def, } } - if (shmem->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("only 'pci' addresses are supported for the " - "shared memory device")); - goto error; - } - if (qemuBuildDeviceAddressStr(&buf, def, &shmem->info, qemuCaps) < 0) goto error; @@ -8507,6 +8476,39 @@ qemuBuildShmemCommandLine(virLogManagerPtr logManager, { char *devstr = NULL; + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_IVSHMEM)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("ivshmem device is not supported " + "with this QEMU binary")); + return -1; + } + + if (shmem->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("only 'pci' addresses are supported for the " + "shared memory device")); + return -1; + } + + if (shmem->size) { + /* + * Thanks to our parsing code, we have a guarantee that the + * size is power of two and is at least a mebibyte in size. + * But because it may change in the future, the checks are + * doubled in here. + */ + if (shmem->size & (shmem->size - 1)) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("shmem size must be a power of two")); + return -1; + } + if (shmem->size < 1024 * 1024) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("shmem size must be at least 1 MiB (1024 KiB)")); + return -1; + } + } + if (!(devstr = qemuBuildShmemDevLegacyStr(def, shmem, qemuCaps))) return -1; virCommandAddArgList(cmd, "-device", devstr, NULL); -- 2.9.2
Always format id first so that we don't need to do that twice in different code paths. Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- src/qemu/qemu_command.c | 6 ++++-- tests/qemuxml2argvdata/qemuxml2argv-shmem.args | 16 ++++++++-------- 2 files changed, 12 insertions(+), 10 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 28c19b58f840..2d670edcd848 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -8413,13 +8413,15 @@ qemuBuildShmemDevLegacyStr(virDomainDefPtr def, virBuffer buf = VIR_BUFFER_INITIALIZER; virBufferAddLit(&buf, "ivshmem"); + virBufferAsprintf(&buf, ",id=%s", shmem->info.alias); + if (shmem->size) virBufferAsprintf(&buf, ",size=%llum", shmem->size >> 20); if (!shmem->server.enabled) { - virBufferAsprintf(&buf, ",shm=%s,id=%s", shmem->name, shmem->info.alias); + virBufferAsprintf(&buf, ",shm=%s", shmem->name); } else { - virBufferAsprintf(&buf, ",chardev=char%s,id=%s", shmem->info.alias, shmem->info.alias); + virBufferAsprintf(&buf, ",chardev=char%s", shmem->info.alias); if (shmem->msi.enabled) { virBufferAddLit(&buf, ",msi=on"); if (shmem->msi.vectors) diff --git a/tests/qemuxml2argvdata/qemuxml2argv-shmem.args b/tests/qemuxml2argvdata/qemuxml2argv-shmem.args index debfb531fc92..bdf660a3c435 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-shmem.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-shmem.args @@ -17,19 +17,19 @@ QEMU_AUDIO_DRV=none \ -no-acpi \ -boot c \ -usb \ --device ivshmem,size=4m,shm=shmem0,id=shmem0,bus=pci.0,addr=0x3 \ --device ivshmem,size=128m,shm=shmem1,id=shmem1,bus=pci.0,addr=0x5 \ --device ivshmem,size=256m,shm=shmem2,id=shmem2,bus=pci.0,addr=0x4 \ --device ivshmem,size=512m,chardev=charshmem3,id=shmem3,bus=pci.0,addr=0x6 \ +-device ivshmem,id=shmem0,size=4m,shm=shmem0,bus=pci.0,addr=0x3 \ +-device ivshmem,id=shmem1,size=128m,shm=shmem1,bus=pci.0,addr=0x5 \ +-device ivshmem,id=shmem2,size=256m,shm=shmem2,bus=pci.0,addr=0x4 \ +-device ivshmem,id=shmem3,size=512m,chardev=charshmem3,bus=pci.0,addr=0x6 \ -chardev socket,id=charshmem3,path=/var/lib/libvirt/shmem-shmem3-sock \ --device ivshmem,size=1024m,chardev=charshmem4,id=shmem4,bus=pci.0,addr=0x7 \ +-device ivshmem,id=shmem4,size=1024m,chardev=charshmem4,bus=pci.0,addr=0x7 \ -chardev socket,id=charshmem4,path=/tmp/shmem4-sock \ --device ivshmem,size=2048m,chardev=charshmem5,id=shmem5,msi=on,ioeventfd=off,\ +-device ivshmem,id=shmem5,size=2048m,chardev=charshmem5,msi=on,ioeventfd=off,\ bus=pci.0,addr=0x8 \ -chardev socket,id=charshmem5,path=/tmp/shmem5-sock \ --device ivshmem,size=4096m,chardev=charshmem6,id=shmem6,msi=on,vectors=16,\ +-device ivshmem,id=shmem6,size=4096m,chardev=charshmem6,msi=on,vectors=16,\ bus=pci.0,addr=0x9 \ -chardev socket,id=charshmem6,path=/tmp/shmem6-sock \ --device ivshmem,size=8192m,chardev=charshmem7,id=shmem7,msi=on,vectors=32,\ +-device ivshmem,id=shmem7,size=8192m,chardev=charshmem7,msi=on,vectors=32,\ ioeventfd=on,bus=pci.0,addr=0xa \ -chardev socket,id=charshmem7,path=/tmp/shmem7-sock -- 2.9.2
QEMU added support for ivshmem-plain and ivshmem-doorbell. Those are reworked varians of legacy ivshmem that are compatible, but have sane specification and handling. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1347049 Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- src/qemu/qemu_command.c | 66 ++++++++++++++++++++-- .../qemuxml2argv-shmem-plain-doorbell.args | 42 ++++++++++++++ .../qemuxml2argv-shmem-plain-doorbell.xml | 54 ++++++++++++++++++ tests/qemuxml2argvtest.c | 3 + 4 files changed, 161 insertions(+), 4 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-shmem-plain-doorbell.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-shmem-plain-doorbell.xml diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 2d670edcd848..40a8d861503e 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -8445,6 +8445,50 @@ qemuBuildShmemDevLegacyStr(virDomainDefPtr def, return NULL; } +static int +qemuBuildShmemDevCommandLine(virCommandPtr cmd, + virDomainDefPtr def, + virDomainShmemDefPtr shmem, + virQEMUCapsPtr qemuCaps) +{ + virBuffer buf = VIR_BUFFER_INITIALIZER; + + virCommandAddArg(cmd, "-device"); + + if (shmem->server.enabled) { + virBufferAddLit(&buf, "ivshmem-doorbell"); + virBufferAsprintf(&buf, ",id=%s,chardev=char%s", + shmem->info.alias, shmem->info.alias); + if (shmem->msi.vectors) + virBufferAsprintf(&buf, ",vectors=%u", shmem->msi.vectors); + if (shmem->msi.ioeventfd) + virBufferAsprintf(&buf, ",ioeventfd=%s", + virTristateSwitchTypeToString(shmem->msi.ioeventfd)); + } else { + virBufferAddLit(&buf, "ivshmem-plain"); + virBufferAsprintf(&buf, ",id=%s,memdev=shmmem-%s", + shmem->info.alias, shmem->info.alias); + } + + if (qemuBuildDeviceAddressStr(&buf, def, &shmem->info, qemuCaps) < 0) { + virBufferFreeAndReset(&buf); + return -1; + } + + virCommandAddArgBuffer(cmd, &buf); + + if (!shmem->server.enabled) { + virBufferAddLit(&buf, "memory-backend-file"); + virBufferAsprintf(&buf, ",id=shmmem-%s,size=%llum,mem-path=/dev/shm/%s", + shmem->info.alias, shmem->size >> 20, shmem->name); + + virCommandAddArg(cmd, "-device"); + virCommandAddArgBuffer(cmd, &buf); + } + + return 0; +} + static char * qemuBuildShmemBackendStr(virLogManagerPtr logManager, virCommandPtr cmd, @@ -8511,10 +8555,24 @@ qemuBuildShmemCommandLine(virLogManagerPtr logManager, } } - if (!(devstr = qemuBuildShmemDevLegacyStr(def, shmem, qemuCaps))) - return -1; - virCommandAddArgList(cmd, "-device", devstr, NULL); - VIR_FREE(devstr); + if (virQEMUCapsGet(qemuCaps, shmem->server.enabled ? + QEMU_CAPS_DEVICE_IVSHMEM_DOORBELL : + QEMU_CAPS_DEVICE_IVSHMEM_PLAIN)) { + if (qemuBuildShmemDevCommandLine(cmd, def, shmem, qemuCaps) < 0) + return -1; + } else { + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_IVSHMEM)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("ivshmem device is not supported " + "with this QEMU binary")); + return -1; + } + + if (!(devstr = qemuBuildShmemDevLegacyStr(def, shmem, qemuCaps))) + return -1; + virCommandAddArgList(cmd, "-device", devstr, NULL); + VIR_FREE(devstr); + } if (shmem->server.enabled) { if (!(devstr = qemuBuildShmemBackendStr(logManager, cmd, cfg, def, diff --git a/tests/qemuxml2argvdata/qemuxml2argv-shmem-plain-doorbell.args b/tests/qemuxml2argvdata/qemuxml2argv-shmem-plain-doorbell.args new file mode 100644 index 000000000000..3f246f7a018e --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-shmem-plain-doorbell.args @@ -0,0 +1,42 @@ +LC_ALL=C \ +PATH=/bin \ +HOME=/home/test \ +USER=test \ +LOGNAME=test \ +QEMU_AUDIO_DRV=none \ +/usr/bin/qemu \ +-name QEMUGuest1 \ +-S \ +-M pc \ +-m 214 \ +-smp 1,sockets=1,cores=1,threads=1 \ +-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \ +-nographic \ +-nodefaults \ +-monitor unix:/tmp/lib/domain--1-QEMUGuest1/monitor.sock,server,nowait \ +-no-acpi \ +-boot c \ +-usb \ +-device ivshmem-plain,id=shmem0,memdev=shmmem-shmem0,bus=pci.0,addr=0x3 \ +-device memory-backend-file,id=shmmem-shmem0,size=4m,mem-path=/dev/shm/shmem0 \ +-device ivshmem-plain,id=shmem1,memdev=shmmem-shmem1,bus=pci.0,addr=0x5 \ +-device memory-backend-file,id=shmmem-shmem1,size=128m,\ +mem-path=/dev/shm/shmem1 \ +-device ivshmem-plain,id=shmem2,memdev=shmmem-shmem2,bus=pci.0,addr=0x4 \ +-device memory-backend-file,id=shmmem-shmem2,size=256m,\ +mem-path=/dev/shm/shmem2 \ +-device ivshmem-doorbell,id=shmem3,chardev=charshmem3,ioeventfd=on,bus=pci.0,\ +addr=0x6 \ +-chardev socket,id=charshmem3,path=/var/lib/libvirt/shmem-shmem3-sock \ +-device ivshmem-doorbell,id=shmem4,chardev=charshmem4,ioeventfd=on,bus=pci.0,\ +addr=0x7 \ +-chardev socket,id=charshmem4,path=/tmp/shmem4-sock \ +-device ivshmem-doorbell,id=shmem5,chardev=charshmem5,ioeventfd=off,bus=pci.0,\ +addr=0x8 \ +-chardev socket,id=charshmem5,path=/tmp/shmem5-sock \ +-device ivshmem-doorbell,id=shmem6,chardev=charshmem6,vectors=16,ioeventfd=on,\ +bus=pci.0,addr=0x9 \ +-chardev socket,id=charshmem6,path=/tmp/shmem6-sock \ +-device ivshmem-doorbell,id=shmem7,chardev=charshmem7,vectors=32,ioeventfd=on,\ +bus=pci.0,addr=0xa \ +-chardev socket,id=charshmem7,path=/tmp/shmem7-sock diff --git a/tests/qemuxml2argvdata/qemuxml2argv-shmem-plain-doorbell.xml b/tests/qemuxml2argvdata/qemuxml2argv-shmem-plain-doorbell.xml new file mode 100644 index 000000000000..5bc49044894c --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-shmem-plain-doorbell.xml @@ -0,0 +1,54 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219136</memory> + <currentMemory unit='KiB'>219136</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='i686' machine='pc'>hvm</type> + <boot dev='hd'/> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu</emulator> + <controller type='usb' index='0'/> + <controller type='pci' index='0' model='pci-root'/> + <input type='mouse' bus='ps2'/> + <input type='keyboard' bus='ps2'/> + <memballoon model='none'/> + <shmem name='shmem0'/> + <shmem name='shmem1'> + <size unit='M'>128</size> + </shmem> + <shmem name='shmem2'> + <size unit='M'>256</size> + <address type='pci' domain='0x0000' bus='0x00' slot='0x04' function='0x0'/> + </shmem> + <shmem name='shmem3'> + <size unit='M'>512</size> + <server/> + </shmem> + <shmem name='shmem4'> + <size unit='M'>1024</size> + <server path='/tmp/shmem4-sock'/> + </shmem> + <shmem name='shmem5'> + <size unit='M'>2048</size> + <server path='/tmp/shmem5-sock'/> + <msi ioeventfd='off'/> + </shmem> + <shmem name='shmem6'> + <size unit='M'>4096</size> + <server path='/tmp/shmem6-sock'/> + <msi vectors='16'/> + </shmem> + <shmem name='shmem7'> + <size unit='M'>8192</size> + <server path='/tmp/shmem7-sock'/> + <msi vectors='32' ioeventfd='on'/> + </shmem> + </devices> +</domain> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index d5948360072f..8f382dd90091 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -1948,6 +1948,9 @@ mymain(void) DO_TEST("fips-enabled", QEMU_CAPS_ENABLE_FIPS); DO_TEST("shmem", QEMU_CAPS_DEVICE_IVSHMEM); + DO_TEST("shmem-plain-doorbell", QEMU_CAPS_DEVICE_IVSHMEM, + QEMU_CAPS_DEVICE_IVSHMEM_PLAIN, + QEMU_CAPS_DEVICE_IVSHMEM_DOORBELL); DO_TEST_FAILURE("shmem", NONE); DO_TEST_FAILURE("shmem-invalid-size", QEMU_CAPS_DEVICE_IVSHMEM); -- 2.9.2
On Wed, Aug 10, 2016 at 01:50:11PM +0200, Martin Kletzander wrote:
QEMU added support for ivshmem-plain and ivshmem-doorbell. Those are reworked varians of legacy ivshmem that are compatible, but have sane specification and handling.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1347049
Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- src/qemu/qemu_command.c | 66 ++++++++++++++++++++-- .../qemuxml2argv-shmem-plain-doorbell.args | 42 ++++++++++++++ .../qemuxml2argv-shmem-plain-doorbell.xml | 54 ++++++++++++++++++ tests/qemuxml2argvtest.c | 3 + 4 files changed, 161 insertions(+), 4 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-shmem-plain-doorbell.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-shmem-plain-doorbell.xml
Isn't this going to break migration compatibility. eg if someone has a guest running on a QEMU with ivshmem, which also has the ivshmem-plain, then when they upgrade libvirt their guest will guest the different device, which has different migration ABI ? Or is there a blocker that entirely prevents migration with ivshmem, making it a non-issue ? Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
On Thu, Aug 11, 2016 at 09:27:47AM +0100, Daniel P. Berrange wrote:
On Wed, Aug 10, 2016 at 01:50:11PM +0200, Martin Kletzander wrote:
QEMU added support for ivshmem-plain and ivshmem-doorbell. Those are reworked varians of legacy ivshmem that are compatible, but have sane specification and handling.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1347049
Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- src/qemu/qemu_command.c | 66 ++++++++++++++++++++-- .../qemuxml2argv-shmem-plain-doorbell.args | 42 ++++++++++++++ .../qemuxml2argv-shmem-plain-doorbell.xml | 54 ++++++++++++++++++ tests/qemuxml2argvtest.c | 3 + 4 files changed, 161 insertions(+), 4 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-shmem-plain-doorbell.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-shmem-plain-doorbell.xml
Isn't this going to break migration compatibility. eg if someone has a guest running on a QEMU with ivshmem, which also has the ivshmem-plain, then when they upgrade libvirt their guest will guest the different device, which has different migration ABI ?
Or is there a blocker that entirely prevents migration with ivshmem, making it a non-issue ?
ivshmem migration is forbidden if the role is "master". It now makes sense with the new ivshmem-plain, but didn't make much sense with the old one, so we didn't add support for it. Thanks to that we can make old ones just role=peer and the new ones to role=master (which are the qemu defaults anyway) and then we can check that in the ABI stability check. I forgot to do that, good point. However, I need to make sure that migration works from legacy to ivshmem-plain, otherwise we'd need to either add a model (I wouldn't like that) or forbid migration based on qemu capabilities and migration cookie flag. I'll have a look at that.
Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On Wed, Aug 10, 2016 at 01:50:04PM +0200, Martin Kletzander wrote:
It's "trivially" because it's just about using different parameters. It is a series because I wanted to make some code paths saner and some output nicer, but I tried keeping every patch self-contained and as small as possible while keeping the readability. Also I wanted to make the series to be looked upon by more people by just using the word "trivial". Sorry for that =)
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1347049
I forgot the mention that some parts of the code were based upon this commit message [1] in qemu that has more details than the spec and/or documentation from libvirt's implementation POV. I amended commit messages for patches 2 and 7 to include the link. [1] http://git.qemu.org/?p=qemu.git;a=commit;h=5400c02b90bb647a961f3210255178b68...
Martin Kletzander (7): qemu: Add capabilities for ivshmem-{plain,doorbell} qemu: Save various defaults for shmem qemu: Make qemuBuildShmemDevStr static qemu: Rename qemuBuildShmemDevStr to qemuBuildShmemDevLegacyStr qemu: Move common checks outside qemuBuildShmemDevLegacyStr qemu: Reorder shmem params nicely qemu: Support newer ivshmem and prefer that over the legacy one
src/qemu/qemu_capabilities.c | 4 + src/qemu/qemu_capabilities.h | 2 + src/qemu/qemu_command.c | 144 +++++++++++++++------ src/qemu/qemu_command.h | 4 - src/qemu/qemu_domain.c | 18 +++ .../caps_2.6.0-gicv2.aarch64.xml | 2 + .../caps_2.6.0-gicv3.aarch64.xml | 2 + tests/qemucapabilitiesdata/caps_2.6.0.ppc64le.xml | 2 + tests/qemucapabilitiesdata/caps_2.6.0.x86_64.xml | 2 + tests/qemucapabilitiesdata/caps_2.7.0.x86_64.xml | 2 + .../qemuxml2argv-shmem-plain-doorbell.args | 42 ++++++ .../qemuxml2argv-shmem-plain-doorbell.xml} | 14 +- tests/qemuxml2argvdata/qemuxml2argv-shmem.args | 16 +-- tests/qemuxml2argvtest.c | 3 + tests/qemuxml2xmloutdata/qemuxml2xmlout-shmem.xml | 1 + 15 files changed, 193 insertions(+), 65 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-shmem-plain-doorbell.args copy tests/{qemuxml2xmloutdata/qemuxml2xmlout-shmem.xml => qemuxml2argvdata/qemuxml2argv-shmem-plain-doorbell.xml} (66%)
-- 2.9.2
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
participants (2)
-
Daniel P. Berrange -
Martin Kletzander