[libvirt] PATCH [0/1] Filesystem pool formatting
Here's an updated patch based on the feedback from DV and danpb. I haven't added support for any additional filesystems beyond what mke2fs supports. The addition of additional filesystems is easily done and better as a separate patch. Dave
* 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]) 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"; + 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); -- 1.6.5.5
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]) 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
We'll need to add a Requires: e2fsprogs to the libvirt.spec for this.
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"; + 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 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 :|
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/
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 :|
On Tue, Feb 23, 2010 at 09:03:05AM +0000, Daniel P. Berrange wrote:
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: [...]
ACK, this looks fine - just add a 'util-linux-ng' dependancy to the RPM spec for mkfs
Well, on Fedora util-linux-ng provides util-linux, and util-linux-ng doesn't exist on RHEL-5, so as we are alreadyu build-requiring util-linux in the spec file, I think it makes more sense to just Requires: util-linux for compatibility, Will do today, 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/
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.
On Tue, Feb 23, 2010 at 01:49:46PM -0500, Dave Allan wrote:
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.
Okay, pushed :-) 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/
participants (4)
-
Daniel P. Berrange -
Daniel Veillard -
Dave Allan -
David Allan