Re: [libvirt] [PATCH 1/1] Format FS pools

On Mon, Feb 22, 2010 at 01:44:39PM -0500, David Allan wrote: > * Create the filesystem on the partition used by the pool. > --- > configure.ac | 5 +++++ > src/storage/storage_backend_fs.c | 22 ++++++++++++++++++++++ > 2 files changed, 27 insertions(+), 0 deletions(-) > > diff --git a/configure.ac b/configure.ac > index 743a357..616bd03 100644 > --- a/configure.ac > +++ b/configure.ac > @@ -1252,12 +1252,15 @@ AM_CONDITIONAL([WITH_STORAGE_DIR], [test "$with_storage_dir" = "yes"]) > if test "$with_storage_fs" = "yes" -o "$with_storage_fs" = "check"; then > AC_PATH_PROG([MOUNT], [mount], [], [$PATH:/sbin:/usr/sbin]) > AC_PATH_PROG([UMOUNT], [umount], [], [$PATH:/sbin:/usr/sbin]) > + AC_PATH_PROG([MKE2FS], [mke2fs], [], [$PATH:/sbin:/usr/sbin]) Actually I think we need to check for mkfs to be filesystem generic > if test "$with_storage_fs" = "yes" ; then > if test -z "$MOUNT" ; then AC_MSG_ERROR([We need mount for FS storage driver]) ; fi > if test -z "$UMOUNT" ; then AC_MSG_ERROR([We need umount for FS storage driver]) ; fi > + if test -z "$MKE2FS" ; then AC_MSG_ERROR([We need mke2fs for FS storage driver]) ; fi > else > if test -z "$MOUNT" ; then with_storage_fs=no ; fi > if test -z "$UMOUNT" ; then with_storage_fs=no ; fi > + if test -z "$MKE2FS" ; then with_storage_fs=no ; fi > > if test "$with_storage_fs" = "check" ; then with_storage_fs=yes ; fi > fi > @@ -1268,6 +1271,8 @@ if test "$with_storage_fs" = "yes" -o "$with_storage_fs" = "check"; then > [Location or name of the mount program]) > AC_DEFINE_UNQUOTED([UMOUNT],["$UMOUNT"], > [Location or name of the mount program]) > + AC_DEFINE_UNQUOTED([MKE2FS],["$MKE2FS"], > + [Location or name of the mke2fs program]) > fi > fi > AM_CONDITIONAL([WITH_STORAGE_FS], [test "$with_storage_fs" = "yes"]) > diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c > index 8279d78..eb5da5e 100644 > --- a/src/storage/storage_backend_fs.c > +++ b/src/storage/storage_backend_fs.c > @@ -45,6 +45,7 @@ > #include "util.h" > #include "memory.h" > #include "xml.h" > +#include "logging.h" > > #define VIR_FROM_THIS VIR_FROM_STORAGE > > @@ -500,6 +501,7 @@ virStorageBackendFileSystemBuild(virConnectPtr conn ATTRIBUTE_UNUSED, > virStoragePoolObjPtr pool, > unsigned int flags ATTRIBUTE_UNUSED) > { > + const char *mke2fsargv[5], *device = NULL, *format = NULL; > int err, ret = -1; > char *parent; > char *p; > @@ -540,6 +542,26 @@ virStorageBackendFileSystemBuild(virConnectPtr conn ATTRIBUTE_UNUSED, > pool->def->target.path); > goto error; > } > + > + device = pool->def->source.devices[0].path; > + format = virStoragePoolFormatFileSystemTypeToString(pool->def->source.format); > + > + VIR_DEBUG("source device: '%s' format: '%s'", device, format); > + > + mke2fsargv[0] = MKE2FS; > + mke2fsargv[1] = "-t"; mkfs has a -t type argument, not mke2fs on my machine > + mke2fsargv[2] = format; > + mke2fsargv[3] = device; > + mke2fsargv[4] = NULL; > + > + if (virRun(mke2fsargv, NULL)< 0) { > + virReportSystemError(errno, > + _("Failed to make filesystem of " > + "type '%s' on device '%s'"), > + format, device); > + goto error; > + } > + > ret = 0; > error: > VIR_FREE(parent); So I come up with the following patch instead. Note that /sbin/mkfs is provided by util-linux which is alread required for the storage fs support so we don't need to add anything (BTW util-linux is now provided by util-linux-ng on recent Fedoras, but keeping the old canonical require name is probably better). Daniel