On Fri, Sep 25, 2009 at 02:27:32PM +0100, Mark McLoughlin wrote:
>
> +int virStorageFileProbeHeader(virConnectPtr conn,
> + const char *path,
> + int *format,
> + char **backingStore,
> + virStorageEncryptionPtr *encryption,
> + virStorageFilePermsPtr perms,
> + unsigned long long *allocation,
> + unsigned long long *capacity);
> +
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 :|
--
Libvir-list mailing list
Libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list