1:37 p.m.
On 05/17/2016 10:39 AM, Andrea Bolognani wrote:
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.
I was just doing it to be consistent with the other callbacks. I just left it
as is since I can imagine some day we are going to want to abide parseFlags at
least.
> + 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().
This callback could be used for other types of testing, like assigning aliases
which we don't presently test. So I decided to leave it in.
Thanks,
Cole