[libvirt] [PATCH 0/2] Make filesystems during fs pool build
This patch implements the formatting of block devices in the pool build operation of filesystem pools. The existing implementation would only make the directory on which to mount the filesystem, but required the user to make the filesystem manually. My only concern with this approach is that it changes the build behavior in a way that's potentially destructive, so I wonder whether it would be worth adding a flag to enable this behavior. Dave
--- configure.in | 5 +++++ src/storage/storage_backend_fs.c | 36 +++++++++++++++++++++++++++++++++--- 2 files changed, 38 insertions(+), 3 deletions(-) diff --git a/configure.in b/configure.in index 3f2f8ff..59ceadd 100644 --- a/configure.in +++ b/configure.in @@ -1243,12 +1243,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 @@ -1259,6 +1262,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 b7d4bd6..ca1f4cb 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 @@ -505,15 +506,44 @@ virStorageBackendFileSystemBuild(virConnectPtr conn, virStoragePoolObjPtr pool, unsigned int flags ATTRIBUTE_UNUSED) { - int err; + const char *mke2fsargv[5], *device = NULL, *format = NULL; + int err, ret = -1; + if ((err = virFileMakePath(pool->def->target.path)) < 0) { virReportSystemError(conn, err, _("cannot create path '%s'"), pool->def->target.path); - return -1; + goto out; } - return 0; + if (pool->def->source.ndevice != 1) { + virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR, + "Exactly one source device " + "may be specified for pool type '%s'", + virStoragePoolFormatFileSystemTypeToString(pool->def->source.format)); + goto out; + } + + 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(conn, mke2fsargv, NULL) < 0) { + VIR_ERROR("Failed to make '%s' filesystem on device '%s'", format, device); + goto out; + } + + ret = 0; + +out: + return ret; } -- 1.6.5.5
On Mon, Jan 04, 2010 at 02:42:38PM -0500, David Allan wrote:
--- configure.in | 5 +++++ src/storage/storage_backend_fs.c | 36 +++++++++++++++++++++++++++++++++--- 2 files changed, 38 insertions(+), 3 deletions(-)
I'm rediscovering that patch, I rebased it since the function changed, I'm just also worried by the potentially destructive nature of this change, so I'm not applying it now, but providing the rebase, 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 01/21/2010 09:02 AM, Daniel Veillard wrote:
On Mon, Jan 04, 2010 at 02:42:38PM -0500, David Allan wrote:
--- configure.in | 5 +++++ src/storage/storage_backend_fs.c | 36 +++++++++++++++++++++++++++++++++--- 2 files changed, 38 insertions(+), 3 deletions(-)
I'm rediscovering that patch, I rebased it since the function changed, I'm just also worried by the potentially destructive nature of this change, so I'm not applying it now, but providing the rebase,
I agree that it should not be applied until we have a discussion on creating a flag to trigger it. Dave
Daniel
* If a user specified a number of devices not equal to one, the error message could be misleading. --- src/storage/storage_backend_fs.c | 12 ++++++++---- 1 files changed, 8 insertions(+), 4 deletions(-) diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c index ca1f4cb..7a36c57 100644 --- a/src/storage/storage_backend_fs.c +++ b/src/storage/storage_backend_fs.c @@ -366,7 +366,10 @@ virStorageBackendFileSystemMount(virConnectPtr conn, } else { if (pool->def->source.ndevice != 1) { virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR, - "%s", _("missing source device")); + _("Exactly one source device " + "may be specified for pool type '%s'"), + virStoragePoolFormatFileSystemTypeToString( + pool->def->source.format)); return -1; } } @@ -518,9 +521,10 @@ virStorageBackendFileSystemBuild(virConnectPtr conn, if (pool->def->source.ndevice != 1) { virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR, - "Exactly one source device " - "may be specified for pool type '%s'", - virStoragePoolFormatFileSystemTypeToString(pool->def->source.format)); + _("Exactly one source device " + "may be specified for pool type '%s'"), + virStoragePoolFormatFileSystemTypeToString( + pool->def->source.format)); goto out; } -- 1.6.5.5
participants (3)
-
Daniel Veillard -
Dave Allan -
David Allan