[libvirt] [PATCH 0/3] Multiple RNG device support
Qemu and kernel support multiple RNG's, so I revived my old patch to add the support to libvirt. Peter Krempa (3): qemu: cgroup: Don't use NULL path on default backed RNGs virtio-rng: allow multiple RNG devices test: qemu: Add tests for multiple virtio-rng devices src/conf/domain_audit.c | 4 +- src/conf/domain_conf.c | 58 +++++++++++----------- src/conf/domain_conf.h | 4 +- src/qemu/qemu_cgroup.c | 27 ++++++---- src/qemu/qemu_command.c | 29 ++++++----- .../qemuxml2argv-virtio-rng-multiple.args | 9 ++++ .../qemuxml2argv-virtio-rng-multiple.xml | 31 ++++++++++++ tests/qemuxml2argvtest.c | 2 + 8 files changed, 109 insertions(+), 55 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-virtio-rng-multiple.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-virtio-rng-multiple.xml -- 2.0.0
The "random" backend for virtio-rng can be started with no path specified which equals to /dev/random. The cgroup code didn't consider this and called few of the functions with NULL resulting into: $ virsh start rng-vm error: Failed to start domain rng-vm error: Path '(null)' is not accessible: Bad address Problem introduced by commit c6320d34637a9883e31c4081d418fc33a4277cf2 --- src/qemu/qemu_cgroup.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c index 795ad90..f649d66 100644 --- a/src/qemu/qemu_cgroup.c +++ b/src/qemu/qemu_cgroup.c @@ -587,10 +587,16 @@ qemuSetupDevicesCgroup(virQEMUDriverPtr driver, if (vm->def->rng && (vm->def->rng->backend == VIR_DOMAIN_RNG_BACKEND_RANDOM)) { VIR_DEBUG("Setting Cgroup ACL for RNG device"); - rv = virCgroupAllowDevicePath(priv->cgroup, vm->def->rng->source.file, + const char *rngpath = vm->def->rng->source.file; + + /* fix path when using the default */ + if (!rngpath) + rngpath = "/dev/random"; + + rv = virCgroupAllowDevicePath(priv->cgroup, rngpath, VIR_CGROUP_DEVICE_RW); virDomainAuditCgroupPath(vm, priv->cgroup, "allow", - vm->def->rng->source.file, "rw", rv == 0); + rngpath, "rw", rv == 0); if (rv < 0 && !virLastErrorIsSystemErrno(ENOENT)) goto cleanup; -- 2.0.0
qemu supports adding multiple RNG devices. This patch allows libvirt to support this. --- src/conf/domain_audit.c | 4 ++-- src/conf/domain_conf.c | 58 ++++++++++++++++++++++++------------------------- src/conf/domain_conf.h | 4 +++- src/qemu/qemu_cgroup.c | 33 ++++++++++++++-------------- src/qemu/qemu_command.c | 29 ++++++++++++++----------- 5 files changed, 67 insertions(+), 61 deletions(-) diff --git a/src/conf/domain_audit.c b/src/conf/domain_audit.c index 3ad58b0..2d1b1fb 100644 --- a/src/conf/domain_audit.c +++ b/src/conf/domain_audit.c @@ -874,8 +874,8 @@ virDomainAuditStart(virDomainObjPtr vm, const char *reason, bool success) for (i = 0; i < vm->def->nsmartcards; i++) virDomainAuditSmartcard(vm, vm->def->smartcards[i], "start", true); - if (vm->def->rng) - virDomainAuditRNG(vm, NULL, vm->def->rng, "start", true); + for (i = 0; i < vm->def->nrngs; i++) + virDomainAuditRNG(vm, NULL, vm->def->rngs[i], "start", true); if (vm->def->tpm) virDomainAuditTPM(vm, vm->def->tpm, "start", true); diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 1b43a26..9c3cd8a 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -2057,7 +2057,9 @@ void virDomainDefFree(virDomainDefPtr def) virDomainRedirdevDefFree(def->redirdevs[i]); VIR_FREE(def->redirdevs); - virDomainRNGDefFree(def->rng); + for (i = 0; i < def->nrngs; i++) + virDomainRNGDefFree(def->rngs[i]); + VIR_FREE(def->rngs); virDomainTPMDefFree(def->tpm); @@ -2745,10 +2747,10 @@ virDomainDeviceInfoIterateInternal(virDomainDefPtr def, if (cb(def, &device, &def->memballoon->info, opaque) < 0) return -1; } - if (def->rng) { - device.type = VIR_DOMAIN_DEVICE_RNG; - device.data.rng = def->rng; - if (cb(def, &device, &def->rng->info, opaque) < 0) + device.type = VIR_DOMAIN_DEVICE_RNG; + for (i = 0; i < def->nrngs; i++) { + device.data.rng = def->rngs[i]; + if (cb(def, &device, &def->rngs[i]->info, opaque) < 0) return -1; } if (def->nvram) { @@ -12821,20 +12823,19 @@ virDomainDefParseXML(xmlDocPtr xml, VIR_FREE(nodes); } - /* Parse the RNG device */ + /* Parse the RNG devices */ if ((n = virXPathNodeSet("./devices/rng", ctxt, &nodes)) < 0) goto error; - - if (n > 1) { - virReportError(VIR_ERR_XML_ERROR, "%s", - _("only a single RNG device is supported")); + if (n && VIR_ALLOC_N(def->rngs, n) < 0) goto error; - } - - if (n > 0) { - if (!(def->rng = virDomainRNGDefParseXML(nodes[0], ctxt, flags))) + for (i = 0; i < n; i++) { + virDomainRNGDefPtr rng = virDomainRNGDefParseXML(nodes[i], + ctxt, + flags); + if (!rng) goto error; - VIR_FREE(nodes); + + def->rngs[def->nrngs++] = rng; } VIR_FREE(nodes); @@ -13864,17 +13865,6 @@ static bool virDomainRNGDefCheckABIStability(virDomainRNGDefPtr src, virDomainRNGDefPtr dst) { - if (!src && !dst) - return true; - - if (!src || !dst) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("Target domain RNG device count '%d' " - "does not match source count '%d'"), - src ? 1 : 0, dst ? 1 : 0); - return false; - } - if (src->model != dst->model) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("Target RNG model '%s' does not match source '%s'"), @@ -14439,8 +14429,16 @@ virDomainDefCheckABIStability(virDomainDefPtr src, dst->memballoon)) goto error; - if (!virDomainRNGDefCheckABIStability(src->rng, dst->rng)) + if (src->nrngs != dst->nrngs) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("Target domain RNG device count %zu " + "does not match source %zu"), dst->nrngs, src->nrngs); goto error; + } + + for (i = 0; i < src->nrngs; i++) + if (!virDomainRNGDefCheckABIStability(src->rngs[i], dst->rngs[i])) + goto error; if (!virDomainPanicCheckABIStability(src->panic, dst->panic)) goto error; @@ -18020,8 +18018,10 @@ virDomainDefFormatInternal(virDomainDefPtr def, if (def->memballoon) virDomainMemballoonDefFormat(buf, def->memballoon, flags); - if (def->rng) - virDomainRNGDefFormat(buf, def->rng, flags); + for (n = 0; n < def->nrngs; n++) { + if (virDomainRNGDefFormat(buf, def->rngs[n], flags)) + goto error; + } if (def->nvram) virDomainNVRAMDefFormat(buf, def->nvram, flags); diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 55292e1..b988b17 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1981,6 +1981,9 @@ struct _virDomainDef { size_t nseclabels; virSecurityLabelDefPtr *seclabels; + size_t nrngs; + virDomainRNGDefPtr *rngs; + /* Only 1 */ virDomainWatchdogDefPtr watchdog; virDomainMemballoonDefPtr memballoon; @@ -1989,7 +1992,6 @@ struct _virDomainDef { virCPUDefPtr cpu; virSysinfoDefPtr sysinfo; virDomainRedirFilterDefPtr redirfilter; - virDomainRNGDefPtr rng; virDomainPanicDefPtr panic; void *namespaceData; diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c index f649d66..419be9a 100644 --- a/src/qemu/qemu_cgroup.c +++ b/src/qemu/qemu_cgroup.c @@ -584,22 +584,23 @@ qemuSetupDevicesCgroup(virQEMUDriverPtr driver, goto cleanup; } - if (vm->def->rng && - (vm->def->rng->backend == VIR_DOMAIN_RNG_BACKEND_RANDOM)) { - VIR_DEBUG("Setting Cgroup ACL for RNG device"); - const char *rngpath = vm->def->rng->source.file; - - /* fix path when using the default */ - if (!rngpath) - rngpath = "/dev/random"; - - rv = virCgroupAllowDevicePath(priv->cgroup, rngpath, - VIR_CGROUP_DEVICE_RW); - virDomainAuditCgroupPath(vm, priv->cgroup, "allow", - rngpath, "rw", rv == 0); - if (rv < 0 && - !virLastErrorIsSystemErrno(ENOENT)) - goto cleanup; + for (i = 0; i < vm->def->nrngs; i++) { + if (vm->def->rngs[i]->backend == VIR_DOMAIN_RNG_BACKEND_RANDOM) { + VIR_DEBUG("Setting Cgroup ACL for RNG device"); + const char *rngpath = vm->def->rngs[i]->source.file; + + /* fix path when using the default */ + if (!rngpath) + rngpath = "/dev/random"; + + rv = virCgroupAllowDevicePath(priv->cgroup, rngpath, + VIR_CGROUP_DEVICE_RW); + virDomainAuditCgroupPath(vm, priv->cgroup, "allow", + rngpath, "rw", rv == 0); + if (rv < 0 && + !virLastErrorIsSystemErrno(ENOENT)) + goto cleanup; + } } ret = 0; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index c5e86b4..7f9357c 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -1050,8 +1050,8 @@ qemuAssignDeviceAliases(virDomainDefPtr def, virQEMUCapsPtr qemuCaps) if (virAsprintf(&def->memballoon->info.alias, "balloon%d", 0) < 0) return -1; } - if (def->rng) { - if (virAsprintf(&def->rng->info.alias, "rng%d", 0) < 0) + for (i = 0; i < def->nrngs; i++) { + if (virAsprintf(&def->rngs[i]->info.alias, "rng%zu", i) < 0) return -1; } if (def->tpm) { @@ -1102,10 +1102,11 @@ qemuDomainPrimeVirtioDeviceAddresses(virDomainDefPtr def, def->memballoon->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) def->memballoon->info.type = type; - if (def->rng && - def->rng->model == VIR_DOMAIN_RNG_MODEL_VIRTIO && - def->rng->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) - def->rng->info.type = type; + for (i = 0; i < def->nrngs; i++) { + if (def->rngs[i]->model == VIR_DOMAIN_RNG_MODEL_VIRTIO && + def->rngs[i]->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) + def->rngs[i]->info.type = type; + } } @@ -2232,11 +2233,13 @@ qemuAssignDevicePCISlots(virDomainDefPtr def, } /* VirtIO RNG */ - if (def->rng && - def->rng->model == VIR_DOMAIN_RNG_MODEL_VIRTIO && - def->rng->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) { + for (i = 0; i < def->nrngs; i++) { + if (def->rngs[i]->model != VIR_DOMAIN_RNG_MODEL_VIRTIO || + def->rngs[i]->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) + continue; + if (virDomainPCIAddressReserveNextSlot(addrs, - &def->rng->info, flags) < 0) + &def->rngs[i]->info, flags) < 0) goto error; } @@ -9163,13 +9166,13 @@ qemuBuildCommandLine(virConnectPtr conn, } } - if (def->rng) { + for (i = 0; i < def->nrngs; i++) { /* add the RNG source backend */ - if (qemuBuildRNGBackendArgs(cmd, def->rng, qemuCaps) < 0) + if (qemuBuildRNGBackendArgs(cmd, def->rngs[i], qemuCaps) < 0) goto error; /* add the device */ - if (qemuBuildRNGDeviceArgs(cmd, def, def->rng, qemuCaps) < 0) + if (qemuBuildRNGDeviceArgs(cmd, def, def->rngs[i], qemuCaps) < 0) goto error; } -- 2.0.0
--- .../qemuxml2argv-virtio-rng-multiple.args | 9 +++++++ .../qemuxml2argv-virtio-rng-multiple.xml | 31 ++++++++++++++++++++++ tests/qemuxml2argvtest.c | 2 ++ 3 files changed, 42 insertions(+) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-virtio-rng-multiple.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-virtio-rng-multiple.xml diff --git a/tests/qemuxml2argvdata/qemuxml2argv-virtio-rng-multiple.args b/tests/qemuxml2argvdata/qemuxml2argv-virtio-rng-multiple.args new file mode 100644 index 0000000..1082ede --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-virtio-rng-multiple.args @@ -0,0 +1,9 @@ +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test QEMU_AUDIO_DRV=none \ +/usr/bin/qemu -S \ +-M pc -m 214 -smp 1 -nographic -nodefaults \ +-monitor unix:/tmp/test-monitor,server,nowait -no-acpi -boot c -usb \ +-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3 \ +-object rng-random,id=rng0 -device virtio-rng-pci,rng=rng0,bus=pci.0,addr=0x7 \ +-chardev socket,id=charrng1,host=1.2.3.4,port=1234 \ +-object rng-egd,chardev=charrng1,id=rng1 \ +-device virtio-rng-pci,rng=rng1,bus=pci.0,addr=0x4 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-virtio-rng-multiple.xml b/tests/qemuxml2argvdata/qemuxml2argv-virtio-rng-multiple.xml new file mode 100644 index 0000000..31269d0 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-virtio-rng-multiple.xml @@ -0,0 +1,31 @@ +<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' cpuset='1-4,8-20,525'>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'/> + <memballoon model='virtio'/> + <rng model='virtio'> + <backend model='random'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x07' function='0x0'/> + </rng> + <rng model='virtio'> + <backend model='egd' type='tcp'> + <source mode='connect' host='1.2.3.4' service='1234'/> + <protocol type='raw'/> + </backend> + </rng> + </devices> +</domain> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 28436f2..115cd37 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -1283,6 +1283,8 @@ mymain(void) QEMU_CAPS_OBJECT_RNG_RANDOM); DO_TEST("virtio-rng-egd", QEMU_CAPS_DEVICE, QEMU_CAPS_DEVICE_VIRTIO_RNG, QEMU_CAPS_OBJECT_RNG_EGD); + DO_TEST("virtio-rng-multiple", QEMU_CAPS_DEVICE, QEMU_CAPS_DEVICE_VIRTIO_RNG, + QEMU_CAPS_OBJECT_RNG_EGD, QEMU_CAPS_OBJECT_RNG_RANDOM); DO_TEST_PARSE_ERROR("virtio-rng-egd-crash", QEMU_CAPS_DEVICE, QEMU_CAPS_DEVICE_VIRTIO_RNG, QEMU_CAPS_OBJECT_RNG_EGD); DO_TEST("virtio-rng-ccw", -- 2.0.0
On 07/24/2014 04:07 PM, Peter Krempa wrote:
Qemu and kernel support multiple RNG's, so I revived my old patch to add the support to libvirt.
Peter Krempa (3): qemu: cgroup: Don't use NULL path on default backed RNGs virtio-rng: allow multiple RNG devices test: qemu: Add tests for multiple virtio-rng devices
src/conf/domain_audit.c | 4 +- src/conf/domain_conf.c | 58 +++++++++++----------- src/conf/domain_conf.h | 4 +- src/qemu/qemu_cgroup.c | 27 ++++++---- src/qemu/qemu_command.c | 29 ++++++----- .../qemuxml2argv-virtio-rng-multiple.args | 9 ++++ .../qemuxml2argv-virtio-rng-multiple.xml | 31 ++++++++++++ tests/qemuxml2argvtest.c | 2 + 8 files changed, 109 insertions(+), 55 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-virtio-rng-multiple.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-virtio-rng-multiple.xml
ACK series, but moving the default setting of '/dev/random' to the XML parser would be nicer. Jan
On 07/24/14 17:15, Ján Tomko wrote:
On 07/24/2014 04:07 PM, Peter Krempa wrote:
Qemu and kernel support multiple RNG's, so I revived my old patch to add the support to libvirt.
Peter Krempa (3): qemu: cgroup: Don't use NULL path on default backed RNGs virtio-rng: allow multiple RNG devices test: qemu: Add tests for multiple virtio-rng devices
src/conf/domain_audit.c | 4 +- src/conf/domain_conf.c | 58 +++++++++++----------- src/conf/domain_conf.h | 4 +- src/qemu/qemu_cgroup.c | 27 ++++++---- src/qemu/qemu_command.c | 29 ++++++----- .../qemuxml2argv-virtio-rng-multiple.args | 9 ++++ .../qemuxml2argv-virtio-rng-multiple.xml | 31 ++++++++++++ tests/qemuxml2argvtest.c | 2 + 8 files changed, 109 insertions(+), 55 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-virtio-rng-multiple.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-virtio-rng-multiple.xml
ACK series, but moving the default setting of '/dev/random' to the XML parser would be nicer.
We could do that in qemu's device post parse callback so we don't hardcode that for other drivers. That would also shed some of the code that handles the possibility of using NULL there afterwards. I'm contemplating on doing that in a separate patch though.
Jan
Peter Krempa <pkrempa@redhat.com> writes:
Qemu and kernel support multiple RNG's, so I revived my old patch to add the support to libvirt.
Peter Krempa (3): qemu: cgroup: Don't use NULL path on default backed RNGs virtio-rng: allow multiple RNG devices test: qemu: Add tests for multiple virtio-rng devices
FYI, I was curious to see how virt-manager performed with this series so I gave it a try and everything seems to work as expected there. Regards, Giuseppe
On 07/24/14 18:20, Giuseppe Scrivano wrote:
Peter Krempa <pkrempa@redhat.com> writes:
Qemu and kernel support multiple RNG's, so I revived my old patch to add the support to libvirt.
Peter Krempa (3): qemu: cgroup: Don't use NULL path on default backed RNGs virtio-rng: allow multiple RNG devices test: qemu: Add tests for multiple virtio-rng devices
FYI, I was curious to see how virt-manager performed with this series so I gave it a try and everything seems to work as expected there.
Cool thanks. I've went ahead and pushed this series and will follow up with a cleanup to the default path handling. Peter
participants (3)
-
Giuseppe Scrivano -
Ján Tomko -
Peter Krempa