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
