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

[Dave Allan]

On 02/23/2010 03:15 AM, 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
>

ACK.  Tested with ext3, ext4 and xfs.