[libvirt] [PATCH v6 2/2] qemu: support to drop disk with 'optional' startupPolicy

Guannan Ren gren at redhat.com
Wed Aug 7 09:32:24 UTC 2013


On 08/06/2013 09:40 PM, Martin Kletzander wrote:
> On 08/02/2013 08:37 AM, Guannan Ren wrote:
>> Go through disks of guest, if one disk doesn't exist or its backing
>> chain is broken, with 'optional' startupPolicy, for CDROM and Floppy
>> we only discard its source path definition in xml, for disks we drop
>> it from disk list and free it.
>> ---
>>   include/libvirt/libvirt.h.in |  1 +
>>   src/qemu/qemu_domain.c       | 69 ++++++++++++++++++++++++++++++++------------
>>   2 files changed, 52 insertions(+), 18 deletions(-)
>>
>> diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
>> index 7bd3559..52ac95d 100644
>> --- a/include/libvirt/libvirt.h.in
>> +++ b/include/libvirt/libvirt.h.in
>> @@ -4727,6 +4727,7 @@ typedef void (*virConnectDomainEventBlockJobCallback)(virConnectPtr conn,
>>    */
>>   typedef enum {
>>       VIR_DOMAIN_EVENT_DISK_CHANGE_MISSING_ON_START = 0, /* oldSrcPath is set */
>> +    VIR_DOMAIN_EVENT_DISK_DROP_MISSING_ON_START = 1,
>>   
>>   #ifdef VIR_ENUM_SENTINELS
>>       VIR_DOMAIN_EVENT_DISK_CHANGE_LAST
>> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
>> index 1ff802c..6794e26 100644
>> --- a/src/qemu/qemu_domain.c
>> +++ b/src/qemu/qemu_domain.c
> [...]
>> @@ -2080,11 +2111,12 @@ qemuDomainCheckDiskPresence(virQEMUDriverPtr driver,
>>                               bool cold_boot)
>>   {
>>       int ret = -1;
>> -    size_t i;
>> +    ssize_t i;
>>       virDomainDiskDefPtr disk;
>> +    size_t ndisks = vm->def->ndisks;
>>   
>>       VIR_DEBUG("Checking for disk presence");
>> -    for (i = 0; i < vm->def->ndisks; i++) {
>> +    for (i = ndisks - 1; i >= 0; i--) {
>>           disk = vm->def->disks[i];
>>   
> Now I noticed that there should be either (1) a check for domain without
> any disks (if ndisks == 0; return 0) or (2) a different for loop, like
> this for example:
>
>      for (i = vm->def->ndisks; i > 0; i--) {
>          disk = vm->def->disks[i - 1];
>
>
> I know that I proposed the faulty version, sorry for that.  From those 2
> versions I like the second one better because it looks cleaner and you
> can skip first lines of this hunk.
>
> It'd be nice to have a test for this to make sure we won't break it in
> future.  At least xml2xml test if we can't test the disk dropping now.
>
> ACK with that fixed and this patch squashed in the previous one (also
> with those mentioned things fixed).
>
> Martin

pushed with these mentioned fixed.

Guannan




More information about the libvir-list mailing list