Re: [libvirt] [PATCH 2/3] qemu: Update x86_64 QEMU 3.0.0 capabilities

On 1/23/19 6:42 AM, Pavel Hrdina wrote: True. Of course results are dependent upon who runs them and on what hardware they run them on and of course how the target QEMU was built. There's nothing documented in qemucapabilitiestest.c that says, "build qemu using" - just how to create the replies file using a built qemu. As I found, before running ./configure a "yum builddep qemu" should also be run to ensure whatever the checked out tag needs is installed. So are the results a fault of a bad process or a bad test? Or just something we have to accept that in order to have "recent results"? Are there qemuxml2argvtest's that care about the CPU model? Obviously not - it's just a string. What does care about the model is the SEV output. Are there rules to what is found acceptable? Seems we've accepted odd results in the past. Two examples, commit d4005609f3 which hand edited in the SEV output and commit 6c50cef8 which tied the testing of "launch-security-sev" to the "hand edited" caps for "2.12.0". It seems that test is the only one where we've allowed a DO_TEST_CAPS_VER without also a DO_TEST_CAPS_LATEST for a test. In thinking about that now, it may be a wrong decision. IIRC the _VER test was meant for when there were differences in output between QEMU caps versions, but that the _LATEST should be provided for completeness. Maybe the solution should be that qemucapsprobe lies and fills in the query-sev-capabilities results via some voodoo and black magic. That depends on whether we truly care about what qemuxml2argvtest is testing. Considering that commit d4005609f3 was allowed to proceed even though: "model-id": "Intel(R) Xeon(R) CPU E5-2630 v3 @ 2.40GHz", was true, then what difference does it make if this series also hand edits things? It was only after posting did I realize the extent of the SEV testing alterations. So we only test that we create a proper sev command line for qemu 2.12 capabilities. If something ever changed in the short term future which necessitated more/different options, then we'd be in the same situation. As noted above - it has nothing to do with downgrading a CPU - it has everything to do with where one runs tests/qemucapsprobe. Generating the patches shouldn't be that much effort, but the methodology used to generate the *.replies files should be consistent and documented. We may have to just accept there will be some output differences. As long as those differences don't require alterations to some existing qemuxml2argvtest output, then does it really matter? I'm more than happy to let someone else generate the "more recent" .replies files. I think it's something we should do each time QEMU reaches it's final version build. As the 2.12 results show the difference between -rc0 and the final tag were perhaps more extensive. At least nothing required qemuxml2argvtest .args output changes. FWIW: The 4.0 caps just pushed used the same model-id that I have: "model-id": "Intel(R) Core(TM) i7-8650U CPU @ 1.90GHz", However, they were built w/o the "--disable-xen" which seems to be how other caps files were built (based on the results I got). If I remove the --disable-xen from my ./configure command above and regenerate results based on the commit id Cole noted for 4.0, then I got very similar, but not exactly similar results. The difference happened to be that "vmx" was false for me, which reminded me I needed to enable nested virt for my new laptop... With that adjustment, I got the same results. Of course that also changes the results for what I've posted... But it also shows perhaps another "difference" that can occur although we don't care about that difference in our tests. Still, should that be mocked to always return true? (probably not, unless we care that much about masking certain differences). Should we mock the 'model-id' output to be something else too? Well, we could also go with the voodoo or black magic logic and mock the results. Of course what we do depends more on what we care more about? Absolutely valid results based on the CPU used to run qemucapsprobe or generating a caps_VER.ARCH.replies file that allows us to test without needing to have "x86_64.intel.replies" and "x86_64.amd.replies" files. Since using DO_TEST*_LATEST has been the most recent push/request when altering qemuxml2argvtest, then I think it seems we care more that the replies files will have results that return valid data for everything and not : "error": { "class": "GenericError", "desc": "SEV feature is not available" } John