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(a)redhat.com>
---
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