From b8a3ed957e643077bbf3b4d8de5ca694903d7f93 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20P=2E=20Berrang=C3=A9?= Date: Tue, 10 Mar 2020 17:03:54 +0000 Subject: [PATCH] qemu: lookup node device against nodedev driver before getting XML MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. Reviewed-by: Michal Privoznik Signed-off-by: Daniel P. Berrangé --- src/libxl/libxl_driver.c | 57 +++++++++++++++++++++++++++++++++++++--- src/qemu/qemu_driver.c | 57 +++++++++++++++++++++++++++++++++++++--- 2 files changed, 108 insertions(+), 6 deletions(-) diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index cc93f1802d..7ec4fcc3d1 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -5786,10 +5786,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)))) + goto cleanup; + + xml = virNodeDeviceGetXMLDesc(nodedev, 0); if (!xml) goto cleanup; @@ -5797,6 +5810,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; @@ -5822,6 +5837,8 @@ libxlNodeDeviceDetachFlags(virNodeDevicePtr dev, cleanup: virPCIDeviceFree(pci); virNodeDeviceDefFree(def); + virObjectUnref(nodedev); + virObjectUnref(nodeconn); VIR_FREE(xml); return ret; } @@ -5842,8 +5859,21 @@ libxlNodeDeviceReAttach(virNodeDevicePtr dev) char *xml = NULL; libxlDriverPrivatePtr driver = dev->conn->privateData; virHostdevManagerPtr hostdev_mgr = driver->hostdevMgr; + virConnectPtr nodeconn = NULL; + virNodeDevicePtr nodedev = NULL; + + if (!(nodeconn = virGetConnectNodeDev())) + goto cleanup; - xml = virNodeDeviceGetXMLDesc(dev, 0); + /* '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)))) + goto cleanup; + + xml = virNodeDeviceGetXMLDesc(nodedev, 0); if (!xml) goto cleanup; @@ -5851,6 +5881,8 @@ libxlNodeDeviceReAttach(virNodeDevicePtr dev) if (!def) goto cleanup; + /* ACL check must happen against original 'dev', + * not the new 'nodedev' we acquired */ if (virNodeDeviceReAttachEnsureACL(dev->conn, def) < 0) goto cleanup; @@ -5869,6 +5901,8 @@ libxlNodeDeviceReAttach(virNodeDevicePtr dev) cleanup: virPCIDeviceFree(pci); virNodeDeviceDefFree(def); + virObjectUnref(nodedev); + virObjectUnref(nodeconn); VIR_FREE(xml); return ret; } @@ -5883,8 +5917,21 @@ libxlNodeDeviceReset(virNodeDevicePtr dev) char *xml = NULL; libxlDriverPrivatePtr driver = dev->conn->privateData; virHostdevManagerPtr hostdev_mgr = driver->hostdevMgr; + virConnectPtr nodeconn = NULL; + virNodeDevicePtr nodedev = NULL; + + 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)))) + goto cleanup; - xml = virNodeDeviceGetXMLDesc(dev, 0); + xml = virNodeDeviceGetXMLDesc(nodedev, 0); if (!xml) goto cleanup; @@ -5892,6 +5939,8 @@ libxlNodeDeviceReset(virNodeDevicePtr dev) if (!def) goto cleanup; + /* ACL check must happen against original 'dev', + * not the new 'nodedev' we acquired */ if (virNodeDeviceResetEnsureACL(dev->conn, def) < 0) goto cleanup; @@ -5910,6 +5959,8 @@ libxlNodeDeviceReset(virNodeDevicePtr dev) cleanup: virPCIDeviceFree(pci); virNodeDeviceDefFree(def); + virObjectUnref(nodedev); + virObjectUnref(nodeconn); VIR_FREE(xml); return ret; } diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 01929c2816..b62947fb4a 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -12965,10 +12965,23 @@ qemuNodeDeviceDetachFlags(virNodeDevicePtr dev, g_autofree char *xml = NULL; bool vfio = qemuHostdevHostSupportsPassthroughVFIO(); 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)))) + goto cleanup; + + xml = virNodeDeviceGetXMLDesc(nodedev, 0); if (!xml) goto cleanup; @@ -12976,6 +12989,8 @@ qemuNodeDeviceDetachFlags(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; @@ -13012,6 +13027,8 @@ qemuNodeDeviceDetachFlags(virNodeDevicePtr dev, cleanup: virPCIDeviceFree(pci); virNodeDeviceDefFree(def); + virObjectUnref(nodedev); + virObjectUnref(nodeconn); return ret; } @@ -13031,8 +13048,21 @@ qemuNodeDeviceReAttach(virNodeDevicePtr dev) virNodeDeviceDefPtr def = NULL; g_autofree char *xml = NULL; virHostdevManagerPtr hostdev_mgr = driver->hostdevMgr; + virConnectPtr nodeconn = NULL; + virNodeDevicePtr nodedev = NULL; + + if (!(nodeconn = virGetConnectNodeDev())) + goto cleanup; - xml = virNodeDeviceGetXMLDesc(dev, 0); + /* '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)))) + goto cleanup; + + xml = virNodeDeviceGetXMLDesc(nodedev, 0); if (!xml) goto cleanup; @@ -13040,6 +13070,8 @@ qemuNodeDeviceReAttach(virNodeDevicePtr dev) if (!def) goto cleanup; + /* ACL check must happen against original 'dev', + * not the new 'nodedev' we acquired */ if (virNodeDeviceReAttachEnsureACL(dev->conn, def) < 0) goto cleanup; @@ -13055,6 +13087,8 @@ qemuNodeDeviceReAttach(virNodeDevicePtr dev) virPCIDeviceFree(pci); cleanup: virNodeDeviceDefFree(def); + virObjectUnref(nodedev); + virObjectUnref(nodeconn); return ret; } @@ -13068,8 +13102,21 @@ qemuNodeDeviceReset(virNodeDevicePtr dev) virNodeDeviceDefPtr def = NULL; g_autofree char *xml = NULL; virHostdevManagerPtr hostdev_mgr = driver->hostdevMgr; + virConnectPtr nodeconn = NULL; + virNodeDevicePtr nodedev = NULL; + + 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)))) + goto cleanup; - xml = virNodeDeviceGetXMLDesc(dev, 0); + xml = virNodeDeviceGetXMLDesc(nodedev, 0); if (!xml) goto cleanup; @@ -13077,6 +13124,8 @@ qemuNodeDeviceReset(virNodeDevicePtr dev) if (!def) goto cleanup; + /* ACL check must happen against original 'dev', + * not the new 'nodedev' we acquired */ if (virNodeDeviceResetEnsureACL(dev->conn, def) < 0) goto cleanup; @@ -13092,6 +13141,8 @@ qemuNodeDeviceReset(virNodeDevicePtr dev) virPCIDeviceFree(pci); cleanup: virNodeDeviceDefFree(def); + virObjectUnref(nodedev); + virObjectUnref(nodeconn); return ret; } -- GitLab