[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