[pve-devel] [PATCH 2/5] add mountpoint_activate sub

[Alexandre DERUMIER]

I wonder if I could move the losetup code into  PVE::Storage::activate_volumes ?
(with an extra flag to enable/disable it)

Seem to be the right place ?



----- Mail original -----
De: "aderumier" <aderumier at odiso.com>
À: "dietmar" <dietmar at proxmox.com>
Cc: "pve-devel" <pve-devel at pve.proxmox.com>
Envoyé: Vendredi 21 Août 2015 11:18:58
Objet: Re: [pve-devel] [PATCH 2/5] add mountpoint_activate sub

>>For example, an naive developer want to call: 
>> 
>>mountpoint_activate('rootfs', ...) 
>> 
>>which does nothing for some reason? 

For this speficic example, I don't understand, 
It's always call activate_volumes , and maybe mount loop devices on some condition. 




>>Also, we want to activate/deactivate all volumes at once, because 
>> 
>>PVE::Storage::activate_volumes($storage_cfg, [...]); 
>> 
>>can take several $volids, and is optimized to activate each 
>>storage only once. 

ok, I can simply keep activate_volumes separate from losetup. 
I'll rework my patches. 

----- Mail original ----- 
De: "dietmar" <dietmar at proxmox.com> 
À: "aderumier" <aderumier at odiso.com>, "pve-devel" <pve-devel at pve.proxmox.com> 
Envoyé: Vendredi 21 Août 2015 11:07:54 
Objet: Re: [pve-devel] [PATCH 2/5] add mountpoint_activate sub 

I do not like this whole series, because you define 
subroutines with unexpected side effects. 

For example, an naive developer want to call: 

mountpoint_activate('rootfs', ...) 

which does nothing for some reason? 

You should really do those checks somewhere else. 

Also, we want to activate/deactivate all volumes at once, because 

PVE::Storage::activate_volumes($storage_cfg, [...]); 

can take several $volids, and is optimized to activate each 
storage only once. 
_______________________________________________ 
pve-devel mailing list 
pve-devel at pve.proxmox.com 
http://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel