[pve-devel] update disk config, always rescan all the storage, can we avoid that ?

[Alexandre DERUMIER]

Works fine, thanks.
Patch sent !



Also, I was thinking to replace

"
PVE::Storage::foreach_volid($dl, sub {
             my ($volumeid) = @_;
             if($volumeid eq $volid) {
                     $foundvolid = 1;
                     return;
             }
});
die "image '$path' does not exists\n" if (!(-f $path || -b $path || $foundvolid));
"

By a specific storage plugin sub, like volume_exist.

- Nexenta,netapp for example, have api to check directly if the volume exist without need to parse all volumes. (listing all volumes take more time for theses storage)
This way, we could use the best/fastest method for each storage plugin.

What do you think about it ?

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

De: "Alexandre DERUMIER" <aderumier at odiso.com> 
À: "Dietmar Maurer" <dietmar at proxmox.com> 
Cc: pve-devel at pve.proxmox.com 
Envoyé: Mardi 4 Juin 2013 20:29:19 
Objet: Re: [pve-devel] update disk config, always rescan all the storage, can we avoid that ? 

>>Can we avoid the useless check by doing something like this: 


I'll try that tomorrow 


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

De: "Dietmar Maurer" <dietmar at proxmox.com> 
À: "Alexandre DERUMIER" <aderumier at odiso.com> 
Cc: pve-devel at pve.proxmox.com 
Envoyé: Mardi 4 Juin 2013 13:26:52 
Objet: RE: [pve-devel] update disk config, always rescan all the storage, can we avoid that ? 

Can we avoid the useless check by doing something like this: 

diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm 
index 24b57ad..111c731 100644 
--- a/PVE/API2/Qemu.pm 
+++ b/PVE/API2/Qemu.pm 
@@ -133,6 +133,15 @@ my $create_disks = sub { 
my $foundvolid = undef; 

if ($storeid) { 
+ 
+ my $volid_is_new = 1; 
+ if ($conf->{$ds}) { 
+ my $olddrive = PVE::QemuServer::parse_drive($ds, $conf->{$ds}); 
+ $volid_is_new = 0 if $olddrive->{file} && $olddrive->{file} eq $volid; 
+ } 
+ 
+ # fixme: avoid storage scans if $volid_is_new = 0 
+ 
PVE::Storage::activate_volumes($storecfg, [ $volid ]); 
my $dl = PVE::Storage::vdisk_list($storecfg, $storeid, undef); 

> > >>Yes. But otherwise $settings does not contain drives, so calling 
> > >>creat_disks shoukd have no effect? 
> > 
> > $settings in never empty, as we update the full disk config. 
> > 
> > example, to update bandwith limit: 
> > 
> > qm update --virtio0 local:1/vm-1-disk-1,cache=none,size=10G,mbps_rd=10 
> 
> ah, got it now - test cases really helps ;-) 
> 
> but I have no real idea how to implement that cleanly. 
> 
> Some time ago I though we can simplify the whole vm_update by restricting 
> the function to change only a single option (and provide a separate update 
> function for each option type), but I am not sure. 
_______________________________________________ 
pve-devel mailing list 
pve-devel at pve.proxmox.com 
http://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel