[pve-devel] [PATCH 8/8] add_unused_volume corrections.

[Alexandre DERUMIER]

Hi Dietmar, thanks for the commit.


>> Please can you post a example to reproduce the bug? 
About the last patch, try this:


qm set XXX -delete virtio1,virtio2


expected result:

unused0: local:XXX/vm-167-disk-1.raw
unused1: local:XXX/vm-167-disk-2.raw

actual result:

unused0: local:167/vm-167-disk-2.raw


that's because we search free unusedX in config file each time

"
   my ($config, $res, $volid) = @_;

    my $key;
    for (my $ind = $MAX_UNUSED_DISKS - 1; $ind >= 0; $ind--) {
        my $test = "unused$ind";
        if (my $vid = $config->{$test}) {
"

So when we deleted more than 1 device, it's always returning the same unusedX, as $config is not yet updated.
Is it clear for you ?


>> Besides, I think we should simplify the update_vm code. 

Yeesss, I agree with that, I have had difficulties to read it the first time ;)


>>The current API tries to guarantee atomic updates if you change multiple 
>>values. I guess That is simply not possible because you can't guarantee 
>>that revert/undo works (especially when we use hotplug). 
Indeed, we can't guarantee revert. (It's works 99% time, but murphy law .....)


>> I suggest that we split updates into single changes, and save the config 
>> file after each changes. On error, we simple stop. I guess we can simplify 
>> the code that way. 
>> What do you think (AFAIR you already suggested that)? 

Yes, it'll be me more simple. (1 prefer 1 loop than many loops)


I can code if you want, I had time for the moment.


----- Mail original ----- 

De: "Dietmar Maurer" <dietmar at proxmox.com> 
À: "Derumier Alexandre" <aderumier at odiso.com>, pve-devel at pve.proxmox.com 
Envoyé: Lundi 23 Janvier 2012 09:34:51 
Objet: RE: [pve-devel] [PATCH 8/8] add_unused_volume corrections. 

Hi Alexandre, 

just applied patch 1/8-7/8, but I do not understand this one: 

> -----Original Message----- 
> From: pve-devel-bounces at pve.proxmox.com [mailto:pve-devel- 
> bounces at pve.proxmox.com] On Behalf Of Derumier Alexandre 
> Sent: Freitag, 20. Jänner 2012 11:42 
> To: pve-devel at pve.proxmox.com 
> Subject: [pve-devel] [PATCH 8/8] add_unused_volume corrections. 
> 
> Currentle add_unused_volume not working when delete more than 1 device. 
> 
> As it reading the vm conf file each time, it always assign the same id to each 
> delete device. 

We only load the config file once in PVE/API2/Qemu.pm line 495? 

Please can you post a example to reproduce the bug? 

Besides, I think we should simplify the update_vm code. 

The current API tries to guarantee atomic updates if you change multiple 
values. I guess That is simply not possible because you can't guarantee 
that revert/undo works (especially when we use hotplug). 

I suggest that we split updates into single changes, and save the config 
file after each changes. On error, we simple stop. I guess we can simplify 
the code that way. 

What do you think (AFAIR you already suggested that)? 

- Dietmar 








-- 

-- 




	
	Alexandre Derumier 
Ingénieur système 
e-mail : aderumier at odiso.com 
Tél : +33 (0)3 20 68 88 90 
Fax : +33 (0)3 20 68 90 81 
45 Bvd du Général Leclerc 
59100 ROUBAIX - FRANCE 













-------------- next part --------------
A non-text attachment was scrubbed...
Name: aderumier.vcf
Type: text/x-vcard
Size: 183 bytes
Desc: not available
URL: <http://lists.proxmox.com/pipermail/pve-devel/attachments/20120124/25da5771/attachment.vcf>