On Tue, Oct 21, 2008 at 03:57:07PM +0200, Chris Lalancette wrote:

Again because of my generic VIR_ENUM_IMPL patch last week, I unwittingly caused a regression in the storage_backend_logical driver. Previously, if you submitted logical pool XML that had no <source><format type='lvm2'/> string, it would just default to VIR_STORAGE_POOL_LOGICAL_LVM2. This would succeed just fine and go on with life. This is no longer the case, and now XML without the format tag fails to define. To maintain backwards compatibility with already existing XML that expects this, add a compatibility wrapper to return VIR_STORAGE_POOL_LOGICAL_LVM2 on an empty format tag.

I think I'd rather that we add a field to the .poolOptions struct called defaultFormat, and just do .defaultFormat = VIR_STORAGE_POOL_LOGICAL_LVM2 And then in the storage_conf.c method where we're parsing formats, if we get a NULL format, explicitly set it to the default. We'd want todo this for all drivers that support formats, not just LVM. Then we wouldn't need to wrap the FromString methods.

@@ -616,7 +629,7 @@ virStorageBackend virStorageBackendLogic .poolOptions = { .flags = (VIR_STORAGE_BACKEND_POOL_SOURCE_NAME | VIR_STORAGE_BACKEND_POOL_SOURCE_DEVICE), - .formatFromString = virStorageBackendLogicalPoolTypeFromString, + .formatFromString = virStorageBackendLogicalFormatFromStringWrap, .formatToString = virStorageBackendLogicalPoolTypeToString, },

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