[pve-devel] [PATCH qemu-server 1/1] fix 1734: clone VM: if deactivation fails demote error to warning

[Fiona Ebner]

Am 06.03.24 um 14:14 schrieb Friedrich Weber:
> On 06/03/2024 13:40, Fiona Ebner wrote:
>> Am 06.03.24 um 11:47 schrieb Hannes Duerr:
>>> @@ -3820,7 +3821,13 @@ __PACKAGE__->register_method({
>>>  
>>>  		if ($target) {
>>>  		    # always deactivate volumes - avoid lvm LVs to be active on several nodes
>>> -		    PVE::Storage::deactivate_volumes($storecfg, $vollist, $snapname) if !$running;
>>> +		    eval {
>>> +			PVE::Storage::deactivate_volumes($storecfg, $vollist, $snapname) if !$running;
>>> +		    };
>>> +		    my $err = $@;
>>> +		    if ($err) {
>>> +			log_warn("$err\n");
>>> +		    }
>>>  		    PVE::Storage::deactivate_volumes($storecfg, $newvollist);
>>
>> We might also want to catch errors here. Otherwise, the whole clone
>> operation (which might've taken hours) can still fail just because of a
>> deactivation error. But when failing here, we shouldn't move the config
>> file (or the LV can get active on multiple nodes more easily).
> 
> I think succeeding but not moving the config file when deactivating
> $newvollist fails sounds like it could lead to unexpected behavior.
> Right now, when running `qm clone 101 [...] --target node2` on node1
> succeeds, one can be sure there will be an VM 101 on node2. But if we
> cannot deactivate $newvollist and thus don't move the config file, the
> command succeeds but VM 101 instead exists on node1 (correct me if I'm
> wrong), which may be confusing e.g. if the clone is automated.
> 

Yes, but the question is what is worse: Needing to re-do the clone or
having the VM config on the wrong node?

> To avoid that, I'd lean towards keeping the behavior of failing the task
> if deactivating $newvollist fails. After all, at least in case of LVM
> not being able to deactivate because the device is in use, we just
> created $newvollist so hopefully nobody else should be accessing it.

Fine by me. Yes, it's unlikely to fail. And we can still adapt later if
users ever complain about it.