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@redhat.com>
---
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
@@ -8818,6 +8846,7 @@ qemuDomainAttachDeviceMknodRecursive(virQEMUDriverPtr driver,
char *target = NULL;
bool isLink;
bool isReg;
+ bool isDir;
if (!ttl) {
virReportSystemError(ELOOP,
@@ -8840,8 +8869,9 @@ qemuDomainAttachDeviceMknodRecursive(virQEMUDriverPtr driver,
isLink = S_ISLNK(data.sb.st_mode);
isReg = S_ISREG(data.sb.st_mode) || S_ISFIFO(data.sb.st_mode) || S_ISSOCK(data.sb.st_mode);
+ isDir = S_ISDIR(data.sb.st_mode);
- if (isReg && STRPREFIX(file, DEVPREFIX)) {
+ if ((isReg || isDir) && STRPREFIX(file, DEVPREFIX)) {
cfg = virQEMUDriverGetConfig(driver);
if (!(target = qemuDomainGetPreservedMountPath(cfg, vm, file)))
goto cleanup;
--
2.14.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On 08/29/2017 10:34 AM, Martin Kletzander wrote:
> 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@redhat.com>
> ---
> 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. */
Or, you can merge this with the previous else branch since it's
practically identical. Just like you're doing in the next hunk.
> } 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
ACK series.
Michal
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
© 2016 - 2025 Red Hat, Inc.