[libvirt] [PATCH v2 2/2] Introduce and use virDomainDiskEmptySource

[Peter Krempa]

On Fri, Mar 31, 2017 at 16:02:14 +0200, Michal Privoznik wrote:
> Currently, if we want to zero out disk source (e,g, due to
> startupPolicy when starting up a domain) we use
> virDomainDiskSetSource(disk, NULL). This works well for file
> based storage (storage type file, dir, or block). But it doesn't
> work at all for other types like volume and network.
> 
> So imagine that you have a domain that has a CDROM configured
> which source is a volume from an inactive pool. Because it is
> startupPolicy='optional', the CDROM is empty when the domain
> starts. However, the source element is not cleared out in the
> status XML and thus when the daemon restarts and tries to
> reconnect to the domain it refreshes the disks (which fails - the
> storage pool is still not running) and thus the domain is killed.
> 
> Signed-off-by: Michal Privoznik <mprivozn at redhat.com>
> ---
> 
> diff to v1:
> - Free more struct entries
> - Set disk type
> - Call the function less frequently
> 
>  src/conf/domain_conf.c   | 33 +++++++++++++++++++++++++++++++++
>  src/conf/domain_conf.h   |  1 +
>  src/libvirt_private.syms |  1 +
>  src/qemu/qemu_domain.c   |  2 +-
>  src/qemu/qemu_process.c  |  2 +-
>  5 files changed, 37 insertions(+), 2 deletions(-)
> 
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 01553b5..f1f0b56 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -1719,6 +1719,39 @@ virDomainDiskSetSource(virDomainDiskDefPtr def, const char *src)
>  }
>  
>  
> +void
> +virDomainDiskEmptySource(virDomainDiskDefPtr def)
> +{
> +    virStorageSourcePtr src = def->src;
> +
> +    switch ((virStorageType) src->type) {
> +    case VIR_STORAGE_TYPE_DIR:
> +    case VIR_STORAGE_TYPE_FILE:
> +    case VIR_STORAGE_TYPE_BLOCK:
> +        VIR_FREE(src->path);
> +        break;
> +    case VIR_STORAGE_TYPE_NETWORK:
> +        VIR_FREE(src->path);
> +        VIR_FREE(src->volume);
> +        virStorageNetHostDefFree(src->nhosts, src->hosts);
> +        src->nhosts = 0;
> +        src->hosts = NULL;
> +        src->protocol = VIR_STORAGE_NET_PROTOCOL_NONE;
> +        break;
> +    case VIR_STORAGE_TYPE_VOLUME:
> +        virStorageSourcePoolDefFree(src->srcpool);
> +        src->srcpool = NULL;
> +        break;
> +    case VIR_STORAGE_TYPE_NONE:
> +    case VIR_STORAGE_TYPE_LAST:
> +        break;
> +    }
> +
> +    virStorageSourceBackingStoreClear(src);

I think that instead of all the above you should just call
virStorageSourceClear(). It also clears the format of the volume,
seclabels, encryption data and all other stuff which is relevant only
for the image itself.

> +    src->type = VIR_STORAGE_TYPE_FILE;

And then fix the type.

> +}
> +
> +
>  const char *
>  virDomainDiskGetDriver(virDomainDiskDefPtr def)
>  {

ACK with the change above. I think this should be safe for freeze.

Peter
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20170331/c9f79533/attachment-0001.sig>