Re: [PATCH 1/2] tests: Add mocking for qemuMonitorJSONGetSEVCapabilities

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

 




On 2/7/19 3:36 AM, Erik Skultety wrote:
> On Wed, Feb 06, 2019 at 03:32:44PM -0500, Cole Robinson wrote:
>> On 1/23/19 3:59 PM, John Ferlan wrote:
>>> Commit d4005609 added "altered" capabilities replies output
>>> in order to fake a 'query-sev-capabilities' reply from QEMU.
>>> This worked fine for the 2.12 processing for qemuxml2argvtest
>>> until the next capabilities was generated and the output wasn't
>>> doctored. Thus commit 6c50cef8 used DO_TEST_CAPS_VER against
>>> 2.12.0 noting that the 2.12.0 capabilities were hand edited
>>> to add AMD specific output into an Intel capabilities reply.
>>>
>>> Instead of "altering" the output or running against a specific
>>> reply that we know was altered, let's instead use the mocking
>>> capabilities to check the return from a real call and mock up
>>> return data if we determine the returned real call doesn't
>>> support compiled-in SEV. This way the qemuxml2argvtest can
>>> use the DO_TEST_CAPS_LATEST which runs only for x86_64 to
>>> determine that noting in "latest" changes with respect to
>>> SEV and effectively fake things out to generate expected
>>> output ensuring that other changes to libvirt/qemu don't
>>> somehow affect SEV support.
>>>
>>
>> Seems reasonable to me. This is an improvement over the current state
>> that appears to implicitly require manually editing caps replies to list
>> SEV support. The downside is that there's no way to test lack of SEV
>> support now, but we weren't testing that anyways it seems. More complete
>> solutions adding caps files for both intel and AMD sound nicer but are
>> also a lot more work.
> 
> I've been working on this as a side project, like you said yourself - *lots* of
> work. At the moment, I don't see any other viable option that continue doing
> what we already do.
> 

What we "are" testing is that the doctored qemu 2.12 replies for an
Intel machine work for this AMD environment. All the patch does is
extend that slightly ;-) to starting with 2.12...

Since we "fake" other things being available - it didn't seem to be too
much of a reach for "just a test"...

Part of my logic for taking this approach is I didn't want to make
another freaking macro in the already overloaded DO_TEST_*CAPS* pile of
macros that would be able to distinguish between Intel and AMD for x86_64.

Additionally I never seem to be able to get generation of the
capabilities output (tests/qemucapabilitiesdata/*) exactly like how any
reviewer would accept, so I've just given up trying. I still dislike
that some of the data doesn't come from a released version (see output
grep package tests/qemucapabilitiesdata/*.xml | grep \>.*v  or | grep
\-, IOW: rc0, rc1, ###-g*).

>>
>> Still it seems like it would be worthwhile to have actual AMD
>> capabilities output in the test suite, even if it's just a single
>> example. But I guess we would need to fine someone with the proper
>> hardware and extend the macros in qemuxml2argv to handle it. Anyways...
> 
> I contacted Yash about this as I'd like to move this whole process to Jenkins
> so we don't have to constantly trip over this with every new QEMU release, the
> question is whether ci.centos.org has an AMD HW available, especially
> EPYC-powered. Yash gave me a contact to a ci.centos.org guy and I need to carry
> on with the discussion.
> 

Automation of data that has to be reviewed and committed to git...
unless I'm misunderstanding the intention.

This one will perhaps sit on the shelf a bit to see if there's other
comments, suggestions, ideas, or forgetaboutit's

John


[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux