[libvirt] [PATCH 2/2] Introduce and use virDomainDiskZeroSource

[Peter Krempa]

On Fri, Mar 31, 2017 at 13:13:51 +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>
> ---
>  src/conf/domain_conf.c   | 23 +++++++++++++++++++++++
>  src/conf/domain_conf.h   |  1 +
>  src/libvirt_private.syms |  1 +
>  src/qemu/qemu_domain.c   |  2 +-
>  src/qemu/qemu_process.c  |  2 +-
>  src/vmx/vmx.c            |  6 +++---
>  src/xenconfig/xen_xm.c   |  2 +-
>  7 files changed, 31 insertions(+), 6 deletions(-)
> 
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 01553b5..a60a456 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -1719,6 +1719,29 @@ virDomainDiskSetSource(virDomainDiskDefPtr def, const char *src)
>  }
>  
>  
> +void
> +virDomainDiskZeroSource(virDomainDiskDefPtr def)

virDomainDiskEmptySource perhaps?

> +{
> +    switch ((virStorageType) def->src->type) {
> +    case VIR_STORAGE_TYPE_DIR:
> +    case VIR_STORAGE_TYPE_FILE:
> +    case VIR_STORAGE_TYPE_BLOCK:
> +        VIR_FREE(def->src->path);
> +        break;
> +    case VIR_STORAGE_TYPE_NETWORK:
> +        VIR_FREE(def->src->volume);

This certainly is not enough to clear network disks. You also need to
clear the hosts and the path component. Resetting the protocol also
would be desired.

> +        break;
> +    case VIR_STORAGE_TYPE_VOLUME:
> +        virStorageSourcePoolDefFree(def->src->srcpool);
> +        def->src->srcpool = NULL;
> +        break;
> +    case VIR_STORAGE_TYPE_NONE:
> +    case VIR_STORAGE_TYPE_LAST:
> +        break;
> +    }

You also should reset the disk type. I'm not sure though wether setting
it to "NONE" is a good idea. "FILE" might be safer.

All of this needs to clear the backing chain too.

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

Usage looks good.
-------------- 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/d23f6912/attachment-0001.sig>