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 :| -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list