[libvirt] [PATCH 5/7] conf: Sync caps data even when SKIP_OSTYPE_CHECKS

[Michal Privoznik]

On 07/24/2018 11:23 PM, Cole Robinson wrote:
> We should still make an effort to fill in data, just not raise
> an error if say an ostype/virttype combo disappeared from caps.
> 
> Signed-off-by: Cole Robinson <crobinso at redhat.com>
> ---
>  src/conf/domain_conf.c                     | 13 ++++++-------
>  tests/qemuxml2argvdata/missing-machine.xml |  2 +-
>  tests/qemuxml2argvtest.c                   |  3 +++
>  3 files changed, 10 insertions(+), 8 deletions(-)
> 
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index b7f6a22e20..78ee000857 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -19178,18 +19178,17 @@ virDomainDefParseCaps(virDomainDefPtr def,
>          goto cleanup;
>      }
>  
> -    if (!(flags & VIR_DOMAIN_DEF_PARSE_SKIP_OSTYPE_CHECKS)) {
> -        if (!(capsdata = virCapabilitiesDomainDataLookup(caps,
> -                def->os.type, def->os.arch, def->virtType,
> -                NULL, NULL)))
> +    if (!(capsdata = virCapabilitiesDomainDataLookup(caps, def->os.type,
> +            def->os.arch, def->virtType, NULL, NULL))) {

This looks misaligned ;-)

> +        if (!(flags & VIR_DOMAIN_DEF_PARSE_SKIP_VALIDATE))
>              goto cleanup;


So you're changing the flag here even though I believe it belongs to the
next patch. It helps downstream maintainers, but in the end the code
will look the same.

> -
> +        virResetLastError();
> +    } else {
>          if (!def->os.arch)
>              def->os.arch = capsdata->arch;
>          if ((!def->os.machine &&
> -             VIR_STRDUP(def->os.machine, capsdata->machinetype) < 0)) {
> +             VIR_STRDUP(def->os.machine, capsdata->machinetype) < 0))
>              goto cleanup;
> -        }
>      }
>  
>      ret = 0;
> diff --git a/tests/qemuxml2argvdata/missing-machine.xml b/tests/qemuxml2argvdata/missing-machine.xml
> index 4ce7b377a5..2900baec90 100644
> --- a/tests/qemuxml2argvdata/missing-machine.xml
> +++ b/tests/qemuxml2argvdata/missing-machine.xml
> @@ -6,7 +6,7 @@
>    <currentMemory unit='KiB'>219100</currentMemory>
>    <vcpu placement='static' cpuset='1'>1</vcpu>
>    <os>
> -    <type arch='i686'>hvm</type>
> +    <type arch='alpha'>hvm</type>

Firstly I was wondering why is this change needed, but then I read the
comment in the next hunk and it makes sense. We need to have
non-existent pair of arch and os type so that the error is triggered.

>      <boot dev='hd'/>
>    </os>
>    <clock offset='utc'/>
> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
> index 1a936faef1..03b6d92912 100644
> --- a/tests/qemuxml2argvtest.c
> +++ b/tests/qemuxml2argvtest.c
> @@ -2773,6 +2773,9 @@ mymain(void)
>              QEMU_CAPS_OBJECT_GPEX,
>              QEMU_CAPS_NEC_USB_XHCI);
>  
> +    /* VM XML has invalid arch/ostype/virttype combo, but the SKIP flag
> +     * will avoid the error. Still, we expect qemu driver to complain about
> +     * missing machine error, and not crash */
>      DO_TEST_PARSE_FLAGS_ERROR("missing-machine",
>                                VIR_DOMAIN_DEF_PARSE_SKIP_OSTYPE_CHECKS,
>                                NONE);
> 

Michal