On 4/17/24 10:04 AM, Cole Robinson wrote: > On 4/9/24 11:19 AM, Boris Fiuczynski wrote: >> On 4/9/24 16:56, Cole Robinson wrote: >>> This was the implied default before nodedevs gained a notion of >>> being inactive, and matches how we handle parsing other objects >>> >>> Signed-off-by: Cole Robinson <crobinso@xxxxxxxxxx> >>> --- >>> src/test/test_driver.c | 1 + >>> 1 file changed, 1 insertion(+) >>> >>> diff --git a/src/test/test_driver.c b/src/test/test_driver.c >>> index 41828f86b6..9db7a44035 100644 >>> --- a/src/test/test_driver.c >>> +++ b/src/test/test_driver.c >>> @@ -1269,6 +1269,7 @@ testParseNodedevs(testDriver *privconn, >>> return -1; >>> } >>> + virNodeDeviceObjSetActive(obj, true); >> >> This will actually render the mdev object to be transient which is an >> active mdev not having a persistent definition. >> The data using virNodeDeviceDefParseXML is stored in the mdevs >> defined_config only therefore the data is showing up when you use "virsh >> nodedev-dumpxml" as it defaults to the "current state" of the nodedev. >> >> For data consistency of the node you should instead do >> virNodeDeviceObjSetPersistent(obj, true); >> Good catch that this does not make the devices persistent, I missed that detail. Ok so there's a bit more context needed here. For test driver XML parsing, all other objects are considered persistent by default. Transient has to be opted in via testdriver specific XML namespace. So yeah makes sense for consistency to make object persistent as well. But same logic applies for making the object 'active' by default. test driver assumes any read in domain/interface/network/storage object is default active and persistent. Makes sense to do the same for nodedev IMO. Notable we need this so `virsh nodedev-list` works in the way it historically did, before nodedev devices could be inactive. >> Now the mdev is inactive and persistent and the virsh command should be >> correct. >> virt-manager/virt-install isn't using virsh, we are using direct API via python bindings. We are calling virNodeDeviceGetXMLDesc with flags=0, the only historically supported value. But even still, this doesn't fix `virsh nodedev-dumpxml $mdev` for test driver, because GetXMLDesc test driver implementation doesn't force INACTIVE XML flag when the object is inactive, like the real node_device_driver.c does. So flags=0 tries to dump 'active' XML of an inactive object. So that's another bit to fix. But still IMO that is not sufficient, since nodedevs parsed via test driver input XML should be considered active by default. I will push patch 1-3 which you and michal reviewed. I will send another version of this series with the 'persistent' bit fixed, and an additional patch to fix nodedev GetXML semantics to match node_device driver. But that still leaves something like patch 5 as a requirement. Thanks, Cole _______________________________________________ Devel mailing list -- devel@xxxxxxxxxxxxxxxxx To unsubscribe send an email to devel-leave@xxxxxxxxxxxxxxxxx
