Re: [libvirt] [PATCH v2 2/2] qemu: Also treat directories properly when using namespaces

When recreating folders with namespaces, the directory type was not being handled at all. It's not special, we probably just didn't know that that can be used as a volume path as well. The code failed gracefully, but we want to allow that so that we can use <disk type='dir'> in domains again. Partially-resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1443434 Signed-off-by: Martin Kletzander <mkletzan(a)redhat.com&gt; --- src/qemu/qemu_domain.c | 40 +++++++++++++++++++++++++++++++++++----- 1 file changed, 35 insertions(+), 5 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 2c77a6442467..2549f9bf3290 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -7855,6 +7855,7 @@ qemuDomainCreateDeviceRecursive(const char *device, bool isLink = false; bool isDev = false; bool isReg = false; + bool isDir = false; bool create = false; #ifdef WITH_SELINUX char *tcon = NULL; @@ -7879,6 +7880,7 @@ qemuDomainCreateDeviceRecursive(const char *device, isLink = S_ISLNK(sb.st_mode); isDev = S_ISCHR(sb.st_mode) || S_ISBLK(sb.st_mode); isReg = S_ISREG(sb.st_mode) || S_ISFIFO(sb.st_mode) || S_ISSOCK(sb.st_mode); + isDir = S_ISDIR(sb.st_mode); /* Here, @device might be whatever path in the system. We * should create the path in the namespace iff it's "/dev" @@ -7996,6 +7998,10 @@ qemuDomainCreateDeviceRecursive(const char *device, goto cleanup; /* Just create the file here so that code below sets * proper owner and mode. Bind mount only after that. */ + } else if (isDir) { + if (create && + virFileMakePathWithMode(devicePath, sb.st_mode) < 0) + goto cleanup; } else { virReportError(VIR_ERR_OPERATION_UNSUPPORTED, _("unsupported device type %s 0%o"), @@ -8057,7 +8063,7 @@ qemuDomainCreateDeviceRecursive(const char *device, #endif /* Finish mount process started earlier. */ - if (isReg && + if ((isReg || isDir) && virFileBindMountDevice(device, devicePath) < 0) goto cleanup; @@ -8686,6 +8692,7 @@ qemuDomainAttachDeviceMknodHelper(pid_t pid ATTRIBUTE_UNUSED, bool isLink = S_ISLNK(data->sb.st_mode); bool isDev = S_ISCHR(data->sb.st_mode) || S_ISBLK(data->sb.st_mode); bool isReg = S_ISREG(data->sb.st_mode) || S_ISFIFO(data->sb.st_mode) || S_ISSOCK(data->sb.st_mode); + bool isDir = S_ISDIR(data->sb.st_mode); qemuSecurityPostFork(data->driver->securityManager); @@ -8741,6 +8748,23 @@ qemuDomainAttachDeviceMknodHelper(pid_t pid ATTRIBUTE_UNUSED, delDevice = true; /* Just create the file here so that code below sets * proper owner and mode. Move the mount only after that. */ + } else if (isDir) { + /* We are not cleaning up disks on virDomainDetachDevice + * because disk might be still in use by different disk + * as its backing chain. This might however clash here. + * Therefore do the cleanup here. */ + if (umount(data->file) < 0 && + errno != ENOENT && errno != EINVAL) { + virReportSystemError(errno, + _("Unable to umount %s"), + data->file); + goto cleanup; + } + if (virFileMakePathWithMode(data->file, data->sb.st_mode) < 0) + goto cleanup; + delDevice = true; + /* Just create the folder here so that code below sets + * proper owner and mode. Move the mount only after that. */

} else { virReportError(VIR_ERR_OPERATION_UNSUPPORTED, _("unsupported device type %s 0%o"), @@ -8788,14 +8812,18 @@ qemuDomainAttachDeviceMknodHelper(pid_t pid ATTRIBUTE_UNUSED, # endif /* Finish mount process started earlier. */ - if (isReg && + if ((isReg || isDir) && virFileMoveMount(data->target, data->file) < 0) goto cleanup; ret = 0; cleanup: - if (ret < 0 && delDevice) - unlink(data->file); + if (ret < 0 && delDevice) { + if (isDir) + virFileDeleteTree(data->file); + else + unlink(data->file); + } # ifdef WITH_SELINUX freecon(data->tcon); # endif