Re: [libvirt] [PATCH 6/7] Move virStorageBackendProbeTarget to libvirt_util

On Fri, Sep 25, 2009 at 02:27:32PM +0100, Mark McLoughlin wrote: This function prototype is the root cause of most of my objections earlier in the series. The way it has evolved over time since I first wrote it has resulted in sub-optimal separation of responsibilities. In particular I don't think this method should be responsible for populating the virStorageEncryptionPtr or virStorageFilePermsPtr structures. It should focus itself to only be concerned with extracting metadata from the file header. The callers can later populate the higher level structures like virStorageFilePermsPtr & virStorageEncryptionPtr as they see fit. For an API in the util/ library I think it'd be reasonable to aim for an simpler API such as int virStorageFileGetMetadata(virConnectPtr conn, const char *path, int *format, char **backingStore, boolean *encrypted unsigned long long *allocation, unsigned long long *capacity); The backingStore, encrypted, allocation & capacity fields would only be filed in for format != VIR_STORAGE_FILE_RAW. There's probably enough return parameters here to justify inventing a small (caller allocated) struct for the returned metadata, such as typedef struct _virStorageFileMetadata { int format; char *backingStore; boolean encrypted; unsigned long long allocation; unsigned long long capacty; } virStorageFileMetadata; So callers would do virStorageFileMetadata meta; memset(&meta, 0, sizeof meta); virStorageFileGetMetadata(conn, path, &meta); The original virStorageBackendProbeTarget() method in storage_backend_fs.c could then be written in terms of a combination of calls to the virStorageFileGetMetadata() and virStorageBackendUpdateVolTargetInfo() methods. This would avoid the need to move most of the code in this series. 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 :|