On a Monday in 2021, Daniel Henrique Barboza wrote:
libxlNodeDeviceDetachFlags() and qemuNodeDeviceDetachFlags() are mostly
equal, aside from how the virHostdevmanager pointer is retrieved and
the PCI stub driver used.
Now that the PCI stub driver verification is done early in both functions,
we can use the virDomainDriverNodeDeviceDetachFlags() helper to reduce
code duplication between them. 'driverName' is checked inside the helper
to set the appropriate stub driver.
Signed-off-by: Daniel Henrique Barboza <danielhb413@xxxxxxxxx>
---
src/hypervisor/domain_driver.c | 60 ++++++++++++++++++++++++++++++++++
src/hypervisor/domain_driver.h | 4 +++
src/libvirt_private.syms | 1 +
src/libxl/libxl_driver.c | 54 ++----------------------------
src/qemu/qemu_driver.c | 49 ++-------------------------
5 files changed, 71 insertions(+), 97 deletions(-)
diff --git a/src/hypervisor/domain_driver.c b/src/hypervisor/domain_driver.c
index ea4c3c9466..6ee74d6dff 100644
--- a/src/hypervisor/domain_driver.c
+++ b/src/hypervisor/domain_driver.c
@@ -459,3 +459,63 @@ virDomainDriverNodeDeviceReAttach(virNodeDevicePtr dev,
return virHostdevPCINodeDeviceReAttach(hostdevMgr, pci);
}
+
+int
+virDomainDriverNodeDeviceDetachFlags(virNodeDevicePtr dev,
+ virHostdevManagerPtr hostdevMgr,
+ const char *driverName)
This helper does not even take flags, so the only reason to keep the 'Flags' in its name is to be consistent with the driver method it implements...
+{
+ virPCIDevicePtr pci = NULL;
+ virPCIDeviceAddress devAddr;
+ int ret = -1;
+ virNodeDeviceDefPtr def = NULL;
+ g_autofree char *xml = NULL;
+ virConnectPtr nodeconn = NULL;
+ virNodeDevicePtr nodedev = NULL;
+
+ if (!driverName)
+ return -1;
+
+ if (!(nodeconn = virGetConnectNodeDev()))
+ goto cleanup;
+
+ /* 'dev' is associated with 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;
+
+ def = virNodeDeviceDefParseString(xml, EXISTING_DEVICE, NULL);
+ 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;
+
... and the ACL check required. But moving the ACL checks into src/hypervisor means they are no longer covered by the check-aclrules check. I'd rather keep the ACL checks repetitive in each driver than risk the chance of missing them, but that invalidates most of these refactors. Any ideas? Jano
+ if (virDomainDriverNodeDeviceGetPCIInfo(def, &devAddr) < 0) + goto cleanup; + + pci = virPCIDeviceNew(&devAddr); + if (!pci) + goto cleanup; + + if (STREQ(driverName, "vfio")) + virPCIDeviceSetStubDriver(pci, VIR_PCI_STUB_DRIVER_VFIO); + else if (STREQ(driverName, "xen")) + virPCIDeviceSetStubDriver(pci, VIR_PCI_STUB_DRIVER_XEN); + + ret = virHostdevPCINodeDeviceDetach(hostdevMgr, pci); + cleanup: + virPCIDeviceFree(pci); + virNodeDeviceDefFree(def); + virObjectUnref(nodedev); + virObjectUnref(nodeconn); + return ret; +}
Attachment:
signature.asc
Description: PGP signature
