Re: [PATCH qemu-server 2/4] fix #8209: clone disk: do not ignore failure to query size of target

[Fiona Ebner <f.ebner@proxmox.com>]

Am 28.10.26 um 10:05 AM schrieb Fiona Ebner:
> Am 22.10.26 um 3:25 PM schrieb Elias Huhsovitz:
>> On Thu Oct 22, 2026 at 11:38 AM CEST, Fiona Ebner wrote:
>>> @@ -4997,9 +4998,15 @@ __PACKAGE__->register_method({
>>>                      $conf->{agent},
>>>                      $movelimit,
>>>                  );
>>> +                my $newdrive = $clone_result->{drive};
>>>                  $conf->{$disk} = PVE::QemuServer::print_drive($newdrive);
>>>  
>>> -                if (!$param->{delete}) {
>>> +                if ($do_delete && $clone_result->{'after-copy-error'}) {
>>> +                    warn "skipping delete of source because of error after copy\n";
>>> +                    $do_delete = 0;
>>
>> Since we now set $clone_result->{'after-copy-error'}, if e.g., the
>> target NFS share returns an error, we set $do_delete = 0 to prevent the
>> source disk from being deleted.
>>
>>> +                }
>>> +
>>> +                if (!$do_delete) {
>>>                      my $unused_key = PVE::QemuConfig->add_unused_volume($conf, $old_volid);
>>
>> So now in the case that $clone_result->{'after-copy-error'} is set, we
>> always call add_unused_volume(). But add_unused_volume() can fail with
>>
>> "Too many unused volumes - please delete them first.\n"
>>
>> according to AbstractConfig::add_unused_volume()
>>
>> Couldn't this kill the disk during a live migration, if we are unable to
>> add the unused volume?
>>
>> In Drive.pm the maximum number for unused disks is set to:
>>
>> our $MAX_UNUSED_DISKS = 256;
>>
>> IMO being unable to add a new disk in a production system, might be a
>> real possibility.
> 
> It's not a new issue (my patch only changes that 'delete' is not used in
> a rare error scenario, delete=0 was already possible before). I'm not
> aware of any reports about people running into this. It should be quite
> rare to reach that limit in practice. Still, it's an issue and I'll add
> a patch to fix it in v2.
> 
> And the issue is also present in case the template_create() call fails,
> I'll guard that too.

Ah no, since the VM can't be running if it's a template ;)