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

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

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