[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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



On Tue, Feb 23, 2010 at 09:15:55AM +0100, Daniel Veillard wrote:
> 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
> 
> -- 
> Daniel Veillard      | libxml Gnome XML XSLT toolkit  http://xmlsoft.org/
> daniel veillard com  | Rpmfind RPM search engine http://rpmfind.net/
> http://veillard.com/ | virtualization library  http://libvirt.org/

> diff --git a/configure.ac b/configure.ac
> index 743a357..117cb20 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([MKFS], [mkfs], [], [$PATH:/sbin:/usr/sbin])
>    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 "$MKFS" ; then AC_MSG_ERROR([We need mkfs 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 "$MKFS" ; 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([MKFS],["$MKFS"],
> +        [Location or name of the mkfs 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..0444fcf 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] = MKFS;
> +    mke2fsargv[1] = "-t";
> +    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);

ACK, this looks fine - just add a 'util-linux-ng' dependancy to the RPM
spec for mkfs


Daniel
-- 
|: Red Hat, Engineering, London    -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :|
|: http://autobuild.org        -o-         http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-   F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]