[libvirt] [PATCH] Add support for probing filesystem with libblkid
Eric Blake
eblake at redhat.com
Tue Nov 1 15:21:50 UTC 2011
On 11/01/2011 09:02 AM, Daniel P. Berrange wrote:
> From: "Daniel P. Berrange"<berrange at redhat.com>
>
> The LXC code for mounting container filesystems from block devices
> tries all filesystems in /etc/filesystems and possibly those in
> /proc/filesystems. The regular mount binary, however, first tries
> using libblkid to detect the format. Add support for doing the same
> in libvirt, since Fedora's /etc/filesystems is missing many formats,
> most notably ext4 which is the default filesystem Fedora uses!
>
> * src/Makefile.am: Link libvirt_lxc to libblkid
> * src/lxc/lxc_container.c: Probe filesystem format with liblkid
s/liblkid/libblkid/
> ---
> src/Makefile.am | 4 ++
> src/lxc/lxc_container.c | 102 ++++++++++++++++++++++++++++++++++++++++++++++-
> 2 files changed, 105 insertions(+), 1 deletions(-)
configure.ac already unconditionally checked for libblkid, so this is
simple enough to start using.
> +#ifdef HAVE_LIBBLKID
> +static int
> +lxcContainerMountDetectFilesystem(const char *src, char **type)
> +{
> + int fd;
> + int ret = -1;
> + int rc;
> + const char *data = NULL;
> + blkid_probe blkid = NULL;
> +
> + *type = NULL;
> +
> + if ((fd = open(src, O_RDONLY))< 0) {
> + virReportSystemError(errno,
> + _("Unable to open filesystem %s"), src);
> + return -1;
If we fail to open() the file, can't we at least fall back to the mount
probing? That is, what do we gain by returning failure here, instead of
returning 0 and letting the fallback code be attempted?
> + }
> +
> + if (!(blkid = blkid_new_probe())) {
> + virReportSystemError(errno, "%s",
> + _("Unable to create blkid library handle"));
> + goto cleanup;
> + }
> + if (blkid_probe_set_device(blkid, fd, 0, 0)< 0) {
> + virReportSystemError(EINVAL,
> + _("Unable to associated device %s with blkid library"),
> + src);
s/associated/associate/
> + goto cleanup;
> + }
> +
> + blkid_probe_enable_superblocks(blkid, 1);
> +
> + blkid_probe_set_superblocks_flags(blkid, BLKID_SUBLKS_TYPE);
> +
> + rc = blkid_do_safeprobe(blkid);
> + if (rc != 0) {
> + if (rc == 1) /* Nothing found, return success with *type == NULL */
> + goto done;
> +
> + if (rc == -2) {
> + virReportSystemError(EINVAL,
> + _("Too many filesystems detected for %s"),
> + src);
> + } else {
> + virReportSystemError(errno,
> + _("Unable to detect filesystem for %s"),
> + src);
> + }
> + goto cleanup;
> + }
> +
> + if (blkid_probe_lookup_value(blkid, "TYPE",&data, NULL)< 0) {
> + virReportSystemError(ENOENT,
> + _("Unable to find filesystem type for %s"),
> + src);
> + goto cleanup;
> + }
> +
> + if (!(*type = strdup(data))) {
> + virReportOOMError();
> + goto cleanup;
> + }
> +
> +done:
> + ret = 0;
> +cleanup:
> + VIR_FORCE_CLOSE(fd);
> + blkid_free_probe(blkid);
Is this safe? In storage_backed_fs.c, you check that probe != NULL
before calling blkid_free_probe. If this function is free-like, we
should add it to cfg.mk and update storage_backend_fs.c to get rid of
the useless conditional; if not, then this needs fixing to avoid
crashing libblkid on a NULL deref.
> + return ret;
> +}
> +#else /* ! HAVE_LIBBLKID */
> +static int
> +lxcContainerMountDetectFilesystem(const char *src ATTRIBUTE_UNUSED,
> + char **type)
> +{
> + /* No libblkid, so just return success with no detected type */
> + *type = NULL;
> + return 0;
> +}
> +#endif /* ! HAVE_LIBBLKID */
>
> /*
> * This functions attempts to do automatic detection of filesystem
> @@ -771,6 +855,8 @@ static int lxcContainerMountFSBlockHelper(virDomainFSDefPtr fs,
> {
> int fsflags = 0;
> int ret = -1;
> + char *format = NULL;
> +
> if (fs->readonly)
> fsflags |= MS_RDONLY;
>
> @@ -781,7 +867,21 @@ static int lxcContainerMountFSBlockHelper(virDomainFSDefPtr fs,
> goto cleanup;
> }
>
> - ret = lxcContainerMountFSBlockAuto(fs, fsflags, src, srcprefix);
> + if (lxcContainerMountDetectFilesystem(src,&format)< 0)
> + goto cleanup;
Given my earlier question, about the only failures that should avoid the
fallback would be OOM failure. But since OOM is a real possibility, I
agree with the signature (return 0 on success, whether or not format ==
NULL).
> +
> + if (format) {
> + VIR_DEBUG("Mount %s with detected format %s", src, format);
> + if (mount(src, fs->dst, format, fsflags, NULL)< 0) {
> + virReportSystemError(errno,
> + _("Failed to mount device %s to %s as %s"),
> + src, fs->dst, format);
> + goto cleanup;
> + }
> + ret = 0;
> + } else {
> + ret = lxcContainerMountFSBlockAuto(fs, fsflags, src, srcprefix);
> + }
This part makes sense. Overall, I like the idea, but it may be worth a
v2 depending on the answers to my questions above.
--
Eric Blake eblake at redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org
More information about the libvir-list
mailing list