From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) by lore.proxmox.com (Postfix) with ESMTPS id E47D41FF396 for ; Wed, 22 May 2024 11:24:20 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 3F5829463; Wed, 22 May 2024 11:24:37 +0200 (CEST) Date: Wed, 22 May 2024 11:24:29 +0200 From: Fabian =?iso-8859-1?q?Gr=FCnbichler?= To: Proxmox VE development discussion References: <20240429112124.3819357-1-d.csapak@proxmox.com> <20240429112124.3819357-3-d.csapak@proxmox.com> In-Reply-To: <20240429112124.3819357-3-d.csapak@proxmox.com> MIME-Version: 1.0 User-Agent: astroid/0.16.0 (https://github.com/astroidmail/astroid) Message-Id: <1716368200.rpvan7m9t9.astroid@yuna.none> X-SPAM-LEVEL: Spam detection results: 0 AWL 0.054 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record URIBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to URIBL was blocked. See http://wiki.apache.org/spamassassin/DnsBlocklists#dnsbl-block for more information. [test.foo, dirplugin.pm, proxmox.com, plugin.pm, ovf.pm, storage.pm] Subject: Re: [pve-devel] [PATCH storage v3 02/10] plugin: dir: implement import content type X-BeenThere: pve-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox VE development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: Proxmox VE development discussion Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: pve-devel-bounces@lists.proxmox.com Sender: "pve-devel" On April 29, 2024 1:21 pm, Dominik Csapak wrote: > in DirPlugin and not Plugin (because of cyclic dependency of > Plugin -> OVF -> Storage -> Plugin otherwise) > > only ovf is currently supported (though ova will be shown in import > listing), expects the files to not be in a subdir, and adjacent to the > ovf file. > > listed will be all ovf/qcow2/raw/vmdk files. > ovf because it can be imported, and the rest because they can be used > in the 'import-from' part of qemu-server. > > Signed-off-by: Dominik Csapak > --- > src/PVE/GuestImport/OVF.pm | 3 +++ > src/PVE/Storage.pm | 8 +++++++ > src/PVE/Storage/DirPlugin.pm | 36 +++++++++++++++++++++++++++++- > src/PVE/Storage/Plugin.pm | 11 ++++++++- > src/test/parse_volname_test.pm | 18 +++++++++++++++ > src/test/path_to_volume_id_test.pm | 21 +++++++++++++++++ > 6 files changed, 95 insertions(+), 2 deletions(-) > > diff --git a/src/PVE/GuestImport/OVF.pm b/src/PVE/GuestImport/OVF.pm > index 055ebf5..0eb5e9c 100644 > --- a/src/PVE/GuestImport/OVF.pm > +++ b/src/PVE/GuestImport/OVF.pm > @@ -222,6 +222,8 @@ ovf:Item[rasd:InstanceID='%s']/rasd:ResourceType", $controller_id); > } > > ($backing_file_path) = $backing_file_path =~ m|^(/.*)|; # untaint > + ($filepath) = $filepath =~ m|^(${PVE::Storage::SAFE_CHAR_CLASS_RE}+)$|; # untaint & check no sub/parent dirs > + die "invalid path\n" if !$filepath; > > my $virtual_size = PVE::Storage::file_size_info($backing_file_path); > die "error parsing $backing_file_path, cannot determine file size\n" > @@ -231,6 +233,7 @@ ovf:Item[rasd:InstanceID='%s']/rasd:ResourceType", $controller_id); > disk_address => $pve_disk_address, > backing_file => $backing_file_path, > virtual_size => $virtual_size > + relative_path => $filepath, syntax error here (cleaned up in next patch) > }; > push @disks, $pve_disk; > > diff --git a/src/PVE/Storage.pm b/src/PVE/Storage.pm > index f19a115..1ed91c2 100755 > --- a/src/PVE/Storage.pm > +++ b/src/PVE/Storage.pm > @@ -114,6 +114,10 @@ our $VZTMPL_EXT_RE_1 = qr/\.tar\.(gz|xz|zst)/i; > > our $BACKUP_EXT_RE_2 = qr/\.(tgz|(?:tar|vma)(?:\.(${\PVE::Storage::Plugin::COMPRESSOR_RE}))?)/; > > +our $IMPORT_EXT_RE_1 = qr/\.(ovf|qcow2|raw|vmdk)/; > + > +our $SAFE_CHAR_CLASS_RE = qr/[a-zA-Z0-9\-\.\+\=\_]/; > + > # FIXME remove with PVE 8.0, add versioned breaks for pve-manager > our $vztmpl_extension_re = $VZTMPL_EXT_RE_1; > > @@ -612,6 +616,7 @@ sub path_to_volume_id { > my $backupdir = $plugin->get_subdir($scfg, 'backup'); > my $privatedir = $plugin->get_subdir($scfg, 'rootdir'); > my $snippetsdir = $plugin->get_subdir($scfg, 'snippets'); > + my $importdir = $plugin->get_subdir($scfg, 'import'); > > if ($path =~ m!^$imagedir/(\d+)/([^/\s]+)$!) { > my $vmid = $1; > @@ -640,6 +645,9 @@ sub path_to_volume_id { > } elsif ($path =~ m!^$snippetsdir/([^/]+)$!) { > my $name = $1; > return ('snippets', "$sid:snippets/$name"); > + } elsif ($path =~ m!^$importdir/(${SAFE_CHAR_CLASS_RE}+${IMPORT_EXT_RE_1})$!) { > + my $name = $1; > + return ('import', "$sid:import/$name"); > } > } > > diff --git a/src/PVE/Storage/DirPlugin.pm b/src/PVE/Storage/DirPlugin.pm > index 2efa8d5..3e3b1e7 100644 > --- a/src/PVE/Storage/DirPlugin.pm > +++ b/src/PVE/Storage/DirPlugin.pm > @@ -10,6 +10,7 @@ use IO::File; > use POSIX; > > use PVE::Storage::Plugin; > +use PVE::GuestImport::OVF; > use PVE::JSONSchema qw(get_standard_option); > > use base qw(PVE::Storage::Plugin); > @@ -22,7 +23,7 @@ sub type { > > sub plugindata { > return { > - content => [ { images => 1, rootdir => 1, vztmpl => 1, iso => 1, backup => 1, snippets => 1, none => 1 }, > + content => [ { images => 1, rootdir => 1, vztmpl => 1, iso => 1, backup => 1, snippets => 1, none => 1, import => 1 }, > { images => 1, rootdir => 1 }], > format => [ { raw => 1, qcow2 => 1, vmdk => 1, subvol => 1 } , 'raw' ], > }; > @@ -247,4 +248,37 @@ sub check_config { > return $opts; > } > > +sub get_import_metadata { > + my ($class, $scfg, $volname, $storeid) = @_; > + > + my ($vtype, $name, undef, undef, undef, undef, $fmt) = $class->parse_volname($volname); > + die "invalid content type '$vtype'\n" if $vtype ne 'import'; > + die "invalid format\n" if $fmt ne 'ova' && $fmt ne 'ovf'; > + > + # NOTE: all types of warnings must be added to the return schema of the import-metadata API endpoint > + my $warnings = []; > + > + my $path = $class->path($scfg, $volname, $storeid, undef); > + my $res = PVE::GuestImport::OVF::parse_ovf($path); > + my $disks = {}; > + for my $disk ($res->{disks}->@*) { > + my $id = $disk->{disk_address}; > + my $size = $disk->{virtual_size}; > + my $path = $disk->{relative_path}; > + $disks->{$id} = { > + volid => "$storeid:import/$path", > + defined($size) ? (size => $size) : (), > + }; > + } > + > + return { > + type => 'vm', > + source => $volname, > + 'create-args' => $res->{qm}, > + 'disks' => $disks, > + warnings => $warnings, > + net => [], > + }; > +} > + > 1; > diff --git a/src/PVE/Storage/Plugin.pm b/src/PVE/Storage/Plugin.pm > index 22a9729..33f0f3a 100644 > --- a/src/PVE/Storage/Plugin.pm > +++ b/src/PVE/Storage/Plugin.pm > @@ -654,6 +654,8 @@ sub parse_volname { > return ('backup', $fn); > } elsif ($volname =~ m!^snippets/([^/]+)$!) { > return ('snippets', $1); > + } elsif ($volname =~ m!^import/(${PVE::Storage::SAFE_CHAR_CLASS_RE}+$PVE::Storage::IMPORT_EXT_RE_1)$!) { > + return ('import', $1, undef, undef, undef, undef, $2); > } > > die "unable to parse directory volume name '$volname'\n"; > @@ -666,6 +668,7 @@ my $vtype_subdirs = { > vztmpl => 'template/cache', > backup => 'dump', > snippets => 'snippets', > + import => 'import', > }; > > sub get_vtype_subdirs { > @@ -1227,7 +1230,7 @@ sub list_images { > return $res; > } > > -# list templates ($tt = ) > +# list templates ($tt = ) > my $get_subdir_files = sub { > my ($sid, $path, $tt, $vmid) = @_; > > @@ -1283,6 +1286,10 @@ my $get_subdir_files = sub { > volid => "$sid:snippets/". basename($fn), > format => 'snippet', > }; > + } elsif ($tt eq 'import') { > + next if $fn !~ m!/(${PVE::Storage::SAFE_CHAR_CLASS_RE}+$PVE::Storage::IMPORT_EXT_RE_1)$!i; > + > + $info = { volid => "$sid:import/$1", format => "$2" }; > } > > $info->{size} = $st->size; > @@ -1317,6 +1324,8 @@ sub list_volumes { > $data = $get_subdir_files->($storeid, $path, 'backup', $vmid); > } elsif ($type eq 'snippets') { > $data = $get_subdir_files->($storeid, $path, 'snippets'); > + } elsif ($type eq 'import') { > + $data = $get_subdir_files->($storeid, $path, 'import'); > } > } > > diff --git a/src/test/parse_volname_test.pm b/src/test/parse_volname_test.pm > index d6ac885..a8c746f 100644 > --- a/src/test/parse_volname_test.pm > +++ b/src/test/parse_volname_test.pm > @@ -81,6 +81,19 @@ my $tests = [ > expected => ['snippets', 'hookscript.pl'], > }, > # > + # Import > + # > + { > + description => "Import, ova", > + volname => 'import/import.ova', > + expected => ['import', 'import.ova', undef, undef, undef ,undef, 'ova'], > + }, with the syntax error above cleaned up, this test here fails since OVA is not yet recognized as format here.. (similarly below as well). > + { > + description => "Import, ovf", > + volname => 'import/import.ovf', > + expected => ['import', 'import.ovf', undef, undef, undef ,undef, 'ovf'], > + }, > + # > # failed matches > # > { > @@ -123,6 +136,11 @@ my $tests = [ > volname => "$vmid/base-$vmid-disk-0.qcow2/ssss/vm-$vmid-disk-0.qcow2", > expected => "unable to parse volume filename 'base-$vmid-disk-0.qcow2/ssss/vm-$vmid-disk-0.qcow2'\n", > }, > + { > + description => "Failed match: import dir but no ova/ovf/disk image", > + volname => "import/test.foo", > + expected => "unable to parse directory volume name 'import/test.foo'\n", > + }, > ]; > > # create more test cases for VM disk images matches > diff --git a/src/test/path_to_volume_id_test.pm b/src/test/path_to_volume_id_test.pm > index 8149c88..0d238f9 100644 > --- a/src/test/path_to_volume_id_test.pm > +++ b/src/test/path_to_volume_id_test.pm > @@ -174,6 +174,22 @@ my @tests = ( > 'local:vztmpl/debian-10.0-standard_10.0-1_amd64.tar.xz', > ], > }, > + { > + description => 'Import, ova', > + volname => "$storage_dir/import/import.ova", > + expected => [ > + 'import', > + 'local:import/import.ova', > + ], > + }, > + { > + description => 'Import, ovf', > + volname => "$storage_dir/import/import.ovf", > + expected => [ > + 'import', > + 'local:import/import.ovf', > + ], > + }, > > # no matches, path or files with failures > { > @@ -231,6 +247,11 @@ my @tests = ( > volname => "$storage_dir/images/ssss/vm-1234-disk-0.qcow2", > expected => [''], > }, > + { > + description => 'Import, non ova/ovf/disk image in import dir', > + volname => "$storage_dir/import/test.foo", > + expected => [''], > + }, > ); > > plan tests => scalar @tests + 1; > -- > 2.39.2 > > > > _______________________________________________ > pve-devel mailing list > pve-devel@lists.proxmox.com > https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel > > > _______________________________________________ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel