[libvirt] [PATCH v2 5/6] tests: qemuxml2xml: assign device addresses

[Laine Stump]

On 02/09/2016 10:59 AM, Cole Robinson wrote:
> We use the PreFormat callback for this. Many test cases need to be extended
> to pass in proper qemuCaps flags so AssignAddresses doesn't throw errors.
>
> One test case (pcie-root-port-too-many) is dropped, since it was meant
> only for checking an error condition in qemuxml2argv, and one we add in
> AssignAddresses it errors here too.
>
> Long term I think AssignAddresses should be handled in qemu's PostParse
> callback, but that's not entirely straightforward. Handling it here
> means we can get the test suite churn over with.
> ---

I'm too lazy to do it myself, but it would be comforting to move the 
xml2xmlout data files into the xml2argvdata directory and run 
qemuxml2argvtest with the existing .args files to verify that all of 
these PCI addresses you've just generated are exactly the same as the 
ones that have been auto-generated over the last several years and put 
into the .args files...

(Hmm - if the xml files in all the test data directories had their 
"qemuxml2XXX-" prefixes removed as we had earlier discussed, such an 
operation would be trivial :-)

> diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c
> index e3b61c0..a06aa4d 100644
> --- a/tests/qemuxml2xmltest.c
> +++ b/tests/qemuxml2xmltest.c
> @@ -37,13 +37,24 @@ struct testInfo {
>   };
>   
>   static int
> +qemuXML2XMLPreFormatCallback(virDomainDefPtr def, const void *opaque)
> +{
> +    const struct testInfo *info = opaque;
> +
> +    if (qemuDomainAssignAddresses(def, info->qemuCaps, NULL))
> +        return -1;
> +
> +    return 0;
> +}
> +
> +static int
>   testXML2XMLActive(const void *opaque)
>   {
>       const struct testInfo *info = opaque;
>   
>       return testCompareDomXML2XMLFiles(driver.caps, driver.xmlopt,
>                                         info->inName, info->outActiveName, true,
> -                                      NULL, NULL);
> +                                      qemuXML2XMLPreFormatCallback, opaque);
>   }
>   
>   
> @@ -54,7 +65,7 @@ testXML2XMLInactive(const void *opaque)
>   
>       return testCompareDomXML2XMLFiles(driver.caps, driver.xmlopt, info->inName,
>                                         info->outInactiveName, false,
> -                                      NULL, NULL);
> +                                      qemuXML2XMLPreFormatCallback, opaque);
>   }
>   
>   
> @@ -138,6 +149,9 @@ testCompareStatusXMLToXMLFiles(const void *opaque)
>           goto cleanup;
>       }
>   
> +    if (qemuDomainAssignAddresses(obj->def, data->qemuCaps, NULL))
> +        goto cleanup;
> +

Why is this needed? I though that's what the pre-format callback was for.


>       /* format it back */
>       if (!(actual = virDomainObjFormat(driver.xmlopt, obj, NULL,
>                                         VIR_DOMAIN_DEF_FORMAT_SECURE))) {
> @@ -379,14 +393,28 @@ mymain(void)
>       DO_TEST("disk-drive-network-rbd-ipv6");
>       DO_TEST("disk-drive-network-rbd-ceph-env");
>       DO_TEST("disk-drive-network-sheepdog");
> -    DO_TEST("disk-scsi-device");
> +    DO_TEST_FULL("disk-scsi-device", WHEN_ACTIVE,
> +            QEMU_CAPS_NODEFCONFIG,
> +            QEMU_CAPS_SCSI_LSI);


The indentation of all of the followon lines to DO_TEST_FULL()'s are 
off. It's consistent, but since it doesn't match what all of our editors 
likely do automatically, so it will start to look ugly as new cases are 
added. (I didn't look - is the indentation also incorrect in 
qemuxml2argvtest.c (which is where I assume you got the caps lists for 
each test)?)

ACK with the question about the extra call to 
qemuDomainAssignAddresses() answered and the indentation fixed so that 
it matches what an editor's auto-indent would do (putting the following 
lines one space past the column of the opening "(" )