Re: [PATCH v2 19/27] qemu: Build command line for virtio-pmem

4 Dec 2020

On Thu, Dec 03, 2020 at 13:36:22 +0100, Michal Privoznik wrote:
...
Now we have everything prepared for generating the command line.
The device alias prefix was chosen to be 'virtiopmem'.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1735375
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
---
 src/qemu/qemu_alias.c                         | 44 ++++++----
 src/qemu/qemu_command.c                       | 82 ++++++++++---------
 ...ory-hotplug-virtio-pmem.x86_64-latest.args | 45 ++++++++++
 tests/qemuxml2argvtest.c                      |  1 +
 4 files changed, 120 insertions(+), 52 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 931dbd4e84..68d4a7b480 100644
--- a/src/qemu/qemu_alias.c
+++ b/src/qemu/qemu_alias.c
@@ -466,6 +466,28 @@ qemuAssignDeviceRNGAlias(virDomainDefPtr def,
 }
+static int
+qemuDeviceMemoryGetAliasID(virDomainDefPtr def,
+                           virDomainMemoryDefPtr mem,
+                           bool oldAlias,
+                           const char *prefix)
+{
+    size_t i;
+    int maxidx = 0;
+
+    if (!oldAlias)
+        return mem->info.addr.dimm.slot;
If oldAlias is false this returns '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,10 +505,8 @@ qemuAssignDeviceMemoryAlias(virDomainDefPtr def,
                             virDomainMemoryDefPtr mem,
                             bool oldAlias)
 {
-    size_t i;
-    int maxidx = 0;
-    int idx;
     const char *prefix = NULL;
+    int idx = 0;
if (mem->info.alias)
         return 0;
@@ -494,26 +514,22 @@ qemuAssignDeviceMemoryAlias(virDomainDefPtr def,
     switch (mem->model) {
     case VIR_DOMAIN_MEMORY_MODEL_DIMM:
         prefix = "dimm";
+        idx = qemuDeviceMemoryGetAliasID(def, mem, oldAlias, prefix);
         break;
     case VIR_DOMAIN_MEMORY_MODEL_NVDIMM:
         prefix = "nvdimm";
+        idx = qemuDeviceMemoryGetAliasID(def, mem, oldAlias, prefix);
         break;
     case VIR_DOMAIN_MEMORY_MODEL_VIRTIO:
+        prefix = "virtiopmem";
+        idx = qemuDeviceMemoryGetAliasID(def, mem, true, prefix);
... and you use it like that here, but virtio-pmem is a PCI device.
...
+        break;
     case VIR_DOMAIN_MEMORY_MODEL_NONE:
     case VIR_DOMAIN_MEMORY_MODEL_LAST:
[...]
...
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 49241fc507..501deff1ee 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
[...]
...
@@ -3143,8 +3149,8 @@ qemuBuildMemoryBackendProps(virJSONValuePtr *backendProps,
         backendType = "memory-backend-ram";
     }
-    if (!priv->memPrealloc &&
-        virJSONValueObjectAdd(props, "B:prealloc", prealloc, NULL) < 0)
+    if (allowPrealloc &&
+        virJSONValueObjectAdd(props, "B:prealloc", wantPrealloc, NULL) < 0)
         return -1;
We can theoretically use a virTristate* and use 'T' type here instead of
having two variables.


[...]
...
@@ -3308,46 +3314,46 @@ 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);
-
-        qemuBuildDeviceAddressStr(&buf, def, &mem->info, qemuCaps);
+        device = "pc-dimm";
+        break;
+    case VIR_DOMAIN_MEMORY_MODEL_NVDIMM:
+        device = "nvdimm";
         break;
case VIR_DOMAIN_MEMORY_MODEL_VIRTIO:
+        device = "virtio-pmem-pci";
+
     case VIR_DOMAIN_MEMORY_MODEL_NONE:
     case VIR_DOMAIN_MEMORY_MODEL_LAST:
         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);
+
+    qemuBuildDeviceAddressStr(&buf, def, &mem->info, qemuCaps);
+
+
     return virBufferContentAndReset(&buf);
 }
Some of these seem to be specific for some devices (such as unarmed is
relevant only for nvdimm)

    

Re: [PATCH v2 19/27] qemu: Build command line for virtio-pmem

Peter Krempa