Hi
Reviewed-by: Marc-André Lureau <marcandre.lureau@xxxxxxxxxx>
otherwise,
Reviewed-by: Marc-André Lureau <marcandre.lureau@xxxxxxxxxx>
On Fri, Feb 10, 2017 at 6:57 PM Michal Privoznik <mprivozn@xxxxxxxxxx> 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@xxxxxxxxxx>
Reviewed-by: Marc-André Lureau <marcandre.lureau@xxxxxxxxxx>
---
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@xxxxxxxxxx>
2.11.0
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
--
Marc-André Lureau
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
