Re: [pve-devel] [PATCH storage v4 03/12] plugin: dir: handle ova files for import
On Fri May 24, 2024 at 3:21 PM CEST, Dominik Csapak wrote:
> 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 000..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
[pve-devel] [PATCH storage v4 03/12] plugin: dir: handle ova files for import
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 000..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,
