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

[Michal Privoznik]

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 at 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