Re: [PATCH v4 3/3] conf: nodedev: Fill active_config at XML parse time

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 4/17/24 17:17, 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.

I still think that the mocking of state changes should be handled inside of the test driver itself of the virNodeDeviceDriver in the implementation the interfaces: nodeDeviceCreateXML => creates a transient mdev from the XML (no persistent config) nodeDeviceDestroy => removes the active mdev (a transient mdev is completely removed)
nodeDeviceDefineXML  => creates a persistent mdev config from the XML
nodeDeviceUndefine => removes the persistent mdev config (if mdev is active the active config remains) nodeDeviceCreate => creates the active config from the persistent config

Therefore for mocking
* copy defined_config to active_config
* reset defined_config
* reset active_config
should be sufficient.

Since there are only nodeDeviceCreateXML and nodeDeviceDestroy implemented in the test driver the first two should do the trick.



Example before:

```
$ 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>
```

Example after:

```
$ 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>
```

Simplest solution is to fill in `active_config` at XML define time
as well. The real node_device driver already takes care to free any
`active_config` when it live updates this info, so we are safe there.

This also lets us drop the test suite logic to duplicate this data.

Signed-off-by: Cole Robinson <crobinso@xxxxxxxxxx>
---
  src/conf/node_device_conf.c |  5 ++++-
  tests/nodedevxml2xmltest.c  | 15 ---------------
  2 files changed, 4 insertions(+), 16 deletions(-)

diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c
index 5cfbd6a7eb..f381ea128c 100644
--- a/src/conf/node_device_conf.c
+++ b/src/conf/node_device_conf.c
@@ -2222,6 +2222,7 @@ virNodeDevCapMdevParseXML(xmlXPathContextPtr ctxt,
                         _("missing type id attribute for '%1$s'"), def->name);
          return -1;
      }
+    mdev->active_config.type = g_strdup(mdev->defined_config.type);
if ((uuidstr = virXPathString("string(./uuid[1])", ctxt))) {
          unsigned char uuidbuf[VIR_UUID_BUFLEN];
@@ -2248,8 +2249,10 @@ virNodeDevCapMdevParseXML(xmlXPathContextPtr ctxt,
      if ((nattrs = virXPathNodeSet("./attr", ctxt, &attrs)) < 0)
          return -1;
- for (i = 0; i < nattrs; i++)
+    for (i = 0; i < nattrs; i++) {
          virNodeDevCapMdevAttributeParseXML(ctxt, attrs[i], &mdev->defined_config);
+        virNodeDevCapMdevAttributeParseXML(ctxt, attrs[i], &mdev->active_config);
+    }
return 0;
  }
diff --git a/tests/nodedevxml2xmltest.c b/tests/nodedevxml2xmltest.c
index e918922672..d2663a8d68 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,20 +51,6 @@ 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 (!(actual = virNodeDeviceDefFormat(dev, flags)))

--
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@xxxxxxxxxxxxxxxxx
To unsubscribe send an email to devel-leave@xxxxxxxxxxxxxxxxx




[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux