On Sat, 2016-05-14 at 16:00 -0400, Cole Robinson wrote:
> This wires up qemuDomainAssignAddresses into the new
> virDomainDefAssignAddressesCallback, so it's always triggered
> via virDomainDefPostParse. We are essentially doing this already
> with open coded calls sprinkled about
Missing period.
> qemu argv parse output changes slightly since previously it wasn't
> hitting qemuDomainAssignAddresses.
> ---
> src/qemu/qemu_domain.c | 25 ++++++++++++++++++++++
> .../qemuargv2xmldata/qemuargv2xml-pseries-disk.xml | 4 +++-
> tests/qemuxml2argvtest.c | 2 +-
> tests/qemuxml2xmltest.c | 11 ++--------
> 4 files changed, 31 insertions(+), 11 deletions(-)
>
> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
> index b0eb3b6..50505a6 100644
> --- a/src/qemu/qemu_domain.c
> +++ b/src/qemu/qemu_domain.c
> @@ -2293,9 +2293,34 @@ qemuDomainDeviceDefPostParse(virDomainDeviceDefPtr dev,
> }
>
>
> +static int
> +qemuDomainDefAssignAddressesCallback(virDomainDef *def,
> + virCapsPtr caps ATTRIBUTE_UNUSED,
> + unsigned int parseFlags ATTRIBUTE_UNUSED,
So these two arguments are unused, and they remain unused by
the end of the series... I guess you wanted to have the same
signature as virDomainDefPostParseCallback()?
Not sure if it's worth keeping them around, but I guess it's
not a big deal either way.
> + void *opaque)
> +{
> + virQEMUDriverPtr driver = opaque;
> + virQEMUCapsPtr qemuCaps = NULL;
> + int ret = -1;
> +
> + if (!(qemuCaps = virQEMUCapsCacheLookup(driver->qemuCapsCache,
> + def->emulator)))
> + goto cleanup;
> +
> + if (qemuDomainAssignAddresses(def, qemuCaps, NULL) < 0)
> + goto cleanup;
> +
> + ret = 0;
> + cleanup:
> + virObjectUnref(qemuCaps);
> + return ret;
> +}
> +
> +
> virDomainDefParserConfig virQEMUDriverDomainDefParserConfig = {
> .devicesPostParseCallback = qemuDomainDeviceDefPostParse,
> .domainPostParseCallback = qemuDomainDefPostParse,
> + .assignAddressesCallback = qemuDomainDefAssignAddressesCallback,
I'd say call it qemuDomainDefAssignAddresses for consistency's
sake.
> .features = VIR_DOMAIN_DEF_FEATURE_MEMORY_HOTPLUG |
> VIR_DOMAIN_DEF_FEATURE_OFFLINE_VCPUPIN
> };
> diff --git a/tests/qemuargv2xmldata/qemuargv2xml-pseries-disk.xml b/tests/qemuargv2xmldata/qemuargv2xml-pseries-
> disk.xml
> index 8cec27c..ab9195a 100644
> --- a/tests/qemuargv2xmldata/qemuargv2xml-pseries-disk.xml
> +++ b/tests/qemuargv2xmldata/qemuargv2xml-pseries-disk.xml
> @@ -29,7 +29,9 @@
> </disk>
> <controller type='usb' index='0'/>
> <controller type='pci' index='0' model='pci-root'/>
> - <controller type='scsi' index='0'/>
> + <controller type='scsi' index='0'>
> + <address type='spapr-vio' reg='0x2000'/>
> + </controller>
> <input type='keyboard' bus='usb'/>
> <input type='mouse' bus='usb'/>
> <graphics type='sdl'/>
> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
> index d1cfbec..840efc9 100644
> --- a/tests/qemuxml2argvtest.c
> +++ b/tests/qemuxml2argvtest.c
> @@ -1382,7 +1382,7 @@ mymain(void)
> QEMU_CAPS_PCI_OHCI, QEMU_CAPS_PCI_MULTIFUNCTION);
> DO_TEST("pseries-vio-user-assigned",
> QEMU_CAPS_CHARDEV, QEMU_CAPS_NODEFCONFIG);
> - DO_TEST_FAILURE("pseries-vio-address-clash",
> + DO_TEST_PARSE_ERROR("pseries-vio-address-clash",
> QEMU_CAPS_CHARDEV, QEMU_CAPS_NODEFCONFIG);
> DO_TEST("pseries-nvram", QEMU_CAPS_DEVICE_NVRAM);
> DO_TEST("pseries-usb-kbd", QEMU_CAPS_PCI_OHCI,
> diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c
> index 5a43fa9..9eb2625 100644
> --- a/tests/qemuxml2xmltest.c
> +++ b/tests/qemuxml2xmltest.c
> @@ -37,13 +37,9 @@ struct testInfo {
> };
>
> static int
> -qemuXML2XMLPreFormatCallback(virDomainDefPtr def, const void *opaque)
> +qemuXML2XMLPreFormatCallback(virDomainDefPtr def ATTRIBUTE_UNUSED,
> + const void *opaque ATTRIBUTE_UNUSED)
> {
> - const struct testInfo *info = opaque;
> -
> - if (qemuDomainAssignAddresses(def, info->qemuCaps, NULL))
> - return -1;
> -
> return 0;
> }
If you're going to remove the whole function body, why not go
the extra mile? Get rid of the function altogether and just
pass NULL to testCompareDomXML2XMLFiles().
> @@ -153,9 +149,6 @@ testCompareStatusXMLToXMLFiles(const void *opaque)
> goto cleanup;
> }
>
> - if (qemuDomainAssignAddresses(obj->def, data->qemuCaps, NULL))
> - goto cleanup;
> -
> /* format it back */
> if (!(actual = virDomainObjFormat(driver.xmlopt, obj, NULL,
> VIR_DOMAIN_DEF_FORMAT_SECURE))) {
ACK
--
Andrea Bolognani
Software Engineer - Virtualization Team
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list