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.