From 887dd0d331e43dc124a858514ab246ecbc18518e Mon Sep 17 00:00:00 2001 From: Daniel Henrique Barboza Date: Sat, 30 Jan 2021 14:23:41 -0300 Subject: [PATCH] qemu, libxl, hypervisor: use virDomainDriverNodeDeviceDetachFlags() helper MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. The helper is named with the 'Flags' suffix, even when the helper itself isn't receiving the flags from the callers, to be compliant with the ACL function virNodeDeviceDetachFlagsEnsureACL() that is being called inside it and was called from the original functions. Renaming the helper would implicate in renaming REMOTE_PROC_NODE_DEVICE_DETACH_FLAGS, and all the related structs inside remote_protocol.x, to be compliant with the ACL rules. This is not being checked at this moment, but we'll fix check-aclrules.py to verify all the helpers that calls ACL functions in domain_driver.c shortly. Reviewed-by: Ján Tomko Signed-off-by: Daniel Henrique Barboza --- src/hypervisor/domain_driver.c | 60 ++++++++++++++++++++++++++++++++++ src/hypervisor/domain_driver.h | 4 +++ src/libvirt_private.syms | 1 + src/libxl/libxl_driver.c | 53 ++---------------------------- src/qemu/qemu_driver.c | 49 ++------------------------- 5 files changed, 71 insertions(+), 96 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) +{ + 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; + + 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; +} diff --git a/src/hypervisor/domain_driver.h b/src/hypervisor/domain_driver.h index 71eed6d5a9..a22a3ee76c 100644 --- a/src/hypervisor/domain_driver.h +++ b/src/hypervisor/domain_driver.h @@ -56,3 +56,7 @@ int virDomainDriverNodeDeviceReset(virNodeDevicePtr dev, int virDomainDriverNodeDeviceReAttach(virNodeDevicePtr dev, virHostdevManagerPtr hostdevMgr); + +int virDomainDriverNodeDeviceDetachFlags(virNodeDevicePtr dev, + virHostdevManagerPtr hostdevMgr, + const char *driverName); diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index cf477c752c..6b7261b987 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1507,6 +1507,7 @@ virDomainCgroupSetupMemtune; virDomainDriverGenerateMachineName; virDomainDriverGenerateRootHash; virDomainDriverMergeBlkioDevice; +virDomainDriverNodeDeviceDetachFlags; virDomainDriverNodeDeviceGetPCIInfo; virDomainDriverNodeDeviceReAttach; virDomainDriverNodeDeviceReset; diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index b44715eee1..75a8d46af0 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -5772,15 +5772,8 @@ libxlNodeDeviceDetachFlags(virNodeDevicePtr dev, const char *driverName, unsigned int flags) { - virPCIDevicePtr pci = NULL; - virPCIDeviceAddress devAddr; - int ret = -1; - virNodeDeviceDefPtr def = NULL; - g_autofree char *xml = NULL; libxlDriverPrivatePtr driver = dev->conn->privateData; virHostdevManagerPtr hostdev_mgr = driver->hostdevMgr; - virConnectPtr nodeconn = NULL; - virNodeDevicePtr nodedev = NULL; virCheckFlags(0, -1); @@ -5790,49 +5783,9 @@ libxlNodeDeviceDetachFlags(virNodeDevicePtr dev, return -1; } - 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; - - 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; - - if (virDomainDriverNodeDeviceGetPCIInfo(def, &devAddr) < 0) - goto cleanup; - - pci = virPCIDeviceNew(&devAddr); - if (!pci) - goto cleanup; - - virPCIDeviceSetStubDriver(pci, VIR_PCI_STUB_DRIVER_XEN); - - if (virHostdevPCINodeDeviceDetach(hostdev_mgr, pci) < 0) - goto cleanup; - - ret = 0; - cleanup: - virPCIDeviceFree(pci); - virNodeDeviceDefFree(def); - virObjectUnref(nodedev); - virObjectUnref(nodeconn); - return ret; + /* virNodeDeviceDetachFlagsEnsureACL() is being called by + * virDomainDriverNodeDeviceDetachFlags() */ + return virDomainDriverNodeDeviceDetachFlags(dev, hostdev_mgr, driverName); } static int diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 888e2ec774..f1035a536e 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -11998,15 +11998,8 @@ qemuNodeDeviceDetachFlags(virNodeDevicePtr dev, unsigned int flags) { virQEMUDriverPtr driver = dev->conn->privateData; - virPCIDevicePtr pci = NULL; - virPCIDeviceAddress devAddr; - int ret = -1; - virNodeDeviceDefPtr def = NULL; - g_autofree char *xml = NULL; bool vfio = qemuHostdevHostSupportsPassthroughVFIO(); virHostdevManagerPtr hostdev_mgr = driver->hostdevMgr; - virConnectPtr nodeconn = NULL; - virNodeDevicePtr nodedev = NULL; virCheckFlags(0, -1); @@ -12029,46 +12022,10 @@ qemuNodeDeviceDetachFlags(virNodeDevicePtr dev, return -1; } - 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; - - 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; - - if (virDomainDriverNodeDeviceGetPCIInfo(def, &devAddr) < 0) - goto cleanup; - - pci = virPCIDeviceNew(&devAddr); - if (!pci) - goto cleanup; - - virPCIDeviceSetStubDriver(pci, VIR_PCI_STUB_DRIVER_VFIO); - - ret = virHostdevPCINodeDeviceDetach(hostdev_mgr, pci); - cleanup: - virPCIDeviceFree(pci); - virNodeDeviceDefFree(def); - virObjectUnref(nodedev); - virObjectUnref(nodeconn); - return ret; + /* virNodeDeviceDetachFlagsEnsureACL() is being called by + * virDomainDriverNodeDeviceDetachFlags() */ + return virDomainDriverNodeDeviceDetachFlags(dev, hostdev_mgr, driverName); } static int