From 81df21507bef94ae53a056156e4aa6661f29237a Mon Sep 17 00:00:00 2001 From: Michal Privoznik Date: Tue, 15 Nov 2016 16:53:04 +0100 Subject: [PATCH] qemu: Manage /dev entry on disk hotplug When attaching a device to a domain that's using separate mount namespace we must maintain /dev entries in order for qemu process to see them. Signed-off-by: Michal Privoznik --- src/qemu/qemu_domain.c | 214 +++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_domain.h | 8 ++ src/qemu/qemu_driver.c | 5 +- src/qemu/qemu_hotplug.c | 22 ++-- src/qemu/qemu_security.c | 32 ++++++ src/qemu/qemu_security.h | 8 ++ 6 files changed, 280 insertions(+), 9 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 49e8534ca0..27f6e7cf24 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -53,6 +53,11 @@ #include "storage/storage_driver.h" +#ifdef MAJOR_IN_MKDEV +# include +#elif MAJOR_IN_SYSMACROS +# include +#endif #include #include #if defined(HAVE_SYS_MOUNT_H) @@ -7460,3 +7465,212 @@ qemuDomainDeleteNamespace(virQEMUDriverPtr driver, virStringListFreeCount(devMountsPath, ndevMountsPath); VIR_FREE(devPath); } + + +struct qemuDomainAttachDeviceMknodData { + virQEMUDriverPtr driver; + virDomainObjPtr vm; + virDomainDeviceDefPtr devDef; + const char *file; + struct stat sb; + void *acl; +}; + + +static int +qemuDomainAttachDeviceMknodHelper(pid_t pid ATTRIBUTE_UNUSED, + void *opaque) +{ + struct qemuDomainAttachDeviceMknodData *data = opaque; + int ret = -1; + + virSecurityManagerPostFork(data->driver->securityManager); + + if (virFileMakeParentPath(data->file) < 0) { + virReportSystemError(errno, + _("Unable to create %s"), data->file); + goto cleanup; + } + + VIR_DEBUG("Creating dev %s (%d,%d)", + data->file, major(data->sb.st_rdev), minor(data->sb.st_rdev)); + if (mknod(data->file, data->sb.st_mode, data->sb.st_rdev) < 0) { + /* Because we are not removing devices on hotunplug, or + * we might be creating part of backing chain that + * already exist due to a different disk plugged to + * domain, accept EEXIST. */ + if (errno != EEXIST) { + virReportSystemError(errno, + _("Unable to create device %s"), + data->file); + goto cleanup; + } + } + + if (virFileSetACLs(data->file, data->acl) < 0 && + errno != ENOTSUP) { + virReportSystemError(errno, + _("Unable to set ACLs on %s"), data->file); + goto cleanup; + } + + switch ((virDomainDeviceType) data->devDef->type) { + case VIR_DOMAIN_DEVICE_DISK: { + virDomainDiskDefPtr def = data->devDef->data.disk; + char *tmpsrc = def->src->path; + def->src->path = (char *) data->file; + if (virSecurityManagerSetDiskLabel(data->driver->securityManager, + data->vm->def, def) < 0) { + def->src->path = tmpsrc; + goto cleanup; + } + def->src->path = tmpsrc; + } break; + + case VIR_DOMAIN_DEVICE_NONE: + case VIR_DOMAIN_DEVICE_LEASE: + case VIR_DOMAIN_DEVICE_FS: + case VIR_DOMAIN_DEVICE_NET: + case VIR_DOMAIN_DEVICE_INPUT: + case VIR_DOMAIN_DEVICE_SOUND: + case VIR_DOMAIN_DEVICE_VIDEO: + case VIR_DOMAIN_DEVICE_HOSTDEV: + case VIR_DOMAIN_DEVICE_WATCHDOG: + case VIR_DOMAIN_DEVICE_CONTROLLER: + case VIR_DOMAIN_DEVICE_GRAPHICS: + case VIR_DOMAIN_DEVICE_HUB: + case VIR_DOMAIN_DEVICE_REDIRDEV: + case VIR_DOMAIN_DEVICE_SMARTCARD: + case VIR_DOMAIN_DEVICE_CHR: + case VIR_DOMAIN_DEVICE_MEMBALLOON: + case VIR_DOMAIN_DEVICE_NVRAM: + case VIR_DOMAIN_DEVICE_RNG: + case VIR_DOMAIN_DEVICE_SHMEM: + case VIR_DOMAIN_DEVICE_TPM: + case VIR_DOMAIN_DEVICE_PANIC: + case VIR_DOMAIN_DEVICE_MEMORY: + case VIR_DOMAIN_DEVICE_IOMMU: + case VIR_DOMAIN_DEVICE_LAST: + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Unexpected device type %d"), + data->devDef->type); + goto cleanup; + } + + ret = 0; + cleanup: + if (ret < 0) + unlink(data->file); + virFileFreeACLs(&data->acl); + return ret; +} + + +static int +qemuDomainAttachDeviceMknod(virQEMUDriverPtr driver, + virDomainObjPtr vm, + virDomainDeviceDefPtr devDef, + const char *file) +{ + struct qemuDomainAttachDeviceMknodData data; + int ret = -1; + + memset(&data, 0, sizeof(data)); + + data.driver = driver; + data.vm = vm; + data.devDef = devDef; + data.file = file; + + if (stat(file, &data.sb) < 0) { + virReportSystemError(errno, + _("Unable to access %s"), file); + return ret; + } + + if (virFileGetACLs(file, &data.acl) < 0 && + errno != ENOTSUP) { + virReportSystemError(errno, + _("Unable to get ACLs on %s"), file); + return ret; + } + + if (virSecurityManagerPreFork(driver->securityManager) < 0) + goto cleanup; + + if (virProcessRunInMountNamespace(vm->pid, + qemuDomainAttachDeviceMknodHelper, + &data) < 0) { + virSecurityManagerPostFork(driver->securityManager); + goto cleanup; + } + + virSecurityManagerPostFork(driver->securityManager); + + ret = 0; + cleanup: + virFileFreeACLs(&data.acl); + return 0; +} + + +int +qemuDomainNamespaceSetupDisk(virQEMUDriverPtr driver, + virDomainObjPtr vm, + virDomainDiskDefPtr disk) +{ + virDomainDeviceDef dev = {.type = VIR_DOMAIN_DEVICE_DISK, .data.disk = disk}; + virStorageSourcePtr next; + const char *src = NULL; + struct stat sb; + int ret = -1; + + if (!qemuDomainNamespaceEnabled(vm, QEMU_DOMAIN_NS_MOUNT)) + return 0; + + for (next = disk->src; next; next = next->backingStore) { + if (!next->path || !virStorageSourceIsBlockLocal(disk->src) || + !STRPREFIX(next->path, "/dev")) { + /* Not creating device. Just continue. */ + continue; + } + + if (stat(next->path, &sb) < 0) { + virReportSystemError(errno, + _("Unable to access %s"), src); + goto cleanup; + } + + if (!S_ISBLK(sb.st_mode)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("Disk source %s must be a block device"), + src); + goto cleanup; + } + + if (qemuDomainAttachDeviceMknod(driver, + vm, + &dev, + next->path) < 0) + goto cleanup; + } + + ret = 0; + cleanup: + return ret; +} + + +int +qemuDomainNamespaceTeardownDisk(virQEMUDriverPtr driver ATTRIBUTE_UNUSED, + virDomainObjPtr vm ATTRIBUTE_UNUSED, + virDomainDiskDefPtr disk ATTRIBUTE_UNUSED) +{ + /* While in hotplug case we create the whole backing chain, + * here we must limit ourselves. The disk we want to remove + * might be a part of backing chain of another disk. + * If you are reading these lines and have some spare time + * you can come up with and algorithm that checks for that. + * I don't, therefore: */ + return 0; +} diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index 9dad5bc7cc..c63f7c9546 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -805,4 +805,12 @@ int qemuDomainCreateNamespace(virQEMUDriverPtr driver, void qemuDomainDeleteNamespace(virQEMUDriverPtr driver, virDomainObjPtr vm); + +int qemuDomainNamespaceSetupDisk(virQEMUDriverPtr driver, + virDomainObjPtr vm, + virDomainDiskDefPtr disk); + +int qemuDomainNamespaceTeardownDisk(virQEMUDriverPtr driver, + virDomainObjPtr vm, + virDomainDiskDefPtr disk); #endif /* __QEMU_DOMAIN_H__ */ diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 88778d491d..ea44f1f392 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -57,6 +57,7 @@ #include "qemu_process.h" #include "qemu_migration.h" #include "qemu_blockjob.h" +#include "qemu_security.h" #include "virerror.h" #include "virlog.h" @@ -16146,9 +16147,9 @@ qemuDomainBlockPivot(virQEMUDriverPtr driver, disk->mirror->format != VIR_STORAGE_FILE_RAW && (virDomainLockDiskAttach(driver->lockManager, cfg->uri, vm, disk) < 0 || + qemuDomainNamespaceSetupDisk(driver, vm, disk) < 0 || qemuSetupDiskCgroup(vm, disk) < 0 || - virSecurityManagerSetDiskLabel(driver->securityManager, vm->def, - disk) < 0)) + qemuSecuritySetDiskLabel(driver, vm, disk) < 0)) goto cleanup; disk->src = oldsrc; diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 55af88850a..bc953c0db7 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -34,6 +34,7 @@ #include "qemu_hostdev.h" #include "qemu_interface.h" #include "qemu_process.h" +#include "qemu_security.h" #include "domain_audit.h" #include "netdev_bandwidth_conf.h" #include "domain_nwfilter.h" @@ -109,13 +110,15 @@ qemuDomainPrepareDisk(virQEMUDriverPtr driver, vm, disk) < 0) goto cleanup; - if (virSecurityManagerSetDiskLabel(driver->securityManager, - vm->def, disk) < 0) + if (qemuSecuritySetDiskLabel(driver, vm, disk) < 0) goto rollback_lock; - if (qemuSetupDiskCgroup(vm, disk) < 0) + if (qemuDomainNamespaceSetupDisk(driver, vm, disk) < 0) goto rollback_label; + if (qemuSetupDiskCgroup(vm, disk) < 0) + goto rollback_namespace; + ret = 0; goto cleanup; @@ -123,10 +126,13 @@ qemuDomainPrepareDisk(virQEMUDriverPtr driver, if (qemuTeardownDiskCgroup(vm, disk) < 0) VIR_WARN("Unable to tear down cgroup access on %s", virDomainDiskGetSource(disk)); + rollback_namespace: + if (qemuDomainNamespaceTeardownDisk(driver, vm, disk) < 0) + VIR_WARN("Unable to remove /dev entry for %s", + virDomainDiskGetSource(disk)); rollback_label: - if (virSecurityManagerRestoreDiskLabel(driver->securityManager, - vm->def, disk) < 0) + if (qemuSecurityRestoreDiskLabel(driver, vm, disk) < 0) VIR_WARN("Unable to restore security label on %s", virDomainDiskGetSource(disk)); @@ -3588,8 +3594,7 @@ qemuDomainRemoveDiskDevice(virQEMUDriverPtr driver, qemuDomainReleaseDeviceAddress(vm, &disk->info, src); - if (virSecurityManagerRestoreDiskLabel(driver->securityManager, - vm->def, disk) < 0) + if (qemuSecurityRestoreDiskLabel(driver, vm, disk) < 0) VIR_WARN("Unable to restore security label on %s", src); if (qemuTeardownDiskCgroup(vm, disk) < 0) @@ -3598,6 +3603,9 @@ qemuDomainRemoveDiskDevice(virQEMUDriverPtr driver, if (virDomainLockDiskDetach(driver->lockManager, vm, disk) < 0) VIR_WARN("Unable to release lock on %s", src); + if (qemuDomainNamespaceTeardownDisk(driver, vm, disk) < 0) + VIR_WARN("Unable to remove /dev entry for %s", src); + dev.type = VIR_DOMAIN_DEVICE_DISK; dev.data.disk = disk; ignore_value(qemuRemoveSharedDevice(driver, &dev, vm->def->name)); diff --git a/src/qemu/qemu_security.c b/src/qemu/qemu_security.c index bfebfe42c3..ef0865b618 100644 --- a/src/qemu/qemu_security.c +++ b/src/qemu/qemu_security.c @@ -130,3 +130,35 @@ qemuSecurityRestoreAllLabel(virQEMUDriverPtr driver, migrated); } } + + +int +qemuSecuritySetDiskLabel(virQEMUDriverPtr driver, + virDomainObjPtr vm, + virDomainDiskDefPtr disk) +{ + if (qemuDomainNamespaceEnabled(vm, QEMU_DOMAIN_NS_MOUNT)) { + /* Already handled by namespace code. */ + return 0; + } + + return virSecurityManagerSetDiskLabel(driver->securityManager, + vm->def, + disk); +} + + +int +qemuSecurityRestoreDiskLabel(virQEMUDriverPtr driver, + virDomainObjPtr vm, + virDomainDiskDefPtr disk) +{ + if (qemuDomainNamespaceEnabled(vm, QEMU_DOMAIN_NS_MOUNT)) { + /* Already handled by namespace code. */ + return 0; + } + + return virSecurityManagerRestoreDiskLabel(driver->securityManager, + vm->def, + disk); +} diff --git a/src/qemu/qemu_security.h b/src/qemu/qemu_security.h index 88c53e765e..e3324ca8c3 100644 --- a/src/qemu/qemu_security.h +++ b/src/qemu/qemu_security.h @@ -36,4 +36,12 @@ int qemuSecuritySetAllLabel(virQEMUDriverPtr driver, void qemuSecurityRestoreAllLabel(virQEMUDriverPtr driver, virDomainObjPtr vm, bool migrated); + +int qemuSecuritySetDiskLabel(virQEMUDriverPtr driver, + virDomainObjPtr vm, + virDomainDiskDefPtr disk); + +int qemuSecurityRestoreDiskLabel(virQEMUDriverPtr driver, + virDomainObjPtr vm, + virDomainDiskDefPtr disk); #endif /* __QEMU_SECURITY_H__ */