Commit v10.0.0-265-ge67bca23e4 added a `active_config` and
`defined_config` to nodedev mdev internal XML handling.
`defined_config` can be filled at XML parse time, but `active_config`
must be filled in by nodedev driver. This wasn't implemented for the
test driver however, which caused virt-manager test suite regressions.

Working example:

```
$ virsh --connect test:///home/crobinso/src/virt-manager/tests/data/testdriver/testdriver.xml nodedev-dumpxml mdev_8e37ee90_2b51_45e3_9b25_bf8283c03110
<device>
  <name>mdev_8e37ee90_2b51_45e3_9b25_bf8283c03110</name>
  <path>/sys/devices/css0/0.0.0023/8e37ee90-2b51-45e3-9b25-bf8283c03110</path>
  <parent>css_0_0_0023</parent>
  <capability type='mdev'>
    <type id='vfio_ccw-io'/>
    <iommuGroup number='0'/>
  </capability>
</device>
```

Broken example:

```
$ virsh --connect test:///home/crobinso/src/virt-manager/tests/data/testdriver/testdriver.xml nodedev-dumpxml mdev_8e37ee90_2b51_45e3_9b25_bf8283c03110
<device>
  <name>mdev_8e37ee90_2b51_45e3_9b25_bf8283c03110</name>
  <path>/sys/devices/css0/0.0.0023/8e37ee90-2b51-45e3-9b25-bf8283c03110</path>
  <parent>css_0_0_0023</parent>
  <capability type='mdev'>
    <iommuGroup number='0'/>
  </capability>
</device>
```

There's already code that does what we want in the test suite.
Move it to a shared function, and call it in test driver when
creating a nodedev from driver startup XML.

Signed-off-by: Cole Robinson <crobinso@redhat.com>
---
 src/conf/node_device_conf.c | 24 ++++++++++++++++++++++++
 src/conf/node_device_conf.h |  3 +++
 src/libvirt_private.syms    |  1 +
 src/test/test_driver.c      |  1 +
 tests/nodedevxml2xmltest.c  | 18 +++---------------
 5 files changed, 32 insertions(+), 15 deletions(-)

diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c
index 5cfbd6a7eb..fe6d9a36b2 100644
--- a/src/conf/node_device_conf.c
+++ b/src/conf/node_device_conf.c
@@ -2804,6 +2804,30 @@ virNodeDeviceCapsListExport(virNodeDeviceDef *def,
     return ncaps;
 }
 
+void
+virNodeDeviceSyncMdevActiveConfig(virNodeDeviceDef *def)
+{
+    size_t i;
+    virNodeDevCapsDef *caps;
+
+    for (caps = def->caps; caps; caps = caps->next) {
+        virNodeDevCapData *data = &caps->data;
+
+        if (caps->data.type != VIR_NODE_DEV_CAP_MDEV)
+            continue;
+
+        data->mdev.active_config.type = g_strdup(data->mdev.defined_config.type);
+        for (i = 0; i < data->mdev.defined_config.nattributes; i++) {
+            g_autoptr(virMediatedDeviceAttr) attr = g_new0(virMediatedDeviceAttr, 1);
+
+            attr->name = g_strdup(data->mdev.defined_config.attributes[i]->name);
+            attr->value = g_strdup(data->mdev.defined_config.attributes[i]->value);
+            VIR_APPEND_ELEMENT(data->mdev.active_config.attributes,
+                               data->mdev.active_config.nattributes,
+                               attr);
+        }
+    }
+}
 
 #ifdef __linux__
 
diff --git a/src/conf/node_device_conf.h b/src/conf/node_device_conf.h
index f0a5333881..4b82636af7 100644
--- a/src/conf/node_device_conf.h
+++ b/src/conf/node_device_conf.h
@@ -470,3 +470,6 @@ virNodeDeviceUpdateCaps(virNodeDeviceDef *def);
 int
 virNodeDeviceCapsListExport(virNodeDeviceDef *def,
                             virNodeDevCapType **list);
+
+void
+virNodeDeviceSyncMdevActiveConfig(virNodeDeviceDef *def);
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 839fe4f545..3186dd6d23 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -888,6 +888,7 @@ virNodeDeviceGetPCIDynamicCaps;
 virNodeDeviceGetSCSIHostCaps;
 virNodeDeviceGetSCSITargetCaps;
 virNodeDeviceGetWWNs;
+virNodeDeviceSyncMdevActiveConfig;
 virNodeDeviceUpdateCaps;
 
 
diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index e7d2b6c866..d2d1bc43e3 100644
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -1272,6 +1272,7 @@ testParseNodedevs(testDriver *privconn,
         virNodeDeviceObjSetPersistent(obj, true);
         virNodeDeviceObjSetActive(obj, true);
         virNodeDeviceObjSetSkipUpdateCaps(obj, true);
+        virNodeDeviceSyncMdevActiveConfig(def);
         virNodeDeviceObjEndAPI(&obj);
     }
 
diff --git a/tests/nodedevxml2xmltest.c b/tests/nodedevxml2xmltest.c
index e918922672..814a817725 100644
--- a/tests/nodedevxml2xmltest.c
+++ b/tests/nodedevxml2xmltest.c
@@ -24,7 +24,6 @@ testCompareXMLToXMLFiles(const char *xml, const char *outfile, unsigned int flag
     int ret = -1;
     virNodeDeviceDef *dev = NULL;
     virNodeDevCapsDef *caps;
-    size_t i;
 
     if (virTestLoadFile(xml, &xmlData) < 0)
         goto fail;
@@ -52,22 +51,11 @@ testCompareXMLToXMLFiles(const char *xml, const char *outfile, unsigned int flag
                                            data->storage.logical_block_size;
             }
         }
-
-        if (caps->data.type == VIR_NODE_DEV_CAP_MDEV &&
-            !(flags & VIR_NODE_DEVICE_XML_INACTIVE)) {
-            data->mdev.active_config.type = g_strdup(data->mdev.defined_config.type);
-            for (i = 0; i < data->mdev.defined_config.nattributes; i++) {
-                g_autoptr(virMediatedDeviceAttr) attr = g_new0(virMediatedDeviceAttr, 1);
-
-                attr->name = g_strdup(data->mdev.defined_config.attributes[i]->name);
-                attr->value = g_strdup(data->mdev.defined_config.attributes[i]->value);
-                VIR_APPEND_ELEMENT(data->mdev.active_config.attributes,
-                                   data->mdev.active_config.nattributes,
-                                   attr);
-            }
-        }
     }
 
+    if (!(flags & VIR_NODE_DEVICE_XML_INACTIVE))
+        virNodeDeviceSyncMdevActiveConfig(dev);
+
     if (!(actual = virNodeDeviceDefFormat(dev, flags)))
         goto fail;
 
-- 
2.44.0
_______________________________________________
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-leave@lists.libvirt.org

Cole, thanks for fixing the problem in test_driver caused by my changes.

Reviewed-by: Boris Fiuczynski <fiuczy@linux.ibm.com>

On 4/23/24 15:44, Cole Robinson wrote:
> Commit v10.0.0-265-ge67bca23e4 added a `active_config` and
> `defined_config` to nodedev mdev internal XML handling.
> `defined_config` can be filled at XML parse time, but `active_config`
> must be filled in by nodedev driver. This wasn't implemented for the
> test driver however, which caused virt-manager test suite regressions.
> 
> Working example:
> 
> ```
> $ virsh --connect test:///home/crobinso/src/virt-manager/tests/data/testdriver/testdriver.xml nodedev-dumpxml mdev_8e37ee90_2b51_45e3_9b25_bf8283c03110
> <device>
>    <name>mdev_8e37ee90_2b51_45e3_9b25_bf8283c03110</name>
>    <path>/sys/devices/css0/0.0.0023/8e37ee90-2b51-45e3-9b25-bf8283c03110</path>
>    <parent>css_0_0_0023</parent>
>    <capability type='mdev'>
>      <type id='vfio_ccw-io'/>
>      <iommuGroup number='0'/>
>    </capability>
> </device>
> ```
> 
> Broken example:
> 
> ```
> $ virsh --connect test:///home/crobinso/src/virt-manager/tests/data/testdriver/testdriver.xml nodedev-dumpxml mdev_8e37ee90_2b51_45e3_9b25_bf8283c03110
> <device>
>    <name>mdev_8e37ee90_2b51_45e3_9b25_bf8283c03110</name>
>    <path>/sys/devices/css0/0.0.0023/8e37ee90-2b51-45e3-9b25-bf8283c03110</path>
>    <parent>css_0_0_0023</parent>
>    <capability type='mdev'>
>      <iommuGroup number='0'/>
>    </capability>
> </device>
> ```
> 
> There's already code that does what we want in the test suite.
> Move it to a shared function, and call it in test driver when
> creating a nodedev from driver startup XML.
> 
> Signed-off-by: Cole Robinson <crobinso@redhat.com>
> ---
>   src/conf/node_device_conf.c | 24 ++++++++++++++++++++++++
>   src/conf/node_device_conf.h |  3 +++
>   src/libvirt_private.syms    |  1 +
>   src/test/test_driver.c      |  1 +
>   tests/nodedevxml2xmltest.c  | 18 +++---------------
>   5 files changed, 32 insertions(+), 15 deletions(-)
> 
> diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c
> index 5cfbd6a7eb..fe6d9a36b2 100644
> --- a/src/conf/node_device_conf.c
> +++ b/src/conf/node_device_conf.c
> @@ -2804,6 +2804,30 @@ virNodeDeviceCapsListExport(virNodeDeviceDef *def,
>       return ncaps;
>   }
>   
> +void
> +virNodeDeviceSyncMdevActiveConfig(virNodeDeviceDef *def)
> +{
> +    size_t i;
> +    virNodeDevCapsDef *caps;
> +
> +    for (caps = def->caps; caps; caps = caps->next) {
> +        virNodeDevCapData *data = &caps->data;
> +
> +        if (caps->data.type != VIR_NODE_DEV_CAP_MDEV)
> +            continue;
> +
> +        data->mdev.active_config.type = g_strdup(data->mdev.defined_config.type);
> +        for (i = 0; i < data->mdev.defined_config.nattributes; i++) {
> +            g_autoptr(virMediatedDeviceAttr) attr = g_new0(virMediatedDeviceAttr, 1);
> +
> +            attr->name = g_strdup(data->mdev.defined_config.attributes[i]->name);
> +            attr->value = g_strdup(data->mdev.defined_config.attributes[i]->value);
> +            VIR_APPEND_ELEMENT(data->mdev.active_config.attributes,
> +                               data->mdev.active_config.nattributes,
> +                               attr);
> +        }
> +    }
> +}
>   
>   #ifdef __linux__
>   
> diff --git a/src/conf/node_device_conf.h b/src/conf/node_device_conf.h
> index f0a5333881..4b82636af7 100644
> --- a/src/conf/node_device_conf.h
> +++ b/src/conf/node_device_conf.h
> @@ -470,3 +470,6 @@ virNodeDeviceUpdateCaps(virNodeDeviceDef *def);
>   int
>   virNodeDeviceCapsListExport(virNodeDeviceDef *def,
>                               virNodeDevCapType **list);
> +
> +void
> +virNodeDeviceSyncMdevActiveConfig(virNodeDeviceDef *def);
> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> index 839fe4f545..3186dd6d23 100644
> --- a/src/libvirt_private.syms
> +++ b/src/libvirt_private.syms
> @@ -888,6 +888,7 @@ virNodeDeviceGetPCIDynamicCaps;
>   virNodeDeviceGetSCSIHostCaps;
>   virNodeDeviceGetSCSITargetCaps;
>   virNodeDeviceGetWWNs;
> +virNodeDeviceSyncMdevActiveConfig;
>   virNodeDeviceUpdateCaps;
>   
>   
> diff --git a/src/test/test_driver.c b/src/test/test_driver.c
> index e7d2b6c866..d2d1bc43e3 100644
> --- a/src/test/test_driver.c
> +++ b/src/test/test_driver.c
> @@ -1272,6 +1272,7 @@ testParseNodedevs(testDriver *privconn,
>           virNodeDeviceObjSetPersistent(obj, true);
>           virNodeDeviceObjSetActive(obj, true);
>           virNodeDeviceObjSetSkipUpdateCaps(obj, true);
> +        virNodeDeviceSyncMdevActiveConfig(def);
>           virNodeDeviceObjEndAPI(&obj);
>       }
>   
> diff --git a/tests/nodedevxml2xmltest.c b/tests/nodedevxml2xmltest.c
> index e918922672..814a817725 100644
> --- a/tests/nodedevxml2xmltest.c
> +++ b/tests/nodedevxml2xmltest.c
> @@ -24,7 +24,6 @@ testCompareXMLToXMLFiles(const char *xml, const char *outfile, unsigned int flag
>       int ret = -1;
>       virNodeDeviceDef *dev = NULL;
>       virNodeDevCapsDef *caps;
> -    size_t i;
>   
>       if (virTestLoadFile(xml, &xmlData) < 0)
>           goto fail;
> @@ -52,22 +51,11 @@ testCompareXMLToXMLFiles(const char *xml, const char *outfile, unsigned int flag
>                                              data->storage.logical_block_size;
>               }
>           }
> -
> -        if (caps->data.type == VIR_NODE_DEV_CAP_MDEV &&
> -            !(flags & VIR_NODE_DEVICE_XML_INACTIVE)) {
> -            data->mdev.active_config.type = g_strdup(data->mdev.defined_config.type);
> -            for (i = 0; i < data->mdev.defined_config.nattributes; i++) {
> -                g_autoptr(virMediatedDeviceAttr) attr = g_new0(virMediatedDeviceAttr, 1);
> -
> -                attr->name = g_strdup(data->mdev.defined_config.attributes[i]->name);
> -                attr->value = g_strdup(data->mdev.defined_config.attributes[i]->value);
> -                VIR_APPEND_ELEMENT(data->mdev.active_config.attributes,
> -                                   data->mdev.active_config.nattributes,
> -                                   attr);
> -            }
> -        }
>       }
>   
> +    if (!(flags & VIR_NODE_DEVICE_XML_INACTIVE))
> +        virNodeDeviceSyncMdevActiveConfig(dev);
> +
>       if (!(actual = virNodeDeviceDefFormat(dev, flags)))
>           goto fail;
>   

-- 
Mit freundlichen Grüßen/Kind regards
    Boris Fiuczynski

IBM Deutschland Research & Development GmbH
Vorsitzender des Aufsichtsrats: Wolfgang Wendt
Geschäftsführung: David Faller
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294
_______________________________________________
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-leave@lists.libvirt.org