Re: [PATCH v5 05/16] qemu: Build command line for virtio-mem

David Hildenbrand <david@xxxxxxxxxx> · Tue, 14 Sep 2021 15:05:50 +0200

On 13.09.21 16:52, Michal Privoznik wrote:
Nothing special is happening here. All important changes were
done when for 'virtio-pmem' (adjusting the code to put virtio
memory on PCI bus, generating alias using
qemuDomainDeviceAliasIndex(). The only bit that might look
suspicious is no prealloc for virtio-mem. But if you think about
it, the whole purpose of this device is to change amount of
memory exposed to guest on the fly. There is no point in locking
the whole backend in memory.

Do I understand correctly that we'll always try setting "reserve=off", 
and "prealloc=off"? That would be awesome :)

I do wonder if we want to warn or bail out if "priv->memPrealloc" is set 
but we are temporarily not able to support it -- as discussed, because 
virtio-mem in QEMU yet has to be taught about it.

FYI: QEMU will bail out if "reserve=off,prealloc=on" is set in combination.


Signed-off-by: Michal Privoznik <mprivozn@xxxxxxxxxx>
---
  src/qemu/qemu_alias.c                         |  9 +++-
  src/qemu/qemu_command.c                       | 24 +++++++++--
  ...mory-hotplug-virtio-mem.x86_64-latest.args | 41 +++++++++++++++++++
  tests/qemuxml2argvtest.c                      |  1 +
  4 files changed, 70 insertions(+), 5 deletions(-)
  create mode 100644 tests/qemuxml2argvdata/memory-hotplug-virtio-mem.x86_64-latest.args

diff --git a/src/qemu/qemu_alias.c b/src/qemu/qemu_alias.c
index 79e8953b2f..81a1e7eeed 100644
--- a/src/qemu/qemu_alias.c
+++ b/src/qemu/qemu_alias.c
@@ -475,8 +475,11 @@ qemuDeviceMemoryGetAliasID(virDomainDef *def,
      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)
+    /* virtio-pmem and virtio-mem go onto PCI bus and thus DIMM address is not
+     * valid */
+    if (!oldAlias &&
+        mem->model != VIR_DOMAIN_MEMORY_MODEL_VIRTIO_PMEM &&
+        mem->model != VIR_DOMAIN_MEMORY_MODEL_VIRTIO_MEM)
          return mem->info.addr.dimm.slot;
  
      for (i = 0; i < def->nmems; i++) {
@@ -523,6 +526,8 @@ qemuAssignDeviceMemoryAlias(virDomainDef *def,
          prefix = "virtiopmem";
          break;
      case VIR_DOMAIN_MEMORY_MODEL_VIRTIO_MEM:
+        prefix = "virtiomem";
+        break;
      case VIR_DOMAIN_MEMORY_MODEL_NONE:
      case VIR_DOMAIN_MEMORY_MODEL_LAST:
      default:
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 594e5643b1..91f3094ac8 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -3136,9 +3136,19 @@ qemuBuildMemoryBackendProps(virJSONValue **backendProps,
          virJSONValueObjectAdd(props, "b:x-use-canonical-path-for-ramblock-id", false, NULL) < 0)
          return -1;
  
-    if (!priv->memPrealloc &&
-        virJSONValueObjectAdd(props, "B:prealloc", prealloc, NULL) < 0)
-        return -1;
+    if (mem->model == VIR_DOMAIN_MEMORY_MODEL_VIRTIO_MEM) {
+        /* Explicitly disable prealloc for virtio-mem */
+        if (priv->memPrealloc &&
+            virJSONValueObjectAppendBoolean(props, "prealloc", 0) < 0)
+            return -1;
+        if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_MEMORY_BACKEND_RESERVE) &&
+            virJSONValueObjectAppendBoolean(props, "reserve", 0) < 0)
+            return -1;
+    } else {
+        if (!priv->memPrealloc &&
+            virJSONValueObjectAdd(props, "B:prealloc", prealloc, NULL) < 0)
+            return -1;
+    }
  
      if (virJSONValueObjectAdd(props, "U:size", mem->size * 1024, NULL) < 0)
          return -1;
@@ -3318,6 +3328,9 @@ qemuBuildMemoryDeviceStr(const virDomainDef *def,
          break;
  
      case VIR_DOMAIN_MEMORY_MODEL_VIRTIO_MEM:
+        device = "virtio-mem-pci";
+        break;
+
      case VIR_DOMAIN_MEMORY_MODEL_NONE:
      case VIR_DOMAIN_MEMORY_MODEL_LAST:
      default:
@@ -3334,6 +3347,11 @@ qemuBuildMemoryDeviceStr(const virDomainDef *def,
      if (mem->labelsize)
          virBufferAsprintf(&buf, "label-size=%llu,", mem->labelsize * 1024);
  
+    if (mem->blocksize) {
+        virBufferAsprintf(&buf, "block-size=%llu,", mem->blocksize * 1024);
+        virBufferAsprintf(&buf, "requested-size=%llu,", mem->requestedsize * 1024);
+    }
+
      if (mem->uuid) {
          char uuidstr[VIR_UUID_STRING_BUFLEN];
  
diff --git a/tests/qemuxml2argvdata/memory-hotplug-virtio-mem.x86_64-latest.args b/tests/qemuxml2argvdata/memory-hotplug-virtio-mem.x86_64-latest.args
new file mode 100644
index 0000000000..22ee1bc459
--- /dev/null
+++ b/tests/qemuxml2argvdata/memory-hotplug-virtio-mem.x86_64-latest.args
@@ -0,0 +1,41 @@
+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 \
+/usr/bin/qemu-system-i386 \
+-name guest=QEMUGuest1,debug-threads=on \
+-S \
+-object '{"qom-type":"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 '{"qom-type":"memory-backend-ram","id":"ram-node0","size":2145386496}' \
+-numa node,nodeid=0,cpus=0-1,memdev=ram-node0 \
+-object '{"qom-type":"memory-backend-ram","id":"memvirtiomem0","reserve":false,"size":1073741824}' \
+-device virtio-mem-pci,node=0,block-size=2097152,requested-size=536870912,memdev=memvirtiomem0,id=virtiomem0,bus=pci.0,addr=0x2 \
+-object '{"qom-type":"memory-backend-file","id":"memvirtiomem1","mem-path":"/dev/hugepages2M/libvirt/qemu/-1-QEMUGuest1","reserve":false,"size":2147483648,"host-nodes":[1,2,3],"policy":"bind"}' \
+-device virtio-mem-pci,node=0,block-size=2097152,requested-size=1073741824,memdev=memvirtiomem1,id=virtiomem1,bus=pci.0,addr=0x3 \
+-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \
+-display none \
+-no-user-config \
+-nodefaults \
+-chardev socket,id=charmonitor,fd=1729,server=on,wait=off \
+-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 \
+-audiodev id=audio1,driver=none \
+-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x4 \
+-sandbox on,obsolete=deny,elevateprivileges=deny,spawn=deny,resourcecontrol=deny \
+-msg timestamp=on
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index 74af93b08f..23f4a8471f 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -3143,6 +3143,7 @@ mymain(void)
                   ARG_END);
      DO_TEST_CAPS_VER("memory-hotplug-virtio-pmem", "5.2.0");
      DO_TEST_CAPS_LATEST("memory-hotplug-virtio-pmem");
+    DO_TEST_CAPS_LATEST("memory-hotplug-virtio-mem");
  
      DO_TEST("machine-aeskeywrap-on-caps",
              QEMU_CAPS_AES_KEY_WRAP,



--
Thanks,

David / dhildenb







