On 10. 3. 2020 19:13, Daniel P. Berrangé wrote: > Some of the node device APIs are a little odd because they accept a > virNodeDevicePtr object but are still implemented by the virt drivers. > The first thing the virt drivers need to do is get the XML config > associated with the node device, and that means talking to the node > device driver. > > This worked previously because with monolithic libvirtd, both the > virt driver and node device driver were in the same daemon and thus > a single virConnectPtr can talk to both drivers. > > With the split daemon world though, the virNodeDevicePtr passed into > the APIs is associated with the QEMU driver virConnectPtr, which has > no ability to invoke APIs against the node device driver. We must thus > get a duplicate virNodeDevicePtr object which is associated with a > virConnectPtr for the node device driver. > > Signed-off-by: Daniel P. Berrangé <berrange@xxxxxxxxxx> > --- > src/libxl/libxl_driver.c | 63 ++++++++++++++++++++++++++++++++++++++-- > src/qemu/qemu_driver.c | 63 ++++++++++++++++++++++++++++++++++++++-- > 2 files changed, 120 insertions(+), 6 deletions(-) > > diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c > index f2387e2a20..eca45da097 100644 > --- a/src/libxl/libxl_driver.c > +++ b/src/libxl/libxl_driver.c > @@ -5787,10 +5787,23 @@ libxlNodeDeviceDetachFlags(virNodeDevicePtr dev, > char *xml = NULL; > libxlDriverPrivatePtr driver = dev->conn->privateData; > virHostdevManagerPtr hostdev_mgr = driver->hostdevMgr; > + virConnectPtr nodeconn = NULL; > + virNodeDevicePtr nodedev = NULL; > > virCheckFlags(0, -1); > > - xml = virNodeDeviceGetXMLDesc(dev, 0); > + if (!(nodeconn = virGetConnectNodeDev())) > + goto cleanup; > + > + /* 'dev' is associated with the QEMU virConnectPtr, > + * so for split daemons, we need to get a copy that > + * is associated with the virnodedevd daemon. > + */ > + if (!(nodedev = virNodeDeviceLookupByName( > + nodeconn, virNodeDeviceGetName(dev)))) No need to split this line IMO. 85 characters long line is not that bad. > + goto cleanup; > + > + xml = virNodeDeviceGetXMLDesc(nodedev, 0); > if (!xml) > goto cleanup; > > @@ -5798,6 +5811,8 @@ libxlNodeDeviceDetachFlags(virNodeDevicePtr dev, > if (!def) > goto cleanup; > > + /* ACL check must happen against original 'dev', > + * not the new 'nodedev' we acquired */ > if (virNodeDeviceDetachFlagsEnsureACL(dev->conn, def) < 0) > goto cleanup; > > @@ -5823,6 +5838,10 @@ libxlNodeDeviceDetachFlags(virNodeDevicePtr dev, > cleanup: > virPCIDeviceFree(pci); > virNodeDeviceDefFree(def); > + if (nodedev) > + virNodeDeviceFree(nodedev); s/virNodeDeviceFree/virObjectUnref/ as we don't really need to call the public API. > + if (nodeconn) > + virConnectClose(nodeconn); And seems like we are preferring virObjectUnref() over virConnectClose() too. > VIR_FREE(xml); > return ret; > } Michal
