Re: [libvirt] [PATCH 5/5] tests: add qemuxml2argv memfd-memory-numa test

On 09/11/2018 09:45 AM, Marc-André Lureau wrote: > Hi > > On Tue, Sep 11, 2018 at 5:21 PM, John Ferlan <jferlan(a)redhat.com&gt; wrote: >> >> [...] >> >>>>> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c >>>>> index 35df63b2ac..76008a8d07 100644 >>>>> --- a/tests/qemuxml2argvtest.c >>>>> +++ b/tests/qemuxml2argvtest.c >>>>> @@ -2928,6 +2928,11 @@ mymain(void) >>>>> DO_TEST("fd-memory-no-numa-topology", QEMU_CAPS_OBJECT_MEMORY_FILE, >>>>> QEMU_CAPS_KVM); >>>>> >>>>> + DO_TEST("memfd-memory-numa", >>>>> + QEMU_CAPS_OBJECT_MEMORY_MEMFD, >>>>> + QEMU_CAPS_OBJECT_MEMORY_MEMFD_HUGETLB, >>>>> + QEMU_CAPS_KVM); >>>>> + >>>> >>>> Theoretically, if we have 3.1 capabilties to test against, then this >>>> would use a DO_TEST_CAPS_LATEST, while a "pre-3.1" would still be using >>>> -ramfd, right? That is, using DO_TEST_CAPS_VER w/ "3.0.0" would >>>> generate different results. >>>> >>>> I'm conflicted if we should wait for someone to generate the 3.1 caps or >>>> not. For whatever reason, when I post them they're not quite right for >>>> someone else's tastes... >>>> >>>> Let's see if anyone else has strong feelings one way or another. >>>> >>> >>> -memfd is available since 2.12. After patch 1 & 2 are applied, we >>> should probably switch to use DO_TEST_CAPS_LATEST. >>> >> >> Theoretically patches 3, 4, and 5 could be one patch, but having >> separate also works well for review purposes! >> >> While MEMFD is there is the HUGETLB and comment in page 2 about QEMU 3.1 >> that is what I was concerned with, especially since 2.12 and 3.0 find >> the value... >> >> Looking at the QEMU sources, I see you added the field in commit >> dbb9e0f40, which is 2.12 based. > > It's added in 2.12: > git describe --contains --match=v2* dbb9e0f40 > v2.12.0-rc0~107^2~8 > > However, only with upcoming patch for 3.1 (queued by Paolo today) will > the hugetlb properties be run-time checked/exposed. > >> Still reading deeper into the comments in patch 2, it just seems that >> @hugetlbsize has some sort run-time issue that gets fixed by 3.1. Harder > > It's not an issue, but it will help libvirt to figure out before > starting qemu if anonymous memfd hugetlb is supported. > >> for libvirt to detect that an issue exists unless something was added in >> 3.1 that libvirt could test on for a capability. I'm not sure what the >> issue is, but maybe that's something document-able at least with respect >> to what values are provided in the XML for memoryBacking. > > If you request anonymous memory & hugetlb today, you have a libvirt > error. With the series, if the host/qemu doesn't support it, you will > get an error. Now I'm getting more confused. With this patch series applied, but without the 3.1 changes, if the anonymous memfd hugetlb is used there will be a run time issue? IOW: Does it really only work in 3.1? If so, then we need to figure out a mechanism for determining that as there's no reason to "default to" -memfd then for 2.12 and 3.0, right?

> > https://libvirt.org/formatdomain.html#elementsMemoryBacking > > There is no documentation about the file memory backing requirement > today (it seems). We could explain it and add that a > memfd-hugetlb-capable doesn't need it (when there is no numa > assignment). Is this what you are asking? > Essentially - I'm sure we'd have to carefully word things to take into account Michal's position of we don't want to describe the conditions related to what backend is being used "by default" and for "which version". Still I think the whether to document or not is related to what the hugetlb problem is. Tough to say don't use this unless you have qemu 3.1 installed even though it's supported back to 2.12. I don't even want to think about describing the migration discussion...

John >> >> >> John >> >>> Before 2.12 (or if the capabilities are not exposed by the host qemu) >>> the argv will use -file. This is already covered by existing tests, >>> like hugepages-shared. >>> >>> thanks >>> >>>> John >>>> >>>>> DO_TEST("cpu-check-none", QEMU_CAPS_KVM); >>>>> DO_TEST("cpu-check-partial", QEMU_CAPS_KVM); >>>>> DO_TEST("cpu-check-full", QEMU_CAPS_KVM); >>>>>