[pve-devel] [PATCH storage/qemu-server/manager v7] implement ova/ovf import for file based storages

[pve-devel] [PATCH storage/qemu-server/manager v7] implement ova/ovf import for file based storages

[Dominik Csapak]

This series enables importing ova/ovf from directory based storages,
inclusive upload/download via the webui (ova only).

It also improves the ovf importer by parsing the ostype, nics, bootorder
(and firmware from vmware exported files).

We now extract the images into either a given target storage or in the
import storage in the 'images' dir so accidentally left over images
are discoverable by the ui/cli.

This version includes a modified qemu-server patch of fabians hardening series:
https://lore.proxmox.com/pve-devel/20241104104221.228730-1-f.gruenbichler@proxmox.com/

I sent the qemu-server patch from fabian again but omitted some
problematic checks. I add them later with a check
against the import vtype again (last patch in qemu-server)

changes from v6:
* rebase on master (omit already applied patches)
* style/indentation fixes
* omit explicit check for .ova in upload path
* use assert_file_validity check for download_file_from_url
* only warn on whitespace in parent image (file_size_info) to preserve backward compatibility
  for possible existing cases
* fix cleanup on live-import  (added wrong variable to cleanup list)
* renamed 'import-extraction-storage' to 'import-working-storage'
* mad priv check for extraction storage simpler (combine branches)
* reworded some gettexts

changes from v5:
* removed leftover hunks in makefile
* moved ova checks to correct patch
* split up error messages for unexpected format
* remove unnecessary untaint
* reword error message
* reintroduce symlink check in ova/ovf check
* added sanity check for ovas after uploading/downloading
* added new patch for checking import vtypes
* fixed issue with files with absolute path

changes from v4:
* rebased on master/fabians series
* added the file_size_info check for untrusted images after extracting

changes from v3:
* fixed dependencies in control file
* removed unnecessary use statements
* removed unnecessary remove helper
* moved 'needs_extract' helper to qemu-server
* removed import storage param from PUT call
* check down/uploaded ova filename more strictly (same as listing)
* improved filepath checking in ovf
* forbid importing when extracted image references a base/backing file
* instead of trying to manually create a proper filename, use 'alloc' to
  create a small (1M) file with the same format and overwrite it with
  renaming. this also solves the cluster locking issue
* prefer using PVE::Storage functions instead of plugin methods in
  ova extraction code
* use $vollist for cleaning up extracted images in qemu-server and
  add manual cleanup for the success case

changes from v2:
* use better 'format' values for embedded images (e.g. ova+vmdk)
* use this format to decide if images should be extracted
* consistent use of the 'safe character' classes when listing
  and parsing
* also list vmdk/qcow2/raw images in content listing
  (this will be useful when we have a gui for the 'import-from'
  in the wizard/disk edit for vms)
* a few gui adaptions


changes from v1:
* move ovf code to GuestImport
* move extract/checking code to GuestImport
* don't return 'image' types from import volumes
* use allow 'safe' characters for filenames of ova/ovfs and inside
* check for non-regular files (e.g. symlinks) after extraction
* add new 'import-extraction-storage' for import
* rename panel in gui for directory storages
* typo fixes
* and probably more, see the individual patches for details

pve-storage:

Dominik Csapak (11):
  plugin: dir: implement import content type
  plugin: dir: handle ova files for import
  ovf: improve and simplify path checking code
  ovf: implement parsing the ostype
  ovf: implement parsing out firmware type
  ovf: implement rudimentary boot order
  ovf: implement parsing nics
  api: allow ova upload/download
  plugin: enable import for nfs/btrfs/cifs/cephfs/glusterfs
  add 'import' content type to 'check_volume_access'
  plugin: file_size_info: warn on parent images with unusual path

 src/PVE/API2/Storage/Status.pm                |  56 +++++-
 src/PVE/GuestImport.pm                        |  79 ++++++++
 src/PVE/GuestImport/OVF.pm                    | 187 ++++++++++++++++--
 src/PVE/Makefile                              |   1 +
 src/PVE/Storage.pm                            |  23 ++-
 src/PVE/Storage/BTRFSPlugin.pm                |   5 +
 src/PVE/Storage/CIFSPlugin.pm                 |   6 +-
 src/PVE/Storage/CephFSPlugin.pm               |   6 +-
 src/PVE/Storage/DirPlugin.pm                  |  52 ++++-
 src/PVE/Storage/GlusterfsPlugin.pm            |   6 +-
 src/PVE/Storage/NFSPlugin.pm                  |   6 +-
 src/PVE/Storage/Plugin.pm                     |  16 +-
 .../ovf_manifests/Win10-Liz_no_default_ns.ovf |   1 +
 src/test/parse_volname_test.pm                |  33 ++++
 src/test/path_to_volume_id_test.pm            |  21 ++
 src/test/run_ovf_tests.pl                     |  14 ++
 16 files changed, 480 insertions(+), 32 deletions(-)
 create mode 100644 src/PVE/GuestImport.pm

qemu-server:

Dominik Csapak (4):
  use OVF from Storage
  api: create: implement extracting disks when needed for import-from
  api: create: add 'import-working-storage' parameter
  api: check untrusted image files for import content type

Fabian Grünbichler (1):
  disk import: add additional safeguards for imported image files

 PVE/API2/Qemu.pm                              | 108 ++++++--
 PVE/CLI/qm.pm                                 |   4 +-
 PVE/QemuServer.pm                             |  12 +
 PVE/QemuServer/Helpers.pm                     |   5 +
 PVE/QemuServer/Makefile                       |   1 -
 PVE/QemuServer/OVF.pm                         | 242 ------------------
 debian/control                                |   2 -
 test/Makefile                                 |   5 +-
 test/ovf_manifests/Win10-Liz-disk1.vmdk       | Bin 65536 -> 0 bytes
 test/ovf_manifests/Win10-Liz.ovf              | 142 ----------
 .../ovf_manifests/Win10-Liz_no_default_ns.ovf | 142 ----------
 test/ovf_manifests/Win_2008_R2_two-disks.ovf  | 145 -----------
 test/ovf_manifests/disk1.vmdk                 | Bin 65536 -> 0 bytes
 test/ovf_manifests/disk2.vmdk                 | Bin 65536 -> 0 bytes
 test/run_ovf_tests.pl                         |  71 -----
 15 files changed, 112 insertions(+), 767 deletions(-)
 delete mode 100644 PVE/QemuServer/OVF.pm
 delete mode 100644 test/ovf_manifests/Win10-Liz-disk1.vmdk
 delete mode 100755 test/ovf_manifests/Win10-Liz.ovf
 delete mode 100755 test/ovf_manifests/Win10-Liz_no_default_ns.ovf
 delete mode 100755 test/ovf_manifests/Win_2008_R2_two-disks.ovf
 delete mode 100644 test/ovf_manifests/disk1.vmdk
 delete mode 100644 test/ovf_manifests/disk2.vmdk
 delete mode 100755 test/run_ovf_tests.pl

pve-manager:

Dominik Csapak (9):
  ui: guest import: add ova-needs-extracting warning text
  ui: enable import content type for relevant storages
  ui: enable upload/download/remove buttons for 'import' type storages
  ui: disable 'import' button for non importable formats
  ui: import: improve rendering of volume names
  ui: guest import: add storage selector for ova extraction storage
  ui: guest import: change icon/text for non-esxi import storage
  ui: import: show size for dir-based storages
  ui: import: adapt live import help text to ova

 www/manager6/Utils.js                    |  9 +++++++-
 www/manager6/form/ContentTypeSelector.js |  2 +-
 www/manager6/storage/Browser.js          | 25 ++++++++++++++++-----
 www/manager6/storage/CephFSEdit.js       |  2 +-
 www/manager6/storage/GlusterFsEdit.js    |  2 +-
 www/manager6/window/GuestImport.js       | 28 +++++++++++++++++++++++-
 www/manager6/window/UploadToStorage.js   |  1 +
 7 files changed, 58 insertions(+), 11 deletions(-)

-- 
2.39.5



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

[pve-devel] [PATCH storage v7 01/11] plugin: dir: implement import content type

[Dominik Csapak]

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 <d.csapak@proxmox.com>
Reviewed-by: Fiona Ebner <f.ebner@proxmox.com>
---
 src/PVE/GuestImport/OVF.pm         |  5 ++++-
 src/PVE/Storage.pm                 |  8 +++++++
 src/PVE/Storage/DirPlugin.pm       | 36 +++++++++++++++++++++++++++++-
 src/PVE/Storage/Plugin.pm          | 11 ++++++++-
 src/test/parse_volname_test.pm     | 13 +++++++++++
 src/test/path_to_volume_id_test.pm | 13 +++++++++++
 6 files changed, 83 insertions(+), 3 deletions(-)

diff --git a/src/PVE/GuestImport/OVF.pm b/src/PVE/GuestImport/OVF.pm
index 3950289..29dfaad 100644
--- a/src/PVE/GuestImport/OVF.pm
+++ b/src/PVE/GuestImport/OVF.pm
@@ -221,6 +221,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"
@@ -229,7 +231,8 @@ ovf:Item[rasd:InstanceID='%s']/rasd:ResourceType", $controller_id);
 	$pve_disk = {
 	    disk_address => $pve_disk_address,
 	    backing_file => $backing_file_path,
-	    virtual_size => $virtual_size
+	    virtual_size => $virtual_size,
+	    relative_path => $filepath,
 	};
 	push @disks, $pve_disk;
 
diff --git a/src/PVE/Storage.pm b/src/PVE/Storage.pm
index 1c4c5cd..6e12a00 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|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 $SAFE_CHAR_CLASS_RE = qr/[a-zA-Z0-9\-\.\+\=\_]/;
+
 # FIXME remove with PVE 9.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..efbca0c 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 '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 437365a..3655e6a 100644
--- a/src/PVE/Storage/Plugin.pm
+++ b/src/PVE/Storage/Plugin.pm
@@ -663,6 +663,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";
@@ -675,6 +677,7 @@ my $vtype_subdirs = {
     vztmpl => 'template/cache',
     backup => 'dump',
     snippets => 'snippets',
+    import => 'import',
 };
 
 sub get_vtype_subdirs {
@@ -1269,7 +1272,7 @@ sub list_images {
     return $res;
 }
 
-# list templates ($tt = <iso|vztmpl|backup|snippets>)
+# list templates ($tt = <iso|vztmpl|backup|snippets|import>)
 my $get_subdir_files = sub {
     my ($sid, $path, $tt, $vmid) = @_;
 
@@ -1325,6 +1328,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;
@@ -1359,6 +1366,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 6c5ba04..92e984f 100644
--- a/src/test/parse_volname_test.pm
+++ b/src/test/parse_volname_test.pm
@@ -86,6 +86,14 @@ my $tests = [
 	expected    => ['snippets', 'hookscript.pl'],
     },
     #
+    # Import
+    #
+    {
+	description => "Import, ovf",
+	volname     => 'import/import.ovf',
+	expected    => ['import', 'import.ovf', undef, undef, undef ,undef, 'ovf'],
+    },
+    #
     # failed matches
     #
     {
@@ -123,6 +131,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 3198752..d954f4b 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, ovf',
+	volname     => "$storage_dir/import/import.ovf",
+	expected    => [
+	    'import',
+	    'local:import/import.ovf',
+	],
+    },
 
     # no matches, path or files with failures
     {
@@ -237,6 +245,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.5



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

[pve-devel] [PATCH storage v7 02/11] plugin: dir: handle ova files for import

[Dominik Csapak]

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>
Reviewed-by: Fiona Ebner <f.ebner@proxmox.com>
---
changes from v6:
* renamed $archive to $packed_image

 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 47f82d7..d30d8fb 100644
--- a/src/PVE/API2/Storage/Status.pm
+++ b/src/PVE/API2/Storage/Status.pm
@@ -761,6 +761,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 6e12a00..f60c5fc 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..8f1c7c8 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 $packed_image = $1;
+	my $format = $2;
+	return ('import', $packed_image, 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

[pve-devel] [PATCH storage v7 03/11] ovf: improve and simplify path checking code

[Dominik Csapak]

moves the filepath code a bit more closer to where it's actually used
checks the contained path before trying to find it's absolute path
properly add error handling to realpath

instead of checking the combined ovf_path + filepath, just make sure
filepath can't point to anythign besides a file in this directory
by checking for '.' and '..' (slashes are not allowed in SAFE_CHAR_CLASS_RE)

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
Reviewed-by: Fiona Ebner <f.ebner@proxmox.com>
---
 src/PVE/GuestImport/OVF.pm | 33 ++++++++++++++++++---------------
 1 file changed, 18 insertions(+), 15 deletions(-)

diff --git a/src/PVE/GuestImport/OVF.pm b/src/PVE/GuestImport/OVF.pm
index c7bff5f..966dcd1 100644
--- a/src/PVE/GuestImport/OVF.pm
+++ b/src/PVE/GuestImport/OVF.pm
@@ -220,15 +220,6 @@ ovf:Disk[\@ovf:diskId='%s']/\@ovf:capacityAllocationUnits", $disk_id);
 	    next;
 	}
 
-	# from Disk Node, find corresponding filepath
-	my $xpath_find_filepath = sprintf("/ovf:Envelope/ovf:References/ovf:File[\@ovf:id='%s']/\@ovf:href", $fileref);
-	my $filepath = $xpc->findvalue($xpath_find_filepath);
-	if (!$filepath) {
-	    warn "invalid file reference $fileref, skipping\n";
-	    next;
-	}
-	print "file path: $filepath\n" if $debug;
-
 	# from Item, find owning Controller type
 	my $controller_id = $xpc->findvalue('rasd:Parent', $item_node);
 	my $xpath_find_parent_type = sprintf("/ovf:Envelope/ovf:VirtualSystem/ovf:VirtualHardwareSection/\
@@ -244,22 +235,34 @@ ovf:Item[rasd:InstanceID='%s']/rasd:ResourceType", $controller_id);
 	my $adress_on_controller = $xpc->findvalue('rasd:AddressOnParent', $item_node);
 	my $pve_disk_address = id_to_pve($controller_type) . $adress_on_controller;
 
+	# from Disk Node, find corresponding filepath
+	my $xpath_find_filepath = sprintf("/ovf:Envelope/ovf:References/ovf:File[\@ovf:id='%s']/\@ovf:href", $fileref);
+	my $filepath = $xpc->findvalue($xpath_find_filepath);
+	if (!$filepath) {
+	    warn "invalid file reference $fileref, skipping\n";
+	    next;
+	}
+	print "file path: $filepath\n" if $debug;
+	my $original_filepath = $filepath;
+	($filepath) = $filepath =~ m|^(${PVE::Storage::SAFE_CHAR_CLASS_RE}+)$|; # untaint & check no sub/parent dirs
+	die "referenced path '$original_filepath' is invalid\n" if !$filepath || $filepath eq "." || $filepath eq "..";
+
 	# resolve symlinks and relative path components
 	# and die if the diskimage is not somewhere under the $ovf path
-	my $ovf_dir = realpath(dirname(File::Spec->rel2abs($ovf)));
-	my $backing_file_path = realpath(join ('/', $ovf_dir, $filepath));
+	my $ovf_dir = realpath(dirname(File::Spec->rel2abs($ovf)))
+	    or die "could not get absolute path of $ovf: $!\n";
+	my $backing_file_path = realpath(join ('/', $ovf_dir, $filepath))
+	    or die "could not get absolute path of $filepath: $!\n";
 	if ($backing_file_path !~ /^\Q${ovf_dir}\E/) {
 	    die "error parsing $filepath, are you using a symlink ?\n";
 	}
 
+	($backing_file_path) = $backing_file_path =~ m|^(/.*)|; # untaint
+
 	if (!-e $backing_file_path && !$isOva) {
 	    die "error parsing $filepath, file seems not to exist at $backing_file_path\n";
 	}
 
-	($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;
-
 	if (!$isOva) {
 	    my $size = PVE::Storage::file_size_info($backing_file_path);
 	    die "error parsing $backing_file_path, cannot determine file size\n"
-- 
2.39.5



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

[pve-devel] [PATCH storage v7 04/11] ovf: implement parsing the ostype

[Dominik Csapak]

use the standards info about the ostypes to map to our own
(see comment for link to the relevant part of the dmtf schema)

every type that is not listed we map to 'other', so no need to have it
in a list.

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
Reviewed-by: Fiona Ebner <f.ebner@proxmox.com>
---
 src/PVE/GuestImport/OVF.pm | 69 ++++++++++++++++++++++++++++++++++++++
 src/test/run_ovf_tests.pl  |  5 +++
 2 files changed, 74 insertions(+)

diff --git a/src/PVE/GuestImport/OVF.pm b/src/PVE/GuestImport/OVF.pm
index 966dcd1..a760e1e 100644
--- a/src/PVE/GuestImport/OVF.pm
+++ b/src/PVE/GuestImport/OVF.pm
@@ -54,6 +54,71 @@ my @resources = (
     { id => 35, dtmf_name => 'Vendor Reserved'}
 );
 
+# see https://schemas.dmtf.org/wbem/cim-html/2.55.0+/CIM_OperatingSystem.html
+my $ostype_ids = {
+    18 => 'winxp', # 'WINNT',
+    29 => 'solaris', # 'Solaris',
+    36 => 'l26', # 'LINUX',
+    58 => 'w2k', # 'Windows 2000',
+    67 => 'wxp', #'Windows XP',
+    69 => 'w2k3', # 'Microsoft Windows Server 2003',
+    70 => 'w2k3', # 'Microsoft Windows Server 2003 64-Bit',
+    71 => 'wxp', # 'Windows XP 64-Bit',
+    72 => 'wxp', # 'Windows XP Embedded',
+    73 => 'wvista', # 'Windows Vista',
+    74 => 'wvista', # 'Windows Vista 64-Bit',
+    75 => 'wxp', # 'Windows Embedded for Point of Service', ??
+    76 => 'w2k8', # 'Microsoft Windows Server 2008',
+    77 => 'w2k8', # 'Microsoft Windows Server 2008 64-Bit',
+    79 => 'l26', # 'RedHat Enterprise Linux',
+    80 => 'l26', # 'RedHat Enterprise Linux 64-Bit',
+    81 => 'solaris', #'Solaris 64-Bit',
+    82 => 'l26', # 'SUSE',
+    83 => 'l26', # 'SUSE 64-Bit',
+    84 => 'l26', # 'SLES',
+    85 => 'l26', # 'SLES 64-Bit',
+    87 => 'l26', # 'Novell Linux Desktop',
+    89 => 'l26', # 'Mandriva',
+    90 => 'l26', # 'Mandriva 64-Bit',
+    91 => 'l26', # 'TurboLinux',
+    92 => 'l26', # 'TurboLinux 64-Bit',
+    93 => 'l26', # 'Ubuntu',
+    94 => 'l26', # 'Ubuntu 64-Bit',
+    95 => 'l26', # 'Debian',
+    96 => 'l26', # 'Debian 64-Bit',
+    97 => 'l24', # 'Linux 2.4.x',
+    98 => 'l24', # 'Linux 2.4.x 64-Bit',
+    99 => 'l26', # 'Linux 2.6.x',
+    100 => 'l26', # 'Linux 2.6.x 64-Bit',
+    101 => 'l26', # 'Linux 64-Bit',
+    103 => 'win7', # 'Microsoft Windows Server 2008 R2',
+    105 => 'win7', # 'Microsoft Windows 7',
+    106 => 'l26', # 'CentOS 32-bit',
+    107 => 'l26', # 'CentOS 64-bit',
+    108 => 'l26', # 'Oracle Linux 32-bit',
+    109 => 'l26', # 'Oracle Linux 64-bit',
+    111 => 'win8', # 'Microsoft Windows Server 2011', ??
+    112 => 'win8', # 'Microsoft Windows Server 2012',
+    113 => 'win8', # 'Microsoft Windows 8',
+    114 => 'win8', # 'Microsoft Windows 8 64-bit',
+    115 => 'win8', # 'Microsoft Windows Server 2012 R2',
+    116 => 'win10', # 'Microsoft Windows Server 2016',
+    117 => 'win8', # 'Microsoft Windows 8.1',
+    118 => 'win8', # 'Microsoft Windows 8.1 64-bit',
+    119 => 'win10', # 'Microsoft Windows 10',
+    120 => 'win10', # 'Microsoft Windows 10 64-bit',
+    121 => 'win10', # 'Microsoft Windows Server 2019',
+    122 => 'win11', # 'Microsoft Windows 11 64-bit',
+    123 => 'win11', # 'Microsoft Windows Server 2022',
+    # others => 'other',
+};
+
+sub get_ostype {
+    my ($id) = @_;
+
+    return $ostype_ids->{$id} // 'other';
+}
+
 sub find_by {
     my ($key, $param) = @_;
     foreach my $resource (@resources) {
@@ -159,6 +224,10 @@ sub parse_ovf {
     my $xpath_find_disks="/ovf:Envelope/ovf:VirtualSystem/ovf:VirtualHardwareSection/ovf:Item[rasd:ResourceType=${disk_id}]";
     my @disk_items = $xpc->findnodes($xpath_find_disks);
 
+    my $xpath_find_ostype_id = "/ovf:Envelope/ovf:VirtualSystem/ovf:OperatingSystemSection/\@ovf:id";
+    my $ostype_id = $xpc->findvalue($xpath_find_ostype_id);
+    $qm->{ostype} = get_ostype($ostype_id);
+
     # disks metadata is split in four different xml elements:
     # * as an Item node of type DiskDrive in the VirtualHardwareSection
     # * as an Disk node in the DiskSection
diff --git a/src/test/run_ovf_tests.pl b/src/test/run_ovf_tests.pl
index 5a80ab2..c433c9d 100755
--- a/src/test/run_ovf_tests.pl
+++ b/src/test/run_ovf_tests.pl
@@ -59,13 +59,18 @@ print "\ntesting vm.conf extraction\n";
 is($win2008->{qm}->{name}, 'Win2008-R2x64', 'win2008 VM name is correct');
 is($win2008->{qm}->{memory}, '2048', 'win2008 VM memory is correct');
 is($win2008->{qm}->{cores}, '1', 'win2008 VM cores are correct');
+is($win2008->{qm}->{ostype}, 'win7', 'win2008 VM ostype is correcty');
 
 is($win10->{qm}->{name}, 'Win10-Liz', 'win10 VM name is correct');
 is($win10->{qm}->{memory}, '6144', 'win10 VM memory is correct');
 is($win10->{qm}->{cores}, '4', 'win10 VM cores are correct');
+# older esxi/ovf standard used 'other' for windows10
+is($win10->{qm}->{ostype}, 'other', 'win10 VM ostype is correct');
 
 is($win10noNs->{qm}->{name}, 'Win10-Liz', 'win10 VM (no default rasd NS) name is correct');
 is($win10noNs->{qm}->{memory}, '6144', 'win10 VM (no default rasd NS) memory is correct');
 is($win10noNs->{qm}->{cores}, '4', 'win10 VM (no default rasd NS) cores are correct');
+# older esxi/ovf standard used 'other' for windows10
+is($win10noNs->{qm}->{ostype}, 'other', 'win10 VM (no default rasd NS) ostype is correct');
 
 done_testing();
-- 
2.39.5



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

[pve-devel] [PATCH storage v7 05/11] ovf: implement parsing out firmware type

[Dominik Csapak]

it seems there is no part of the ovf standard that handles which type of
bios there is (at least i could not find it). Every ovf/ova i tested
either has no info about it, or has it in a vmware specific property
which we parse here.

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
Reviewed-by: Fiona Ebner <f.ebner@proxmox.com>
---
 src/PVE/GuestImport/OVF.pm                         | 5 +++++
 src/PVE/Storage/DirPlugin.pm                       | 5 +++++
 src/test/ovf_manifests/Win10-Liz_no_default_ns.ovf | 1 +
 src/test/run_ovf_tests.pl                          | 1 +
 4 files changed, 12 insertions(+)

diff --git a/src/PVE/GuestImport/OVF.pm b/src/PVE/GuestImport/OVF.pm
index a760e1e..08e9d0f 100644
--- a/src/PVE/GuestImport/OVF.pm
+++ b/src/PVE/GuestImport/OVF.pm
@@ -228,6 +228,11 @@ sub parse_ovf {
     my $ostype_id = $xpc->findvalue($xpath_find_ostype_id);
     $qm->{ostype} = get_ostype($ostype_id);
 
+    # vmware specific firmware config, seems to not be standardized in ovf ?
+    my $xpath_find_firmware = "/ovf:Envelope/ovf:VirtualSystem/ovf:VirtualHardwareSection/vmw:Config[\@vmw:key=\"firmware\"]/\@vmw:value";
+    my $firmware = $xpc->findvalue($xpath_find_firmware) || 'seabios';
+    $qm->{bios} = 'ovmf' if $firmware eq 'efi';
+
     # disks metadata is split in four different xml elements:
     # * as an Item node of type DiskDrive in the VirtualHardwareSection
     # * as an Disk node in the DiskSection
diff --git a/src/PVE/Storage/DirPlugin.pm b/src/PVE/Storage/DirPlugin.pm
index 04a0485..0c32242 100644
--- a/src/PVE/Storage/DirPlugin.pm
+++ b/src/PVE/Storage/DirPlugin.pm
@@ -282,6 +282,11 @@ sub get_import_metadata {
 	};
     }
 
+    if (defined($res->{qm}->{bios}) && $res->{qm}->{bios} eq 'ovmf') {
+	$disks->{efidisk0} = 1;
+	push @$warnings, { type => 'efi-state-lost', key => 'bios', value => 'ovmf' };
+    }
+
     return {
 	type => 'vm',
 	source => $volname,
diff --git a/src/test/ovf_manifests/Win10-Liz_no_default_ns.ovf b/src/test/ovf_manifests/Win10-Liz_no_default_ns.ovf
index b93540f..10ccaf1 100755
--- a/src/test/ovf_manifests/Win10-Liz_no_default_ns.ovf
+++ b/src/test/ovf_manifests/Win10-Liz_no_default_ns.ovf
@@ -137,6 +137,7 @@
       <vmw:Config ovf:required="false" vmw:key="powerOpInfo.resetType" vmw:value="soft"/>
       <vmw:Config ovf:required="false" vmw:key="powerOpInfo.suspendType" vmw:value="soft"/>
       <vmw:Config ovf:required="false" vmw:key="tools.syncTimeWithHost" vmw:value="false"/>
+      <vmw:Config ovf:required="false" vmw:key="firmware" vmw:value="efi"/>
     </VirtualHardwareSection>
   </VirtualSystem>
 </Envelope>
diff --git a/src/test/run_ovf_tests.pl b/src/test/run_ovf_tests.pl
index c433c9d..e92258d 100755
--- a/src/test/run_ovf_tests.pl
+++ b/src/test/run_ovf_tests.pl
@@ -72,5 +72,6 @@ is($win10noNs->{qm}->{memory}, '6144', 'win10 VM (no default rasd NS) memory is
 is($win10noNs->{qm}->{cores}, '4', 'win10 VM (no default rasd NS) cores are correct');
 # older esxi/ovf standard used 'other' for windows10
 is($win10noNs->{qm}->{ostype}, 'other', 'win10 VM (no default rasd NS) ostype is correct');
+is($win10noNs->{qm}->{bios}, 'ovmf', 'win10 VM (no default rasd NS) bios is correct');
 
 done_testing();
-- 
2.39.5



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

[pve-devel] [PATCH storage v7 06/11] ovf: implement rudimentary boot order

[Dominik Csapak]

simply add all parsed disks to the boot order in the order we encounter
them (similar to the esxi plugin).

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
Reviewed-by: Fiona Ebner <f.ebner@proxmox.com>
---
 src/PVE/GuestImport/OVF.pm | 6 +++++-
 src/test/run_ovf_tests.pl  | 3 +++
 2 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/src/PVE/GuestImport/OVF.pm b/src/PVE/GuestImport/OVF.pm
index 08e9d0f..d08cc51 100644
--- a/src/PVE/GuestImport/OVF.pm
+++ b/src/PVE/GuestImport/OVF.pm
@@ -244,6 +244,8 @@ sub parse_ovf {
     # when all the nodes has been found out, we copy the relevant information to
     # a $pve_disk hash ref, which we push to @disks;
 
+    my $boot_order = [];
+
     foreach my $item_node (@disk_items) {
 
 	my $disk_node;
@@ -352,9 +354,11 @@ ovf:Item[rasd:InstanceID='%s']/rasd:ResourceType", $controller_id);
 	};
 	$pve_disk->{virtual_size} = $virtual_size if defined($virtual_size);
 	push @disks, $pve_disk;
-
+	push @$boot_order, $pve_disk_address;
     }
 
+    $qm->{boot} = "order=" . join(';', @$boot_order) if scalar(@$boot_order) > 0;
+
     return {qm => $qm, disks => \@disks};
 }
 
diff --git a/src/test/run_ovf_tests.pl b/src/test/run_ovf_tests.pl
index e92258d..3b04100 100755
--- a/src/test/run_ovf_tests.pl
+++ b/src/test/run_ovf_tests.pl
@@ -56,17 +56,20 @@ is($win10noNs->{disks}->[0]->{virtual_size}, 2048, 'single disk vm (no default r
 
 print "\ntesting vm.conf extraction\n";
 
+is($win2008->{qm}->{boot}, 'order=scsi0;scsi1', 'win2008 VM boot is correct');
 is($win2008->{qm}->{name}, 'Win2008-R2x64', 'win2008 VM name is correct');
 is($win2008->{qm}->{memory}, '2048', 'win2008 VM memory is correct');
 is($win2008->{qm}->{cores}, '1', 'win2008 VM cores are correct');
 is($win2008->{qm}->{ostype}, 'win7', 'win2008 VM ostype is correcty');
 
+is($win10->{qm}->{boot}, 'order=scsi0', 'win10 VM boot is correct');
 is($win10->{qm}->{name}, 'Win10-Liz', 'win10 VM name is correct');
 is($win10->{qm}->{memory}, '6144', 'win10 VM memory is correct');
 is($win10->{qm}->{cores}, '4', 'win10 VM cores are correct');
 # older esxi/ovf standard used 'other' for windows10
 is($win10->{qm}->{ostype}, 'other', 'win10 VM ostype is correct');
 
+is($win10noNs->{qm}->{boot}, 'order=scsi0', 'win10 VM (no default rasd NS) boot is correct');
 is($win10noNs->{qm}->{name}, 'Win10-Liz', 'win10 VM (no default rasd NS) name is correct');
 is($win10noNs->{qm}->{memory}, '6144', 'win10 VM (no default rasd NS) memory is correct');
 is($win10noNs->{qm}->{cores}, '4', 'win10 VM (no default rasd NS) cores are correct');
-- 
2.39.5



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

[pve-devel] [PATCH storage v7 07/11] ovf: implement parsing nics

[Dominik Csapak]

by iterating over the relevant parts and trying to parse out the
'ResourceSubType'. The content of that is not standardized, but I only
ever found examples that are compatible with vmware, meaning it's
either 'e1000', 'e1000e' or 'vmxnet3' (in various capitalizations; thus
the `lc()`)

As a fallback i used e1000, since that is our default too, and should
work for most guest operating systems.

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
Reviewed-by: Fiona Ebner <f.ebner@proxmox.com>
---
 src/PVE/GuestImport/OVF.pm   | 23 ++++++++++++++++++++++-
 src/PVE/Storage/DirPlugin.pm |  2 +-
 src/test/run_ovf_tests.pl    |  5 +++++
 3 files changed, 28 insertions(+), 2 deletions(-)

diff --git a/src/PVE/GuestImport/OVF.pm b/src/PVE/GuestImport/OVF.pm
index d08cc51..4af58ed 100644
--- a/src/PVE/GuestImport/OVF.pm
+++ b/src/PVE/GuestImport/OVF.pm
@@ -119,6 +119,12 @@ sub get_ostype {
     return $ostype_ids->{$id} // 'other';
 }
 
+my $allowed_nic_models = [
+    'e1000',
+    'e1000e',
+    'vmxnet3',
+];
+
 sub find_by {
     my ($key, $param) = @_;
     foreach my $resource (@resources) {
@@ -359,7 +365,22 @@ ovf:Item[rasd:InstanceID='%s']/rasd:ResourceType", $controller_id);
 
     $qm->{boot} = "order=" . join(';', @$boot_order) if scalar(@$boot_order) > 0;
 
-    return {qm => $qm, disks => \@disks};
+    my $nic_id = dtmf_name_to_id('Ethernet Adapter');
+    my $xpath_find_nics = "/ovf:Envelope/ovf:VirtualSystem/ovf:VirtualHardwareSection/ovf:Item[rasd:ResourceType=${nic_id}]";
+    my @nic_items = $xpc->findnodes($xpath_find_nics);
+
+    my $net = {};
+
+    my $net_count = 0;
+    for my $item_node (@nic_items) {
+	my $model = $xpc->findvalue('rasd:ResourceSubType', $item_node);
+	$model = lc($model);
+	$model = 'e1000' if ! grep { $_ eq $model } @$allowed_nic_models;
+	$net->{"net${net_count}"} = { model => $model };
+	$net_count++;
+    }
+
+    return {qm => $qm, disks => \@disks, net => $net};
 }
 
 1;
diff --git a/src/PVE/Storage/DirPlugin.pm b/src/PVE/Storage/DirPlugin.pm
index 0c32242..fb23e0a 100644
--- a/src/PVE/Storage/DirPlugin.pm
+++ b/src/PVE/Storage/DirPlugin.pm
@@ -293,7 +293,7 @@ sub get_import_metadata {
 	'create-args' => $res->{qm},
 	'disks' => $disks,
 	warnings => $warnings,
-	net => [],
+	net => $res->{net},
     };
 }
 
diff --git a/src/test/run_ovf_tests.pl b/src/test/run_ovf_tests.pl
index 3b04100..b8fa4b1 100755
--- a/src/test/run_ovf_tests.pl
+++ b/src/test/run_ovf_tests.pl
@@ -54,6 +54,11 @@ is($win10noNs->{disks}->[0]->{disk_address}, 'scsi0', 'single disk vm (no defaul
 is($win10noNs->{disks}->[0]->{backing_file}, "$test_manifests/Win10-Liz-disk1.vmdk", 'single disk vm (no default rasd NS) has the correct disk backing device');
 is($win10noNs->{disks}->[0]->{virtual_size}, 2048, 'single disk vm (no default rasd NS) has the correct size');
 
+print "testing nics\n";
+is($win2008->{net}->{net0}->{model}, 'e1000', 'win2008 has correct nic model');
+is($win10->{net}->{net0}->{model}, 'e1000e', 'win10 has correct nic model');
+is($win10noNs->{net}->{net0}->{model}, 'e1000e', 'win10 (no default rasd NS) has correct nic model');
+
 print "\ntesting vm.conf extraction\n";
 
 is($win2008->{qm}->{boot}, 'order=scsi0;scsi1', 'win2008 VM boot is correct');
-- 
2.39.5



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

[pve-devel] [PATCH storage v7 08/11] api: allow ova upload/download

[Dominik Csapak]

introducing a separate regex that only contains ova, since
upload/downloading ovfs does not make sense (since the disks are then
missing).

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
changes from v6:
* fix indentation/style
* omit check for '.ova' extension
* use assert_file_validity callback for download check

 src/PVE/API2/Storage/Status.pm | 55 +++++++++++++++++++++++++++++++---
 src/PVE/Storage.pm             | 11 +++++++
 2 files changed, 62 insertions(+), 4 deletions(-)

diff --git a/src/PVE/API2/Storage/Status.pm b/src/PVE/API2/Storage/Status.pm
index d30d8fb..e9a451c 100644
--- a/src/PVE/API2/Storage/Status.pm
+++ b/src/PVE/API2/Storage/Status.pm
@@ -41,6 +41,24 @@ __PACKAGE__->register_method ({
    path => '{storage}/file-restore',
 });
 
+my sub assert_ova_contents {
+    my ($file) = @_;
+
+    # test if it's really a tar file with an ovf file inside
+    my $hasOvf = 0;
+    run_command(['tar', '-t', '-f', $file], outfunc => sub {
+	my ($line) = @_;
+
+	if ($line =~ m/\.ovf$/) {
+	    $hasOvf = 1;
+	}
+    });
+
+    die "ova archive has no .ovf file inside\n" if !$hasOvf;
+
+    return 1;
+}
+
 __PACKAGE__->register_method ({
     name => 'index',
     path => '',
@@ -369,7 +387,7 @@ __PACKAGE__->register_method ({
     name => 'upload',
     path => '{storage}/upload',
     method => 'POST',
-    description => "Upload templates and ISO images.",
+    description => "Upload templates, ISO images and OVAs.",
     permissions => {
 	check => ['perm', '/storage/{storage}', ['Datastore.AllocateTemplate']],
     },
@@ -382,7 +400,7 @@ __PACKAGE__->register_method ({
 	    content => {
 		description => "Content type.",
 		type => 'string', format => 'pve-storage-content',
-		enum => ['iso', 'vztmpl'],
+		enum => ['iso', 'vztmpl', 'import'],
 	    },
 	    filename => {
 		description => "The name of the file to create. Caution: This will be normalized!",
@@ -437,6 +455,7 @@ __PACKAGE__->register_method ({
 	my $filename = PVE::Storage::normalize_content_filename($param->{filename});
 
 	my $path;
+	my $isOva = 0;
 
 	if ($content eq 'iso') {
 	    if ($filename !~ m![^/]+$PVE::Storage::ISO_EXT_RE_0$!) {
@@ -448,6 +467,13 @@ __PACKAGE__->register_method ({
 		raise_param_exc({ filename => "wrong file extension" });
 	    }
 	    $path = PVE::Storage::get_vztmpl_dir($cfg, $param->{storage});
+	} elsif ($content eq 'import') {
+	    if ($filename !~ m!${PVE::Storage::SAFE_CHAR_CLASS_RE}+$PVE::Storage::UPLOAD_IMPORT_EXT_RE_1$!) {
+		raise_param_exc({ filename => "invalid filename or wrong extension" });
+	    }
+
+	    $isOva = 1;
+	    $path = PVE::Storage::get_import_dir($cfg, $param->{storage});
 	} else {
 	    raise_param_exc({ content => "upload content type '$content' not allowed" });
 	}
@@ -514,6 +540,10 @@ __PACKAGE__->register_method ({
 		if ($content eq 'iso') {
 		    PVE::Storage::assert_iso_content($tmpfilename);
 		}
+
+		if ($isOva) {
+		    assert_ova_contents($tmpfilename);
+		}
 	    };
 	    if (my $err = $@) {
 		# unlinks only the temporary file from the http server
@@ -548,7 +578,7 @@ __PACKAGE__->register_method({
     name => 'download_url',
     path => '{storage}/download-url',
     method => 'POST',
-    description => "Download templates and ISO images by using an URL.",
+    description => "Download templates, ISO images and OVAs by using an URL.",
     proxyto => 'node',
     permissions => {
 	description => 'Requires allocation access on the storage and as this allows one to probe'
@@ -576,7 +606,7 @@ __PACKAGE__->register_method({
 	    content => {
 		description => "Content type.", # TODO: could be optional & detected in most cases
 		type => 'string', format => 'pve-storage-content',
-		enum => ['iso', 'vztmpl'],
+		enum => ['iso', 'vztmpl', 'import'],
 	    },
 	    filename => {
 		description => "The name of the file to create. Caution: This will be normalized!",
@@ -636,6 +666,8 @@ __PACKAGE__->register_method({
 	my $filename = PVE::Storage::normalize_content_filename($param->{filename});
 
 	my $path;
+	my $isOva = 0;
+
 	if ($content eq 'iso') {
 	    if ($filename !~ m![^/]+$PVE::Storage::ISO_EXT_RE_0$!) {
 		raise_param_exc({ filename => "wrong file extension" });
@@ -646,6 +678,16 @@ __PACKAGE__->register_method({
 		raise_param_exc({ filename => "wrong file extension" });
 	    }
 	    $path = PVE::Storage::get_vztmpl_dir($cfg, $storage);
+	} elsif ($content eq 'import') {
+	    if ($filename !~ m!${PVE::Storage::SAFE_CHAR_CLASS_RE}+$PVE::Storage::UPLOAD_IMPORT_EXT_RE_1$!) {
+		raise_param_exc({ filename => "invalid filename or wrong extension" });
+	    }
+
+	    if ($filename =~ m/\.ova$/) {
+		$isOva = 1;
+	    }
+
+	    $path = PVE::Storage::get_import_dir($cfg, $param->{storage});
 	} else {
 	    raise_param_exc({ content => "upload content-type '$content' is not allowed" });
 	}
@@ -672,6 +714,10 @@ __PACKAGE__->register_method({
 	    if ($content eq 'iso') {
 		PVE::Storage::assert_iso_content($tmp_path);
 	    }
+
+	    if ($isOva) {
+		assert_ova_contents($tmp_path);
+	    }
 	};
 
 	my $worker = sub {
@@ -681,6 +727,7 @@ __PACKAGE__->register_method({
 		die "no decompression method found\n" if !$info->{decompressor};
 		$opts->{decompression_command} = $info->{decompressor};
 	    }
+
 	    PVE::Tools::download_file_from_url("$path/$filename", $url, $opts);
 	};
 
diff --git a/src/PVE/Storage.pm b/src/PVE/Storage.pm
index f60c5fc..de730b7 100755
--- a/src/PVE/Storage.pm
+++ b/src/PVE/Storage.pm
@@ -116,6 +116,8 @@ our $BACKUP_EXT_RE_2 = qr/\.(tgz|(?:tar|vma)(?:\.(${\PVE::Storage::Plugin::COMPR
 
 our $IMPORT_EXT_RE_1 = qr/\.(ova|ovf|qcow2|raw|vmdk)/;
 
+our $UPLOAD_IMPORT_EXT_RE_1 = qr/\.(ova)/;
+
 our $SAFE_CHAR_CLASS_RE = qr/[a-zA-Z0-9\-\.\+\=\_]/;
 
 our $OVA_CONTENT_RE_1 = qr/${SAFE_CHAR_CLASS_RE}+\.(qcow2|raw|vmdk)/;
@@ -466,6 +468,15 @@ sub get_iso_dir {
     return $plugin->get_subdir($scfg, 'iso');
 }
 
+sub get_import_dir {
+    my ($cfg, $storeid) = @_;
+
+    my $scfg = storage_config($cfg, $storeid);
+    my $plugin = PVE::Storage::Plugin->lookup($scfg->{type});
+
+    return $plugin->get_subdir($scfg, 'import');
+}
+
 sub get_vztmpl_dir {
     my ($cfg, $storeid) = @_;
 
-- 
2.39.5



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

[pve-devel] [PATCH storage v7 09/11] plugin: enable import for nfs/btrfs/cifs/cephfs/glusterfs

[Dominik Csapak]

and reuse the DirPlugin implementation

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
Reviewed-by: Fiona Ebner <f.ebner@proxmox.com>
---
 src/PVE/Storage/BTRFSPlugin.pm     | 5 +++++
 src/PVE/Storage/CIFSPlugin.pm      | 6 +++++-
 src/PVE/Storage/CephFSPlugin.pm    | 6 +++++-
 src/PVE/Storage/GlusterfsPlugin.pm | 6 +++++-
 src/PVE/Storage/NFSPlugin.pm       | 6 +++++-
 5 files changed, 25 insertions(+), 4 deletions(-)

diff --git a/src/PVE/Storage/BTRFSPlugin.pm b/src/PVE/Storage/BTRFSPlugin.pm
index abc5bba..d28e681 100644
--- a/src/PVE/Storage/BTRFSPlugin.pm
+++ b/src/PVE/Storage/BTRFSPlugin.pm
@@ -40,6 +40,7 @@ sub plugindata {
 		backup => 1,
 		snippets => 1,
 		none => 1,
+		import => 1,
 	    },
 	    { images => 1, rootdir => 1 },
 	],
@@ -963,4 +964,8 @@ sub rename_volume {
     return "${storeid}:$target_volname";
 }
 
+sub get_import_metadata {
+    return PVE::Storage::DirPlugin::get_import_metadata(@_);
+}
+
 1
diff --git a/src/PVE/Storage/CIFSPlugin.pm b/src/PVE/Storage/CIFSPlugin.pm
index 2184471..475065a 100644
--- a/src/PVE/Storage/CIFSPlugin.pm
+++ b/src/PVE/Storage/CIFSPlugin.pm
@@ -99,7 +99,7 @@ sub type {
 sub plugindata {
     return {
 	content => [ { images => 1, rootdir => 1, vztmpl => 1, iso => 1,
-		   backup => 1, snippets => 1}, { images => 1 }],
+		   backup => 1, snippets => 1, import => 1}, { images => 1 }],
 	format => [ { raw => 1, qcow2 => 1, vmdk => 1 } , 'raw' ],
     };
 }
@@ -314,4 +314,8 @@ sub update_volume_attribute {
     return PVE::Storage::DirPlugin::update_volume_attribute(@_);
 }
 
+sub get_import_metadata {
+    return PVE::Storage::DirPlugin::get_import_metadata(@_);
+}
+
 1;
diff --git a/src/PVE/Storage/CephFSPlugin.pm b/src/PVE/Storage/CephFSPlugin.pm
index 8aad518..36c64ea 100644
--- a/src/PVE/Storage/CephFSPlugin.pm
+++ b/src/PVE/Storage/CephFSPlugin.pm
@@ -116,7 +116,7 @@ sub type {
 
 sub plugindata {
     return {
-	content => [ { vztmpl => 1, iso => 1, backup => 1, snippets => 1},
+	content => [ { vztmpl => 1, iso => 1, backup => 1, snippets => 1, import => 1 },
 		     { backup => 1 }],
     };
 }
@@ -261,4 +261,8 @@ sub update_volume_attribute {
     return PVE::Storage::DirPlugin::update_volume_attribute(@_);
 }
 
+sub get_import_metadata {
+    return PVE::Storage::DirPlugin::get_import_metadata(@_);
+}
+
 1;
diff --git a/src/PVE/Storage/GlusterfsPlugin.pm b/src/PVE/Storage/GlusterfsPlugin.pm
index 2b7f9e1..9d17180 100644
--- a/src/PVE/Storage/GlusterfsPlugin.pm
+++ b/src/PVE/Storage/GlusterfsPlugin.pm
@@ -97,7 +97,7 @@ sub type {
 
 sub plugindata {
     return {
-	content => [ { images => 1, vztmpl => 1, iso => 1, backup => 1, snippets => 1},
+	content => [ { images => 1, vztmpl => 1, iso => 1, backup => 1, snippets => 1, import => 1},
 		     { images => 1 }],
 	format => [ { raw => 1, qcow2 => 1, vmdk => 1 } , 'raw' ],
     };
@@ -352,4 +352,8 @@ sub check_connection {
     return defined($server) ? 1 : 0;
 }
 
+sub get_import_metadata {
+    return PVE::Storage::DirPlugin::get_import_metadata(@_);
+}
+
 1;
diff --git a/src/PVE/Storage/NFSPlugin.pm b/src/PVE/Storage/NFSPlugin.pm
index f2e4c0d..72e9c6d 100644
--- a/src/PVE/Storage/NFSPlugin.pm
+++ b/src/PVE/Storage/NFSPlugin.pm
@@ -53,7 +53,7 @@ sub type {
 
 sub plugindata {
     return {
-	content => [ { images => 1, rootdir => 1, vztmpl => 1, iso => 1, backup => 1, snippets => 1 },
+	content => [ { images => 1, rootdir => 1, vztmpl => 1, iso => 1, backup => 1, snippets => 1, import => 1 },
 		     { images => 1 }],
 	format => [ { raw => 1, qcow2 => 1, vmdk => 1 } , 'raw' ],
     };
@@ -223,4 +223,8 @@ sub update_volume_attribute {
     return PVE::Storage::DirPlugin::update_volume_attribute(@_);
 }
 
+sub get_import_metadata {
+    return PVE::Storage::DirPlugin::get_import_metadata(@_);
+}
+
 1;
-- 
2.39.5



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

[pve-devel] [PATCH storage v7 10/11] add 'import' content type to 'check_volume_access'

[Dominik Csapak]

in the same branch as 'vztmpl' and 'iso'

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
Reviewed-by: Fiona Ebner <f.ebner@proxmox.com>
---
 src/PVE/Storage.pm | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/PVE/Storage.pm b/src/PVE/Storage.pm
index de730b7..1e12b51 100755
--- a/src/PVE/Storage.pm
+++ b/src/PVE/Storage.pm
@@ -542,7 +542,7 @@ sub check_volume_access {
 
 	return if $rpcenv->check($user, "/storage/$sid", ['Datastore.Allocate'], 1);
 
-	if ($vtype eq 'iso' || $vtype eq 'vztmpl') {
+	if ($vtype eq 'iso' || $vtype eq 'vztmpl' || $vtype eq 'import') {
 	    # require at least read access to storage, (custom) templates/ISOs could be sensitive
 	    $rpcenv->check_any($user, "/storage/$sid", ['Datastore.AllocateSpace', 'Datastore.Audit']);
 	} elsif (defined($ownervm) && defined($vmid) && ($ownervm == $vmid)) {
-- 
2.39.5



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

[pve-devel] [PATCH storage v7 11/11] plugin: file_size_info: warn on parent images with unusual path

[Dominik Csapak]

If the base image (parent) of an image contains e.g. whitespace in it's
path, the current untainting would not match and it would seem there was
no parent.

Since untrusted files are not allowed to have backing parts, just warn,
when encountering this case to keep backwards compatibility.

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
changes from v6:
* only warn if we find invalid characters but keep behaviour, for
  backwards compat for existing setups

 src/PVE/Storage/Plugin.pm | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/PVE/Storage/Plugin.pm b/src/PVE/Storage/Plugin.pm
index 8f1c7c8..2407eba 100644
--- a/src/PVE/Storage/Plugin.pm
+++ b/src/PVE/Storage/Plugin.pm
@@ -1031,6 +1031,7 @@ sub file_size_info {
     ($format) = ($format =~ /^(\S+)$/); # untaint
     die "format '$format' includes whitespace\n" if !defined($format);
     if (defined($parent)) {
+	warn "strange parent name path '$parent' found\n" if $parent =~ m/[^\S]/;
 	($parent) = ($parent =~ /^(\S+)$/); # untaint
     }
     return wantarray ? ($size, $format, $used, $parent, $st->ctime) : $size;
-- 
2.39.5



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

[pve-devel] [PATCH qemu-server v7 1/5] disk import: add additional safeguards for imported image files

[Dominik Csapak]

From: Fabian Grünbichler <f.gruenbichler@proxmox.com>

creating non-raw disk images with arbitrary content is only possible with raw
access to the storage, but checking for references to external files doesn't
hurt, in case for non pve-managed volumes.

Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
 [ DC: removed problematic checks for pve-managed volumes ]
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
changes from v6:
* remove unrelated hunks

 PVE/API2/Qemu.pm | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
index c50c42be..528ec6d1 100644
--- a/PVE/API2/Qemu.pm
+++ b/PVE/API2/Qemu.pm
@@ -442,7 +442,8 @@ my sub create_disks : prototype($$$$$$$$$$) {
 		    }
 		} else {
 		    $source = PVE::Storage::abs_filesystem_path($storecfg, $source, 1);
-		    ($size, my $source_format) = PVE::Storage::file_size_info($source);
+		    # check potentially untrusted image file!
+		    ($size, my $source_format) = PVE::Storage::file_size_info($source, undef, 1);
 		    die "could not get file size of $source\n" if !$size;
 
 		    if ($live_import && $ds ne 'efidisk0') {
-- 
2.39.5



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

[pve-devel] [PATCH qemu-server v7 2/5] use OVF from Storage

[Dominik Csapak]

and delete it here (incl tests; they live in pve-storage now).

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
changes from v6:
* group imports correctly

 PVE/CLI/qm.pm                                 |   4 +-
 PVE/QemuServer/Makefile                       |   1 -
 PVE/QemuServer/OVF.pm                         | 242 ------------------
 debian/control                                |   2 -
 test/Makefile                                 |   5 +-
 test/ovf_manifests/Win10-Liz-disk1.vmdk       | Bin 65536 -> 0 bytes
 test/ovf_manifests/Win10-Liz.ovf              | 142 ----------
 .../ovf_manifests/Win10-Liz_no_default_ns.ovf | 142 ----------
 test/ovf_manifests/Win_2008_R2_two-disks.ovf  | 145 -----------
 test/ovf_manifests/disk1.vmdk                 | Bin 65536 -> 0 bytes
 test/ovf_manifests/disk2.vmdk                 | Bin 65536 -> 0 bytes
 test/run_ovf_tests.pl                         |  71 -----
 12 files changed, 3 insertions(+), 751 deletions(-)
 delete mode 100644 PVE/QemuServer/OVF.pm
 delete mode 100644 test/ovf_manifests/Win10-Liz-disk1.vmdk
 delete mode 100755 test/ovf_manifests/Win10-Liz.ovf
 delete mode 100755 test/ovf_manifests/Win10-Liz_no_default_ns.ovf
 delete mode 100755 test/ovf_manifests/Win_2008_R2_two-disks.ovf
 delete mode 100644 test/ovf_manifests/disk1.vmdk
 delete mode 100644 test/ovf_manifests/disk2.vmdk
 delete mode 100755 test/run_ovf_tests.pl

diff --git a/PVE/CLI/qm.pm b/PVE/CLI/qm.pm
index bce87020..b562f2ee 100755
--- a/PVE/CLI/qm.pm
+++ b/PVE/CLI/qm.pm
@@ -19,6 +19,7 @@ use PVE::APIClient::LWP;
 use PVE::Cluster;
 use PVE::Exception qw(raise_param_exc);
 use PVE::GuestHelpers;
+use PVE::GuestImport::OVF;
 use PVE::INotify;
 use PVE::JSONSchema qw(get_standard_option);
 use PVE::Network;
@@ -34,7 +35,6 @@ use PVE::QemuServer::Helpers;
 use PVE::QemuServer::Agent qw(agent_available);
 use PVE::QemuServer::ImportDisk;
 use PVE::QemuServer::Monitor qw(mon_cmd);
-use PVE::QemuServer::OVF;
 use PVE::QemuServer::QMPHelpers;
 use PVE::QemuServer;
 
@@ -756,7 +756,7 @@ __PACKAGE__->register_method ({
 	my $storecfg = PVE::Storage::config();
 	PVE::Storage::storage_check_enabled($storecfg, $storeid);
 
-	my $parsed = PVE::QemuServer::OVF::parse_ovf($ovf_file);
+	my $parsed = PVE::GuestImport::OVF::parse_ovf($ovf_file);
 
 	if ($dryrun) {
 	    print to_json($parsed, { pretty => 1, canonical => 1});
diff --git a/PVE/QemuServer/Makefile b/PVE/QemuServer/Makefile
index ac26e56f..89d12091 100644
--- a/PVE/QemuServer/Makefile
+++ b/PVE/QemuServer/Makefile
@@ -2,7 +2,6 @@ SOURCES=PCI.pm		\
 	USB.pm		\
 	Memory.pm	\
 	ImportDisk.pm	\
-	OVF.pm		\
 	Cloudinit.pm	\
 	Agent.pm	\
 	Helpers.pm	\
diff --git a/PVE/QemuServer/OVF.pm b/PVE/QemuServer/OVF.pm
deleted file mode 100644
index eb9cf8e8..00000000
--- a/PVE/QemuServer/OVF.pm
+++ /dev/null
@@ -1,242 +0,0 @@
-# Open Virtualization Format import routines
-# https://www.dmtf.org/standards/ovf
-package PVE::QemuServer::OVF;
-
-use strict;
-use warnings;
-
-use XML::LibXML;
-use File::Spec;
-use File::Basename;
-use Data::Dumper;
-use Cwd 'realpath';
-
-use PVE::Tools;
-use PVE::Storage;
-
-# map OVF resources types to descriptive strings
-# this will allow us to explore the xml tree without using magic numbers
-# http://schemas.dmtf.org/wbem/cim-html/2/CIM_ResourceAllocationSettingData.html
-my @resources = (
-    { id => 1, dtmf_name => 'Other' },
-    { id => 2, dtmf_name => 'Computer System' },
-    { id => 3, dtmf_name => 'Processor' },
-    { id => 4, dtmf_name => 'Memory' },
-    { id => 5, dtmf_name => 'IDE Controller', pve_type => 'ide' },
-    { id => 6, dtmf_name => 'Parallel SCSI HBA', pve_type => 'scsi' },
-    { id => 7, dtmf_name => 'FC HBA' },
-    { id => 8, dtmf_name => 'iSCSI HBA' },
-    { id => 9, dtmf_name => 'IB HCA' },
-    { id => 10, dtmf_name => 'Ethernet Adapter' },
-    { id => 11, dtmf_name => 'Other Network Adapter' },
-    { id => 12, dtmf_name => 'I/O Slot' },
-    { id => 13, dtmf_name => 'I/O Device' },
-    { id => 14, dtmf_name => 'Floppy Drive' },
-    { id => 15, dtmf_name => 'CD Drive' },
-    { id => 16, dtmf_name => 'DVD drive' },
-    { id => 17, dtmf_name => 'Disk Drive' },
-    { id => 18, dtmf_name => 'Tape Drive' },
-    { id => 19, dtmf_name => 'Storage Extent' },
-    { id => 20, dtmf_name => 'Other storage device', pve_type => 'sata'},
-    { id => 21, dtmf_name => 'Serial port' },
-    { id => 22, dtmf_name => 'Parallel port' },
-    { id => 23, dtmf_name => 'USB Controller' },
-    { id => 24, dtmf_name => 'Graphics controller' },
-    { id => 25, dtmf_name => 'IEEE 1394 Controller' },
-    { id => 26, dtmf_name => 'Partitionable Unit' },
-    { id => 27, dtmf_name => 'Base Partitionable Unit' },
-    { id => 28, dtmf_name => 'Power' },
-    { id => 29, dtmf_name => 'Cooling Capacity' },
-    { id => 30, dtmf_name => 'Ethernet Switch Port' },
-    { id => 31, dtmf_name => 'Logical Disk' },
-    { id => 32, dtmf_name => 'Storage Volume' },
-    { id => 33, dtmf_name => 'Ethernet Connection' },
-    { id => 34, dtmf_name => 'DMTF reserved' },
-    { id => 35, dtmf_name => 'Vendor Reserved'}
-);
-
-sub find_by {
-    my ($key, $param) = @_;
-    foreach my $resource (@resources) {
-	if ($resource->{$key} eq $param) {
-	    return ($resource);
-	}
-    }
-    return;
-}
-
-sub dtmf_name_to_id {
-    my ($dtmf_name) = @_;
-    my $found = find_by('dtmf_name', $dtmf_name);
-    if ($found) {
-	return $found->{id};
-    } else {
-	return;
-    }
-}
-
-sub id_to_pve {
-    my ($id) = @_;
-    my $resource = find_by('id', $id);
-    if ($resource) {
-	return $resource->{pve_type};
-    } else {
-	return;
-    }
-}
-
-# returns two references, $qm which holds qm.conf style key/values, and \@disks
-sub parse_ovf {
-    my ($ovf, $debug) = @_;
-
-    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
-    my $xpc = XML::LibXML::XPathContext->new($dom);
-    $xpc->registerNs('ovf', 'http://schemas.dmtf.org/ovf/envelope/1');
-    $xpc->registerNs('rasd', 'http://schemas.dmtf.org/wbem/wscim/1/cim-schema/2/CIM_ResourceAllocationSettingData');
-    $xpc->registerNs('vssd', 'http://schemas.dmtf.org/wbem/wscim/1/cim-schema/2/CIM_VirtualSystemSettingData');
-
-
-    # hash to save qm.conf parameters
-    my $qm;
-
-    #array to save a disk list
-    my @disks;
-
-    # easy xpath
-    # walk down the dom until we find the matching XML element
-    my $xpath_find_name = "/ovf:Envelope/ovf:VirtualSystem/ovf:Name";
-    my $ovf_name = $xpc->findvalue($xpath_find_name);
-
-    if ($ovf_name) {
-	# PVE::QemuServer::confdesc requires a valid DNS name
-	($qm->{name} = $ovf_name) =~ s/[^a-zA-Z0-9\-\.]//g;
-    } else {
-	warn "warning: unable to parse the VM name in this OVF manifest, generating a default value\n";
-    }
-
-    # middle level xpath
-    # element[child] search the elements which have this [child]
-    my $processor_id = dtmf_name_to_id('Processor');
-    my $xpath_find_vcpu_count = "/ovf:Envelope/ovf:VirtualSystem/ovf:VirtualHardwareSection/ovf:Item[rasd:ResourceType=${processor_id}]/rasd:VirtualQuantity";
-    $qm->{'cores'} = $xpc->findvalue($xpath_find_vcpu_count);
-
-    my $memory_id = dtmf_name_to_id('Memory');
-    my $xpath_find_memory = ("/ovf:Envelope/ovf:VirtualSystem/ovf:VirtualHardwareSection/ovf:Item[rasd:ResourceType=${memory_id}]/rasd:VirtualQuantity");
-    $qm->{'memory'} = $xpc->findvalue($xpath_find_memory);
-
-    # middle level xpath
-    # here we expect multiple results, so we do not read the element value with
-    # findvalue() but store multiple elements with findnodes()
-    my $disk_id = dtmf_name_to_id('Disk Drive');
-    my $xpath_find_disks="/ovf:Envelope/ovf:VirtualSystem/ovf:VirtualHardwareSection/ovf:Item[rasd:ResourceType=${disk_id}]";
-    my @disk_items = $xpc->findnodes($xpath_find_disks);
-
-    # disks metadata is split in four different xml elements:
-    # * as an Item node of type DiskDrive in the VirtualHardwareSection
-    # * as an Disk node in the DiskSection
-    # * as a File node in the References section
-    # * each Item node also holds a reference to its owning controller
-    #
-    # we iterate over the list of Item nodes of type disk drive, and for each item,
-    # find the corresponding Disk node, and File node and owning controller
-    # when all the nodes has been found out, we copy the relevant information to
-    # a $pve_disk hash ref, which we push to @disks;
-
-    foreach my $item_node (@disk_items) {
-
-	my $disk_node;
-	my $file_node;
-	my $controller_node;
-	my $pve_disk;
-
-	print "disk item:\n", $item_node->toString(1), "\n" if $debug;
-
-	# from Item, find corresponding Disk node
-	# here the dot means the search should start from the current element in dom
-	my $host_resource = $xpc->findvalue('rasd:HostResource', $item_node);
-	my $disk_section_path;
-	my $disk_id;
-
-	# RFC 3986 "2.3.  Unreserved Characters"
-	my $valid_uripath_chars = qr/[[:alnum:]]|[\-\._~]/;
-
-	if ($host_resource =~ m|^ovf:/(${valid_uripath_chars}+)/(${valid_uripath_chars}+)$|) {
-	    $disk_section_path = $1;
-	    $disk_id = $2;
-	} else {
-	   warn "invalid host resource $host_resource, skipping\n";
-	   next;
-	}
-	printf "disk section path: $disk_section_path and disk id: $disk_id\n" if $debug;
-
-	# tricky xpath
-	# @ means we filter the result query based on a the value of an item attribute ( @ = attribute)
-	# @ 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 $fileref = $xpc->findvalue($xpath_find_fileref);
-
-	my $valid_url_chars = qr@${valid_uripath_chars}|/@;
-	if (!$fileref || $fileref !~ m/^${valid_url_chars}+$/) {
-	    warn "invalid host resource $host_resource, skipping\n";
-	    next;
-	}
-
-	# from Disk Node, find corresponding filepath
-	my $xpath_find_filepath = sprintf("/ovf:Envelope/ovf:References/ovf:File[\@ovf:id='%s']/\@ovf:href", $fileref);
-	my $filepath = $xpc->findvalue($xpath_find_filepath);
-	if (!$filepath) {
-	    warn "invalid file reference $fileref, skipping\n";
-	    next;
-	}
-	print "file path: $filepath\n" if $debug;
-
-	# from Item, find owning Controller type
-	my $controller_id = $xpc->findvalue('rasd:Parent', $item_node);
-	my $xpath_find_parent_type = sprintf("/ovf:Envelope/ovf:VirtualSystem/ovf:VirtualHardwareSection/\
-ovf:Item[rasd:InstanceID='%s']/rasd:ResourceType", $controller_id);
-	my $controller_type = $xpc->findvalue($xpath_find_parent_type);
-	if (!$controller_type) {
-	    warn "invalid or missing controller: $controller_type, skipping\n";
-	    next;
-	}
-	print "owning controller type: $controller_type\n" if $debug;
-
-	# extract corresponding Controller node details
-	my $adress_on_controller = $xpc->findvalue('rasd:AddressOnParent', $item_node);
-	my $pve_disk_address = id_to_pve($controller_type) . $adress_on_controller;
-
-	# resolve symlinks and relative path components
-	# and die if the diskimage is not somewhere under the $ovf path
-	my $ovf_dir = realpath(dirname(File::Spec->rel2abs($ovf)));
-	my $backing_file_path = realpath(join ('/', $ovf_dir, $filepath));
-	if ($backing_file_path !~ /^\Q${ovf_dir}\E/) {
-	    die "error parsing $filepath, are you using a symlink ?\n";
-	}
-
-	if (!-e $backing_file_path) {
-	    die "error parsing $filepath, file seems not to exist at $backing_file_path\n";
-	}
-
-	($backing_file_path) = $backing_file_path =~ m|^(/.*)|; # untaint
-
-	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;
-
-	$pve_disk = {
-	    disk_address => $pve_disk_address,
-	    backing_file => $backing_file_path,
-	    virtual_size => $virtual_size
-	};
-	push @disks, $pve_disk;
-
-    }
-
-    return {qm => $qm, disks => \@disks};
-}
-
-1;
diff --git a/debian/control b/debian/control
index aa5f4c6d..33012650 100644
--- a/debian/control
+++ b/debian/control
@@ -14,7 +14,6 @@ Build-Depends: debhelper-compat (= 13),
                libtest-mockmodule-perl,
                liburi-perl,
                libuuid-perl,
-               libxml-libxml-perl,
                lintian,
                perl,
                pkg-config,
@@ -44,7 +43,6 @@ Depends: dbus,
          libterm-readline-gnu-perl,
          liburi-perl,
          libuuid-perl,
-         libxml-libxml-perl,
          perl (>= 5.10.0-19),
          proxmox-websocket-tunnel,
          pve-cluster,
diff --git a/test/Makefile b/test/Makefile
index 9e6d39e8..65ed7bc4 100644
--- a/test/Makefile
+++ b/test/Makefile
@@ -1,14 +1,11 @@
 all: test
 
-test: test_snapshot test_ovf test_cfg_to_cmd test_pci_addr_conflicts test_qemu_img_convert test_migration test_restore_config
+test: test_snapshot test_cfg_to_cmd test_pci_addr_conflicts test_qemu_img_convert test_migration test_restore_config
 
 test_snapshot: run_snapshot_tests.pl
 	./run_snapshot_tests.pl
 	./test_get_replicatable_volumes.pl
 
-test_ovf: run_ovf_tests.pl
-	./run_ovf_tests.pl
-
 test_cfg_to_cmd: run_config2command_tests.pl cfg2cmd/*.conf
 	perl -I../ ./run_config2command_tests.pl
 
diff --git a/test/ovf_manifests/Win10-Liz-disk1.vmdk b/test/ovf_manifests/Win10-Liz-disk1.vmdk
deleted file mode 100644
index 662354a3d1333a2f6c4364005e53bfe7cd8b9044..0000000000000000000000000000000000000000
GIT binary patch
literal 0
HcmV?d00001

literal 65536
zcmeIvy>HV%7zbeUF`dK)46s<q+^B&Pi6H|eMIb;zP1VdMK8V$P$u?2T)IS|NNta69
zSXw<No$qu%-}$}AUq|21A0<ihr0Gwa-nQ%QGfCR@wmshsN%A;JUhL<u_T%+U7Sd<o
zW^TMU0^M{}R2S(eR@1Ur*Q@eVF^^#r%c@u{hyC#J%V?Nq?*?z)=UG^1Wn9+n(yx6B
z(=ujtJiA)QVP~;guI5EOE2iV-%_??6=%y!^b+aeU_aA6Z4X2azC>{U!a5_FoJCkDB
zKRozW{5{B<Li)YUBEQ&fJe$RRZCRbA$5|CacQiT<A<uvIHbq(g$>yIY=etVNVcI$B
zY@^?CwTN|j)tg?;i)G&AZFqPqoW(5P2K~XUq>9sqVVe!!?y@Y;)^#k~TefEvd2_XU
z^M@5mfB*pk1PBlyK!5-N0t5&UAV7cs0RjXF5FkK+009C72oNAZfB*pk4^iOdL%ftb
z5g<T-009C72;3>~`p!f^fB*pk1PBlyK!5-N0t5&UAV7cs0RjXF5FkK+009C72oNAZ
zfB*pk1PBlyK!5-N0t5&UAV7cs0RjXF5FkK+009C72oNAZfB*pk1PBlyK!5-N0t5&U
zAV7cs0RjXF5FkK+009C72oNAZfB*pk1PBlyK!5-N0t5&UAV7cs0RjXF5FkK+009C7
z2oNAZfB*pk1PBlyK!5-N0t5&UAV7cs0RjXF5FkK+009C72oNAZfB*pk1PBlyK!5-N
p0t5&UAV7cs0RjXF5FkK+009C72oNAZfB*pk1PBlyK;Zui`~%h>XmtPp

diff --git a/test/ovf_manifests/Win10-Liz.ovf b/test/ovf_manifests/Win10-Liz.ovf
deleted file mode 100755
index 46642c04..00000000
--- a/test/ovf_manifests/Win10-Liz.ovf
+++ /dev/null
@@ -1,142 +0,0 @@
-<?xml version="1.0" encoding="UTF-8"?>
-<!--Generated by VMware ovftool 4.1.0 (build-2982904), UTC time: 2017-02-07T13:50:15.265014Z-->
-<Envelope vmw:buildId="build-2982904" xmlns="http://schemas.dmtf.org/ovf/envelope/1" xmlns:cim="http://schemas.dmtf.org/wbem/wscim/1/common" xmlns:ovf="http://schemas.dmtf.org/ovf/envelope/1" xmlns:rasd="http://schemas.dmtf.org/wbem/wscim/1/cim-schema/2/CIM_ResourceAllocationSettingData" xmlns:vmw="http://www.vmware.com/schema/ovf" xmlns:vssd="http://schemas.dmtf.org/wbem/wscim/1/cim-schema/2/CIM_VirtualSystemSettingData" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance">
-  <References>
-    <File ovf:href="Win10-Liz-disk1.vmdk" ovf:id="file1" ovf:size="9155243008"/>
-  </References>
-  <DiskSection>
-    <Info>Virtual disk information</Info>
-    <Disk ovf:capacity="128" ovf:capacityAllocationUnits="byte * 2^30" ovf:diskId="vmdisk1" ovf:fileRef="file1" ovf:format="http://www.vmware.com/interfaces/specifications/vmdk.html#streamOptimized" ovf:populatedSize="16798056448"/>
-  </DiskSection>
-  <NetworkSection>
-    <Info>The list of logical networks</Info>
-    <Network ovf:name="bridged">
-      <Description>The bridged network</Description>
-    </Network>
-  </NetworkSection>
-  <VirtualSystem ovf:id="vm">
-    <Info>A virtual machine</Info>
-    <Name>Win10-Liz</Name>
-    <OperatingSystemSection ovf:id="1" vmw:osType="windows9_64Guest">
-      <Info>The kind of installed guest operating system</Info>
-    </OperatingSystemSection>
-    <VirtualHardwareSection>
-      <Info>Virtual hardware requirements</Info>
-      <System>
-        <vssd:ElementName>Virtual Hardware Family</vssd:ElementName>
-        <vssd:InstanceID>0</vssd:InstanceID>
-        <vssd:VirtualSystemIdentifier>Win10-Liz</vssd:VirtualSystemIdentifier>
-        <vssd:VirtualSystemType>vmx-11</vssd:VirtualSystemType>
-      </System>
-      <Item>
-        <rasd:AllocationUnits>hertz * 10^6</rasd:AllocationUnits>
-        <rasd:Description>Number of Virtual CPUs</rasd:Description>
-        <rasd:ElementName>4 virtual CPU(s)</rasd:ElementName>
-        <rasd:InstanceID>1</rasd:InstanceID>
-        <rasd:ResourceType>3</rasd:ResourceType>
-        <rasd:VirtualQuantity>4</rasd:VirtualQuantity>
-      </Item>
-      <Item>
-        <rasd:AllocationUnits>byte * 2^20</rasd:AllocationUnits>
-        <rasd:Description>Memory Size</rasd:Description>
-        <rasd:ElementName>6144MB of memory</rasd:ElementName>
-        <rasd:InstanceID>2</rasd:InstanceID>
-        <rasd:ResourceType>4</rasd:ResourceType>
-        <rasd:VirtualQuantity>6144</rasd:VirtualQuantity>
-      </Item>
-      <Item>
-        <rasd:Address>0</rasd:Address>
-        <rasd:Description>SATA Controller</rasd:Description>
-        <rasd:ElementName>sataController0</rasd:ElementName>
-        <rasd:InstanceID>3</rasd:InstanceID>
-        <rasd:ResourceSubType>vmware.sata.ahci</rasd:ResourceSubType>
-        <rasd:ResourceType>20</rasd:ResourceType>
-      </Item>
-      <Item ovf:required="false">
-        <rasd:Address>0</rasd:Address>
-        <rasd:Description>USB Controller (XHCI)</rasd:Description>
-        <rasd:ElementName>usb3</rasd:ElementName>
-        <rasd:InstanceID>4</rasd:InstanceID>
-        <rasd:ResourceSubType>vmware.usb.xhci</rasd:ResourceSubType>
-        <rasd:ResourceType>23</rasd:ResourceType>
-      </Item>
-      <Item ovf:required="false">
-        <rasd:Address>0</rasd:Address>
-        <rasd:Description>USB Controller (EHCI)</rasd:Description>
-        <rasd:ElementName>usb</rasd:ElementName>
-        <rasd:InstanceID>5</rasd:InstanceID>
-        <rasd:ResourceSubType>vmware.usb.ehci</rasd:ResourceSubType>
-        <rasd:ResourceType>23</rasd:ResourceType>
-        <vmw:Config ovf:required="false" vmw:key="ehciEnabled" vmw:value="true"/>
-      </Item>
-      <Item>
-        <rasd:Address>0</rasd:Address>
-        <rasd:Description>SCSI Controller</rasd:Description>
-        <rasd:ElementName>scsiController0</rasd:ElementName>
-        <rasd:InstanceID>6</rasd:InstanceID>
-        <rasd:ResourceSubType>lsilogicsas</rasd:ResourceSubType>
-        <rasd:ResourceType>6</rasd:ResourceType>
-      </Item>
-      <Item ovf:required="false">
-        <rasd:AutomaticAllocation>true</rasd:AutomaticAllocation>
-        <rasd:ElementName>serial0</rasd:ElementName>
-        <rasd:InstanceID>7</rasd:InstanceID>
-        <rasd:ResourceType>21</rasd:ResourceType>
-        <vmw:Config ovf:required="false" vmw:key="yieldOnPoll" vmw:value="false"/>
-      </Item>
-      <Item>
-        <rasd:AddressOnParent>0</rasd:AddressOnParent>
-        <rasd:ElementName>disk0</rasd:ElementName>
-        <rasd:HostResource>ovf:/disk/vmdisk1</rasd:HostResource>
-        <rasd:InstanceID>8</rasd:InstanceID>
-        <rasd:Parent>6</rasd:Parent>
-        <rasd:ResourceType>17</rasd:ResourceType>
-      </Item>
-      <Item>
-        <rasd:AddressOnParent>2</rasd:AddressOnParent>
-        <rasd:AutomaticAllocation>true</rasd:AutomaticAllocation>
-        <rasd:Connection>bridged</rasd:Connection>
-        <rasd:Description>E1000e ethernet adapter on &quot;bridged&quot;</rasd:Description>
-        <rasd:ElementName>ethernet0</rasd:ElementName>
-        <rasd:InstanceID>9</rasd:InstanceID>
-        <rasd:ResourceSubType>E1000e</rasd:ResourceSubType>
-        <rasd:ResourceType>10</rasd:ResourceType>
-        <vmw:Config ovf:required="false" vmw:key="wakeOnLanEnabled" vmw:value="false"/>
-      </Item>
-      <Item ovf:required="false">
-        <rasd:AutomaticAllocation>false</rasd:AutomaticAllocation>
-        <rasd:ElementName>sound</rasd:ElementName>
-        <rasd:InstanceID>10</rasd:InstanceID>
-        <rasd:ResourceSubType>vmware.soundcard.hdaudio</rasd:ResourceSubType>
-        <rasd:ResourceType>1</rasd:ResourceType>
-      </Item>
-      <Item ovf:required="false">
-        <rasd:AutomaticAllocation>false</rasd:AutomaticAllocation>
-        <rasd:ElementName>video</rasd:ElementName>
-        <rasd:InstanceID>11</rasd:InstanceID>
-        <rasd:ResourceType>24</rasd:ResourceType>
-        <vmw:Config ovf:required="false" vmw:key="enable3DSupport" vmw:value="true"/>
-      </Item>
-      <Item ovf:required="false">
-        <rasd:AutomaticAllocation>false</rasd:AutomaticAllocation>
-        <rasd:ElementName>vmci</rasd:ElementName>
-        <rasd:InstanceID>12</rasd:InstanceID>
-        <rasd:ResourceSubType>vmware.vmci</rasd:ResourceSubType>
-        <rasd:ResourceType>1</rasd:ResourceType>
-      </Item>
-      <Item ovf:required="false">
-        <rasd:AddressOnParent>1</rasd:AddressOnParent>
-        <rasd:AutomaticAllocation>false</rasd:AutomaticAllocation>
-        <rasd:ElementName>cdrom0</rasd:ElementName>
-        <rasd:InstanceID>13</rasd:InstanceID>
-        <rasd:Parent>3</rasd:Parent>
-        <rasd:ResourceType>15</rasd:ResourceType>
-      </Item>
-      <vmw:Config ovf:required="false" vmw:key="memoryHotAddEnabled" vmw:value="true"/>
-      <vmw:Config ovf:required="false" vmw:key="powerOpInfo.powerOffType" vmw:value="soft"/>
-      <vmw:Config ovf:required="false" vmw:key="powerOpInfo.resetType" vmw:value="soft"/>
-      <vmw:Config ovf:required="false" vmw:key="powerOpInfo.suspendType" vmw:value="soft"/>
-      <vmw:Config ovf:required="false" vmw:key="tools.syncTimeWithHost" vmw:value="false"/>
-    </VirtualHardwareSection>
-  </VirtualSystem>
-</Envelope>
\ No newline at end of file
diff --git a/test/ovf_manifests/Win10-Liz_no_default_ns.ovf b/test/ovf_manifests/Win10-Liz_no_default_ns.ovf
deleted file mode 100755
index b93540f4..00000000
--- a/test/ovf_manifests/Win10-Liz_no_default_ns.ovf
+++ /dev/null
@@ -1,142 +0,0 @@
-<?xml version="1.0" encoding="UTF-8"?>
-<!--Generated by VMware ovftool 4.1.0 (build-2982904), UTC time: 2017-02-07T13:50:15.265014Z-->
-<Envelope vmw:buildId="build-2982904" xmlns="http://schemas.dmtf.org/ovf/envelope/1" xmlns:cim="http://schemas.dmtf.org/wbem/wscim/1/common" xmlns:ovf="http://schemas.dmtf.org/ovf/envelope/1" xmlns:vmw="http://www.vmware.com/schema/ovf" xmlns:vssd="http://schemas.dmtf.org/wbem/wscim/1/cim-schema/2/CIM_VirtualSystemSettingData" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance">
-  <References>
-    <File ovf:href="Win10-Liz-disk1.vmdk" ovf:id="file1" ovf:size="9155243008"/>
-  </References>
-  <DiskSection>
-    <Info>Virtual disk information</Info>
-    <Disk ovf:capacity="128" ovf:capacityAllocationUnits="byte * 2^30" ovf:diskId="vmdisk1" ovf:fileRef="file1" ovf:format="http://www.vmware.com/interfaces/specifications/vmdk.html#streamOptimized" ovf:populatedSize="16798056448"/>
-  </DiskSection>
-  <NetworkSection>
-    <Info>The list of logical networks</Info>
-    <Network ovf:name="bridged">
-      <Description>The bridged network</Description>
-    </Network>
-  </NetworkSection>
-  <VirtualSystem ovf:id="vm">
-    <Info>A virtual machine</Info>
-    <Name>Win10-Liz</Name>
-    <OperatingSystemSection ovf:id="1" vmw:osType="windows9_64Guest">
-      <Info>The kind of installed guest operating system</Info>
-    </OperatingSystemSection>
-    <VirtualHardwareSection>
-      <Info>Virtual hardware requirements</Info>
-      <System>
-        <vssd:ElementName>Virtual Hardware Family</vssd:ElementName>
-        <vssd:InstanceID>0</vssd:InstanceID>
-        <vssd:VirtualSystemIdentifier>Win10-Liz</vssd:VirtualSystemIdentifier>
-        <vssd:VirtualSystemType>vmx-11</vssd:VirtualSystemType>
-      </System>
-      <Item>
-        <rasd:AllocationUnits xmlns:rasd="http://schemas.dmtf.org/wbem/wscim/1/cim-schema/2/CIM_ResourceAllocationSettingData">hertz * 10^6</rasd:AllocationUnits>
-        <rasd:Description xmlns:rasd="http://schemas.dmtf.org/wbem/wscim/1/cim-schema/2/CIM_ResourceAllocationSettingData">Number of Virtual CPUs</rasd:Description>
-        <rasd:ElementName xmlns:rasd="http://schemas.dmtf.org/wbem/wscim/1/cim-schema/2/CIM_ResourceAllocationSettingData">4 virtual CPU(s)</rasd:ElementName>
-        <rasd:InstanceID xmlns:rasd="http://schemas.dmtf.org/wbem/wscim/1/cim-schema/2/CIM_ResourceAllocationSettingData">1</rasd:InstanceID>
-        <rasd:ResourceType xmlns:rasd="http://schemas.dmtf.org/wbem/wscim/1/cim-schema/2/CIM_ResourceAllocationSettingData">3</rasd:ResourceType>
-        <rasd:VirtualQuantity xmlns:rasd="http://schemas.dmtf.org/wbem/wscim/1/cim-schema/2/CIM_ResourceAllocationSettingData">4</rasd:VirtualQuantity>
-      </Item>
-      <Item>
-        <rasd:AllocationUnits xmlns:rasd="http://schemas.dmtf.org/wbem/wscim/1/cim-schema/2/CIM_ResourceAllocationSettingData">byte * 2^20</rasd:AllocationUnits>
-        <rasd:Description xmlns:rasd="http://schemas.dmtf.org/wbem/wscim/1/cim-schema/2/CIM_ResourceAllocationSettingData">Memory Size</rasd:Description>
-        <rasd:ElementName xmlns:rasd="http://schemas.dmtf.org/wbem/wscim/1/cim-schema/2/CIM_ResourceAllocationSettingData">6144MB of memory</rasd:ElementName>
-        <rasd:InstanceID xmlns:rasd="http://schemas.dmtf.org/wbem/wscim/1/cim-schema/2/CIM_ResourceAllocationSettingData">2</rasd:InstanceID>
-        <rasd:ResourceType xmlns:rasd="http://schemas.dmtf.org/wbem/wscim/1/cim-schema/2/CIM_ResourceAllocationSettingData">4</rasd:ResourceType>
-        <rasd:VirtualQuantity xmlns:rasd="http://schemas.dmtf.org/wbem/wscim/1/cim-schema/2/CIM_ResourceAllocationSettingData">6144</rasd:VirtualQuantity>
-      </Item>
-      <Item>
-        <rasd:Address xmlns:rasd="http://schemas.dmtf.org/wbem/wscim/1/cim-schema/2/CIM_ResourceAllocationSettingData">0</rasd:Address>
-        <rasd:Description xmlns:rasd="http://schemas.dmtf.org/wbem/wscim/1/cim-schema/2/CIM_ResourceAllocationSettingData">SATA Controller</rasd:Description>
-        <rasd:ElementName xmlns:rasd="http://schemas.dmtf.org/wbem/wscim/1/cim-schema/2/CIM_ResourceAllocationSettingData">sataController0</rasd:ElementName>
-        <rasd:InstanceID xmlns:rasd="http://schemas.dmtf.org/wbem/wscim/1/cim-schema/2/CIM_ResourceAllocationSettingData">3</rasd:InstanceID>
-        <rasd:ResourceSubType xmlns:rasd="http://schemas.dmtf.org/wbem/wscim/1/cim-schema/2/CIM_ResourceAllocationSettingData">vmware.sata.ahci</rasd:ResourceSubType>
-        <rasd:ResourceType xmlns:rasd="http://schemas.dmtf.org/wbem/wscim/1/cim-schema/2/CIM_ResourceAllocationSettingData">20</rasd:ResourceType>
-      </Item>
-      <Item ovf:required="false">
-        <rasd:Address xmlns:rasd="http://schemas.dmtf.org/wbem/wscim/1/cim-schema/2/CIM_ResourceAllocationSettingData">0</rasd:Address>
-        <rasd:Description xmlns:rasd="http://schemas.dmtf.org/wbem/wscim/1/cim-schema/2/CIM_ResourceAllocationSettingData">USB Controller (XHCI)</rasd:Description>
-        <rasd:ElementName xmlns:rasd="http://schemas.dmtf.org/wbem/wscim/1/cim-schema/2/CIM_ResourceAllocationSettingData">usb3</rasd:ElementName>
-        <rasd:InstanceID xmlns:rasd="http://schemas.dmtf.org/wbem/wscim/1/cim-schema/2/CIM_ResourceAllocationSettingData">4</rasd:InstanceID>
-        <rasd:ResourceSubType xmlns:rasd="http://schemas.dmtf.org/wbem/wscim/1/cim-schema/2/CIM_ResourceAllocationSettingData">vmware.usb.xhci</rasd:ResourceSubType>
-        <rasd:ResourceType xmlns:rasd="http://schemas.dmtf.org/wbem/wscim/1/cim-schema/2/CIM_ResourceAllocationSettingData">23</rasd:ResourceType>
-      </Item>
-      <Item ovf:required="false">
-        <rasd:Address xmlns:rasd="http://schemas.dmtf.org/wbem/wscim/1/cim-schema/2/CIM_ResourceAllocationSettingData">0</rasd:Address>
-        <rasd:Description xmlns:rasd="http://schemas.dmtf.org/wbem/wscim/1/cim-schema/2/CIM_ResourceAllocationSettingData">USB Controller (EHCI)</rasd:Description>
-        <rasd:ElementName xmlns:rasd="http://schemas.dmtf.org/wbem/wscim/1/cim-schema/2/CIM_ResourceAllocationSettingData">usb</rasd:ElementName>
-        <rasd:InstanceID xmlns:rasd="http://schemas.dmtf.org/wbem/wscim/1/cim-schema/2/CIM_ResourceAllocationSettingData">5</rasd:InstanceID>
-        <rasd:ResourceSubType xmlns:rasd="http://schemas.dmtf.org/wbem/wscim/1/cim-schema/2/CIM_ResourceAllocationSettingData">vmware.usb.ehci</rasd:ResourceSubType>
-        <rasd:ResourceType xmlns:rasd="http://schemas.dmtf.org/wbem/wscim/1/cim-schema/2/CIM_ResourceAllocationSettingData">23</rasd:ResourceType>
-        <vmw:Config ovf:required="false" vmw:key="ehciEnabled" vmw:value="true"/>
-      </Item>
-      <Item>
-        <rasd:Address xmlns:rasd="http://schemas.dmtf.org/wbem/wscim/1/cim-schema/2/CIM_ResourceAllocationSettingData">0</rasd:Address>
-        <rasd:Description xmlns:rasd="http://schemas.dmtf.org/wbem/wscim/1/cim-schema/2/CIM_ResourceAllocationSettingData">SCSI Controller</rasd:Description>
-        <rasd:ElementName xmlns:rasd="http://schemas.dmtf.org/wbem/wscim/1/cim-schema/2/CIM_ResourceAllocationSettingData">scsiController0</rasd:ElementName>
-        <rasd:InstanceID xmlns:rasd="http://schemas.dmtf.org/wbem/wscim/1/cim-schema/2/CIM_ResourceAllocationSettingData">6</rasd:InstanceID>
-        <rasd:ResourceSubType xmlns:rasd="http://schemas.dmtf.org/wbem/wscim/1/cim-schema/2/CIM_ResourceAllocationSettingData">lsilogicsas</rasd:ResourceSubType>
-        <rasd:ResourceType xmlns:rasd="http://schemas.dmtf.org/wbem/wscim/1/cim-schema/2/CIM_ResourceAllocationSettingData">6</rasd:ResourceType>
-      </Item>
-      <Item ovf:required="false">
-        <rasd:AutomaticAllocation xmlns:rasd="http://schemas.dmtf.org/wbem/wscim/1/cim-schema/2/CIM_ResourceAllocationSettingData">true</rasd:AutomaticAllocation>
-        <rasd:ElementName xmlns:rasd="http://schemas.dmtf.org/wbem/wscim/1/cim-schema/2/CIM_ResourceAllocationSettingData">serial0</rasd:ElementName>
-        <rasd:InstanceID xmlns:rasd="http://schemas.dmtf.org/wbem/wscim/1/cim-schema/2/CIM_ResourceAllocationSettingData">7</rasd:InstanceID>
-        <rasd:ResourceType xmlns:rasd="http://schemas.dmtf.org/wbem/wscim/1/cim-schema/2/CIM_ResourceAllocationSettingData">21</rasd:ResourceType>
-        <vmw:Config ovf:required="false" vmw:key="yieldOnPoll" vmw:value="false"/>
-      </Item>
-      <Item>
-        <rasd:AddressOnParent xmlns:rasd="http://schemas.dmtf.org/wbem/wscim/1/cim-schema/2/CIM_ResourceAllocationSettingData" >0</rasd:AddressOnParent>
-        <rasd:ElementName xmlns:rasd="http://schemas.dmtf.org/wbem/wscim/1/cim-schema/2/CIM_ResourceAllocationSettingData" >disk0</rasd:ElementName>
-        <rasd:HostResource xmlns:rasd="http://schemas.dmtf.org/wbem/wscim/1/cim-schema/2/CIM_ResourceAllocationSettingData" >ovf:/disk/vmdisk1</rasd:HostResource>
-        <rasd:InstanceID xmlns:rasd="http://schemas.dmtf.org/wbem/wscim/1/cim-schema/2/CIM_ResourceAllocationSettingData">8</rasd:InstanceID>
-        <rasd:Parent xmlns:rasd="http://schemas.dmtf.org/wbem/wscim/1/cim-schema/2/CIM_ResourceAllocationSettingData">6</rasd:Parent>
-        <rasd:ResourceType xmlns:rasd="http://schemas.dmtf.org/wbem/wscim/1/cim-schema/2/CIM_ResourceAllocationSettingData">17</rasd:ResourceType>
-      </Item>
-      <Item>
-        <rasd:AddressOnParent xmlns:rasd="http://schemas.dmtf.org/wbem/wscim/1/cim-schema/2/CIM_ResourceAllocationSettingData">2</rasd:AddressOnParent>
-        <rasd:AutomaticAllocation xmlns:rasd="http://schemas.dmtf.org/wbem/wscim/1/cim-schema/2/CIM_ResourceAllocationSettingData">true</rasd:AutomaticAllocation>
-        <rasd:Connection xmlns:rasd="http://schemas.dmtf.org/wbem/wscim/1/cim-schema/2/CIM_ResourceAllocationSettingData">bridged</rasd:Connection>
-        <rasd:Description xmlns:rasd="http://schemas.dmtf.org/wbem/wscim/1/cim-schema/2/CIM_ResourceAllocationSettingData">E1000e ethernet adapter on &quot;bridged&quot;</rasd:Description>
-        <rasd:ElementName xmlns:rasd="http://schemas.dmtf.org/wbem/wscim/1/cim-schema/2/CIM_ResourceAllocationSettingData">ethernet0</rasd:ElementName>
-        <rasd:InstanceID xmlns:rasd="http://schemas.dmtf.org/wbem/wscim/1/cim-schema/2/CIM_ResourceAllocationSettingData">9</rasd:InstanceID>
-        <rasd:ResourceSubType xmlns:rasd="http://schemas.dmtf.org/wbem/wscim/1/cim-schema/2/CIM_ResourceAllocationSettingData">E1000e</rasd:ResourceSubType>
-        <rasd:ResourceType xmlns:rasd="http://schemas.dmtf.org/wbem/wscim/1/cim-schema/2/CIM_ResourceAllocationSettingData">10</rasd:ResourceType>
-        <vmw:Config ovf:required="false" vmw:key="wakeOnLanEnabled" vmw:value="false"/>
-      </Item>
-      <Item ovf:required="false">
-        <rasd:AutomaticAllocation xmlns:rasd="http://schemas.dmtf.org/wbem/wscim/1/cim-schema/2/CIM_ResourceAllocationSettingData">false</rasd:AutomaticAllocation>
-        <rasd:ElementName xmlns:rasd="http://schemas.dmtf.org/wbem/wscim/1/cim-schema/2/CIM_ResourceAllocationSettingData">sound</rasd:ElementName>
-        <rasd:InstanceID xmlns:rasd="http://schemas.dmtf.org/wbem/wscim/1/cim-schema/2/CIM_ResourceAllocationSettingData">10</rasd:InstanceID>
-        <rasd:ResourceSubType xmlns:rasd="http://schemas.dmtf.org/wbem/wscim/1/cim-schema/2/CIM_ResourceAllocationSettingData">vmware.soundcard.hdaudio</rasd:ResourceSubType>
-        <rasd:ResourceType xmlns:rasd="http://schemas.dmtf.org/wbem/wscim/1/cim-schema/2/CIM_ResourceAllocationSettingData">1</rasd:ResourceType>
-      </Item>
-      <Item ovf:required="false">
-        <rasd:AutomaticAllocation xmlns:rasd="http://schemas.dmtf.org/wbem/wscim/1/cim-schema/2/CIM_ResourceAllocationSettingData">false</rasd:AutomaticAllocation>
-        <rasd:ElementName xmlns:rasd="http://schemas.dmtf.org/wbem/wscim/1/cim-schema/2/CIM_ResourceAllocationSettingData">video</rasd:ElementName>
-        <rasd:InstanceID xmlns:rasd="http://schemas.dmtf.org/wbem/wscim/1/cim-schema/2/CIM_ResourceAllocationSettingData">11</rasd:InstanceID>
-        <rasd:ResourceType xmlns:rasd="http://schemas.dmtf.org/wbem/wscim/1/cim-schema/2/CIM_ResourceAllocationSettingData">24</rasd:ResourceType>
-        <vmw:Config ovf:required="false" vmw:key="enable3DSupport" vmw:value="true"/>
-      </Item>
-      <Item ovf:required="false">
-        <rasd:AutomaticAllocation xmlns:rasd="http://schemas.dmtf.org/wbem/wscim/1/cim-schema/2/CIM_ResourceAllocationSettingData">false</rasd:AutomaticAllocation>
-        <rasd:ElementName xmlns:rasd="http://schemas.dmtf.org/wbem/wscim/1/cim-schema/2/CIM_ResourceAllocationSettingData">vmci</rasd:ElementName>
-        <rasd:InstanceID xmlns:rasd="http://schemas.dmtf.org/wbem/wscim/1/cim-schema/2/CIM_ResourceAllocationSettingData">12</rasd:InstanceID>
-        <rasd:ResourceSubType xmlns:rasd="http://schemas.dmtf.org/wbem/wscim/1/cim-schema/2/CIM_ResourceAllocationSettingData">vmware.vmci</rasd:ResourceSubType>
-        <rasd:ResourceType xmlns:rasd="http://schemas.dmtf.org/wbem/wscim/1/cim-schema/2/CIM_ResourceAllocationSettingData">1</rasd:ResourceType>
-      </Item>
-      <Item ovf:required="false">
-        <rasd:AddressOnParent xmlns:rasd="http://schemas.dmtf.org/wbem/wscim/1/cim-schema/2/CIM_ResourceAllocationSettingData">1</rasd:AddressOnParent>
-        <rasd:AutomaticAllocation xmlns:rasd="http://schemas.dmtf.org/wbem/wscim/1/cim-schema/2/CIM_ResourceAllocationSettingData">false</rasd:AutomaticAllocation>
-        <rasd:ElementName xmlns:rasd="http://schemas.dmtf.org/wbem/wscim/1/cim-schema/2/CIM_ResourceAllocationSettingData">cdrom0</rasd:ElementName>
-        <rasd:InstanceID xmlns:rasd="http://schemas.dmtf.org/wbem/wscim/1/cim-schema/2/CIM_ResourceAllocationSettingData">13</rasd:InstanceID>
-        <rasd:Parent xmlns:rasd="http://schemas.dmtf.org/wbem/wscim/1/cim-schema/2/CIM_ResourceAllocationSettingData">3</rasd:Parent>
-        <rasd:ResourceType xmlns:rasd="http://schemas.dmtf.org/wbem/wscim/1/cim-schema/2/CIM_ResourceAllocationSettingData">15</rasd:ResourceType>
-      </Item>
-      <vmw:Config ovf:required="false" vmw:key="memoryHotAddEnabled" vmw:value="true"/>
-      <vmw:Config ovf:required="false" vmw:key="powerOpInfo.powerOffType" vmw:value="soft"/>
-      <vmw:Config ovf:required="false" vmw:key="powerOpInfo.resetType" vmw:value="soft"/>
-      <vmw:Config ovf:required="false" vmw:key="powerOpInfo.suspendType" vmw:value="soft"/>
-      <vmw:Config ovf:required="false" vmw:key="tools.syncTimeWithHost" vmw:value="false"/>
-    </VirtualHardwareSection>
-  </VirtualSystem>
-</Envelope>
diff --git a/test/ovf_manifests/Win_2008_R2_two-disks.ovf b/test/ovf_manifests/Win_2008_R2_two-disks.ovf
deleted file mode 100755
index a563aabb..00000000
--- a/test/ovf_manifests/Win_2008_R2_two-disks.ovf
+++ /dev/null
@@ -1,145 +0,0 @@
-<?xml version="1.0" encoding="UTF-8"?>
-<!--Generated by VMware ovftool 4.1.0 (build-2982904), UTC time: 2017-02-27T15:09:29.768974Z-->
-<Envelope vmw:buildId="build-2982904" xmlns="http://schemas.dmtf.org/ovf/envelope/1" xmlns:cim="http://schemas.dmtf.org/wbem/wscim/1/common" xmlns:ovf="http://schemas.dmtf.org/ovf/envelope/1" xmlns:rasd="http://schemas.dmtf.org/wbem/wscim/1/cim-schema/2/CIM_ResourceAllocationSettingData" xmlns:vmw="http://www.vmware.com/schema/ovf" xmlns:vssd="http://schemas.dmtf.org/wbem/wscim/1/cim-schema/2/CIM_VirtualSystemSettingData" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance">
-  <References>
-    <File ovf:href="disk1.vmdk" ovf:id="file1" ovf:size="3481968640"/>
-    <File ovf:href="disk2.vmdk" ovf:id="file2" ovf:size="68096"/>
-  </References>
-  <DiskSection>
-    <Info>Virtual disk information</Info>
-    <Disk ovf:capacity="40" ovf:capacityAllocationUnits="byte * 2^30" ovf:diskId="vmdisk1" ovf:fileRef="file1" ovf:format="http://www.vmware.com/interfaces/specifications/vmdk.html#streamOptimized" ovf:populatedSize="7684882432"/>
-    <Disk ovf:capacity="1" ovf:capacityAllocationUnits="byte * 2^30" ovf:diskId="vmdisk2" ovf:fileRef="file2" ovf:format="http://www.vmware.com/interfaces/specifications/vmdk.html#streamOptimized" ovf:populatedSize="0"/>
-  </DiskSection>
-  <NetworkSection>
-    <Info>The list of logical networks</Info>
-    <Network ovf:name="bridged">
-      <Description>The bridged network</Description>
-    </Network>
-  </NetworkSection>
-  <VirtualSystem ovf:id="vm">
-    <Info>A virtual machine</Info>
-    <Name>Win_2008-R2x64</Name>
-    <OperatingSystemSection ovf:id="103" vmw:osType="windows7Server64Guest">
-      <Info>The kind of installed guest operating system</Info>
-    </OperatingSystemSection>
-    <VirtualHardwareSection>
-      <Info>Virtual hardware requirements</Info>
-      <System>
-        <vssd:ElementName>Virtual Hardware Family</vssd:ElementName>
-        <vssd:InstanceID>0</vssd:InstanceID>
-        <vssd:VirtualSystemIdentifier>Win_2008-R2x64</vssd:VirtualSystemIdentifier>
-        <vssd:VirtualSystemType>vmx-11</vssd:VirtualSystemType>
-      </System>
-      <Item>
-        <rasd:AllocationUnits>hertz * 10^6</rasd:AllocationUnits>
-        <rasd:Description>Number of Virtual CPUs</rasd:Description>
-        <rasd:ElementName>1 virtual CPU(s)</rasd:ElementName>
-        <rasd:InstanceID>1</rasd:InstanceID>
-        <rasd:ResourceType>3</rasd:ResourceType>
-        <rasd:VirtualQuantity>1</rasd:VirtualQuantity>
-      </Item>
-      <Item>
-        <rasd:AllocationUnits>byte * 2^20</rasd:AllocationUnits>
-        <rasd:Description>Memory Size</rasd:Description>
-        <rasd:ElementName>2048MB of memory</rasd:ElementName>
-        <rasd:InstanceID>2</rasd:InstanceID>
-        <rasd:ResourceType>4</rasd:ResourceType>
-        <rasd:VirtualQuantity>2048</rasd:VirtualQuantity>
-      </Item>
-      <Item>
-        <rasd:Address>0</rasd:Address>
-        <rasd:Description>SATA Controller</rasd:Description>
-        <rasd:ElementName>sataController0</rasd:ElementName>
-        <rasd:InstanceID>3</rasd:InstanceID>
-        <rasd:ResourceSubType>vmware.sata.ahci</rasd:ResourceSubType>
-        <rasd:ResourceType>20</rasd:ResourceType>
-      </Item>
-      <Item ovf:required="false">
-        <rasd:Address>0</rasd:Address>
-        <rasd:Description>USB Controller (EHCI)</rasd:Description>
-        <rasd:ElementName>usb</rasd:ElementName>
-        <rasd:InstanceID>4</rasd:InstanceID>
-        <rasd:ResourceSubType>vmware.usb.ehci</rasd:ResourceSubType>
-        <rasd:ResourceType>23</rasd:ResourceType>
-        <vmw:Config ovf:required="false" vmw:key="ehciEnabled" vmw:value="true"/>
-      </Item>
-      <Item>
-        <rasd:Address>0</rasd:Address>
-        <rasd:Description>SCSI Controller</rasd:Description>
-        <rasd:ElementName>scsiController0</rasd:ElementName>
-        <rasd:InstanceID>5</rasd:InstanceID>
-        <rasd:ResourceSubType>lsilogicsas</rasd:ResourceSubType>
-        <rasd:ResourceType>6</rasd:ResourceType>
-      </Item>
-      <Item ovf:required="false">
-        <rasd:AutomaticAllocation>true</rasd:AutomaticAllocation>
-        <rasd:ElementName>serial0</rasd:ElementName>
-        <rasd:InstanceID>6</rasd:InstanceID>
-        <rasd:ResourceType>21</rasd:ResourceType>
-        <vmw:Config ovf:required="false" vmw:key="yieldOnPoll" vmw:value="false"/>
-      </Item>
-      <Item>
-        <rasd:AddressOnParent>0</rasd:AddressOnParent>
-        <rasd:ElementName>disk0</rasd:ElementName>
-        <rasd:HostResource>ovf:/disk/vmdisk1</rasd:HostResource>
-        <rasd:InstanceID>7</rasd:InstanceID>
-        <rasd:Parent>5</rasd:Parent>
-        <rasd:ResourceType>17</rasd:ResourceType>
-      </Item>
-      <Item>
-        <rasd:AddressOnParent>1</rasd:AddressOnParent>
-        <rasd:ElementName>disk1</rasd:ElementName>
-        <rasd:HostResource>ovf:/disk/vmdisk2</rasd:HostResource>
-        <rasd:InstanceID>8</rasd:InstanceID>
-        <rasd:Parent>5</rasd:Parent>
-        <rasd:ResourceType>17</rasd:ResourceType>
-      </Item>
-      <Item>
-        <rasd:AddressOnParent>2</rasd:AddressOnParent>
-        <rasd:AutomaticAllocation>true</rasd:AutomaticAllocation>
-        <rasd:Connection>bridged</rasd:Connection>
-        <rasd:Description>E1000 ethernet adapter on &quot;bridged&quot;</rasd:Description>
-        <rasd:ElementName>ethernet0</rasd:ElementName>
-        <rasd:InstanceID>9</rasd:InstanceID>
-        <rasd:ResourceSubType>E1000</rasd:ResourceSubType>
-        <rasd:ResourceType>10</rasd:ResourceType>
-        <vmw:Config ovf:required="false" vmw:key="wakeOnLanEnabled" vmw:value="false"/>
-      </Item>
-      <Item ovf:required="false">
-        <rasd:AutomaticAllocation>false</rasd:AutomaticAllocation>
-        <rasd:ElementName>sound</rasd:ElementName>
-        <rasd:InstanceID>10</rasd:InstanceID>
-        <rasd:ResourceSubType>vmware.soundcard.hdaudio</rasd:ResourceSubType>
-        <rasd:ResourceType>1</rasd:ResourceType>
-      </Item>
-      <Item ovf:required="false">
-        <rasd:AutomaticAllocation>false</rasd:AutomaticAllocation>
-        <rasd:ElementName>video</rasd:ElementName>
-        <rasd:InstanceID>11</rasd:InstanceID>
-        <rasd:ResourceType>24</rasd:ResourceType>
-        <vmw:Config ovf:required="false" vmw:key="enable3DSupport" vmw:value="true"/>
-      </Item>
-      <Item ovf:required="false">
-        <rasd:AutomaticAllocation>false</rasd:AutomaticAllocation>
-        <rasd:ElementName>vmci</rasd:ElementName>
-        <rasd:InstanceID>12</rasd:InstanceID>
-        <rasd:ResourceSubType>vmware.vmci</rasd:ResourceSubType>
-        <rasd:ResourceType>1</rasd:ResourceType>
-      </Item>
-      <Item ovf:required="false">
-        <rasd:AddressOnParent>1</rasd:AddressOnParent>
-        <rasd:AutomaticAllocation>false</rasd:AutomaticAllocation>
-        <rasd:ElementName>cdrom0</rasd:ElementName>
-        <rasd:InstanceID>13</rasd:InstanceID>
-        <rasd:Parent>3</rasd:Parent>
-        <rasd:ResourceType>15</rasd:ResourceType>
-      </Item>
-      <vmw:Config ovf:required="false" vmw:key="cpuHotAddEnabled" vmw:value="true"/>
-      <vmw:Config ovf:required="false" vmw:key="memoryHotAddEnabled" vmw:value="true"/>
-      <vmw:Config ovf:required="false" vmw:key="powerOpInfo.powerOffType" vmw:value="soft"/>
-      <vmw:Config ovf:required="false" vmw:key="powerOpInfo.resetType" vmw:value="soft"/>
-      <vmw:Config ovf:required="false" vmw:key="powerOpInfo.suspendType" vmw:value="soft"/>
-      <vmw:Config ovf:required="false" vmw:key="tools.syncTimeWithHost" vmw:value="false"/>
-    </VirtualHardwareSection>
-  </VirtualSystem>
-</Envelope>
diff --git a/test/ovf_manifests/disk1.vmdk b/test/ovf_manifests/disk1.vmdk
deleted file mode 100644
index 8660602343a1a955f9bcf2e6beaed99316dd8167..0000000000000000000000000000000000000000
GIT binary patch
literal 0
HcmV?d00001

literal 65536
zcmeIvy>HV%7zbeUF`dK)46s<q9ua7|WuUkSgpg2EwX+*viPe0`HWAtSr(-ASlAWQ|
zbJF?F{+-YFKK_yYyn2=-$&0qXY<t)4ch@B8o_Fo_en^t%N%H0}e|H$~AF`0X3J-JR
zqY>z*Sy|tuS*)j3xo%d~*K!`iCRTO1T8@X|%lB-2b2}Q2@{gmi&a1d=x<|K%7N%9q
zn|Qfh$8m45TCV10Gb^W)c4ZxVA@tMpzfJp2S{y#m?iwzx)01@a>+{9rJna?j=ZAyM
zqPW{FznsOxiSi~-&+<BkewLkuP!u<VO<6U6^7*&xtNr=XaoRiS?V{gtwTMl%9Za|L
za#^%_7k)SjXE85!!SM7bspGUQewUqo+Glx@ubWtPwRL-yMO)CL`L7O2fB*pk1PBly
zK!5-N0t5&UAV7cs0RjXF5FkK+009C72oNAZfB*pkPg~&a(=JbS1PBlyK!5-N0!ISx
zkM7+PAV7cs0RjXF5FkK+009C72oNAZfB*pk1PBlyK!5-N0t5&UAV7cs0RjXF5FkK+
z009C72oNAZfB*pk1PBlyK!5-N0t5&UAV7cs0RjXF5FkK+009C72oNAZfB*pk1PBly
zK!5-N0t5&UAV7cs0RjXF5FkK+009C72oNAZfB*pk1PBlyK!5-N0t5&UAV7cs0RjXF
z5FkK+009C72oNAZfB*pk1PBlyK!5-N0t5&UAV7cs0RjXF5FkK+009C72oNAZfB*pk
d1PBlyK!5-N0t5&UAV7cs0RjXF5cr=0{{Ra(Wheju

diff --git a/test/ovf_manifests/disk2.vmdk b/test/ovf_manifests/disk2.vmdk
deleted file mode 100644
index c4634513348b392202898374f1c8d2d51d565b27..0000000000000000000000000000000000000000
GIT binary patch
literal 0
HcmV?d00001

literal 65536
zcmeIvy>HV%7zbeUF`dK)46s<q9#IIDI%J@v2!xPOQ?;`jUy0Rx$u<$$`lr`+(j_}X
ztLLQio&7tX?|uAp{Oj^rk|Zyh{<7(9yX&q=(mrq7>)ntf&y(cMe*SJh-aTX?eH9+&
z#z!O2Psc@dn~q~OEsJ%%D!&!;7&fu2iq&#-6u$l#k8ZBB&%=0f64qH6mv#5(X4k^B
zj9DEow(B_REmq6byr^fzbkeM>VlRY#diJkw-bwTQ2bx{O`BgehC%?a(PtMX_-hBS!
zV6(_?yX6<NxIa-=XX$BH#n2y*PeaJ_>%pcd>%ZCj`_<*{eCa6d4SQYmC$1K;F1Lf}
zc3v#=CU3(J2jMJcc^4cVA0$<rHpO?@@uyvu<=MK9Wm{XjSCKabJ(~aOpacjIAV7cs
z0RjXF5FkK+009C72oNAZfB*pk1PBlyK!5-N0t5&UAn>#W-ahT}R7ZdS0RjXF5Fl_M
z@c!W5Edc@q2oNAZfB*pk1PBlyK!5-N0t5&UAV7cs0RjXF5FkK+009C72oNAZfB*pk
z1PBlyK!5-N0t5&UAV7cs0RjXF5FkK+009C72oNAZfB*pk1PBlyK!5-N0t5&UAV7cs
z0RjXF5FkK+009C72oNAZfB*pk1PBlyK!5-N0t5&UAV7cs0RjXF5FkK+009C72oNAZ
zfB*pk1PBlyK!5-N0t5&UAV7cs0RjXF5FkK+009C72oNAZfB*pk1PBlyK!5-N0t5&U
eAV7cs0RjXF5FkK+009C72oNAZfB=F2DR2*l=VfOA

diff --git a/test/run_ovf_tests.pl b/test/run_ovf_tests.pl
deleted file mode 100755
index ff6c7863..00000000
--- a/test/run_ovf_tests.pl
+++ /dev/null
@@ -1,71 +0,0 @@
-#!/usr/bin/perl
-
-use strict;
-use warnings;
-use lib qw(..); # prepend .. to @INC so we use the local version of PVE packages
-
-use FindBin '$Bin';
-use PVE::QemuServer::OVF;
-use Test::More;
-
-use Data::Dumper;
-
-my $test_manifests = join ('/', $Bin, 'ovf_manifests');
-
-print "parsing ovfs\n";
-
-my $win2008 = eval { PVE::QemuServer::OVF::parse_ovf("$test_manifests/Win_2008_R2_two-disks.ovf") };
-if (my $err = $@) {
-    fail('parse win2008');
-    warn("error: $err\n");
-} else {
-    ok('parse win2008');
-}
-my $win10 = eval { PVE::QemuServer::OVF::parse_ovf("$test_manifests/Win10-Liz.ovf") };
-if (my $err = $@) {
-    fail('parse win10');
-    warn("error: $err\n");
-} else {
-    ok('parse win10');
-}
-my $win10noNs = eval { PVE::QemuServer::OVF::parse_ovf("$test_manifests/Win10-Liz_no_default_ns.ovf") };
-if (my $err = $@) {
-    fail("parse win10 no default rasd NS");
-    warn("error: $err\n");
-} else {
-    ok('parse win10 no default rasd NS');
-}
-
-print "testing disks\n";
-
-is($win2008->{disks}->[0]->{disk_address}, 'scsi0', 'multidisk vm has the correct first disk controller');
-is($win2008->{disks}->[0]->{backing_file}, "$test_manifests/disk1.vmdk", 'multidisk vm has the correct first disk backing device');
-is($win2008->{disks}->[0]->{virtual_size}, 2048, 'multidisk vm has the correct first disk size');
-
-is($win2008->{disks}->[1]->{disk_address}, 'scsi1', 'multidisk vm has the correct second disk controller');
-is($win2008->{disks}->[1]->{backing_file}, "$test_manifests/disk2.vmdk", 'multidisk vm has the correct second disk backing device');
-is($win2008->{disks}->[1]->{virtual_size}, 2048, 'multidisk vm has the correct second disk size');
-
-is($win10->{disks}->[0]->{disk_address}, 'scsi0', 'single disk vm has the correct disk controller');
-is($win10->{disks}->[0]->{backing_file}, "$test_manifests/Win10-Liz-disk1.vmdk", 'single disk vm has the correct disk backing device');
-is($win10->{disks}->[0]->{virtual_size}, 2048, 'single disk vm has the correct size');
-
-is($win10noNs->{disks}->[0]->{disk_address}, 'scsi0', 'single disk vm (no default rasd NS) has the correct disk controller');
-is($win10noNs->{disks}->[0]->{backing_file}, "$test_manifests/Win10-Liz-disk1.vmdk", 'single disk vm (no default rasd NS) has the correct disk backing device');
-is($win10noNs->{disks}->[0]->{virtual_size}, 2048, 'single disk vm (no default rasd NS) has the correct size');
-
-print "\ntesting vm.conf extraction\n";
-
-is($win2008->{qm}->{name}, 'Win2008-R2x64', 'win2008 VM name is correct');
-is($win2008->{qm}->{memory}, '2048', 'win2008 VM memory is correct');
-is($win2008->{qm}->{cores}, '1', 'win2008 VM cores are correct');
-
-is($win10->{qm}->{name}, 'Win10-Liz', 'win10 VM name is correct');
-is($win10->{qm}->{memory}, '6144', 'win10 VM memory is correct');
-is($win10->{qm}->{cores}, '4', 'win10 VM cores are correct');
-
-is($win10noNs->{qm}->{name}, 'Win10-Liz', 'win10 VM (no default rasd NS) name is correct');
-is($win10noNs->{qm}->{memory}, '6144', 'win10 VM (no default rasd NS) memory is correct');
-is($win10noNs->{qm}->{cores}, '4', 'win10 VM (no default rasd NS) cores are correct');
-
-done_testing();
-- 
2.39.5



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

[pve-devel] [PATCH qemu-server v7 3/5] api: create: implement extracting disks when needed for import-from

[Dominik Csapak]

when 'import-from' contains a disk image that needs extraction
(currently only from an 'ova' archive), do that in 'create_disks'
and overwrite the '$source' volid.

Collect the names into a 'delete_sources' list, that we use later
to clean it up again (either when we're finished with importing or in an
error case).

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
changes from v6:
* add correct variable to delete_sources list

 PVE/API2/Qemu.pm          | 59 +++++++++++++++++++++++++++++++--------
 PVE/QemuServer.pm         | 12 ++++++++
 PVE/QemuServer/Helpers.pm |  5 ++++
 3 files changed, 65 insertions(+), 11 deletions(-)

diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
index 528ec6d1..c947f09c 100644
--- a/PVE/API2/Qemu.pm
+++ b/PVE/API2/Qemu.pm
@@ -24,6 +24,7 @@ use PVE::JSONSchema qw(get_standard_option);
 use PVE::RESTHandler;
 use PVE::ReplicationConfig;
 use PVE::GuestHelpers qw(assert_tag_permissions);
+use PVE::GuestImport;
 use PVE::QemuConfig;
 use PVE::QemuServer;
 use PVE::QemuServer::Cloudinit;
@@ -163,10 +164,20 @@ my $check_storage_access = sub {
 
 	if (my $src_image = $drive->{'import-from'}) {
 	    my $src_vmid;
-	    if (PVE::Storage::parse_volume_id($src_image, 1)) { # PVE-managed volume
-		(my $vtype, undef, $src_vmid) = PVE::Storage::parse_volname($storecfg, $src_image);
-		raise_param_exc({ $ds => "$src_image has wrong type '$vtype' - not an image" })
-		    if $vtype ne 'images';
+	    my ($storeid, $volname) = PVE::Storage::parse_volume_id($src_image, 1);
+	    if ($storeid) { # PVE-managed volume
+		my $scfg = PVE::Storage::storage_config($storecfg, $storeid);
+		my $plugin = PVE::Storage::Plugin->lookup($scfg->{type});
+		(my $vtype, undef, $src_vmid, undef, undef, undef, my $fmt) = $plugin->parse_volname($volname);
+
+		raise_param_exc({ $ds => "$src_image has wrong type '$vtype' - needs to be 'images' or 'import'" })
+		    if $vtype ne 'images' && $vtype ne 'import';
+
+		if (PVE::QemuServer::Helpers::needs_extraction($vtype, $fmt)) {
+		    raise_param_exc({ $ds => "$src_image is not on an storage with 'images' content type."})
+			if !$scfg->{content}->{images};
+		    $rpcenv->check($authuser, "/storage/$storeid", ['Datastore.AllocateSpace']);
+		}
 	    }
 
 	    if ($src_vmid) { # might be actively used by VM and will be copied via clone_disk()
@@ -413,17 +424,37 @@ my sub create_disks : prototype($$$$$$$$$$) {
 
 		$needs_creation = $live_import;
 
-		if (PVE::Storage::parse_volume_id($source, 1)) { # PVE-managed volume
+		my ($source_storage, $source_volid) = PVE::Storage::parse_volume_id($source, 1);
+
+		if ($source_storage) { # PVE-managed volume
+		    my ($vtype, undef, undef, undef, undef, undef, $fmt)
+			= PVE::Storage::parse_volname($storecfg, $source);
+		    my $needs_extraction = PVE::QemuServer::Helpers::needs_extraction($vtype, $fmt);
+		    if ($needs_extraction) {
+			print "extracting $source\n";
+			my $extracted_volid
+			     = PVE::GuestImport::extract_disk_from_import_file($source, $vmid);
+			print "finished extracting to $extracted_volid\n";
+			push @$vollist, $extracted_volid;
+			$source = $extracted_volid;
+
+			my (undef, undef, undef, $parent)
+			    = PVE::Storage::volume_size_info($storecfg, $source);
+			die "importing from extracted images with backing file ($parent) not allowed\n"
+			    if $parent;
+		    }
+
 		    if ($live_import && $ds ne 'efidisk0') {
 			my $path = PVE::Storage::path($storecfg, $source)
 			    or die "failed to get a path for '$source'\n";
-			$source = $path;
-			($size, my $source_format) = PVE::Storage::file_size_info($source);
-			die "could not get file size of $source\n" if !$size;
+			($size, my $source_format) = PVE::Storage::file_size_info($path);
+			die "could not get file size of $path\n" if !$size;
 			$live_import_mapping->{$ds} = {
-			    path => $source,
+			    path => $path,
 			    format => $source_format,
 			};
+			$live_import_mapping->{$ds}->{'delete-after-finish'} = $source
+			    if $needs_extraction;
 		    } else {
 			my $dest_info = {
 			    vmid => $vmid,
@@ -435,8 +466,14 @@ my sub create_disks : prototype($$$$$$$$$$) {
 			$dest_info->{efisize} = PVE::QemuServer::get_efivars_size($conf, $disk)
 			    if $ds eq 'efidisk0';
 
-			($dst_volid, $size) = eval {
-			    $import_from_volid->($storecfg, $source, $dest_info, $vollist);
+			eval {
+			    ($dst_volid, $size)
+				= $import_from_volid->($storecfg, $source, $dest_info, $vollist);
+
+			    # remove extracted volumes after importing
+			    PVE::Storage::vdisk_free($storecfg, $source) if $needs_extraction;
+			    print "cleaned up extracted image $source\n";
+			    @$vollist = grep { $_ ne $source } @$vollist;
 			};
 			die "cannot import from '$source' - $@" if $@;
 		    }
diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
index 8da1b4d2..7fd0449f 100644
--- a/PVE/QemuServer.pm
+++ b/PVE/QemuServer.pm
@@ -7407,6 +7407,7 @@ sub live_import_from_files {
     my ($mapping, $vmid, $conf, $restore_options) = @_;
 
     my $live_restore_backing = {};
+    my $sources_to_remove = [];
     for my $dev (keys %$mapping) {
 	die "disk not support for live-restoring: '$dev'\n"
 	    if !is_valid_drivename($dev) || $dev =~ /^(?:efidisk|tpmstate)/;
@@ -7427,6 +7428,9 @@ sub live_import_from_files {
 	    . ",read-only=on"
 	    . ",file.driver=file,file.filename=$path"
 	};
+
+	my $source_volid = $info->{'delete-after-finish'};
+	push $sources_to_remove->@*, $source_volid if defined($source_volid);
     };
 
     my $storecfg = PVE::Storage::config();
@@ -7471,6 +7475,14 @@ sub live_import_from_files {
 
     my $err = $@;
 
+    for my $volid ($sources_to_remove->@*) {
+	eval {
+	    PVE::Storage::vdisk_free($storecfg, $volid);
+	    print "cleaned up extracted image $volid\n";
+	};
+	warn "An error occurred while cleaning up source images: $@\n" if $@;
+    }
+
     if ($err) {
 	warn "An error occurred during live-restore: $err\n";
 	_do_vm_stop($storecfg, $vmid, 1, 1, 10, 0, 1);
diff --git a/PVE/QemuServer/Helpers.pm b/PVE/QemuServer/Helpers.pm
index 0afb6317..15e2496c 100644
--- a/PVE/QemuServer/Helpers.pm
+++ b/PVE/QemuServer/Helpers.pm
@@ -225,4 +225,9 @@ sub windows_version {
     return $winversion;
 }
 
+sub needs_extraction {
+    my ($vtype, $fmt) = @_;
+    return $vtype eq 'import' && $fmt =~ m/^ova\+(.*)$/;
+}
+
 1;
-- 
2.39.5



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

[pve-devel] [PATCH qemu-server v7 4/5] api: create: add 'import-working-storage' parameter

[Dominik Csapak]

this is to override the target extraction storage for the option disk
extraction for 'import-from'. This way if the storage does not
supports the content type 'images', one can give an alternative  one.

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
changes from v6:
* rename 'import-extraction-storage' to 'import-working-storage'
* rework permission checks (single branch)

i opted to not make the target the first default, since i did not want
to introduce such a change this late in the patch/review cycle and
AFAICS quite a few code changes would have been necessary for that
(we can still change that later too)

 PVE/API2/Qemu.pm | 48 +++++++++++++++++++++++++++++++++++++++---------
 1 file changed, 39 insertions(+), 9 deletions(-)

diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
index c947f09c..701558a7 100644
--- a/PVE/API2/Qemu.pm
+++ b/PVE/API2/Qemu.pm
@@ -132,7 +132,7 @@ my $check_drive_param = sub {
 };
 
 my $check_storage_access = sub {
-   my ($rpcenv, $authuser, $storecfg, $vmid, $settings, $default_storage) = @_;
+   my ($rpcenv, $authuser, $storecfg, $vmid, $settings, $default_storage, $extraction_storage) = @_;
 
    $foreach_volume_with_alloc->($settings, sub {
 	my ($ds, $drive) = @_;
@@ -174,9 +174,18 @@ my $check_storage_access = sub {
 		    if $vtype ne 'images' && $vtype ne 'import';
 
 		if (PVE::QemuServer::Helpers::needs_extraction($vtype, $fmt)) {
-		    raise_param_exc({ $ds => "$src_image is not on an storage with 'images' content type."})
-			if !$scfg->{content}->{images};
-		    $rpcenv->check($authuser, "/storage/$storeid", ['Datastore.AllocateSpace']);
+		    my $extraction_scfg = defined($extraction_storage) ?
+			PVE::Storage::storage_config($storecfg, $extraction_storage) :
+			$scfg;
+		    my $extraction_param = defined($extraction_storage) ? 'import-working-storage' : $ds;
+
+		    if (!$extraction_scfg->{content}->{images} || !$extraction_scfg->{path}) {
+			raise_param_exc({
+			    $extraction_param => "storage selected for extraction does not support"
+				." 'images' content type or is not file based.",
+			});
+		    }
+		    $rpcenv->check($authuser, "/storage/" . ($extraction_storage // $storeid), ['Datastore.AllocateSpace']);
 		}
 	    }
 
@@ -349,7 +358,7 @@ my sub prohibit_tpm_version_change {
 
 # Note: $pool is only needed when creating a VM, because pool permissions
 # are automatically inherited if VM already exists inside a pool.
-my sub create_disks : prototype($$$$$$$$$$) {
+my sub create_disks : prototype($$$$$$$$$$$) {
     my (
 	$rpcenv,
 	$authuser,
@@ -361,6 +370,7 @@ my sub create_disks : prototype($$$$$$$$$$) {
 	$settings,
 	$default_storage,
 	$is_live_import,
+	$extraction_storage,
     ) = @_;
 
     my $vollist = [];
@@ -432,8 +442,8 @@ my sub create_disks : prototype($$$$$$$$$$) {
 		    my $needs_extraction = PVE::QemuServer::Helpers::needs_extraction($vtype, $fmt);
 		    if ($needs_extraction) {
 			print "extracting $source\n";
-			my $extracted_volid
-			     = PVE::GuestImport::extract_disk_from_import_file($source, $vmid);
+			my $extracted_volid = PVE::GuestImport::extract_disk_from_import_file(
+			    $source, $vmid, $extraction_storage);
 			print "finished extracting to $extracted_volid\n";
 			push @$vollist, $extracted_volid;
 			$source = $extracted_volid;
@@ -980,6 +990,13 @@ __PACKAGE__->register_method({
 		    default => 0,
 		    description => "Start VM after it was created successfully.",
 		},
+		'import-working-storage' => get_standard_option('pve-storage-id', {
+		    description => "A file-based storage with 'images' content-type enabled, which"
+			." is used as an intermediary extraction storage during import. Defaults to"
+			." the source storage.",
+		    optional => 1,
+		    completion => \&PVE::QemuServer::complete_storage,
+		}),
 	    },
 	    1, # with_disk_alloc
 	),
@@ -1006,6 +1023,7 @@ __PACKAGE__->register_method({
 	my $storage = extract_param($param, 'storage');
 	my $unique = extract_param($param, 'unique');
 	my $live_restore = extract_param($param, 'live-restore');
+	my $extraction_storage = extract_param($param, 'import-working-storage');
 
 	if (defined(my $ssh_keys = $param->{sshkeys})) {
 		$ssh_keys = URI::Escape::uri_unescape($ssh_keys);
@@ -1068,7 +1086,8 @@ __PACKAGE__->register_method({
 	if (scalar(keys $param->%*) > 0) {
 	    &$resolve_cdrom_alias($param);
 
-	    &$check_storage_access($rpcenv, $authuser, $storecfg, $vmid, $param, $storage);
+	    &$check_storage_access(
+		$rpcenv, $authuser, $storecfg, $vmid, $param, $storage, $extraction_storage);
 
 	    &$check_vm_modify_config_perm($rpcenv, $authuser, $vmid, $pool, [ keys %$param]);
 
@@ -1183,6 +1202,7 @@ __PACKAGE__->register_method({
 			$param,
 			$storage,
 			$live_restore,
+			$extraction_storage
 		    );
 		    $conf->{$_} = $created_opts->{$_} for keys $created_opts->%*;
 
@@ -1725,6 +1745,8 @@ my $update_vm_api  = sub {
 
     my $skip_cloud_init = extract_param($param, 'skip_cloud_init');
 
+    my $extraction_storage = extract_param($param, 'import-working-storage');
+
     my @paramarr = (); # used for log message
     foreach my $key (sort keys %$param) {
 	my $value = $key eq 'cipassword' ? '<hidden>' : $param->{$key};
@@ -1841,7 +1863,7 @@ my $update_vm_api  = sub {
 
     &$check_vm_modify_config_perm($rpcenv, $authuser, $vmid, undef, [keys %$param]);
 
-    &$check_storage_access($rpcenv, $authuser, $storecfg, $vmid, $param);
+    &$check_storage_access($rpcenv, $authuser, $storecfg, $vmid, $param, undef, $extraction_storage);
 
     PVE::QemuServer::check_bridge_access($rpcenv, $authuser, $param);
 
@@ -2025,6 +2047,7 @@ my $update_vm_api  = sub {
 			{$opt => $param->{$opt}},
 			undef,
 			undef,
+			$extraction_storage,
 		    );
 		    $conf->{pending}->{$_} = $created_opts->{$_} for keys $created_opts->%*;
 
@@ -2227,6 +2250,13 @@ __PACKAGE__->register_method({
 		    maximum => 30,
 		    optional => 1,
 		},
+		'import-working-storage' => get_standard_option('pve-storage-id', {
+		    description => "A file-based storage with 'images' content-type enabled, which"
+			." is used as an intermediary extraction storage during import. Defaults to"
+			." the source storage.",
+		    optional => 1,
+		    completion => \&PVE::QemuServer::complete_storage,
+		}),
 	    },
 	    1, # with_disk_alloc
 	),
-- 
2.39.5



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

[pve-devel] [PATCH qemu-server v7 5/5] api: check untrusted image files for import content type

[Dominik Csapak]

check to be imported files for external references if they are of
content type 'import'.

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
changes from v6:
* remove unused $scfg assignment

 PVE/API2/Qemu.pm | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
index 701558a7..8db443db 100644
--- a/PVE/API2/Qemu.pm
+++ b/PVE/API2/Qemu.pm
@@ -440,6 +440,7 @@ my sub create_disks : prototype($$$$$$$$$$$) {
 		    my ($vtype, undef, undef, undef, undef, undef, $fmt)
 			= PVE::Storage::parse_volname($storecfg, $source);
 		    my $needs_extraction = PVE::QemuServer::Helpers::needs_extraction($vtype, $fmt);
+		    my $untrusted = $vtype eq 'import' ? 1 : 0;
 		    if ($needs_extraction) {
 			print "extracting $source\n";
 			my $extracted_volid = PVE::GuestImport::extract_disk_from_import_file(
@@ -457,7 +458,8 @@ my sub create_disks : prototype($$$$$$$$$$$) {
 		    if ($live_import && $ds ne 'efidisk0') {
 			my $path = PVE::Storage::path($storecfg, $source)
 			    or die "failed to get a path for '$source'\n";
-			($size, my $source_format) = PVE::Storage::file_size_info($path);
+			#·check·potentially·untrusted·image·file·for·import·vtype
+			($size, my $source_format) = PVE::Storage::file_size_info($path, undef, $untrusted);
 			die "could not get file size of $path\n" if !$size;
 			$live_import_mapping->{$ds} = {
 			    path => $path,
@@ -466,6 +468,12 @@ my sub create_disks : prototype($$$$$$$$$$$) {
 			$live_import_mapping->{$ds}->{'delete-after-finish'} = $source
 			    if $needs_extraction;
 		    } else {
+			# check potentially untrusted image file for import vtype
+			if ($untrusted) {
+			    my $path = PVE::Storage::path($storecfg, $source);
+			    PVE::Storage::file_size_info($path, undef, 1);
+			}
+
 			my $dest_info = {
 			    vmid => $vmid,
 			    drivename => $ds,
-- 
2.39.5



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

[pve-devel] [PATCH manager v7 1/9] ui: guest import: add ova-needs-extracting warning text

[Dominik Csapak]

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
changes from v6:
* reworded warning
 www/manager6/window/GuestImport.js | 1 +
 1 file changed, 1 insertion(+)

diff --git a/www/manager6/window/GuestImport.js b/www/manager6/window/GuestImport.js
index 2577ece2..1a08195e 100644
--- a/www/manager6/window/GuestImport.js
+++ b/www/manager6/window/GuestImport.js
@@ -937,6 +937,7 @@ Ext.define('PVE.window.GuestImport', {
 		    gettext('EFI state cannot be imported, you may need to reconfigure the boot order (see {0})'),
 		    '<a href="https://pve.proxmox.com/wiki/OVMF/UEFI_Boot_Entries">OVMF/UEFI Boot Entries</a>',
 		),
+		'ova-needs-extracting': gettext('Importing an OVA temporarily requires extra space on the working storage while extracting the contained disks for further processing.'),
 	    };
             let message = warningsCatalogue[w.type];
 	    if (!w.type || !message) {
-- 
2.39.5



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

[pve-devel] [PATCH manager v7 2/9] ui: enable import content type for relevant storages

[Dominik Csapak]

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 www/manager6/Utils.js                    | 1 +
 www/manager6/form/ContentTypeSelector.js | 2 +-
 www/manager6/storage/CephFSEdit.js       | 2 +-
 www/manager6/storage/GlusterFsEdit.js    | 2 +-
 4 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/www/manager6/Utils.js b/www/manager6/Utils.js
index 3691d66a..08d88c83 100644
--- a/www/manager6/Utils.js
+++ b/www/manager6/Utils.js
@@ -691,6 +691,7 @@ Ext.define('PVE.Utils', {
 	'iso': gettext('ISO image'),
 	'rootdir': gettext('Container'),
 	'snippets': gettext('Snippets'),
+	'import': gettext('Import'),
     },
 
     volume_is_qemu_backup: function(volid, format) {
diff --git a/www/manager6/form/ContentTypeSelector.js b/www/manager6/form/ContentTypeSelector.js
index d0fa0b08..431bd948 100644
--- a/www/manager6/form/ContentTypeSelector.js
+++ b/www/manager6/form/ContentTypeSelector.js
@@ -10,7 +10,7 @@ Ext.define('PVE.form.ContentTypeSelector', {
 	me.comboItems = [];
 
 	if (me.cts === undefined) {
-	    me.cts = ['images', 'iso', 'vztmpl', 'backup', 'rootdir', 'snippets'];
+	    me.cts = ['images', 'iso', 'vztmpl', 'backup', 'rootdir', 'snippets', 'import'];
 	}
 
 	Ext.Array.each(me.cts, function(ct) {
diff --git a/www/manager6/storage/CephFSEdit.js b/www/manager6/storage/CephFSEdit.js
index 6a95a00a..2cdcf7cd 100644
--- a/www/manager6/storage/CephFSEdit.js
+++ b/www/manager6/storage/CephFSEdit.js
@@ -92,7 +92,7 @@ Ext.define('PVE.storage.CephFSInputPanel', {
 	me.column2 = [
 	    {
 		xtype: 'pveContentTypeSelector',
-		cts: ['backup', 'iso', 'vztmpl', 'snippets'],
+		cts: ['backup', 'iso', 'vztmpl', 'snippets', 'import'],
 		fieldLabel: gettext('Content'),
 		name: 'content',
 		value: 'backup',
diff --git a/www/manager6/storage/GlusterFsEdit.js b/www/manager6/storage/GlusterFsEdit.js
index 8155d9c2..df7fe23f 100644
--- a/www/manager6/storage/GlusterFsEdit.js
+++ b/www/manager6/storage/GlusterFsEdit.js
@@ -99,7 +99,7 @@ Ext.define('PVE.storage.GlusterFsInputPanel', {
 	    },
 	    {
 		xtype: 'pveContentTypeSelector',
-		cts: ['images', 'iso', 'backup', 'vztmpl', 'snippets'],
+		cts: ['images', 'iso', 'backup', 'vztmpl', 'snippets', 'import'],
 		name: 'content',
 		value: 'images',
 		multiSelect: true,
-- 
2.39.5



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

[pve-devel] [PATCH manager v7 3/9] ui: enable upload/download/remove buttons for 'import' type storages

[Dominik Csapak]

but only for non esxi ones, since that does not allow
uploading/downloading there

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 www/manager6/storage/Browser.js        | 9 +++++++--
 www/manager6/window/UploadToStorage.js | 1 +
 2 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/www/manager6/storage/Browser.js b/www/manager6/storage/Browser.js
index 2123141d..934ce706 100644
--- a/www/manager6/storage/Browser.js
+++ b/www/manager6/storage/Browser.js
@@ -28,7 +28,9 @@ Ext.define('PVE.storage.Browser', {
 	let res = storageInfo.data;
 	let plugin = res.plugintype;
 
-	me.items = plugin !== 'esxi' ? [
+	let isEsxi = plugin === 'esxi';
+
+	me.items = !isEsxi ? [
 	    {
 		title: gettext('Summary'),
 		xtype: 'pveStorageSummary',
@@ -142,8 +144,11 @@ Ext.define('PVE.storage.Browser', {
 		    iconCls: 'fa fa-desktop',
 		    itemId: 'contentImport',
 		    content: 'import',
-		    useCustomRemoveButton: true, // hide default remove button
+		    useCustomRemoveButton: isEsxi, // hide default remove button for esxi
 		    showColumns: ['name', 'format'],
+		    enableUploadButton: enableUpload && !isEsxi,
+		    enableDownloadUrlButton: enableDownloadUrl && !isEsxi,
+		    useUploadButton: !isEsxi,
 		    itemdblclick: (view, record) => createGuestImportWindow(record),
 		    tbar: [
 			{
diff --git a/www/manager6/window/UploadToStorage.js b/www/manager6/window/UploadToStorage.js
index 3c5bba88..cdf548a8 100644
--- a/www/manager6/window/UploadToStorage.js
+++ b/www/manager6/window/UploadToStorage.js
@@ -9,6 +9,7 @@ Ext.define('PVE.window.UploadToStorage', {
     title: gettext('Upload'),
 
     acceptedExtensions: {
+	'import': ['.ova'],
 	iso: ['.img', '.iso'],
 	vztmpl: ['.tar.gz', '.tar.xz', '.tar.zst'],
     },
-- 
2.39.5



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

[pve-devel] [PATCH manager v7 4/9] ui: disable 'import' button for non importable formats

[Dominik Csapak]

importable formats are currently ova/ovf/vmx

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 www/manager6/storage/Browser.js | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/www/manager6/storage/Browser.js b/www/manager6/storage/Browser.js
index 934ce706..822257e7 100644
--- a/www/manager6/storage/Browser.js
+++ b/www/manager6/storage/Browser.js
@@ -124,6 +124,7 @@ Ext.define('PVE.storage.Browser', {
 		});
 	    }
 	    if (contents.includes('import')) {
+		let isImportable = format => ['ova', 'ovf', 'vmx'].indexOf(format) !== -1;
 		let createGuestImportWindow = (selection) => {
 		    if (!selection) {
 			return;
@@ -149,13 +150,18 @@ Ext.define('PVE.storage.Browser', {
 		    enableUploadButton: enableUpload && !isEsxi,
 		    enableDownloadUrlButton: enableDownloadUrl && !isEsxi,
 		    useUploadButton: !isEsxi,
-		    itemdblclick: (view, record) => createGuestImportWindow(record),
+		    itemdblclick: (view, record) => {
+			if (isImportable(record.data.format)) {
+			    createGuestImportWindow(record);
+			}
+		    },
 		    tbar: [
 			{
 			    xtype: 'proxmoxButton',
 			    disabled: true,
 			    text: gettext('Import'),
 			    iconCls: 'fa fa-cloud-download',
+			    enableFn: rec => isImportable(rec.data.format),
 			    handler: function() {
 				let grid = this.up('pveStorageContentView');
 				let selection = grid.getSelection()?.[0];
-- 
2.39.5



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

[pve-devel] [PATCH manager v7 5/9] ui: import: improve rendering of volume names

[Dominik Csapak]

in directory storages, we don't need the 'import/' part of the volumes,
as that is implied in dir based storages

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 www/manager6/Utils.js | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/www/manager6/Utils.js b/www/manager6/Utils.js
index 08d88c83..97dbbae2 100644
--- a/www/manager6/Utils.js
+++ b/www/manager6/Utils.js
@@ -1025,7 +1025,13 @@ Ext.define('PVE.Utils', {
 		Ext.String.leftPad(data.channel, 2, '0') +
 		" ID " + data.id + " LUN " + data.lun;
 	} else if (data.content === 'import') {
-	    result = data.volid.replace(/^.*?:/, '');
+	    if (data.volid.match(/^.*?:import\//)) {
+		// dir-based storages
+		result = data.volid.replace(/^.*?:import\//, '');
+	    } else {
+		// esxi storage
+		result = data.volid.replace(/^.*?:/, '');
+	    }
 	} else {
 	    result = data.volid.replace(/^.*?:(.*?\/)?/, '');
 	}
-- 
2.39.5



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

[pve-devel] [PATCH manager v7 6/9] ui: guest import: add storage selector for ova extraction storage

[Dominik Csapak]

but only when we detect the 'ova-needs-extraction' warning.
This can be used to select the storage where the disks contained in an
OVA will be extracted to temporarily.

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
changes from v6:
* adapted to new parameter
* reworded import working storage label

 www/manager6/window/GuestImport.js | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)

diff --git a/www/manager6/window/GuestImport.js b/www/manager6/window/GuestImport.js
index 1a08195e..7c263241 100644
--- a/www/manager6/window/GuestImport.js
+++ b/www/manager6/window/GuestImport.js
@@ -303,6 +303,7 @@ Ext.define('PVE.window.GuestImport', {
 	    os: 'l26',
 	    maxCdDrives: false,
 	    uniqueMACAdresses: false,
+	    isOva: false,
 	    warnings: [],
 	},
 
@@ -432,6 +433,10 @@ Ext.define('PVE.window.GuestImport', {
 			}
 		    }
 
+		    if (config['import-working-storage'] === '') {
+			delete config['import-working-storage'];
+		    }
+
 		    return config;
 		},
 
@@ -553,6 +558,22 @@ Ext.define('PVE.window.GuestImport', {
 			allowBlank: false,
 			fieldLabel: gettext('Default Bridge'),
 		    },
+		    {
+			xtype: 'pveStorageSelector',
+			reference: 'extractionStorage',
+			fieldLabel: gettext('Import Working Storage'),
+			storageContent: 'images',
+			emptyText: gettext('Import Storage'),
+			autoSelect: false,
+			name: 'import-working-storage',
+			disabled: true,
+			hidden: true,
+			allowBlank: true,
+			bind: {
+			    disabled: '{!isOva}',
+			    hidden: '{!isOva}',
+			},
+		    },
 		],
 
 		columnB: [
@@ -925,6 +946,7 @@ Ext.define('PVE.window.GuestImport', {
 
 	me.lookup('defaultStorage').setNodename(me.nodename);
 	me.lookup('defaultBridge').setNodename(me.nodename);
+	me.lookup('extractionStorage').setNodename(me.nodename);
 
 	let renderWarning = w => {
 	    const warningsCatalogue = {
@@ -1006,6 +1028,7 @@ Ext.define('PVE.window.GuestImport', {
 		}
 
 		me.getViewModel().set('warnings', data.warnings.map(w => renderWarning(w)));
+		me.getViewModel().set('isOva', data.warnings.map(w => w.type).indexOf('ova-needs-extracting') !== -1);
 
 		let osinfo = PVE.Utils.get_kvm_osinfo(me.vmConfig.ostype ?? '');
 		let prepareForVirtIO = (me.vmConfig.ostype ?? '').startsWith('w') && (me.vmConfig.bios ?? '').indexOf('ovmf') !== -1;
-- 
2.39.5



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

[pve-devel] [PATCH manager v7 7/9] ui: guest import: change icon/text for non-esxi import storage

[Dominik Csapak]

since 'virtual guests' only make sense for a hypervisor, not e.g. a
directory for OVAs

also change the icon from 'desktop' to 'cloud-download' in the
non-esxi case

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 www/manager6/storage/Browser.js | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/www/manager6/storage/Browser.js b/www/manager6/storage/Browser.js
index 822257e7..763abc70 100644
--- a/www/manager6/storage/Browser.js
+++ b/www/manager6/storage/Browser.js
@@ -141,8 +141,10 @@ Ext.define('PVE.storage.Browser', {
 		};
 		me.items.push({
 		    xtype: 'pveStorageContentView',
-		    title: gettext('Virtual Guests'),
-		    iconCls: 'fa fa-desktop',
+		    // each gettext needs to be in a separate line
+		    title: isEsxi ? gettext('Virtual Guests')
+			: gettext('Import'),
+		    iconCls: isEsxi ? 'fa fa-desktop' : 'fa fa-cloud-download',
 		    itemId: 'contentImport',
 		    content: 'import',
 		    useCustomRemoveButton: isEsxi, // hide default remove button for esxi
-- 
2.39.5



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

[pve-devel] [PATCH manager v7 8/9] ui: import: show size for dir-based storages

[Dominik Csapak]

since there we already have the size information

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 www/manager6/storage/Browser.js | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/www/manager6/storage/Browser.js b/www/manager6/storage/Browser.js
index 763abc70..c0b66acc 100644
--- a/www/manager6/storage/Browser.js
+++ b/www/manager6/storage/Browser.js
@@ -148,7 +148,7 @@ Ext.define('PVE.storage.Browser', {
 		    itemId: 'contentImport',
 		    content: 'import',
 		    useCustomRemoveButton: isEsxi, // hide default remove button for esxi
-		    showColumns: ['name', 'format'],
+		    showColumns: isEsxi ? ['name', 'format'] : ['name', 'size', 'format'],
 		    enableUploadButton: enableUpload && !isEsxi,
 		    enableDownloadUrlButton: enableDownloadUrl && !isEsxi,
 		    useUploadButton: !isEsxi,
-- 
2.39.5



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

[pve-devel] [PATCH manager v7 9/9] ui: import: adapt live import help text to ova

[Dominik Csapak]

For ova import, the help text for live-import is not really fitting, as
there is no 'old' vm that needs to be stopped, etc.

So show a more appropriate one if we import an ova

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
new in v7
 www/manager6/window/GuestImport.js | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/www/manager6/window/GuestImport.js b/www/manager6/window/GuestImport.js
index 7c263241..022e85a3 100644
--- a/www/manager6/window/GuestImport.js
+++ b/www/manager6/window/GuestImport.js
@@ -315,6 +315,8 @@ Ext.define('PVE.window.GuestImport', {
 	    liveImportNote: get => !get('liveImport') ? ''
 	        : gettext('Note: If anything goes wrong during the live-import, new data written by the VM may be lost.'),
 	    isWindows: get => (get('os') ?? '').startsWith('w'),
+	    liveImportText: get => get('isOva') ? gettext('Starts a VM and imports the disks in the background')
+		: gettext('Starts a previously stopped VM on Proxmox VE and imports the disks in the background.'),
 	},
     },
 
@@ -582,9 +584,9 @@ Ext.define('PVE.window.GuestImport', {
 			fieldLabel: gettext('Live Import'),
 			reference: 'liveimport',
 			isFormField: false,
-			boxLabel: gettext('Starts a previously stopped VM on Proxmox VE and imports the disks in the background.'),
 			bind: {
 			    value: '{liveImport}',
+			    boxLabel: '{liveImportText}',
 			},
 		    },
 		    {
-- 
2.39.5



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

Re: [pve-devel] [PATCH storage/qemu-server/manager v7] implement ova/ovf import for file based storages

[Aaron Lauterer]

Gave this series a test.


GNS3 OVA, the following variants:

OVA location -> Target Storage

local -> local: 	worked
local -> zfs: 		worked
local -> lvm-thin: 	worked
local -> rbd: 		worked

cephfs -> any: did not work. got the following error:
---
scsi0: storage selected for extraction does not support 'images' content 
type or is not file based.
---
Which makes me wonder, because in API2/Qemu.pm 184+ we check for 
content->image or if the storage has a path. The the cephfs does have a 
path set.
More inline in [PATCH qemu-server v7 4/5] api: create: add 
'import-working-storage' parameter

Also tested once, overriding the default config:
* CPU type host
* cores
* unique mac
* different vmbr/network
* VirtIO NIC
* VirtIO SCSI controller
* Override the target storage for one of the two disks

Which worked fine.


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

Re: [pve-devel] [PATCH storage/qemu-server/manager v7] implement ova/ovf import for file based storages

[Filip Schauer]

Hello,

I tried to quickly test the OVA import with a CentOS 7 OVA file that I
downloaded from:
https://dlconusc1.linuxvmimages.com/046389e06777452db2ccf9a32efa3760:dldatac/VirtualBox/C/7/CentOS_7.9.2009_VBM.zip

6785cb04dc5e7ab5a28ec139f631dfc8aeb8a6278c5fc66e1e7030d3f5a83b18 
/var/lib/vz/import/CentOS_7.9.2009_VBM_LinuxVMImages.COM.ova

After enabling the "import" content type on my local directory storage,
the ova file shows up in the import section of the storage.

Upon clicking "Import" in the web UI the "Import Guest" dialog opens
along with an error message stating:
XPath error : Undefined namespace prefix at 
/usr/share/perl5/PVE/GuestImport/OVF.pm line 239. (500)

Closing the error message also closes the import dialog, preventing me
from importing.

The same thing happens with this ArcoLinux OVA file:
https://master.dl.sourceforge.net/project/linuxovafiles/GUI/ArcoLinux-vm.ova?viasf=1 
to /var/lib/vz/import/ArcoLinux-vm.ova

2b353ac59598bac24d4cf0721e367935d00178a31d84e09c94425bdee3dab6a1 
/var/lib/vz/import/ArcoLinux-vm.ova

Tested-by: Filip Schauer <f.schauer@proxmox.com>

On 18/11/2024 16:29, Dominik Csapak wrote:
> This series enables importing ova/ovf from directory based storages,
> inclusive upload/download via the webui (ova only).
>
> It also improves the ovf importer by parsing the ostype, nics, bootorder
> (and firmware from vmware exported files).
>
> We now extract the images into either a given target storage or in the
> import storage in the 'images' dir so accidentally left over images
> are discoverable by the ui/cli.
>
> This version includes a modified qemu-server patch of fabians hardening series:
> https://lore.proxmox.com/pve-devel/20241104104221.228730-1-f.gruenbichler@proxmox.com/
>
> I sent the qemu-server patch from fabian again but omitted some
> problematic checks. I add them later with a check
> against the import vtype again (last patch in qemu-server)
>
> changes from v6:
> * rebase on master (omit already applied patches)
> * style/indentation fixes
> * omit explicit check for .ova in upload path
> * use assert_file_validity check for download_file_from_url
> * only warn on whitespace in parent image (file_size_info) to preserve backward compatibility
>    for possible existing cases
> * fix cleanup on live-import  (added wrong variable to cleanup list)
> * renamed 'import-extraction-storage' to 'import-working-storage'
> * mad priv check for extraction storage simpler (combine branches)
> * reworded some gettexts
>
> changes from v5:
> * removed leftover hunks in makefile
> * moved ova checks to correct patch
> * split up error messages for unexpected format
> * remove unnecessary untaint
> * reword error message
> * reintroduce symlink check in ova/ovf check
> * added sanity check for ovas after uploading/downloading
> * added new patch for checking import vtypes
> * fixed issue with files with absolute path
>
> changes from v4:
> * rebased on master/fabians series
> * added the file_size_info check for untrusted images after extracting
>
> changes from v3:
> * fixed dependencies in control file
> * removed unnecessary use statements
> * removed unnecessary remove helper
> * moved 'needs_extract' helper to qemu-server
> * removed import storage param from PUT call
> * check down/uploaded ova filename more strictly (same as listing)
> * improved filepath checking in ovf
> * forbid importing when extracted image references a base/backing file
> * instead of trying to manually create a proper filename, use 'alloc' to
>    create a small (1M) file with the same format and overwrite it with
>    renaming. this also solves the cluster locking issue
> * prefer using PVE::Storage functions instead of plugin methods in
>    ova extraction code
> * use $vollist for cleaning up extracted images in qemu-server and
>    add manual cleanup for the success case
>
> changes from v2:
> * use better 'format' values for embedded images (e.g. ova+vmdk)
> * use this format to decide if images should be extracted
> * consistent use of the 'safe character' classes when listing
>    and parsing
> * also list vmdk/qcow2/raw images in content listing
>    (this will be useful when we have a gui for the 'import-from'
>    in the wizard/disk edit for vms)
> * a few gui adaptions
>
>
> changes from v1:
> * move ovf code to GuestImport
> * move extract/checking code to GuestImport
> * don't return 'image' types from import volumes
> * use allow 'safe' characters for filenames of ova/ovfs and inside
> * check for non-regular files (e.g. symlinks) after extraction
> * add new 'import-extraction-storage' for import
> * rename panel in gui for directory storages
> * typo fixes
> * and probably more, see the individual patches for details
>
> pve-storage:
>
> Dominik Csapak (11):
>    plugin: dir: implement import content type
>    plugin: dir: handle ova files for import
>    ovf: improve and simplify path checking code
>    ovf: implement parsing the ostype
>    ovf: implement parsing out firmware type
>    ovf: implement rudimentary boot order
>    ovf: implement parsing nics
>    api: allow ova upload/download
>    plugin: enable import for nfs/btrfs/cifs/cephfs/glusterfs
>    add 'import' content type to 'check_volume_access'
>    plugin: file_size_info: warn on parent images with unusual path
>
>   src/PVE/API2/Storage/Status.pm                |  56 +++++-
>   src/PVE/GuestImport.pm                        |  79 ++++++++
>   src/PVE/GuestImport/OVF.pm                    | 187 ++++++++++++++++--
>   src/PVE/Makefile                              |   1 +
>   src/PVE/Storage.pm                            |  23 ++-
>   src/PVE/Storage/BTRFSPlugin.pm                |   5 +
>   src/PVE/Storage/CIFSPlugin.pm                 |   6 +-
>   src/PVE/Storage/CephFSPlugin.pm               |   6 +-
>   src/PVE/Storage/DirPlugin.pm                  |  52 ++++-
>   src/PVE/Storage/GlusterfsPlugin.pm            |   6 +-
>   src/PVE/Storage/NFSPlugin.pm                  |   6 +-
>   src/PVE/Storage/Plugin.pm                     |  16 +-
>   .../ovf_manifests/Win10-Liz_no_default_ns.ovf |   1 +
>   src/test/parse_volname_test.pm                |  33 ++++
>   src/test/path_to_volume_id_test.pm            |  21 ++
>   src/test/run_ovf_tests.pl                     |  14 ++
>   16 files changed, 480 insertions(+), 32 deletions(-)
>   create mode 100644 src/PVE/GuestImport.pm
>
> qemu-server:
>
> Dominik Csapak (4):
>    use OVF from Storage
>    api: create: implement extracting disks when needed for import-from
>    api: create: add 'import-working-storage' parameter
>    api: check untrusted image files for import content type
>
> Fabian Grünbichler (1):
>    disk import: add additional safeguards for imported image files
>
>   PVE/API2/Qemu.pm                              | 108 ++++++--
>   PVE/CLI/qm.pm                                 |   4 +-
>   PVE/QemuServer.pm                             |  12 +
>   PVE/QemuServer/Helpers.pm                     |   5 +
>   PVE/QemuServer/Makefile                       |   1 -
>   PVE/QemuServer/OVF.pm                         | 242 ------------------
>   debian/control                                |   2 -
>   test/Makefile                                 |   5 +-
>   test/ovf_manifests/Win10-Liz-disk1.vmdk       | Bin 65536 -> 0 bytes
>   test/ovf_manifests/Win10-Liz.ovf              | 142 ----------
>   .../ovf_manifests/Win10-Liz_no_default_ns.ovf | 142 ----------
>   test/ovf_manifests/Win_2008_R2_two-disks.ovf  | 145 -----------
>   test/ovf_manifests/disk1.vmdk                 | Bin 65536 -> 0 bytes
>   test/ovf_manifests/disk2.vmdk                 | Bin 65536 -> 0 bytes
>   test/run_ovf_tests.pl                         |  71 -----
>   15 files changed, 112 insertions(+), 767 deletions(-)
>   delete mode 100644 PVE/QemuServer/OVF.pm
>   delete mode 100644 test/ovf_manifests/Win10-Liz-disk1.vmdk
>   delete mode 100755 test/ovf_manifests/Win10-Liz.ovf
>   delete mode 100755 test/ovf_manifests/Win10-Liz_no_default_ns.ovf
>   delete mode 100755 test/ovf_manifests/Win_2008_R2_two-disks.ovf
>   delete mode 100644 test/ovf_manifests/disk1.vmdk
>   delete mode 100644 test/ovf_manifests/disk2.vmdk
>   delete mode 100755 test/run_ovf_tests.pl
>
> pve-manager:
>
> Dominik Csapak (9):
>    ui: guest import: add ova-needs-extracting warning text
>    ui: enable import content type for relevant storages
>    ui: enable upload/download/remove buttons for 'import' type storages
>    ui: disable 'import' button for non importable formats
>    ui: import: improve rendering of volume names
>    ui: guest import: add storage selector for ova extraction storage
>    ui: guest import: change icon/text for non-esxi import storage
>    ui: import: show size for dir-based storages
>    ui: import: adapt live import help text to ova
>
>   www/manager6/Utils.js                    |  9 +++++++-
>   www/manager6/form/ContentTypeSelector.js |  2 +-
>   www/manager6/storage/Browser.js          | 25 ++++++++++++++++-----
>   www/manager6/storage/CephFSEdit.js       |  2 +-
>   www/manager6/storage/GlusterFsEdit.js    |  2 +-
>   www/manager6/window/GuestImport.js       | 28 +++++++++++++++++++++++-
>   www/manager6/window/UploadToStorage.js   |  1 +
>   7 files changed, 58 insertions(+), 11 deletions(-)
>


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

Re: [pve-devel] [PATCH qemu-server v7 4/5] api: create: add 'import-working-storage' parameter

[Aaron Lauterer]

On  2024-11-18  16:29, Dominik Csapak wrote:
> this is to override the target extraction storage for the option disk
> extraction for 'import-from'. This way if the storage does not
> supports the content type 'images', one can give an alternative  one.
> 
> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
> ---
> changes from v6:
> * rename 'import-extraction-storage' to 'import-working-storage'
> * rework permission checks (single branch)
> 
> i opted to not make the target the first default, since i did not want
> to introduce such a change this late in the patch/review cycle and
> AFAICS quite a few code changes would have been necessary for that
> (we can still change that later too)
> 
>   PVE/API2/Qemu.pm | 48 +++++++++++++++++++++++++++++++++++++++---------
>   1 file changed, 39 insertions(+), 9 deletions(-)
> 
> diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
> index c947f09c..701558a7 100644
> --- a/PVE/API2/Qemu.pm
> +++ b/PVE/API2/Qemu.pm
> @@ -132,7 +132,7 @@ my $check_drive_param = sub {
>   };
>   
>   my $check_storage_access = sub {
> -   my ($rpcenv, $authuser, $storecfg, $vmid, $settings, $default_storage) = @_;
> +   my ($rpcenv, $authuser, $storecfg, $vmid, $settings, $default_storage, $extraction_storage) = @_;
>   
>      $foreach_volume_with_alloc->($settings, sub {
>   	my ($ds, $drive) = @_;
> @@ -174,9 +174,18 @@ my $check_storage_access = sub {
>   		    if $vtype ne 'images' && $vtype ne 'import';
>   
>   		if (PVE::QemuServer::Helpers::needs_extraction($vtype, $fmt)) {
> -		    raise_param_exc({ $ds => "$src_image is not on an storage with 'images' content type."})
> -			if !$scfg->{content}->{images};
> -		    $rpcenv->check($authuser, "/storage/$storeid", ['Datastore.AllocateSpace']);
> +		    my $extraction_scfg = defined($extraction_storage) ?
> +			PVE::Storage::storage_config($storecfg, $extraction_storage) :
> +			$scfg;
> +		    my $extraction_param = defined($extraction_storage) ? 'import-working-storage' : $ds;
> +
> +		    if (!$extraction_scfg->{content}->{images} || !$extraction_scfg->{path}) {

I think the if condition here is grouped wrong.

As it is, once if one of the items (content->images or path) is falsy, 
we trigger it, but it should not be the case, if the other is actually true.

Consider the following diff, basically grouping the content->images || 
path condition, and only if the result is falsy, we want to raise the error.

diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
index 8db443d..545fe31 100644
--- a/PVE/API2/Qemu.pm
+++ b/PVE/API2/Qemu.pm
@@ -179,7 +179,7 @@ my $check_storage_access = sub {
                         $scfg;
                     my $extraction_param = defined($extraction_storage) 
? 'import-working-storage' : $ds;

-                   if (!$extraction_scfg->{content}->{images} || 
!$extraction_scfg->{path}) {
+                   if (!($extraction_scfg->{content}->{images} || 
$extraction_scfg->{path})) {
                         raise_param_exc({
                             $extraction_param => "storage selected for 
extraction does not support"
                                 ." 'images' content type or is not file 
based.",



> +			raise_param_exc({
> +			    $extraction_param => "storage selected for extraction does not support"
> +				." 'images' content type or is not file based.",
> +			});
> +		    }
> +		    $rpcenv->check($authuser, "/storage/" . ($extraction_storage // $storeid), ['Datastore.AllocateSpace']);
>   		}
>   	    }
>   
> @@ -349,7 +358,7 @@ my sub prohibit_tpm_version_change {


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

Re: [pve-devel] [PATCH storage/qemu-server/manager v7] implement ova/ovf import for file based storages

[Aaron Lauterer]

On  2024-11-18  18:14, Filip Schauer wrote:
> Hello,
> 
> I tried to quickly test the OVA import with a CentOS 7 OVA file that I
> downloaded from:
> https:// 
> dlconusc1.linuxvmimages.com/046389e06777452db2ccf9a32efa3760:dldatac/ 
> VirtualBox/C/7/CentOS_7.9.2009_VBM.zip
> 
> 6785cb04dc5e7ab5a28ec139f631dfc8aeb8a6278c5fc66e1e7030d3f5a83b18 /var/ 
> lib/vz/import/CentOS_7.9.2009_VBM_LinuxVMImages.COM.ova
> 
> After enabling the "import" content type on my local directory storage,
> the ova file shows up in the import section of the storage.
> 
> Upon clicking "Import" in the web UI the "Import Guest" dialog opens
> along with an error message stating:
> XPath error : Undefined namespace prefix at /usr/share/perl5/PVE/ 
> GuestImport/OVF.pm line 239. (500)

I untar'ed that OVA and inspected the OVF. The filter for that is:
     my $xpath_find_firmware = 
"/ovf:Envelope/ovf:VirtualSystem/ovf:VirtualHardwareSection/vmw:Config[\@vmw:key=\"firmware\"]/\@vmw:value";

The GNS3 OVF does have `vmw:Config` elements, though none for the 
firmware -> Default SeaBIOS

The CentOS OVF does not have any `vmw:Config` sub elements in that 
search path. That is probably what is causing the issue.

> 
> Closing the error message also closes the import dialog, preventing me
> from importing.
> 
> The same thing happens with this ArcoLinux OVA file:
> https://master.dl.sourceforge.net/project/linuxovafiles/GUI/ArcoLinux- 
> vm.ova?viasf=1 to /var/lib/vz/import/ArcoLinux-vm.ova
> 
> 2b353ac59598bac24d4cf0721e367935d00178a31d84e09c94425bdee3dab6a1 /var/ 
> lib/vz/import/ArcoLinux-vm.ova
> 
> Tested-by: Filip Schauer <f.schauer@proxmox.com>
> 


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

Re: [pve-devel] [PATCH qemu-server v7 4/5] api: create: add 'import-working-storage' parameter

[Aaron Lauterer]

lore.proxmox.com seems to show the diff wrong...

On  2024-11-18  18:24, Aaron Lauterer wrote:
> 
> 
> On  2024-11-18  16:29, Dominik Csapak wrote:
>> this is to override the target extraction storage for the option disk
>> extraction for 'import-from'. This way if the storage does not
>> supports the content type 'images', one can give an alternative  one.
>>
>> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
>> ---
>> changes from v6:
>> * rename 'import-extraction-storage' to 'import-working-storage'
>> * rework permission checks (single branch)
>>
>> i opted to not make the target the first default, since i did not want
>> to introduce such a change this late in the patch/review cycle and
>> AFAICS quite a few code changes would have been necessary for that
>> (we can still change that later too)
>>
>>   PVE/API2/Qemu.pm | 48 +++++++++++++++++++++++++++++++++++++++---------
>>   1 file changed, 39 insertions(+), 9 deletions(-)
>>
>> diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
>> index c947f09c..701558a7 100644
>> --- a/PVE/API2/Qemu.pm
>> +++ b/PVE/API2/Qemu.pm
>> @@ -132,7 +132,7 @@ my $check_drive_param = sub {
>>   };
>>   my $check_storage_access = sub {
>> -   my ($rpcenv, $authuser, $storecfg, $vmid, $settings, 
>> $default_storage) = @_;
>> +   my ($rpcenv, $authuser, $storecfg, $vmid, $settings, 
>> $default_storage, $extraction_storage) = @_;
>>      $foreach_volume_with_alloc->($settings, sub {
>>       my ($ds, $drive) = @_;
>> @@ -174,9 +174,18 @@ my $check_storage_access = sub {
>>               if $vtype ne 'images' && $vtype ne 'import';
>>           if (PVE::QemuServer::Helpers::needs_extraction($vtype, $fmt)) {
>> -            raise_param_exc({ $ds => "$src_image is not on an storage 
>> with 'images' content type."})
>> -            if !$scfg->{content}->{images};
>> -            $rpcenv->check($authuser, "/storage/$storeid", 
>> ['Datastore.AllocateSpace']);
>> +            my $extraction_scfg = defined($extraction_storage) ?
>> +            PVE::Storage::storage_config($storecfg, 
>> $extraction_storage) :
>> +            $scfg;
>> +            my $extraction_param = defined($extraction_storage) ? 
>> 'import-working-storage' : $ds;
>> +
>> +            if (!$extraction_scfg->{content}->{images} || ! 
>> $extraction_scfg->{path}) {
> 
> I think the if condition here is grouped wrong.
> 
> As it is, once if one of the items (content->images or path) is falsy, 
> we trigger it, but it should not be the case, if the other is actually 
> true.
> 
> Consider the following diff, basically grouping the content->images || 
> path condition, and only if the result is falsy, we want to raise the 
> error.
> 
> diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
> index 8db443d..545fe31 100644
> --- a/PVE/API2/Qemu.pm
> +++ b/PVE/API2/Qemu.pm
> @@ -179,7 +179,7 @@ my $check_storage_access = sub {
>                          $scfg;
>                      my $extraction_param = 
> defined($extraction_storage) ? 'import-working-storage' : $ds;
> 
> -                   if (!$extraction_scfg->{content}->{images} || ! 
> $extraction_scfg->{path}) {
> +                   if (!($extraction_scfg->{content}->{images} || 
> $extraction_scfg->{path})) {
>                          raise_param_exc({
>                              $extraction_param => "storage selected for 
> extraction does not support"
>                                  ." 'images' content type or is not file 
> based.",

Lets try it once more, hopefully the line endings will be as I want 
them, even on lore.proxmox.com


diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
index 8db443d..545fe31 100644
--- a/PVE/API2/Qemu.pm
+++ b/PVE/API2/Qemu.pm
@@ -179,7 +179,7 @@ my $check_storage_access = sub {
                         $scfg;
                     my $extraction_param = defined($extraction_storage) 
? 'import-working-storage' : $ds;

-                   if (!$extraction_scfg->{content}->{images} || 
!$extraction_scfg->{path}) {
+                   if (!($extraction_scfg->{content}->{images} || 
$extraction_scfg->{path})) {
                         raise_param_exc({
                             $extraction_param => "storage selected for 
extraction does not support"
                                 ." 'images' content type or is not file 
based.",


Also, I was able to run into this by uploading the OVA to a CephFS.

> 
> 
> 
>> +            raise_param_exc({
>> +                $extraction_param => "storage selected for extraction 
>> does not support"
>> +                ." 'images' content type or is not file based.",
>> +            });
>> +            }
>> +            $rpcenv->check($authuser, "/storage/" . 
>> ($extraction_storage // $storeid), ['Datastore.AllocateSpace']);
>>           }
>>           }
>> @@ -349,7 +358,7 @@ my sub prohibit_tpm_version_change {
> 
> 
> _______________________________________________
> 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

Re: [pve-devel] [PATCH qemu-server v7 4/5] api: create: add 'import-working-storage' parameter

[Thomas Lamprecht]

Am 18.11.24 um 18:39 schrieb Aaron Lauterer:
> lore.proxmox.com seems to show the diff wrong...
> 

I see them "wrong" too in my inbox, seems your mail user agent messes
with them?


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

Re: [pve-devel] [PATCH storage/qemu-server/manager v7] implement ova/ovf import for file based storages

[Filip Schauer]

I tried importing the GNS3 OVA files found here:
https://www.gns3.com/software/download-vm

I got the same XPath error with the VirtualBox file.

The ESXi and Workstation & Fusion OVA files were imported successfully
and they boot aswell.


On 18/11/2024 18:14, Filip Schauer wrote:
> Hello,
>
> I tried to quickly test the OVA import with a CentOS 7 OVA file that I
> downloaded from:
> https://dlconusc1.linuxvmimages.com/046389e06777452db2ccf9a32efa3760:dldatac/VirtualBox/C/7/CentOS_7.9.2009_VBM.zip 
>
>
> 6785cb04dc5e7ab5a28ec139f631dfc8aeb8a6278c5fc66e1e7030d3f5a83b18 
> /var/lib/vz/import/CentOS_7.9.2009_VBM_LinuxVMImages.COM.ova
>
> After enabling the "import" content type on my local directory storage,
> the ova file shows up in the import section of the storage.
>
> Upon clicking "Import" in the web UI the "Import Guest" dialog opens
> along with an error message stating:
> XPath error : Undefined namespace prefix at 
> /usr/share/perl5/PVE/GuestImport/OVF.pm line 239. (500)
>
> Closing the error message also closes the import dialog, preventing me
> from importing.
>
> The same thing happens with this ArcoLinux OVA file:
> https://master.dl.sourceforge.net/project/linuxovafiles/GUI/ArcoLinux-vm.ova?viasf=1 
> to /var/lib/vz/import/ArcoLinux-vm.ova
>
> 2b353ac59598bac24d4cf0721e367935d00178a31d84e09c94425bdee3dab6a1 
> /var/lib/vz/import/ArcoLinux-vm.ova
>
> Tested-by: Filip Schauer <f.schauer@proxmox.com>
>
> On 18/11/2024 16:29, Dominik Csapak wrote:
>> This series enables importing ova/ovf from directory based storages,
>> inclusive upload/download via the webui (ova only).
>>
>> It also improves the ovf importer by parsing the ostype, nics, bootorder
>> (and firmware from vmware exported files).
>>
>> We now extract the images into either a given target storage or in the
>> import storage in the 'images' dir so accidentally left over images
>> are discoverable by the ui/cli.
>>
>> This version includes a modified qemu-server patch of fabians 
>> hardening series:
>> https://lore.proxmox.com/pve-devel/20241104104221.228730-1-f.gruenbichler@proxmox.com/ 
>>
>>
>> I sent the qemu-server patch from fabian again but omitted some
>> problematic checks. I add them later with a check
>> against the import vtype again (last patch in qemu-server)
>>
>> changes from v6:
>> * rebase on master (omit already applied patches)
>> * style/indentation fixes
>> * omit explicit check for .ova in upload path
>> * use assert_file_validity check for download_file_from_url
>> * only warn on whitespace in parent image (file_size_info) to 
>> preserve backward compatibility
>>    for possible existing cases
>> * fix cleanup on live-import  (added wrong variable to cleanup list)
>> * renamed 'import-extraction-storage' to 'import-working-storage'
>> * mad priv check for extraction storage simpler (combine branches)
>> * reworded some gettexts
>>
>> changes from v5:
>> * removed leftover hunks in makefile
>> * moved ova checks to correct patch
>> * split up error messages for unexpected format
>> * remove unnecessary untaint
>> * reword error message
>> * reintroduce symlink check in ova/ovf check
>> * added sanity check for ovas after uploading/downloading
>> * added new patch for checking import vtypes
>> * fixed issue with files with absolute path
>>
>> changes from v4:
>> * rebased on master/fabians series
>> * added the file_size_info check for untrusted images after extracting
>>
>> changes from v3:
>> * fixed dependencies in control file
>> * removed unnecessary use statements
>> * removed unnecessary remove helper
>> * moved 'needs_extract' helper to qemu-server
>> * removed import storage param from PUT call
>> * check down/uploaded ova filename more strictly (same as listing)
>> * improved filepath checking in ovf
>> * forbid importing when extracted image references a base/backing file
>> * instead of trying to manually create a proper filename, use 'alloc' to
>>    create a small (1M) file with the same format and overwrite it with
>>    renaming. this also solves the cluster locking issue
>> * prefer using PVE::Storage functions instead of plugin methods in
>>    ova extraction code
>> * use $vollist for cleaning up extracted images in qemu-server and
>>    add manual cleanup for the success case
>>
>> changes from v2:
>> * use better 'format' values for embedded images (e.g. ova+vmdk)
>> * use this format to decide if images should be extracted
>> * consistent use of the 'safe character' classes when listing
>>    and parsing
>> * also list vmdk/qcow2/raw images in content listing
>>    (this will be useful when we have a gui for the 'import-from'
>>    in the wizard/disk edit for vms)
>> * a few gui adaptions
>>
>>
>> changes from v1:
>> * move ovf code to GuestImport
>> * move extract/checking code to GuestImport
>> * don't return 'image' types from import volumes
>> * use allow 'safe' characters for filenames of ova/ovfs and inside
>> * check for non-regular files (e.g. symlinks) after extraction
>> * add new 'import-extraction-storage' for import
>> * rename panel in gui for directory storages
>> * typo fixes
>> * and probably more, see the individual patches for details
>>
>> pve-storage:
>>
>> Dominik Csapak (11):
>>    plugin: dir: implement import content type
>>    plugin: dir: handle ova files for import
>>    ovf: improve and simplify path checking code
>>    ovf: implement parsing the ostype
>>    ovf: implement parsing out firmware type
>>    ovf: implement rudimentary boot order
>>    ovf: implement parsing nics
>>    api: allow ova upload/download
>>    plugin: enable import for nfs/btrfs/cifs/cephfs/glusterfs
>>    add 'import' content type to 'check_volume_access'
>>    plugin: file_size_info: warn on parent images with unusual path
>>
>>   src/PVE/API2/Storage/Status.pm                |  56 +++++-
>>   src/PVE/GuestImport.pm                        |  79 ++++++++
>>   src/PVE/GuestImport/OVF.pm                    | 187 ++++++++++++++++--
>>   src/PVE/Makefile                              |   1 +
>>   src/PVE/Storage.pm                            |  23 ++-
>>   src/PVE/Storage/BTRFSPlugin.pm                |   5 +
>>   src/PVE/Storage/CIFSPlugin.pm                 |   6 +-
>>   src/PVE/Storage/CephFSPlugin.pm               |   6 +-
>>   src/PVE/Storage/DirPlugin.pm                  |  52 ++++-
>>   src/PVE/Storage/GlusterfsPlugin.pm            |   6 +-
>>   src/PVE/Storage/NFSPlugin.pm                  |   6 +-
>>   src/PVE/Storage/Plugin.pm                     |  16 +-
>>   .../ovf_manifests/Win10-Liz_no_default_ns.ovf |   1 +
>>   src/test/parse_volname_test.pm                |  33 ++++
>>   src/test/path_to_volume_id_test.pm            |  21 ++
>>   src/test/run_ovf_tests.pl                     |  14 ++
>>   16 files changed, 480 insertions(+), 32 deletions(-)
>>   create mode 100644 src/PVE/GuestImport.pm
>>
>> qemu-server:
>>
>> Dominik Csapak (4):
>>    use OVF from Storage
>>    api: create: implement extracting disks when needed for import-from
>>    api: create: add 'import-working-storage' parameter
>>    api: check untrusted image files for import content type
>>
>> Fabian Grünbichler (1):
>>    disk import: add additional safeguards for imported image files
>>
>>   PVE/API2/Qemu.pm                              | 108 ++++++--
>>   PVE/CLI/qm.pm                                 |   4 +-
>>   PVE/QemuServer.pm                             |  12 +
>>   PVE/QemuServer/Helpers.pm                     |   5 +
>>   PVE/QemuServer/Makefile                       |   1 -
>>   PVE/QemuServer/OVF.pm                         | 242 ------------------
>>   debian/control                                |   2 -
>>   test/Makefile                                 |   5 +-
>>   test/ovf_manifests/Win10-Liz-disk1.vmdk       | Bin 65536 -> 0 bytes
>>   test/ovf_manifests/Win10-Liz.ovf              | 142 ----------
>>   .../ovf_manifests/Win10-Liz_no_default_ns.ovf | 142 ----------
>>   test/ovf_manifests/Win_2008_R2_two-disks.ovf  | 145 -----------
>>   test/ovf_manifests/disk1.vmdk                 | Bin 65536 -> 0 bytes
>>   test/ovf_manifests/disk2.vmdk                 | Bin 65536 -> 0 bytes
>>   test/run_ovf_tests.pl                         |  71 -----
>>   15 files changed, 112 insertions(+), 767 deletions(-)
>>   delete mode 100644 PVE/QemuServer/OVF.pm
>>   delete mode 100644 test/ovf_manifests/Win10-Liz-disk1.vmdk
>>   delete mode 100755 test/ovf_manifests/Win10-Liz.ovf
>>   delete mode 100755 test/ovf_manifests/Win10-Liz_no_default_ns.ovf
>>   delete mode 100755 test/ovf_manifests/Win_2008_R2_two-disks.ovf
>>   delete mode 100644 test/ovf_manifests/disk1.vmdk
>>   delete mode 100644 test/ovf_manifests/disk2.vmdk
>>   delete mode 100755 test/run_ovf_tests.pl
>>
>> pve-manager:
>>
>> Dominik Csapak (9):
>>    ui: guest import: add ova-needs-extracting warning text
>>    ui: enable import content type for relevant storages
>>    ui: enable upload/download/remove buttons for 'import' type storages
>>    ui: disable 'import' button for non importable formats
>>    ui: import: improve rendering of volume names
>>    ui: guest import: add storage selector for ova extraction storage
>>    ui: guest import: change icon/text for non-esxi import storage
>>    ui: import: show size for dir-based storages
>>    ui: import: adapt live import help text to ova
>>
>>   www/manager6/Utils.js                    |  9 +++++++-
>>   www/manager6/form/ContentTypeSelector.js |  2 +-
>>   www/manager6/storage/Browser.js          | 25 ++++++++++++++++-----
>>   www/manager6/storage/CephFSEdit.js       |  2 +-
>>   www/manager6/storage/GlusterFsEdit.js    |  2 +-
>>   www/manager6/window/GuestImport.js       | 28 +++++++++++++++++++++++-
>>   www/manager6/window/UploadToStorage.js   |  1 +
>>   7 files changed, 58 insertions(+), 11 deletions(-)
>>
>
>
> _______________________________________________
> 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

Re: [pve-devel] [PATCH storage/qemu-server/manager v7] implement ova/ovf import for file based storages

[Thomas Lamprecht]

Am 18.11.24 um 18:35 schrieb Aaron Lauterer:
>> Upon clicking "Import" in the web UI the "Import Guest" dialog opens
>> along with an error message stating:
>> XPath error : Undefined namespace prefix at /usr/share/perl5/PVE/ 
>> GuestImport/OVF.pm line 239. (500)
> I untar'ed that OVA and inspected the OVF. The filter for that is:
>      my $xpath_find_firmware = 
> "/ovf:Envelope/ovf:VirtualSystem/ovf:VirtualHardwareSection/vmw:Config[\@vmw:key=\"firmware\"]/\@vmw:value";
> 
> The GNS3 OVF does have `vmw:Config` elements, though none for the 
> firmware -> Default SeaBIOS
> 
> The CentOS OVF does not have any `vmw:Config` sub elements in that 
> search path. That is probably what is causing the issue.

There might also be a 

$xpc->registerNs('vmw', "???");

missing further above?

But I need to relearn that xpath/xml stuff, I was blessed for way to long
to just ignore (and forget) most of it.


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

Re: [pve-devel] [PATCH storage/qemu-server/manager v7] implement ova/ovf import for file based storages

[Thomas Lamprecht]

Am 18.11.24 um 18:44 schrieb Filip Schauer:
> I tried importing the GNS3 OVA files found here:
> https://www.gns3.com/software/download-vm
> 
> I got the same XPath error with the VirtualBox file.

Can you try something like:

diff --git a/src/PVE/GuestImport/OVF.pm b/src/PVE/GuestImport/OVF.pm
index 4af58ed..0348807 100644
--- a/src/PVE/GuestImport/OVF.pm
+++ b/src/PVE/GuestImport/OVF.pm
@@ -191,6 +191,7 @@ sub parse_ovf {
     # 'ovf' is the default namespace so it will prepended to each xml element
     my $xpc = XML::LibXML::XPathContext->new($dom);
     $xpc->registerNs('ovf', 'http://schemas.dmtf.org/ovf/envelope/1');
+    $xpc->registerNs('vmf', 'http://schemas.dmtf.org/ovf/envelope/1'); # FIXME: fix schema URL
     $xpc->registerNs('rasd', 'http://schemas.dmtf.org/wbem/wscim/1/cim-schema/2/CIM_ResourceAllocationSettingData');
     $xpc->registerNs('vssd', 'http://schemas.dmtf.org/wbem/wscim/1/cim-schema/2/CIM_VirtualSystemSettingData');



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

Re: [pve-devel] [PATCH storage/qemu-server/manager v7] implement ova/ovf import for file based storages

[Thomas Lamprecht]

Am 18.11.24 um 18:49 schrieb Thomas Lamprecht:
> Am 18.11.24 um 18:44 schrieb Filip Schauer:
>> I tried importing the GNS3 OVA files found here:
>> https://www.gns3.com/software/download-vm
>>
>> I got the same XPath error with the VirtualBox file.
> 
> Can you try something like:
> 
> diff --git a/src/PVE/GuestImport/OVF.pm b/src/PVE/GuestImport/OVF.pm
> index 4af58ed..0348807 100644
> --- a/src/PVE/GuestImport/OVF.pm
> +++ b/src/PVE/GuestImport/OVF.pm
> @@ -191,6 +191,7 @@ sub parse_ovf {
>      # 'ovf' is the default namespace so it will prepended to each xml element
>      my $xpc = XML::LibXML::XPathContext->new($dom);
>      $xpc->registerNs('ovf', 'http://schemas.dmtf.org/ovf/envelope/1');
> +    $xpc->registerNs('vmf', 'http://schemas.dmtf.org/ovf/envelope/1'); # FIXME: fix schema URL

I meant to write s/vmf/vmw/ i.e.:

diff --git a/src/PVE/GuestImport/OVF.pm b/src/PVE/GuestImport/OVF.pm
index 4af58ed..ed6fcea 100644
--- a/src/PVE/GuestImport/OVF.pm
+++ b/src/PVE/GuestImport/OVF.pm
@@ -191,6 +191,7 @@ sub parse_ovf {
     # 'ovf' is the default namespace so it will prepended to each xml element
     my $xpc = XML::LibXML::XPathContext->new($dom);
     $xpc->registerNs('ovf', 'http://schemas.dmtf.org/ovf/envelope/1');
+    $xpc->registerNs('vmw', 'http://www.vmware.com/schema/ovf');
     $xpc->registerNs('rasd', 'http://schemas.dmtf.org/wbem/wscim/1/cim-schema/2/CIM_ResourceAllocationSettingData');
     $xpc->registerNs('vssd', 'http://schemas.dmtf.org/wbem/wscim/1/cim-schema/2/CIM_VirtualSystemSettingData');
 


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

Re: [pve-devel] [PATCH storage/qemu-server/manager v7] implement ova/ovf import for file based storages

[Thomas Lamprecht]

Am 18.11.24 um 18:53 schrieb Thomas Lamprecht:
> diff --git a/src/PVE/GuestImport/OVF.pm b/src/PVE/GuestImport/OVF.pm
> index 4af58ed..ed6fcea 100644
> --- a/src/PVE/GuestImport/OVF.pm
> +++ b/src/PVE/GuestImport/OVF.pm
> @@ -191,6 +191,7 @@ sub parse_ovf {
>      # 'ovf' is the default namespace so it will prepended to each xml element
>      my $xpc = XML::LibXML::XPathContext->new($dom);
>      $xpc->registerNs('ovf', 'http://schemas.dmtf.org/ovf/envelope/1');
> +    $xpc->registerNs('vmw', 'http://www.vmware.com/schema/ovf');
>      $xpc->registerNs('rasd', 'http://schemas.dmtf.org/wbem/wscim/1/cim-schema/2/CIM_ResourceAllocationSettingData');
>      $xpc->registerNs('vssd', 'http://schemas.dmtf.org/wbem/wscim/1/cim-schema/2/CIM_VirtualSystemSettingData');
>  

both, the GNS3 VirtualBox and the CentOS 7 image you linked work with above.
So I would just commit that as follow-up, or do you have other results?


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

Re: [pve-devel] [PATCH storage/qemu-server/manager v7] implement ova/ovf import for file based storages

[Filip Schauer]

On 18/11/2024 18:53, Thomas Lamprecht wrote:
> diff --git a/src/PVE/GuestImport/OVF.pm b/src/PVE/GuestImport/OVF.pm 
> index 4af58ed..ed6fcea 100644 --- a/src/PVE/GuestImport/OVF.pm +++ 
> b/src/PVE/GuestImport/OVF.pm @@ -191,6 +191,7 @@ sub parse_ovf { # 
> 'ovf' is the default namespace so it will prepended to each xml 
> element my $xpc = XML::LibXML::XPathContext->new($dom); 
> $xpc->registerNs('ovf', 'http://schemas.dmtf.org/ovf/envelope/1'); + 
> $xpc->registerNs('vmw', 'http://www.vmware.com/schema/ovf'); 
> $xpc->registerNs('rasd', 
> 'http://schemas.dmtf.org/wbem/wscim/1/cim-schema/2/CIM_ResourceAllocationSettingData'); 
> $xpc->registerNs('vssd', 
> 'http://schemas.dmtf.org/wbem/wscim/1/cim-schema/2/CIM_VirtualSystemSettingData');

This fixes the XPath error, but the "name" text box in the import dialog
is empty. When giving it a name and clicking "Import" a VM is
immediately created but it has no disks.



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

Re: [pve-devel] [PATCH storage/qemu-server/manager v7] implement ova/ovf import for file based storages

[Thomas Lamprecht]

Am 18.11.24 um 19:03 schrieb Filip Schauer:
> On 18/11/2024 18:53, Thomas Lamprecht wrote:
>> diff --git a/src/PVE/GuestImport/OVF.pm b/src/PVE/GuestImport/OVF.pm 
>> index 4af58ed..ed6fcea 100644 --- a/src/PVE/GuestImport/OVF.pm +++ 
>> b/src/PVE/GuestImport/OVF.pm @@ -191,6 +191,7 @@ sub parse_ovf { # 
>> 'ovf' is the default namespace so it will prepended to each xml 
>> element my $xpc = XML::LibXML::XPathContext->new($dom); 
>> $xpc->registerNs('ovf', 'http://schemas.dmtf.org/ovf/envelope/1'); + 
>> $xpc->registerNs('vmw', 'http://www.vmware.com/schema/ovf'); 
>> $xpc->registerNs('rasd', 
>> 'http://schemas.dmtf.org/wbem/wscim/1/cim-schema/2/CIM_ResourceAllocationSettingData'); 
>> $xpc->registerNs('vssd', 
>> 'http://schemas.dmtf.org/wbem/wscim/1/cim-schema/2/CIM_VirtualSystemSettingData');
> 
> This fixes the XPath error, but the "name" text box in the import dialog
> is empty. When giving it a name and clicking "Import" a VM is
> immediately created but it has no disks.

That is another issue though. Some other detections, like the two vNICs for GNS3
does work, so I guess here we just will need to add a few quirks for common
and/or popular OVA.


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

Re: [pve-devel] [PATCH storage/qemu-server/manager v7] implement ova/ovf import for file based storages

[Thomas Lamprecht]

Am 18.11.24 um 19:03 schrieb Filip Schauer:
> On 18/11/2024 18:53, Thomas Lamprecht wrote:
>> diff --git a/src/PVE/GuestImport/OVF.pm b/src/PVE/GuestImport/OVF.pm 
>> index 4af58ed..ed6fcea 100644 --- a/src/PVE/GuestImport/OVF.pm +++ 
>> b/src/PVE/GuestImport/OVF.pm @@ -191,6 +191,7 @@ sub parse_ovf { # 
>> 'ovf' is the default namespace so it will prepended to each xml 
>> element my $xpc = XML::LibXML::XPathContext->new($dom); 
>> $xpc->registerNs('ovf', 'http://schemas.dmtf.org/ovf/envelope/1'); + 
>> $xpc->registerNs('vmw', 'http://www.vmware.com/schema/ovf'); 
>> $xpc->registerNs('rasd', 
>> 'http://schemas.dmtf.org/wbem/wscim/1/cim-schema/2/CIM_ResourceAllocationSettingData'); 
>> $xpc->registerNs('vssd', 
>> 'http://schemas.dmtf.org/wbem/wscim/1/cim-schema/2/CIM_VirtualSystemSettingData');
> 
> This fixes the XPath error, but the "name" text box in the import dialog
> is empty. When giving it a name and clicking "Import" a VM is
> immediately created but it has no disks.

And FWIW, pveproxy does indeed log the following for GNS3:

pvedaemon[2813654]: warning: unable to parse the VM name in this OVF manifest, generating a default value
pvedaemon[2813654]: invalid host resource /disk/vmdisk1, skipping
pvedaemon[2813654]: invalid host resource /disk/vmdisk2, skipping


@Dominik, should we use the RPCEnv log_warn (or on info level) so that those
make it also into the task log? I mean, they should not really expose anything
hidden/secret, the OVA file is user provided after all and if anybody can use
it for an import they also should be able to see odd quirks?


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

Re: [pve-devel] [PATCH storage/qemu-server/manager v7] implement ova/ovf import for file based storages

[Thomas Lamprecht]

Am 18.11.24 um 19:03 schrieb Filip Schauer:
> On 18/11/2024 18:53, Thomas Lamprecht wrote:
>> diff --git a/src/PVE/GuestImport/OVF.pm b/src/PVE/GuestImport/OVF.pm 
>> index 4af58ed..ed6fcea 100644 --- a/src/PVE/GuestImport/OVF.pm +++ 
>> b/src/PVE/GuestImport/OVF.pm @@ -191,6 +191,7 @@ sub parse_ovf { # 
>> 'ovf' is the default namespace so it will prepended to each xml 
>> element my $xpc = XML::LibXML::XPathContext->new($dom); 
>> $xpc->registerNs('ovf', 'http://schemas.dmtf.org/ovf/envelope/1'); + 
>> $xpc->registerNs('vmw', 'http://www.vmware.com/schema/ovf'); 
>> $xpc->registerNs('rasd', 
>> 'http://schemas.dmtf.org/wbem/wscim/1/cim-schema/2/CIM_ResourceAllocationSettingData'); 
>> $xpc->registerNs('vssd', 
>> 'http://schemas.dmtf.org/wbem/wscim/1/cim-schema/2/CIM_VirtualSystemSettingData');
> 
> This fixes the XPath error, but the "name" text box in the import dialog
> is empty. When giving it a name and clicking "Import" a VM is
> immediately created but it has no disks.
> 

I now got the GNS3 and CentOS images you linked too to work, needed a few
improvements, but seem all not _that_ quirky. I applied them on top of
Dominik's series:

https://git.proxmox.com/?p=pve-storage.git;a=shortlog;h=811aa863cb66c1f49e6e4bd725c94626ed5082b5


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

Re: [pve-devel] [PATCH qemu-server v7 4/5] api: create: add 'import-working-storage' parameter

[Thomas Lamprecht]

I now looked into your diff.

Am 18.11.24 um 18:24 schrieb Aaron Lauterer:
> On  2024-11-18  16:29, Dominik Csapak wrote:
>> +		    if (!$extraction_scfg->{content}->{images} || !$extraction_scfg->{path}) {
> 
> I think the if condition here is grouped wrong.
> 
> As it is, once if one of the items (content->images or path) is falsy, 
> we trigger it, but it should not be the case, if the other is actually true.

how so? It must be both, file based and allow the import content type?

> Consider the following diff, basically grouping the content->images || 
> path condition, and only if the result is falsy, we want to raise the error.
> 
> diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
> index 8db443d..545fe31 100644
> --- a/PVE/API2/Qemu.pm
> +++ b/PVE/API2/Qemu.pm
> @@ -179,7 +179,7 @@ my $check_storage_access = sub {
>                          $scfg;
>                      my $extraction_param = defined($extraction_storage) ? 'import-working-storage' : $ds;
> 
> -                   if (!$extraction_scfg->{content}->{images} || !$extraction_scfg->{path}) {
> +                   if (!($extraction_scfg->{content}->{images} || $extraction_scfg->{path})) {

Besides above, the error would now be wrong too, it states:

"storage selected for extraction does not support 'images' content type or is not file based"

(not (support 'images' content type)) or is (not (file based))

Exactly what Dominik used.

The fix here would add the 'images' content-type as supported one for
CephFS.


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

[pve-devel] applied-series: [PATCH storage/qemu-server/manager v7] implement ova/ovf import for file based storages

[Thomas Lamprecht]

Am 18.11.24 um 16:29 schrieb Dominik Csapak:
> This series enables importing ova/ovf from directory based storages,
> inclusive upload/download via the webui (ova only).
> 
> It also improves the ovf importer by parsing the ostype, nics, bootorder
> (and firmware from vmware exported files).
> 
> We now extract the images into either a given target storage or in the
> import storage in the 'images' dir so accidentally left over images
> are discoverable by the ui/cli.
> 
> This version includes a modified qemu-server patch of fabians hardening series:
> https://lore.proxmox.com/pve-devel/20241104104221.228730-1-f.gruenbichler@proxmox.com/
> 
> I sent the qemu-server patch from fabian again but omitted some
> problematic checks. I add them later with a check
> against the import vtype again (last patch in qemu-server)
> 

applied series, many thanks! The result is IMO quite nice, well if one doesn't
run into to many OVA/OVF quirks, but I tried to address a few as follow ups in
pve-storage, maybe you could cross-check those changes out to ensure I didn't
miss/break something.


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

Re: [pve-devel] applied-series: [PATCH storage/qemu-server/manager v7] implement ova/ovf import for file based storages

[Dominik Csapak]

On 11/18/24 22:15, Thomas Lamprecht wrote:
> Am 18.11.24 um 16:29 schrieb Dominik Csapak:
>> This series enables importing ova/ovf from directory based storages,
>> inclusive upload/download via the webui (ova only).
>>
>> It also improves the ovf importer by parsing the ostype, nics, bootorder
>> (and firmware from vmware exported files).
>>
>> We now extract the images into either a given target storage or in the
>> import storage in the 'images' dir so accidentally left over images
>> are discoverable by the ui/cli.
>>
>> This version includes a modified qemu-server patch of fabians hardening series:
>> https://lore.proxmox.com/pve-devel/20241104104221.228730-1-f.gruenbichler@proxmox.com/
>>
>> I sent the qemu-server patch from fabian again but omitted some
>> problematic checks. I add them later with a check
>> against the import vtype again (last patch in qemu-server)
>>
> 
> applied series, many thanks! The result is IMO quite nice, well if one doesn't
> run into to many OVA/OVF quirks, but I tried to address a few as follow ups in
> pve-storage, maybe you could cross-check those changes out to ensure I didn't
> miss/break something.


yeah i'll check those patches (but looking good at a glance)

thanks!


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

Re: [pve-devel] applied-series: [PATCH storage/qemu-server/manager v7] implement ova/ovf import for file based storages

[Dominik Csapak]

On 11/18/24 22:15, Thomas Lamprecht wrote:
> Am 18.11.24 um 16:29 schrieb Dominik Csapak:
>> This series enables importing ova/ovf from directory based storages,
>> inclusive upload/download via the webui (ova only).
>>
>> It also improves the ovf importer by parsing the ostype, nics, bootorder
>> (and firmware from vmware exported files).
>>
>> We now extract the images into either a given target storage or in the
>> import storage in the 'images' dir so accidentally left over images
>> are discoverable by the ui/cli.
>>
>> This version includes a modified qemu-server patch of fabians hardening series:
>> https://lore.proxmox.com/pve-devel/20241104104221.228730-1-f.gruenbichler@proxmox.com/
>>
>> I sent the qemu-server patch from fabian again but omitted some
>> problematic checks. I add them later with a check
>> against the import vtype again (last patch in qemu-server)
>>
> 
> applied series, many thanks! The result is IMO quite nice, well if one doesn't
> run into to many OVA/OVF quirks, but I tried to address a few as follow ups in
> pve-storage, maybe you could cross-check those changes out to ensure I didn't
> miss/break something.

looked at them more closely and they seem all good to me, also
tested a bit around, and more things worked than yesterday
(e.g. the homeassistant ova)


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

Re: [pve-devel] [PATCH qemu-server v7 4/5] api: create: add 'import-working-storage' parameter

[Aaron Lauterer]

On  2024-11-18  21:22, Thomas Lamprecht wrote:
> I now looked into your diff.
> 
> Am 18.11.24 um 18:24 schrieb Aaron Lauterer:
>> On  2024-11-18  16:29, Dominik Csapak wrote:
>>> +		    if (!$extraction_scfg->{content}->{images} || !$extraction_scfg->{path}) {
>>
>> I think the if condition here is grouped wrong.
>>
>> As it is, once if one of the items (content->images or path) is falsy,
>> we trigger it, but it should not be the case, if the other is actually true.
> 
> how so? It must be both, file based and allow the import content type?
> 
>> Consider the following diff, basically grouping the content->images ||
>> path condition, and only if the result is falsy, we want to raise the error.
>>
>> diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
>> index 8db443d..545fe31 100644
>> --- a/PVE/API2/Qemu.pm
>> +++ b/PVE/API2/Qemu.pm
>> @@ -179,7 +179,7 @@ my $check_storage_access = sub {
>>                           $scfg;
>>                       my $extraction_param = defined($extraction_storage) ? 'import-working-storage' : $ds;
>>
>> -                   if (!$extraction_scfg->{content}->{images} || !$extraction_scfg->{path}) {
>> +                   if (!($extraction_scfg->{content}->{images} || $extraction_scfg->{path})) {
> 
> Besides above, the error would now be wrong too, it states:
> 
> "storage selected for extraction does not support 'images' content type or is not file based"
> 
> (not (support 'images' content type)) or is (not (file based))
> 
> Exactly what Dominik used.

For completeness’ sake: I talked in person with Dominik and yes, my 
understanding was wrong.

AFAIU Dominik will send a few follow-up patches, one of them will change 
the error message to make it clearer that the issue is with the working 
or OVA source storage. As that would probably have helped me yesterday 
to not come to the wrong conclusion.

> 
> The fix here would add the 'images' content-type as supported one for
> CephFS.

This is something I would like to avoid unless we have a way to prevent 
using these images in guest configs. Starting a VM or CT with the disk 
image on a Ceph FS will lead to all kinds of issues. RBD is what should 
be used to store guest disk images in Ceph.


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