[libvirt] [RFC v2 2/3] nvdimm: update qemu command-line generating for NVDIMM memory
According to the result parsing from xml, add corresponding properties into QEMU command line, including 'align', 'pmem', 'unarmed' and 'nvdimm-persistence'. Signed-off-by: Luyao Zhong <luyao.zhong@intel.com> --- src/qemu/qemu_capabilities.c | 17 +++++++ src/qemu/qemu_capabilities.h | 5 ++ src/qemu/qemu_command.c | 50 ++++++++++++++++++- src/qemu/qemu_command.h | 3 +- src/qemu/qemu_hotplug.c | 2 +- .../memory-hotplug-nvdimm-align.args | 31 ++++++++++++ .../memory-hotplug-nvdimm-persistence.args | 31 ++++++++++++ .../memory-hotplug-nvdimm-pmem.args | 31 ++++++++++++ .../memory-hotplug-nvdimm-unarmed.args | 31 ++++++++++++ tests/qemuxml2argvtest.c | 15 ++++++ 10 files changed, 212 insertions(+), 4 deletions(-) create mode 100644 tests/qemuxml2argvdata/memory-hotplug-nvdimm-align.args create mode 100644 tests/qemuxml2argvdata/memory-hotplug-nvdimm-persistence.args create mode 100644 tests/qemuxml2argvdata/memory-hotplug-nvdimm-pmem.args create mode 100644 tests/qemuxml2argvdata/memory-hotplug-nvdimm-unarmed.args diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 20a1a0c201..1bb9ceb888 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -516,6 +516,11 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST, "memory-backend-memfd.hugetlb", "iothread.poll-max-ns", "machine.pseries.cap-nested-hv", + "nvdimm-align", + "nvdimm-pmem", + + /* 325 */ + "nvdimm-unarmed", ); @@ -4190,6 +4195,18 @@ virQEMUCapsInitQMPMonitor(virQEMUCapsPtr qemuCaps, virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_NVDIMM)) virQEMUCapsClear(qemuCaps, QEMU_CAPS_DEVICE_NVDIMM); + if (qemuCaps->version < 2001200 && + virQEMUCapsGet(qemuCaps, QEMU_CAPS_OBJECT_MEMORY_FILE_ALIGN)) + virQEMUCapsClear(qemuCaps, QEMU_CAPS_OBJECT_MEMORY_FILE_ALIGN); + + if (qemuCaps->version < 3000000 && + virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_NVDIMM_UNARMED)) + virQEMUCapsClear(qemuCaps, QEMU_CAPS_DEVICE_NVDIMM_UNARMED); + + if (qemuCaps->version < 3001000 && + virQEMUCapsGet(qemuCaps, QEMU_CAPS_OBJECT_MEMORY_FILE_PMEM)) + virQEMUCapsClear(qemuCaps, QEMU_CAPS_OBJECT_MEMORY_FILE_PMEM); + if (ARCH_IS_X86(qemuCaps->arch) && virQEMUCapsGet(qemuCaps, QEMU_CAPS_QUERY_CPU_MODEL_EXPANSION)) virQEMUCapsSet(qemuCaps, QEMU_CAPS_CPU_CACHE); diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index b1990b6bb8..8d5fc5e8e5 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -500,6 +500,11 @@ typedef enum { /* virQEMUCapsFlags grouping marker for syntax-check */ QEMU_CAPS_OBJECT_MEMORY_MEMFD_HUGETLB, /* -object memory-backend-memfd.hugetlb */ QEMU_CAPS_IOTHREAD_POLLING, /* -object iothread.poll-max-ns */ QEMU_CAPS_MACHINE_PSERIES_CAP_NESTED_HV, /* -machine pseries.cap-nested-hv */ + QEMU_CAPS_OBJECT_MEMORY_FILE_ALIGN, /* -object memory-backend-file supports align property */ + QEMU_CAPS_OBJECT_MEMORY_FILE_PMEM, /* -object memory-backend-file supports pmem property*/ + + /* 325 */ + QEMU_CAPS_DEVICE_NVDIMM_UNARMED, /* -device nvdimm supports unarmed property*/ QEMU_CAPS_LAST /* this must always be the last item */ } virQEMUCapsFlags; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 315419c71b..219f5734d1 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -3400,6 +3400,34 @@ qemuBuildMemoryBackendProps(virJSONValuePtr *backendProps, if (virJSONValueObjectAdd(props, "U:size", mem->size * 1024, NULL) < 0) goto cleanup; + if (mem->alignsize) { + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_OBJECT_MEMORY_FILE_ALIGN)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("align property is not available " + "with this QEMU binary")); + goto cleanup; + } + if (virJSONValueObjectAdd(props, + "U:align", + mem->alignsize * 1024, + NULL) < 0) + goto cleanup; + } + + if (mem->nvdimmPmem) { + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_OBJECT_MEMORY_FILE_PMEM)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("pmem property is not available " + "with this QEMU binary")); + goto cleanup; + } + if (virJSONValueObjectAdd(props, + "s:pmem", + mem->nvdimmPmem ? "on" : "off", + NULL) < 0) + goto cleanup; + } + if (mem->sourceNodes) { nodemask = mem->sourceNodes; } else { @@ -3541,8 +3569,10 @@ qemuBuildMemoryDimmBackendStr(virBufferPtr buf, char * -qemuBuildMemoryDeviceStr(virDomainMemoryDefPtr mem) +qemuBuildMemoryDeviceStr(virDomainMemoryDefPtr mem, + qemuDomainObjPrivatePtr priv) { + virQEMUCapsPtr qemuCaps = priv->qemuCaps; virBuffer buf = VIR_BUFFER_INITIALIZER; const char *device; @@ -3569,6 +3599,17 @@ qemuBuildMemoryDeviceStr(virDomainMemoryDefPtr mem) if (mem->labelsize) virBufferAsprintf(&buf, "label-size=%llu,", mem->labelsize * 1024); + if (mem->nvdimmUnarmed) { + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_NVDIMM_UNARMED)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("unarmed is not available " + "with this QEMU binary")); + return NULL; + } + virBufferAsprintf(&buf, "unarmed=%s,", + mem->nvdimmUnarmed ? "on" : "off"); + } + virBufferAsprintf(&buf, "memdev=mem%s,id=%s", mem->info.alias, mem->info.alias); @@ -7383,6 +7424,11 @@ qemuBuildMachineCommandLine(virCommandPtr cmd, goto cleanup; } virBufferAddLit(&buf, ",nvdimm=on"); + + if (def->mems[i]->nvdimmPersistence) { + virBufferAsprintf(&buf, ",nvdimm-persistence=%s", + virDomainMemoryPersistenceTypeToString(def->mems[i]->nvdimmPersistence)); + } break; } } @@ -7860,7 +7906,7 @@ qemuBuildMemoryDeviceCommandLine(virCommandPtr cmd, virCommandAddArg(cmd, "-object"); virCommandAddArgBuffer(cmd, &buf); - if (!(dimmStr = qemuBuildMemoryDeviceStr(def->mems[i]))) + if (!(dimmStr = qemuBuildMemoryDeviceStr(def->mems[i], priv))) return -1; virCommandAddArgList(cmd, "-device", dimmStr, NULL); diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h index d382cd592a..eee13a20d8 100644 --- a/src/qemu/qemu_command.h +++ b/src/qemu/qemu_command.h @@ -131,7 +131,8 @@ int qemuBuildMemoryBackendProps(virJSONValuePtr *backendProps, virBitmapPtr autoNodeset, bool force); -char *qemuBuildMemoryDeviceStr(virDomainMemoryDefPtr mem); +char *qemuBuildMemoryDeviceStr(virDomainMemoryDefPtr mem, + qemuDomainObjPrivatePtr priv); /* Current, best practice */ char *qemuBuildPCIHostdevDevStr(const virDomainDef *def, diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 5f756b7267..2da056ed1f 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -2564,7 +2564,7 @@ qemuDomainAttachMemory(virQEMUDriverPtr driver, if (virAsprintf(&objalias, "mem%s", mem->info.alias) < 0) goto cleanup; - if (!(devstr = qemuBuildMemoryDeviceStr(mem))) + if (!(devstr = qemuBuildMemoryDeviceStr(mem, priv))) goto cleanup; if (qemuBuildMemoryBackendProps(&props, objalias, cfg, diff --git a/tests/qemuxml2argvdata/memory-hotplug-nvdimm-align.args b/tests/qemuxml2argvdata/memory-hotplug-nvdimm-align.args new file mode 100644 index 0000000000..432f6222a1 --- /dev/null +++ b/tests/qemuxml2argvdata/memory-hotplug-nvdimm-align.args @@ -0,0 +1,31 @@ +LC_ALL=C \ +PATH=/bin \ +HOME=/home/test \ +USER=test \ +LOGNAME=test \ +QEMU_AUDIO_DRV=none \ +/usr/bin/qemu-system-i686 \ +-name QEMUGuest1 \ +-S \ +-machine pc,accel=tcg,usb=off,dump-guest-core=off,nvdimm=on \ +-m size=219136k,slots=16,maxmem=1099511627776k \ +-smp 2,sockets=2,cores=1,threads=1 \ +-numa node,nodeid=0,cpus=0-1,mem=214 \ +-object memory-backend-file,id=memnvdimm0,prealloc=yes,mem-path=/tmp/nvdimm,\ +share=no,size=536870912,align=2097152 \ +-device nvdimm,node=0,memdev=memnvdimm0,id=nvdimm0,slot=0 \ +-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \ +-display none \ +-no-user-config \ +-nodefaults \ +-chardev socket,id=charmonitor,path=/tmp/lib/domain--1-QEMUGuest1/monitor.sock,\ +server,nowait \ +-mon chardev=charmonitor,id=monitor,mode=control \ +-rtc base=utc \ +-no-shutdown \ +-no-acpi \ +-usb \ +-drive file=/dev/HostVG/QEMUGuest1,format=raw,if=none,id=drive-ide0-0-0 \ +-device ide-drive,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0,\ +bootindex=1 \ +-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3 diff --git a/tests/qemuxml2argvdata/memory-hotplug-nvdimm-persistence.args b/tests/qemuxml2argvdata/memory-hotplug-nvdimm-persistence.args new file mode 100644 index 0000000000..8ff03e3ca3 --- /dev/null +++ b/tests/qemuxml2argvdata/memory-hotplug-nvdimm-persistence.args @@ -0,0 +1,31 @@ +LC_ALL=C \ +PATH=/bin \ +HOME=/home/test \ +USER=test \ +LOGNAME=test \ +QEMU_AUDIO_DRV=none \ +/usr/bin/qemu-system-i686 \ +-name QEMUGuest1 \ +-S \ +-machine pc,accel=tcg,usb=off,dump-guest-core=off,nvdimm=on,nvdimm-persistence=mem-ctrl \ +-m size=219136k,slots=16,maxmem=1099511627776k \ +-smp 2,sockets=2,cores=1,threads=1 \ +-numa node,nodeid=0,cpus=0-1,mem=214 \ +-object memory-backend-file,id=memnvdimm0,prealloc=yes,mem-path=/tmp/nvdimm,\ +share=no,size=536870912 \ +-device nvdimm,node=0,memdev=memnvdimm0,id=nvdimm0,slot=0 \ +-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \ +-display none \ +-no-user-config \ +-nodefaults \ +-chardev socket,id=charmonitor,path=/tmp/lib/domain--1-QEMUGuest1/monitor.sock,\ +server,nowait \ +-mon chardev=charmonitor,id=monitor,mode=control \ +-rtc base=utc \ +-no-shutdown \ +-no-acpi \ +-usb \ +-drive file=/dev/HostVG/QEMUGuest1,format=raw,if=none,id=drive-ide0-0-0 \ +-device ide-drive,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0,\ +bootindex=1 \ +-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3 diff --git a/tests/qemuxml2argvdata/memory-hotplug-nvdimm-pmem.args b/tests/qemuxml2argvdata/memory-hotplug-nvdimm-pmem.args new file mode 100644 index 0000000000..d34ac5b4c5 --- /dev/null +++ b/tests/qemuxml2argvdata/memory-hotplug-nvdimm-pmem.args @@ -0,0 +1,31 @@ +LC_ALL=C \ +PATH=/bin \ +HOME=/home/test \ +USER=test \ +LOGNAME=test \ +QEMU_AUDIO_DRV=none \ +/usr/bin/qemu-system-i686 \ +-name QEMUGuest1 \ +-S \ +-machine pc,accel=tcg,usb=off,dump-guest-core=off,nvdimm=on \ +-m size=219136k,slots=16,maxmem=1099511627776k \ +-smp 2,sockets=2,cores=1,threads=1 \ +-numa node,nodeid=0,cpus=0-1,mem=214 \ +-object memory-backend-file,id=memnvdimm0,prealloc=yes,mem-path=/tmp/nvdimm,\ +share=no,size=536870912,pmem=on \ +-device nvdimm,node=0,memdev=memnvdimm0,id=nvdimm0,slot=0 \ +-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \ +-display none \ +-no-user-config \ +-nodefaults \ +-chardev socket,id=charmonitor,path=/tmp/lib/domain--1-QEMUGuest1/monitor.sock,\ +server,nowait \ +-mon chardev=charmonitor,id=monitor,mode=control \ +-rtc base=utc \ +-no-shutdown \ +-no-acpi \ +-usb \ +-drive file=/dev/HostVG/QEMUGuest1,format=raw,if=none,id=drive-ide0-0-0 \ +-device ide-drive,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0,\ +bootindex=1 \ +-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3 diff --git a/tests/qemuxml2argvdata/memory-hotplug-nvdimm-unarmed.args b/tests/qemuxml2argvdata/memory-hotplug-nvdimm-unarmed.args new file mode 100644 index 0000000000..64dcc5a250 --- /dev/null +++ b/tests/qemuxml2argvdata/memory-hotplug-nvdimm-unarmed.args @@ -0,0 +1,31 @@ +LC_ALL=C \ +PATH=/bin \ +HOME=/home/test \ +USER=test \ +LOGNAME=test \ +QEMU_AUDIO_DRV=none \ +/usr/bin/qemu-system-i686 \ +-name QEMUGuest1 \ +-S \ +-machine pc,accel=tcg,usb=off,dump-guest-core=off,nvdimm=on \ +-m size=219136k,slots=16,maxmem=1099511627776k \ +-smp 2,sockets=2,cores=1,threads=1 \ +-numa node,nodeid=0,cpus=0-1,mem=214 \ +-object memory-backend-file,id=memnvdimm0,prealloc=yes,mem-path=/tmp/nvdimm,\ +share=no,size=536870912 \ +-device nvdimm,node=0,unarmed=on,memdev=memnvdimm0,id=nvdimm0,slot=0 \ +-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \ +-display none \ +-no-user-config \ +-nodefaults \ +-chardev socket,id=charmonitor,path=/tmp/lib/domain--1-QEMUGuest1/monitor.sock,\ +server,nowait \ +-mon chardev=charmonitor,id=monitor,mode=control \ +-rtc base=utc \ +-no-shutdown \ +-no-acpi \ +-usb \ +-drive file=/dev/HostVG/QEMUGuest1,format=raw,if=none,id=drive-ide0-0-0 \ +-device ide-drive,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0,\ +bootindex=1 \ +-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3 diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index eae2b7edf7..5a1677ec01 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -2754,6 +2754,21 @@ mymain(void) DO_TEST("memory-hotplug-nvdimm-label", QEMU_CAPS_DEVICE_NVDIMM, QEMU_CAPS_NUMA, QEMU_CAPS_OBJECT_MEMORY_RAM, QEMU_CAPS_OBJECT_MEMORY_FILE); + DO_TEST("memory-hotplug-nvdimm-align", + QEMU_CAPS_DEVICE_NVDIMM, + QEMU_CAPS_NUMA, QEMU_CAPS_OBJECT_MEMORY_RAM, QEMU_CAPS_OBJECT_MEMORY_FILE, + QEMU_CAPS_OBJECT_MEMORY_FILE_ALIGN); + DO_TEST("memory-hotplug-nvdimm-pmem", + QEMU_CAPS_DEVICE_NVDIMM, + QEMU_CAPS_NUMA, QEMU_CAPS_OBJECT_MEMORY_RAM, QEMU_CAPS_OBJECT_MEMORY_FILE, + QEMU_CAPS_OBJECT_MEMORY_FILE_PMEM); + DO_TEST("memory-hotplug-nvdimm-unarmed", + QEMU_CAPS_DEVICE_NVDIMM, + QEMU_CAPS_NUMA, QEMU_CAPS_OBJECT_MEMORY_RAM, QEMU_CAPS_OBJECT_MEMORY_FILE, + QEMU_CAPS_DEVICE_NVDIMM_UNARMED); + DO_TEST("memory-hotplug-nvdimm-persistence", + QEMU_CAPS_DEVICE_NVDIMM, + QEMU_CAPS_NUMA, QEMU_CAPS_OBJECT_MEMORY_RAM, QEMU_CAPS_OBJECT_MEMORY_FILE); DO_TEST("machine-aeskeywrap-on-caps", QEMU_CAPS_AES_KEY_WRAP, -- 2.17.1
On Wed, Nov 28, 2018 at 22:09:01 +0800, Luyao Zhong wrote:
According to the result parsing from xml, add corresponding properties into QEMU command line, including 'align', 'pmem', 'unarmed' and 'nvdimm-persistence'.
Signed-off-by: Luyao Zhong <luyao.zhong@intel.com> --- src/qemu/qemu_capabilities.c | 17 +++++++ src/qemu/qemu_capabilities.h | 5 ++ src/qemu/qemu_command.c | 50 ++++++++++++++++++- src/qemu/qemu_command.h | 3 +- src/qemu/qemu_hotplug.c | 2 +- .../memory-hotplug-nvdimm-align.args | 31 ++++++++++++ .../memory-hotplug-nvdimm-persistence.args | 31 ++++++++++++ .../memory-hotplug-nvdimm-pmem.args | 31 ++++++++++++ .../memory-hotplug-nvdimm-unarmed.args | 31 ++++++++++++ tests/qemuxml2argvtest.c | 15 ++++++ 10 files changed, 212 insertions(+), 4 deletions(-) create mode 100644 tests/qemuxml2argvdata/memory-hotplug-nvdimm-align.args create mode 100644 tests/qemuxml2argvdata/memory-hotplug-nvdimm-persistence.args create mode 100644 tests/qemuxml2argvdata/memory-hotplug-nvdimm-pmem.args create mode 100644 tests/qemuxml2argvdata/memory-hotplug-nvdimm-unarmed.args
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 20a1a0c201..1bb9ceb888 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -516,6 +516,11 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST, "memory-backend-memfd.hugetlb", "iothread.poll-max-ns", "machine.pseries.cap-nested-hv", + "nvdimm-align", + "nvdimm-pmem", + + /* 325 */ + "nvdimm-unarmed", );
@@ -4190,6 +4195,18 @@ virQEMUCapsInitQMPMonitor(virQEMUCapsPtr qemuCaps, virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_NVDIMM)) virQEMUCapsClear(qemuCaps, QEMU_CAPS_DEVICE_NVDIMM);
+ if (qemuCaps->version < 2001200 && + virQEMUCapsGet(qemuCaps, QEMU_CAPS_OBJECT_MEMORY_FILE_ALIGN)) + virQEMUCapsClear(qemuCaps, QEMU_CAPS_OBJECT_MEMORY_FILE_ALIGN); + + if (qemuCaps->version < 3000000 && + virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_NVDIMM_UNARMED)) + virQEMUCapsClear(qemuCaps, QEMU_CAPS_DEVICE_NVDIMM_UNARMED); + + if (qemuCaps->version < 3001000 && + virQEMUCapsGet(qemuCaps, QEMU_CAPS_OBJECT_MEMORY_FILE_PMEM)) + virQEMUCapsClear(qemuCaps, QEMU_CAPS_OBJECT_MEMORY_FILE_PMEM);
This is completely broken. This code just clears the capability bits for older versions but really does never set them anywhere. So they'll never be present in real caps. That is visible that in the fact that this patch does not trigger an update of capability output test files, which it should. Additionally these really should check the presence of the fields in the device properties. For properties of object memory-backend-file we already call the appropriate qom-list-properties, so they will be trivial to modify. For the nvdimm property it will be slightly more hassle, but we really don't want to see version number based capabilities.
+ if (ARCH_IS_X86(qemuCaps->arch) && virQEMUCapsGet(qemuCaps, QEMU_CAPS_QUERY_CPU_MODEL_EXPANSION)) virQEMUCapsSet(qemuCaps, QEMU_CAPS_CPU_CACHE); diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index b1990b6bb8..8d5fc5e8e5 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -500,6 +500,11 @@ typedef enum { /* virQEMUCapsFlags grouping marker for syntax-check */ QEMU_CAPS_OBJECT_MEMORY_MEMFD_HUGETLB, /* -object memory-backend-memfd.hugetlb */ QEMU_CAPS_IOTHREAD_POLLING, /* -object iothread.poll-max-ns */ QEMU_CAPS_MACHINE_PSERIES_CAP_NESTED_HV, /* -machine pseries.cap-nested-hv */ + QEMU_CAPS_OBJECT_MEMORY_FILE_ALIGN, /* -object memory-backend-file supports align property */ + QEMU_CAPS_OBJECT_MEMORY_FILE_PMEM, /* -object memory-backend-file supports pmem property*/ + + /* 325 */ + QEMU_CAPS_DEVICE_NVDIMM_UNARMED, /* -device nvdimm supports unarmed property*/
QEMU_CAPS_LAST /* this must always be the last item */ } virQEMUCapsFlags;
[...]
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index eae2b7edf7..5a1677ec01 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -2754,6 +2754,21 @@ mymain(void) DO_TEST("memory-hotplug-nvdimm-label", QEMU_CAPS_DEVICE_NVDIMM, QEMU_CAPS_NUMA, QEMU_CAPS_OBJECT_MEMORY_RAM, QEMU_CAPS_OBJECT_MEMORY_FILE); + DO_TEST("memory-hotplug-nvdimm-align", + QEMU_CAPS_DEVICE_NVDIMM, + QEMU_CAPS_NUMA, QEMU_CAPS_OBJECT_MEMORY_RAM, QEMU_CAPS_OBJECT_MEMORY_FILE, + QEMU_CAPS_OBJECT_MEMORY_FILE_ALIGN); + DO_TEST("memory-hotplug-nvdimm-pmem", + QEMU_CAPS_DEVICE_NVDIMM, + QEMU_CAPS_NUMA, QEMU_CAPS_OBJECT_MEMORY_RAM, QEMU_CAPS_OBJECT_MEMORY_FILE, + QEMU_CAPS_OBJECT_MEMORY_FILE_PMEM); + DO_TEST("memory-hotplug-nvdimm-unarmed", + QEMU_CAPS_DEVICE_NVDIMM, + QEMU_CAPS_NUMA, QEMU_CAPS_OBJECT_MEMORY_RAM, QEMU_CAPS_OBJECT_MEMORY_FILE, + QEMU_CAPS_DEVICE_NVDIMM_UNARMED); + DO_TEST("memory-hotplug-nvdimm-persistence", + QEMU_CAPS_DEVICE_NVDIMM, + QEMU_CAPS_NUMA, QEMU_CAPS_OBJECT_MEMORY_RAM, QEMU_CAPS_OBJECT_MEMORY_FILE);
Please use DO_TEST_CAPS_LATEST. That way it's tested with real capabilities and not something you've made up. Here it would be visible, that your capability setting does not work at all. Also please separate capability changes from command line generator modifications.
On 2018/11/28 下午10:32, Peter Krempa wrote:
On Wed, Nov 28, 2018 at 22:09:01 +0800, Luyao Zhong wrote:
According to the result parsing from xml, add corresponding properties into QEMU command line, including 'align', 'pmem', 'unarmed' and 'nvdimm-persistence'.
Signed-off-by: Luyao Zhong <luyao.zhong@intel.com> --- src/qemu/qemu_capabilities.c | 17 +++++++ src/qemu/qemu_capabilities.h | 5 ++ src/qemu/qemu_command.c | 50 ++++++++++++++++++- src/qemu/qemu_command.h | 3 +- src/qemu/qemu_hotplug.c | 2 +- .../memory-hotplug-nvdimm-align.args | 31 ++++++++++++ .../memory-hotplug-nvdimm-persistence.args | 31 ++++++++++++ .../memory-hotplug-nvdimm-pmem.args | 31 ++++++++++++ .../memory-hotplug-nvdimm-unarmed.args | 31 ++++++++++++ tests/qemuxml2argvtest.c | 15 ++++++ 10 files changed, 212 insertions(+), 4 deletions(-) create mode 100644 tests/qemuxml2argvdata/memory-hotplug-nvdimm-align.args create mode 100644 tests/qemuxml2argvdata/memory-hotplug-nvdimm-persistence.args create mode 100644 tests/qemuxml2argvdata/memory-hotplug-nvdimm-pmem.args create mode 100644 tests/qemuxml2argvdata/memory-hotplug-nvdimm-unarmed.args
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 20a1a0c201..1bb9ceb888 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -516,6 +516,11 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST, "memory-backend-memfd.hugetlb", "iothread.poll-max-ns", "machine.pseries.cap-nested-hv", + "nvdimm-align", + "nvdimm-pmem", + + /* 325 */ + "nvdimm-unarmed", );
@@ -4190,6 +4195,18 @@ virQEMUCapsInitQMPMonitor(virQEMUCapsPtr qemuCaps, virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_NVDIMM)) virQEMUCapsClear(qemuCaps, QEMU_CAPS_DEVICE_NVDIMM);
+ if (qemuCaps->version < 2001200 && + virQEMUCapsGet(qemuCaps, QEMU_CAPS_OBJECT_MEMORY_FILE_ALIGN)) + virQEMUCapsClear(qemuCaps, QEMU_CAPS_OBJECT_MEMORY_FILE_ALIGN); + + if (qemuCaps->version < 3000000 && + virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_NVDIMM_UNARMED)) + virQEMUCapsClear(qemuCaps, QEMU_CAPS_DEVICE_NVDIMM_UNARMED); + + if (qemuCaps->version < 3001000 && + virQEMUCapsGet(qemuCaps, QEMU_CAPS_OBJECT_MEMORY_FILE_PMEM)) + virQEMUCapsClear(qemuCaps, QEMU_CAPS_OBJECT_MEMORY_FILE_PMEM);
This is completely broken. This code just clears the capability bits for older versions but really does never set them anywhere. So they'll never be present in real caps.
That is visible that in the fact that this patch does not trigger an update of capability output test files, which it should.
Additionally these really should check the presence of the fields in the device properties. For properties of object memory-backend-file we already call the appropriate qom-list-properties, so they will be trivial to modify.
For the nvdimm property it will be slightly more hassle, but we really don't want to see version number based capabilities.
Sorry, I'm a very new libvirt developer and not very familiar with libvirt, so I feel a little confused about this. I don't know how to add a new qemu capability correctly, could you give an example as my reference？ Thank you in advance. Besides, I noticed that the one previous patch 'Introduce label-size for NVDIMMs' (See e433546bef6ff5be92aa0cfca7794fff67c80cac) do not touch the qemu capabilities things, Can I do like that?
+ if (ARCH_IS_X86(qemuCaps->arch) && virQEMUCapsGet(qemuCaps, QEMU_CAPS_QUERY_CPU_MODEL_EXPANSION)) virQEMUCapsSet(qemuCaps, QEMU_CAPS_CPU_CACHE); diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index b1990b6bb8..8d5fc5e8e5 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -500,6 +500,11 @@ typedef enum { /* virQEMUCapsFlags grouping marker for syntax-check */ QEMU_CAPS_OBJECT_MEMORY_MEMFD_HUGETLB, /* -object memory-backend-memfd.hugetlb */ QEMU_CAPS_IOTHREAD_POLLING, /* -object iothread.poll-max-ns */ QEMU_CAPS_MACHINE_PSERIES_CAP_NESTED_HV, /* -machine pseries.cap-nested-hv */ + QEMU_CAPS_OBJECT_MEMORY_FILE_ALIGN, /* -object memory-backend-file supports align property */ + QEMU_CAPS_OBJECT_MEMORY_FILE_PMEM, /* -object memory-backend-file supports pmem property*/ + + /* 325 */ + QEMU_CAPS_DEVICE_NVDIMM_UNARMED, /* -device nvdimm supports unarmed property*/
QEMU_CAPS_LAST /* this must always be the last item */ } virQEMUCapsFlags;
[...]
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index eae2b7edf7..5a1677ec01 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -2754,6 +2754,21 @@ mymain(void) DO_TEST("memory-hotplug-nvdimm-label", QEMU_CAPS_DEVICE_NVDIMM, QEMU_CAPS_NUMA, QEMU_CAPS_OBJECT_MEMORY_RAM, QEMU_CAPS_OBJECT_MEMORY_FILE); + DO_TEST("memory-hotplug-nvdimm-align", + QEMU_CAPS_DEVICE_NVDIMM, + QEMU_CAPS_NUMA, QEMU_CAPS_OBJECT_MEMORY_RAM, QEMU_CAPS_OBJECT_MEMORY_FILE, + QEMU_CAPS_OBJECT_MEMORY_FILE_ALIGN); + DO_TEST("memory-hotplug-nvdimm-pmem", + QEMU_CAPS_DEVICE_NVDIMM, + QEMU_CAPS_NUMA, QEMU_CAPS_OBJECT_MEMORY_RAM, QEMU_CAPS_OBJECT_MEMORY_FILE, + QEMU_CAPS_OBJECT_MEMORY_FILE_PMEM); + DO_TEST("memory-hotplug-nvdimm-unarmed", + QEMU_CAPS_DEVICE_NVDIMM, + QEMU_CAPS_NUMA, QEMU_CAPS_OBJECT_MEMORY_RAM, QEMU_CAPS_OBJECT_MEMORY_FILE, + QEMU_CAPS_DEVICE_NVDIMM_UNARMED); + DO_TEST("memory-hotplug-nvdimm-persistence", + QEMU_CAPS_DEVICE_NVDIMM, + QEMU_CAPS_NUMA, QEMU_CAPS_OBJECT_MEMORY_RAM, QEMU_CAPS_OBJECT_MEMORY_FILE);
Please use DO_TEST_CAPS_LATEST. That way it's tested with real capabilities and not something you've made up.
Here it would be visible, that your capability setting does not work at all.
Also please separate capability changes from command line generator modifications.
Got it, but as I mentioned above, why does the 'memory-hotplug-nvdimm-label' test not use the DO_TEST_CAPS_LATEST. Thank you very much.
On Thu, Nov 29, 2018 at 12:08:58 +0800, Luyao Zhong wrote:
On 2018/11/28 下午10:32, Peter Krempa wrote:
On Wed, Nov 28, 2018 at 22:09:01 +0800, Luyao Zhong wrote:
According to the result parsing from xml, add corresponding properties into QEMU command line, including 'align', 'pmem', 'unarmed' and 'nvdimm-persistence'.
Signed-off-by: Luyao Zhong <luyao.zhong@intel.com> ---
[...]
This is completely broken. This code just clears the capability bits for older versions but really does never set them anywhere. So they'll never be present in real caps.
That is visible that in the fact that this patch does not trigger an update of capability output test files, which it should.
Additionally these really should check the presence of the fields in the device properties. For properties of object memory-backend-file we already call the appropriate qom-list-properties, so they will be trivial to modify.
For the nvdimm property it will be slightly more hassle, but we really don't want to see version number based capabilities.
Sorry, I'm a very new libvirt developer and not very familiar with libvirt, so I feel a little confused about this. I don't know how to add a new qemu capability correctly, could you give an example as my reference？ Thank you in advance.
The properties of memory-backed-file are automatically detected declaratively by adding the property and required flag into the virQEMUCapsObjectPropsMemoryBackendFile array. Device properties have a similar approach but we don't have anything for nvdimm yet. You'll need to add nvdimm into virQEMUCapsDeviceProps[] with appropriate arrays for detecting the features. Note that the above will trigger test failures in the capabilities test as it will add an additional command into the query procedure. You'll need to regenerate/fix the test data for any version of qemu we have in tests/qemucapabilitiesdata/caps_* which supports NVDIMM. Note that only relevant changes should be included in such regeneration, e.g. if you have a different CPU than the original files, the CPU related data should not be changed. Regeneratin of the *.replies files can be done with tests/qemucapsprobe tool, or if you opt to add the relevant sections manually, you can use tests/qemucapsfixreplies to fix the numbering of the commands.
Besides, I noticed that the one previous patch 'Introduce label-size for NVDIMMs' (See e433546bef6ff5be92aa0cfca7794fff67c80cac) do not touch the qemu capabilities things, Can I do like that?
I don't really know when that field was introduced. If it was there from the time NVDIMM was added into qemu, no capability bit is necessary. Also in some cases, when the feture is really niche and not expected to be widely used (as in fact the whole nvdimm stuff is) we can skip that and just rely on errors from qemu.
On 2018/11/29 下午4:52, Peter Krempa wrote:
On Thu, Nov 29, 2018 at 12:08:58 +0800, Luyao Zhong wrote:
On 2018/11/28 下午10:32, Peter Krempa wrote:
On Wed, Nov 28, 2018 at 22:09:01 +0800, Luyao Zhong wrote:
According to the result parsing from xml, add corresponding properties into QEMU command line, including 'align', 'pmem', 'unarmed' and 'nvdimm-persistence'.
Signed-off-by: Luyao Zhong <luyao.zhong@intel.com> ---
[...]
This is completely broken. This code just clears the capability bits for older versions but really does never set them anywhere. So they'll never be present in real caps.
That is visible that in the fact that this patch does not trigger an update of capability output test files, which it should.
Additionally these really should check the presence of the fields in the device properties. For properties of object memory-backend-file we already call the appropriate qom-list-properties, so they will be trivial to modify.
For the nvdimm property it will be slightly more hassle, but we really don't want to see version number based capabilities.
Sorry, I'm a very new libvirt developer and not very familiar with libvirt, so I feel a little confused about this. I don't know how to add a new qemu capability correctly, could you give an example as my reference？ Thank you in advance.
The properties of memory-backed-file are automatically detected declaratively by adding the property and required flag into the virQEMUCapsObjectPropsMemoryBackendFile array.
Device properties have a similar approach but we don't have anything for nvdimm yet. You'll need to add nvdimm into virQEMUCapsDeviceProps[] with appropriate arrays for detecting the features.
Note that the above will trigger test failures in the capabilities test as it will add an additional command into the query procedure.
You'll need to regenerate/fix the test data for any version of qemu we have in tests/qemucapabilitiesdata/caps_* which supports NVDIMM.
Note that only relevant changes should be included in such regeneration, e.g. if you have a different CPU than the original files, the CPU related data should not be changed.
Regeneratin of the *.replies files can be done with tests/qemucapsprobe tool, or if you opt to add the relevant sections manually, you can use tests/qemucapsfixreplies to fix the numbering of the commands.
Thank you so much for these details. I learned a lot from the comments.
Besides, I noticed that the one previous patch 'Introduce label-size for NVDIMMs' (See e433546bef6ff5be92aa0cfca7794fff67c80cac) do not touch the qemu capabilities things, Can I do like that?
I don't really know when that field was introduced. If it was there from the time NVDIMM was added into qemu, no capability bit is necessary.
No, the 'label-size' later than NVDIMM.
Also in some cases, when the feture is really niche and not expected to be widely used (as in fact the whole nvdimm stuff is) we can skip that and just rely on errors from qemu.
So you recommend keeping this way to rely on qemu errors?
participants (2)
-
Luyao Zhong -
Peter Krempa