On 09/27/2016 08:24 AM, Martin Kletzander wrote: > QEMU added support for ivshmem-plain and ivshmem-doorbell. Those are > reworked varians of legacy ivshmem that are compatible from the guest > POV, but not from host's POV and have sane specification and handling. > It seems less "added support for" and more "forced libvirt to choose" between one or the other as of qemu v2.6. > Details about the newer device type can be found in qemu's commit > 5400c02b90bb: > > http://git.qemu.org/?p=qemu.git;a=commit;h=5400c02b90bb > > Signed-off-by: Martin Kletzander <mkletzan@xxxxxxxxxx> > --- > src/qemu/qemu_command.c | 88 +++++++++++++++++++++- > src/qemu/qemu_command.h | 10 +++ > .../qemuxml2argv-shmem-plain-doorbell.args | 43 +++++++++++ > .../qemuxml2argv-shmem-plain-doorbell.xml | 63 ++++++++++++++++ > tests/qemuxml2argvtest.c | 3 + > 5 files changed, 204 insertions(+), 3 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 aa8cff80e8b1..29f7130ef911 100644 > --- a/src/qemu/qemu_command.c > +++ b/src/qemu/qemu_command.c > @@ -8456,6 +8456,39 @@ qemuBuildShmemDevLegacyStr(virDomainDefPtr def, > return NULL; > } > > +char * > +qemuBuildShmemDevStr(virDomainDefPtr def, > + virDomainShmemDefPtr shmem, > + virQEMUCapsPtr qemuCaps) > +{ > + virBuffer buf = VIR_BUFFER_INITIALIZER; > + > + virBufferAsprintf(&buf, "%s", virDomainShmemModelTypeToString(shmem->model)); > + virBufferAsprintf(&buf, ",id=%s", shmem->info.alias); > + > + if (shmem->server.enabled) > + virBufferAsprintf(&buf, ",chardev=char%s", shmem->info.alias); > + else > + virBufferAsprintf(&buf, ",memdev=shmmem-%s", 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)); > + } > + > + if (qemuBuildDeviceAddressStr(&buf, def, &shmem->info, qemuCaps) < 0) { > + virBufferFreeAndReset(&buf); > + return NULL; > + } > + > + if (virBufferCheckError(&buf) < 0) Still would need to FreeAndReset - I'd be OK if it were an || to the previous if, although I know that causes agita for others. I suppose you could to the error: label path/option too. > + return NULL; > + > + return virBufferContentAndReset(&buf); > +} > + > static char * > qemuBuildShmemBackendChrStr(virLogManagerPtr logManager, > virCommandPtr cmd, > @@ -8476,6 +8509,50 @@ qemuBuildShmemBackendChrStr(virLogManagerPtr logManager, > return devstr; > } > > + > +virJSONValuePtr > +qemuBuildShmemBackendMemProps(virDomainShmemDefPtr shmem) > +{ > + char *mem_path = NULL; > + virJSONValuePtr ret = NULL; > + > + if (virAsprintf(&mem_path, "/dev/shm/%s", shmem->name) < 0) > + return NULL; > + > + virJSONValueObjectCreate(&ret, > + "s:mem-path", mem_path, > + "U:size", shmem->size, > + NULL); Hmm... perhaps not so much of an issue as previously thought... This will only be called for the -plain anyway and well shmem->size had better not be zero (since you're using "U:").. So still leaves me wondering if we should fail if a size is provided for -doorbell > + > + VIR_FREE(mem_path); > + return ret; > +} > + > + > +static char * > +qemuBuildShmemBackendMemStr(virDomainShmemDefPtr shmem) > +{ > + char *ret = NULL; > + char *alias = NULL; > + virJSONValuePtr props = qemuBuildShmemBackendMemProps(shmem); > + > + if (!props) > + return NULL; > + > + if (virAsprintf(&alias, "shmmem-%s", shmem->info.alias) < 0) > + goto cleanup; > + > + ret = virQEMUBuildObjectCommandlineFromJSON("memory-backend-file", > + alias, > + props); > + cleanup: > + VIR_FREE(alias); > + virJSONValueFree(props); > + > + return ret; > +} > + > + > static int > qemuBuildShmemCommandLine(virLogManagerPtr logManager, > virCommandPtr cmd, > @@ -8518,10 +8595,15 @@ qemuBuildShmemCommandLine(virLogManagerPtr logManager, > break; > Should there be caps checks for : QEMU_CAPS_DEVICE_IVSHMEM_PLAIN QEMU_CAPS_DEVICE_IVSHMEM_DOORBELL You added the caps in the xml2argvtest, so I'd expect them... Obviously they wouldn't work on qemu 2.5 or earlier. As long as the memory leak is handled and there's an answer for the caps checks, this is ACK-able from my POV. John > case VIR_DOMAIN_SHMEM_MODEL_IVSHMEM_PLAIN: > + if (!(devstr = qemuBuildShmemBackendMemStr(shmem))) > + return -1; > + > + virCommandAddArgList(cmd, "-object", devstr, NULL); > + VIR_FREE(devstr); > + > + /* fall-through */ > case VIR_DOMAIN_SHMEM_MODEL_IVSHMEM_DOORBELL: > - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, > - _("%s device is not supported with this QEMU binary"), > - virDomainShmemModelTypeToString(shmem->model)); > + devstr = qemuBuildShmemDevStr(def, shmem, qemuCaps); > break; > > case VIR_DOMAIN_SHMEM_MODEL_LAST: > diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h > index 2f2a6ff877e7..facc833bf886 100644 > --- a/src/qemu/qemu_command.h > +++ b/src/qemu/qemu_command.h > @@ -186,4 +186,14 @@ bool qemuCheckCCWS390AddressSupport(const virDomainDef *def, > virJSONValuePtr qemuBuildHotpluggableCPUProps(const virDomainVcpuDef *vcpu) > ATTRIBUTE_NONNULL(1); > > +virJSONValuePtr qemuBuildShmemBackendMemProps(virDomainShmemDefPtr shmem) > + ATTRIBUTE_NONNULL(1); > + > +char *qemuBuildShmemDevStr(virDomainDefPtr def, > + virDomainShmemDefPtr shmem, > + virQEMUCapsPtr qemuCaps) > + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3); > + > + > + > #endif /* __QEMU_COMMAND_H__*/ > diff --git a/tests/qemuxml2argvdata/qemuxml2argv-shmem-plain-doorbell.args b/tests/qemuxml2argvdata/qemuxml2argv-shmem-plain-doorbell.args > new file mode 100644 > index 000000000000..7abc7f8c4be5 > --- /dev/null > +++ b/tests/qemuxml2argvdata/qemuxml2argv-shmem-plain-doorbell.args > @@ -0,0 +1,43 @@ > +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 \ > +-object memory-backend-file,id=shmmem-shmem0,mem-path=/dev/shm/shmem0,\ > +size=4194304 \ > +-device ivshmem-plain,id=shmem0,memdev=shmmem-shmem0,bus=pci.0,addr=0x3 \ > +-object memory-backend-file,id=shmmem-shmem1,mem-path=/dev/shm/shmem1,\ > +size=134217728 \ > +-device ivshmem-plain,id=shmem1,memdev=shmmem-shmem1,bus=pci.0,addr=0x5 \ > +-object memory-backend-file,id=shmmem-shmem2,mem-path=/dev/shm/shmem2,\ > +size=268435456 \ > +-device ivshmem-plain,id=shmem2,memdev=shmmem-shmem2,bus=pci.0,addr=0x4 \ > +-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..0e10ce31fb1a > --- /dev/null > +++ b/tests/qemuxml2argvdata/qemuxml2argv-shmem-plain-doorbell.xml > @@ -0,0 +1,63 @@ > +<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'> > + <model type='ivshmem-plain'/> > + </shmem> > + <shmem name='shmem1'> > + <model type='ivshmem-plain'/> > + <size unit='M'>128</size> > + </shmem> > + <shmem name='shmem2'> > + <model type='ivshmem-plain'/> > + <size unit='M'>256</size> > + <address type='pci' domain='0x0000' bus='0x00' slot='0x04' function='0x0'/> > + </shmem> > + <shmem name='shmem3'> > + <model type='ivshmem-doorbell'/> > + <size unit='M'>512</size> > + <server/> > + </shmem> > + <shmem name='shmem4'> > + <model type='ivshmem-doorbell'/> > + <size unit='M'>1024</size> > + <server path='/tmp/shmem4-sock'/> > + </shmem> > + <shmem name='shmem5'> > + <model type='ivshmem-doorbell'/> > + <size unit='M'>2048</size> > + <server path='/tmp/shmem5-sock'/> > + <msi ioeventfd='off'/> > + </shmem> > + <shmem name='shmem6'> > + <model type='ivshmem-doorbell'/> > + <size unit='M'>4096</size> > + <server path='/tmp/shmem6-sock'/> > + <msi vectors='16'/> > + </shmem> > + <shmem name='shmem7'> > + <model type='ivshmem-doorbell'/> > + <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 0af71a1dbce5..03e3ca746f14 100644 > --- a/tests/qemuxml2argvtest.c > +++ b/tests/qemuxml2argvtest.c > @@ -2020,6 +2020,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); > -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list