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

[Daniel J Walsh]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 06/25/2012 05:56 AM, Daniel P. Berrange wrote:
> On Fri, Jun 22, 2012 at 09:59:58AM -0400, Daniel J Walsh wrote: 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.
> 
>> 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
> 

Great thanks.
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.12 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAk/oZDsACgkQrlYvE4MpobMa6ACg30+YwXBo1R8Tk7HjRGCEqqJN
DQQAn0t26CHFffhWv73PZhmg1JSx7xRv
=iM5A
-----END PGP SIGNATURE-----