Re: [libvirt] [PATCH] Add support for probing filesystem with libblkid

On 11/01/2011 09:02 AM, Daniel P. Berrange wrote: >From: "Daniel P. Berrange&quot;&lt;berrange(a)redhat.com&gt; > >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; >+ }

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