[libvirt] [PATCH 15/n] conf: use common struct in storage volumes

[Peter Krempa]

On 03/30/14 05:38, Eric Blake wrote:
> A fairly smooth transition.  And now that domain disks and
> storage volumes share a common struct, it opens the doors for
> a future patch to expose more details in the XML for both
> objects.
> 
> * src/conf/storage_conf.h (_virStorageVolTarget): Delete.
> (_virStorageVolDef): Use common type.
> * src/conf/storage_conf.c (virStorageVolDefFree)
> (virStorageVolTargetDefFormat): Update clients.
> * src/storage/storage_backend.h: Likewise.
> * src/storage/storage_backend.c
> (virStorageBackendUpdateVolTargetInfo)
> (virStorageBackendUpdateVolTargetInfoFD)
> (virStorageBackendDetectBlockVolFormatFD): Likewise.
> * src/storage/storage_backend_fs.c (virStorageBackendProbeTarget):
> Likewise.
> * src/storage/storage_backend_scsi.c
> (virStorageBackendSCSIUpdateVolTargetInfo): Likewise.
> * src/storage/storage_backend_mpath.c
> (virStorageBackendMpathUpdateVolTargetInfo): Likewise.
> 
> Signed-off-by: Eric Blake <eblake at redhat.com>
> ---
>  src/conf/storage_conf.c             | 20 +++-----------------
>  src/conf/storage_conf.h             | 23 ++---------------------
>  src/storage/storage_backend.c       |  6 +++---
>  src/storage/storage_backend.h       |  6 +++---
>  src/storage/storage_backend_fs.c    |  2 +-
>  src/storage/storage_backend_mpath.c |  4 ++--
>  src/storage/storage_backend_scsi.c  |  2 +-
>  7 files changed, 15 insertions(+), 48 deletions(-)
> 

...

> diff --git a/src/conf/storage_conf.h b/src/conf/storage_conf.h
> index eb09443..d16d679 100644
> --- a/src/conf/storage_conf.h
> +++ b/src/conf/storage_conf.h
> @@ -53,25 +53,6 @@ struct _virStorageVolSource {
>  };
> 
> 
> -/*
> - * How the volume appears on the host
> - */
> -typedef struct _virStorageVolTarget virStorageVolTarget;
> -typedef virStorageVolTarget *virStorageVolTargetPtr;
> -struct _virStorageVolTarget {
> -    char *path;
> -    int format; /* enum virStorageFileFormat */
> -    virStoragePermsPtr perms;
> -    virStorageTimestampsPtr timestamps;
> -    int partType; /* enum virStorageVolTypeDisk, only used by disk
> -                   * backend for partition type */
> -    /* The next three are currently only used in vol->target,
> -     * not in vol->backingStore. */
> -    virStorageEncryptionPtr encryption;
> -    virBitmapPtr features;
> -    char *compat;
> -};
> -
>  typedef struct _virStorageVolDef virStorageVolDef;
>  typedef virStorageVolDef *virStorageVolDefPtr;
>  struct _virStorageVolDef {
> @@ -85,8 +66,8 @@ struct _virStorageVolDef {
>      unsigned long long capacity; /* bytes */
> 
>      virStorageVolSource source;
> -    virStorageVolTarget target;
> -    virStorageVolTarget backingStore;
> +    virStorageSource target;

Hmmm, converting target to a source definition doesn't seem to be a good
idea to me:

* As in other parts of the code, the semantics of the target and source
differ in the data needed to store about those fields.

* Tracking of the target requires us to add additional data to the
struct which makes it to bloat and it's probable that those metadata
won't be used by other parts of the code.

* Other parts that are already present in the source structure are
irrelevant to the host representation, such as the network information.

I'd rather see those separated. It might seem that the structs carry
similar data but they are semantically different IMO.

> +    virStorageSource backingStore;

In case of the backing chain I think that the backing store is a
property of the source and not the target thus I'm fine with changing this.

>  };
> 
>  typedef struct _virStorageVolDefList virStorageVolDefList;

Peter

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 901 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20140331/9f464faf/attachment-0001.sig>