[pve-devel] [PATCH storage v6 03/12] plugin: dir: handle ova files for import

[Dominik Csapak <d.csapak@proxmox.com>]

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 a function to use for that:

* 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


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 <d.csapak@proxmox.com>
---
changes from v5:
* adapted commit message to reflect actual changes
* split up errors
* removed unnecessary untaint
* improved error message with context
* check $fmt instead of $name for ova formats

 src/PVE/API2/Storage/Status.pm     |  1 +
 src/PVE/GuestImport.pm             | 79 ++++++++++++++++++++++++++++++
 src/PVE/GuestImport/OVF.pm         | 52 +++++++++++++++++---
 src/PVE/Makefile                   |  1 +
 src/PVE/Storage.pm                 |  4 +-
 src/PVE/Storage/DirPlugin.pm       | 17 +++++--
 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, 176 insertions(+), 10 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 f86e5d3..bdf1c18 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..f7ebf92
--- /dev/null
+++ b/src/PVE/GuestImport.pm
@@ -0,0 +1,79 @@
+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';
+
+    die "only files from 'ova' format can be extracted\n"
+	if $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;
+
+    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 "extracted file '$inner_file' from archive '$archive_volid' is not a regular file\n";
+	}
+
+	# check potentially untrusted image file!
+	PVE::Storage::file_size_info($source_path, undef, 1);
+
+	# 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 29dfaad..c7bff5f 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 78a3405..4df1a84 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|bz2)/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 9.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 efbca0c..04a0485 100644
--- a/src/PVE/Storage/DirPlugin.pm
+++ b/src/PVE/Storage/DirPlugin.pm
@@ -253,20 +253,31 @@ sub get_import_metadata {
 
     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 'ovf';
+    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 $isOva = 0;
+    if ($fmt =~ 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 3655e6a..eed764d 100644
--- a/src/PVE/Storage/Plugin.pm
+++ b/src/PVE/Storage/Plugin.pm
@@ -663,6 +663,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 92e984f..eecd7df 100644
--- a/src/test/parse_volname_test.pm
+++ b/src/test/parse_volname_test.pm
@@ -88,11 +88,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 d954f4b..23c5a23 100644
--- a/src/test/path_to_volume_id_test.pm
+++ b/src/test/path_to_volume_id_test.pm
@@ -190,6 +190,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.5



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel