[libvirt] This patch creates a mount point for libvirt-lxc containers to mount on the destination system if it does not exist.

[Daniel P. Berrange]

On Fri, Jun 22, 2012 at 09:59:58AM -0400, Daniel J Walsh wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
> 
> Currently libvirt-lxc checks to see if the destination exists and is a
> directory.  If it is not a directory then the mount fails.  Since libvirt-lxc
> can bind mount files on an inode, this patch is needed to allow us to bind
> mount files on files.  Currently we want to bind mount on top of
> /etc/machine-id, and /etc/adjtime
> 
> If the destination of the mount point does not exists, it checks if the src is
> a directory and then attempts to create a directory, otherwise it creates an
> empty file for the destination.  The code will then bind mount over the
> destination.
> 
> Current code blows up if the destination was not a directory.  We want to be
> able to bind mount files on files.
> 
> Sorry if you are seeing this patch for the second time, since I sent it to the
> wrong list.
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1.4.12 (GNU/Linux)
> Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/
> 
> iEYEARECAAYFAk/kel4ACgkQrlYvE4MpobP7uwCfXUPMJP4bNZiBGTwnJ70dezVf
> KPwAnjt3MYSlHxkcwZCe5H1X4C0P4ky/
> =1te/
> -----END PGP SIGNATURE-----

> diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c
> index 24b1017..6cd8760 100644
> --- a/src/lxc/lxc_container.c
> +++ b/src/lxc/lxc_container.c
> @@ -648,17 +665,37 @@ static int lxcContainerMountFSBind(virDomainFSDefPtr fs,
>  {
>      char *src = NULL;
>      int ret = -1;
> +    struct stat st;
>  
>      if (virAsprintf(&src, "%s%s", srcprefix, fs->src) < 0) {
>          virReportOOMError();
>          goto cleanup;
>      }
>  
> -    if (virFileMakePath(fs->dst) < 0) {
> -        virReportSystemError(errno,
> -                             _("Failed to create %s"),
> -                             fs->dst);
> -        goto cleanup;
> +    if ((stat(fs->dst, &st) < 0) && (errno == ENOENT)) {
> +        if (stat(src, &st) >= 0) {

We should be reporting errors if either of the stat() calls
fails.

> +            if (S_ISDIR(st.st_mode)) {
> +                if (virFileMakePath(fs->dst) < 0) {
> +                    virReportSystemError(errno,
> +                                         _("Failed to create %s"),
> +                                         fs->dst);
> +                    goto cleanup;
> +                }
> +            } else {
> +                /* Create Empty file for target mount point */
> +                int fd = open(fs->dst, O_WRONLY|O_CREAT|O_NOCTTY|O_NONBLOCK, 0666);
> +                if (fd >= 0) {
> +                    close(fd);

This needs to be VIR_CLOSE & have it exit stats checked,
since close() can fail (particularly on NFS).

> +                } else {
> +                    if (errno != EEXIST) {
> +                        virReportSystemError(errno,
> +                                             _("Failed to create %s"),
> +                                             fs->dst);
> +                        goto cleanup;
> +                    }
> +                }
> +            }
> +        }


I'm applying the following variation on your patch which fixes the
issues I mention above:


-    if (virFileMakePath(fs->dst) < 0) {
-        virReportSystemError(errno,
-                             _("Failed to create %s"),
-                             fs->dst);
-        goto cleanup;
+    if (stat(fs->dst, &st) < 0) {
+        if (errno != ENOENT) {
+            virReportSystemError(errno, _("Unable to stat bind target %s"),
+                                 fs->dst);
+            goto cleanup;
+        }
+        /* ENOENT => create the target dir or file */
+        if (stat(src, &st) < 0) {
+            virReportSystemError(errno, _("Unable to stat bind source %s"),
+                                 src);
+            goto cleanup;
+        }
+        if (S_ISDIR(st.st_mode)) {
+            if (virFileMakePath(fs->dst) < 0) {
+                virReportSystemError(errno,
+                                     _("Failed to create %s"),
+                                     fs->dst);
+                goto cleanup;
+            }
+        } else {
+            /* Create Empty file for target mount point */
+            int fd = open(fs->dst, O_WRONLY|O_CREAT|O_NOCTTY|O_NONBLOCK, 0666);
+            if (fd < 0) {
+                if (errno != EEXIST) {
+                    virReportSystemError(errno,
+                                         _("Failed to create bind target %s"),
+                                         fs->dst);
+                    goto cleanup;
+                }
+            }
+            if (VIR_CLOSE(fd) < 0) {
+                virReportSystemError(errno,
+                                     _("Failed to close bind target %s"),
+                                     fs->dst);
+                goto cleanup;
+            }
+        }


Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|