qemu: Build command line for virtio-pmem

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@redhat.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>

diff --git a/src/qemu/qemu_alias.c b/src/qemu/qemu_alias.c
index dcb6c7156df71c661d2373789e6569c056dbbeca..b3492d6e853eb433f4954e58592e5b4850d45868 100644 (file)
--- 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 6a631a897503adf86e671c2e01d7f49098fc7f0b..99761c217df9c4aa7aee3277ac80cf45c79fe82f 100644 (file)
--- 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.
+             * Similarly, virtio-pmem-pci doesn't need prealloc either. */
+            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,46 +3292,49 @@ qemuBuildMemoryDeviceStr(const virDomainDef *def,
     }
 
     switch (mem->model) {
-    case VIR_DOMAIN_MEMORY_MODEL_NVDIMM:
     case VIR_DOMAIN_MEMORY_MODEL_DIMM:
+        device = "pc-dimm";
+        break;
+    case VIR_DOMAIN_MEMORY_MODEL_NVDIMM:
+        device = "nvdimm";
+        break;
 
-        if (mem->model == VIR_DOMAIN_MEMORY_MODEL_DIMM)
-            device = "pc-dimm";
-        else
-            device = "nvdimm";
-
-        virBufferAsprintf(&buf, "%s,", device);
+    case VIR_DOMAIN_MEMORY_MODEL_VIRTIO_PMEM:
+        device = "virtio-pmem-pci";
+        break;
 
-        if (mem->targetNode >= 0)
-            virBufferAsprintf(&buf, "node=%d,", mem->targetNode);
+    case VIR_DOMAIN_MEMORY_MODEL_NONE:
+    case VIR_DOMAIN_MEMORY_MODEL_LAST:
+    default:
+        virReportEnumRangeError(virDomainMemoryModel, mem->model);
+        return NULL;
+        break;
+    }
 
-        if (mem->labelsize)
-            virBufferAsprintf(&buf, "label-size=%llu,", mem->labelsize * 1024);
+    virBufferAsprintf(&buf, "%s,", device);
 
-        if (virUUIDIsValid(mem->uuid)) {
-            char uuidstr[VIR_UUID_STRING_BUFLEN];
+    if (mem->targetNode >= 0)
+        virBufferAsprintf(&buf, "node=%d,", mem->targetNode);
 
-            virUUIDFormat(mem->uuid, uuidstr);
-            virBufferAsprintf(&buf, "uuid=%s,", uuidstr);
-        }
+    if (mem->labelsize)
+        virBufferAsprintf(&buf, "label-size=%llu,", mem->labelsize * 1024);
 
-        if (mem->readonly) {
-            virBufferAddLit(&buf, "unarmed=on,");
-        }
+    if (virUUIDIsValid(mem->uuid)) {
+        char uuidstr[VIR_UUID_STRING_BUFLEN];
 
-        virBufferAsprintf(&buf, "memdev=mem%s,id=%s",
-                          mem->info.alias, mem->info.alias);
+        virUUIDFormat(mem->uuid, uuidstr);
+        virBufferAsprintf(&buf, "uuid=%s,", uuidstr);
+    }
 
-        if (qemuBuildDeviceAddressStr(&buf, def, &mem->info, qemuCaps) < 0)
-            return NULL;
-        break;
+    if (mem->readonly) {
+        virBufferAddLit(&buf, "unarmed=on,");
+    }
 
-    case VIR_DOMAIN_MEMORY_MODEL_VIRTIO_PMEM:
-    case VIR_DOMAIN_MEMORY_MODEL_NONE:
-    case VIR_DOMAIN_MEMORY_MODEL_LAST:
-        break;
+    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 (file)
index 0000000..e2f5097
--- /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 4a2ab6bec16d23a01e6a3bdb6dac971dbdee02a6..cf77224fc37c215263f16529c893edcd2abfd777 100644 (file)
--- 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,