On Fri May 24, 2024 at 3:22 PM CEST, Dominik Csapak wrote: > this is to override the target extraction storage for the option disk > extraction for 'import-from'. This way if the storage does not > supports the content type 'images', one can give an alternative one. > > Signed-off-by: Dominik Csapak <d.csa...@proxmox.com> > --- > PVE/API2/Qemu.pm | 46 +++++++++++++++++++++++++++++++++++++--------- > 1 file changed, 37 insertions(+), 9 deletions(-) > > diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm > index 8c335ac4..80ea52c5 100644 > --- a/PVE/API2/Qemu.pm > +++ b/PVE/API2/Qemu.pm > @@ -128,7 +128,7 @@ my $check_drive_param = sub { > }; > > my $check_storage_access = sub { > - my ($rpcenv, $authuser, $storecfg, $vmid, $settings, $default_storage) = > @_; > + my ($rpcenv, $authuser, $storecfg, $vmid, $settings, $default_storage, > $extraction_storage) = @_; > > $foreach_volume_with_alloc->($settings, sub { > my ($ds, $drive) = @_; > @@ -169,9 +169,18 @@ my $check_storage_access = sub { > if $vtype ne 'images' && $vtype ne 'import'; > > if (PVE::QemuServer::Helpers::needs_extraction($vtype, $fmt)) { > - raise_param_exc({ $ds => "$src_image is not on an storage > with 'images' content type."}) > - if !$scfg->{content}->{images}; > - $rpcenv->check($authuser, "/storage/$storeid", > ['Datastore.AllocateSpace']); > + if (defined($extraction_storage)) { > + my $extraction_scfg = > PVE::Storage::storage_config($storecfg, $extraction_storage); > + raise_param_exc({ 'import-extraction-storage' => > "$extraction_storage does not support" > + ." 'images' content type or is not file > based."})
Is there perhaps a way to display "Disk images" like in the storage config drop down menu? Also, this is where the error handling could be more fine-grained so the user immediately knows what's wrong, as mentioned in my response to your cover letter. > + if !$extraction_scfg->{content}->{images} || > !$extraction_scfg->{path}; Style: The `raise_param_exc` plus `if` here should IMO be indented like you did ... > + $rpcenv->check($authuser, > "/storage/$extraction_storage", ['Datastore.AllocateSpace']); > + } else { > + raise_param_exc({ $ds => "$src_image is not on an > storage with 'images'" s/an storage/a storage > + ." content type and no 'import-extraction-storage' > was given."}) > + if !$scfg->{content}->{images}; ... here. > + $rpcenv->check($authuser, "/storage/$storeid", > ['Datastore.AllocateSpace']); > + } > } > } > > @@ -326,7 +335,7 @@ my $import_from_volid = sub { > > # Note: $pool is only needed when creating a VM, because pool permissions > # are automatically inherited if VM already exists inside a pool. > -my sub create_disks : prototype($$$$$$$$$$) { > +my sub create_disks : prototype($$$$$$$$$$$) { > my ( > $rpcenv, > $authuser, > @@ -338,6 +347,7 @@ my sub create_disks : prototype($$$$$$$$$$) { > $settings, > $default_storage, > $is_live_import, > + $extraction_storage, > ) = @_; > > my $vollist = []; > @@ -407,8 +417,8 @@ my sub create_disks : prototype($$$$$$$$$$) { > my $needs_extraction = > PVE::QemuServer::Helpers::needs_extraction($vtype, $fmt); > if ($needs_extraction) { > print "extracting $source\n"; > - my $extracted_volid > - = > PVE::GuestImport::extract_disk_from_import_file($source, $vmid); > + my $extracted_volid = > PVE::GuestImport::extract_disk_from_import_file( > + $source, $vmid, $extraction_storage); > print "finished extracting to $extracted_volid\n"; > push @$vollist, $extracted_volid; > $source = $extracted_volid; > @@ -941,6 +951,12 @@ __PACKAGE__->register_method({ > default => 0, > description => "Start VM after it was created > successfully.", > }, > + 'import-extraction-storage' => > get_standard_option('pve-storage-id', { > + description => "Storage for temporarily extracted images > 'import-from' image" > + ." files (default: import source storage)", > + optional => 1, > + completion => \&PVE::QemuServer::complete_storage, > + }), > }, > 1, # with_disk_alloc > ), > @@ -967,6 +983,7 @@ __PACKAGE__->register_method({ > my $storage = extract_param($param, 'storage'); > my $unique = extract_param($param, 'unique'); > my $live_restore = extract_param($param, 'live-restore'); > + my $extraction_storage = extract_param($param, > 'import-extraction-storage'); > > if (defined(my $ssh_keys = $param->{sshkeys})) { > $ssh_keys = URI::Escape::uri_unescape($ssh_keys); > @@ -1026,7 +1043,8 @@ __PACKAGE__->register_method({ > if (scalar(keys $param->%*) > 0) { > &$resolve_cdrom_alias($param); > > - &$check_storage_access($rpcenv, $authuser, $storecfg, $vmid, > $param, $storage); > + &$check_storage_access( > + $rpcenv, $authuser, $storecfg, $vmid, $param, $storage, > $extraction_storage); > > &$check_vm_modify_config_perm($rpcenv, $authuser, $vmid, $pool, [ > keys %$param]); > > @@ -1141,6 +1159,7 @@ __PACKAGE__->register_method({ > $param, > $storage, > $live_restore, > + $extraction_storage > ); > $conf->{$_} = $created_opts->{$_} for keys > $created_opts->%*; > > @@ -1683,6 +1702,8 @@ my $update_vm_api = sub { > > my $skip_cloud_init = extract_param($param, 'skip_cloud_init'); > > + my $extraction_storage = extract_param($param, > 'import-extraction-storage'); > + > if (defined(my $cipassword = $param->{cipassword})) { > # Same logic as in cloud-init (but with the regex fixed...) > $param->{cipassword} = PVE::Tools::encrypt_pw($cipassword) > @@ -1802,7 +1823,7 @@ my $update_vm_api = sub { > > &$check_vm_modify_config_perm($rpcenv, $authuser, $vmid, undef, [keys > %$param]); > > - &$check_storage_access($rpcenv, $authuser, $storecfg, $vmid, $param); > + &$check_storage_access($rpcenv, $authuser, $storecfg, $vmid, $param, > undef, $extraction_storage); Style: Should perhaps be indented like above: &$check_storage_access( $rpcenv, $authuser, $storecfg, $vmid, $param, $storage, $extraction_storage); > > PVE::QemuServer::check_bridge_access($rpcenv, $authuser, $param); > > @@ -1984,6 +2005,7 @@ my $update_vm_api = sub { > {$opt => $param->{$opt}}, > undef, > undef, > + $extraction_storage, > ); > $conf->{pending}->{$_} = $created_opts->{$_} for keys > $created_opts->%*; > > @@ -2179,6 +2201,12 @@ __PACKAGE__->register_method({ > maximum => 30, > optional => 1, > }, > + 'import-extraction-storage' => > get_standard_option('pve-storage-id', { > + description => "Storage for temporarily extracted images > 'import-from' image" > + ." files (default: import source storage)", > + optional => 1, > + completion => \&PVE::QemuServer::complete_storage, > + }), > }, > 1, # with_disk_alloc > ), _______________________________________________ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel