[libvirt] PATCH [0/3] Filesystem pool formatting

I finished these patches this afternoon, and I need to do a bit more testing before I'm comfortable having these patches committed, but I'm sending them along so I can get everybody's feedback overnight. In particular, I'm wondering if my move of the pool unref call is correct. Dave

--- tools/virsh.c | 3 ++- 1 files changed, 2 insertions(+), 1 deletions(-) diff --git a/tools/virsh.c b/tools/virsh.c index e1d1300..bd6b6be 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -4036,9 +4036,10 @@ cmdPoolBuild(vshControl *ctl, const vshCmd *cmd) } else { vshError(ctl, _("Failed to build pool %s"), name); ret = FALSE; - virStoragePoolFree(pool); } + virStoragePoolFree(pool); + return ret; } -- 1.6.5.5

On Thu, Feb 18, 2010 at 05:58:05PM -0500, David Allan wrote:
--- tools/virsh.c | 3 ++- 1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/tools/virsh.c b/tools/virsh.c index e1d1300..bd6b6be 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -4036,9 +4036,10 @@ cmdPoolBuild(vshControl *ctl, const vshCmd *cmd) } else { vshError(ctl, _("Failed to build pool %s"), name); ret = FALSE; - virStoragePoolFree(pool); }
+ virStoragePoolFree(pool); + return ret; }
Looks fine, ACK, 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/

* If the user supplies the appropriate flag, create the filesystem on the partition used by the pool. --- configure.ac | 5 +++++ include/libvirt/libvirt.h.in | 3 ++- src/storage/storage_backend_fs.c | 25 ++++++++++++++++++++++++- 3 files changed, 31 insertions(+), 2 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/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index 260505e..a7751b8 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -1053,7 +1053,8 @@ typedef enum { typedef enum { VIR_STORAGE_POOL_BUILD_NEW = 0, /* Regular build from scratch */ VIR_STORAGE_POOL_BUILD_REPAIR = 1, /* Repair / reinitialize */ - VIR_STORAGE_POOL_BUILD_RESIZE = 2 /* Extend existing pool */ + VIR_STORAGE_POOL_BUILD_RESIZE = 2, /* Extend existing pool */ + VIR_STORAGE_POOL_CREATE_FORMAT = 3 /* Format filesystem during build */ } virStoragePoolBuildFlags; typedef enum { diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c index bbd5787..eedbe2b 100644 --- a/src/storage/storage_backend_fs.c +++ b/src/storage/storage_backend_fs.c @@ -39,12 +39,14 @@ #include <libxml/xpath.h> #include "virterror_internal.h" +#include "internal.h" #include "storage_backend_fs.h" #include "storage_conf.h" #include "storage_file.h" #include "util.h" #include "memory.h" #include "xml.h" +#include "logging.h" #define VIR_FROM_THIS VIR_FROM_STORAGE @@ -498,8 +500,9 @@ virStorageBackendFileSystemStart(virConnectPtr conn ATTRIBUTE_UNUSED, static int virStorageBackendFileSystemBuild(virConnectPtr conn ATTRIBUTE_UNUSED, virStoragePoolObjPtr pool, - unsigned int flags ATTRIBUTE_UNUSED) + unsigned int flags) { + const char *mke2fsargv[5], *device = NULL, *format = NULL; int err, ret = -1; char *parent; char *p; @@ -540,6 +543,26 @@ virStorageBackendFileSystemBuild(virConnectPtr conn ATTRIBUTE_UNUSED, pool->def->target.path); goto error; } + + if (flags & VIR_STORAGE_POOL_CREATE_FORMAT) { + 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) { + VIR_ERROR("Failed to make '%s' filesystem on device '%s'", + format, device); + goto error; + } + } + ret = 0; error: VIR_FREE(parent); -- 1.6.5.5

On Thu, Feb 18, 2010 at 05:58:06PM -0500, David Allan wrote:
* If the user supplies the appropriate flag, create the filesystem on the partition used by the pool. --- configure.ac | 5 +++++ include/libvirt/libvirt.h.in | 3 ++- src/storage/storage_backend_fs.c | 25 ++++++++++++++++++++++++- 3 files changed, 31 insertions(+), 2 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
That's where things starts to be nasty, why ext2, and not say ext3, then someone surely will want ext4 or xfs :-) I'm afraid we probably need to implement those 4 and give them different enums (below).
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/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index 260505e..a7751b8 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -1053,7 +1053,8 @@ typedef enum { typedef enum { VIR_STORAGE_POOL_BUILD_NEW = 0, /* Regular build from scratch */ VIR_STORAGE_POOL_BUILD_REPAIR = 1, /* Repair / reinitialize */ - VIR_STORAGE_POOL_BUILD_RESIZE = 2 /* Extend existing pool */ + VIR_STORAGE_POOL_BUILD_RESIZE = 2, /* Extend existing pool */ + VIR_STORAGE_POOL_CREATE_FORMAT = 3 /* Format filesystem during build */
Hum, I'm not so sure, CREATE_FORMAT == BUILD_REPAIR + BUILD_RESIZE which after all sounds a plausible combination, I would use different bits in the field, so set VIR_STORAGE_POOL_CREATE_FORMAT to 4. But now we need to cope with EXT3/EXT4/XFS variants, they should probably be allocated on separate bits even if they are mutually exclusive
} virStoragePoolBuildFlags;
typedef enum { diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c index bbd5787..eedbe2b 100644 --- a/src/storage/storage_backend_fs.c +++ b/src/storage/storage_backend_fs.c @@ -39,12 +39,14 @@ #include <libxml/xpath.h>
#include "virterror_internal.h" +#include "internal.h" #include "storage_backend_fs.h" #include "storage_conf.h" #include "storage_file.h" #include "util.h" #include "memory.h" #include "xml.h" +#include "logging.h"
#define VIR_FROM_THIS VIR_FROM_STORAGE
@@ -498,8 +500,9 @@ virStorageBackendFileSystemStart(virConnectPtr conn ATTRIBUTE_UNUSED, static int virStorageBackendFileSystemBuild(virConnectPtr conn ATTRIBUTE_UNUSED, virStoragePoolObjPtr pool, - unsigned int flags ATTRIBUTE_UNUSED) + unsigned int flags) { + const char *mke2fsargv[5], *device = NULL, *format = NULL; int err, ret = -1; char *parent; char *p; @@ -540,6 +543,26 @@ virStorageBackendFileSystemBuild(virConnectPtr conn ATTRIBUTE_UNUSED, pool->def->target.path); goto error; } + + if (flags & VIR_STORAGE_POOL_CREATE_FORMAT) {
We are really testing for the bit :-)
+ 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) { + VIR_ERROR("Failed to make '%s' filesystem on device '%s'", + format, device); + goto error; + } + } + ret = 0; error: VIR_FREE(parent);
I think the core is there but we probably need to expand the format options (detection at configure time sounds sufficient to me), most of the code addition in virStorageBackendFileSystemBuild() can probably be factorized for the diffferent formats. That should not be hard :-) thanks ! 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/

Daniel Veillard wrote:
On Thu, Feb 18, 2010 at 05:58:06PM -0500, David Allan wrote:
* If the user supplies the appropriate flag, create the filesystem on the partition used by the pool. --- configure.ac | 5 +++++ include/libvirt/libvirt.h.in | 3 ++- src/storage/storage_backend_fs.c | 25 ++++++++++++++++++++++++- 3 files changed, 31 insertions(+), 2 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
That's where things starts to be nasty, why ext2, and not say ext3, then someone surely will want ext4 or xfs :-) I'm afraid we probably need to implement those 4 and give them different enums (below).
How about all file systems defined in virStoragePoolFormatFileSystem that support formatting. Regards, Jim

On Sun, Feb 21, 2010 at 03:40:35PM -0700, Jim Fehlig wrote:
Daniel Veillard wrote:
On Thu, Feb 18, 2010 at 05:58:06PM -0500, David Allan wrote:
* If the user supplies the appropriate flag, create the filesystem on the partition used by the pool. --- configure.ac | 5 +++++ include/libvirt/libvirt.h.in | 3 ++- src/storage/storage_backend_fs.c | 25 ++++++++++++++++++++++++- 3 files changed, 31 insertions(+), 2 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
That's where things starts to be nasty, why ext2, and not say ext3, then someone surely will want ext4 or xfs :-) I'm afraid we probably need to implement those 4 and give them different enums (below).
How about all file systems defined in virStoragePoolFormatFileSystem that support formatting.
Well, first we can use mkfs to build support for the ext* familly, so that reduces the set of binaries to check. Otherwise XFS (and maybe vfat) makes some sense to check, but I don't see the point for the others which are either legacy unix/mac or formats with very specific use cases, so yeah I would add an fsck.xfs check in configure.in too 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 Sun, Feb 21, 2010 at 03:48:24PM +0100, Daniel Veillard wrote:
On Thu, Feb 18, 2010 at 05:58:06PM -0500, David Allan wrote:
AM_CONDITIONAL([WITH_STORAGE_FS], [test "$with_storage_fs" = "yes"]) diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index 260505e..a7751b8 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -1053,7 +1053,8 @@ typedef enum { typedef enum { VIR_STORAGE_POOL_BUILD_NEW = 0, /* Regular build from scratch */ VIR_STORAGE_POOL_BUILD_REPAIR = 1, /* Repair / reinitialize */ - VIR_STORAGE_POOL_BUILD_RESIZE = 2 /* Extend existing pool */ + VIR_STORAGE_POOL_BUILD_RESIZE = 2, /* Extend existing pool */ + VIR_STORAGE_POOL_CREATE_FORMAT = 3 /* Format filesystem during build */
Hum, I'm not so sure, CREATE_FORMAT == BUILD_REPAIR + BUILD_RESIZE which after all sounds a plausible combination, I would use different bits in the field, so set VIR_STORAGE_POOL_CREATE_FORMAT to 4.
But now we need to cope with EXT3/EXT4/XFS variants, they should probably be allocated on separate bits even if they are mutually exclusive
This extra flag is causing the FS pool to diverge from semantics of the other pool types. As per the comment inthe header, the 'VIR_STORAGE_POOL_BUILD_NEW' is defined to be a full initialize of the pool from scratch. For logical pool this pvcreate + vgcreates the data. For disk backend this writes a new empty partition table label. For FS pools, this should be formatting the filesystem. The type of filesystem is actually visible via the XML, so we don't need to worry about ext3/4/etc in this context. Regards, 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 04:42:04PM +0000, Daniel P. Berrange wrote:
On Sun, Feb 21, 2010 at 03:48:24PM +0100, Daniel Veillard wrote:
On Thu, Feb 18, 2010 at 05:58:06PM -0500, David Allan wrote:
AM_CONDITIONAL([WITH_STORAGE_FS], [test "$with_storage_fs" = "yes"]) diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index 260505e..a7751b8 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -1053,7 +1053,8 @@ typedef enum { typedef enum { VIR_STORAGE_POOL_BUILD_NEW = 0, /* Regular build from scratch */ VIR_STORAGE_POOL_BUILD_REPAIR = 1, /* Repair / reinitialize */ - VIR_STORAGE_POOL_BUILD_RESIZE = 2 /* Extend existing pool */ + VIR_STORAGE_POOL_BUILD_RESIZE = 2, /* Extend existing pool */ + VIR_STORAGE_POOL_CREATE_FORMAT = 3 /* Format filesystem during build */
Hum, I'm not so sure, CREATE_FORMAT == BUILD_REPAIR + BUILD_RESIZE which after all sounds a plausible combination, I would use different bits in the field, so set VIR_STORAGE_POOL_CREATE_FORMAT to 4.
But now we need to cope with EXT3/EXT4/XFS variants, they should probably be allocated on separate bits even if they are mutually exclusive
This extra flag is causing the FS pool to diverge from semantics of the other pool types. As per the comment inthe header, the 'VIR_STORAGE_POOL_BUILD_NEW' is defined to be a full initialize of the pool from scratch. For logical pool this pvcreate + vgcreates the data. For disk backend this writes a new empty partition table label. For FS pools, this should be formatting the filesystem.
The type of filesystem is actually visible via the XML, so we don't need to worry about ext3/4/etc in this context.
Yeah, after checking a bit more, I agree :-) 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/

--- tools/virsh.c | 7 ++++++- 1 files changed, 6 insertions(+), 1 deletions(-) diff --git a/tools/virsh.c b/tools/virsh.c index bd6b6be..de8c67d 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -4015,6 +4015,7 @@ static const vshCmdInfo info_pool_build[] = { static const vshCmdOptDef opts_pool_build[] = { {"pool", VSH_OT_DATA, VSH_OFLAG_REQ, gettext_noop("pool name or uuid")}, + {"format", VSH_OT_BOOL, 0, gettext_noop("format the pool (destructive)")}, {NULL, 0, 0, NULL} }; @@ -4023,6 +4024,7 @@ cmdPoolBuild(vshControl *ctl, const vshCmd *cmd) { virStoragePoolPtr pool; int ret = TRUE; + int flags = 0; char *name; if (!vshConnectionUsability(ctl, ctl->conn, TRUE)) @@ -4031,7 +4033,10 @@ cmdPoolBuild(vshControl *ctl, const vshCmd *cmd) if (!(pool = vshCommandOptPool(ctl, cmd, "pool", &name))) return FALSE; - if (virStoragePoolBuild(pool, 0) == 0) { + if (vshCommandOptBool (cmd, "format")) + flags |= VIR_STORAGE_POOL_CREATE_FORMAT; + + if (virStoragePoolBuild(pool, flags) == 0) { vshPrint(ctl, _("Pool %s built\n"), name); } else { vshError(ctl, _("Failed to build pool %s"), name); -- 1.6.5.5

On Thu, Feb 18, 2010 at 05:58:07PM -0500, David Allan wrote:
--- tools/virsh.c | 7 ++++++- 1 files changed, 6 insertions(+), 1 deletions(-)
diff --git a/tools/virsh.c b/tools/virsh.c index bd6b6be..de8c67d 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -4015,6 +4015,7 @@ static const vshCmdInfo info_pool_build[] = {
static const vshCmdOptDef opts_pool_build[] = { {"pool", VSH_OT_DATA, VSH_OFLAG_REQ, gettext_noop("pool name or uuid")}, + {"format", VSH_OT_BOOL, 0, gettext_noop("format the pool (destructive)")}, {NULL, 0, 0, NULL} };
@@ -4023,6 +4024,7 @@ cmdPoolBuild(vshControl *ctl, const vshCmd *cmd) { virStoragePoolPtr pool; int ret = TRUE; + int flags = 0; char *name;
if (!vshConnectionUsability(ctl, ctl->conn, TRUE)) @@ -4031,7 +4033,10 @@ cmdPoolBuild(vshControl *ctl, const vshCmd *cmd) if (!(pool = vshCommandOptPool(ctl, cmd, "pool", &name))) return FALSE;
- if (virStoragePoolBuild(pool, 0) == 0) { + if (vshCommandOptBool (cmd, "format")) + flags |= VIR_STORAGE_POOL_CREATE_FORMAT; + + if (virStoragePoolBuild(pool, flags) == 0) { vshPrint(ctl, _("Pool %s built\n"), name); } else { vshError(ctl, _("Failed to build pool %s"), name);
Same thing we probably need to extend this a bit for ext2/ext3/ext4/xfs being able to format a pool with an FS supporting very fast fallocate() for images can make a serious performance change, so I think it's worth the extra work, no ? 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/18/2010 05:58 PM, David Allan wrote:
I finished these patches this afternoon, and I need to do a bit more testing before I'm comfortable having these patches committed, but I'm sending them along so I can get everybody's feedback overnight. In particular, I'm wondering if my move of the pool unref call is correct.
Having done some additional testing and code review, I think these patches are ready to go. Dave
participants (5)
-
Daniel P. Berrange
-
Daniel Veillard
-
Dave Allan
-
David Allan
-
Jim Fehlig