[libvirt] [PATCH 12/n] conf: let snapshots share disk source struct

[Peter Krempa]

On 03/29/14 23:20, Eric Blake wrote:
> Now that we have a common struct, it's time to start using it!
> Since external snapshots make a longer backing chain, it is
> only natural to use the same struct for the file created by
> the snapshot as what we use for <domain> disks.
> 
> * src/conf/snapshot_conf.h (_virDomainSnapshotDiskDef): Use common
> struct instead of open-coded duplicate fields.
> * src/conf/snapshot_conf.c (virDomainSnapshotDiskDefClear)
> (virDomainSnapshotDiskDefParseXML, virDomainSnapshotAlignDisks)
> (virDomainSnapshotDiskDefFormat)
> (virDomainSnapshotDiskGetActualType): Adjust clients.
> * src/qemu/qemu_conf.c (qemuTranslateSnapshotDiskSourcePool):
> Likewise.
> * src/qemu/qemu_driver.c (qemuDomainSnapshotDiskGetSourceString)
> (qemuDomainSnapshotCreateInactiveExternal)
> (qemuDomainSnapshotPrepareDiskExternalOverlayActive)
> (qemuDomainSnapshotPrepareDiskExternal)
> (qemuDomainSnapshotPrepare)
> (qemuDomainSnapshotCreateSingleDiskActive): Likewise.
> * src/storage/storage_driver.c
> (virStorageFileInitFromSnapshotDef): Likewise.
> 
> Signed-off-by: Eric Blake <eblake at redhat.com>
> ---
>  src/conf/snapshot_conf.c     | 68 +++++++++++++++++------------------
>  src/conf/snapshot_conf.h     |  8 ++---
>  src/qemu/qemu_conf.c         |  2 +-
>  src/qemu/qemu_driver.c       | 86 ++++++++++++++++++++++----------------------
>  src/storage/storage_driver.c |  8 ++---
>  5 files changed, 85 insertions(+), 87 deletions(-)
> 
> diff --git a/src/conf/snapshot_conf.c b/src/conf/snapshot_conf.c
> index ebb3bb4..3bd4732 100644


> @@ -632,16 +632,16 @@ virDomainSnapshotDiskDefFormat(virBufferPtr buf,
>      virBufferAsprintf(buf, " type='%s'>\n", virStorageTypeToString(type));
>      virBufferAdjustIndent(buf, 2);
> 
> -    if (disk->format > 0)
> +    if (disk->src.format > 0)
>          virBufferEscapeString(buf, "<driver type='%s'/>\n",
> -                              virStorageFileFormatTypeToString(disk->format));
> +                              virStorageFileFormatTypeToString(disk->src.format));
>      virDomainDiskSourceDefFormatInternal(buf,

Hmm, now that we have a common struct, we can unify the source formatter
and have it accept the common struct instead of splitting out separate
fields ...

>                                           type,
> -                                         disk->file,
> +                                         disk->src.path,
>                                           0,
> -                                         disk->protocol,
> -                                         disk->nhosts,
> -                                         disk->hosts,
> +                                         disk->src.protocol,
> +                                         disk->src.nhosts,
> +                                         disk->src.hosts,
>                                           0, NULL, NULL, 0);
> 
>      virBufferAdjustIndent(buf, -2);


> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 9d48a46..9a2de12 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -12028,10 +12028,10 @@ qemuDomainSnapshotDiskGetSourceString(virDomainSnapshotDiskDefPtr disk,
>      *source = NULL;
> 
>      return qemuGetDriveSourceString(virDomainSnapshotDiskGetActualType(disk),

same here. This function can now be refactored to take the struct
instead of the fields separately.

> -                                    disk->file,
> -                                    disk->protocol,
> -                                    disk->nhosts,
> -                                    disk->hosts,
> +                                    disk->src.path,
> +                                    disk->src.protocol,
> +                                    disk->src.nhosts,
> +                                    disk->src.hosts,
>                                      NULL,
>                                      NULL,
>                                      source);
> 

> @@ -12874,9 +12876,9 @@ qemuDomainSnapshotCreateSingleDiskActive(virQEMUDriverPtr driver,
> 
>      disk->src.path = newsource;
>      disk->src.format = format;
> -    disk->src.type = snap->type;
> -    disk->src.protocol = snap->protocol;
> -    disk->src.nhosts = snap->nhosts;
> +    disk->src.type = snap->src.type;
> +    disk->src.protocol = snap->src.protocol;
> +    disk->src.nhosts = snap->src.nhosts;
>      disk->src.hosts = newhosts;

And here we can introduce a function to copy the source instead of doing
it manually.

> 
>      newsource = NULL;
> @@ -12889,9 +12891,9 @@ qemuDomainSnapshotCreateSingleDiskActive(virQEMUDriverPtr driver,
> 
>          persistDisk->src.path = persistSource;
>          persistDisk->src.format = format;
> -        persistDisk->src.type = snap->type;
> -        persistDisk->src.protocol = snap->protocol;
> -        persistDisk->src.nhosts = snap->nhosts;
> +        persistDisk->src.type = snap->src.type;
> +        persistDisk->src.protocol = snap->src.protocol;
> +        persistDisk->src.nhosts = snap->src.nhosts;
>          persistDisk->src.hosts = persistHosts;
> 
>          persistSource = NULL;


 diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
> index cdb8536..6fca3d2 100644
> --- a/src/storage/storage_driver.c
> +++ b/src/storage/storage_driver.c
> @@ -2802,10 +2802,10 @@ virStorageFilePtr
>  virStorageFileInitFromSnapshotDef(virDomainSnapshotDiskDefPtr disk)
>  {
>      return virStorageFileInitInternal(virDomainSnapshotDiskGetActualType(disk),
> -                                      disk->file,
> -                                      disk->protocol,
> -                                      disk->nhosts,
> -                                      disk->hosts);
> +                                      disk->src.path,
> +                                      disk->src.protocol,
> +                                      disk->src.nhosts,
> +                                      disk->src.hosts);
>  }
> 
> 

ACK to the patch. The proposed changes are definitely separable and
better to review separately.

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/3d5e6306/attachment-0001.sig>