[libvirt PATCH 3/3] qemu: lookup node device against nodedev driver before getting XML
Michal Prívozník
mprivozn at redhat.com
Mon Mar 16 15:28:48 UTC 2020
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 at 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
More information about the libvir-list
mailing list