public inbox for pve-devel@lists.proxmox.com
 help / color / mirror / Atom feed
* [pve-devel] [PATCH v7 storage] Optionally allow blockdev in abs_filesystem_path
@ 2021-03-26 12:32 Dominic Jäger
  2021-03-26 12:32 ` [pve-devel] [PATCH v7 qemu-server] Add API for import wizards Dominic Jäger
                   ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: Dominic Jäger @ 2021-03-26 12:32 UTC (permalink / raw)
  To: pve-devel

This is required to import from LVM storages, for example

Signed-off-by: Dominic Jäger <d.jaeger@proxmox.com>
---
v6->v7: Feedback Fabian G
 - Variables with _ instead of camelCase
 - single if instead of if/else

 PVE/Storage.pm | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/PVE/Storage.pm b/PVE/Storage.pm
index 18c03ec..38dffdb 100755
--- a/PVE/Storage.pm
+++ b/PVE/Storage.pm
@@ -609,22 +609,22 @@ sub path {
 }
 
 sub abs_filesystem_path {
-    my ($cfg, $volid) = @_;
+    my ($cfg, $volid, $allow_blockdev) = @_;
 
     my $path;
     if (parse_volume_id ($volid, 1)) {
 	activate_volumes($cfg, [ $volid ]);
 	$path = PVE::Storage::path($cfg, $volid);
     } else {
-	if (-f $volid) {
+	if (-f $volid || ($allow_blockdev && -b $volid)) {
 	    my $abspath = abs_path($volid);
 	    if ($abspath && $abspath =~ m|^(/.+)$|) {
 		$path = $1; # untaint any path
 	    }
 	}
     }
-
-    die "can't find file '$volid'\n" if !($path && -f $path);
+    die "can't find file '$volid'\n"
+	if !($path && (-f $path || ($allow_blockdev && -b $path)));
 
     return $path;
 }
-- 
2.20.1




^ permalink raw reply	[flat|nested] 7+ messages in thread

* [pve-devel] [PATCH v7 qemu-server] Add API for import wizards
  2021-03-26 12:32 [pve-devel] [PATCH v7 storage] Optionally allow blockdev in abs_filesystem_path Dominic Jäger
@ 2021-03-26 12:32 ` Dominic Jäger
  2021-03-31 15:12   ` Fabian Grünbichler
  2021-03-26 12:32 ` [pve-devel] [PATCH v7 manager] gui: Add import for disk & VM Dominic Jäger
  2021-04-01 13:40 ` [pve-devel] applied: [PATCH v7 storage] Optionally allow blockdev in abs_filesystem_path Thomas Lamprecht
  2 siblings, 1 reply; 7+ messages in thread
From: Dominic Jäger @ 2021-03-26 12:32 UTC (permalink / raw)
  To: pve-devel

Extend qm importdisk/importovf functionality to the API.

Signed-off-by: Dominic Jäger <d.jaeger@proxmox.com>

---
v6->v7: Feedback by Fabian G
- Introduce a regex for the import syntax <storeid>:0
- Use parameter list instead of hash for import helper
- More parsing, less string magic
- More VM config digest checking
- Create a schema format for diskimage source mapping
- Preliminarily remove some boot parameter handling
- Dare to really edit schema format subs for a cleaner solution
- Whitespace, variable names, ...

 PVE/API2/Qemu.pm       | 383 ++++++++++++++++++++++++++++++++++++++++-
 PVE/API2/Qemu/Makefile |   2 +-
 PVE/API2/Qemu/OVF.pm   |  68 ++++++++
 PVE/QemuServer.pm      |  52 +++++-
 PVE/QemuServer/OVF.pm  |  10 +-
 5 files changed, 502 insertions(+), 13 deletions(-)
 create mode 100644 PVE/API2/Qemu/OVF.pm

diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
index e95ab13..2f50f38 100644
--- a/PVE/API2/Qemu.pm
+++ b/PVE/API2/Qemu.pm
@@ -45,7 +45,6 @@ BEGIN {
     }
 }
 
-use Data::Dumper; # fixme: remove
 
 use base qw(PVE::RESTHandler);
 
@@ -62,6 +61,7 @@ my $resolve_cdrom_alias = sub {
 };
 
 my $NEW_DISK_RE = qr!^(([^/:\s]+):)?(\d+(\.\d+)?)$!;
+my $IMPORT_DISK_RE = qr!^([^/:\s]+):0$!;
 my $check_storage_access = sub {
    my ($rpcenv, $authuser, $storecfg, $vmid, $settings, $default_storage) = @_;
 
@@ -4377,4 +4377,385 @@ __PACKAGE__->register_method({
 	return PVE::QemuServer::Cloudinit::dump_cloudinit_config($conf, $param->{vmid}, $param->{type});
     }});
 
+# Raise exception if $format is not supported by $storeid
+my $check_format_is_supported = sub {
+    my ($format, $storeid, $storecfg) = @_;
+    die "storage ID parameter must be passed to the sub" if !$storeid;
+    die "storage configuration must be passed to the sub" if !$storecfg;
+
+    return if !$format;
+
+    my (undef, $valid_formats) = PVE::Storage::storage_default_format($storecfg, $storeid);
+    my $supported = grep { $_ eq $format } @$valid_formats;
+
+    die "format '$format' is not supported on storage $storeid" if !$supported;
+};
+
+# storecfg ... PVE::Storage::config()
+# vmid ... target VM ID
+# vmconf ... target VM configuration
+# source ... source image (volid or absolute path)
+# target ... hash with
+#    storeid => storage ID
+#    format => disk format (optional)
+#    options => hash with device options (may or may not contain <storeid>:0)
+#    device => device where the disk is attached (for example, scsi3) (optional)
+#
+# returns ... volid of the allocated disk image (e.g. local-lvm:vm-100-disk-2)
+my $import_disk_image = sub {
+    my ($storecfg, $vmid, $vmconf, $source, $target) = @_;
+    my $requested_format = $target->{format};
+    my $storeid = $target->{storeid};
+
+    die "Source parameter is undefined!" if !defined $source;
+    $source = PVE::Storage::abs_filesystem_path($storecfg, $source, 1);
+
+    eval { PVE::Storage::storage_config($storecfg, $storeid) };
+    die "Error while importing disk image $source: $@\n" if $@;
+
+    my $src_size = PVE::Storage::file_size_info($source);
+    die "Could not get file size of $source" if !defined($src_size);
+
+    $check_format_is_supported->($requested_format, $storeid, $storecfg);
+
+    my $dst_format = PVE::QemuServer::resolve_dst_disk_format(
+	$storecfg,
+	$storeid,
+	undef,
+	$requested_format,
+    );
+    my $dst_volid = PVE::Storage::vdisk_alloc(
+	$storecfg,
+	$storeid,
+	$vmid,
+	$dst_format,
+	undef,
+	$src_size / 102,
+    );
+
+    print "Importing disk image '$source' as '$dst_volid'...\n";
+    eval {
+	local $SIG{INT} =
+	local $SIG{TERM} =
+	local $SIG{QUIT} =
+	local $SIG{HUP} =
+	local $SIG{PIPE} = sub { die "Interrupted by signal $!\n"; };
+
+	my $zeroinit = PVE::Storage::volume_has_feature(
+	    $storecfg,
+	    'sparseinit',
+	    $dst_volid,
+	);
+	PVE::Storage::activate_volumes($storecfg, [$dst_volid]);
+	PVE::QemuServer::qemu_img_convert(
+	    $source,
+	    $dst_volid,
+	    $src_size,
+	    undef,
+	    $zeroinit,
+	);
+	PVE::Storage::deactivate_volumes($storecfg, [$dst_volid]);
+
+    };
+    if (my $err = $@) {
+	eval { PVE::Storage::vdisk_free($storecfg, $dst_volid) };
+	warn "Cleanup of $dst_volid failed: $@ \n" if $@;
+
+	die "Importing disk '$source' failed: $err\n" if $err;
+    }
+
+    $target->{options}->{file} = $dst_volid;
+    my $options_string = PVE::QemuServer::print_drive($target->{options});
+    $target->{device} = PVE::QemuConfig->add_unused_volume($vmconf, $dst_volid)
+	if !$target->{device};
+
+    $update_vm_api->(
+	{
+	    vmid => $vmid,
+	    $target->{device} => $options_string,
+	    skiplock => 1,
+	    digest => $vmconf->{digest},
+	},
+	1,
+    );
+
+    return $dst_volid;
+};
+
+__PACKAGE__->register_method ({
+    name => 'importdisk',
+    path => '{vmid}/importdisk',
+    method => 'POST',
+    proxyto => 'node',
+    protected => 1,
+    description => "Import an external disk image into a VM. The image format ".
+	"has to be supported by qemu-img.",
+    parameters => {
+	additionalProperties => 0,
+	properties => {
+	    node => get_standard_option('pve-node'),
+	    vmid => get_standard_option('pve-vmid',
+		{completion => \&PVE::QemuServer::complete_vmid}),
+	    source => {
+		description => "Disk image to import. Can be a volid ".
+		    "(local:99/imageToImport.raw) or an absolute path on the server.",
+		type => 'string',
+	    },
+	    device => {
+		type => 'string',
+		description => "Bus/Device type of the new disk (e.g. 'ide0', ".
+		    "'scsi2'). Will add the image as unused disk if omitted.",
+		enum => [PVE::QemuServer::Drive::valid_drive_names()],
+		optional => 1,
+	    },
+	    device_options => {
+		type => 'string',
+		description => "Options to set for the new disk (e.g. 'discard=on,backup=0')",
+		optional => 1,
+		requires => 'device',
+	    },
+	    storage => get_standard_option('pve-storage-id', {
+		description => "The storage to which the image will be imported to.",
+		completion => \&PVE::QemuServer::complete_storage,
+	    }),
+	    format => {
+		type => 'string',
+		description => 'Target format.',
+		enum => [ 'raw', 'qcow2', 'vmdk' ],
+		optional => 1,
+	    },
+	    digest => get_standard_option('pve-config-digest'),
+	},
+    },
+    returns => { type => 'string'},
+    code => sub {
+	my ($param) = @_;
+	my $vmid = extract_param($param, 'vmid');
+	my $node = extract_param($param, 'node');
+	my $source = extract_param($param, 'source');
+	my $digest = extract_param($param, 'digest');
+	my $device_options = extract_param($param, 'device_options');
+	my $device = extract_param($param, 'device');
+	my $storeid = extract_param($param, 'storage');
+
+	my $rpcenv = PVE::RPCEnvironment::get();
+	my $authuser = $rpcenv->get_user();
+	my $storecfg = PVE::Storage::config();
+	PVE::Storage::storage_config($storecfg, $storeid);
+
+
+	if ($device_options) {
+	    # $device_options may or may not contain <storeid>:0
+	    my $parsed = PVE::QemuServer::Drive::parse_drive($device, $device_options);
+	    if ($parsed) {
+		raise_param_exc({$device_options => "Invalid import syntax"})
+		    if !($parsed->{file} =~ $IMPORT_DISK_RE);
+	    } else {
+		my $fake = "$storeid:0,$device_options";
+		$parsed = PVE::QemuServer::Drive::parse_drive($device, $fake);
+	    }
+	    delete $parsed->{file};
+	    delete $parsed->{interface};
+	    delete $parsed->{index};
+	    $device_options = $parsed;
+	}
+
+	# Format can be set explicitly "--format vmdk"
+	# or as part of device options "--device_options discard=on,format=vmdk"
+	my $format = extract_param($param, 'format');
+	if ($device_options) {
+	    raise_param_exc({format => "Format already specified in device_options!"})
+		if $format && $device_options->{format};
+	    $format = $format || $device_options->{format}; # may be undefined
+	}
+	$check_format_is_supported->($format, $storeid, $storecfg);
+
+	# quick checks before fork + lock
+	my $conf = PVE::QemuConfig->load_config($vmid);
+	PVE::QemuConfig->check_lock($conf);
+	PVE::Tools::assert_if_modified($conf->{digest}, $digest);
+	if ($device && $conf->{$device}) {
+	    die "Could not import because device $device is already in ".
+		"use in VM $vmid. Choose a different device!";
+	}
+
+	my $worker = sub {
+	    PVE::QemuConfig->lock_config($vmid, sub {
+		$conf = PVE::QemuConfig->load_config($vmid);
+		PVE::QemuConfig->check_lock($conf);
+
+		PVE::Tools::assert_if_modified($conf->{digest}, $digest);
+		PVE::QemuConfig->set_lock($vmid, 'import');
+		$conf = PVE::QemuConfig->load_config($vmid);
+	    });
+
+	    my $target = {
+		node => $node,
+		storeid => $storeid,
+	    };
+	    $target->{format} = $format;
+	    $target->{device} = $device;
+	    $target->{options} = $device_options;
+	    eval { $import_disk_image->($storecfg, $vmid, $conf, $source, $target) };
+	    my $err = $@;
+	    eval { PVE::QemuConfig->remove_lock($vmid, 'import') };
+	    warn $@ if $@;
+	    die $err if $err;
+	};
+	return $rpcenv->fork_worker('importdisk', $vmid, $authuser, $worker);
+    }});
+
+__PACKAGE__->register_method({
+    name => 'importvm',
+    path => '{vmid}/importvm',
+    method => 'POST',
+    description => "Import a VM from existing disk images.",
+    protected => 1,
+    proxyto => 'node',
+    parameters => {
+	additionalProperties => 0,
+	properties => PVE::QemuServer::json_config_properties(
+	    {
+		node => get_standard_option('pve-node'),
+		vmid => get_standard_option('pve-vmid', { completion =>
+		    \&PVE::Cluster::complete_next_vmid }),
+		diskimage => {
+		    description => "\\0 delimited mapping of devices to disk images. For " .
+			"example, scsi0=/mnt/nfs/image1.vmdk",
+		    type => 'string',
+		    format => 'device-image-pair-alist',
+		},
+		start => {
+		    optional => 1,
+		    type => 'boolean',
+		    default => 0,
+		    description => "Start VM after it was imported successfully.",
+		},
+	    }),
+    },
+    returns => {
+	type => 'string',
+    },
+    code => sub {
+	my ($param) = @_;
+	my $node = extract_param($param, 'node');
+	my $vmid = extract_param($param, 'vmid');
+	my $diskimages_string = extract_param($param, 'diskimage');
+	my $boot = extract_param($param, 'boot');
+	my $start = extract_param($param, 'start');
+
+	my $rpcenv = PVE::RPCEnvironment::get();
+	my $authuser = $rpcenv->get_user();
+	my $storecfg = PVE::Storage::config();
+
+	PVE::Cluster::check_cfs_quorum();
+
+	my $import_param = {};
+	foreach my $opt (keys %$param) {
+	    next if $opt eq 'efidisk0';
+	    raise_param_exc({bootdisk => "Deprecated: Use --boot order= instead"})
+		if $opt eq 'bootdisk';
+
+	    if (PVE::QemuServer::Drive::is_valid_drivename($opt)) {
+		my $drive = PVE::QemuServer::parse_drive($opt, $param->{$opt});
+		if ($drive->{file} =~ $IMPORT_DISK_RE) {
+		    $import_param->{$opt} = $drive;
+		    delete $param->{$opt};
+		}
+	    }
+	}
+
+	my $diskimages = {};
+	foreach my $pair (PVE::Tools::split_list($diskimages_string)) {
+	    my ($device, $diskimage) = split('=', $pair);
+	    $diskimages->{$device} = $diskimage;
+	    raise_param_exc({
+		$device => "Device '$device' not marked for import, " .
+		    "but import source '$diskimage' specified",
+	    }) if !defined($import_param->{$device});
+	    PVE::Storage::abs_filesystem_path($storecfg, $diskimage, 1);
+	}
+
+	foreach my $device (keys %$import_param) {
+	    raise_param_exc({
+		$device => "Device '$device' marked for import, but no source given\n",
+	    }) if !defined($diskimages->{$device});
+	}
+
+	eval { PVE::QemuConfig->create_and_lock_config($vmid, 0, 'import') };
+	die "Unable to create config for VM import: $@" if $@;
+
+	my $worker = sub {
+	    my $reload_conf = sub {
+		my ($vmid) = @_;
+		my $conf = PVE::QemuConfig->load_config($vmid);
+		return $conf if PVE::QemuConfig->has_lock($conf, 'import');
+		die "import lock in VM $vmid config file missing!";
+	    };
+
+	    my $conf = $reload_conf->($vmid);
+	    $update_vm_api->(
+		{
+		    %$param,
+		    node => $node,
+		    vmid => $vmid,
+		    skiplock => 1,
+		    digest => $conf->{digest},
+		},
+		1
+	    );
+
+	    eval {
+		foreach my $device (keys %$import_param) {
+		    $conf = $reload_conf->($vmid);
+		    my $drive = $import_param->{$device};
+		    my $storeid = PVE::Storage::parse_volume_id($drive->{file});
+		    my $imported = $import_disk_image->(
+			$storecfg,
+			$vmid,
+			$conf,
+			$diskimages->{$device},
+			{
+			    storeid => $storeid,
+			    format => $drive->{format},
+			    options => $drive,
+			    device => $device,
+			},
+		    );
+		}
+	    };
+	    my $err = $@;
+	    if ($err) {
+		eval { PVE::QemuServer::destroy_vm($storecfg, $vmid, 1) };
+		warn "Could not destroy VM $vmid: $@" if "$@";
+
+		die "Import failed: $err";
+	    }
+
+	    $conf = $reload_conf->($vmid);
+	    if (!$boot) {
+		my $bootdevs = PVE::QemuServer::get_default_bootdevices($conf);
+		$boot = PVE::QemuServer::print_bootorder($bootdevs);
+	    }
+	    $update_vm_api->(
+		{
+		    node => $node,
+		    vmid => $vmid,
+		    boot => $boot,
+		    skiplock => 1,
+		    digest => $conf->{digest},
+		},
+		1,
+	    );
+
+	    eval { PVE::QemuConfig->remove_lock($vmid, 'import') };
+	    warn $@ if $@;
+
+	    PVE::QemuServer::vm_start($storecfg, $vmid) if $start;
+	};
+
+	return $rpcenv->fork_worker('importvm', $vmid, $authuser, $worker);
+    }});
+
+
 1;
diff --git a/PVE/API2/Qemu/Makefile b/PVE/API2/Qemu/Makefile
index 5d4abda..bdd4762 100644
--- a/PVE/API2/Qemu/Makefile
+++ b/PVE/API2/Qemu/Makefile
@@ -1,4 +1,4 @@
-SOURCES=Agent.pm CPU.pm Machine.pm
+SOURCES=Agent.pm CPU.pm Machine.pm OVF.pm
 
 .PHONY: install
 install:
diff --git a/PVE/API2/Qemu/OVF.pm b/PVE/API2/Qemu/OVF.pm
new file mode 100644
index 0000000..bd6e90b
--- /dev/null
+++ b/PVE/API2/Qemu/OVF.pm
@@ -0,0 +1,68 @@
+package PVE::API2::Qemu::OVF;
+
+use strict;
+use warnings;
+
+use PVE::RESTHandler;
+use PVE::JSONSchema qw(get_standard_option);
+use PVE::QemuServer::OVF;
+
+use base qw(PVE::RESTHandler);
+
+__PACKAGE__->register_method ({
+    name => 'index',
+    path => '',
+    method => 'GET',
+    proxyto => 'node',
+    description => "Read an .ovf manifest.",
+    parameters => {
+	additionalProperties => 0,
+	properties => {
+	    node => get_standard_option('pve-node'),
+	    manifest => {
+		description => ".ovf manifest",
+		type => 'string',
+	    },
+	},
+    },
+    returns => {
+	description => "VM config according to .ovf manifest and digest of manifest",
+	type => "object",
+    },
+    returns => {
+	type => 'object',
+	additionalProperties => 1,
+	properties => PVE::QemuServer::json_ovf_properties({
+	    name => {
+		type => 'string',
+		optional => 1,
+	    },
+	    cores => {
+		type => 'integer',
+		optional => 1,
+	    },
+	    memory => {
+		type => 'integer',
+		optional => 1,
+	    },
+	}),
+    },
+    code => sub {
+	my ($param) = @_;
+
+	my $manifest = $param->{manifest};
+	die "$manifest: non-existent or non-regular file\n" if (! -f $manifest);
+
+	my $parsed = PVE::QemuServer::OVF::parse_ovf($manifest, 0, 1);
+	my $result;
+	$result->{cores} = $parsed->{qm}->{cores};
+	$result->{name} =  $parsed->{qm}->{name};
+	$result->{memory} = $parsed->{qm}->{memory};
+	my $disks = $parsed->{disks};
+	foreach my $disk (@$disks) {
+	    $result->{$disk->{disk_address}} = $disk->{backing_file};
+	}
+	return $result;
+    }});
+
+1;
\ No newline at end of file
diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
index 1c0b5c2..131c0b6 100644
--- a/PVE/QemuServer.pm
+++ b/PVE/QemuServer.pm
@@ -300,7 +300,7 @@ my $confdesc = {
 	optional => 1,
 	type => 'string',
 	description => "Lock/unlock the VM.",
-	enum => [qw(backup clone create migrate rollback snapshot snapshot-delete suspending suspended)],
+	enum => [qw(backup clone create migrate rollback snapshot snapshot-delete suspending suspended import)],
     },
     cpulimit => {
 	optional => 1,
@@ -985,19 +985,41 @@ PVE::JSONSchema::register_format('pve-volume-id-or-qm-path', \&verify_volume_id_
 sub verify_volume_id_or_qm_path {
     my ($volid, $noerr) = @_;
 
-    if ($volid eq 'none' || $volid eq 'cdrom' || $volid =~ m|^/|) {
-	return $volid;
-    }
+    return $volid eq 'none' || $volid eq 'cdrom' ?
+	$volid :
+	verify_volume_id_or_absolute_path($volid, $noerr);
+}
+
+PVE::JSONSchema::register_format('pve-volume-id-or-absolute-path', \&verify_volume_id_or_absolute_path);
+sub verify_volume_id_or_absolute_path {
+    my ($volid, $noerr) = @_;
+
+    return $volid if $volid =~ m|^/|;
 
-    # if its neither 'none' nor 'cdrom' nor a path, check if its a volume-id
     $volid = eval { PVE::JSONSchema::check_format('pve-volume-id', $volid, '') };
     if ($@) {
-	return if $noerr;
+	return undef if $noerr;
 	die $@;
     }
     return $volid;
 }
 
+PVE::JSONSchema::register_format('device-image-pair', \&verify_device_image_pair);
+sub verify_device_image_pair {
+    my ($pair, $noerr) = @_;
+
+    my $error = sub {
+	return undef if $noerr;
+	die $@;
+    };
+
+    my ($device, $image) = split('=', $pair);
+    $error->("Invalid device '$device'") if !PVE::QemuServer::Drive::is_valid_drivename($device);
+    $error->("Invalid image '$image'") if !verify_volume_id_or_absolute_path($image);
+
+    return $pair;
+}
+
 my $usb_fmt = {
     host => {
 	default_key => 1,
@@ -2030,6 +2052,22 @@ sub json_config_properties {
     return $prop;
 }
 
+# Properties that we can read from an OVF file
+sub json_ovf_properties {
+    my $prop = shift;
+
+    foreach my $device ( PVE::QemuServer::Drive::valid_drive_names()) {
+	$prop->{$device} = {
+	    type => 'string',
+	    format => 'pve-volume-id-or-absolute-path',
+	    description => "Disk image that gets imported to $device",
+	    optional => 1,
+	};
+    }
+
+    return $prop;
+}
+
 # return copy of $confdesc_cloudinit to generate documentation
 sub cloudinit_config_properties {
 
@@ -6748,7 +6786,7 @@ sub qemu_img_convert {
 	$src_path = PVE::Storage::path($storecfg, $src_volid, $snapname);
 	$src_is_iscsi = ($src_path =~ m|^iscsi://|);
 	$cachemode = 'none' if $src_scfg->{type} eq 'zfspool';
-    } elsif (-f $src_volid) {
+    } elsif (-f $src_volid || -b $src_volid) {
 	$src_path = $src_volid;
 	if ($src_path =~ m/\.($PVE::QemuServer::Drive::QEMU_FORMAT_RE)$/) {
 	    $src_format = $1;
diff --git a/PVE/QemuServer/OVF.pm b/PVE/QemuServer/OVF.pm
index c76c199..48146e9 100644
--- a/PVE/QemuServer/OVF.pm
+++ b/PVE/QemuServer/OVF.pm
@@ -87,7 +87,7 @@ sub id_to_pve {
 
 # returns two references, $qm which holds qm.conf style key/values, and \@disks
 sub parse_ovf {
-    my ($ovf, $debug) = @_;
+    my ($ovf, $debug, $manifest_only) = @_;
 
     my $dom = XML::LibXML->load_xml(location => $ovf, no_blanks => 1);
 
@@ -220,9 +220,11 @@ ovf:Item[rasd:InstanceID='%s']/rasd:ResourceType", $controller_id);
 	    die "error parsing $filepath, file seems not to exist at $backing_file_path\n";
 	}
 
-	my $virtual_size;
-	if ( !($virtual_size = PVE::Storage::file_size_info($backing_file_path)) ) {
-	    die "error parsing $backing_file_path, size seems to be $virtual_size\n";
+	my $virtual_size = undef;
+	if (!$manifest_only) { # Not possible if manifest is uploaded in web gui
+	    if ( !($virtual_size = PVE::Storage::file_size_info($backing_file_path)) ) {
+		die "error parsing $backing_file_path: Could not get file size info: $@\n";
+	    }
 	}
 
 	$pve_disk = {
-- 
2.20.1




^ permalink raw reply	[flat|nested] 7+ messages in thread

* [pve-devel] [PATCH v7 manager] gui: Add import for disk & VM
  2021-03-26 12:32 [pve-devel] [PATCH v7 storage] Optionally allow blockdev in abs_filesystem_path Dominic Jäger
  2021-03-26 12:32 ` [pve-devel] [PATCH v7 qemu-server] Add API for import wizards Dominic Jäger
@ 2021-03-26 12:32 ` Dominic Jäger
  2021-04-01 13:40 ` [pve-devel] applied: [PATCH v7 storage] Optionally allow blockdev in abs_filesystem_path Thomas Lamprecht
  2 siblings, 0 replies; 7+ messages in thread
From: Dominic Jäger @ 2021-03-26 12:32 UTC (permalink / raw)
  To: pve-devel

Add GUI wizard to import whole VMs and a window to import single disks in
Hardware View.

Signed-off-by: Dominic Jäger <d.jaeger@proxmox.com>

---
v6->v7:
- Update to API changes
- Add helpers to Utils
- Whitespace & line breaks according to style guide
- Making conditional branches in HDEdit easier to read

 PVE/API2/Nodes.pm                       |   7 +
 www/manager6/Makefile                   |   2 +
 www/manager6/Utils.js                   |  12 +
 www/manager6/Workspace.js               |  15 ++
 www/manager6/form/ControllerSelector.js |  15 ++
 www/manager6/node/CmdMenu.js            |  13 +
 www/manager6/qemu/HDEdit.js             | 169 +++++++++++-
 www/manager6/qemu/HardwareView.js       |  25 ++
 www/manager6/qemu/ImportWizard.js       | 332 ++++++++++++++++++++++++
 www/manager6/qemu/MultiHDEdit.js        | 277 ++++++++++++++++++++
 www/manager6/window/Wizard.js           |   2 +
 11 files changed, 856 insertions(+), 13 deletions(-)
 create mode 100644 www/manager6/qemu/ImportWizard.js
 create mode 100644 www/manager6/qemu/MultiHDEdit.js

diff --git a/PVE/API2/Nodes.pm b/PVE/API2/Nodes.pm
index ba6621c6..1cee6cb5 100644
--- a/PVE/API2/Nodes.pm
+++ b/PVE/API2/Nodes.pm
@@ -48,6 +48,7 @@ use PVE::API2::LXC;
 use PVE::API2::Network;
 use PVE::API2::NodeConfig;
 use PVE::API2::Qemu::CPU;
+use PVE::API2::Qemu::OVF;
 use PVE::API2::Qemu;
 use PVE::API2::Replication;
 use PVE::API2::Services;
@@ -76,6 +77,11 @@ __PACKAGE__->register_method ({
     path => 'cpu',
 });
 
+__PACKAGE__->register_method ({
+    subclass => "PVE::API2::Qemu::OVF",
+    path => 'readovf',
+});
+
 __PACKAGE__->register_method ({
     subclass => "PVE::API2::LXC",
     path => 'lxc',
@@ -2183,6 +2189,7 @@ __PACKAGE__->register_method ({
 	return undef;
     }});
 
+
 # bash completion helper
 
 sub complete_templet_repo {
diff --git a/www/manager6/Makefile b/www/manager6/Makefile
index a2f7be6d..753cd1c0 100644
--- a/www/manager6/Makefile
+++ b/www/manager6/Makefile
@@ -196,8 +196,10 @@ JSSRC= 							\
 	qemu/CmdMenu.js					\
 	qemu/Config.js					\
 	qemu/CreateWizard.js				\
+	qemu/ImportWizard.js				\
 	qemu/DisplayEdit.js				\
 	qemu/HDEdit.js					\
+	qemu/MultiHDEdit.js					\
 	qemu/HDEfi.js					\
 	qemu/HDMove.js					\
 	qemu/HDResize.js				\
diff --git a/www/manager6/Utils.js b/www/manager6/Utils.js
index f502950f..dbfd65ce 100644
--- a/www/manager6/Utils.js
+++ b/www/manager6/Utils.js
@@ -1708,6 +1708,16 @@ Ext.define('PVE.Utils', {
 	});
     },
 
+    // collection ... collection of strings of a subset of the descendants of container
+    // visible ... true to show and enable, false to hide and disable
+    setDescendantsVisible: function(container, collection, visible = 1) {
+	const hide = (element, value) => {
+	    element.setHidden(value);
+	    element.setDisabled(value);
+	};
+	collection.map(e => container.down(e)).forEach(e => hide(e, !visible));
+    },
+
     cpu_vendor_map: {
 	'default': 'QEMU',
 	'AuthenticAMD': 'AMD',
@@ -1787,6 +1797,8 @@ Ext.define('PVE.Utils', {
 	    hastop: ['HA', gettext('Stop')],
 	    imgcopy: ['', gettext('Copy data')],
 	    imgdel: ['', gettext('Erase data')],
+	    importdisk: ['VM', gettext('Import disk')],
+	    importvm: ['VM', gettext('Import VM')],
 	    lvmcreate: [gettext('LVM Storage'), gettext('Create')],
 	    lvmthincreate: [gettext('LVM-Thin Storage'), gettext('Create')],
 	    migrateall: ['', gettext('Migrate all VMs and Containers')],
diff --git a/www/manager6/Workspace.js b/www/manager6/Workspace.js
index 0c1b9e0c..631739a0 100644
--- a/www/manager6/Workspace.js
+++ b/www/manager6/Workspace.js
@@ -280,11 +280,25 @@ Ext.define('PVE.StdWorkspace', {
 	    },
 	});
 
+	var importVM = Ext.createWidget('button', {
+	    pack: 'end',
+	    margin: '3 5 0 0',
+	    baseCls: 'x-btn',
+	    iconCls: 'fa fa-desktop',
+	    text: gettext("Import VM"),
+	    hidden: Proxmox.UserName !== 'root@pam',
+	    handler: function() {
+		var wiz = Ext.create('PVE.qemu.ImportWizard', {});
+		wiz.show();
+	    },
+	});
+
 	sprovider.on('statechange', function(sp, key, value) {
 	    if (key === 'GuiCap' && value) {
 		caps = value;
 		createVM.setDisabled(!caps.vms['VM.Allocate']);
 		createCT.setDisabled(!caps.vms['VM.Allocate']);
+		importVM.setDisabled(!caps.vms['VM.Allocate']);
 	    }
 	});
 
@@ -332,6 +346,7 @@ Ext.define('PVE.StdWorkspace', {
 			},
 			createVM,
 			createCT,
+			importVM,
 			{
 			    pack: 'end',
 			    margin: '0 5 0 0',
diff --git a/www/manager6/form/ControllerSelector.js b/www/manager6/form/ControllerSelector.js
index 23c61159..8e9aee98 100644
--- a/www/manager6/form/ControllerSelector.js
+++ b/www/manager6/form/ControllerSelector.js
@@ -68,6 +68,21 @@ clist_loop:
 	deviceid.validate();
     },
 
+    getValues: function() {
+	return this.query('field').map(x => x.getValue());
+    },
+
+    getValuesAsString: function() {
+	return this.getValues().join('');
+    },
+
+    setValue: function(value) {
+	let regex = /([a-z]+)(\d+)/;
+	let [_, controller, deviceid] = regex.exec(value);
+	this.down('field[name=controller]').setValue(controller);
+	this.down('field[name=deviceid]').setValue(deviceid);
+    },
+
     initComponent: function() {
 	var me = this;
 
diff --git a/www/manager6/node/CmdMenu.js b/www/manager6/node/CmdMenu.js
index b650bfa0..407cf2d0 100644
--- a/www/manager6/node/CmdMenu.js
+++ b/www/manager6/node/CmdMenu.js
@@ -29,6 +29,19 @@ Ext.define('PVE.node.CmdMenu', {
 		wiz.show();
 	    },
 	},
+	{
+	    text: gettext("Import VM"),
+	    hidden: Proxmox.UserName !== 'root@pam',
+	    itemId: 'importvm',
+	    iconCls: 'fa fa-cube',
+	    handler: function() {
+		var me = this.up('menu');
+		var wiz = Ext.create('PVE.qemu.ImportWizard', {
+		    nodename: me.nodename,
+		});
+		wiz.show();
+	    },
+	},
 	{ xtype: 'menuseparator' },
 	{
 	    text: gettext('Bulk Start'),
diff --git a/www/manager6/qemu/HDEdit.js b/www/manager6/qemu/HDEdit.js
index e22111bf..a2f6c95a 100644
--- a/www/manager6/qemu/HDEdit.js
+++ b/www/manager6/qemu/HDEdit.js
@@ -58,6 +58,21 @@ Ext.define('PVE.qemu.HDInputPanel', {
 	},
     },
 
+    isImport: function() {
+	return this.isImportVM || this.isImportDisk;
+    },
+
+    /*
+    All radiofields (esp. sourceRadioPath and sourceRadioStorage) have the
+    same scope for name. But we need a different scope for each HDInputPanel in
+    a MultiHDInputPanel to get the selection for each HDInputPanel => Make
+    names so that those within one HDInputPanel are equal, but different from other
+    HDInputPanels
+    */
+    getSourceTypeID() {
+	return 'sourceType_' + this.id;
+    },
+
     onGetValues: function(values) {
 	var me = this;
 
@@ -70,6 +85,8 @@ Ext.define('PVE.qemu.HDInputPanel', {
 	} else if (me.isCreate) {
 	    if (values.hdimage) {
 		me.drive.file = values.hdimage;
+	    } else if (me.isImport()) {
+		me.drive.file = `${values.hdstorage}:0`;
 	    } else {
 		me.drive.file = values.hdstorage + ":" + values.disksize;
 	    }
@@ -83,15 +100,33 @@ Ext.define('PVE.qemu.HDInputPanel', {
 	PVE.Utils.propertyStringSet(me.drive, values.iothread, 'iothread', 'on');
 	PVE.Utils.propertyStringSet(me.drive, values.cache, 'cache');
 
-        var names = ['mbps_rd', 'mbps_wr', 'iops_rd', 'iops_wr'];
-        Ext.Array.each(names, function(name) {
-            var burst_name = name + '_max';
+	var names = ['mbps_rd', 'mbps_wr', 'iops_rd', 'iops_wr'];
+	Ext.Array.each(names, function(name) {
+	    var burst_name = name + '_max';
 	    PVE.Utils.propertyStringSet(me.drive, values[name], name);
 	    PVE.Utils.propertyStringSet(me.drive, values[burst_name], burst_name);
-        });
+	});
+
+	const getSourceImageLocation = function() {
+	    const type = values[me.getSourceTypeID()];
+	    return type === 'storage' ? values.sourceVolid : values.sourcePath;
+	};
 
+	if (me.isImportVM) {
+	    params.diskimage = `${confid}=${getSourceImageLocation()}`;
+	}
+
+	const options = PVE.Parser.printQemuDrive(me.drive);
 
-	params[confid] = PVE.Parser.printQemuDrive(me.drive);
+	if (me.isImportDisk) {
+	    params.device = confid;
+	    params.device_options = options;
+	    params.source = getSourceImageLocation();
+	    params.device = values.controller + values.deviceid;
+	    params.storage = values.hdstorage;
+	} else {
+	    params[confid] = options;
+	}
 
 	return params;
     },
@@ -149,6 +184,10 @@ Ext.define('PVE.qemu.HDInputPanel', {
 	me.setValues(values);
     },
 
+    getDevice: function() {
+	    return this.bussel.getValuesAsString();
+    },
+
     setNodename: function(nodename) {
 	var me = this;
 	me.down('#hdstorage').setNodename(nodename);
@@ -169,10 +208,15 @@ Ext.define('PVE.qemu.HDInputPanel', {
 	me.advancedColumn2 = [];
 
 	if (!me.confid || me.unused) {
+	    let controllerColumn = me.isImport() ? me.column2 : me.column1;
 	    me.bussel = Ext.create('PVE.form.ControllerSelector', {
+		itemId: 'bussel',
 		vmconfig: me.insideWizard ? { ide2: 'cdrom' } : {},
 	    });
-	    me.column1.push(me.bussel);
+	    if (me.isImport()) {
+		me.bussel.fieldLabel = 'Target Device';
+	    }
+	    controllerColumn.push(me.bussel);
 
 	    me.scsiController = Ext.create('Ext.form.field.Display', {
 		fieldLabel: gettext('SCSI Controller'),
@@ -184,7 +228,7 @@ Ext.define('PVE.qemu.HDInputPanel', {
 		submitValue: false,
 		hidden: true,
 	    });
-	    me.column1.push(me.scsiController);
+	    controllerColumn.push(me.scsiController);
 	}
 
 	if (me.unused) {
@@ -199,14 +243,21 @@ Ext.define('PVE.qemu.HDInputPanel', {
 		allowBlank: false,
 	    });
 	    me.column1.push(me.unusedDisks);
-	} else if (me.isCreate) {
-	    me.column1.push({
+	} else if (me.isCreate || me.isImport()) {
+	    let selector = {
 		xtype: 'pveDiskStorageSelector',
 		storageContent: 'images',
 		name: 'disk',
 		nodename: me.nodename,
-		autoSelect: me.insideWizard,
-	    });
+		hideSize: me.isImport(),
+		autoSelect: me.insideWizard || me.isImport(),
+	    };
+	    if (me.isImport()) {
+		selector.storageLabel = gettext('Target storage');
+		me.column2.push(selector);
+	    } else {
+		me.column1.push(selector);
+	    }
 	} else {
 	    me.column1.push({
 		xtype: 'textfield',
@@ -217,6 +268,12 @@ Ext.define('PVE.qemu.HDInputPanel', {
 	    });
 	}
 
+	if (me.isImport()) {
+	    me.column2.push({
+		xtype: 'box',
+		autoEl: { tag: 'hr' },
+	    });
+	}
 	me.column2.push(
 	    {
 		xtype: 'CacheTypeSelector',
@@ -231,6 +288,84 @@ Ext.define('PVE.qemu.HDInputPanel', {
 		name: 'discard',
 	    },
 	);
+	if (me.isImport()) {
+	    me.column1.unshift(
+		{
+		    xtype: 'radiofield',
+		    itemId: 'sourceRadioStorage',
+		    name: me.getSourceTypeID(),
+		    inputValue: 'storage',
+		    boxLabel: gettext('Use a storage as source'),
+		    hidden: Proxmox.UserName !== 'root@pam',
+		    checked: true,
+		    listeners: {
+			change: (_, newValue) => {
+			    const selectors = [
+				'#sourceStorageSelector',
+				'#sourceFileSelector',
+			    ];
+			    PVE.Utils.setDescendantsVisible(me, selectors, newValue);
+			},
+		    },
+		}, {
+		    xtype: 'pveStorageSelector',
+		    itemId: 'sourceStorageSelector',
+		    name: 'inputImageStorage',
+		    nodename: me.nodename,
+		    fieldLabel: gettext('Source Storage'),
+		    storageContent: 'images',
+		    autoSelect: me.insideWizard,
+		    hidden: true,
+		    disabled: true,
+		    listeners: {
+			change: function(_, selectedStorage) {
+			    me.down('#sourceFileSelector').setStorage(selectedStorage);
+			},
+		    },
+		}, {
+		    xtype: 'pveFileSelector',
+		    itemId: 'sourceFileSelector',
+		    name: 'sourceVolid',
+		    nodename: me.nodename,
+		    storageContent: 'images',
+		    hidden: true,
+		    disabled: true,
+		    fieldLabel: gettext('Source Image'),
+		    autoEl: {
+			tag: 'div',
+			'data-qtip': gettext("Place your source images into a new folder <storageRoot>/images/<newVMID>, for example /var/lib/vz/images/999"),
+		    },
+		}, {
+		    xtype: 'radiofield',
+		    itemId: 'sourceRadioPath',
+		    name: me.getSourceTypeID(),
+		    inputValue: 'path',
+		    boxLabel: gettext('Use an absolute path as source'),
+		    hidden: Proxmox.UserName !== 'root@pam',
+		    listeners: {
+			change: (_, newValue) => {
+			    PVE.Utils.setDescendantsVisible(me, ['#sourcePathTextfield'], newValue);
+			},
+		    },
+		}, {
+		    xtype: 'textfield',
+		    itemId: 'sourcePathTextfield',
+		    fieldLabel: gettext('Source Path'),
+		    name: 'sourcePath',
+		    autoEl: {
+			tag: 'div',
+			'data-qtip': gettext('Absolute path to the source disk image, for example: /home/user/somedisk.qcow2'),
+		    },
+		    hidden: true,
+		    disabled: true,
+		    validator: function(insertedText) {
+			return insertedText.startsWith('/') ||
+			    insertedText.startsWith('http') ||
+			    gettext('Must be an absolute path or URL');
+		    },
+		},
+	    );
+	}
 
 	me.advancedColumn1.push(
 	    {
@@ -373,13 +508,18 @@ Ext.define('PVE.qemu.HDEdit', {
 	    nodename: nodename,
 	    unused: unused,
 	    isCreate: me.isCreate,
+	    isImportVM: me.isImportVM,
+	    isImportDisk: me.isImportDisk,
 	});
 
-	var subject;
 	if (unused) {
 	    me.subject = gettext('Unused Disk');
+	} else if (me.isImportDisk) {
+	    me.subject = gettext('Import Disk');
+	    me.submitText = 'Import';
+	    me.backgroundDelay = undefined;
 	} else if (me.isCreate) {
-            me.subject = gettext('Hard Disk');
+	    me.subject = gettext('Hard Disk');
 	} else {
            me.subject = gettext('Hard Disk') + ' (' + me.confid + ')';
 	}
@@ -404,6 +544,9 @@ Ext.define('PVE.qemu.HDEdit', {
 		    ipanel.setDrive(drive);
 		    me.isValid(); // trigger validation
 		}
+		if (me.isImportDisk) {
+		    me.url = me.url.replace(/\/config$/, "/importdisk");
+		}
 	    },
 	});
     },
diff --git a/www/manager6/qemu/HardwareView.js b/www/manager6/qemu/HardwareView.js
index 98352e3f..4fbf0e5e 100644
--- a/www/manager6/qemu/HardwareView.js
+++ b/www/manager6/qemu/HardwareView.js
@@ -431,6 +431,30 @@ Ext.define('PVE.qemu.HardwareView', {
 	    handler: run_move,
 	});
 
+	var import_btn = new Proxmox.button.Button({
+	    text: gettext('Import disk'),
+	    hidden: Proxmox.UserName !== 'root@pam',
+	    handler: function() {
+		var win = Ext.create('PVE.qemu.HDEdit', {
+		    method: 'POST',
+		    url: `/api2/extjs/${baseurl}`,
+		    pveSelNode: me.pveSelNode,
+		    isImportDisk: true,
+		    listeners: {
+			add: function(_, component) {
+			    const selectors = [
+				'#sourceStorageSelector',
+				'#sourceFileSelector',
+			    ];
+			    PVE.Utils.setDescendantsVisible(component, selectors);
+			},
+		    },
+		});
+		win.on('destroy', me.reload, me);
+		win.show();
+	    },
+	});
+
 	var remove_btn = new Proxmox.button.Button({
 	    text: gettext('Remove'),
 	    defaultText: gettext('Remove'),
@@ -759,6 +783,7 @@ Ext.define('PVE.qemu.HardwareView', {
 		edit_btn,
 		resize_btn,
 		move_btn,
+		import_btn,
 		revert_btn,
 	    ],
 	    rows: rows,
diff --git a/www/manager6/qemu/ImportWizard.js b/www/manager6/qemu/ImportWizard.js
new file mode 100644
index 00000000..0066adc4
--- /dev/null
+++ b/www/manager6/qemu/ImportWizard.js
@@ -0,0 +1,332 @@
+/*jslint confusion: true*/
+Ext.define('PVE.qemu.ImportWizard', {
+    extend: 'PVE.window.Wizard',
+    alias: 'widget.pveQemuImportWizard',
+    mixins: ['Proxmox.Mixin.CBind'],
+
+    viewModel: {
+	data: {
+	    nodename: '',
+	    current: {
+		scsihw: '',
+	    },
+	},
+    },
+
+    cbindData: {
+	nodename: undefined,
+    },
+
+    subject: gettext('Import Virtual Machine'),
+
+    isImportVM: true,
+
+    addDiskFunction: function() {
+	let me = this;
+	let wizard;
+	if (me.xtype === 'button') {
+		wizard = me.up('window');
+	} else if (me.xtype === 'pveQemuImportWizard') {
+		wizard = me;
+	}
+	let multihd = wizard.down('pveQemuMultiHDInputPanel');
+	multihd.addDiskFunction();
+    },
+
+    items: [
+	{
+	    xtype: 'inputpanel',
+	    title: gettext('Import'),
+	    itemId: 'importInputpanel',
+	    column1: [
+		{
+		    xtype: 'pveNodeSelector',
+		    name: 'nodename',
+		    cbind: {
+			selectCurNode: '{!nodename}',
+			preferredValue: '{nodename}',
+		    },
+		    bind: {
+			value: '{nodename}',
+		    },
+		    fieldLabel: gettext('Node'),
+		    allowBlank: false,
+		    onlineValidator: true,
+		},
+		{
+		    xtype: 'pveGuestIDSelector',
+		    name: 'vmid',
+		    guestType: 'qemu',
+		    value: '',
+		    loadNextFreeID: true,
+		    validateExists: false,
+		},
+	    ],
+	    column2: [
+		{
+		    xtype: 'label',
+		    itemId: 'successTextfield',
+		    hidden: true,
+		    html: gettext('Manifest successfully uploaded'),
+		    margin: '0 0 0 10',
+		},
+		{
+		    xtype: 'textfield',
+		    itemId: 'server_ovf_manifest',
+		    name: 'ovf_textfield',
+		    emptyText: '/mnt/nfs/exported.ovf',
+		    fieldLabel: 'Absolute path to .ovf manifest on your PVE host',
+		    listeners: {
+			validitychange: function(_, isValid) {
+			    let button = Ext.ComponentQuery.query('#load_remote_manifest_button').pop();
+			    button.setDisabled(!isValid);
+			},
+		    },
+		    validator: function(value) {
+			return (value && value.startsWith('/')) || gettext("Must start with /");
+		    },
+		},
+		{
+		    xtype: 'proxmoxButton',
+		    itemId: 'load_remote_manifest_button',
+		    text: gettext('Load remote manifest'),
+		    disabled: true,
+		    handler: function() {
+			let inputpanel = this.up('#importInputpanel');
+			let nodename = inputpanel.down('pveNodeSelector').getValue();
+			 // independent of onGetValues(), so that value of
+			 // ovf_textfield can be removed for submit
+			let ovf_textfield_value = inputpanel.down('textfield[name=ovf_textfield]').getValue();
+			let wizard = this.up('window');
+			Proxmox.Utils.API2Request({
+			    url: '/nodes/' + nodename + '/readovf',
+			    method: 'GET',
+			    params: {
+				manifest: ovf_textfield_value,
+			    },
+			    success: function(response) {
+				let ovfdata = response.result.data;
+				wizard.down('#vmNameTextfield').setValue(ovfdata.name);
+				wizard.down('#cpupanel').getViewModel().set('coreCount', ovfdata.cores);
+				wizard.down('#memorypanel').down('pveMemoryField').setValue(ovfdata.memory);
+				delete ovfdata.name;
+				delete ovfdata.cores;
+				delete ovfdata.memory;
+				delete ovfdata.digest;
+				let devices = Object.keys(ovfdata); // e.g. ide0, sata2
+				let multihd = wizard.down('pveQemuMultiHDInputPanel');
+				if (devices.length > 0) {
+				    multihd.removeAllDisks();
+				}
+				for (var device of devices) {
+				    multihd.addDiskFunction(device, ovfdata[device]);
+				}
+			    },
+			    failure: function(response, opts) {
+				console.warn("Failure of load manifest button");
+				console.warn(response);
+			    },
+			});
+		    },
+		},
+	    ],
+	    onGetValues: function(values) {
+		delete values.server_ovf_manifest;
+		delete values.ovf_textfield;
+		return values;
+	    },
+	},
+	{
+	    xtype: 'inputpanel',
+	    title: gettext('General'),
+	    onlineHelp: 'qm_general_settings',
+	    column1: [
+		{
+		    xtype: 'textfield',
+		    name: 'name',
+		    itemId: 'vmNameTextfield',
+		    vtype: 'DnsName',
+		    value: '',
+		    fieldLabel: gettext('Name'),
+		    allowBlank: true,
+		},
+	    ],
+	    column2: [
+		{
+		    xtype: 'pvePoolSelector',
+		    fieldLabel: gettext('Resource Pool'),
+		    name: 'pool',
+		    value: '',
+		    allowBlank: true,
+		},
+	    ],
+	    advancedColumn1: [
+		{
+		    xtype: 'proxmoxcheckbox',
+		    name: 'onboot',
+		    uncheckedValue: 0,
+		    defaultValue: 0,
+		    deleteDefaultValue: true,
+		    fieldLabel: gettext('Start at boot'),
+		},
+	    ],
+	    advancedColumn2: [
+		{
+		    xtype: 'textfield',
+		    name: 'order',
+		    defaultValue: '',
+		    emptyText: 'any',
+		    labelWidth: 120,
+		    fieldLabel: gettext('Start/Shutdown order'),
+		},
+		{
+		    xtype: 'textfield',
+		    name: 'up',
+		    defaultValue: '',
+		    emptyText: 'default',
+		    labelWidth: 120,
+		    fieldLabel: gettext('Startup delay'),
+		},
+		{
+		    xtype: 'textfield',
+		    name: 'down',
+		    defaultValue: '',
+		    emptyText: 'default',
+		    labelWidth: 120,
+		    fieldLabel: gettext('Shutdown timeout'),
+		},
+	    ],
+	    onGetValues: function(values) {
+		['name', 'pool', 'onboot', 'agent'].forEach(function(field) {
+		    if (!values[field]) {
+			delete values[field];
+		    }
+		});
+
+		var res = PVE.Parser.printStartup({
+		    order: values.order,
+		    up: values.up,
+		    down: values.down,
+		});
+
+		if (res) {
+		    values.startup = res;
+		}
+
+		delete values.order;
+		delete values.up;
+		delete values.down;
+
+		return values;
+	    },
+	},
+	{
+	    xtype: 'pveQemuSystemPanel',
+	    title: gettext('System'),
+	    isCreate: true,
+	    insideWizard: true,
+	},
+	{
+	    xtype: 'pveQemuMultiHDInputPanel',
+	    title: gettext('Hard Disk'),
+	    bind: {
+		nodename: '{nodename}',
+	    },
+	    isCreate: true,
+	    insideWizard: true,
+	},
+	{
+	    itemId: 'cpupanel',
+	    xtype: 'pveQemuProcessorPanel',
+	    insideWizard: true,
+	    title: gettext('CPU'),
+	},
+	{
+	    itemId: 'memorypanel',
+	    xtype: 'pveQemuMemoryPanel',
+	    insideWizard: true,
+	    title: gettext('Memory'),
+	},
+	{
+	    xtype: 'pveQemuNetworkInputPanel',
+	    bind: {
+		nodename: '{nodename}',
+	    },
+	    title: gettext('Network'),
+	    insideWizard: true,
+	},
+	{
+	    title: gettext('Confirm'),
+	    layout: 'fit',
+	    items: [
+		{
+		    xtype: 'grid',
+		    store: {
+			model: 'KeyValue',
+			sorters: [{
+			    property: 'key',
+			    direction: 'ASC',
+			}],
+		    },
+		    columns: [
+			{ header: 'Key', width: 150, dataIndex: 'key' },
+			{ header: 'Value', flex: 1, dataIndex: 'value' },
+		    ],
+		},
+	    ],
+	    dockedItems: [
+		{
+		    xtype: 'proxmoxcheckbox',
+		    name: 'start',
+		    dock: 'bottom',
+		    margin: '5 0 0 0',
+		    boxLabel: gettext('Start after created'),
+		},
+	    ],
+	    listeners: {
+		show: function(panel) {
+		    var kv = this.up('window').getValues();
+		    var data = [];
+		    Ext.Object.each(kv, function(key, value) {
+			if (key === 'delete') { // ignore
+			    return;
+			}
+			data.push({ key: key, value: value });
+		    });
+
+		    var summarystore = panel.down('grid').getStore();
+		    summarystore.suspendEvents();
+		    summarystore.removeAll();
+		    summarystore.add(data);
+		    summarystore.sort();
+		    summarystore.resumeEvents();
+		    summarystore.fireEvent('refresh');
+		},
+	    },
+	    onSubmit: function() {
+		var wizard = this.up('window');
+		var params = wizard.getValues();
+
+		var nodename = params.nodename;
+		delete params.nodename;
+		delete params.delete;
+		if (Array.isArray(params.diskimages)) {
+			params.diskimages = params.diskimages.join(',');
+		}
+
+		Proxmox.Utils.API2Request({
+		    url: `/nodes/${nodename}/qemu/${params.vmid}/importvm`,
+		    waitMsgTarget: wizard,
+		    method: 'POST',
+		    params: params,
+		    success: function() {
+			wizard.close();
+		    },
+		    failure: function(response) {
+			Ext.Msg.alert(gettext('Error'), response.htmlStatus);
+		    },
+		});
+	    },
+	},
+    ],
+});
diff --git a/www/manager6/qemu/MultiHDEdit.js b/www/manager6/qemu/MultiHDEdit.js
new file mode 100644
index 00000000..641a802f
--- /dev/null
+++ b/www/manager6/qemu/MultiHDEdit.js
@@ -0,0 +1,277 @@
+Ext.define('PVE.qemu.MultiHDInputPanel', {
+    extend: 'Proxmox.panel.InputPanel',
+    alias: 'widget.pveQemuMultiHDInputPanel',
+
+    insideWizard: false,
+
+    hiddenDisks: [],
+
+    leftColumnRatio: 0.25,
+
+    column1: [
+	{
+	    // Adding to the HDInputPanelContainer below automatically adds
+	    // items to this store
+	    xtype: 'gridpanel',
+	    scrollable: true,
+	    store: {
+		xtype: 'store',
+		storeId: 'importwizard_diskstorage',
+		// Use the panel as id
+		// Panels have are objects and therefore unique
+		// E.g. while adding new panels 'device' is ambiguous
+		fields: ['device', 'panel'],
+		removeByPanel: function(panel) {
+		    let recordIndex = this.findBy(record => record.data.panel === panel);
+		    this.removeAt(recordIndex);
+		    return recordIndex;
+		},
+	    },
+	    columns: [
+		{
+		    text: gettext('Target device'),
+		    dataIndex: 'device',
+		    flex: 1,
+		    resizable: false,
+		},
+	    ],
+	    listeners: {
+		select: function(_, record) {
+		    this.up('pveQemuMultiHDInputPanel')
+			.down('#HDInputPanelContainer')
+			.setActiveItem(record.data.panel);
+		},
+	    },
+	    anchor: '100% 90%',
+	}, {
+	    xtype: 'container',
+	    layout: 'hbox',
+	    center: true,
+	    defaults: {
+		margin: '5',
+		xtype: 'button',
+	    },
+	    items: [
+		{
+		    iconCls: 'fa fa-plus-circle',
+		    itemId: 'addDisk',
+		    handler: function(button) {
+			button.up('pveQemuMultiHDInputPanel').addDiskFunction();
+		    },
+		}, {
+		    iconCls: 'fa fa-trash-o',
+		    itemId: 'removeDisk',
+		    handler: function(button) {
+			button.up('pveQemuMultiHDInputPanel').removeCurrentDisk();
+		    },
+		},
+	    ],
+	},
+    ],
+    column2: [
+	{
+	    itemId: 'HDInputPanelContainer',
+	    xtype: 'container',
+	    layout: 'card',
+	    items: [],
+	    listeners: {
+		beforeRender: function() {
+		    // Initial disk if none have been added by manifest yet
+		    if (this.items.items.length === 0) {
+			this.addDiskFunction();
+		    }
+		},
+		add: function(container, newPanel, index) {
+		    let store = Ext.getStore('importwizard_diskstorage');
+		    store.add({ device: newPanel.getDevice(), panel: newPanel });
+		    container.setActiveItem(newPanel);
+		},
+		remove: function(HDInputPanelContainer, HDInputPanel, eOpts) {
+		    let store = Ext.getStore('importwizard_diskstorage');
+		    let indexOfRemoved = store.removeByPanel(HDInputPanel);
+		    if (HDInputPanelContainer.items.getCount() > 0) {
+			HDInputPanelContainer.setActiveItem(indexOfRemoved - 1);
+		    }
+		},
+	    },
+	    defaultItem: {
+		xtype: 'pveQemuHDInputPanel',
+		bind: {
+		    nodename: '{nodename}',
+		},
+		isCreate: true,
+		isImportVM: true,
+		returnSingleKey: true,
+		insideWizard: true,
+		setNodename: function(nodename) {
+		    this.down('#hdstorage').setNodename(nodename);
+		    this.down('#hdimage').setStorage(undefined, nodename);
+		    this.down('#sourceStorageSelector').setNodename(nodename);
+		    this.down('#sourceFileSelector').setNodename(nodename);
+		},
+		listeners: {
+		    // newHDInputPanel ... the defaultItem that has just been
+		    //   cloned and added into HDInputPnaleContainer parameter
+		    // HDInputPanelContainer ... the container from column2
+		    //   where all the new panels go into
+		    added: function(newHDInputPanel, HDInputPanelContainer, pos) {
+			// The listeners cannot be added earlier, because its fields don't exist earlier
+			Ext.Array.each(this.down('pveControllerSelector')
+			    .query('field'), function(field) {
+				field.on('change', function() {
+				    // Note that one setValues in a controller
+				    // selector makes one setValue in each of
+				    // the two fields, so this listener fires
+				    // two times in a row so to say e.g.
+				    // changing controller selector from ide0 to
+				    // sata1 makes ide0->sata0 and then
+				    // sata0->sata1
+				    let store = Ext.getStore('importwizard_diskstorage');
+				    let controllerSelector = field.up('pveQemuHDInputPanel')
+					.down('pveControllerSelector');
+				    /*
+				     * controller+device (ide0) might be
+				     * ambiguous during creation => find by
+				     * panel object instead
+				     *
+				     * There is no function that takes a
+				     * function and returns the model directly
+				     * => index & getAt
+				     */
+				    let recordIndex = store.findBy(record =>
+					record.data.panel === field.up('pveQemuHDInputPanel'),
+				    );
+				    let newControllerAndId = controllerSelector.getValuesAsString();
+				    store.getAt(recordIndex).set('device', newControllerAndId);
+				});
+			    },
+			);
+			let wizard = this.up('pveQemuImportWizard');
+			Ext.Array.each(this.query('field'), function(field) {
+			    field.on('change', wizard.validcheck);
+			    field.on('validitychange', wizard.validcheck);
+			});
+		    },
+		},
+		validator: function() {
+		    var valid = true;
+		    var fields = this.query('field, fieldcontainer');
+		    Ext.Array.each(fields, function(field) {
+			// Note: not all fielcontainer have isValid()
+			if (Ext.isFunction(field.isValid) && !field.isValid()) {
+			    valid = false;
+			}
+		    });
+		    return valid;
+		},
+	    },
+
+	    // device ... device that the new disk should be assigned to, e.g.
+	    //   ide0, sata2
+	    // path ... if this is set to x then the disk will
+	    //   backed/imported from the path x, that is, the textfield will
+	    //   contain the value x
+	    addDiskFunction(device, path) {
+		// creating directly removes binding => no storage found?
+		let item = Ext.clone(this.defaultItem);
+		let added = this.add(item);
+		// At this point the 'added' listener has fired and the fields
+		// in the variable added have the change listeners that update
+		// the store Therefore we can now set values only on the field
+		// and they will be updated in the store
+		if (path) {
+		    added.down('#sourceRadioPath').setValue(true);
+		    added.down('#sourcePathTextfield').setValue(path);
+		} else {
+		    added.down('#sourceRadioStorage').setValue(true);
+		    added.down('#sourceStorageSelector').setHidden(false);
+		    added.down('#sourceFileSelector').setHidden(false);
+		    added.down('#sourceFileSelector').enable();
+		    added.down('#sourceStorageSelector').enable();
+		}
+
+		let sp = Ext.state.Manager.getProvider();
+		let advanced_checkbox = sp.get('proxmox-advanced-cb');
+		added.setAdvancedVisible(advanced_checkbox);
+
+		if (device) {
+		    // This happens after the 'add' and 'added' listeners of the
+		    // item/defaultItem clone/pveQemuHDInputPanel/added have fired
+		    added.down('pveControllerSelector').setValue(device);
+		}
+	    },
+	    removeCurrentDisk: function() {
+		let activePanel = this.getLayout().activeItem; // panel = disk
+		if (activePanel) {
+		    this.remove(activePanel);
+		}
+	    },
+	},
+    ],
+
+    addDiskFunction: function(device, path) {
+	this.down('#HDInputPanelContainer').addDiskFunction(device, path);
+    },
+    removeCurrentDisk: function() {
+	this.down('#HDInputPanelContainer').removeCurrentDisk();
+    },
+    removeAllDisks: function() {
+	let container = this.down('#HDInputPanelContainer');
+	while (container.items.items.length > 0) {
+		container.removeCurrentDisk();
+	}
+    },
+
+    beforeRender: function() {
+	let leftColumnPanel = this.items.get(0).items.get(0);
+	leftColumnPanel.setFlex(this.leftColumnRatio);
+	// any other panel because this has no height yet
+	let panelHeight = this.up('tabpanel').items.items[0].getHeight();
+	leftColumnPanel.setHeight(panelHeight);
+    },
+
+    setNodename: function(nodename) {
+	this.nodename = nodename;
+    },
+
+    // Call with defined parameter or without
+    hasDuplicateDevices: function(values) {
+	if (!values) {
+	    values = this.up('form').getValues();
+	}
+	if (!Array.isArray(values.controller)) {
+	    return false;
+	}
+	for (let i = 0; i < values.controller.length - 1; i++) {
+	    for (let j = i+1; j < values.controller.length; j++) {
+		if (values.controller[i] === values.controller[j]) {
+		    if (values.deviceid[i] === values.deviceid[j]) {
+			return true;
+		    }
+		}
+	    }
+	}
+	return false;
+    },
+
+    onGetValues: function(values) {
+	// Returning anything here would give wrong data in the form at the end
+	// of the wizrad Each HDInputPanel in this MultiHD panel already has a
+	// sufficient onGetValues() function for the form at the end of the
+	// wizard
+	if (this.hasDuplicateDevices(values)) {
+	    Ext.Msg.alert(gettext('Error'), 'Equal target devices are forbidden. Make all unique!');
+	}
+    },
+
+    validator: function() {
+	let inputpanels = this.down('#HDInputPanelContainer').items.getRange();
+	if (inputpanels.some(panel => !panel.validator())) {
+	    return false;
+	}
+	if (this.hasDuplicateDevices()) {
+	    return false;
+	}
+	return true;
+    },
+});
diff --git a/www/manager6/window/Wizard.js b/www/manager6/window/Wizard.js
index 8b930bbd..a3e3b690 100644
--- a/www/manager6/window/Wizard.js
+++ b/www/manager6/window/Wizard.js
@@ -261,6 +261,8 @@ Ext.define('PVE.window.Wizard', {
 	    };
 	    field.on('change', validcheck);
 	    field.on('validitychange', validcheck);
+	    // Make available for fields that get added later
+	    me.validcheck = validcheck;
 	});
     },
 });
-- 
2.20.1




^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [pve-devel] [PATCH v7 qemu-server] Add API for import wizards
  2021-03-26 12:32 ` [pve-devel] [PATCH v7 qemu-server] Add API for import wizards Dominic Jäger
@ 2021-03-31 15:12   ` Fabian Grünbichler
  2021-04-01 10:19     ` Dominic Jäger
  0 siblings, 1 reply; 7+ messages in thread
From: Fabian Grünbichler @ 2021-03-31 15:12 UTC (permalink / raw)
  To: Proxmox VE development discussion

this is starting to shape up nicely. as promised, I now took a stab at 
(roughly!) integrating this into our regular flow (see diff below):
- IMPORT_DISK_RE now uses -1, as 0 actually can be mismatched by 
  NEW_DISK_RE
- the actual import happens in create_disks
- only the basic checks (match of "marked for import" with "import 
  sources") happen early on
- the target storage and source volume are permission checked now
- importvm is dropped in favor of calling create_vm with import_sources
- update_vm_async now also supports import_sources

the last two IMHO have some nice benefits:
- we can now easily mix and match importing and newly allocating blank 
  disks in a single create call (allowing us to use a single GUI wizard 
  as well if we want)
- create_vm and importvm don't have to duplicate all the surrounding 
  logic (or have strange contortions to make one call the other)
- we could likely drop the separate import_disk API call, and let the 
  `importdisk` CLI command prepare parameters for a regular VM config 
  update

I'm sure I've missed some corner cases, as I've only tested create_vm 
with importing, and not the other newly exposed APIs.

stat shows how much boilerplace/duplication this removes, although there 
is probably even more potential here since `create_vm` partly duplicates 
the `update_vm_api` sub that ends up calling `create_disks`:

 PVE/API2/Qemu.pm | 481 ++++++++++++++++++++-----------------------------------
 1 file changed, 171 insertions(+), 310 deletions(-)

the original patch was +381 for that file, so in total we are now at 
+242 instead.

could you take a look and see if I missed anything fundamental?

-----8<-----

diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
index 2f50f38..41e1ab7 100644
--- a/PVE/API2/Qemu.pm
+++ b/PVE/API2/Qemu.pm
@@ -60,8 +60,19 @@ my $resolve_cdrom_alias = sub {
     }
 };
 
+my $parse_import_sources = sub {
+    my $param = shift;
+    my $import = {};
+    foreach my $pair (PVE::Tools::split_list($param)) {
+	my ($device, $diskimage) = split('=', $pair);
+	$import->{$device} = $diskimage;
+    }
+
+    return $import;
+};
+
 my $NEW_DISK_RE = qr!^(([^/:\s]+):)?(\d+(\.\d+)?)$!;
-my $IMPORT_DISK_RE = qr!^([^/:\s]+):0$!;
+my $IMPORT_DISK_RE = qr!^([^/:\s]+):-1$!;
 my $check_storage_access = sub {
    my ($rpcenv, $authuser, $storecfg, $vmid, $settings, $default_storage) = @_;
 
@@ -84,6 +95,12 @@ my $check_storage_access = sub {
 	    my $scfg = PVE::Storage::storage_config($storecfg, $storeid);
 	    raise_param_exc({ storage => "storage '$storeid' does not support vm images"})
 		if !$scfg->{content}->{images};
+	} elsif ($volid =~ $IMPORT_DISK_RE) {
+	    my $storeid = $1;
+	    $rpcenv->check($authuser, "/storage/$storeid", ['Datastore.AllocateSpace']);
+	    my $scfg = PVE::Storage::storage_config($storecfg, $storeid);
+	    raise_param_exc({ storage => "storage '$storeid' does not support vm images"})
+		if !$scfg->{content}->{images};
 	} else {
 	    PVE::Storage::check_volume_access($rpcenv, $authuser, $storecfg, $vmid, $volid);
 	}
@@ -91,6 +108,13 @@ my $check_storage_access = sub {
 
    $rpcenv->check($authuser, "/storage/$settings->{vmstatestorage}", ['Datastore.AllocateSpace'])
        if defined($settings->{vmstatestorage});
+
+    if (defined($settings->{import_sources})) {
+	my $images = $parse_import_sources->($settings->{import_sources});
+	foreach my $source_image (values %$images) {
+	    PVE::Storage::check_volume_access($rpcenv, $authuser, $storecfg, $vmid, $source_image);
+	}
+    }
 };
 
 my $check_storage_access_clone = sub {
@@ -133,10 +157,25 @@ my $check_storage_access_clone = sub {
    return $sharedvm;
 };
 
+# Raise exception if $format is not supported by $storeid
+my $check_format_is_supported = sub {
+    my ($format, $storeid, $storecfg) = @_;
+    die "storage ID parameter must be passed to the sub" if !$storeid;
+    die "storage configuration must be passed to the sub" if !$storecfg;
+
+    return if !$format;
+
+    my (undef, $valid_formats) = PVE::Storage::storage_default_format($storecfg, $storeid);
+    my $supported = grep { $_ eq $format } @$valid_formats;
+
+    die "format '$format' is not supported on storage $storeid" if !$supported;
+};
+
+
 # Note: $pool is only needed when creating a VM, because pool permissions
 # are automatically inherited if VM already exists inside a pool.
 my $create_disks = sub {
-    my ($rpcenv, $authuser, $conf, $arch, $storecfg, $vmid, $pool, $settings, $default_storage) = @_;
+    my ($rpcenv, $authuser, $conf, $arch, $storecfg, $vmid, $pool, $settings, $default_storage, $import) = @_;
 
     my $vollist = [];
 
@@ -192,6 +231,69 @@ my $create_disks = sub {
 	    $disk->{size} = PVE::Tools::convert_size($size, 'kb' => 'b');
 	    delete $disk->{format}; # no longer needed
 	    $res->{$ds} = PVE::QemuServer::print_drive($disk);
+	} elsif ($volid =~ $IMPORT_DISK_RE) {
+	    my $target_storage = $1;
+
+	    my $source = $import->{$ds};
+	    die "cannot import '$ds', no import source defined\n" if !$source;
+	    $source = PVE::Storage::abs_filesystem_path($storecfg, $source, 1);
+	    my $src_size = PVE::Storage::file_size_info($source);
+	    die "Could not get file size of $source" if !defined($src_size);
+
+	    $check_format_is_supported->($disk->{format}, $storeid, $storecfg);
+
+	    my $dst_format = PVE::QemuServer::resolve_dst_disk_format(
+		$storecfg,
+		$storeid,
+		undef,
+		$disk->{format},
+	    );
+	    my $dst_volid = PVE::Storage::vdisk_alloc(
+		$storecfg,
+		$storeid,
+		$vmid,
+		$dst_format,
+		undef,
+		PVE::Tools::convert_size($src_size, 'b' => 'kb'),
+	    );
+
+	    print "Importing disk image '$source' as '$dst_volid'...\n";
+	    eval {
+		local $SIG{INT} =
+		local $SIG{TERM} =
+		local $SIG{QUIT} =
+		local $SIG{HUP} =
+		local $SIG{PIPE} = sub { die "Interrupted by signal $!\n"; };
+
+		my $zeroinit = PVE::Storage::volume_has_feature(
+		    $storecfg,
+		    'sparseinit',
+		    $dst_volid,
+		);
+		PVE::Storage::activate_volumes($storecfg, [$dst_volid]);
+		PVE::QemuServer::qemu_img_convert(
+		    $source,
+		    $dst_volid,
+		    $src_size,
+		    undef,
+		    $zeroinit,
+		);
+		PVE::Storage::deactivate_volumes($storecfg, [$dst_volid]);
+
+	    };
+	    if (my $err = $@) {
+		eval { PVE::Storage::vdisk_free($storecfg, $dst_volid) };
+		warn "Cleanup of $dst_volid failed: $@ \n" if $@;
+
+		die "Importing disk '$source' failed: $err\n" if $err;
+	    }
+	    push @$vollist, $dst_volid;
+	    $disk->{file} = $dst_volid;
+	    if ($ds !~ /^unused\d+$/) {
+		$disk->{size} = $src_size;
+		delete $disk->{format}; # no longer needed
+	    }
+	    $res->{$ds} = PVE::QemuServer::print_drive($disk);
 	} else {
 
 	    PVE::Storage::check_volume_access($rpcenv, $authuser, $storecfg, $vmid, $volid);
@@ -218,7 +320,7 @@ my $create_disks = sub {
 	}
     };
 
-    eval { PVE::QemuConfig->foreach_volume($settings, $code); };
+    eval { PVE::QemuConfig->foreach_volume_full($settings, { include_unused => 1 }, $code); };
 
     # free allocated images on error
     if (my $err = $@) {
@@ -544,6 +646,13 @@ __PACKAGE__->register_method({
 		    default => 0,
 		    description => "Start VM after it was created successfully.",
 		},
+		import_sources => {
+		    description => "\\0 delimited mapping of devices to disk images to import." .
+			"For example, scsi0=/mnt/nfs/image1.vmdk",
+		    type => 'string',
+		    format => 'device-image-pair-alist',
+		    optional => 1,
+		},
 	    }),
     },
     returns => {
@@ -608,21 +717,34 @@ __PACKAGE__->register_method({
 
 	    &$check_cpu_model_access($rpcenv, $authuser, $param);
 
+	    my $import_devices = $parse_import_sources->($param->{import_sources});
+
 	    foreach my $opt (keys %$param) {
 		if (PVE::QemuServer::is_valid_drivename($opt)) {
 		    my $drive = PVE::QemuServer::parse_drive($opt, $param->{$opt});
 		    raise_param_exc({ $opt => "unable to parse drive options" }) if !$drive;
+		    raise_param_exc({ $opt => "not marked for import, but import source defined" })
+			if $drive->{file} !~ $IMPORT_DISK_RE && $import_devices->{$opt};
+		    raise_param_exc({ $opt => "marked for import, but no import source defined" })
+			if $drive->{file} =~ $IMPORT_DISK_RE && !$import_devices->{$opt};
 
 		    PVE::QemuServer::cleanup_drive_path($opt, $storecfg, $drive);
 		    $param->{$opt} = PVE::QemuServer::print_drive($drive);
 		}
 	    }
+	    foreach my $opt (keys %$import_devices) {
+		raise_param_exc({ import_sources => "$opt not marked for import, but import source defined" })
+		    if !defined($param->{$opt});
+ 
+	    }
 
 	    PVE::QemuServer::add_random_macs($param);
 	} else {
 	    my $keystr = join(' ', keys %$param);
 	    raise_param_exc({ archive => "option conflicts with other options ($keystr)"}) if $keystr;
 
+	    raise_param_exc({ import_sources => "cannot import existing disk and restore backup." }) if $param->{import_sources};
+
 	    if ($archive eq '-') {
 		die "pipe requires cli environment\n"
 		    if $rpcenv->{type} ne 'cli';
@@ -690,10 +812,11 @@ __PACKAGE__->register_method({
 	    my $realcmd = sub {
 		my $conf = $param;
 		my $arch = PVE::QemuServer::get_vm_arch($conf);
+		my $import = $parse_import_sources->(extract_param($param, "import_sources"));
 
 		my $vollist = [];
 		eval {
-		    $vollist = &$create_disks($rpcenv, $authuser, $conf, $arch, $storecfg, $vmid, $pool, $param, $storage);
+		    $vollist = &$create_disks($rpcenv, $authuser, $conf, $arch, $storecfg, $vmid, $pool, $param, $storage, $import);
 
 		    if (!$conf->{boot}) {
 			my $devs = PVE::QemuServer::get_default_bootdevices($conf);
@@ -1163,11 +1286,17 @@ my $update_vm_api  = sub {
 	die "cannot add non-replicatable volume to a replicated VM\n";
     };
 
+    my $import_devices = $parse_import_sources->($param->{import_sources});
+
     foreach my $opt (keys %$param) {
 	if (PVE::QemuServer::is_valid_drivename($opt)) {
 	    # cleanup drive path
 	    my $drive = PVE::QemuServer::parse_drive($opt, $param->{$opt});
 	    raise_param_exc({ $opt => "unable to parse drive options" }) if !$drive;
+	    raise_param_exc({ $opt => "not marked for import, but import source defined" })
+		if $drive->{file} !~ $IMPORT_DISK_RE && $import_devices->{$opt};
+	    raise_param_exc({ $opt => "marked for import, but no import source defined" })
+		if $drive->{file} =~ $IMPORT_DISK_RE && !$import_devices->{$opt};
 	    PVE::QemuServer::cleanup_drive_path($opt, $storecfg, $drive);
 	    $check_replication->($drive);
 	    $param->{$opt} = PVE::QemuServer::print_drive($drive);
@@ -1185,12 +1314,20 @@ my $update_vm_api  = sub {
 	}
     }
 
+    foreach my $opt (keys %$import_devices) {
+	raise_param_exc({ import_sources => "$opt not marked for import, but import source defined" })
+	    if !defined($param->{$opt});
+
+    }
+
     &$check_vm_modify_config_perm($rpcenv, $authuser, $vmid, undef, [@delete]);
 
     &$check_vm_modify_config_perm($rpcenv, $authuser, $vmid, undef, [keys %$param]);
 
     &$check_storage_access($rpcenv, $authuser, $storecfg, $vmid, $param);
 
+    delete $param->{import_sources};
+
     my $updatefn =  sub {
 
 	my $conf = PVE::QemuConfig->load_config($vmid);
@@ -1332,7 +1469,7 @@ my $update_vm_api  = sub {
 		    PVE::QemuServer::vmconfig_register_unused_drive($storecfg, $vmid, $conf, PVE::QemuServer::parse_drive($opt, $conf->{pending}->{$opt}))
 			if defined($conf->{pending}->{$opt});
 
-		    &$create_disks($rpcenv, $authuser, $conf->{pending}, $arch, $storecfg, $vmid, undef, {$opt => $param->{$opt}});
+		    &$create_disks($rpcenv, $authuser, $conf->{pending}, $arch, $storecfg, $vmid, undef, {$opt => $param->{$opt}}, {$opt => $import_devices->{$opt}});
 		} elsif ($opt =~ m/^serial\d+/) {
 		    if ((!defined($conf->{$opt}) || $conf->{$opt} eq 'socket') && $param->{$opt} eq 'socket') {
 			$rpcenv->check_vm_perm($authuser, $vmid, undef, ['VM.Config.HWType']);
@@ -1476,6 +1613,13 @@ __PACKAGE__->register_method({
 		    optional => 1,
 		    requires => 'delete',
 		},
+		import_sources => {
+		    description => "\\0 delimited mapping of devices to disk images to import." .
+			"For example, scsi0=/mnt/nfs/image1.vmdk",
+		    type => 'string',
+		    format => 'device-image-pair-alist',
+		    optional => 1,
+		},
 		digest => {
 		    type => 'string',
 		    description => 'Prevent changes if current configuration file has different SHA1 digest. This can be used to prevent concurrent modifications.',
@@ -4377,111 +4521,6 @@ __PACKAGE__->register_method({
 	return PVE::QemuServer::Cloudinit::dump_cloudinit_config($conf, $param->{vmid}, $param->{type});
     }});
 
-# Raise exception if $format is not supported by $storeid
-my $check_format_is_supported = sub {
-    my ($format, $storeid, $storecfg) = @_;
-    die "storage ID parameter must be passed to the sub" if !$storeid;
-    die "storage configuration must be passed to the sub" if !$storecfg;
-
-    return if !$format;
-
-    my (undef, $valid_formats) = PVE::Storage::storage_default_format($storecfg, $storeid);
-    my $supported = grep { $_ eq $format } @$valid_formats;
-
-    die "format '$format' is not supported on storage $storeid" if !$supported;
-};
-
-# storecfg ... PVE::Storage::config()
-# vmid ... target VM ID
-# vmconf ... target VM configuration
-# source ... source image (volid or absolute path)
-# target ... hash with
-#    storeid => storage ID
-#    format => disk format (optional)
-#    options => hash with device options (may or may not contain <storeid>:0)
-#    device => device where the disk is attached (for example, scsi3) (optional)
-#
-# returns ... volid of the allocated disk image (e.g. local-lvm:vm-100-disk-2)
-my $import_disk_image = sub {
-    my ($storecfg, $vmid, $vmconf, $source, $target) = @_;
-    my $requested_format = $target->{format};
-    my $storeid = $target->{storeid};
-
-    die "Source parameter is undefined!" if !defined $source;
-    $source = PVE::Storage::abs_filesystem_path($storecfg, $source, 1);
-
-    eval { PVE::Storage::storage_config($storecfg, $storeid) };
-    die "Error while importing disk image $source: $@\n" if $@;
-
-    my $src_size = PVE::Storage::file_size_info($source);
-    die "Could not get file size of $source" if !defined($src_size);
-
-    $check_format_is_supported->($requested_format, $storeid, $storecfg);
-
-    my $dst_format = PVE::QemuServer::resolve_dst_disk_format(
-	$storecfg,
-	$storeid,
-	undef,
-	$requested_format,
-    );
-    my $dst_volid = PVE::Storage::vdisk_alloc(
-	$storecfg,
-	$storeid,
-	$vmid,
-	$dst_format,
-	undef,
-	$src_size / 102,
-    );
-
-    print "Importing disk image '$source' as '$dst_volid'...\n";
-    eval {
-	local $SIG{INT} =
-	local $SIG{TERM} =
-	local $SIG{QUIT} =
-	local $SIG{HUP} =
-	local $SIG{PIPE} = sub { die "Interrupted by signal $!\n"; };
-
-	my $zeroinit = PVE::Storage::volume_has_feature(
-	    $storecfg,
-	    'sparseinit',
-	    $dst_volid,
-	);
-	PVE::Storage::activate_volumes($storecfg, [$dst_volid]);
-	PVE::QemuServer::qemu_img_convert(
-	    $source,
-	    $dst_volid,
-	    $src_size,
-	    undef,
-	    $zeroinit,
-	);
-	PVE::Storage::deactivate_volumes($storecfg, [$dst_volid]);
-
-    };
-    if (my $err = $@) {
-	eval { PVE::Storage::vdisk_free($storecfg, $dst_volid) };
-	warn "Cleanup of $dst_volid failed: $@ \n" if $@;
-
-	die "Importing disk '$source' failed: $err\n" if $err;
-    }
-
-    $target->{options}->{file} = $dst_volid;
-    my $options_string = PVE::QemuServer::print_drive($target->{options});
-    $target->{device} = PVE::QemuConfig->add_unused_volume($vmconf, $dst_volid)
-	if !$target->{device};
-
-    $update_vm_api->(
-	{
-	    vmid => $vmid,
-	    $target->{device} => $options_string,
-	    skiplock => 1,
-	    digest => $vmconf->{digest},
-	},
-	1,
-    );
-
-    return $dst_volid;
-};
-
 __PACKAGE__->register_method ({
     name => 'importdisk',
     path => '{vmid}/importdisk',
@@ -4544,22 +4583,25 @@ __PACKAGE__->register_method ({
 	PVE::Storage::storage_config($storecfg, $storeid);
 
 
-	if ($device_options) {
-	    # $device_options may or may not contain <storeid>:0
-	    my $parsed = PVE::QemuServer::Drive::parse_drive($device, $device_options);
-	    if ($parsed) {
-		raise_param_exc({$device_options => "Invalid import syntax"})
-		    if !($parsed->{file} =~ $IMPORT_DISK_RE);
-	    } else {
-		my $fake = "$storeid:0,$device_options";
-		$parsed = PVE::QemuServer::Drive::parse_drive($device, $fake);
-	    }
-	    delete $parsed->{file};
-	    delete $parsed->{interface};
-	    delete $parsed->{index};
-	    $device_options = $parsed;
+	if (!$device_options) {
+	    $device_options = "$storeid:0";
+	}
+
+	# $device_options may or may not contain <storeid>:0
+	my $parsed = PVE::QemuServer::Drive::parse_drive($device, $device_options);
+
+	if ($parsed) {
+	    raise_param_exc({$device_options => "Invalid import syntax"})
+		if !($parsed->{file} =~ $IMPORT_DISK_RE);
+	} else {
+	    my $fake = "$storeid:0,$device_options";
+	    $parsed = PVE::QemuServer::Drive::parse_drive($device, $fake);
 	}
 
+	delete $parsed->{interface};
+	delete $parsed->{index};
+	$device_options = $parsed;
+
 	# Format can be set explicitly "--format vmdk"
 	# or as part of device options "--device_options discard=on,format=vmdk"
 	my $format = extract_param($param, 'format');
@@ -4570,192 +4612,11 @@ __PACKAGE__->register_method ({
 	}
 	$check_format_is_supported->($format, $storeid, $storecfg);
 
-	# quick checks before fork + lock
-	my $conf = PVE::QemuConfig->load_config($vmid);
-	PVE::QemuConfig->check_lock($conf);
-	PVE::Tools::assert_if_modified($conf->{digest}, $digest);
-	if ($device && $conf->{$device}) {
-	    die "Could not import because device $device is already in ".
-		"use in VM $vmid. Choose a different device!";
-	}
-
-	my $worker = sub {
-	    PVE::QemuConfig->lock_config($vmid, sub {
-		$conf = PVE::QemuConfig->load_config($vmid);
-		PVE::QemuConfig->check_lock($conf);
-
-		PVE::Tools::assert_if_modified($conf->{digest}, $digest);
-		PVE::QemuConfig->set_lock($vmid, 'import');
-		$conf = PVE::QemuConfig->load_config($vmid);
-	    });
-
-	    my $target = {
-		node => $node,
-		storeid => $storeid,
-	    };
-	    $target->{format} = $format;
-	    $target->{device} = $device;
-	    $target->{options} = $device_options;
-	    eval { $import_disk_image->($storecfg, $vmid, $conf, $source, $target) };
-	    my $err = $@;
-	    eval { PVE::QemuConfig->remove_lock($vmid, 'import') };
-	    warn $@ if $@;
-	    die $err if $err;
-	};
-	return $rpcenv->fork_worker('importdisk', $vmid, $authuser, $worker);
+	return $update_vm_api->({
+	    $device => PVE::QemuServer::Drive::print_drive($device_options),
+	    import_sources => "$device=$source",
+	    digest => $digest,
+	});
     }});
 
-__PACKAGE__->register_method({
-    name => 'importvm',
-    path => '{vmid}/importvm',
-    method => 'POST',
-    description => "Import a VM from existing disk images.",
-    protected => 1,
-    proxyto => 'node',
-    parameters => {
-	additionalProperties => 0,
-	properties => PVE::QemuServer::json_config_properties(
-	    {
-		node => get_standard_option('pve-node'),
-		vmid => get_standard_option('pve-vmid', { completion =>
-		    \&PVE::Cluster::complete_next_vmid }),
-		diskimage => {
-		    description => "\\0 delimited mapping of devices to disk images. For " .
-			"example, scsi0=/mnt/nfs/image1.vmdk",
-		    type => 'string',
-		    format => 'device-image-pair-alist',
-		},
-		start => {
-		    optional => 1,
-		    type => 'boolean',
-		    default => 0,
-		    description => "Start VM after it was imported successfully.",
-		},
-	    }),
-    },
-    returns => {
-	type => 'string',
-    },
-    code => sub {
-	my ($param) = @_;
-	my $node = extract_param($param, 'node');
-	my $vmid = extract_param($param, 'vmid');
-	my $diskimages_string = extract_param($param, 'diskimage');
-	my $boot = extract_param($param, 'boot');
-	my $start = extract_param($param, 'start');
-
-	my $rpcenv = PVE::RPCEnvironment::get();
-	my $authuser = $rpcenv->get_user();
-	my $storecfg = PVE::Storage::config();
-
-	PVE::Cluster::check_cfs_quorum();
-
-	my $import_param = {};
-	foreach my $opt (keys %$param) {
-	    next if $opt eq 'efidisk0';
-	    raise_param_exc({bootdisk => "Deprecated: Use --boot order= instead"})
-		if $opt eq 'bootdisk';
-
-	    if (PVE::QemuServer::Drive::is_valid_drivename($opt)) {
-		my $drive = PVE::QemuServer::parse_drive($opt, $param->{$opt});
-		if ($drive->{file} =~ $IMPORT_DISK_RE) {
-		    $import_param->{$opt} = $drive;
-		    delete $param->{$opt};
-		}
-	    }
-	}
-
-	my $diskimages = {};
-	foreach my $pair (PVE::Tools::split_list($diskimages_string)) {
-	    my ($device, $diskimage) = split('=', $pair);
-	    $diskimages->{$device} = $diskimage;
-	    raise_param_exc({
-		$device => "Device '$device' not marked for import, " .
-		    "but import source '$diskimage' specified",
-	    }) if !defined($import_param->{$device});
-	    PVE::Storage::abs_filesystem_path($storecfg, $diskimage, 1);
-	}
-
-	foreach my $device (keys %$import_param) {
-	    raise_param_exc({
-		$device => "Device '$device' marked for import, but no source given\n",
-	    }) if !defined($diskimages->{$device});
-	}
-
-	eval { PVE::QemuConfig->create_and_lock_config($vmid, 0, 'import') };
-	die "Unable to create config for VM import: $@" if $@;
-
-	my $worker = sub {
-	    my $reload_conf = sub {
-		my ($vmid) = @_;
-		my $conf = PVE::QemuConfig->load_config($vmid);
-		return $conf if PVE::QemuConfig->has_lock($conf, 'import');
-		die "import lock in VM $vmid config file missing!";
-	    };
-
-	    my $conf = $reload_conf->($vmid);
-	    $update_vm_api->(
-		{
-		    %$param,
-		    node => $node,
-		    vmid => $vmid,
-		    skiplock => 1,
-		    digest => $conf->{digest},
-		},
-		1
-	    );
-
-	    eval {
-		foreach my $device (keys %$import_param) {
-		    $conf = $reload_conf->($vmid);
-		    my $drive = $import_param->{$device};
-		    my $storeid = PVE::Storage::parse_volume_id($drive->{file});
-		    my $imported = $import_disk_image->(
-			$storecfg,
-			$vmid,
-			$conf,
-			$diskimages->{$device},
-			{
-			    storeid => $storeid,
-			    format => $drive->{format},
-			    options => $drive,
-			    device => $device,
-			},
-		    );
-		}
-	    };
-	    my $err = $@;
-	    if ($err) {
-		eval { PVE::QemuServer::destroy_vm($storecfg, $vmid, 1) };
-		warn "Could not destroy VM $vmid: $@" if "$@";
-
-		die "Import failed: $err";
-	    }
-
-	    $conf = $reload_conf->($vmid);
-	    if (!$boot) {
-		my $bootdevs = PVE::QemuServer::get_default_bootdevices($conf);
-		$boot = PVE::QemuServer::print_bootorder($bootdevs);
-	    }
-	    $update_vm_api->(
-		{
-		    node => $node,
-		    vmid => $vmid,
-		    boot => $boot,
-		    skiplock => 1,
-		    digest => $conf->{digest},
-		},
-		1,
-	    );
-
-	    eval { PVE::QemuConfig->remove_lock($vmid, 'import') };
-	    warn $@ if $@;
-
-	    PVE::QemuServer::vm_start($storecfg, $vmid) if $start;
-	};
-
-	return $rpcenv->fork_worker('importvm', $vmid, $authuser, $worker);
-    }});
-
-
 1;

----->8-----

On March 26, 2021 1:32 pm, Dominic Jäger wrote:
> Extend qm importdisk/importovf functionality to the API.
> 
> Signed-off-by: Dominic Jäger <d.jaeger@proxmox.com>
> 
> ---
> v6->v7: Feedback by Fabian G
> - Introduce a regex for the import syntax <storeid>:0
> - Use parameter list instead of hash for import helper
> - More parsing, less string magic
> - More VM config digest checking
> - Create a schema format for diskimage source mapping
> - Preliminarily remove some boot parameter handling
> - Dare to really edit schema format subs for a cleaner solution
> - Whitespace, variable names, ...
> 
>  PVE/API2/Qemu.pm       | 383 ++++++++++++++++++++++++++++++++++++++++-
>  PVE/API2/Qemu/Makefile |   2 +-
>  PVE/API2/Qemu/OVF.pm   |  68 ++++++++
>  PVE/QemuServer.pm      |  52 +++++-
>  PVE/QemuServer/OVF.pm  |  10 +-
>  5 files changed, 502 insertions(+), 13 deletions(-)
>  create mode 100644 PVE/API2/Qemu/OVF.pm
> 
> diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
> index e95ab13..2f50f38 100644
> --- a/PVE/API2/Qemu.pm
> +++ b/PVE/API2/Qemu.pm
> @@ -45,7 +45,6 @@ BEGIN {
>      }
>  }
>  
> -use Data::Dumper; # fixme: remove
>  
>  use base qw(PVE::RESTHandler);
>  
> @@ -62,6 +61,7 @@ my $resolve_cdrom_alias = sub {
>  };
>  
>  my $NEW_DISK_RE = qr!^(([^/:\s]+):)?(\d+(\.\d+)?)$!;
> +my $IMPORT_DISK_RE = qr!^([^/:\s]+):0$!;
>  my $check_storage_access = sub {
>     my ($rpcenv, $authuser, $storecfg, $vmid, $settings, $default_storage) = @_;
>  
> @@ -4377,4 +4377,385 @@ __PACKAGE__->register_method({
>  	return PVE::QemuServer::Cloudinit::dump_cloudinit_config($conf, $param->{vmid}, $param->{type});
>      }});
>  
> +# Raise exception if $format is not supported by $storeid
> +my $check_format_is_supported = sub {
> +    my ($format, $storeid, $storecfg) = @_;
> +    die "storage ID parameter must be passed to the sub" if !$storeid;
> +    die "storage configuration must be passed to the sub" if !$storecfg;
> +
> +    return if !$format;
> +
> +    my (undef, $valid_formats) = PVE::Storage::storage_default_format($storecfg, $storeid);
> +    my $supported = grep { $_ eq $format } @$valid_formats;
> +
> +    die "format '$format' is not supported on storage $storeid" if !$supported;
> +};
> +
> +# storecfg ... PVE::Storage::config()
> +# vmid ... target VM ID
> +# vmconf ... target VM configuration
> +# source ... source image (volid or absolute path)
> +# target ... hash with
> +#    storeid => storage ID
> +#    format => disk format (optional)
> +#    options => hash with device options (may or may not contain <storeid>:0)
> +#    device => device where the disk is attached (for example, scsi3) (optional)
> +#
> +# returns ... volid of the allocated disk image (e.g. local-lvm:vm-100-disk-2)
> +my $import_disk_image = sub {
> +    my ($storecfg, $vmid, $vmconf, $source, $target) = @_;
> +    my $requested_format = $target->{format};
> +    my $storeid = $target->{storeid};
> +
> +    die "Source parameter is undefined!" if !defined $source;
> +    $source = PVE::Storage::abs_filesystem_path($storecfg, $source, 1);
> +
> +    eval { PVE::Storage::storage_config($storecfg, $storeid) };
> +    die "Error while importing disk image $source: $@\n" if $@;
> +
> +    my $src_size = PVE::Storage::file_size_info($source);
> +    die "Could not get file size of $source" if !defined($src_size);
> +
> +    $check_format_is_supported->($requested_format, $storeid, $storecfg);
> +
> +    my $dst_format = PVE::QemuServer::resolve_dst_disk_format(
> +	$storecfg,
> +	$storeid,
> +	undef,
> +	$requested_format,
> +    );
> +    my $dst_volid = PVE::Storage::vdisk_alloc(
> +	$storecfg,
> +	$storeid,
> +	$vmid,
> +	$dst_format,
> +	undef,
> +	$src_size / 102,
> +    );
> +
> +    print "Importing disk image '$source' as '$dst_volid'...\n";
> +    eval {
> +	local $SIG{INT} =
> +	local $SIG{TERM} =
> +	local $SIG{QUIT} =
> +	local $SIG{HUP} =
> +	local $SIG{PIPE} = sub { die "Interrupted by signal $!\n"; };
> +
> +	my $zeroinit = PVE::Storage::volume_has_feature(
> +	    $storecfg,
> +	    'sparseinit',
> +	    $dst_volid,
> +	);
> +	PVE::Storage::activate_volumes($storecfg, [$dst_volid]);
> +	PVE::QemuServer::qemu_img_convert(
> +	    $source,
> +	    $dst_volid,
> +	    $src_size,
> +	    undef,
> +	    $zeroinit,
> +	);
> +	PVE::Storage::deactivate_volumes($storecfg, [$dst_volid]);
> +
> +    };
> +    if (my $err = $@) {
> +	eval { PVE::Storage::vdisk_free($storecfg, $dst_volid) };
> +	warn "Cleanup of $dst_volid failed: $@ \n" if $@;
> +
> +	die "Importing disk '$source' failed: $err\n" if $err;
> +    }
> +
> +    $target->{options}->{file} = $dst_volid;
> +    my $options_string = PVE::QemuServer::print_drive($target->{options});
> +    $target->{device} = PVE::QemuConfig->add_unused_volume($vmconf, $dst_volid)
> +	if !$target->{device};
> +
> +    $update_vm_api->(
> +	{
> +	    vmid => $vmid,
> +	    $target->{device} => $options_string,
> +	    skiplock => 1,
> +	    digest => $vmconf->{digest},
> +	},
> +	1,
> +    );
> +
> +    return $dst_volid;
> +};
> +
> +__PACKAGE__->register_method ({
> +    name => 'importdisk',
> +    path => '{vmid}/importdisk',
> +    method => 'POST',
> +    proxyto => 'node',
> +    protected => 1,
> +    description => "Import an external disk image into a VM. The image format ".
> +	"has to be supported by qemu-img.",
> +    parameters => {
> +	additionalProperties => 0,
> +	properties => {
> +	    node => get_standard_option('pve-node'),
> +	    vmid => get_standard_option('pve-vmid',
> +		{completion => \&PVE::QemuServer::complete_vmid}),
> +	    source => {
> +		description => "Disk image to import. Can be a volid ".
> +		    "(local:99/imageToImport.raw) or an absolute path on the server.",
> +		type => 'string',
> +	    },
> +	    device => {
> +		type => 'string',
> +		description => "Bus/Device type of the new disk (e.g. 'ide0', ".
> +		    "'scsi2'). Will add the image as unused disk if omitted.",
> +		enum => [PVE::QemuServer::Drive::valid_drive_names()],
> +		optional => 1,
> +	    },
> +	    device_options => {
> +		type => 'string',
> +		description => "Options to set for the new disk (e.g. 'discard=on,backup=0')",
> +		optional => 1,
> +		requires => 'device',
> +	    },
> +	    storage => get_standard_option('pve-storage-id', {
> +		description => "The storage to which the image will be imported to.",
> +		completion => \&PVE::QemuServer::complete_storage,
> +	    }),
> +	    format => {
> +		type => 'string',
> +		description => 'Target format.',
> +		enum => [ 'raw', 'qcow2', 'vmdk' ],
> +		optional => 1,
> +	    },
> +	    digest => get_standard_option('pve-config-digest'),
> +	},
> +    },
> +    returns => { type => 'string'},
> +    code => sub {
> +	my ($param) = @_;
> +	my $vmid = extract_param($param, 'vmid');
> +	my $node = extract_param($param, 'node');
> +	my $source = extract_param($param, 'source');
> +	my $digest = extract_param($param, 'digest');
> +	my $device_options = extract_param($param, 'device_options');
> +	my $device = extract_param($param, 'device');
> +	my $storeid = extract_param($param, 'storage');
> +
> +	my $rpcenv = PVE::RPCEnvironment::get();
> +	my $authuser = $rpcenv->get_user();
> +	my $storecfg = PVE::Storage::config();
> +	PVE::Storage::storage_config($storecfg, $storeid);
> +
> +
> +	if ($device_options) {
> +	    # $device_options may or may not contain <storeid>:0
> +	    my $parsed = PVE::QemuServer::Drive::parse_drive($device, $device_options);
> +	    if ($parsed) {
> +		raise_param_exc({$device_options => "Invalid import syntax"})
> +		    if !($parsed->{file} =~ $IMPORT_DISK_RE);
> +	    } else {
> +		my $fake = "$storeid:0,$device_options";
> +		$parsed = PVE::QemuServer::Drive::parse_drive($device, $fake);
> +	    }
> +	    delete $parsed->{file};
> +	    delete $parsed->{interface};
> +	    delete $parsed->{index};
> +	    $device_options = $parsed;
> +	}
> +
> +	# Format can be set explicitly "--format vmdk"
> +	# or as part of device options "--device_options discard=on,format=vmdk"
> +	my $format = extract_param($param, 'format');
> +	if ($device_options) {
> +	    raise_param_exc({format => "Format already specified in device_options!"})
> +		if $format && $device_options->{format};
> +	    $format = $format || $device_options->{format}; # may be undefined
> +	}
> +	$check_format_is_supported->($format, $storeid, $storecfg);
> +
> +	# quick checks before fork + lock
> +	my $conf = PVE::QemuConfig->load_config($vmid);
> +	PVE::QemuConfig->check_lock($conf);
> +	PVE::Tools::assert_if_modified($conf->{digest}, $digest);
> +	if ($device && $conf->{$device}) {
> +	    die "Could not import because device $device is already in ".
> +		"use in VM $vmid. Choose a different device!";
> +	}
> +
> +	my $worker = sub {
> +	    PVE::QemuConfig->lock_config($vmid, sub {
> +		$conf = PVE::QemuConfig->load_config($vmid);
> +		PVE::QemuConfig->check_lock($conf);
> +
> +		PVE::Tools::assert_if_modified($conf->{digest}, $digest);
> +		PVE::QemuConfig->set_lock($vmid, 'import');
> +		$conf = PVE::QemuConfig->load_config($vmid);
> +	    });
> +
> +	    my $target = {
> +		node => $node,
> +		storeid => $storeid,
> +	    };
> +	    $target->{format} = $format;
> +	    $target->{device} = $device;
> +	    $target->{options} = $device_options;
> +	    eval { $import_disk_image->($storecfg, $vmid, $conf, $source, $target) };
> +	    my $err = $@;
> +	    eval { PVE::QemuConfig->remove_lock($vmid, 'import') };
> +	    warn $@ if $@;
> +	    die $err if $err;
> +	};
> +	return $rpcenv->fork_worker('importdisk', $vmid, $authuser, $worker);
> +    }});
> +
> +__PACKAGE__->register_method({
> +    name => 'importvm',
> +    path => '{vmid}/importvm',
> +    method => 'POST',
> +    description => "Import a VM from existing disk images.",
> +    protected => 1,
> +    proxyto => 'node',
> +    parameters => {
> +	additionalProperties => 0,
> +	properties => PVE::QemuServer::json_config_properties(
> +	    {
> +		node => get_standard_option('pve-node'),
> +		vmid => get_standard_option('pve-vmid', { completion =>
> +		    \&PVE::Cluster::complete_next_vmid }),
> +		diskimage => {
> +		    description => "\\0 delimited mapping of devices to disk images. For " .
> +			"example, scsi0=/mnt/nfs/image1.vmdk",
> +		    type => 'string',
> +		    format => 'device-image-pair-alist',
> +		},
> +		start => {
> +		    optional => 1,
> +		    type => 'boolean',
> +		    default => 0,
> +		    description => "Start VM after it was imported successfully.",
> +		},
> +	    }),
> +    },
> +    returns => {
> +	type => 'string',
> +    },
> +    code => sub {
> +	my ($param) = @_;
> +	my $node = extract_param($param, 'node');
> +	my $vmid = extract_param($param, 'vmid');
> +	my $diskimages_string = extract_param($param, 'diskimage');
> +	my $boot = extract_param($param, 'boot');
> +	my $start = extract_param($param, 'start');
> +
> +	my $rpcenv = PVE::RPCEnvironment::get();
> +	my $authuser = $rpcenv->get_user();
> +	my $storecfg = PVE::Storage::config();
> +
> +	PVE::Cluster::check_cfs_quorum();
> +
> +	my $import_param = {};
> +	foreach my $opt (keys %$param) {
> +	    next if $opt eq 'efidisk0';
> +	    raise_param_exc({bootdisk => "Deprecated: Use --boot order= instead"})
> +		if $opt eq 'bootdisk';
> +
> +	    if (PVE::QemuServer::Drive::is_valid_drivename($opt)) {
> +		my $drive = PVE::QemuServer::parse_drive($opt, $param->{$opt});
> +		if ($drive->{file} =~ $IMPORT_DISK_RE) {
> +		    $import_param->{$opt} = $drive;
> +		    delete $param->{$opt};
> +		}
> +	    }
> +	}
> +
> +	my $diskimages = {};
> +	foreach my $pair (PVE::Tools::split_list($diskimages_string)) {
> +	    my ($device, $diskimage) = split('=', $pair);
> +	    $diskimages->{$device} = $diskimage;
> +	    raise_param_exc({
> +		$device => "Device '$device' not marked for import, " .
> +		    "but import source '$diskimage' specified",
> +	    }) if !defined($import_param->{$device});
> +	    PVE::Storage::abs_filesystem_path($storecfg, $diskimage, 1);
> +	}
> +
> +	foreach my $device (keys %$import_param) {
> +	    raise_param_exc({
> +		$device => "Device '$device' marked for import, but no source given\n",
> +	    }) if !defined($diskimages->{$device});
> +	}
> +
> +	eval { PVE::QemuConfig->create_and_lock_config($vmid, 0, 'import') };
> +	die "Unable to create config for VM import: $@" if $@;
> +
> +	my $worker = sub {
> +	    my $reload_conf = sub {
> +		my ($vmid) = @_;
> +		my $conf = PVE::QemuConfig->load_config($vmid);
> +		return $conf if PVE::QemuConfig->has_lock($conf, 'import');
> +		die "import lock in VM $vmid config file missing!";
> +	    };
> +
> +	    my $conf = $reload_conf->($vmid);
> +	    $update_vm_api->(
> +		{
> +		    %$param,
> +		    node => $node,
> +		    vmid => $vmid,
> +		    skiplock => 1,
> +		    digest => $conf->{digest},
> +		},
> +		1
> +	    );
> +
> +	    eval {
> +		foreach my $device (keys %$import_param) {
> +		    $conf = $reload_conf->($vmid);
> +		    my $drive = $import_param->{$device};
> +		    my $storeid = PVE::Storage::parse_volume_id($drive->{file});
> +		    my $imported = $import_disk_image->(
> +			$storecfg,
> +			$vmid,
> +			$conf,
> +			$diskimages->{$device},
> +			{
> +			    storeid => $storeid,
> +			    format => $drive->{format},
> +			    options => $drive,
> +			    device => $device,
> +			},
> +		    );
> +		}
> +	    };
> +	    my $err = $@;
> +	    if ($err) {
> +		eval { PVE::QemuServer::destroy_vm($storecfg, $vmid, 1) };
> +		warn "Could not destroy VM $vmid: $@" if "$@";
> +
> +		die "Import failed: $err";
> +	    }
> +
> +	    $conf = $reload_conf->($vmid);
> +	    if (!$boot) {
> +		my $bootdevs = PVE::QemuServer::get_default_bootdevices($conf);
> +		$boot = PVE::QemuServer::print_bootorder($bootdevs);
> +	    }
> +	    $update_vm_api->(
> +		{
> +		    node => $node,
> +		    vmid => $vmid,
> +		    boot => $boot,
> +		    skiplock => 1,
> +		    digest => $conf->{digest},
> +		},
> +		1,
> +	    );
> +
> +	    eval { PVE::QemuConfig->remove_lock($vmid, 'import') };
> +	    warn $@ if $@;
> +
> +	    PVE::QemuServer::vm_start($storecfg, $vmid) if $start;
> +	};
> +
> +	return $rpcenv->fork_worker('importvm', $vmid, $authuser, $worker);
> +    }});
> +
> +
>  1;
> diff --git a/PVE/API2/Qemu/Makefile b/PVE/API2/Qemu/Makefile
> index 5d4abda..bdd4762 100644
> --- a/PVE/API2/Qemu/Makefile
> +++ b/PVE/API2/Qemu/Makefile
> @@ -1,4 +1,4 @@
> -SOURCES=Agent.pm CPU.pm Machine.pm
> +SOURCES=Agent.pm CPU.pm Machine.pm OVF.pm
>  
>  .PHONY: install
>  install:
> diff --git a/PVE/API2/Qemu/OVF.pm b/PVE/API2/Qemu/OVF.pm
> new file mode 100644
> index 0000000..bd6e90b
> --- /dev/null
> +++ b/PVE/API2/Qemu/OVF.pm
> @@ -0,0 +1,68 @@
> +package PVE::API2::Qemu::OVF;
> +
> +use strict;
> +use warnings;
> +
> +use PVE::RESTHandler;
> +use PVE::JSONSchema qw(get_standard_option);
> +use PVE::QemuServer::OVF;
> +
> +use base qw(PVE::RESTHandler);
> +
> +__PACKAGE__->register_method ({
> +    name => 'index',
> +    path => '',
> +    method => 'GET',
> +    proxyto => 'node',
> +    description => "Read an .ovf manifest.",
> +    parameters => {
> +	additionalProperties => 0,
> +	properties => {
> +	    node => get_standard_option('pve-node'),
> +	    manifest => {
> +		description => ".ovf manifest",
> +		type => 'string',
> +	    },
> +	},
> +    },
> +    returns => {
> +	description => "VM config according to .ovf manifest and digest of manifest",
> +	type => "object",
> +    },
> +    returns => {
> +	type => 'object',
> +	additionalProperties => 1,
> +	properties => PVE::QemuServer::json_ovf_properties({
> +	    name => {
> +		type => 'string',
> +		optional => 1,
> +	    },
> +	    cores => {
> +		type => 'integer',
> +		optional => 1,
> +	    },
> +	    memory => {
> +		type => 'integer',
> +		optional => 1,
> +	    },
> +	}),
> +    },
> +    code => sub {
> +	my ($param) = @_;
> +
> +	my $manifest = $param->{manifest};
> +	die "$manifest: non-existent or non-regular file\n" if (! -f $manifest);
> +
> +	my $parsed = PVE::QemuServer::OVF::parse_ovf($manifest, 0, 1);
> +	my $result;
> +	$result->{cores} = $parsed->{qm}->{cores};
> +	$result->{name} =  $parsed->{qm}->{name};
> +	$result->{memory} = $parsed->{qm}->{memory};
> +	my $disks = $parsed->{disks};
> +	foreach my $disk (@$disks) {
> +	    $result->{$disk->{disk_address}} = $disk->{backing_file};
> +	}
> +	return $result;
> +    }});
> +
> +1;
> \ No newline at end of file
> diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
> index 1c0b5c2..131c0b6 100644
> --- a/PVE/QemuServer.pm
> +++ b/PVE/QemuServer.pm
> @@ -300,7 +300,7 @@ my $confdesc = {
>  	optional => 1,
>  	type => 'string',
>  	description => "Lock/unlock the VM.",
> -	enum => [qw(backup clone create migrate rollback snapshot snapshot-delete suspending suspended)],
> +	enum => [qw(backup clone create migrate rollback snapshot snapshot-delete suspending suspended import)],
>      },
>      cpulimit => {
>  	optional => 1,
> @@ -985,19 +985,41 @@ PVE::JSONSchema::register_format('pve-volume-id-or-qm-path', \&verify_volume_id_
>  sub verify_volume_id_or_qm_path {
>      my ($volid, $noerr) = @_;
>  
> -    if ($volid eq 'none' || $volid eq 'cdrom' || $volid =~ m|^/|) {
> -	return $volid;
> -    }
> +    return $volid eq 'none' || $volid eq 'cdrom' ?
> +	$volid :
> +	verify_volume_id_or_absolute_path($volid, $noerr);
> +}
> +
> +PVE::JSONSchema::register_format('pve-volume-id-or-absolute-path', \&verify_volume_id_or_absolute_path);
> +sub verify_volume_id_or_absolute_path {
> +    my ($volid, $noerr) = @_;
> +
> +    return $volid if $volid =~ m|^/|;
>  
> -    # if its neither 'none' nor 'cdrom' nor a path, check if its a volume-id
>      $volid = eval { PVE::JSONSchema::check_format('pve-volume-id', $volid, '') };
>      if ($@) {
> -	return if $noerr;
> +	return undef if $noerr;
>  	die $@;
>      }
>      return $volid;
>  }
>  
> +PVE::JSONSchema::register_format('device-image-pair', \&verify_device_image_pair);
> +sub verify_device_image_pair {
> +    my ($pair, $noerr) = @_;
> +
> +    my $error = sub {
> +	return undef if $noerr;
> +	die $@;
> +    };
> +
> +    my ($device, $image) = split('=', $pair);
> +    $error->("Invalid device '$device'") if !PVE::QemuServer::Drive::is_valid_drivename($device);
> +    $error->("Invalid image '$image'") if !verify_volume_id_or_absolute_path($image);
> +
> +    return $pair;
> +}
> +
>  my $usb_fmt = {
>      host => {
>  	default_key => 1,
> @@ -2030,6 +2052,22 @@ sub json_config_properties {
>      return $prop;
>  }
>  
> +# Properties that we can read from an OVF file
> +sub json_ovf_properties {
> +    my $prop = shift;
> +
> +    foreach my $device ( PVE::QemuServer::Drive::valid_drive_names()) {
> +	$prop->{$device} = {
> +	    type => 'string',
> +	    format => 'pve-volume-id-or-absolute-path',
> +	    description => "Disk image that gets imported to $device",
> +	    optional => 1,
> +	};
> +    }
> +
> +    return $prop;
> +}
> +
>  # return copy of $confdesc_cloudinit to generate documentation
>  sub cloudinit_config_properties {
>  
> @@ -6748,7 +6786,7 @@ sub qemu_img_convert {
>  	$src_path = PVE::Storage::path($storecfg, $src_volid, $snapname);
>  	$src_is_iscsi = ($src_path =~ m|^iscsi://|);
>  	$cachemode = 'none' if $src_scfg->{type} eq 'zfspool';
> -    } elsif (-f $src_volid) {
> +    } elsif (-f $src_volid || -b $src_volid) {
>  	$src_path = $src_volid;
>  	if ($src_path =~ m/\.($PVE::QemuServer::Drive::QEMU_FORMAT_RE)$/) {
>  	    $src_format = $1;
> diff --git a/PVE/QemuServer/OVF.pm b/PVE/QemuServer/OVF.pm
> index c76c199..48146e9 100644
> --- a/PVE/QemuServer/OVF.pm
> +++ b/PVE/QemuServer/OVF.pm
> @@ -87,7 +87,7 @@ sub id_to_pve {
>  
>  # returns two references, $qm which holds qm.conf style key/values, and \@disks
>  sub parse_ovf {
> -    my ($ovf, $debug) = @_;
> +    my ($ovf, $debug, $manifest_only) = @_;
>  
>      my $dom = XML::LibXML->load_xml(location => $ovf, no_blanks => 1);
>  
> @@ -220,9 +220,11 @@ ovf:Item[rasd:InstanceID='%s']/rasd:ResourceType", $controller_id);
>  	    die "error parsing $filepath, file seems not to exist at $backing_file_path\n";
>  	}
>  
> -	my $virtual_size;
> -	if ( !($virtual_size = PVE::Storage::file_size_info($backing_file_path)) ) {
> -	    die "error parsing $backing_file_path, size seems to be $virtual_size\n";
> +	my $virtual_size = undef;
> +	if (!$manifest_only) { # Not possible if manifest is uploaded in web gui
> +	    if ( !($virtual_size = PVE::Storage::file_size_info($backing_file_path)) ) {
> +		die "error parsing $backing_file_path: Could not get file size info: $@\n";
> +	    }
>  	}
>  
>  	$pve_disk = {
> -- 
> 2.20.1
> 
> 
> _______________________________________________
> pve-devel mailing list
> pve-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
> 




^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [pve-devel] [PATCH v7 qemu-server] Add API for import wizards
  2021-03-31 15:12   ` Fabian Grünbichler
@ 2021-04-01 10:19     ` Dominic Jäger
  2021-04-01 11:30       ` Fabian Grünbichler
  0 siblings, 1 reply; 7+ messages in thread
From: Dominic Jäger @ 2021-04-01 10:19 UTC (permalink / raw)
  To: Proxmox VE development discussion

On Wed, Mar 31, 2021 at 05:12:28PM +0200, Fabian Grünbichler wrote:
> this is starting to shape up nicely. as promised, I now took a stab at 
> (roughly!) integrating this into our regular flow (see diff below):
> ....
> 
> - we could likely drop the separate import_disk API call, and let the 
>   `importdisk` CLI command prepare parameters for a regular VM config 
>   update
I think dropping importdisk is a good idea, too.

> I'm sure I've missed some corner cases, as I've only tested create_vm 
> with importing, and not the other newly exposed APIs.

I didn't get the importdisk API to work yet.
But I'd look closer at stuff like that
	if (!$device_options) {
	    $device_options = "$storeid:0"; <=== should be -1
	}
only if we keep it?

One thing is important: That import parameter was at the place of the default storage

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

diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
index 41e1ab7..73af497 100644
--- a/PVE/API2/Qemu.pm
+++ b/PVE/API2/Qemu.pm
@@ -1469,7 +1469,7 @@ my $update_vm_api  = sub {
 		    PVE::QemuServer::vmconfig_register_unused_drive($storecfg, $vmid, $conf, PVE::QemuServer::parse_drive($opt, $conf->{pending}->{$opt}))
 			if defined($conf->{pending}->{$opt});
 
-		    &$create_disks($rpcenv, $authuser, $conf->{pending}, $arch, $storecfg, $vmid, undef, {$opt => $param->{$opt}}, {$opt => $import_devices->{$opt}});
+		    &$create_disks($rpcenv, $authuser, $conf->{pending}, $arch, $storecfg, $vmid, undef, {$opt => $param->{$opt}}, undef, {$opt => $import_devices->{$opt}});
 		} elsif ($opt =~ m/^serial\d+/) {
 		    if ((!defined($conf->{$opt}) || $conf->{$opt} eq 'socket') && $param->{$opt} eq 'socket') {
 			$rpcenv->check_vm_perm($authuser, $vmid, undef, ['VM.Config.HWType']);


> stat shows how much boilerplace/duplication this removes, although there 
> is probably even more potential here since `create_vm` partly duplicates 
> the `update_vm_api` sub that ends up calling `create_disks`:
Is that urgent?




^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [pve-devel] [PATCH v7 qemu-server] Add API for import wizards
  2021-04-01 10:19     ` Dominic Jäger
@ 2021-04-01 11:30       ` Fabian Grünbichler
  0 siblings, 0 replies; 7+ messages in thread
From: Fabian Grünbichler @ 2021-04-01 11:30 UTC (permalink / raw)
  To: Dominic Jäger, Proxmox VE development discussion

On April 1, 2021 12:19 pm, Dominic Jäger wrote:
> On Wed, Mar 31, 2021 at 05:12:28PM +0200, Fabian Grünbichler wrote:
>> this is starting to shape up nicely. as promised, I now took a stab at 
>> (roughly!) integrating this into our regular flow (see diff below):
>> ....
>> 
>> - we could likely drop the separate import_disk API call, and let the 
>>   `importdisk` CLI command prepare parameters for a regular VM config 
>>   update
> I think dropping importdisk is a good idea, too.
> 
>> I'm sure I've missed some corner cases, as I've only tested create_vm 
>> with importing, and not the other newly exposed APIs.
> 
> I didn't get the importdisk API to work yet.
> But I'd look closer at stuff like that
> 	if (!$device_options) {
> 	    $device_options = "$storeid:0"; <=== should be -1
> 	}
> only if we keep it?
> 
> One thing is important: That import parameter was at the place of the default storage

yes, I just roughly moved the code to see whether the approach works out 
and then tested a single entry point. if there are no objections into 
merging it like that, it would definitely need a cleanup and more 
testing :)

> 
>  PVE/API2/Qemu.pm | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
> index 41e1ab7..73af497 100644
> --- a/PVE/API2/Qemu.pm
> +++ b/PVE/API2/Qemu.pm
> @@ -1469,7 +1469,7 @@ my $update_vm_api  = sub {
>  		    PVE::QemuServer::vmconfig_register_unused_drive($storecfg, $vmid, $conf, PVE::QemuServer::parse_drive($opt, $conf->{pending}->{$opt}))
>  			if defined($conf->{pending}->{$opt});
>  
> -		    &$create_disks($rpcenv, $authuser, $conf->{pending}, $arch, $storecfg, $vmid, undef, {$opt => $param->{$opt}}, {$opt => $import_devices->{$opt}});
> +		    &$create_disks($rpcenv, $authuser, $conf->{pending}, $arch, $storecfg, $vmid, undef, {$opt => $param->{$opt}}, undef, {$opt => $import_devices->{$opt}});
>  		} elsif ($opt =~ m/^serial\d+/) {
>  		    if ((!defined($conf->{$opt}) || $conf->{$opt} eq 'socket') && $param->{$opt} eq 'socket') {
>  			$rpcenv->check_vm_perm($authuser, $vmid, undef, ['VM.Config.HWType']);
> 
> 
>> stat shows how much boilerplace/duplication this removes, although there 
>> is probably even more potential here since `create_vm` partly duplicates 
>> the `update_vm_api` sub that ends up calling `create_disks`:
> Is that urgent?

no, it's just something I noticed since I added the same code in the 
same context in two places ;)




^ permalink raw reply	[flat|nested] 7+ messages in thread

* [pve-devel] applied: [PATCH v7 storage] Optionally allow blockdev in abs_filesystem_path
  2021-03-26 12:32 [pve-devel] [PATCH v7 storage] Optionally allow blockdev in abs_filesystem_path Dominic Jäger
  2021-03-26 12:32 ` [pve-devel] [PATCH v7 qemu-server] Add API for import wizards Dominic Jäger
  2021-03-26 12:32 ` [pve-devel] [PATCH v7 manager] gui: Add import for disk & VM Dominic Jäger
@ 2021-04-01 13:40 ` Thomas Lamprecht
  2 siblings, 0 replies; 7+ messages in thread
From: Thomas Lamprecht @ 2021-04-01 13:40 UTC (permalink / raw)
  To: Proxmox VE development discussion, Dominic Jäger

On 26.03.21 13:32, Dominic Jäger wrote:
> This is required to import from LVM storages, for example
> 
> Signed-off-by: Dominic Jäger <d.jaeger@proxmox.com>
> ---
> v6->v7: Feedback Fabian G
>  - Variables with _ instead of camelCase
>  - single if instead of if/else
> 
>  PVE/Storage.pm | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
>

applied, thanks!




^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2021-04-01 13:40 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-26 12:32 [pve-devel] [PATCH v7 storage] Optionally allow blockdev in abs_filesystem_path Dominic Jäger
2021-03-26 12:32 ` [pve-devel] [PATCH v7 qemu-server] Add API for import wizards Dominic Jäger
2021-03-31 15:12   ` Fabian Grünbichler
2021-04-01 10:19     ` Dominic Jäger
2021-04-01 11:30       ` Fabian Grünbichler
2021-03-26 12:32 ` [pve-devel] [PATCH v7 manager] gui: Add import for disk & VM Dominic Jäger
2021-04-01 13:40 ` [pve-devel] applied: [PATCH v7 storage] Optionally allow blockdev in abs_filesystem_path Thomas Lamprecht

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal