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

When doing logical pool discovery, it currently returns XML like:

<sources><source><name>myvg</name></source></sources>

However, for oVirt, we need to know the device name that corresponds to this logical volume group as well. Therefore, extend the virStorageBackendLogicalFindPoolSources function to return this information as well. This required pretty much a complete rewrite of the discovery function, since I needed to be able to add <device> sections to already existing XML nodes. For that reason, this code uses direct libxml2 tree API's to handle the parsing and the adding of nodes where necessary.

This really makes my eyes hurt. This is a great example of why we centralized all the XML parsing & formatting & introduced proper struct's everywhere. We need to take advantage of the stuff already provided in storage_conf.c/h. We have a struct to represent a single storage pool source, so we just need another to represent a list of them. eg typedef struct _virStoragePoolSourceList virStoragePoolSourceList; typedef virStoragePoolSourceList *virStoragePoolSourceListPtr; struct _virStoragePoolSourceList { unsigned int nsources; virStoragePoolSourcePtr sources; }; And a corresponding API char *virStoragePoolSourceListFormat(virConnectPtr conn, virStoragePoolSourceListPtr def); So, the virStorageBackendLogicalFindPoolSourcesFunc() would then just accept a virStoragePoolSourceListPtr() as the opaque param, and be able to nicely populate another virStoragePoolSourcePtr object in it. All the XML handling would be kept out of the driver implementation code. Regards, 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 :|