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

[Guannan Ren]

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.