[pve-devel] [PATCH qemu-server v2 14/15] api: migrate_vm: use volume content type assertion helpers

[Fiona Ebner]

Am 20.02.25 um 18:50 schrieb Daniel Kral:
> On 2/20/25 15:46, Fiona Ebner wrote:
>> Am 11.02.25 um 17:08 schrieb Daniel Kral:
>>> diff --git a/PVE/QemuMigrate.pm b/PVE/QemuMigrate.pm
>>> index ed5ede30..827e54b7 100644
>>> --- a/PVE/QemuMigrate.pm
>>> +++ b/PVE/QemuMigrate.pm
>>> @@ -158,15 +158,13 @@ sub target_storage_check_available {
>>>       my ($self, $storecfg, $targetsid, $volid) = @_;
>>>         if (!$self->{opts}->{remote}) {
>>> -    # check if storage is available on target node
>>> -    my $target_scfg = PVE::Storage::storage_check_enabled(
>>> -        $storecfg,
>>> -        $targetsid,
>>> -        $self->{node},
>>> -    );
>>> -    my ($vtype) = PVE::Storage::parse_volname($storecfg, $volid);
>>> -    die "$volid: content type '$vtype' is not available on storage
>>> '$targetsid'\n"
>>> -        if !$target_scfg->{content}->{$vtype};
>>> +    # check if storage is available on target node and supports the
>>> volume's content type
>>> +    eval {
>>> +        PVE::Storage::storage_check_enabled($storecfg, $targetsid,
>>> $self->{node});
>>> +        my ($vtype) = PVE::Storage::parse_volname($storecfg, $volid);
>>> +        PVE::Storage::assert_content_type_supported($storecfg,
>>> $targetsid, $vtype);
>>
>> (In v3) this needs to pass along the node. And could use
>> assert_volume_type_supported() or?
> 
> Except I'm missing something, in this case not, because we do want to
> check whether the target storage $targetsid supports the content type,
> but `assert_volume_type_supported` would check whether the current
> storage of the volume supports the content type, not the target one.

Right :) Would be nice to have a short code comment for this to avoid
the question in the future.