From: "Fabian Grünbichler" <f.gruenbichler@proxmox.com>
To: Proxmox VE development discussion <pve-devel@lists.proxmox.com>
Subject: Re: [pve-devel] [PATCH storage v3 02/10] plugin: dir: implement import content type
Date: Wed, 22 May 2024 11:24:29 +0200 [thread overview]
Message-ID: <1716368200.rpvan7m9t9.astroid@yuna.none> (raw)
In-Reply-To: <20240429112124.3819357-3-d.csapak@proxmox.com>
On April 29, 2024 1:21 pm, Dominik Csapak wrote:
> 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>
> ---
> src/PVE/GuestImport/OVF.pm | 3 +++
> src/PVE/Storage.pm | 8 +++++++
> src/PVE/Storage/DirPlugin.pm | 36 +++++++++++++++++++++++++++++-
> src/PVE/Storage/Plugin.pm | 11 ++++++++-
> src/test/parse_volname_test.pm | 18 +++++++++++++++
> src/test/path_to_volume_id_test.pm | 21 +++++++++++++++++
> 6 files changed, 95 insertions(+), 2 deletions(-)
>
> diff --git a/src/PVE/GuestImport/OVF.pm b/src/PVE/GuestImport/OVF.pm
> index 055ebf5..0eb5e9c 100644
> --- a/src/PVE/GuestImport/OVF.pm
> +++ b/src/PVE/GuestImport/OVF.pm
> @@ -222,6 +222,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"
> @@ -231,6 +233,7 @@ ovf:Item[rasd:InstanceID='%s']/rasd:ResourceType", $controller_id);
> disk_address => $pve_disk_address,
> backing_file => $backing_file_path,
> virtual_size => $virtual_size
> + relative_path => $filepath,
syntax error here (cleaned up in next patch)
> };
> push @disks, $pve_disk;
>
> diff --git a/src/PVE/Storage.pm b/src/PVE/Storage.pm
> index f19a115..1ed91c2 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)/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 8.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..3e3b1e7 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 '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 $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 22a9729..33f0f3a 100644
> --- a/src/PVE/Storage/Plugin.pm
> +++ b/src/PVE/Storage/Plugin.pm
> @@ -654,6 +654,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";
> @@ -666,6 +668,7 @@ my $vtype_subdirs = {
> vztmpl => 'template/cache',
> backup => 'dump',
> snippets => 'snippets',
> + import => 'import',
> };
>
> sub get_vtype_subdirs {
> @@ -1227,7 +1230,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) = @_;
>
> @@ -1283,6 +1286,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;
> @@ -1317,6 +1324,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 d6ac885..a8c746f 100644
> --- a/src/test/parse_volname_test.pm
> +++ b/src/test/parse_volname_test.pm
> @@ -81,6 +81,19 @@ my $tests = [
> expected => ['snippets', 'hookscript.pl'],
> },
> #
> + # Import
> + #
> + {
> + description => "Import, ova",
> + volname => 'import/import.ova',
> + expected => ['import', 'import.ova', undef, undef, undef ,undef, 'ova'],
> + },
with the syntax error above cleaned up, this test here fails since OVA
is not yet recognized as format here.. (similarly below as well).
> + {
> + description => "Import, ovf",
> + volname => 'import/import.ovf',
> + expected => ['import', 'import.ovf', undef, undef, undef ,undef, 'ovf'],
> + },
> + #
> # failed matches
> #
> {
> @@ -123,6 +136,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 8149c88..0d238f9 100644
> --- a/src/test/path_to_volume_id_test.pm
> +++ b/src/test/path_to_volume_id_test.pm
> @@ -174,6 +174,22 @@ 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",
> + expected => [
> + 'import',
> + 'local:import/import.ovf',
> + ],
> + },
>
> # no matches, path or files with failures
> {
> @@ -231,6 +247,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.2
>
>
>
> _______________________________________________
> 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
next prev parent reply other threads:[~2024-05-22 9:24 UTC|newest]
Thread overview: 38+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-04-29 11:21 [pve-devel] [PATCH storage/qemu-server/manager v3] implement ova/ovf import for file based storages Dominik Csapak
2024-04-29 11:21 ` [pve-devel] [PATCH storage v3 01/10] copy OVF.pm from qemu-server Dominik Csapak
2024-05-22 8:56 ` Fabian Grünbichler
2024-05-22 9:35 ` Fabian Grünbichler
2024-04-29 11:21 ` [pve-devel] [PATCH storage v3 02/10] plugin: dir: implement import content type Dominik Csapak
2024-05-22 9:24 ` Fabian Grünbichler [this message]
2024-04-29 11:21 ` [pve-devel] [PATCH storage v3 03/10] plugin: dir: handle ova files for import Dominik Csapak
2024-05-22 10:08 ` Fabian Grünbichler
2024-05-23 10:40 ` Dominik Csapak
2024-05-23 12:25 ` Fabian Grünbichler
2024-05-23 12:32 ` Dominik Csapak
2024-05-22 13:13 ` Fabian Grünbichler
2024-04-29 11:21 ` [pve-devel] [PATCH storage v3 04/10] ovf: implement parsing the ostype Dominik Csapak
2024-04-29 11:21 ` [pve-devel] [PATCH storage v3 05/10] ovf: implement parsing out firmware type Dominik Csapak
2024-04-29 11:21 ` [pve-devel] [PATCH storage v3 06/10] ovf: implement rudimentary boot order Dominik Csapak
2024-04-29 11:21 ` [pve-devel] [PATCH storage v3 07/10] ovf: implement parsing nics Dominik Csapak
2024-04-29 11:21 ` [pve-devel] [PATCH storage v3 08/10] api: allow ova upload/download Dominik Csapak
2024-05-22 10:20 ` Fabian Grünbichler
2024-04-29 11:21 ` [pve-devel] [PATCH storage v3 09/10] plugin: enable import for nfs/btrfs/cifs/cephfs/glusterfs Dominik Csapak
2024-04-29 11:21 ` [pve-devel] [PATCH storage v3 10/10] add 'import' content type to 'check_volume_access' Dominik Csapak
2024-04-29 11:21 ` [pve-devel] [PATCH qemu-server v3 1/4] api: delete unused OVF.pm Dominik Csapak
2024-05-22 10:25 ` Fabian Grünbichler
2024-05-22 10:26 ` Fabian Grünbichler
2024-04-29 11:21 ` [pve-devel] [PATCH qemu-server v3 2/4] use OVF from Storage Dominik Csapak
2024-04-29 11:21 ` [pve-devel] [PATCH qemu-server v3 3/4] api: create: implement extracting disks when needed for import-from Dominik Csapak
2024-05-22 12:55 ` Fabian Grünbichler
2024-04-29 11:21 ` [pve-devel] [PATCH qemu-server v3 4/4] api: create: add 'import-extraction-storage' parameter Dominik Csapak
2024-05-22 12:16 ` Fabian Grünbichler
2024-04-29 11:21 ` [pve-devel] [PATCH manager v3 1/9] ui: fix special 'import' icon for non-esxi storages Dominik Csapak
2024-04-29 11:21 ` [pve-devel] [PATCH manager v3 2/9] ui: guest import: add ova-needs-extracting warning text Dominik Csapak
2024-04-29 11:21 ` [pve-devel] [PATCH manager v3 3/9] ui: enable import content type for relevant storages Dominik Csapak
2024-04-29 11:21 ` [pve-devel] [PATCH manager v3 4/9] ui: enable upload/download/remove buttons for 'import' type storages Dominik Csapak
2024-04-29 11:21 ` [pve-devel] [PATCH manager v3 5/9] ui: disable 'import' button for non importable formats Dominik Csapak
2024-04-29 11:21 ` [pve-devel] [PATCH manager v3 6/9] ui: import: improve rendering of volume names Dominik Csapak
2024-04-29 11:21 ` [pve-devel] [PATCH manager v3 7/9] ui: guest import: add storage selector for ova extraction storage Dominik Csapak
2024-04-29 11:21 ` [pve-devel] [PATCH manager v3 8/9] ui: guest import: change icon/text for non-esxi import storage Dominik Csapak
2024-04-29 11:21 ` [pve-devel] [PATCH manager v3 9/9] ui: import: show size for dir-based storages Dominik Csapak
2024-05-24 13:38 ` [pve-devel] [PATCH storage/qemu-server/manager v3] implement ova/ovf import for file based storages Dominik Csapak
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1716368200.rpvan7m9t9.astroid@yuna.none \
--to=f.gruenbichler@proxmox.com \
--cc=pve-devel@lists.proxmox.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox