[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [RFC v2 2/3] nvdimm: update qemu command-line generating for NVDIMM memory





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?


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]