From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [IPv6:2a01:7e0:0:424::9]) by lore.proxmox.com (Postfix) with ESMTPS id 841661FF389 for ; Wed, 22 May 2024 15:13:34 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id D190CDD5C; Wed, 22 May 2024 15:13:49 +0200 (CEST) Date: Wed, 22 May 2024 15:13:43 +0200 From: Fabian =?iso-8859-1?q?Gr=FCnbichler?= To: Proxmox VE development discussion References: <20240429112124.3819357-1-d.csapak@proxmox.com> <20240429112124.3819357-4-d.csapak@proxmox.com> In-Reply-To: <20240429112124.3819357-4-d.csapak@proxmox.com> MIME-Version: 1.0 User-Agent: astroid/0.16.0 (https://github.com/astroidmail/astroid) Message-Id: <1716383078.q9srlosw0e.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. [guestimport.pm] Subject: Re: [pve-devel] [PATCH storage v3 03/10] plugin: dir: handle ova files for import 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: > diff --git a/src/PVE/GuestImport.pm b/src/PVE/GuestImport.pm > new file mode 100644 > index 0000000..d405e30 > --- /dev/null > +++ b/src/PVE/GuestImport.pm > @@ -0,0 +1,100 @@ > +package PVE::GuestImport; > + > +use strict; > +use warnings; > + > +use File::Path; > + > +use PVE::Storage; > +use PVE::Tools qw(run_command); > + > +sub copy_needs_extraction { > + my ($volid) = @_; > + my $cfg = PVE::Storage::config(); > + my ($vtype, $name, undef, undef, undef, undef, $fmt) = PVE::Storage::parse_volname($cfg, $volid); > + > + # only volumes inside ovas need extraction > + return $vtype eq 'import' && $fmt =~ m/^ova\+(.*)$/; > +} > + > +sub extract_disk_from_import_file { > + my ($volid, $vmid, $target_storeid) = @_; > + > + my ($source_storeid, $volname) = PVE::Storage::parse_volume_id($volid); > + $target_storeid //= $source_storeid; > + my $cfg = PVE::Storage::config(); > + my $source_scfg = PVE::Storage::storage_config($cfg, $source_storeid); > + my $source_plugin = PVE::Storage::Plugin->lookup($source_scfg->{type}); > + > + my ($vtype, $name, undef, undef, undef, undef, $fmt) = > + $source_plugin->parse_volname($volname); > + > + die "only files with content type 'import' can be extracted\n" > + if $vtype ne 'import' || $fmt !~ m/^ova\+/; > + > + # extract the inner file from the name > + my $archive; > + my $inner_file; > + if ($name =~ m!^(.*\.ova)/(${PVE::Storage::SAFE_CHAR_CLASS_RE}+)$!) { > + $archive = "import/$1"; > + $inner_file = $2; > + ($fmt) = $fmt =~ /^ova\+(.*)$/; > + } else { > + die "cannot extract $volid - invalid volname $volname\n"; > + } > + > + my ($ova_path) = $source_plugin->path($source_scfg, $archive, $source_storeid); > + > + my $target_scfg = PVE::Storage::storage_config($cfg, $target_storeid); > + my $target_plugin = PVE::Storage::Plugin->lookup($target_scfg->{type}); > + > + my $destdir = $target_plugin->get_subdir($target_scfg, 'images'); > + > + my $pid = $$; > + $destdir .= "/tmp_${pid}_${vmid}"; > + mkpath $destdir; > + > + ($ova_path) = $ova_path =~ m|^(.*)$|; # untaint > + > + my $source_path = "$destdir/$inner_file"; > + my $target_path; > + my $target_volname; > + eval { > + run_command(['tar', '-x', '--force-local', '-C', $destdir, '-f', $ova_path, $inner_file]); > + > + # check for symlinks and other non regular files > + if (-l $source_path || ! -f $source_path) { > + die "only regular files are allowed\n"; > + } > + > + my $target_diskname > + = $target_plugin->find_free_diskname($target_storeid, $target_scfg, $vmid, $fmt, 1); thought some more about this part. I don't think we currently consider find_free_diskname to be public API for consumption outside of plugins (rightfully so, IMHO). I wonder how we could avoid that problem here. we could extend the existing rename feature to allow moving from an arbitrary path to the target storage (but that is risky, since it might mean we are copying and not moving/renaming, unless we add extra checks)? or we could handle the "temp extracted volume" as a volume, allowing a regular PVE::Storage::rename_volume call to work? maybe would risk breaking existing external plugins, but we could bump APIVER and APIAGE and check APIVER to only allow storages as extraction target that explicitly opted into it? > + $target_volname = "$vmid/" . $target_diskname; > + $target_path = $target_plugin->filesystem_path($target_scfg, $target_volname); > + > + print "renaming $source_path to $target_path\n"; > + my $imagedir = $target_plugin->get_subdir($target_scfg, 'images'); > + mkpath "$imagedir/$vmid"; > + > + rename($source_path, $target_path) or die "unable to move - $!\n"; > + }; > + if (my $err = $@) { > + unlink $source_path; > + unlink $target_path if defined($target_path); > + rmdir $destdir; > + die "error during extraction: $err\n"; > + } > + > + rmdir $destdir; > + > + return "$target_storeid:$target_volname"; > +} > + > +sub cleanup_extracted_image { > + my ($source) = @_; > + > + my $cfg = PVE::Storage::config(); > + PVE::Storage::vdisk_free($cfg, $source); > +} > + > +1; _______________________________________________ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel