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 3393C1FF390 for ; Fri, 24 May 2024 15:22:59 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 5F6E690D8; Fri, 24 May 2024 15:22:25 +0200 (CEST) From: Dominik Csapak To: pve-devel@lists.proxmox.com Date: Fri, 24 May 2024 15:21:47 +0200 Message-Id: <20240524132209.703402-4-d.csapak@proxmox.com> X-Mailer: git-send-email 2.39.2 In-Reply-To: <20240524132209.703402-1-d.csapak@proxmox.com> References: <20240524132209.703402-1-d.csapak@proxmox.com> MIME-Version: 1.0 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.016 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 Subject: [pve-devel] [PATCH storage v4 03/12] 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" since we want to handle ova files (which are only ovf+images bundled in a tar file) for import, add code that handles that. we introduce a valid volname for files contained in ovas like this: storage:import/archive.ova/disk-1.vmdk by basically treating the last part of the path as the name for the contained disk we want. in that case we return 'import' as type with 'vmdk/qcow2/raw' as format (we cannot use something like 'ova+vmdk' without extending the 'format' parsing to that for all storages/formats. This is because it runs though a verify format check at least once) we then provide 3 functions to use for that: * copy_needs_extraction: determines from the given volid (like above) if that needs extraction to copy it, currently only 'import' vtype + a volid with the above format returns true * extract_disk_from_import_file: this actually extracts the file from the archive. Currently only ova is supported, so the extraction with 'tar' is hardcoded, but again we can easily extend/modify that should we need to. we currently extract into the either the import storage or a given target storage in the images directory so if the cleanup does not happen, the user can still see and interact with the image via api/cli/gui * cleanup_extracted_image: intended to cleanup the extracted images from above we have to modify the `parse_ovf` a bit to handle the missing disk images, and we parse the size out of the ovf part (since this is informal only, it should be no problem if we cannot parse it sometimes) Signed-off-by: Dominik Csapak --- src/PVE/API2/Storage/Status.pm | 1 + src/PVE/GuestImport.pm | 77 ++++++++++++++++++++++++++++++ src/PVE/GuestImport/OVF.pm | 52 +++++++++++++++++--- src/PVE/Makefile | 1 + src/PVE/Storage.pm | 4 +- src/PVE/Storage/DirPlugin.pm | 15 +++++- src/PVE/Storage/Plugin.pm | 4 ++ src/test/parse_volname_test.pm | 20 ++++++++ src/test/path_to_volume_id_test.pm | 8 ++++ 9 files changed, 173 insertions(+), 9 deletions(-) create mode 100644 src/PVE/GuestImport.pm diff --git a/src/PVE/API2/Storage/Status.pm b/src/PVE/API2/Storage/Status.pm index dc6cc69..acde730 100644 --- a/src/PVE/API2/Storage/Status.pm +++ b/src/PVE/API2/Storage/Status.pm @@ -749,6 +749,7 @@ __PACKAGE__->register_method({ 'efi-state-lost', 'guest-is-running', 'nvme-unsupported', + 'ova-needs-extracting', 'ovmf-with-lsi-unsupported', 'serial-port-socket-only', ], diff --git a/src/PVE/GuestImport.pm b/src/PVE/GuestImport.pm new file mode 100644 index 0000000..988d1f6 --- /dev/null +++ b/src/PVE/GuestImport.pm @@ -0,0 +1,77 @@ +package PVE::GuestImport; + +use strict; +use warnings; + +use File::Path; + +use PVE::Storage; +use PVE::Tools qw(run_command); + +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 ($vtype, $name, undef, undef, undef, undef, $fmt) = + PVE::Storage::parse_volname($cfg, $volid); + + 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_volid; + my $inner_file; + my $inner_fmt; + if ($name =~ m!^(.*\.ova)/(${PVE::Storage::SAFE_CHAR_CLASS_RE}+)$!) { + $archive_volid = "$source_storeid:import/$1"; + $inner_file = $2; + ($inner_fmt) = $fmt =~ /^ova\+(.*)$/; + } else { + die "cannot extract $volid - invalid volname $volname\n"; + } + + my $ova_path = PVE::Storage::path($cfg, $archive_volid); + + my $tmpdir = PVE::Storage::get_image_dir($cfg, $target_storeid, $vmid); + my $pid = $$; + $tmpdir .= "/tmp_${pid}_${vmid}"; + mkpath $tmpdir; + + ($ova_path) = $ova_path =~ m|^(.*)$|; # untaint + + my $source_path = "$tmpdir/$inner_file"; + my $target_path; + my $target_volid; + eval { + run_command(['tar', '-x', '--force-local', '-C', $tmpdir, '-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"; + } + + # TODO check for base images in file + + # create temporary 1M image that will get overwritten by the rename + # to reserve the filename and take care of locking + $target_volid = PVE::Storage::vdisk_alloc($cfg, $target_storeid, $vmid, $inner_fmt, undef, 1024); + $target_path = PVE::Storage::path($cfg, $target_volid); + + print "renaming $source_path to $target_path\n"; + + rename($source_path, $target_path) or die "unable to move - $!\n"; + }; + if (my $err = $@) { + File::Path::remove_tree($tmpdir); + die "error during extraction: $err\n"; + } + + File::Path::remove_tree($tmpdir); + + return $target_volid; +} + +1; diff --git a/src/PVE/GuestImport/OVF.pm b/src/PVE/GuestImport/OVF.pm index 899cfdc..0728684 100644 --- a/src/PVE/GuestImport/OVF.pm +++ b/src/PVE/GuestImport/OVF.pm @@ -84,11 +84,37 @@ sub id_to_pve { } } +# technically defined in DSP0004 (https://www.dmtf.org/dsp/DSP0004) as an ABNF +# but realistically this always takes the form of 'byte * base^exponent' +sub try_parse_capacity_unit { + my ($unit_text) = @_; + + if ($unit_text =~ m/^\s*byte\s*\*\s*([0-9]+)\s*\^\s*([0-9]+)\s*$/) { + my $base = $1; + my $exp = $2; + return $base ** $exp; + } + + return undef; +} + # returns two references, $qm which holds qm.conf style key/values, and \@disks sub parse_ovf { - my ($ovf, $debug) = @_; + my ($ovf, $isOva, $debug) = @_; + + # we have to ignore missing disk images for ova + my $dom; + if ($isOva) { + my $raw = ""; + PVE::Tools::run_command(['tar', '-xO', '--wildcards', '--occurrence=1', '-f', $ovf, '*.ovf'], outfunc => sub { + my $line = shift; + $raw .= $line; + }); + $dom = XML::LibXML->load_xml(string => $raw, no_blanks => 1); + } else { + $dom = XML::LibXML->load_xml(location => $ovf, no_blanks => 1); + } - my $dom = XML::LibXML->load_xml(location => $ovf, no_blanks => 1); # register the xml namespaces in a xpath context object # 'ovf' is the default namespace so it will prepended to each xml element @@ -176,7 +202,17 @@ sub parse_ovf { # @ needs to be escaped to prevent Perl double quote interpolation my $xpath_find_fileref = sprintf("/ovf:Envelope/ovf:DiskSection/\ ovf:Disk[\@ovf:diskId='%s']/\@ovf:fileRef", $disk_id); + my $xpath_find_capacity = sprintf("/ovf:Envelope/ovf:DiskSection/\ +ovf:Disk[\@ovf:diskId='%s']/\@ovf:capacity", $disk_id); + my $xpath_find_capacity_unit = sprintf("/ovf:Envelope/ovf:DiskSection/\ +ovf:Disk[\@ovf:diskId='%s']/\@ovf:capacityAllocationUnits", $disk_id); my $fileref = $xpc->findvalue($xpath_find_fileref); + my $capacity = $xpc->findvalue($xpath_find_capacity); + my $capacity_unit = $xpc->findvalue($xpath_find_capacity_unit); + my $virtual_size; + if (my $factor = try_parse_capacity_unit($capacity_unit)) { + $virtual_size = $capacity * $factor; + } my $valid_url_chars = qr@${valid_uripath_chars}|/@; if (!$fileref || $fileref !~ m/^${valid_url_chars}+$/) { @@ -216,7 +252,7 @@ ovf:Item[rasd:InstanceID='%s']/rasd:ResourceType", $controller_id); die "error parsing $filepath, are you using a symlink ?\n"; } - if (!-e $backing_file_path) { + if (!-e $backing_file_path && !$isOva) { die "error parsing $filepath, file seems not to exist at $backing_file_path\n"; } @@ -224,16 +260,20 @@ ovf:Item[rasd:InstanceID='%s']/rasd:ResourceType", $controller_id); ($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" - if !$virtual_size; + if (!$isOva) { + my $size = PVE::Storage::file_size_info($backing_file_path); + die "error parsing $backing_file_path, cannot determine file size\n" + if !$size; + $virtual_size = $size; + } $pve_disk = { disk_address => $pve_disk_address, backing_file => $backing_file_path, virtual_size => $virtual_size, relative_path => $filepath, }; + $pve_disk->{virtual_size} = $virtual_size if defined($virtual_size); push @disks, $pve_disk; } diff --git a/src/PVE/Makefile b/src/PVE/Makefile index e15a275..0af3081 100644 --- a/src/PVE/Makefile +++ b/src/PVE/Makefile @@ -5,6 +5,7 @@ install: install -D -m 0644 Storage.pm ${DESTDIR}${PERLDIR}/PVE/Storage.pm install -D -m 0644 Diskmanage.pm ${DESTDIR}${PERLDIR}/PVE/Diskmanage.pm install -D -m 0644 CephConfig.pm ${DESTDIR}${PERLDIR}/PVE/CephConfig.pm + install -D -m 0644 GuestImport.pm ${DESTDIR}${PERLDIR}/PVE/GuestImport.pm make -C Storage install make -C GuestImport install make -C API2 install diff --git a/src/PVE/Storage.pm b/src/PVE/Storage.pm index 1ed91c2..bd6c4df 100755 --- a/src/PVE/Storage.pm +++ b/src/PVE/Storage.pm @@ -114,10 +114,12 @@ 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 $IMPORT_EXT_RE_1 = qr/\.(ova|ovf|qcow2|raw|vmdk)/; our $SAFE_CHAR_CLASS_RE = qr/[a-zA-Z0-9\-\.\+\=\_]/; +our $OVA_CONTENT_RE_1 = qr/${SAFE_CHAR_CLASS_RE}+\.(qcow2|raw|vmdk)/; + # FIXME remove with PVE 8.0, add versioned breaks for pve-manager our $vztmpl_extension_re = $VZTMPL_EXT_RE_1; diff --git a/src/PVE/Storage/DirPlugin.pm b/src/PVE/Storage/DirPlugin.pm index 3e3b1e7..ea89464 100644 --- a/src/PVE/Storage/DirPlugin.pm +++ b/src/PVE/Storage/DirPlugin.pm @@ -258,15 +258,26 @@ sub get_import_metadata { # NOTE: all types of warnings must be added to the return schema of the import-metadata API endpoint my $warnings = []; + my $isOva = 0; + if ($name =~ m/\.ova$/) { + $isOva = 1; + push @$warnings, { type => 'ova-needs-extracting' }; + } my $path = $class->path($scfg, $volname, $storeid, undef); - my $res = PVE::GuestImport::OVF::parse_ovf($path); + my $res = PVE::GuestImport::OVF::parse_ovf($path, $isOva); my $disks = {}; for my $disk ($res->{disks}->@*) { my $id = $disk->{disk_address}; my $size = $disk->{virtual_size}; my $path = $disk->{relative_path}; + my $volid; + if ($isOva) { + $volid = "$storeid:$volname/$path"; + } else { + $volid = "$storeid:import/$path", + } $disks->{$id} = { - volid => "$storeid:import/$path", + volid => $volid, defined($size) ? (size => $size) : (), }; } diff --git a/src/PVE/Storage/Plugin.pm b/src/PVE/Storage/Plugin.pm index fc30894..848e053 100644 --- a/src/PVE/Storage/Plugin.pm +++ b/src/PVE/Storage/Plugin.pm @@ -654,6 +654,10 @@ sub parse_volname { return ('backup', $fn); } elsif ($volname =~ m!^snippets/([^/]+)$!) { return ('snippets', $1); + } elsif ($volname =~ m!^import/(${PVE::Storage::SAFE_CHAR_CLASS_RE}+\.ova\/${PVE::Storage::OVA_CONTENT_RE_1})$!) { + my $archive = $1; + my $format = $2; + return ('import', $archive, undef, undef, undef, undef, "ova+$format"); } elsif ($volname =~ m!^import/(${PVE::Storage::SAFE_CHAR_CLASS_RE}+$PVE::Storage::IMPORT_EXT_RE_1)$!) { return ('import', $1, undef, undef, undef, undef, $2); } diff --git a/src/test/parse_volname_test.pm b/src/test/parse_volname_test.pm index 0367b83..bc7b4e8 100644 --- a/src/test/parse_volname_test.pm +++ b/src/test/parse_volname_test.pm @@ -83,11 +83,31 @@ my $tests = [ # # Import # + { + description => "Import, ova", + volname => 'import/import.ova', + expected => ['import', 'import.ova', undef, undef, undef ,undef, 'ova'], + }, { description => "Import, ovf", volname => 'import/import.ovf', expected => ['import', 'import.ovf', undef, undef, undef ,undef, 'ovf'], }, + { + description => "Import, innner file of ova", + volname => 'import/import.ova/disk.qcow2', + expected => ['import', 'import.ova/disk.qcow2', undef, undef, undef, undef, 'ova+qcow2'], + }, + { + description => "Import, innner file of ova", + volname => 'import/import.ova/disk.vmdk', + expected => ['import', 'import.ova/disk.vmdk', undef, undef, undef, undef, 'ova+vmdk'], + }, + { + description => "Import, innner file of ova", + volname => 'import/import.ova/disk.raw', + expected => ['import', 'import.ova/disk.raw', undef, undef, undef, undef, 'ova+raw'], + }, # # failed matches # diff --git a/src/test/path_to_volume_id_test.pm b/src/test/path_to_volume_id_test.pm index 0e0c15f..0d238f9 100644 --- a/src/test/path_to_volume_id_test.pm +++ b/src/test/path_to_volume_id_test.pm @@ -174,6 +174,14 @@ 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", -- 2.39.2 _______________________________________________ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel