[libvirt] [PATCH v4 1/4] conf: add startupPolicy attribute for harddisk

[Martin Kletzander]

On 07/15/2013 05:01 PM, Guannan Ren wrote:
> On 07/15/2013 09:31 PM, Martin Kletzander wrote:
>> On 07/02/2013 11:35 AM, Guannan Ren wrote:
>>> Add startupPolicy attribute policy for harddisk with type "file",
>>> "block" and "dir". The "network" type disk is still not supported.
>>> ---
>>>   docs/schemas/domaincommon.rng |  6 ++++++
>>>   src/conf/domain_conf.c        | 20 +++++++++++++-------
>>>   2 files changed, 19 insertions(+), 7 deletions(-)
>>>
>> [...]
>>> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
>>> index 011de71..1040b40 100644
>>> --- a/src/conf/domain_conf.c
>>> +++ b/src/conf/domain_conf.c
>> [...]
>>> @@ -5410,11 +5410,10 @@
>>> virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt,
>>>               goto error;
>>>           }
>>>   -        if (def->device != VIR_DOMAIN_DISK_DEVICE_CDROM &&
>>> -            def->device != VIR_DOMAIN_DISK_DEVICE_FLOPPY) {
>>> +        if (def->type == VIR_DOMAIN_DISK_TYPE_NETWORK) {
>>>               virReportError(VIR_ERR_INVALID_ARG,
>> Pre-existing, but shouldn't this be VIR_ERR_XML_ERROR ?
>>
>> Otherwise the patch looks ok, but is missing documentation.  And in this
>> case it is important to have it there, I think.
>>
>> The possibility of specifying startupPolicy should be also documented
>> and it should be mentioned there that the whole disk will be dropped in
>> case this attribute is specified and the disk (or any other in it's
>> backing chain) is not found.  I'm trying to stress this out because for
>> cdrom and floppy disks we can safely drop the source only, it can be
>> plugged in even with old QEMU, OSes are used to that.  But with disks it
>> changes what is seen from the guest.  I believe that's also why
>> startupPolicy can be specified only for cdrom, floppy and USB hostdevs
>> for now.  This particular functionality calls for potentional problems
>> IMHO in case it is not properly documented.
>>
>> Also, 'requisite' for disks would mean that you have to hot-unplug the
>> disk if it needs to be removed due to migration because otherwise qemu
>> wouldn't start on the destination, or would it?
> 
> Yeah, the hot-unplugging disk is necessary in the case of 'requisite' if
> diskchains checking failed during migration.
> 
> 
>>
>> The more I'm staring into the code, the more I feel like this is not a
>> good idea and should be managed from upper application if this behavior
>> is required for disks.
> 
> I am not sure it is upper application's work. libvirt collected
> diskchain data for each disk at guest bootup already
> so it is easier to remove or hot-unplug a disk if backingchain is broken
> from libvirt point of view.
> upper application know nothing about backingchain for guest disks.
> 

I haven't explained myself correctly here.  In the last paragraph I was
talking about the whole 'startupPolicy for harddisk' approach, not only
about removing the disk on migration.

Martin