[libvirt] [PATCH 4/7] qemuSetupHostdevCgroup: Use qemuDomainGetHostdevPath
Marc-André Lureau
marcandre.lureau at gmail.com
Thu Feb 16 12:20:37 UTC 2017
Hi
On Fri, Feb 10, 2017 at 6:57 PM Michal Privoznik <mprivozn at redhat.com>
wrote:
> Since these two functions are nearly identical (with
> qemuSetupHostdevCgroup actually calling virCgroupAllowDevicePath)
> we can have one function call the other and thus de-duplicate
> some code.
>
> Signed-off-by: Michal Privoznik <mprivozn at redhat.com>
>
Reviewed-by: Marc-André Lureau <marcandre.lureau at redhat.com>
---
> src/qemu/qemu_cgroup.c | 147
> +++++--------------------------------------------
> src/qemu/qemu_domain.c | 31 +++++++++--
> src/qemu/qemu_domain.h | 4 ++
> 3 files changed, 43 insertions(+), 139 deletions(-)
>
> diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c
> index 89854b5bd..19832c209 100644
> --- a/src/qemu/qemu_cgroup.c
> +++ b/src/qemu/qemu_cgroup.c
> @@ -264,147 +264,26 @@ int
> qemuSetupHostdevCgroup(virDomainObjPtr vm,
> virDomainHostdevDefPtr dev)
> {
> - int ret = -1;
> qemuDomainObjPrivatePtr priv = vm->privateData;
> - virDomainHostdevSubsysUSBPtr usbsrc = &dev->source.subsys.u.usb;
> - virDomainHostdevSubsysPCIPtr pcisrc = &dev->source.subsys.u.pci;
> - virDomainHostdevSubsysSCSIPtr scsisrc = &dev->source.subsys.u.scsi;
> - virDomainHostdevSubsysSCSIVHostPtr hostsrc =
> &dev->source.subsys.u.scsi_host;
> - virPCIDevicePtr pci = NULL;
> - virUSBDevicePtr usb = NULL;
> - virSCSIDevicePtr scsi = NULL;
> - virSCSIVHostDevicePtr host = NULL;
> char *path = NULL;
> - int rv;
> + int perms;
> + int ret = -1;
>
> - /* currently this only does something for PCI devices using vfio
> - * for device assignment, but it is called for *all* hostdev
> - * devices.
> - */
> + if (qemuDomainGetHostdevPath(dev, &path, &perms) < 0)
> + goto cleanup;
>
> - if (!virCgroupHasController(priv->cgroup,
> VIR_CGROUP_CONTROLLER_DEVICES))
> - return 0;
> -
> - if (dev->mode == VIR_DOMAIN_HOSTDEV_MODE_SUBSYS) {
> -
> - switch ((virDomainHostdevSubsysType) dev->source.subsys.type) {
> - case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI:
> - if (pcisrc->backend == VIR_DOMAIN_HOSTDEV_PCI_BACKEND_VFIO) {
> - pci = virPCIDeviceNew(pcisrc->addr.domain,
> - pcisrc->addr.bus,
> - pcisrc->addr.slot,
> - pcisrc->addr.function);
> - if (!pci)
> - goto cleanup;
> -
> - if (!(path = virPCIDeviceGetIOMMUGroupDev(pci)))
> - goto cleanup;
> -
> - VIR_DEBUG("Cgroup allow %s for PCI device assignment",
> path);
> - rv = virCgroupAllowDevicePath(priv->cgroup, path,
> - VIR_CGROUP_DEVICE_RW,
> false);
> - virDomainAuditCgroupPath(vm, priv->cgroup,
> - "allow", path, "rw", rv == 0);
> - if (rv < 0)
> - goto cleanup;
> - }
> - break;
> -
> - case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB:
> - /* NB: hostdev->missing wasn't previously checked in the
> - * case of hotplug, only when starting a domain. Now it is
> - * always checked, and the cgroup setup skipped if true.
> - */
> - if (dev->missing)
> - break;
> - if ((usb = virUSBDeviceNew(usbsrc->bus, usbsrc->device,
> - NULL)) == NULL) {
> - goto cleanup;
> - }
> -
> - if (VIR_STRDUP(path, virUSBDeviceGetPath(usb)) < 0)
> - goto cleanup;
> -
> - VIR_DEBUG("Process path '%s' for USB device", path);
> - rv = virCgroupAllowDevicePath(priv->cgroup, path,
> - VIR_CGROUP_DEVICE_RW, false);
> - virDomainAuditCgroupPath(vm, priv->cgroup, "allow", path,
> "rw", rv == 0);
> - if (rv < 0)
> - goto cleanup;
> - break;
> -
> - case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI: {
> - if (scsisrc->protocol ==
> - VIR_DOMAIN_HOSTDEV_SCSI_PROTOCOL_TYPE_ISCSI) {
> - virDomainHostdevSubsysSCSIiSCSIPtr iscsisrc =
> &scsisrc->u.iscsi;
> - /* Follow qemuSetupDiskCgroup() and
> qemuSetImageCgroupInternal()
> - * which does nothing for non local storage
> - */
> - VIR_DEBUG("Not updating cgroups for hostdev iSCSI path
> '%s'",
> - iscsisrc->path);
> - } else {
> - virDomainHostdevSubsysSCSIHostPtr scsihostsrc =
> - &scsisrc->u.host;
> - if ((scsi = virSCSIDeviceNew(NULL,
> - scsihostsrc->adapter,
> - scsihostsrc->bus,
> - scsihostsrc->target,
> - scsihostsrc->unit,
> - dev->readonly,
> - dev->shareable)) == NULL)
> - goto cleanup;
> -
> - if (VIR_STRDUP(path, virSCSIDeviceGetPath(scsi)) < 0)
> - goto cleanup;
> -
> - VIR_DEBUG("Process path '%s' for SCSI device", path);
> - rv = virCgroupAllowDevicePath(priv->cgroup, path,
> -
> virSCSIDeviceGetReadonly(scsi) ?
> - VIR_CGROUP_DEVICE_READ :
> - VIR_CGROUP_DEVICE_RW,
> false);
> -
> - virDomainAuditCgroupPath(vm, priv->cgroup, "allow", path,
> - virSCSIDeviceGetReadonly(scsi) ?
> "r" : "rw",
> - rv == 0);
> - if (rv < 0)
> - goto cleanup;
> - }
> - break;
> - }
> -
> - case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI_HOST: {
> - if (hostsrc->protocol ==
> - VIR_DOMAIN_HOSTDEV_SUBSYS_SCSI_HOST_PROTOCOL_TYPE_VHOST) {
> - if (!(host = virSCSIVHostDeviceNew(hostsrc->wwpn)))
> - goto cleanup;
> -
> - if (VIR_STRDUP(path, virSCSIVHostDeviceGetPath(host)) < 0)
> - goto cleanup;
> -
> - VIR_DEBUG("Process path '%s' for scsi_host device", path);
> -
> - rv = virCgroupAllowDevicePath(priv->cgroup, path,
> - VIR_CGROUP_DEVICE_RW,
> false);
> -
> - virDomainAuditCgroupPath(vm, priv->cgroup,
> - "allow", path, "rw", rv == 0);
> - if (rv < 0)
> - goto cleanup;
> - }
> - break;
> - }
> -
> - case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_LAST:
> - break;
> - }
> + if (!path) {
> + /* There's no path that we need to allow. Claim success. */
> + ret = 0;
> + goto cleanup;
> }
>
> - ret = 0;
> + VIR_DEBUG("Cgroup allow %s perms=%d", path, perms);
> + ret = virCgroupAllowDevicePath(priv->cgroup, path, perms, false);
> + virDomainAuditCgroupPath(vm, priv->cgroup, "allow", path,
> + virCgroupGetDevicePermsString(perms), ret ==
> 0);
> +
> cleanup:
> - virPCIDeviceFree(pci);
> - virUSBDeviceFree(usb);
> - virSCSIDeviceFree(scsi);
> - virSCSIVHostDeviceFree(host);
> VIR_FREE(path);
> return ret;
> }
> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
> index 7c696963e..c6d32525f 100644
> --- a/src/qemu/qemu_domain.c
> +++ b/src/qemu/qemu_domain.c
> @@ -6831,9 +6831,21 @@ qemuDomainSupportsVideoVga(virDomainVideoDefPtr
> video,
> }
>
>
> -static int
> +/**
> + * qemuDomainGetHostdevPath:
> + * @dev: host device definition
> + * @path: resulting path to @dev
> + * @perms: Optional pointer to VIR_CGROUP_DEVICE_* perms
> + *
> + * For given device @dev fetch its host path and store it at @path.
> Optionally,
> + * caller can get @perms on the path (e.g. rw/ro).
> + *
> + * Returns 0 on success, -1 otherwise.
> + */
> +int
> qemuDomainGetHostdevPath(virDomainHostdevDefPtr dev,
> - char **path)
> + char **path,
> + int *perms)
> {
> int ret = -1;
> virDomainHostdevSubsysUSBPtr usbsrc = &dev->source.subsys.u.usb;
> @@ -6864,6 +6876,8 @@ qemuDomainGetHostdevPath(virDomainHostdevDefPtr dev,
> if (!(tmpPath = virPCIDeviceGetIOMMUGroupDev(pci)))
> goto cleanup;
> freeTmpPath = true;
> + if (perms)
> + *perms = VIR_CGROUP_DEVICE_RW;
> }
> break;
>
> @@ -6878,6 +6892,8 @@ qemuDomainGetHostdevPath(virDomainHostdevDefPtr dev,
>
> if (!(tmpPath = (char *) virUSBDeviceGetPath(usb)))
> goto cleanup;
> + if (perms)
> + *perms = VIR_CGROUP_DEVICE_RW;
> break;
>
> case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI:
> @@ -6902,6 +6918,9 @@ qemuDomainGetHostdevPath(virDomainHostdevDefPtr dev,
>
> if (!(tmpPath = (char *) virSCSIDeviceGetPath(scsi)))
> goto cleanup;
> + if (perms)
> + *perms = virSCSIDeviceGetReadonly(scsi) ?
> + VIR_CGROUP_DEVICE_READ :VIR_CGROUP_DEVICE_RW;
>
missing a space after :
> }
> break;
>
> @@ -6913,6 +6932,8 @@ qemuDomainGetHostdevPath(virDomainHostdevDefPtr dev,
>
> if (!(tmpPath = (char *) virSCSIVHostDeviceGetPath(host)))
> goto cleanup;
> + if (perms)
> + *perms = VIR_CGROUP_DEVICE_RW;
> }
> break;
> }
> @@ -7328,7 +7349,7 @@ qemuDomainSetupHostdev(virQEMUDriverPtr driver
> ATTRIBUTE_UNUSED,
> int ret = -1;
> char *path = NULL;
>
> - if (qemuDomainGetHostdevPath(dev, &path) < 0)
> + if (qemuDomainGetHostdevPath(dev, &path, NULL) < 0)
> goto cleanup;
>
> if (!path) {
> @@ -7964,7 +7985,7 @@ qemuDomainNamespaceSetupHostdev(virQEMUDriverPtr
> driver,
> if (!qemuDomainNamespaceEnabled(vm, QEMU_DOMAIN_NS_MOUNT))
> return 0;
>
> - if (qemuDomainGetHostdevPath(hostdev, &path) < 0)
> + if (qemuDomainGetHostdevPath(hostdev, &path, NULL) < 0)
> goto cleanup;
>
> if (!path) {
> @@ -7995,7 +8016,7 @@ qemuDomainNamespaceTeardownHostdev(virQEMUDriverPtr
> driver,
> if (!qemuDomainNamespaceEnabled(vm, QEMU_DOMAIN_NS_MOUNT))
> return 0;
>
> - if (qemuDomainGetHostdevPath(hostdev, &path) < 0)
> + if (qemuDomainGetHostdevPath(hostdev, &path, NULL) < 0)
> goto cleanup;
>
> if (!path) {
> diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
> index 5cfa3e114..f81550e2f 100644
> --- a/src/qemu/qemu_domain.h
> +++ b/src/qemu/qemu_domain.h
> @@ -802,6 +802,10 @@ int qemuDomainCheckMonitor(virQEMUDriverPtr driver,
> bool qemuDomainSupportsVideoVga(virDomainVideoDefPtr video,
> virQEMUCapsPtr qemuCaps);
>
> +int qemuDomainGetHostdevPath(virDomainHostdevDefPtr dev,
> + char **path,
> + int *perms);
> +
> int qemuDomainBuildNamespace(virQEMUDriverPtr driver,
> virDomainObjPtr vm);
>
> --
>
otherwise,
Reviewed-by: Marc-André Lureau <marcandre.lureau at redhat.com>
2.11.0
>
> --
> libvir-list mailing list
> libvir-list at redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list
>
--
Marc-André Lureau
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20170216/c301b82c/attachment-0001.htm>
More information about the libvir-list
mailing list