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

[Marc-André Lureau]

Hi

On Thu, Sep 13, 2018 at 2:25 AM, John Ferlan <jferlan at redhat.com> wrote:
>
> [...]
>
>>>
>>> So all that's "left":
>>>
>>>  1. "Add" a check in qemuDomainABIStabilityCheck to ensure we're not
>>> changing from memory-backend-ram to memory-backend-memfd.  We already
>>> check that "(src->mem.source != dst->mem.source)" - so we know we're
>>> already anonymous or not.
>>>
>>> Any suggestions?  If source is anonymous, then what?  I think we can use
>>> the qemuDomainObjPrivatePtr in some way to determine that we were
>>> started with -memfd (or not started that way).
>>
>> No idea how we could save that information across various restarts /
>> version changes.
>
> I think it'd be ugly... I think migration cookies would have to be
> used... I considered other mechanisms, but each wouldn't quite work.
> Without writing the code, if we cared to do this, then we'd have:
>
> 1. Add a field to qemuDomainObjPrivatePtr that indicates what got
> started (none, memfd, file, or ram). Add a typedef enum that has
> unknown, none, memfd, file, and ram. Add the Parse/Format code to handle
> the field.
>
> 2. Modify the qemu_command code to set the field in priv based on what
> got started, if something got started. The value would be > 0...
>
> 3. Mess with the migration cookie logic to add checks for what the
> source started. On the destination side of that cookie if we had the
> "right capabilities", then check the source cookie to see what it has.
> If it didn't have that field, then I think one could assume the source
> with anonymous memory backing would be using -ram. We'd already fail the
> src/dst mem.source check if one used -file. I'm not all the versed in
> the cookies, but I think that'd work "logically thinking" at least. The
> devil would be in the details.
>
> Assuming your 3.1 patches do something to handle the condition, I guess
> it comes does to how much of a problem it's believed this could be in
> 2.12 and 3.0 if someone is running -ram and migrates to a host that
> would default to -memfd.

I am afraid we will need to do it to handle transparent -memfd usage.
I'll look at it with your help.

>
>>
>> Tbh, I would try to migrate, and let qemu fail if something is
>> incompatible (such as incompatible memory backends or memory region
>> name mismatch). See also my qemu series "[PATCH 0/9] hostmem-ram: use
>> whole path for region name with >= 3.1". It feels like libvirt
>> duplicates some qemu logic/error otherwise.
>>
>
> I'm sure there's lots of duplication, but generally doing the checks in
> libvirt allow for a bit "easier" (in least in terms of libvirt) backout
> logic. Once the qemu process starts - if the process eventually dies
> because of something, then the logging only goes to libvirt log files.
> If the process fails to start, libvirt does capture and give that
> information back to the consumer. So call it preventative duplication. I
> think historically some qemu error messages have been a bit too vague to
> figure out why something didn't work.
>
>>>
>>>  2. Get the patches I posted today to cleanup/move the memory backing
>>> checks from domain_conf to qemu_domain:
>>>
>>>    https://www.redhat.com/archives/libvir-list/2018-September/msg00463.html
>>>
>>> reviewed and pushed so that patch4 can use the qemu_domain API to alter
>>> it's hugepages check.
>>
>> done
>>
>
> Thanks - I pushed that...
>
>> feel free to update & resend my series, or else I will rebase and resend it
>>
>> thanks
>>
>
> OK - I adjusted your changes to handle the previously agreed upon
> "issues" and was ready to push the series when it dawned on me that the
> MEMFD and MEMFD_HUGETLB capabilities both use the 2.12 release - so
> realistically would the latter really be necessary?
>
> Again if something doesn't quite work in 2.12 and 3.0 for hugetlb, then
> perhaps there's something in 3.1 that can be checked.
>
> I can remove or keep patch 2. If removed, then just use MEMFD as the
> basis. Your call.

I'd keep the MEMFD_HUGETLB check, even with <3.1.