* [pve-devel] [PATCH v6 storage] Optionally allow blockdev in abs_filesystem_path
@ 2021-03-09 10:43 Dominic Jäger
2021-03-09 10:43 ` [pve-devel] [PATCH v6 qemu-server] Add API for import wizards Dominic Jäger
` (2 more replies)
0 siblings, 3 replies; 5+ messages in thread
From: Dominic Jäger @ 2021-03-09 10:43 UTC (permalink / raw)
To: pve-devel
This is required to import from LVM storages
Signed-off-by: Dominic Jäger <d.jaeger@proxmox.com>
---
v5->v6: unchanged
PVE/Storage.pm | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/PVE/Storage.pm b/PVE/Storage.pm
index 8ee2c92..7c2e24e 100755
--- a/PVE/Storage.pm
+++ b/PVE/Storage.pm
@@ -609,7 +609,7 @@ sub path {
}
sub abs_filesystem_path {
- my ($cfg, $volid) = @_;
+ my ($cfg, $volid, $allowBlockdev) = @_;
my $path;
if (parse_volume_id ($volid, 1)) {
@@ -623,8 +623,11 @@ sub abs_filesystem_path {
}
}
}
-
- die "can't find file '$volid'\n" if !($path && -f $path);
+ if ($allowBlockdev) {
+ die "can't find file '$volid'\n" if !($path && (-f $path || -b $path));
+ } else {
+ die "can't find file '$volid'\n" if !($path && -f $path);
+ }
return $path;
}
--
2.20.1
^ permalink raw reply [flat|nested] 5+ messages in thread
* [pve-devel] [PATCH v6 qemu-server] Add API for import wizards
2021-03-09 10:43 [pve-devel] [PATCH v6 storage] Optionally allow blockdev in abs_filesystem_path Dominic Jäger
@ 2021-03-09 10:43 ` Dominic Jäger
[not found] ` <<20210309104318.317454-2-d.jaeger@proxmox.com>
2021-03-09 10:43 ` [pve-devel] [PATCH v6 manager] gui: Add import for disk & VM Dominic Jäger
[not found] ` <<20210309104318.317454-1-d.jaeger@proxmox.com>
2 siblings, 1 reply; 5+ messages in thread
From: Dominic Jäger @ 2021-03-09 10:43 UTC (permalink / raw)
To: pve-devel
Extend qm importdisk/importovf functionality to the API.
Signed-off-by: Dominic Jäger <d.jaeger@proxmox.com>
---
v5->v6:
More parsing
Fix regex
Improve --boot handling
Move readovf from manager to qemu-server (like CPU)
Create properties helper for readovf return values
PVE/API2/Qemu.pm | 458 ++++++++++++++++++++++++++++++++++++++++-
PVE/API2/Qemu/Makefile | 2 +-
PVE/API2/Qemu/OVF.pm | 68 ++++++
PVE/QemuServer.pm | 32 ++-
PVE/QemuServer/OVF.pm | 10 +-
5 files changed, 562 insertions(+), 8 deletions(-)
create mode 100644 PVE/API2/Qemu/OVF.pm
diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
index 6706b55..a689c9e 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);
@@ -4383,4 +4382,461 @@ __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 "You have to provide storage ID" if !$storeid;
+ die "You have to provide the storage configurration" 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 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 => string 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 ($param) = @_;
+ my $storecfg = $param->{storecfg};
+ my $vmid = $param->{vmid};
+ my $vmconf = $param->{vmconf};
+ my $target = $param->{target};
+ my $requested_format = $target->{format};
+ my $storeid = $target->{storeid};
+
+ die "Source parameter is undefined!" if !defined $param->{source};
+ my $source = PVE::Storage::abs_filesystem_path($storecfg, $param->{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);
+ # Previous abs_filesystem_path performs additional checks
+ 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 / 1024);
+
+ print "Importing disk image '$source'...\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;
+ }
+
+ my $drive = $dst_volid;
+ if ($target->{device}) {
+ # Attach to target device with options if they are specified
+ if (defined $target->{options}) {
+ # Options string with or without storeid is allowed
+ # => Avoid potential duplicate storeid for update
+ $target->{options} =~ s/$storeid:0,?//; # ? for if only storeid:0 present
+ $drive .= ",$target->{options}" ;
+ }
+ } else {
+ $target->{device} = PVE::QemuConfig->add_unused_volume($vmconf, $dst_volid);
+ }
+ print "Imported '$source' to $dst_volid\n";
+ $update_vm_api->(
+ {
+ node => $target->{node},
+ vmid => $vmid,
+ $target->{device} => $drive,
+ skiplock => 1,
+ },
+ 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_param = 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); # check for errors
+
+ # Format can be set explicitly "--format vmdk"
+ # or as part of device options "--device_options discard=on,format=vmdk"
+ # or not at all, but not both together
+ my $device_options_format;
+ if ($device_options) {
+ # parse device_options string according to disk schema for
+ # validation and to make usage easier
+
+ # any existing storage ID is OK to get a valid (fake) string for parse_drive
+ my $valid_string = "$storeid:0,$device_options";
+
+ # member "file" is fake
+ my $drive_full = PVE::QemuServer::Drive::parse_drive($device, $valid_string);
+ $device_options_format = $drive_full->{format};
+ }
+
+ my $format = extract_param($param, 'format'); # may be undefined
+ if ($device_options) {
+ if ($format && $device_options_format) {
+ raise_param_exc({format => "Format already specified in device_options!"});
+ } else {
+ $format = $format || $device_options_format; # may still be undefined
+ }
+ }
+
+ $check_format_is_supported->($format, $storeid, $storecfg);
+
+ # provide a useful error (in the API response) before forking
+ my $no_lock_conf = PVE::QemuConfig->load_config($vmid);
+ PVE::QemuConfig->check_lock($no_lock_conf);
+ PVE::Tools::assert_if_modified($no_lock_conf->{digest}, $digest_param);
+ if ($device && $no_lock_conf->{$device}) {
+ die "Could not import because device $device is already in ".
+ "use in VM $vmid. Choose a different device!";
+ }
+
+ my $worker = sub {
+ my $conf;
+ PVE::QemuConfig->lock_config($vmid, sub {
+ $conf = PVE::QemuConfig->load_config($vmid);
+ PVE::QemuConfig->check_lock($conf);
+
+ # Our device-in-use check may be invalid if the new conf is different
+ PVE::Tools::assert_if_modified($conf->{digest}, $no_lock_conf->{digest});
+
+ PVE::QemuConfig->set_lock($vmid, 'import');
+ });
+
+ my $target = {
+ node => $node,
+ storeid => $storeid,
+ };
+ # Avoid keys with undef values
+ $target->{format} = $format if defined $format;
+ $target->{device} = $device if defined $device;
+ $target->{options} = $device_options if defined $device_options;
+ $import_disk_image->({
+ storecfg => $storecfg,
+ vmid => $vmid,
+ vmconf => $conf,
+ source => $source,
+ target => $target,
+ });
+
+ PVE::QemuConfig->remove_lock($vmid, 'import');
+ };
+ 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 }),
+ diskimages => {
+ description => "Mapping of devices to disk images. For " .
+ "example, scsi0=/mnt/nfs/image1.vmdk,scsi1=/mnt/nfs/image2",
+ type => 'string',
+ },
+ 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, 'diskimages');
+ my $boot = extract_param($param, 'boot');
+
+ my $rpcenv = PVE::RPCEnvironment::get();
+ my $authuser = $rpcenv->get_user();
+ my $storecfg = PVE::Storage::config();
+
+ PVE::Cluster::check_cfs_quorum();
+
+ # Return true iff $opt is to be imported, that means it is a device
+ # like scsi2 and the special import syntax/zero is specified for it,
+ # for example local-lvm:0 but not local-lvm:5
+ my $is_import = sub {
+ my ($opt) = @_;
+ return 0 if $opt eq 'efidisk0';
+ if (PVE::QemuServer::Drive::is_valid_drivename($opt)) {
+ my $drive = PVE::QemuServer::parse_drive($opt, $param->{$opt});
+ return $drive->{file} =~ m/0$/;
+ }
+ return 0;
+ };
+
+ # bus/device like ide0, scsi5 where the imported disk images get attached
+ my $target_devices = [];
+ # List of VM parameters like memory, cpu type, but also disks that are newly created
+ my $vm_params = [];
+ foreach my $opt (keys %$param) {
+ next if ($opt eq 'start'); # does not belong in config
+ # New function, so we can forbid deprecated
+ raise_param_exc({bootdisk => "Deprecated: Use --boot order= instead"})
+ if $opt eq 'bootdisk';
+
+ my $list = $is_import->($opt) ? $target_devices : $vm_params;
+ push @$list, $opt;
+ }
+
+ my $diskimages = {};
+ foreach (split ',', $diskimages_string) {
+ my ($device, $diskimage) = split('=', $_);
+ $diskimages->{$device} = $diskimage;
+ if (defined $param->{$device}) {
+ my $drive = PVE::QemuServer::parse_drive($device, $param->{$device});
+ $drive->{file} =~ m/(\d)$/;
+ if ($1 != 0) {
+ raise_param_exc({
+ $device => "Each entry of --diskimages must have a ".
+ "corresponding device with special import syntax " .
+ "(e.g. --scsi3 local-lvm:0). $device from " .
+ "--diskimages has a matching device but the size " .
+ "of that is $1 instead of 0!",
+ });
+ }
+ } else {
+ # It is possible to also create new empty disk images during
+ # import by adding something like scsi2=local:10, therefore
+ # vice-versa check is not required
+ raise_param_exc({
+ diskimages => "There must be a matching device for each " .
+ "--diskimages entry that should be imported, but " .
+ "there is no matching device for $device!\n" .
+ " For example, for --diskimages scsi0=/source/path,scsi1=/other/path " .
+ "there must be --scsi0 local-lvm:0,discard=on --scsi1 local:0,cache=unsafe",
+ });
+ }
+ # Dies if $diskimage cannot be found
+ PVE::Storage::abs_filesystem_path($storecfg, $diskimage, 1);
+ }
+ foreach my $device (@$target_devices) {
+ my $drive = PVE::QemuServer::parse_drive($device, $param->{$device});
+ if ($drive->{file} =~ m/0$/) {
+ if (!defined $diskimages->{$device}) {
+ raise_param_exc({
+ $device => "Each device with the special import " .
+ "syntax (the 0) must have a corresponding in " .
+ "--diskimages that specifies the source of the " .
+ "import, but there is no such entry for $device!",
+ });
+ }
+ }
+ }
+
+ # After devices are ensured to be correct
+ if ($boot) {
+ my $new_bootcfg = PVE::JSONSchema::parse_property_string('pve-qm-boot', $boot);
+ if ($new_bootcfg->{order}) {
+ my @devs = PVE::Tools::split_list($new_bootcfg->{order});
+ for my $dev (@devs) {
+ my $will_be_imported = grep (/^$dev$/, @$target_devices);
+ my $will_be_created = grep (/^$dev$/, @$vm_params);
+ if ( !($will_be_imported || $will_be_created)) {
+ raise_param_exc({boot => "$dev will be neither imported " .
+ "nor created, so it cannot be a boot device!"});
+ }
+ }
+ } else {
+ raise_param_exc({boot => "Deprecated: Use --boot order= instead"});
+ }
+ }
+
+ eval { PVE::QemuConfig->create_and_lock_config($vmid, 0, 'import') };
+ die "Unable to create config for VM import: $@" if $@;
+
+ my $worker = sub {
+ my $get_conf = sub {
+ my ($vmid) = @_;
+ PVE::QemuConfig->lock_config($vmid, sub {
+ my $conf = PVE::QemuConfig->load_config($vmid);
+ if (PVE::QemuConfig->has_lock($conf, 'import')) {
+ return $conf;
+ } else {
+ die "import lock in VM $vmid config file missing!";
+ }
+ });
+ };
+
+ $get_conf->($vmid); # quick check for lock
+
+ my $short_update = {
+ node => $node,
+ vmid => $vmid,
+ skiplock => 1,
+ };
+ foreach ( @$vm_params ) {
+ $short_update->{$_} = $param->{$_};
+ }
+ $update_vm_api->($short_update, 1); # writes directly in config file
+
+ my $conf = $get_conf->($vmid);
+
+ # When all short updates were succesfull, then the long imports
+ my @imported_successfully = ();
+ eval { foreach my $device (@$target_devices) {
+ my $param_parsed = PVE::QemuServer::parse_drive($device, $param->{$device});
+ die "Parsing $param->{$device} failed" if !$param_parsed;
+ my $storeid = PVE::Storage::parse_volume_id($param_parsed->{file});
+
+ my $imported = $import_disk_image->({
+ storecfg => $storecfg,
+ vmid => $vmid,
+ vmconf => $conf,
+ source => $diskimages->{$device},
+ target => {
+ storeid => $storeid,
+ format => $param_parsed->{format},
+ options => $param->{$device},
+ device => $device,
+ },
+ });
+ push @imported_successfully, $imported;
+ }};
+ my $err = $@;
+ if ($err) {
+ foreach my $volid (@imported_successfully) {
+ eval { PVE::Storage::vdisk_free($storecfg, $volid) };
+ warn $@ if $@;
+ }
+
+ eval {
+ my $conffile = PVE::QemuConfig->config_file($vmid);
+ unlink($conffile) or die "Failed to remove config file: $!";
+ };
+ warn $@ if $@;
+
+ die "Import aborted: $err";
+ }
+ if (!$boot) {
+ $conf = $get_conf->($vmid); # import_disk_image changed config file directly
+ 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,
+ },
+ 1,
+ );
+
+ eval { PVE::QemuConfig->remove_lock($vmid, 'import') };
+ warn $@ if $@;
+
+ if ($param->{start}) {
+ PVE::QemuServer::vm_start($storecfg, $vmid);
+ }
+ };
+
+ 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 1410ecb..d4017b7 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,
@@ -998,6 +998,18 @@ sub verify_volume_id_or_qm_path {
return $volid;
}
+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) = @_;
+
+ # Exactly these 2 are allowed in id_or_qm_path but should not be allowed here
+ if ($volid eq 'none' || $volid eq 'cdrom') {
+ return undef if $noerr;
+ die "Invalid format! Should be volume ID or absolute path.";
+ }
+ return verify_volume_id_or_qm_path($volid, $noerr);
+}
+
my $usb_fmt = {
host => {
default_key => 1,
@@ -2030,6 +2042,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-qm-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 {
@@ -6722,7 +6750,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 _) { # -b required to import from LVM images
$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..36b7fff 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, $ignore_size) = @_;
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 = 0;
+ if (!$ignore_size) { # 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] 5+ messages in thread
* [pve-devel] [PATCH v6 manager] gui: Add import for disk & VM
2021-03-09 10:43 [pve-devel] [PATCH v6 storage] Optionally allow blockdev in abs_filesystem_path Dominic Jäger
2021-03-09 10:43 ` [pve-devel] [PATCH v6 qemu-server] Add API for import wizards Dominic Jäger
@ 2021-03-09 10:43 ` Dominic Jäger
[not found] ` <<20210309104318.317454-1-d.jaeger@proxmox.com>
2 siblings, 0 replies; 5+ messages in thread
From: Dominic Jäger @ 2021-03-09 10:43 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>
---
v5->v6:
Rebase
Move readovf to qemu-server
PVE/API2/Nodes.pm | 7 +
www/manager6/Makefile | 2 +
www/manager6/Workspace.js | 15 +
www/manager6/form/ControllerSelector.js | 15 +
| 13 +
www/manager6/qemu/HDEdit.js | 194 ++++++++++++-
www/manager6/qemu/HardwareView.js | 25 ++
www/manager6/qemu/ImportWizard.js | 356 ++++++++++++++++++++++++
www/manager6/qemu/MultiHDEdit.js | 282 +++++++++++++++++++
www/manager6/window/Wizard.js | 2 +
10 files changed, 897 insertions(+), 14 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/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;
--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..5d039134 100644
--- a/www/manager6/qemu/HDEdit.js
+++ b/www/manager6/qemu/HDEdit.js
@@ -8,6 +8,10 @@ Ext.define('PVE.qemu.HDInputPanel', {
unused: false, // ADD usused disk imaged
+ showSourcePathTextfield: false, // to import a disk from an aritrary path
+
+ returnSingleKey: true, // {vmid}/importdisk expects multiple keys => false
+
vmconfig: {}, // used to select usused disks
viewModel: {},
@@ -58,6 +62,29 @@ Ext.define('PVE.qemu.HDInputPanel', {
},
},
+ /*
+ 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 selectionf or each HDInputPanel => Make
+ names so that those in one HDInputPanel are equal but different from other
+ HDInputPanels
+ */
+ getSourceTypeIdentifier() {
+ return 'sourceType_' + this.id;
+ },
+
+ // values ... the values from onGetValues
+ getSourceValue: function(values) {
+ let result;
+ let type = values[this.getSourceTypeIdentifier()];
+ if (type === 'storage') {
+ result = values.sourceVolid;
+ } else {
+ result = values.sourcePath;
+ }
+ return result;
+ },
+
onGetValues: function(values) {
var me = this;
@@ -68,8 +95,12 @@ Ext.define('PVE.qemu.HDInputPanel', {
me.drive.file = me.vmconfig[values.unusedId];
confid = values.controller + values.deviceid;
} else if (me.isCreate) {
+ // disk format & size should not be part of propertyString for import
if (values.hdimage) {
me.drive.file = values.hdimage;
+ } else if (me.isImport) {
+ me.drive.file = `${values.hdstorage}:0`; // so that API allows it
+ me.test = `test`;
} else {
me.drive.file = values.hdstorage + ":" + values.disksize;
}
@@ -83,16 +114,31 @@ 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);
- });
-
-
- params[confid] = PVE.Parser.printQemuDrive(me.drive);
+ });
+ if (me.returnSingleKey) {
+ if (me.isImport) {
+ me.drive.importsource = this.getSourceValue(values);
+ params.diskimages = [confid, me.drive.importsource].join('=');
+ }
+ delete me.drive.importsource;
+ params[confid] = PVE.Parser.printQemuDrive(me.drive);
+ } else {
+ delete me.drive.file;
+ delete me.drive.format;
+ params.device_options = PVE.Parser.printPropertyString(me.drive);
+ params.source = this.getSourceValue(values);
+ params.device = values.controller + values.deviceid;
+ params.storage = values.hdstorage;
+ if (values.diskformat) {
+ params.format = values.diskformat;
+ }
+ }
return params;
},
@@ -149,10 +195,16 @@ 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);
me.down('#hdimage').setStorage(undefined, nodename);
+ // me.down('#sourceStorageSelector').setNodename(nodename);
+ // me.down('#sourceFileSelector').setNodename(nodename);
},
initComponent: function() {
@@ -168,11 +220,18 @@ Ext.define('PVE.qemu.HDInputPanel', {
me.advancedColumn1 = [];
me.advancedColumn2 = [];
+
+ let nodename = me.nodename;
if (!me.confid || me.unused) {
+ let controllerColumn = me.showSourcePathTextfield ? 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.showSourcePathTextfield) {
+ me.bussel.fieldLabel = 'Target Device';
+ }
+ controllerColumn.push(me.bussel);
me.scsiController = Ext.create('Ext.form.field.Display', {
fieldLabel: gettext('SCSI Controller'),
@@ -184,7 +243,7 @@ Ext.define('PVE.qemu.HDInputPanel', {
submitValue: false,
hidden: true,
});
- me.column1.push(me.scsiController);
+ controllerColumn.push(me.scsiController);
}
if (me.unused) {
@@ -199,14 +258,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.showSourcePathTextfield) {
+ let selector = {
xtype: 'pveDiskStorageSelector',
storageContent: 'images',
name: 'disk',
nodename: me.nodename,
- autoSelect: me.insideWizard,
- });
+ hideSize: me.showSourcePathTextfield,
+ autoSelect: me.insideWizard || me.showSourcePathTextfield,
+ };
+ if (me.showSourcePathTextfield) {
+ selector.storageLabel = gettext('Target storage');
+ me.column2.push(selector);
+ } else {
+ me.column1.push(selector);
+ }
} else {
me.column1.push({
xtype: 'textfield',
@@ -217,6 +283,12 @@ Ext.define('PVE.qemu.HDInputPanel', {
});
}
+ if (me.showSourcePathTextfield) {
+ me.column2.push({
+ xtype: 'box',
+ autoEl: { tag: 'hr' },
+ });
+ }
me.column2.push(
{
xtype: 'CacheTypeSelector',
@@ -231,6 +303,90 @@ Ext.define('PVE.qemu.HDInputPanel', {
name: 'discard',
},
);
+ if (me.showSourcePathTextfield) {
+ let show = (element, value) => {
+ element.setHidden(!value);
+ element.setDisabled(!value);
+ };
+
+ me.column1.unshift(
+ {
+ xtype: 'radiofield',
+ itemId: 'sourceRadioStorage',
+ name: me.getSourceTypeIdentifier(),
+ inputValue: 'storage',
+ boxLabel: gettext('Use a storage as source'),
+ hidden: Proxmox.UserName !== 'root@pam',
+ checked: true,
+ listeners: {
+ change: (_, newValue) => {
+ let storageSelectors = [
+ me.down('#sourceStorageSelector'),
+ me.down('#sourceFileSelector'),
+ ];
+ for (const selector of storageSelectors) {
+ show(selector, newValue);
+ }
+ },
+ },
+ }, {
+ xtype: 'pveStorageSelector',
+ itemId: 'sourceStorageSelector',
+ name: 'inputImageStorage',
+ nodename: 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: 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.getSourceTypeIdentifier(),
+ inputValue: 'path',
+ boxLabel: gettext('Use an absolute path as source'),
+ hidden: Proxmox.UserName !== 'root@pam',
+ listeners: {
+ change: (_, newValue) => {
+ show(me.down('#sourcePathTextfield'), newValue);
+ },
+ },
+ }, {
+ xtype: 'textfield',
+ itemId: 'sourcePathTextfield',
+ fieldLabel: gettext('Source Path'),
+ name: 'sourcePath',
+ autoEl: {
+ tag: 'div',
+ // 'data-qtip': gettext('Absolute path or URL to the source disk image, for example: /home/user/somedisk.qcow2, http://example.com/WindowsImage.zip'),
+ 'data-qtip': gettext('Absolute path to the source disk image, for example: /home/user/somedisk.qcow2'),
+ },
+ hidden: true,
+ disabled: true,
+ validator: (insertedText) =>
+ insertedText.startsWith('/') || insertedText.startsWith('http') ||
+ gettext('Must be an absolute path or URL'),
+ },
+ );
+ }
me.advancedColumn1.push(
{
@@ -373,13 +529,20 @@ Ext.define('PVE.qemu.HDEdit', {
nodename: nodename,
unused: unused,
isCreate: me.isCreate,
+ showSourcePathTextfield: me.isImport,
+ isImport: me.isImport,
+ returnSingleKey: !me.isImport,
});
var subject;
if (unused) {
me.subject = gettext('Unused Disk');
+ } else if (me.isImport) {
+ 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 +567,9 @@ Ext.define('PVE.qemu.HDEdit', {
ipanel.setDrive(drive);
me.isValid(); // trigger validation
}
+ if (me.isImport) {
+ me.url = me.url.replace(/\/config$/, "/importdisk");
+ }
},
});
},
diff --git a/www/manager6/qemu/HardwareView.js b/www/manager6/qemu/HardwareView.js
index 470baa46..4d18d385 100644
--- a/www/manager6/qemu/HardwareView.js
+++ b/www/manager6/qemu/HardwareView.js
@@ -421,6 +421,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,
+ isImport: true,
+ listeners: {
+ add: function(_, component) {
+ component.down('#sourceRadioStorage').setValue(true);
+ component.down('#sourceStorageSelector').setHidden(false);
+ component.down('#sourceFileSelector').setHidden(false);
+ component.down('#sourceFileSelector').enable();
+ component.down('#sourceStorageSelector').enable();
+ },
+ },
+ });
+ win.on('destroy', me.reload, me);
+ win.show();
+ },
+ });
+
var remove_btn = new Proxmox.button.Button({
text: gettext('Remove'),
defaultText: gettext('Remove'),
@@ -749,6 +773,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..2aabe74e
--- /dev/null
+++ b/www/manager6/qemu/ImportWizard.js
@@ -0,0 +1,356 @@
+/*jslint confusion: true*/
+Ext.define('PVE.qemu.ImportWizard', {
+ extend: 'PVE.window.Wizard',
+ alias: 'widget.pveQemuImportWizard',
+ mixins: ['Proxmox.Mixin.CBind'],
+
+ viewModel: {
+ data: {
+ nodename: '',
+ current: {
+ scsihw: '', // TODO there is some error with apply after render_scsihw??
+ },
+ },
+ },
+
+ cbindData: {
+ nodename: undefined,
+ },
+
+ subject: gettext('Import Virtual Machine'),
+
+ isImport: 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: [
+ // { // TODO implement the rest
+ // xtype: 'filebutton',
+ // text: gettext('Load local manifest ...'),
+ // allowBlank: true,
+ // hidden: Proxmox.UserName !== 'root@pam',
+ // disabled: Proxmox.UserName !== 'root@pam',
+ // listeners: {
+ // change: (button,event,) => {
+ // var reader = new FileReader();
+ // let wizard = button.up('window');
+ // reader.onload = (e) => {
+ // let uploaded_ovf = e.target.result;
+ // // TODO set fields here
+ // // TODO When to upload disks to server?
+ // };
+ // reader.readAsText(event.target.files[0]);
+ // button.disable(); // TODO implement complete reload
+ // wizard.down('#successTextfield').show();
+ // }
+ // }
+ // },
+ {
+ 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) {
+ if (value && !value.startsWith('/')) {
+ return gettext("Must start with /");
+ }
+ return true;
+ },
+ },
+ {
+ 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..403ad6df
--- /dev/null
+++ b/www/manager6/qemu/MultiHDEdit.js
@@ -0,0 +1,282 @@
+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%', // TODO Resize to parent
+ }, {
+ xtype: 'container',
+ layout: 'hbox',
+ center: true, // TODO fix me
+ 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,
+ isImport: true,
+ showSourcePathTextfield: 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);
+ } else {
+ // TODO Add tooltip to Remove disk button
+ }
+ },
+ },
+ ],
+
+ 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 (static function so to say)
+ 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] 5+ messages in thread
* Re: [pve-devel] [PATCH v6 qemu-server] Add API for import wizards
[not found] ` <<20210309104318.317454-2-d.jaeger@proxmox.com>
@ 2021-03-15 9:25 ` Fabian Grünbichler
0 siblings, 0 replies; 5+ messages in thread
From: Fabian Grünbichler @ 2021-03-15 9:25 UTC (permalink / raw)
To: Proxmox VE development discussion
still a few more comments inline - but this is taking up shape (and the
next iteration will be shorter ;)).
On March 9, 2021 11:43 am, Dominic Jäger wrote:
> Extend qm importdisk/importovf functionality to the API.
>
> Signed-off-by: Dominic Jäger <d.jaeger@proxmox.com>
>
> ---
> v5->v6:
> More parsing
> Fix regex
> Improve --boot handling
> Move readovf from manager to qemu-server (like CPU)
> Create properties helper for readovf return values
>
> PVE/API2/Qemu.pm | 458 ++++++++++++++++++++++++++++++++++++++++-
> PVE/API2/Qemu/Makefile | 2 +-
> PVE/API2/Qemu/OVF.pm | 68 ++++++
> PVE/QemuServer.pm | 32 ++-
> PVE/QemuServer/OVF.pm | 10 +-
> 5 files changed, 562 insertions(+), 8 deletions(-)
> create mode 100644 PVE/API2/Qemu/OVF.pm
>
> diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
> index 6706b55..a689c9e 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);
>
> @@ -4383,4 +4382,461 @@ __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 "You have to provide storage ID" if !$storeid;
> + die "You have to provide the storage configurration" if !$storecfg;
these two are basic sanity checks - they can only trigger if the code is
wrong. the 'You' is this potentially misleading. also, typo in the
second message ;)
> +
> + return if !$format;
> +
> + my (undef, $valid_formats) = PVE::Storage::storage_default_format($storecfg, $storeid);
> + my $supported = grep { $_ eq $format } @$valid_formats;
> +
> + die "$format is not supported on storage $storeid" if !$supported;
format '$format', else this could read strange.
> +};
> +
> +# 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 => string 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 ($param) = @_;
you misunderstood my comments here - I really meant
my ($storecfg, $vmid, $conf, $source, $target) = @_;
putting everything into a single param hash is usually a sign of "need
to refactor this", and tricks you into thinking "bolting on
yet-another-parameter is fine" ;)
> + my $storecfg = $param->{storecfg};
> + my $vmid = $param->{vmid};
> + my $vmconf = $param->{vmconf};
> + my $target = $param->{target};
> + my $requested_format = $target->{format};
> + my $storeid = $target->{storeid};
> +
> + die "Source parameter is undefined!" if !defined $param->{source};
> + my $source = PVE::Storage::abs_filesystem_path($storecfg, $param->{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);
> + # Previous abs_filesystem_path performs additional checks
this comment doesn't tell me anything. IF you really want, add a comment
above abs_file_system_path like
# Check foo, bar and baz
abs_file_system_path
# Check storage exists
storage_config
file_size_info
> + 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 / 1024);
wrong continuation/indentation/wrapping for both
> +
> + print "Importing disk image '$source'...\n";
as '$dst_volid'..
?
> + 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);
same indentation issue
> +
> + PVE::Storage::activate_volumes($storecfg, [$dst_volid]);
> + PVE::QemuServer::qemu_img_convert($source, $dst_volid,
> + $src_size, undef, $zeroinit);
same
> + 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;
> + }
> +
> + my $drive = $dst_volid;
> + if ($target->{device}) {
> + # Attach to target device with options if they are specified
> + if (defined $target->{options}) {
> + # Options string with or without storeid is allowed
> + # => Avoid potential duplicate storeid for update
> + $target->{options} =~ s/$storeid:0,?//; # ? for if only storeid:0 present
> + $drive .= ",$target->{options}" ;
please use parsed options (by passing them in from the call site instead
of the plain string - currently, the same thing gets parsed again and
again, which is not needed).
> + }
> + } else {
> + $target->{device} = PVE::QemuConfig->add_unused_volume($vmconf, $dst_volid);
> + }
> + print "Imported '$source' to $dst_volid\n";
shouldn't this go higher up, e.g. in case adding the unused volume dies?
also, if we mention both paths/volids in the "Importing ..." message,
this can just become a single "Done" or "Success", directly after the
actual import error handling?
> + $update_vm_api->(
> + {
> + node => $target->{node},
why? not mentioned in the comment describing this method, also, this can
by definition only be the current node anyway?
> + vmid => $vmid,
> + $target->{device} => $drive,
> + skiplock => 1,
> + },
> + 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_param = 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); # check for errors
check that storage exists?
> +
> + # Format can be set explicitly "--format vmdk"
> + # or as part of device options "--device_options discard=on,format=vmdk"
> + # or not at all, but not both together
> + my $device_options_format;
> + if ($device_options) {
> + # parse device_options string according to disk schema for
> + # validation and to make usage easier
> +
> + # any existing storage ID is OK to get a valid (fake) string for parse_drive
> + my $valid_string = "$storeid:0,$device_options";
> +
> + # member "file" is fake
> + my $drive_full = PVE::QemuServer::Drive::parse_drive($device, $valid_string);
so this might be written back to $device_options, makes the rest of the
code more reliable/usable and allows us to only parse here once..
> + $device_options_format = $drive_full->{format};
no need for this then, it's just $device_options->{format}
> + }
> +
> + my $format = extract_param($param, 'format'); # may be undefined
> + if ($device_options) {
> + if ($format && $device_options_format) {
> + raise_param_exc({format => "Format already specified in device_options!"});
> + } else {
> + $format = $format || $device_options_format; # may still be undefined
> + }
> + }
> +
> + $check_format_is_supported->($format, $storeid, $storecfg);
> +
> + # provide a useful error (in the API response) before forking
> + my $no_lock_conf = PVE::QemuConfig->load_config($vmid);
$conf is fine for this, and a simple
# quick checks before fork + lock
should be enough, it's a common pattern in our API2 modules..
> + PVE::QemuConfig->check_lock($no_lock_conf);
> + PVE::Tools::assert_if_modified($no_lock_conf->{digest}, $digest_param);
> + if ($device && $no_lock_conf->{$device}) {
> + die "Could not import because device $device is already in ".
> + "use in VM $vmid. Choose a different device!";
> + }
> +
> + my $worker = sub {
> + my $conf;
> + PVE::QemuConfig->lock_config($vmid, sub {
> + $conf = PVE::QemuConfig->load_config($vmid);
> + PVE::QemuConfig->check_lock($conf);
> +
> + # Our device-in-use check may be invalid if the new conf is different
> + PVE::Tools::assert_if_modified($conf->{digest}, $no_lock_conf->{digest});
> +
> + PVE::QemuConfig->set_lock($vmid, 'import');
> + });
> +
> + my $target = {
> + node => $node,
> + storeid => $storeid,
> + };
> + # Avoid keys with undef values
why?
> + $target->{format} = $format if defined $format;
> + $target->{device} = $device if defined $device;
> + $target->{options} = $device_options if defined $device_options;
> + $import_disk_image->({
> + storecfg => $storecfg,
> + vmid => $vmid,
> + vmconf => $conf,
> + source => $source,
> + target => $target,
> + });
> +
> + PVE::QemuConfig->remove_lock($vmid, 'import');
> + };
> + 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 }),
> + diskimages => {
> + description => "Mapping of devices to disk images. For " .
> + "example, scsi0=/mnt/nfs/image1.vmdk,scsi1=/mnt/nfs/image2",
> + type => 'string',
should probably get a proper format, and this can then be a
'device-image-mapping-list' or whatever fancy name you come up with ;)
\0-delimited might be a good idea - else you restrict what kind of paths
can be imported.. (for CLI, the parameter can be passed multiple times
to defined the list IIRC). ping me if you can't find an example for how
to do this!
> + },
> + 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, 'diskimages');
> + my $boot = extract_param($param, 'boot');
> +
> + my $rpcenv = PVE::RPCEnvironment::get();
> + my $authuser = $rpcenv->get_user();
> + my $storecfg = PVE::Storage::config();
> +
> + PVE::Cluster::check_cfs_quorum();
> +
> + # Return true iff $opt is to be imported, that means it is a device
> + # like scsi2 and the special import syntax/zero is specified for it,
> + # for example local-lvm:0 but not local-lvm:5
> + my $is_import = sub {
> + my ($opt) = @_;
> + return 0 if $opt eq 'efidisk0';
> + if (PVE::QemuServer::Drive::is_valid_drivename($opt)) {
> + my $drive = PVE::QemuServer::parse_drive($opt, $param->{$opt});
> + return $drive->{file} =~ m/0$/;
and if I want to create 10GB volume? note that we have a NEW_DISK_RE at
the top of this module, maybe we want a IMPORT_DISK_RE next to it?
> + }
> + return 0;
> + };
> +
> + # bus/device like ide0, scsi5 where the imported disk images get attached
> + my $target_devices = [];
> + # List of VM parameters like memory, cpu type, but also disks that are newly created
# everything else
?
why not build the $param hashes directly, instead of indirectly via
lists? e.g., you can just remove the import params and put them in a new
hash, and keep $params for the rest?
> + my $vm_params = [];
> + foreach my $opt (keys %$param) {
> + next if ($opt eq 'start'); # does not belong in config
> + # New function, so we can forbid deprecated
> + raise_param_exc({bootdisk => "Deprecated: Use --boot order= instead"})
> + if $opt eq 'bootdisk';
> +
> + my $list = $is_import->($opt) ? $target_devices : $vm_params;
> + push @$list, $opt;
> + }
> +
> + my $diskimages = {};
> + foreach (split ',', $diskimages_string) {
> + my ($device, $diskimage) = split('=', $_);
avoid using $_ for such things please.
> + $diskimages->{$device} = $diskimage;
> + if (defined $param->{$device}) {
> + my $drive = PVE::QemuServer::parse_drive($device, $param->{$device});
> + $drive->{file} =~ m/(\d)$/;
you define a helper above, but not use it here.. also, the check is
wrong here as well ;) if you split the $import_params from $params above,
then you can just check if $import_params->{$device} is defined, in
which case you know it has the special import syntax.
> + if ($1 != 0) {
> + raise_param_exc({
> + $device => "Each entry of --diskimages must have a ".
> + "corresponding device with special import syntax " .
> + "(e.g. --scsi3 local-lvm:0). $device from " .
> + "--diskimages has a matching device but the size " .
> + "of that is $1 instead of 0!",
> + });
here the question is - which parameter is wrong ;)
"device '$device' not marked for import, but import source '..'
specified"
> + }
> + } else {
> + # It is possible to also create new empty disk images during
> + # import by adding something like scsi2=local:10, therefore
> + # vice-versa check is not required
same message as for the other branch, no comment needed. in fact, this
could just be
die/raise_param_exc ..
if (!defined($param->{$device}) || !$is_import->($param->{$device}));
altogether to replace both branches and the comment and the overly long
messages. by splitting $params early, it then becomes
die/...
if !defined($import_params{$device});
> + raise_param_exc({
> + diskimages => "There must be a matching device for each " .
> + "--diskimages entry that should be imported, but " .
> + "there is no matching device for $device!\n" .
> + " For example, for --diskimages scsi0=/source/path,scsi1=/other/path " .
> + "there must be --scsi0 local-lvm:0,discard=on --scsi1 local:0,cache=unsafe",
> + });
> + }
> + # Dies if $diskimage cannot be found
> + PVE::Storage::abs_filesystem_path($storecfg, $diskimage, 1);
> + }
> + foreach my $device (@$target_devices) {
> + my $drive = PVE::QemuServer::parse_drive($device, $param->{$device});
> + if ($drive->{file} =~ m/0$/) {
> + if (!defined $diskimages->{$device}) {
> + raise_param_exc({
> + $device => "Each device with the special import " .
> + "syntax (the 0) must have a corresponding in " .
> + "--diskimages that specifies the source of the " .
> + "import, but there is no such entry for $device!",
> + });
> + }
so if you extract the import_params instead of using a target_devices
list, you can just map and grep here to compare $diskimages keys with
$import_params keys.. no need to parse again and again
or if you want a loop:
foreach my $import_device (keys %$import_params) {
die/raise_param_exc "device '$import_device' marked for import, but no source given\n"
if !defined($diskimages->{$import_device});
}
> + }
> + }
> +
> + # After devices are ensured to be correct
why? could also go before, you don't even differentiate between import
or create below? also, we don't have such a check when creating a VM, do
we? just when updating. maybe it would make sense to unify the handling?
this is not related to import at all, so could also be split out for a
follow-up.
> + if ($boot) {
> + my $new_bootcfg = PVE::JSONSchema::parse_property_string('pve-qm-boot', $boot);
> + if ($new_bootcfg->{order}) {
> + my @devs = PVE::Tools::split_list($new_bootcfg->{order});
> + for my $dev (@devs) {
> + my $will_be_imported = grep (/^$dev$/, @$target_devices);
> + my $will_be_created = grep (/^$dev$/, @$vm_params);
> + if ( !($will_be_imported || $will_be_created)) {
> + raise_param_exc({boot => "$dev will be neither imported " .
> + "nor created, so it cannot be a boot device!"});
> + }
> + }
> + } else {
> + raise_param_exc({boot => "Deprecated: Use --boot order= instead"});
couldn't this go above then next to the bootdisk check?
> + }
> + }
> +
> + eval { PVE::QemuConfig->create_and_lock_config($vmid, 0, 'import') };
> + die "Unable to create config for VM import: $@" if $@;
> +
> + my $worker = sub {
> + my $get_conf = sub {
reload_conf might be a better name
> + my ($vmid) = @_;
> + PVE::QemuConfig->lock_config($vmid, sub {
why? you don't update the config here? an flock is needed for *writing*
the config (or *blocking writes* by others for a short period)..
> + my $conf = PVE::QemuConfig->load_config($vmid);
> + if (PVE::QemuConfig->has_lock($conf, 'import')) {
> + return $conf;
> + } else {
> + die "import lock in VM $vmid config file missing!";
> + }
> + });
> + };
> +
> + $get_conf->($vmid); # quick check for lock
> +
> + my $short_update = {
this is a misleading name - this contains all the disks that are not
imported, which can take a while (not as long as importing, but
still..). and it's not needed if we split the import_params out of
$params above anyway..
> + node => $node,
> + vmid => $vmid,
> + skiplock => 1,
> + };
> + foreach ( @$vm_params ) {
> + $short_update->{$_} = $param->{$_};
> + }
> + $update_vm_api->($short_update, 1); # writes directly in config file
yeah, so this does an flock internally, but to guarantee it still is the
right config you need to set the digest in $short_update.. same for
other calls to update_vm_api - you reload the config, check your config
lock is still there, then you have a valid digest to pass to
update_vm_api, which obtains an flock (== no other writes possible),
checks the digest (== no writes happened since our config reload before
calling update_vm_api) and then does the config modification and write
before releasing the flock. of course, now the digest and our $conf is
invalid, so we need to reload (and now we have a digest again for the
next call ;)).
> +
> + my $conf = $get_conf->($vmid);
> +
> + # When all short updates were succesfull, then the long imports
> + my @imported_successfully = ();
> + eval { foreach my $device (@$target_devices) {
> + my $param_parsed = PVE::QemuServer::parse_drive($device, $param->{$device});
we already did that, so please reuse the result from earlier..
> + die "Parsing $param->{$device} failed" if !$param_parsed;
> + my $storeid = PVE::Storage::parse_volume_id($param_parsed->{file});
> +
> + my $imported = $import_disk_image->({
> + storecfg => $storecfg,
> + vmid => $vmid,
> + vmconf => $conf,
> + source => $diskimages->{$device},
> + target => {
> + storeid => $storeid,
> + format => $param_parsed->{format},
> + options => $param->{$device},
and pass the parsed options here..
> + device => $device,
> + },
> + });
> + push @imported_successfully, $imported;
> + }};
> + my $err = $@;
> + if ($err) {
> + foreach my $volid (@imported_successfully) {
> + eval { PVE::Storage::vdisk_free($storecfg, $volid) };
> + warn $@ if $@;
> + }
> +
> + eval {
> + my $conffile = PVE::QemuConfig->config_file($vmid);
> + unlink($conffile) or die "Failed to remove config file: $!";
at this point we'd need a proper VM removal - disks might have been
allocated, and who knows what else..
> + };
> + warn $@ if $@;
> +
> + die "Import aborted: $err";
> + }
> + if (!$boot) {
> + $conf = $get_conf->($vmid); # import_disk_image changed config file directly
> + 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,
> + },
> + 1,
> + );
> +
> + eval { PVE::QemuConfig->remove_lock($vmid, 'import') };
> + warn $@ if $@;
> +
> + if ($param->{start}) {
> + PVE::QemuServer::vm_start($storecfg, $vmid);
> + }
> + };
> +
> + 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 1410ecb..d4017b7 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)],
can be fine - not 100% convinced yet that we need to treat this as
separate from 'create' ;)
> },
> cpulimit => {
> optional => 1,
> @@ -998,6 +998,18 @@ sub verify_volume_id_or_qm_path {
> return $volid;
> }
>
> +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) = @_;
> +
> + # Exactly these 2 are allowed in id_or_qm_path but should not be allowed here
> + if ($volid eq 'none' || $volid eq 'cdrom') {
then maybe turn it around? it makes more sense to have a base format and
then an extension IMHO. else now someone has to keep in mind that there
is a filter here when updating the other format, which is easily
missed..
> + return undef if $noerr;
> + die "Invalid format! Should be volume ID or absolute path.";
> + }
> + return verify_volume_id_or_qm_path($volid, $noerr);
> +}
> +
> my $usb_fmt = {
> host => {
> default_key => 1,
> @@ -2030,6 +2042,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-qm-path',
wrong format?
> + description => "Disk image that gets imported to $device",
> + optional => 1,
> + };
> + }
> +
> + return $prop;
> +}
> +
> # return copy of $confdesc_cloudinit to generate documentation
> sub cloudinit_config_properties {
>
> @@ -6722,7 +6750,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 _) { # -b required to import from LVM images
-b $src_volid
the comment is not needed (and wrong) - '-b' already states that we
check for block devices, and this is not LVM specific at all, it also
handles direct references to zvols, mapped rbd images, physical disks,
loopback devices, or, well, any other block device ;)
> $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..36b7fff 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, $ignore_size) = @_;
maybe something like $manifest_only - we might want to skip other
futures stuff as well, not just reading the file size.
>
> 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";
> }
doesn't this already fail?
>
> - 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 = 0;
is 0 a good default here? wouldn't undef make more sense?
> + if (!$ignore_size) { # 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] 5+ messages in thread
* Re: [pve-devel] [PATCH v6 storage] Optionally allow blockdev in abs_filesystem_path
[not found] ` <<20210309104318.317454-1-d.jaeger@proxmox.com>
@ 2021-03-15 9:25 ` Fabian Grünbichler
0 siblings, 0 replies; 5+ messages in thread
From: Fabian Grünbichler @ 2021-03-15 9:25 UTC (permalink / raw)
To: Proxmox VE development discussion
On March 9, 2021 11:43 am, Dominic Jäger wrote:
> This is required to import from LVM storages
>
> Signed-off-by: Dominic Jäger <d.jaeger@proxmox.com>
> ---
> v5->v6: unchanged
>
> PVE/Storage.pm | 9 ++++++---
> 1 file changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/PVE/Storage.pm b/PVE/Storage.pm
> index 8ee2c92..7c2e24e 100755
> --- a/PVE/Storage.pm
> +++ b/PVE/Storage.pm
> @@ -609,7 +609,7 @@ sub path {
> }
>
> sub abs_filesystem_path {
> - my ($cfg, $volid) = @_;
> + my ($cfg, $volid, $allowBlockdev) = @_;
that's not how we name perl variables..
>
> my $path;
> if (parse_volume_id ($volid, 1)) {
> @@ -623,8 +623,11 @@ sub abs_filesystem_path {
> }
> }
> }
> -
> - die "can't find file '$volid'\n" if !($path && -f $path);
> + if ($allowBlockdev) {
> + die "can't find file '$volid'\n" if !($path && (-f $path || -b $path));
> + } else {
> + die "can't find file '$volid'\n" if !($path && -f $path);
> + }
this could easily still be a single if:
die "can't find file '$volid'\n"
if !($path && -f $path) && !($path && $allow_block_dev && -b $path);
or some other transformation of the condition, like
if !$path || !(-f $path || ($allow_block_dev && -b $path))
or
if !($path && (-f $path || ($allow_block_dev && -b $path)))
>
> return $path;
> }
> --
> 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] 5+ messages in thread
end of thread, other threads:[~2021-03-15 9:26 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-09 10:43 [pve-devel] [PATCH v6 storage] Optionally allow blockdev in abs_filesystem_path Dominic Jäger
2021-03-09 10:43 ` [pve-devel] [PATCH v6 qemu-server] Add API for import wizards Dominic Jäger
[not found] ` <<20210309104318.317454-2-d.jaeger@proxmox.com>
2021-03-15 9:25 ` Fabian Grünbichler
2021-03-09 10:43 ` [pve-devel] [PATCH v6 manager] gui: Add import for disk & VM Dominic Jäger
[not found] ` <<20210309104318.317454-1-d.jaeger@proxmox.com>
2021-03-15 9:25 ` [pve-devel] [PATCH v6 storage] Optionally allow blockdev in abs_filesystem_path Fabian Grünbichler
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox