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

[John Ferlan]

On 09/13/2018 03:39 AM, Marc-André Lureau wrote:
> 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.
> 

Let's see what I can cobble together. I'll repost the series a bit later
today hopefully.

John

>>
>>>
>>> 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.
>