[PATCH 6/6] qemu: Build command line for virtio-pmem
Daniel Henrique Barboza
danielhb413 at gmail.com
Sat Jan 16 17:14:03 UTC 2021
On 1/15/21 11:12 AM, Michal Privoznik wrote:
> Now we have everything prepared for generating the command line.
> The device alias prefix was chosen to be 'virtiopmem'.
>
> Since virtio-pmem-pci device goes onto PCI bus generating device
> alias must have been changed slightly because
> qemuAssignDeviceMemoryAlias() might have used DIMM slot number to
> generate the alias. This obviously won't work and thus the "old"
> way (which includes qemuDomainDeviceAliasIndex()) must be used.
>
> Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1735375
> Signed-off-by: Michal Privoznik <mprivozn at redhat.com>
> ---
> src/qemu/qemu_alias.c | 58 ++++++++++-----
> src/qemu/qemu_command.c | 72 ++++++++++---------
> ...ory-hotplug-virtio-pmem.x86_64-latest.args | 45 ++++++++++++
> tests/qemuxml2argvtest.c | 1 +
> 4 files changed, 126 insertions(+), 50 deletions(-)
> create mode 100644 tests/qemuxml2argvdata/memory-hotplug-virtio-pmem.x86_64-latest.args
>
> diff --git a/src/qemu/qemu_alias.c b/src/qemu/qemu_alias.c
> index dcb6c7156d..b3492d6e85 100644
> --- a/src/qemu/qemu_alias.c
> +++ b/src/qemu/qemu_alias.c
> @@ -466,6 +466,29 @@ qemuAssignDeviceRNGAlias(virDomainDefPtr def,
> }
>
>
> +static int
> +qemuDeviceMemoryGetAliasID(virDomainDefPtr def,
> + virDomainMemoryDefPtr mem,
> + bool oldAlias,
> + const char *prefix)
> +{
> + size_t i;
> + int maxidx = 0;
> +
> + /* virtio-pmem goes onto PCI bus and thus DIMM address is not valid */
> + if (!oldAlias && mem->model != VIR_DOMAIN_MEMORY_MODEL_VIRTIO_PMEM)
> + return mem->info.addr.dimm.slot;
> +
> + for (i = 0; i < def->nmems; i++) {
> + int idx;
> + if ((idx = qemuDomainDeviceAliasIndex(&def->mems[i]->info, prefix)) >= maxidx)
> + maxidx = idx + 1;
> + }
> +
> + return maxidx;
> +}
> +
> +
> /**
> * qemuAssignDeviceMemoryAlias:
> * @def: domain definition. Necessary only if @oldAlias is true.
> @@ -483,29 +506,32 @@ qemuAssignDeviceMemoryAlias(virDomainDefPtr def,
> virDomainMemoryDefPtr mem,
> bool oldAlias)
> {
> - size_t i;
> - int maxidx = 0;
> - int idx;
> - const char *prefix;
> + const char *prefix = NULL;
> + int idx = 0;
>
> if (mem->info.alias)
> return 0;
>
> - if (mem->model == VIR_DOMAIN_MEMORY_MODEL_DIMM)
> + switch (mem->model) {
> + case VIR_DOMAIN_MEMORY_MODEL_DIMM:
> prefix = "dimm";
> - else
> + break;
> + case VIR_DOMAIN_MEMORY_MODEL_NVDIMM:
> prefix = "nvdimm";
> -
> - if (oldAlias) {
> - for (i = 0; i < def->nmems; i++) {
> - if ((idx = qemuDomainDeviceAliasIndex(&def->mems[i]->info, prefix)) >= maxidx)
> - maxidx = idx + 1;
> - }
> - } else {
> - maxidx = mem->info.addr.dimm.slot;
> + break;
> + case VIR_DOMAIN_MEMORY_MODEL_VIRTIO_PMEM:
> + prefix = "virtiopmem";
> + break;
> + case VIR_DOMAIN_MEMORY_MODEL_NONE:
> + case VIR_DOMAIN_MEMORY_MODEL_LAST:
> + default:
> + virReportEnumRangeError(virDomainMemoryModel, mem->model);
> + return -1;
> + break;
> }
>
> - mem->info.alias = g_strdup_printf("%s%d", prefix, maxidx);
> + idx = qemuDeviceMemoryGetAliasID(def, mem, oldAlias, prefix);
> + mem->info.alias = g_strdup_printf("%s%d", prefix, idx);
>
> return 0;
> }
> @@ -680,7 +706,7 @@ qemuAssignDeviceAliases(virDomainDefPtr def, virQEMUCapsPtr qemuCaps)
> return -1;
> }
> for (i = 0; i < def->nmems; i++) {
> - if (qemuAssignDeviceMemoryAlias(NULL, def->mems[i], false) < 0)
> + if (qemuAssignDeviceMemoryAlias(def, def->mems[i], false) < 0)
> return -1;
> }
> if (def->vsock) {
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index 6a4ad6c412..64b189fd11 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -3087,8 +3087,9 @@ qemuBuildMemoryBackendProps(virJSONValuePtr *backendProps,
> if (mem->nvdimmPath) {
> memPath = g_strdup(mem->nvdimmPath);
> /* If the NVDIMM is a real device then there's nothing to prealloc.
> - * If anything, we would be only wearing off the device. */
> - if (!mem->nvdimmPmem)
> + * If anything, we would be only wearing off the device.
> + * Simirarly, virtio-pmem-pci doesn't need prealloc either. */
s/Simirarly/Similarly
Reviewed-by: Daniel Henrique Barboza <danielhb413 at gmail.com>
> + if (!mem->nvdimmPmem && mem->model != VIR_DOMAIN_MEMORY_MODEL_VIRTIO_PMEM)
> prealloc = true;
> } else if (useHugepage) {
> if (qemuGetDomainHupageMemPath(priv->driver, def, pagesize, &memPath) < 0)
> @@ -3282,7 +3283,7 @@ qemuBuildMemoryDeviceStr(const virDomainDef *def,
> virQEMUCapsPtr qemuCaps)
> {
> g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER;
> - const char *device;
> + const char *device = NULL;
>
> if (!mem->info.alias) {
> virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> @@ -3291,47 +3292,50 @@ qemuBuildMemoryDeviceStr(const virDomainDef *def,
> }
>
> switch (mem->model) {
> - case VIR_DOMAIN_MEMORY_MODEL_NVDIMM:
> case VIR_DOMAIN_MEMORY_MODEL_DIMM:
> -
> - if (mem->model == VIR_DOMAIN_MEMORY_MODEL_DIMM)
> - device = "pc-dimm";
> - else
> - device = "nvdimm";
> -
> - virBufferAsprintf(&buf, "%s,", device);
> -
> - if (mem->targetNode >= 0)
> - virBufferAsprintf(&buf, "node=%d,", mem->targetNode);
> -
> - if (mem->labelsize)
> - virBufferAsprintf(&buf, "label-size=%llu,", mem->labelsize * 1024);
> -
> - if (virUUIDIsValid(mem->uuid)) {
> - char uuidstr[VIR_UUID_STRING_BUFLEN];
> -
> - virUUIDFormat(mem->uuid, uuidstr);
> - virBufferAsprintf(&buf, "uuid=%s,", uuidstr);
> - }
> -
> - if (mem->readonly) {
> - virBufferAddLit(&buf, "unarmed=on,");
> - }
> -
> - virBufferAsprintf(&buf, "memdev=mem%s,id=%s",
> - mem->info.alias, mem->info.alias);
> -
> - if (qemuBuildDeviceAddressStr(&buf, def, &mem->info, qemuCaps) < 0)
> - return NULL;
> + device = "pc-dimm";
> + break;
> + case VIR_DOMAIN_MEMORY_MODEL_NVDIMM:
> + device = "nvdimm";
> break;
>
> case VIR_DOMAIN_MEMORY_MODEL_VIRTIO_PMEM:
> + device = "virtio-pmem-pci";
> + break;
> +
> case VIR_DOMAIN_MEMORY_MODEL_NONE:
> case VIR_DOMAIN_MEMORY_MODEL_LAST:
> + default:
> + virReportEnumRangeError(virDomainMemoryModel, mem->model);
> + return NULL;
> break;
> + }
> +
> + virBufferAsprintf(&buf, "%s,", device);
> +
> + if (mem->targetNode >= 0)
> + virBufferAsprintf(&buf, "node=%d,", mem->targetNode);
> +
> + if (mem->labelsize)
> + virBufferAsprintf(&buf, "label-size=%llu,", mem->labelsize * 1024);
> +
> + if (virUUIDIsValid(mem->uuid)) {
> + char uuidstr[VIR_UUID_STRING_BUFLEN];
>
> + virUUIDFormat(mem->uuid, uuidstr);
> + virBufferAsprintf(&buf, "uuid=%s,", uuidstr);
> }
>
> + if (mem->readonly) {
> + virBufferAddLit(&buf, "unarmed=on,");
> + }
> +
> + virBufferAsprintf(&buf, "memdev=mem%s,id=%s",
> + mem->info.alias, mem->info.alias);
> +
> + if (qemuBuildDeviceAddressStr(&buf, def, &mem->info, qemuCaps) < 0)
> + return NULL;
> +
> return virBufferContentAndReset(&buf);
> }
>
> diff --git a/tests/qemuxml2argvdata/memory-hotplug-virtio-pmem.x86_64-latest.args b/tests/qemuxml2argvdata/memory-hotplug-virtio-pmem.x86_64-latest.args
> new file mode 100644
> index 0000000000..e2f5097424
> --- /dev/null
> +++ b/tests/qemuxml2argvdata/memory-hotplug-virtio-pmem.x86_64-latest.args
> @@ -0,0 +1,45 @@
> +LC_ALL=C \
> +PATH=/bin \
> +HOME=/tmp/lib/domain--1-QEMUGuest1 \
> +USER=test \
> +LOGNAME=test \
> +XDG_DATA_HOME=/tmp/lib/domain--1-QEMUGuest1/.local/share \
> +XDG_CACHE_HOME=/tmp/lib/domain--1-QEMUGuest1/.cache \
> +XDG_CONFIG_HOME=/tmp/lib/domain--1-QEMUGuest1/.config \
> +QEMU_AUDIO_DRV=none \
> +/usr/bin/qemu-system-i386 \
> +-name guest=QEMUGuest1,debug-threads=on \
> +-S \
> +-object secret,id=masterKey0,format=raw,\
> +file=/tmp/lib/domain--1-QEMUGuest1/master-key.aes \
> +-machine pc,accel=kvm,usb=off,dump-guest-core=off \
> +-cpu qemu64 \
> +-m size=2095104k,slots=16,maxmem=1099511627776k \
> +-overcommit mem-lock=off \
> +-smp 2,sockets=2,dies=1,cores=1,threads=1 \
> +-object memory-backend-ram,id=ram-node0,size=2145386496 \
> +-numa node,nodeid=0,cpus=0-1,memdev=ram-node0 \
> +-object memory-backend-file,id=memvirtiopmem0,mem-path=/tmp/virtio_pmem,\
> +share=yes,size=536870912 \
> +-device virtio-pmem-pci,memdev=memvirtiopmem0,id=virtiopmem0,bus=pci.0,\
> +addr=0x5 \
> +-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \
> +-display none \
> +-no-user-config \
> +-nodefaults \
> +-chardev socket,id=charmonitor,fd=1729,server,nowait \
> +-mon chardev=charmonitor,id=monitor,mode=control \
> +-rtc base=utc \
> +-no-shutdown \
> +-no-acpi \
> +-boot strict=on \
> +-device piix3-usb-uhci,id=usb,bus=pci.0,addr=0x1.0x2 \
> +-blockdev '{"driver":"host_device","filename":"/dev/HostVG/QEMUGuest1",\
> +"node-name":"libvirt-1-storage","auto-read-only":true,"discard":"unmap"}' \
> +-blockdev '{"node-name":"libvirt-1-format","read-only":false,"driver":"raw",\
> +"file":"libvirt-1-storage"}' \
> +-device ide-hd,bus=ide.0,unit=0,drive=libvirt-1-format,id=ide0-0-0,bootindex=1 \
> +-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3 \
> +-sandbox on,obsolete=deny,elevateprivileges=deny,spawn=deny,\
> +resourcecontrol=deny \
> +-msg timestamp=on
> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
> index 4a2ab6bec1..cf77224fc3 100644
> --- a/tests/qemuxml2argvtest.c
> +++ b/tests/qemuxml2argvtest.c
> @@ -3053,6 +3053,7 @@ mymain(void)
> QEMU_CAPS_OBJECT_MEMORY_FILE,
> QEMU_CAPS_DEVICE_NVDIMM,
> QEMU_CAPS_LAST);
> + DO_TEST_CAPS_LATEST("memory-hotplug-virtio-pmem");
>
> DO_TEST("machine-aeskeywrap-on-caps",
> QEMU_CAPS_AES_KEY_WRAP,
>
More information about the libvir-list
mailing list