On 4/9/24 11:20 AM, Boris Fiuczynski wrote: > On 4/9/24 16:56, 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. >> >> 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. > > I do not think that it is a good idea to hack the general code which > creates in the real environments fake data. > I can't tell how this affects real environments. Is there a place in the udev driver where XML is parsed, and then active_config isn't explicitly updated? If not, do you object to me pushing this with Michal's ACK? This is causing some pain with virt-manager dev workflow > If your tests require active mdevs than the mocking code should set > these active and also do the config copy. > Obviously this can work but IMO it sets a bad precedent. If this active/inactive_config structure is extended in the future, the test driver syncing needs to be extended to match, or we need more plumbing to share more of this Personally I would have rather seen this implemented using ->newDef pattern used by domain, network, storage. That's the closest thing to an internal standard for handling differences between inactive config and runtime config. ->def is copied to ->newDef at object startup time, and the drivers change newDef as needed to reflect runtime state of the object. Thanks, Cole _______________________________________________ Devel mailing list -- devel@xxxxxxxxxxxxxxxxx To unsubscribe send an email to devel-leave@xxxxxxxxxxxxxxxxx
