[libvirt] [RFC v2 2/3] nvdimm: update qemu command-line generating for NVDIMM memory
Luyao Zhong
luyao.zhong at intel.com
Thu Nov 29 04:08:58 UTC 2018
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 at 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.
More information about the libvir-list
mailing list