[libvirt] [PATCH 2/2] Introduce and use virDomainDiskZeroSource
Michal Privoznik
mprivozn at redhat.com
Fri Mar 31 13:03:35 UTC 2017
On 03/31/2017 02:18 PM, Peter Krempa wrote:
> 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.
Fair enough.
>
>> + 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.
This doesn't sound fair. If the original disk was type of volume, why it
should all of a sudden be reported as type of file?
If we are afraid of other areas of the code failing or being unprepared
for, I say have them fail so that they can be fixed.
Michal
More information about the libvir-list
mailing list