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

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

 



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.

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

> Signed-off-by: John Ferlan <jferlan@xxxxxxxxxx>
> ---
>  tests/qemucapsprobemock.c                     | 50 +++++++++++++++++++
>  ...=> launch-security-sev.x86_64-latest.args} |  0
>  tests/qemuxml2argvtest.c                      |  2 +-
>  3 files changed, 51 insertions(+), 1 deletion(-)
>  rename tests/qemuxml2argvdata/{launch-security-sev.x86_64-2.12.0.args => launch-security-sev.x86_64-latest.args} (100%)
> 
> diff --git a/tests/qemucapsprobemock.c b/tests/qemucapsprobemock.c
> index f3f17f2116..0351b946b2 100644
> --- a/tests/qemucapsprobemock.c
> +++ b/tests/qemucapsprobemock.c
> @@ -22,9 +22,15 @@
>  #include "internal.h"
>  #include "viralloc.h"
>  #include "virjson.h"
> +#include "virlog.h"
> +#include "virstring.h"
>  #include "qemu/qemu_monitor.h"
>  #include "qemu/qemu_monitor_json.h"
>  
> +#define VIR_FROM_THIS VIR_FROM_NONE
> +
> +VIR_LOG_INIT("tests.qemucapsprobemock");
> +
>  #define REAL_SYM(realFunc) \
>      do { \
>          if (!realFunc && !(realFunc = dlsym(RTLD_NEXT, __FUNCTION__))) { \
> @@ -120,3 +126,47 @@ qemuMonitorJSONIOProcessLine(qemuMonitorPtr mon,
>      virJSONValueFree(value);
>      return ret;
>  }
> +
> +
> +static int (*realQemuMonitorJSONGetSEVCapabilities)(qemuMonitorPtr mon,
> +                                                    virSEVCapability **capabilities);
> +
> +int
> +qemuMonitorJSONGetSEVCapabilities(qemuMonitorPtr mon,
> +                                  virSEVCapability **capabilities)
> +{
> +    int ret = -1;
> +    VIR_AUTOPTR(virSEVCapability) capability = NULL;
> +
> +    VIR_DEBUG("mocked qemuMonitorJSONGetSEVCapabilities");
> +
> +    REAL_SYM(realQemuMonitorJSONGetSEVCapabilities);
> +
> +    ret = realQemuMonitorJSONGetSEVCapabilities(mon, capabilities);
> +
> +    if (ret == 0) {
> +        /* QEMU has only compiled-in support of SEV in which case we
> +         * can mock up a response instead since generation of SEV output
> +         * is only possible on AMD hardware. Since the qemuxml2argvtest
> +         * doesn't currently distinguish between AMD and Intel for x86_64
> +         * if we "alter" the pseudo failure we can at least allow the
> +         * test to succeed using the latest replies rather than a specific
> +         * version with altered reply data */

My brain kinda chokes on this comment. Maybe something like

* QEMU only reports SEV support if it was compiled on appropriate AMD
* hardware. Report SEV unconditionally here so we aren't dependent on
* AMD specific output in our test capabilities .replies

Or something like that.

Reviewed-by: Cole Robinson <crobinso@xxxxxxxxxx>

- Cole

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list



[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