diff mbox

[2/4] qemu: Assign device addresses in PostParse

Message ID 4c2c4892374e97c6d3cfc71a5ed0448629fe375e.1463255562.git.crobinso@redhat.com
State New
Headers show

Commit Message

Cole Robinson May 14, 2016, 8 p.m. UTC 
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

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(-)

-- 
2.7.4

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

Comments

Cole Robinson May 18, 2016, 6:37 p.m. UTC | #1 
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

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

Patch

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,
+                                     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,
     .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;
 }
 
@@ -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))) {