* [pve-devel] [PATCH v4 0/2] Close #2886: Add GUI for importdisk
@ 2020-09-15 11:33 Dominic Jäger
2020-09-15 11:33 ` [pve-devel] [PATCH qemu-server v4 1/2] Move importdisk from qm to API Dominic Jäger
2020-09-15 11:33 ` [pve-devel] [PATCH manager v4 2/2] Hardware View: Add GUI for importdisk Dominic Jäger
0 siblings, 2 replies; 7+ messages in thread
From: Dominic Jäger @ 2020-09-15 11:33 UTC (permalink / raw)
To: pve-devel
This series makes importing disks possible via GUI which is one step to make
migrating from other hypervisors easier.
qemu-server: Dominic Jäger (1):
Move importdisk from qm to API
pve-manager: Dominic Jäger (1):
Hardware View: Add GUI for importdisk
--
2.20.1
^ permalink raw reply [flat|nested] 7+ messages in thread
* [pve-devel] [PATCH qemu-server v4 1/2] Move importdisk from qm to API
2020-09-15 11:33 [pve-devel] [PATCH v4 0/2] Close #2886: Add GUI for importdisk Dominic Jäger
@ 2020-09-15 11:33 ` Dominic Jäger
2020-10-28 13:01 ` Fabian Grünbichler
2020-09-15 11:33 ` [pve-devel] [PATCH manager v4 2/2] Hardware View: Add GUI for importdisk Dominic Jäger
1 sibling, 1 reply; 7+ messages in thread
From: Dominic Jäger @ 2020-09-15 11:33 UTC (permalink / raw)
To: pve-devel
Required to create a GUI for importdisk.
Add parameters that enable directly attaching the disk to a bus/device with all
known disk options. This avoids intermediate steps as unused disk.
We allow different places as source
* Regular VM images on PVE storages (Normal users + root)
* Other disk images on PVE storages (Normal users + root) (they have already
been displayed in the FileSelector before)
* Any path (root only)
Using any path for normal users would be security risk. But if you have to
move a disk image to a PVE storage you only are not too many steps
* rename image according to PVE schema
* qm rescan
* double click in GUI to attach
away from making the whole importdisk obsolete.
Enabling arbitrary paths for root additionally makes it unnecessary to move the
disk image or create an appropriate storage. That means no knowledge about PVE
storage content naming schemes ("why do I have to move it into a images/<vmid>
subfolder of a directory based storage?") is required. Importing could then be
comprised of only two steps:
1. mount external drive (hopefully most PVE admins can figure this out)
2. Click through GUI window and insert /mount/externalDrive/exportedFromEsxi.vmdk
Uploading disk images to avoid the PVE storage naming knowledge can still be
added in the future. However, such a function might not be ideal for big images
because
* Upload via browser might fail easily?
* Potentially copying huge images from server to local to server?
So having the absolute path as an option between renaming everything manually
and just uploading it in GUI without CLI knowledge looks like a useful addition
to me.
This patch combines the main part of the previous qm importdisk and do_import
into a helper $convert_and_attach in PVE/API2/Qemu.pm to avoid race conditions
and potentially duplicating code from update_vm_api into do_import.
Furthermore, the only other place where it was invoked was importovf, which now
also uses the helper. importovf will be moved to PVE/API2/Qemu.pm, too, so
placing the helper somewhere else does not look useful to me.
Signed-off-by: Dominic Jäger <d.jaeger@proxmox.com>
---
v3->v4:
* More detailed permissions
* Solve race conditions and code reuse questions by completely moving do_import
to PVE/API2/Qemu.pm; lock the whole procedure
* As I found both discussed approaches
- Image selector (Thomas)
- Paths (Dominik)
useful I included in both. Hope I didn't misunderstand any of you.
PVE/API2/Qemu.pm | 184 ++++++++++++++++++++++++++++++++++-
PVE/CLI/qm.pm | 70 ++-----------
PVE/QemuServer.pm | 2 +-
PVE/QemuServer/Drive.pm | 13 +++
PVE/QemuServer/ImportDisk.pm | 85 ----------------
PVE/QemuServer/Makefile | 1 -
6 files changed, 205 insertions(+), 150 deletions(-)
delete mode 100755 PVE/QemuServer/ImportDisk.pm
diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
index 8da616a..9aa85b5 100644
--- a/PVE/API2/Qemu.pm
+++ b/PVE/API2/Qemu.pm
@@ -45,8 +45,6 @@ BEGIN {
}
}
-use Data::Dumper; # fixme: remove
-
use base qw(PVE::RESTHandler);
my $opt_force_description = "Force physical removal. Without this, we simple remove the disk from the config file and create an additional configuration entry called 'unused[n]', which contains the volume ID. Unlink of unused[n] always cause physical removal.";
@@ -4265,4 +4263,186 @@ __PACKAGE__->register_method({
return PVE::QemuServer::Cloudinit::dump_cloudinit_config($conf, $param->{vmid}, $param->{type});
}});
+# TODO Make locally scoped when importovf is moved from qm to API / this package
+our $convert_and_attach_disk = sub {
+ my ($param) = @_;
+ my $vm_conf = PVE::QemuConfig->load_config($param->{vmid});
+ my $store_conf = PVE::Storage::config();
+ PVE::QemuConfig->check_lock($vm_conf) if !$param->{skiplock};
+ if ($param->{device} && $vm_conf->{$param->{device}}) {
+ die "Could not import because device $param->{device} is already in ".
+ "use in VM $param->{vmid}. Choose a different device!\n";
+ }
+ if ($param->{digest} && $param->{digest} ne $vm_conf->{digest}) {
+ die "VM $param->{vmid} config checksum missmatch (file change by other user?)\n";
+ }
+
+ my $msg = $param->{device} ? "to $param->{device} on" : 'as unused disk to';
+ print "Importing disk '$param->{source_as_path}' $msg VM $param->{vmid}...\n";
+
+ my $src_size = PVE::Storage::file_size_info($param->{source_as_path});
+ my $dst_format = PVE::QemuServer::resolve_dst_disk_format(
+ $store_conf, $param->{storage}, undef, $param->{format});
+ my $dst_volid = PVE::Storage::vdisk_alloc($store_conf, $param->{storage},
+ $param->{vmid}, $dst_format, undef, $src_size / 1024);
+
+ 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($store_conf,
+ 'sparseinit', $dst_volid);
+
+ PVE::Storage::activate_volumes($store_conf, [$dst_volid]);
+ PVE::QemuServer::qemu_img_convert($param->{source_as_path}, $dst_volid,
+ $src_size, undef, $zeroinit);
+ PVE::Storage::deactivate_volumes($store_conf, [$dst_volid]);
+
+ if ($param->{device}) {
+ my $device_param = $dst_volid;
+ $device_param .= ",$param->{device_options}" if $param->{device_options};
+ $update_vm_api->({
+ vmid => $param->{vmid},
+ $param->{device} => $device_param,
+ skiplock => $param->{skiplock} || 0, # Avoid uninitialized values
+ }, 1);
+ } else {
+ $param->{device} = PVE::QemuConfig->add_unused_volume($vm_conf, $dst_volid);
+ PVE::QemuConfig->write_config($param->{vmid}, $vm_conf);
+ }
+ };
+ if (my $err = $@) {
+ eval { PVE::Storage::vdisk_free($store_conf, $dst_volid) };
+ warn "Cleanup of $dst_volid failed: $@ \n" if $@;
+
+ die "Importing disk '$param->{source_as_path}' failed: $err\n" if $err;
+ }
+
+ return $dst_volid;
+};
+
+__PACKAGE__->register_method ({
+ name => 'importdisk',
+ path => '{vmid}/importdisk',
+ method => 'POST',
+ protected => 1, # for worker upid file
+ proxyto => 'node',
+ description => "Import an external disk image into a VM. The image format ".
+ "has to be supported by qemu-img.",
+ permissions => {
+ check => [ 'and',
+ [ 'perm', '/storage/{storage}', ['Datastore.Audit']],
+ [ 'perm', '/storage/{storage}', ['Datastore.Allocate']],
+ [ 'perm', '/storage/{storage}', ['Datastore.AllocateTemplate']],
+ [ 'perm', '/storage/{storage}', ['Datastore.AllocateSpace']],
+ [ 'perm', '/vms/{vmid}', ['VM.Allocate']],
+ [ 'perm', '/vms/{vmid}', ['VM.Config.Disk']],
+ ],
+ },
+ 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-lvm:vm-104-disk-0), an image on a PVE storage ".
+ "(local:104/toImport.raw) or (for root only) 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',
+ format => 'drive_options',
+ description => "Options to set for the new disk ".
+ "(e.g. 'discard=on,backup=0')",
+ optional => 1,
+ },
+ 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'),
+ skiplock => get_standard_option('skiplock'),
+ },
+ },
+ returns => { type => 'string'},
+ code => sub {
+ my ($param) = @_;
+ my $rpcenv = PVE::RPCEnvironment::get();
+ my $authuser = $rpcenv->get_user();
+
+ my $vmid = extract_param($param, 'vmid');
+ my $original_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');
+ # importovf holds a lock itself which would make automatically updating
+ # VM configs fail
+ my $skiplock = extract_param($param, 'skiplock');
+ my $storecfg = PVE::Storage::config();
+
+ if ($skiplock && $authuser ne 'root@pam') {
+ raise_perm_exc("Only root may use skiplock.");
+ }
+ if ($original_source eq "") {
+ die "Could not import because source parameter is an empty string!\n";
+ }
+ if ($device && !PVE::QemuServer::is_valid_drivename($device)) {
+ die "Invalid device name: $device!";
+ }
+ if ($device_options && !$device) {
+ die "Cannot use --device_options without specifying --device!"
+ }
+ eval {
+ PVE::Storage::check_volume_access($rpcenv, $authuser, $storecfg,
+ $vmid, $original_source)
+ };
+ raise_perm_exc($@) if $@;
+
+ # A path is required for $convert_and_attach_disk
+ my $volid_as_path = eval { # Nonempty iff $original_source is a volid
+ PVE::Storage::path($storecfg, $original_source);
+ };
+ my $source_as_path = $volid_as_path || $original_source ;
+ if (!-e $source_as_path) {
+ die "Could not import because source '$original_source' does not exist!\n";
+ }
+
+ my $worker = sub {
+ my $dst_volid = PVE::QemuConfig->lock_config($vmid, $convert_and_attach_disk,
+ {
+ vmid => $vmid,
+ original_source => $original_source,
+ device => $device,
+ device_options => $device_options,
+ storage => extract_param($param, 'storage'),
+ source_as_path => $source_as_path,
+ format => extract_param($param, 'format'),
+ skiplock => $skiplock,
+ });
+ print "Successfully imported disk '$original_source ' as ".
+ "$device: $dst_volid\n";
+ };
+
+ return $rpcenv->fork_worker('importdisk', $vmid, $authuser, $worker);
+ }});
+
1;
diff --git a/PVE/CLI/qm.pm b/PVE/CLI/qm.pm
index 282fa86..f71b3d6 100755
--- a/PVE/CLI/qm.pm
+++ b/PVE/CLI/qm.pm
@@ -31,7 +31,6 @@ use PVE::QemuConfig;
use PVE::QemuServer::Drive;
use PVE::QemuServer::Helpers;
use PVE::QemuServer::Agent qw(agent_available);
-use PVE::QemuServer::ImportDisk;
use PVE::QemuServer::Monitor qw(mon_cmd);
use PVE::QemuServer::OVF;
use PVE::QemuServer;
@@ -445,61 +444,6 @@ __PACKAGE__->register_method ({
return undef;
}});
-__PACKAGE__->register_method ({
- name => 'importdisk',
- path => 'importdisk',
- method => 'POST',
- description => "Import an external disk image as an unused disk in a VM. The
- image format has to be supported by qemu-img(1).",
- parameters => {
- additionalProperties => 0,
- properties => {
- vmid => get_standard_option('pve-vmid', {completion => \&PVE::QemuServer::complete_vmid}),
- source => {
- description => 'Path to the disk image to import',
- type => 'string',
- optional => 0,
- },
- storage => get_standard_option('pve-storage-id', {
- description => 'Target storage ID',
- completion => \&PVE::QemuServer::complete_storage,
- optional => 0,
- }),
- format => {
- type => 'string',
- description => 'Target format',
- enum => [ 'raw', 'qcow2', 'vmdk' ],
- optional => 1,
- },
- },
- },
- returns => { type => 'null'},
- code => sub {
- my ($param) = @_;
-
- my $vmid = extract_param($param, 'vmid');
- my $source = extract_param($param, 'source');
- my $storeid = extract_param($param, 'storage');
- my $format = extract_param($param, 'format');
-
- my $vm_conf = PVE::QemuConfig->load_config($vmid);
- PVE::QemuConfig->check_lock($vm_conf);
- die "$source: non-existent or non-regular file\n" if (! -f $source);
-
- my $storecfg = PVE::Storage::config();
- PVE::Storage::storage_check_enabled($storecfg, $storeid);
-
- my $target_storage_config = PVE::Storage::storage_config($storecfg, $storeid);
- die "storage $storeid does not support vm images\n"
- if !$target_storage_config->{content}->{images};
-
- print "importing disk '$source' to VM $vmid ...\n";
- my ($drive_id, $volid) = PVE::QemuServer::ImportDisk::do_import($source, $vmid, $storeid, { format => $format });
- print "Successfully imported disk as '$drive_id:$volid'\n";
-
- return undef;
- }});
-
__PACKAGE__->register_method ({
name => 'terminal',
path => 'terminal',
@@ -640,17 +584,21 @@ __PACKAGE__->register_method ({
$conf->{cores} = $parsed->{qm}->{cores} if defined($parsed->{qm}->{cores});
eval {
- # order matters, as do_import() will load_config() internally
+ # order matters, as $convert_and_attach_disk will load_config() internally
$conf->{vmgenid} = PVE::QemuServer::generate_uuid();
$conf->{smbios1} = PVE::QemuServer::generate_smbios1_uuid();
PVE::QemuConfig->write_config($vmid, $conf);
foreach my $disk (@{ $parsed->{disks} }) {
my ($file, $drive) = ($disk->{backing_file}, $disk->{disk_address});
- PVE::QemuServer::ImportDisk::do_import($file, $vmid, $storeid, {
- drive_name => $drive,
+ $PVE::API2::Qemu::convert_and_attach_disk->({
+ node => $nodename,
+ vmid => $vmid,
+ source_as_path => $file,
+ storage => $storeid,
+ device => $drive,
format => $format,
- skiplock => 1,
+ skiplock => 1, # Required to update VM configs
});
}
@@ -984,7 +932,7 @@ our $cmddef = {
terminal => [ __PACKAGE__, 'terminal', ['vmid']],
- importdisk => [ __PACKAGE__, 'importdisk', ['vmid', 'source', 'storage']],
+ importdisk => [ "PVE::API2::Qemu", 'importdisk', ['vmid', 'source', 'storage'], { node => $nodename }],
importovf => [ __PACKAGE__, 'importovf', ['vmid', 'manifest', 'storage']],
diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
index 2747c66..715c507 100644
--- a/PVE/QemuServer.pm
+++ b/PVE/QemuServer.pm
@@ -6613,7 +6613,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 for LVM images for example
$src_path = $src_volid;
if ($src_path =~ m/\.($PVE::QemuServer::Drive::QEMU_FORMAT_RE)$/) {
$src_format = $1;
diff --git a/PVE/QemuServer/Drive.pm b/PVE/QemuServer/Drive.pm
index 91c33f8..af52035 100644
--- a/PVE/QemuServer/Drive.pm
+++ b/PVE/QemuServer/Drive.pm
@@ -308,6 +308,19 @@ my $alldrive_fmt = {
%wwn_fmt,
};
+my %optional_file_drivedesc_base = %drivedesc_base;
+$optional_file_drivedesc_base{file}{optional} = 1;
+my $drive_options_fmt = {
+ %optional_file_drivedesc_base,
+ %iothread_fmt,
+ %model_fmt,
+ %queues_fmt,
+ %scsiblock_fmt,
+ %ssd_fmt,
+ %wwn_fmt,
+};
+PVE::JSONSchema::register_format('drive_options', $drive_options_fmt);
+
my $efidisk_fmt = {
volume => { alias => 'file' },
file => {
diff --git a/PVE/QemuServer/ImportDisk.pm b/PVE/QemuServer/ImportDisk.pm
deleted file mode 100755
index 51ad52e..0000000
--- a/PVE/QemuServer/ImportDisk.pm
+++ /dev/null
@@ -1,85 +0,0 @@
-package PVE::QemuServer::ImportDisk;
-
-use strict;
-use warnings;
-
-use PVE::Storage;
-use PVE::QemuServer;
-use PVE::Tools qw(run_command extract_param);
-
-# imports an external disk image to an existing VM
-# and creates by default a drive entry unused[n] pointing to the created volume
-# $params->{drive_name} may be used to specify ide0, scsi1, etc ...
-# $params->{format} may be used to specify qcow2, raw, etc ...
-sub do_import {
- my ($src_path, $vmid, $storage_id, $params) = @_;
-
- my $drive_name = extract_param($params, 'drive_name');
- my $format = extract_param($params, 'format');
- if ($drive_name && !(PVE::QemuServer::is_valid_drivename($drive_name))) {
- die "invalid drive name: $drive_name\n";
- }
-
- # get the needed size from source disk
- my $src_size = PVE::Storage::file_size_info($src_path);
-
- # get target format, target image's path, and whether it's possible to sparseinit
- my $storecfg = PVE::Storage::config();
- my $dst_format = PVE::QemuServer::resolve_dst_disk_format($storecfg, $storage_id, undef, $format);
-
- my $dst_volid = PVE::Storage::vdisk_alloc($storecfg, $storage_id, $vmid, $dst_format, undef, $src_size / 1024);
-
- my $zeroinit = PVE::Storage::volume_has_feature($storecfg, 'sparseinit', $dst_volid);
-
- my $create_drive = sub {
- my $vm_conf = PVE::QemuConfig->load_config($vmid);
- if (!$params->{skiplock}) {
- PVE::QemuConfig->check_lock($vm_conf);
- }
-
- if ($drive_name) {
- # should never happen as setting $drive_name is not exposed to public interface
- die "cowardly refusing to overwrite existing entry: $drive_name\n" if $vm_conf->{$drive_name};
-
- my $modified = {}; # record what $option we modify
- $modified->{$drive_name} = 1;
- $vm_conf->{pending}->{$drive_name} = $dst_volid;
- PVE::QemuConfig->write_config($vmid, $vm_conf);
-
- my $running = PVE::QemuServer::check_running($vmid);
- if ($running) {
- my $errors = {};
- PVE::QemuServer::vmconfig_hotplug_pending($vmid, $vm_conf, $storecfg, $modified, $errors);
- warn "hotplugging imported disk '$_' failed: $errors->{$_}\n" for keys %$errors;
- } else {
- PVE::QemuServer::vmconfig_apply_pending($vmid, $vm_conf, $storecfg);
- }
- } else {
- $drive_name = PVE::QemuConfig->add_unused_volume($vm_conf, $dst_volid);
- PVE::QemuConfig->write_config($vmid, $vm_conf);
- }
- };
-
- eval {
- # trap interrupts so we have a chance to clean up
- local $SIG{INT} =
- local $SIG{TERM} =
- local $SIG{QUIT} =
- local $SIG{HUP} =
- local $SIG{PIPE} = sub { die "interrupted by signal $!\n"; };
-
- PVE::Storage::activate_volumes($storecfg, [$dst_volid]);
- PVE::QemuServer::qemu_img_convert($src_path, $dst_volid, $src_size, undef, $zeroinit);
- PVE::Storage::deactivate_volumes($storecfg, [$dst_volid]);
- PVE::QemuConfig->lock_config($vmid, $create_drive);
- };
- if (my $err = $@) {
- eval { PVE::Storage::vdisk_free($storecfg, $dst_volid) };
- warn "cleanup of $dst_volid failed: $@\n" if $@;
- die $err;
- }
-
- return ($drive_name, $dst_volid);
-}
-
-1;
diff --git a/PVE/QemuServer/Makefile b/PVE/QemuServer/Makefile
index fd8cfbb..ecdab56 100644
--- a/PVE/QemuServer/Makefile
+++ b/PVE/QemuServer/Makefile
@@ -1,7 +1,6 @@
SOURCES=PCI.pm \
USB.pm \
Memory.pm \
- ImportDisk.pm \
OVF.pm \
Cloudinit.pm \
Agent.pm \
--
2.20.1
^ permalink raw reply [flat|nested] 7+ messages in thread
* [pve-devel] [PATCH manager v4 2/2] Hardware View: Add GUI for importdisk
2020-09-15 11:33 [pve-devel] [PATCH v4 0/2] Close #2886: Add GUI for importdisk Dominic Jäger
2020-09-15 11:33 ` [pve-devel] [PATCH qemu-server v4 1/2] Move importdisk from qm to API Dominic Jäger
@ 2020-09-15 11:33 ` Dominic Jäger
2020-09-15 12:17 ` Gilberto Nunes
2020-09-16 7:43 ` Dominic Jäger
1 sibling, 2 replies; 7+ messages in thread
From: Dominic Jäger @ 2020-09-15 11:33 UTC (permalink / raw)
To: pve-devel
Make importing single disks easier.
Required to import a whole VM via GUI.
Signed-off-by: Dominic Jäger <d.jaeger@proxmox.com>
---
v3->v4:
* Reuse propertyStringSet instead of building it myself
* More detailed permissions
* Reorder GUI elements such that source is first
* Assemble importdisk URL here instead of widget-toolkit & use regex for
correct replacement
* Allow selecting images from PVE storages (Normal users + root) or all paths
(root)
www/manager6/qemu/HDEdit.js | 134 ++++++++++++++++++++++++++----
www/manager6/qemu/HardwareView.js | 24 ++++++
2 files changed, 141 insertions(+), 17 deletions(-)
diff --git a/www/manager6/qemu/HDEdit.js b/www/manager6/qemu/HDEdit.js
index e2a5b914..5e0a3981 100644
--- a/www/manager6/qemu/HDEdit.js
+++ b/www/manager6/qemu/HDEdit.js
@@ -67,7 +67,8 @@ Ext.define('PVE.qemu.HDInputPanel', {
if (me.unused) {
me.drive.file = me.vmconfig[values.unusedId];
confid = values.controller + values.deviceid;
- } else if (me.isCreate) {
+ } else if (me.isCreate && !me.isImport) {
+ // disk format & size should not be part of propertyString for import
if (values.hdimage) {
me.drive.file = values.hdimage;
} else {
@@ -83,16 +84,22 @@ 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.isImport) {
+ params.device_options = PVE.Parser.printPropertyString(me.drive);
+ params.source = values.sourceType === 'storage'
+ ? values.sourceVolid : values.sourcePath;
+ params.device = values.controller + values.deviceid;
+ params.storage = values.hdstorage;
+ if (values.diskformat) params.format = values.diskformat;
+ } else {
+ params[confid] = PVE.Parser.printQemuDrive(me.drive);
+ }
return params;
},
@@ -169,10 +176,14 @@ 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', {
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 +195,7 @@ Ext.define('PVE.qemu.HDInputPanel', {
submitValue: false,
hidden: true
});
- me.column1.push(me.scsiController);
+ controllerColumn.push(me.scsiController);
}
if (me.unused) {
@@ -199,14 +210,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 +235,12 @@ Ext.define('PVE.qemu.HDInputPanel', {
});
}
+ if (me.isImport) {
+ me.column2.push({
+ xtype: 'box',
+ autoEl: { tag: 'hr' },
+ });
+ }
me.column2.push(
{
xtype: 'CacheTypeSelector',
@@ -231,6 +255,74 @@ Ext.define('PVE.qemu.HDInputPanel', {
name: 'discard'
}
);
+ if (me.isImport) {
+ let show = (element, value) => {
+ element.setHidden(!value);
+ element.setDisabled(!value);
+ };
+ me.sourceRadioStorage = Ext.create('Ext.form.field.Radio', {
+ name: 'sourceType',
+ inputValue: 'storage',
+ boxLabel: gettext('Use a storage as source'),
+ checked: true,
+ hidden: Proxmox.UserName !== 'root@pam',
+ listeners: {
+ added: () => show(me.sourcePathTextfield, false),
+ change: (_, storageRadioChecked) => {
+ show(me.sourcePathTextfield, !storageRadioChecked);
+ let selectors = [
+ me.sourceStorageSelector,
+ me.sourceFileSelector,
+ ];
+ for (const selector of selectors) {
+ show(selector, storageRadioChecked);
+ }
+ },
+ },
+ });
+ me.sourceStorageSelector = Ext.create('PVE.form.StorageSelector', {
+ name: 'inputImageStorage',
+ nodename: me.nodename,
+ fieldLabel: gettext('Source Storage'),
+ storageContent: 'images',
+ autoSelect: me.insideWizard,
+ listeners: {
+ change: function(_, selectedStorage) {
+ me.sourceFileSelector.setStorage(selectedStorage);
+ },
+ },
+ });
+ me.sourceFileSelector = Ext.create('PVE.form.FileSelector', {
+ name: 'sourceVolid',
+ nodename: me.nodename,
+ storageContent: 'images',
+ fieldLabel: gettext('Source Image'),
+ });
+ me.sourceRadioPath = Ext.create('Ext.form.field.Radio', {
+ name: 'sourceType',
+ inputValue: 'path',
+ boxLabel: gettext('Use an absolute path as source'),
+ hidden: Proxmox.UserName !== 'root@pam',
+ });
+ me.sourcePathTextfield = Ext.create('Ext.form.field.Text', {
+ xtype: 'textfield',
+ fieldLabel: gettext('Source Path'),
+ name: 'sourcePath',
+ emptyText: '/home/user/disk.qcow2',
+ hidden: Proxmox.UserName !== 'root@pam',
+ validator: function(insertedText) {
+ return insertedText.startsWith('/') ||
+ gettext('Must be an absolute path');
+ },
+ });
+ me.column1.unshift(
+ me.sourceRadioStorage,
+ me.sourceStorageSelector,
+ me.sourceFileSelector,
+ me.sourceRadioPath,
+ me.sourcePathTextfield,
+ );
+ }
me.advancedColumn1.push(
{
@@ -372,14 +464,19 @@ Ext.define('PVE.qemu.HDEdit', {
confid: me.confid,
nodename: nodename,
unused: unused,
- isCreate: me.isCreate
+ isCreate: me.isCreate,
+ isImport: 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 +501,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 40b3fe86..dc5e217e 100644
--- a/www/manager6/qemu/HardwareView.js
+++ b/www/manager6/qemu/HardwareView.js
@@ -436,6 +436,29 @@ Ext.define('PVE.qemu.HardwareView', {
handler: run_move
});
+ var import_btn = new Proxmox.button.Button({
+ text: gettext('Import disk'),
+ hidden: !(
+ caps.storage['Datastore.Audit'] &&
+ caps.storage['Datastore.Allocate'] &&
+ caps.storage['Datastore.AllocateTemplate'] &&
+ caps.storage['Datastore.AllocateSpace'] &&
+ caps.vms['VM.Allocate'] &&
+ caps.vms['VM.Config.Disk'] &&
+ true
+ ),
+ handler: function() {
+ var win = Ext.create('PVE.qemu.HDEdit', {
+ method: 'POST',
+ url: `/api2/extjs/${baseurl}`,
+ pveSelNode: me.pveSelNode,
+ isImport: true,
+ });
+ win.on('destroy', me.reload, me);
+ win.show();
+ },
+ });
+
var remove_btn = new Proxmox.button.Button({
text: gettext('Remove'),
defaultText: gettext('Remove'),
@@ -752,6 +775,7 @@ Ext.define('PVE.qemu.HardwareView', {
edit_btn,
resize_btn,
move_btn,
+ import_btn,
revert_btn
],
rows: rows,
--
2.20.1
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [pve-devel] [PATCH manager v4 2/2] Hardware View: Add GUI for importdisk
2020-09-15 11:33 ` [pve-devel] [PATCH manager v4 2/2] Hardware View: Add GUI for importdisk Dominic Jäger
@ 2020-09-15 12:17 ` Gilberto Nunes
2020-09-16 7:43 ` Dominic Jäger
1 sibling, 0 replies; 7+ messages in thread
From: Gilberto Nunes @ 2020-09-15 12:17 UTC (permalink / raw)
To: Proxmox VE development discussion
I'm looking forward to test this new feature.
Sometimes is really annoying make importdisk from cli.
I also would like to recommend add some feature to import OVA/OVF images
using WEB interface, if this is possible, of course.
Thanks for such wonderful work!
---
Gilberto Nunes Ferreira
Em ter., 15 de set. de 2020 às 08:34, Dominic Jäger <d.jaeger@proxmox.com>
escreveu:
> Make importing single disks easier.
> Required to import a whole VM via GUI.
>
> Signed-off-by: Dominic Jäger <d.jaeger@proxmox.com>
> ---
> v3->v4:
> * Reuse propertyStringSet instead of building it myself
> * More detailed permissions
> * Reorder GUI elements such that source is first
> * Assemble importdisk URL here instead of widget-toolkit & use regex for
> correct replacement
> * Allow selecting images from PVE storages (Normal users + root) or all
> paths
> (root)
>
> www/manager6/qemu/HDEdit.js | 134 ++++++++++++++++++++++++++----
> www/manager6/qemu/HardwareView.js | 24 ++++++
> 2 files changed, 141 insertions(+), 17 deletions(-)
>
> diff --git a/www/manager6/qemu/HDEdit.js b/www/manager6/qemu/HDEdit.js
> index e2a5b914..5e0a3981 100644
> --- a/www/manager6/qemu/HDEdit.js
> +++ b/www/manager6/qemu/HDEdit.js
> @@ -67,7 +67,8 @@ Ext.define('PVE.qemu.HDInputPanel', {
> if (me.unused) {
> me.drive.file = me.vmconfig[values.unusedId];
> confid = values.controller + values.deviceid;
> - } else if (me.isCreate) {
> + } else if (me.isCreate && !me.isImport) {
> + // disk format & size should not be part of propertyString for
> import
> if (values.hdimage) {
> me.drive.file = values.hdimage;
> } else {
> @@ -83,16 +84,22 @@ 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.isImport) {
> + params.device_options =
> PVE.Parser.printPropertyString(me.drive);
> + params.source = values.sourceType === 'storage'
> + ? values.sourceVolid : values.sourcePath;
> + params.device = values.controller + values.deviceid;
> + params.storage = values.hdstorage;
> + if (values.diskformat) params.format = values.diskformat;
> + } else {
> + params[confid] = PVE.Parser.printQemuDrive(me.drive);
> + }
> return params;
> },
>
> @@ -169,10 +176,14 @@ 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', {
> 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 +195,7 @@ Ext.define('PVE.qemu.HDInputPanel', {
> submitValue: false,
> hidden: true
> });
> - me.column1.push(me.scsiController);
> + controllerColumn.push(me.scsiController);
> }
>
> if (me.unused) {
> @@ -199,14 +210,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 +235,12 @@ Ext.define('PVE.qemu.HDInputPanel', {
> });
> }
>
> + if (me.isImport) {
> + me.column2.push({
> + xtype: 'box',
> + autoEl: { tag: 'hr' },
> + });
> + }
> me.column2.push(
> {
> xtype: 'CacheTypeSelector',
> @@ -231,6 +255,74 @@ Ext.define('PVE.qemu.HDInputPanel', {
> name: 'discard'
> }
> );
> + if (me.isImport) {
> + let show = (element, value) => {
> + element.setHidden(!value);
> + element.setDisabled(!value);
> + };
> + me.sourceRadioStorage = Ext.create('Ext.form.field.Radio', {
> + name: 'sourceType',
> + inputValue: 'storage',
> + boxLabel: gettext('Use a storage as source'),
> + checked: true,
> + hidden: Proxmox.UserName !== 'root@pam',
> + listeners: {
> + added: () => show(me.sourcePathTextfield, false),
> + change: (_, storageRadioChecked) => {
> + show(me.sourcePathTextfield, !storageRadioChecked);
> + let selectors = [
> + me.sourceStorageSelector,
> + me.sourceFileSelector,
> + ];
> + for (const selector of selectors) {
> + show(selector, storageRadioChecked);
> + }
> + },
> + },
> + });
> + me.sourceStorageSelector =
> Ext.create('PVE.form.StorageSelector', {
> + name: 'inputImageStorage',
> + nodename: me.nodename,
> + fieldLabel: gettext('Source Storage'),
> + storageContent: 'images',
> + autoSelect: me.insideWizard,
> + listeners: {
> + change: function(_, selectedStorage) {
> + me.sourceFileSelector.setStorage(selectedStorage);
> + },
> + },
> + });
> + me.sourceFileSelector = Ext.create('PVE.form.FileSelector', {
> + name: 'sourceVolid',
> + nodename: me.nodename,
> + storageContent: 'images',
> + fieldLabel: gettext('Source Image'),
> + });
> + me.sourceRadioPath = Ext.create('Ext.form.field.Radio', {
> + name: 'sourceType',
> + inputValue: 'path',
> + boxLabel: gettext('Use an absolute path as source'),
> + hidden: Proxmox.UserName !== 'root@pam',
> + });
> + me.sourcePathTextfield = Ext.create('Ext.form.field.Text', {
> + xtype: 'textfield',
> + fieldLabel: gettext('Source Path'),
> + name: 'sourcePath',
> + emptyText: '/home/user/disk.qcow2',
> + hidden: Proxmox.UserName !== 'root@pam',
> + validator: function(insertedText) {
> + return insertedText.startsWith('/') ||
> + gettext('Must be an absolute path');
> + },
> + });
> + me.column1.unshift(
> + me.sourceRadioStorage,
> + me.sourceStorageSelector,
> + me.sourceFileSelector,
> + me.sourceRadioPath,
> + me.sourcePathTextfield,
> + );
> + }
>
> me.advancedColumn1.push(
> {
> @@ -372,14 +464,19 @@ Ext.define('PVE.qemu.HDEdit', {
> confid: me.confid,
> nodename: nodename,
> unused: unused,
> - isCreate: me.isCreate
> + isCreate: me.isCreate,
> + isImport: 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 +501,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 40b3fe86..dc5e217e 100644
> --- a/www/manager6/qemu/HardwareView.js
> +++ b/www/manager6/qemu/HardwareView.js
> @@ -436,6 +436,29 @@ Ext.define('PVE.qemu.HardwareView', {
> handler: run_move
> });
>
> + var import_btn = new Proxmox.button.Button({
> + text: gettext('Import disk'),
> + hidden: !(
> + caps.storage['Datastore.Audit'] &&
> + caps.storage['Datastore.Allocate'] &&
> + caps.storage['Datastore.AllocateTemplate'] &&
> + caps.storage['Datastore.AllocateSpace'] &&
> + caps.vms['VM.Allocate'] &&
> + caps.vms['VM.Config.Disk'] &&
> + true
> + ),
> + handler: function() {
> + var win = Ext.create('PVE.qemu.HDEdit', {
> + method: 'POST',
> + url: `/api2/extjs/${baseurl}`,
> + pveSelNode: me.pveSelNode,
> + isImport: true,
> + });
> + win.on('destroy', me.reload, me);
> + win.show();
> + },
> + });
> +
> var remove_btn = new Proxmox.button.Button({
> text: gettext('Remove'),
> defaultText: gettext('Remove'),
> @@ -752,6 +775,7 @@ Ext.define('PVE.qemu.HardwareView', {
> edit_btn,
> resize_btn,
> move_btn,
> + import_btn,
> revert_btn
> ],
> rows: rows,
> --
> 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 manager v4 2/2] Hardware View: Add GUI for importdisk
2020-09-15 11:33 ` [pve-devel] [PATCH manager v4 2/2] Hardware View: Add GUI for importdisk Dominic Jäger
2020-09-15 12:17 ` Gilberto Nunes
@ 2020-09-16 7:43 ` Dominic Jäger
1 sibling, 0 replies; 7+ messages in thread
From: Dominic Jäger @ 2020-09-16 7:43 UTC (permalink / raw)
To: pve-devel
If you only want to take a quick lock, I put a screenshot in Bugzilla
https://bugzilla.proxmox.com/show_bug.cgi?id=2886#c2
On Tue, Sep 15, 2020 at 01:33:24PM +0200, Dominic Jäger wrote:
> Make importing single disks easier.
> Required to import a whole VM via GUI.
>
> Signed-off-by: Dominic Jäger <d.jaeger@proxmox.com>
> ---
> v3->v4:
> * Reuse propertyStringSet instead of building it myself
> * More detailed permissions
> * Reorder GUI elements such that source is first
> * Assemble importdisk URL here instead of widget-toolkit & use regex for
> correct replacement
> * Allow selecting images from PVE storages (Normal users + root) or all paths
> (root)
>
> www/manager6/qemu/HDEdit.js | 134 ++++++++++++++++++++++++++----
> www/manager6/qemu/HardwareView.js | 24 ++++++
> 2 files changed, 141 insertions(+), 17 deletions(-)
>
> diff --git a/www/manager6/qemu/HDEdit.js b/www/manager6/qemu/HDEdit.js
> index e2a5b914..5e0a3981 100644
> --- a/www/manager6/qemu/HDEdit.js
> +++ b/www/manager6/qemu/HDEdit.js
> @@ -67,7 +67,8 @@ Ext.define('PVE.qemu.HDInputPanel', {
> if (me.unused) {
> me.drive.file = me.vmconfig[values.unusedId];
> confid = values.controller + values.deviceid;
> - } else if (me.isCreate) {
> + } else if (me.isCreate && !me.isImport) {
> + // disk format & size should not be part of propertyString for import
> if (values.hdimage) {
> me.drive.file = values.hdimage;
> } else {
> @@ -83,16 +84,22 @@ 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.isImport) {
> + params.device_options = PVE.Parser.printPropertyString(me.drive);
> + params.source = values.sourceType === 'storage'
> + ? values.sourceVolid : values.sourcePath;
> + params.device = values.controller + values.deviceid;
> + params.storage = values.hdstorage;
> + if (values.diskformat) params.format = values.diskformat;
> + } else {
> + params[confid] = PVE.Parser.printQemuDrive(me.drive);
> + }
> return params;
> },
>
> @@ -169,10 +176,14 @@ 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', {
> 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 +195,7 @@ Ext.define('PVE.qemu.HDInputPanel', {
> submitValue: false,
> hidden: true
> });
> - me.column1.push(me.scsiController);
> + controllerColumn.push(me.scsiController);
> }
>
> if (me.unused) {
> @@ -199,14 +210,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 +235,12 @@ Ext.define('PVE.qemu.HDInputPanel', {
> });
> }
>
> + if (me.isImport) {
> + me.column2.push({
> + xtype: 'box',
> + autoEl: { tag: 'hr' },
> + });
> + }
> me.column2.push(
> {
> xtype: 'CacheTypeSelector',
> @@ -231,6 +255,74 @@ Ext.define('PVE.qemu.HDInputPanel', {
> name: 'discard'
> }
> );
> + if (me.isImport) {
> + let show = (element, value) => {
> + element.setHidden(!value);
> + element.setDisabled(!value);
> + };
> + me.sourceRadioStorage = Ext.create('Ext.form.field.Radio', {
> + name: 'sourceType',
> + inputValue: 'storage',
> + boxLabel: gettext('Use a storage as source'),
> + checked: true,
> + hidden: Proxmox.UserName !== 'root@pam',
> + listeners: {
> + added: () => show(me.sourcePathTextfield, false),
> + change: (_, storageRadioChecked) => {
> + show(me.sourcePathTextfield, !storageRadioChecked);
> + let selectors = [
> + me.sourceStorageSelector,
> + me.sourceFileSelector,
> + ];
> + for (const selector of selectors) {
> + show(selector, storageRadioChecked);
> + }
> + },
> + },
> + });
> + me.sourceStorageSelector = Ext.create('PVE.form.StorageSelector', {
> + name: 'inputImageStorage',
> + nodename: me.nodename,
> + fieldLabel: gettext('Source Storage'),
> + storageContent: 'images',
> + autoSelect: me.insideWizard,
> + listeners: {
> + change: function(_, selectedStorage) {
> + me.sourceFileSelector.setStorage(selectedStorage);
> + },
> + },
> + });
> + me.sourceFileSelector = Ext.create('PVE.form.FileSelector', {
> + name: 'sourceVolid',
> + nodename: me.nodename,
> + storageContent: 'images',
> + fieldLabel: gettext('Source Image'),
> + });
> + me.sourceRadioPath = Ext.create('Ext.form.field.Radio', {
> + name: 'sourceType',
> + inputValue: 'path',
> + boxLabel: gettext('Use an absolute path as source'),
> + hidden: Proxmox.UserName !== 'root@pam',
> + });
> + me.sourcePathTextfield = Ext.create('Ext.form.field.Text', {
> + xtype: 'textfield',
> + fieldLabel: gettext('Source Path'),
> + name: 'sourcePath',
> + emptyText: '/home/user/disk.qcow2',
> + hidden: Proxmox.UserName !== 'root@pam',
> + validator: function(insertedText) {
> + return insertedText.startsWith('/') ||
> + gettext('Must be an absolute path');
> + },
> + });
> + me.column1.unshift(
> + me.sourceRadioStorage,
> + me.sourceStorageSelector,
> + me.sourceFileSelector,
> + me.sourceRadioPath,
> + me.sourcePathTextfield,
> + );
> + }
>
> me.advancedColumn1.push(
> {
> @@ -372,14 +464,19 @@ Ext.define('PVE.qemu.HDEdit', {
> confid: me.confid,
> nodename: nodename,
> unused: unused,
> - isCreate: me.isCreate
> + isCreate: me.isCreate,
> + isImport: 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 +501,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 40b3fe86..dc5e217e 100644
> --- a/www/manager6/qemu/HardwareView.js
> +++ b/www/manager6/qemu/HardwareView.js
> @@ -436,6 +436,29 @@ Ext.define('PVE.qemu.HardwareView', {
> handler: run_move
> });
>
> + var import_btn = new Proxmox.button.Button({
> + text: gettext('Import disk'),
> + hidden: !(
> + caps.storage['Datastore.Audit'] &&
> + caps.storage['Datastore.Allocate'] &&
> + caps.storage['Datastore.AllocateTemplate'] &&
> + caps.storage['Datastore.AllocateSpace'] &&
> + caps.vms['VM.Allocate'] &&
> + caps.vms['VM.Config.Disk'] &&
> + true
> + ),
> + handler: function() {
> + var win = Ext.create('PVE.qemu.HDEdit', {
> + method: 'POST',
> + url: `/api2/extjs/${baseurl}`,
> + pveSelNode: me.pveSelNode,
> + isImport: true,
> + });
> + win.on('destroy', me.reload, me);
> + win.show();
> + },
> + });
> +
> var remove_btn = new Proxmox.button.Button({
> text: gettext('Remove'),
> defaultText: gettext('Remove'),
> @@ -752,6 +775,7 @@ Ext.define('PVE.qemu.HardwareView', {
> edit_btn,
> resize_btn,
> move_btn,
> + import_btn,
> revert_btn
> ],
> rows: rows,
> --
> 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 qemu-server v4 1/2] Move importdisk from qm to API
2020-09-15 11:33 ` [pve-devel] [PATCH qemu-server v4 1/2] Move importdisk from qm to API Dominic Jäger
@ 2020-10-28 13:01 ` Fabian Grünbichler
0 siblings, 0 replies; 7+ messages in thread
From: Fabian Grünbichler @ 2020-10-28 13:01 UTC (permalink / raw)
To: Proxmox VE development discussion
needs a rebase ;)
On September 15, 2020 1:33 pm, Dominic Jäger wrote:
> Required to create a GUI for importdisk.
>
> Add parameters that enable directly attaching the disk to a bus/device with all
> known disk options. This avoids intermediate steps as unused disk.
>
> We allow different places as source
> * Regular VM images on PVE storages (Normal users + root)
> * Other disk images on PVE storages (Normal users + root) (they have already
> been displayed in the FileSelector before)
> * Any path (root only)
>
> Using any path for normal users would be security risk. But if you have to
> move a disk image to a PVE storage you only are not too many steps
> * rename image according to PVE schema
> * qm rescan
> * double click in GUI to attach
> away from making the whole importdisk obsolete.
>
> Enabling arbitrary paths for root additionally makes it unnecessary to move the
> disk image or create an appropriate storage. That means no knowledge about PVE
> storage content naming schemes ("why do I have to move it into a images/<vmid>
> subfolder of a directory based storage?") is required. Importing could then be
> comprised of only two steps:
> 1. mount external drive (hopefully most PVE admins can figure this out)
> 2. Click through GUI window and insert /mount/externalDrive/exportedFromEsxi.vmdk
>
> Uploading disk images to avoid the PVE storage naming knowledge can still be
> added in the future. However, such a function might not be ideal for big images
> because
> * Upload via browser might fail easily?
> * Potentially copying huge images from server to local to server?
>
> So having the absolute path as an option between renaming everything manually
> and just uploading it in GUI without CLI knowledge looks like a useful addition
> to me.
>
> This patch combines the main part of the previous qm importdisk and do_import
> into a helper $convert_and_attach in PVE/API2/Qemu.pm to avoid race conditions
> and potentially duplicating code from update_vm_api into do_import.
> Furthermore, the only other place where it was invoked was importovf, which now
> also uses the helper. importovf will be moved to PVE/API2/Qemu.pm, too, so
> placing the helper somewhere else does not look useful to me.
>
> Signed-off-by: Dominic Jäger <d.jaeger@proxmox.com>
> ---
> v3->v4:
> * More detailed permissions
> * Solve race conditions and code reuse questions by completely moving do_import
> to PVE/API2/Qemu.pm; lock the whole procedure
> * As I found both discussed approaches
> - Image selector (Thomas)
> - Paths (Dominik)
> useful I included in both. Hope I didn't misunderstand any of you.
>
>
> PVE/API2/Qemu.pm | 184 ++++++++++++++++++++++++++++++++++-
> PVE/CLI/qm.pm | 70 ++-----------
> PVE/QemuServer.pm | 2 +-
> PVE/QemuServer/Drive.pm | 13 +++
> PVE/QemuServer/ImportDisk.pm | 85 ----------------
> PVE/QemuServer/Makefile | 1 -
> 6 files changed, 205 insertions(+), 150 deletions(-)
> delete mode 100755 PVE/QemuServer/ImportDisk.pm
>
> diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
> index 8da616a..9aa85b5 100644
> --- a/PVE/API2/Qemu.pm
> +++ b/PVE/API2/Qemu.pm
> @@ -45,8 +45,6 @@ BEGIN {
> }
> }
>
> -use Data::Dumper; # fixme: remove
> -
> use base qw(PVE::RESTHandler);
>
> my $opt_force_description = "Force physical removal. Without this, we simple remove the disk from the config file and create an additional configuration entry called 'unused[n]', which contains the volume ID. Unlink of unused[n] always cause physical removal.";
> @@ -4265,4 +4263,186 @@ __PACKAGE__->register_method({
> return PVE::QemuServer::Cloudinit::dump_cloudinit_config($conf, $param->{vmid}, $param->{type});
> }});
>
> +# TODO Make locally scoped when importovf is moved from qm to API / this package
maybe we could have a description of $param here? doing that often helps
to show whether this is a good choice ;) e.g., in this case
$param->{original_source} is never used..
I would suggest moving the config loading outside and passing the
configs to this. importovf will probably hold a lock and load the
configs once, so no need to put this inside this shared method.
> +our $convert_and_attach_disk = sub {
> + my ($param) = @_;
> + my $vm_conf = PVE::QemuConfig->load_config($param->{vmid});
> + my $store_conf = PVE::Storage::config();
> + PVE::QemuConfig->check_lock($vm_conf) if !$param->{skiplock};
> + if ($param->{device} && $vm_conf->{$param->{device}}) {
> + die "Could not import because device $param->{device} is already in ".
> + "use in VM $param->{vmid}. Choose a different device!\n";
> + }
> + if ($param->{digest} && $param->{digest} ne $vm_conf->{digest}) {
> + die "VM $param->{vmid} config checksum missmatch (file change by other user?)\n";
> + }
use assert_if_modified, but also move this to the API call where the
config is locked+loaded..
> +
> + my $msg = $param->{device} ? "to $param->{device} on" : 'as unused disk to';
> + print "Importing disk '$param->{source_as_path}' $msg VM $param->{vmid}...\n";
this sounds strange, maybe Dylan can contribute a proper English phrasing
for this ;)
> +
> + my $src_size = PVE::Storage::file_size_info($param->{source_as_path});
needs check for undef
> + my $dst_format = PVE::QemuServer::resolve_dst_disk_format(
> + $store_conf, $param->{storage}, undef, $param->{format});
isn't this a bit strange? if I request to import as qcow2, but my
storage does not support it, I probably want to get an error and not
silently be converted to the default format of the storage?
> + my $dst_volid = PVE::Storage::vdisk_alloc($store_conf, $param->{storage},
> + $param->{vmid}, $dst_format, undef, $src_size / 1024);
> +
> + 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($store_conf,
> + 'sparseinit', $dst_volid);
> +
> + PVE::Storage::activate_volumes($store_conf, [$dst_volid]);
> + PVE::QemuServer::qemu_img_convert($param->{source_as_path}, $dst_volid,
> + $src_size, undef, $zeroinit);
> + PVE::Storage::deactivate_volumes($store_conf, [$dst_volid]);
> +
> + if ($param->{device}) {
> + my $device_param = $dst_volid;
> + $device_param .= ",$param->{device_options}" if $param->{device_options};
> + $update_vm_api->({
> + vmid => $param->{vmid},
> + $param->{device} => $device_param,
> + skiplock => $param->{skiplock} || 0, # Avoid uninitialized values
> + }, 1);
> + } else {
> + $param->{device} = PVE::QemuConfig->add_unused_volume($vm_conf, $dst_volid);
> + PVE::QemuConfig->write_config($param->{vmid}, $vm_conf);
> + }
> + };
> + if (my $err = $@) {
> + eval { PVE::Storage::vdisk_free($store_conf, $dst_volid) };
> + warn "Cleanup of $dst_volid failed: $@ \n" if $@;
> +
> + die "Importing disk '$param->{source_as_path}' failed: $err\n" if $err;
> + }
> +
> + return $dst_volid;
> +};
> +
> +__PACKAGE__->register_method ({
> + name => 'importdisk',
> + path => '{vmid}/importdisk',
> + method => 'POST',
> + protected => 1, # for worker upid file
> + proxyto => 'node',
> + description => "Import an external disk image into a VM. The image format ".
> + "has to be supported by qemu-img.",
> + permissions => {
> + check => [ 'and',
> + [ 'perm', '/storage/{storage}', ['Datastore.Audit']],
> + [ 'perm', '/storage/{storage}', ['Datastore.Allocate']],
> + [ 'perm', '/storage/{storage}', ['Datastore.AllocateTemplate']],
> + [ 'perm', '/storage/{storage}', ['Datastore.AllocateSpace']],
> + [ 'perm', '/vms/{vmid}', ['VM.Allocate']],
> + [ 'perm', '/vms/{vmid}', ['VM.Config.Disk']],
I doubt this is actually what you wanted here (all those permissions
combined don't really make sense).
Let's see whether we can disentangle it a bit:
- we are modifying a VM config (adding a new disk entry or a new unused
entry) - this is should require the same permissions as modifying that
part of the config, which is probably VM.Config.Disk and
Datastore.AllocateSpace
- we are allocating a new vdisk volume on a storage (this should require
Datastore.Allocatespace)
we also potentially read from a storage (which requires a separate
check) or from an arbitrary path (which requires yet another check).
> + ],
> + },
> + 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-lvm:vm-104-disk-0), an image on a PVE storage ".
> + "(local:104/toImport.raw) or (for root only) an absolute ".
> + "path on the server.",
> + type => 'string',
we have a format for something similar (volid or path) already, maybe we
can extend it?
> + },
> + 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',
> + format => 'drive_options',
> + description => "Options to set for the new disk ".
> + "(e.g. 'discard=on,backup=0')",
> + optional => 1,
> + },
> + 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'),
> + skiplock => get_standard_option('skiplock'),
> + },
> + },
> + returns => { type => 'string'},
> + code => sub {
> + my ($param) = @_;
> + my $rpcenv = PVE::RPCEnvironment::get();
> + my $authuser = $rpcenv->get_user();
> +
> + my $vmid = extract_param($param, 'vmid');
> + my $original_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');
> + # importovf holds a lock itself which would make automatically updating
> + # VM configs fail
> + my $skiplock = extract_param($param, 'skiplock');
> + my $storecfg = PVE::Storage::config();
> +
> + if ($skiplock && $authuser ne 'root@pam') {
> + raise_perm_exc("Only root may use skiplock.");
> + }
> + if ($original_source eq "") {
> + die "Could not import because source parameter is an empty string!\n";
> + }
should be handled by schema, not manually.
> + if ($device && !PVE::QemuServer::is_valid_drivename($device)) {
> + die "Invalid device name: $device!";
> + }
this should not be possible, since it's an enum and already checked by
the API
> + if ($device_options && !$device) {
> + die "Cannot use --device_options without specifying --device!"
> + }
this can be encoded in the schema (with 'requires')
> + eval {
> + PVE::Storage::check_volume_access($rpcenv, $authuser, $storecfg,
> + $vmid, $original_source)
> + };
so the STORAGE:PATH example from the description is also limited to
root@pam
> + raise_perm_exc($@) if $@;
> +
> + # A path is required for $convert_and_attach_disk
> + my $volid_as_path = eval { # Nonempty iff $original_source is a volid
> + PVE::Storage::path($storecfg, $original_source);
> + };
> + my $source_as_path = $volid_as_path || $original_source ;
> + if (!-e $source_as_path) {
> + die "Could not import because source '$original_source' does not exist!\n";
> + }
> +
> + my $worker = sub {
> + my $dst_volid = PVE::QemuConfig->lock_config($vmid, $convert_and_attach_disk,
> + {
> + vmid => $vmid,
> + original_source => $original_source,
> + device => $device,
> + device_options => $device_options,
> + storage => extract_param($param, 'storage'),
> + source_as_path => $source_as_path,
> + format => extract_param($param, 'format'),
> + skiplock => $skiplock,
> + });
> + print "Successfully imported disk '$original_source ' as ".
> + "$device: $dst_volid\n";
$device is potentially undef here
> + };
> +
> + return $rpcenv->fork_worker('importdisk', $vmid, $authuser, $worker);
> + }});
> +
> 1;
> diff --git a/PVE/CLI/qm.pm b/PVE/CLI/qm.pm
> index 282fa86..f71b3d6 100755
> --- a/PVE/CLI/qm.pm
> +++ b/PVE/CLI/qm.pm
> @@ -31,7 +31,6 @@ use PVE::QemuConfig;
> use PVE::QemuServer::Drive;
> use PVE::QemuServer::Helpers;
> use PVE::QemuServer::Agent qw(agent_available);
> -use PVE::QemuServer::ImportDisk;
> use PVE::QemuServer::Monitor qw(mon_cmd);
> use PVE::QemuServer::OVF;
> use PVE::QemuServer;
> @@ -445,61 +444,6 @@ __PACKAGE__->register_method ({
> return undef;
> }});
>
> -__PACKAGE__->register_method ({
> - name => 'importdisk',
> - path => 'importdisk',
> - method => 'POST',
> - description => "Import an external disk image as an unused disk in a VM. The
> - image format has to be supported by qemu-img(1).",
> - parameters => {
> - additionalProperties => 0,
> - properties => {
> - vmid => get_standard_option('pve-vmid', {completion => \&PVE::QemuServer::complete_vmid}),
> - source => {
> - description => 'Path to the disk image to import',
> - type => 'string',
> - optional => 0,
> - },
> - storage => get_standard_option('pve-storage-id', {
> - description => 'Target storage ID',
> - completion => \&PVE::QemuServer::complete_storage,
> - optional => 0,
> - }),
> - format => {
> - type => 'string',
> - description => 'Target format',
> - enum => [ 'raw', 'qcow2', 'vmdk' ],
> - optional => 1,
> - },
> - },
> - },
> - returns => { type => 'null'},
> - code => sub {
> - my ($param) = @_;
> -
> - my $vmid = extract_param($param, 'vmid');
> - my $source = extract_param($param, 'source');
> - my $storeid = extract_param($param, 'storage');
> - my $format = extract_param($param, 'format');
> -
> - my $vm_conf = PVE::QemuConfig->load_config($vmid);
> - PVE::QemuConfig->check_lock($vm_conf);
> - die "$source: non-existent or non-regular file\n" if (! -f $source);
> -
> - my $storecfg = PVE::Storage::config();
> - PVE::Storage::storage_check_enabled($storecfg, $storeid);
> -
> - my $target_storage_config = PVE::Storage::storage_config($storecfg, $storeid);
> - die "storage $storeid does not support vm images\n"
> - if !$target_storage_config->{content}->{images};
> -
> - print "importing disk '$source' to VM $vmid ...\n";
> - my ($drive_id, $volid) = PVE::QemuServer::ImportDisk::do_import($source, $vmid, $storeid, { format => $format });
> - print "Successfully imported disk as '$drive_id:$volid'\n";
> -
> - return undef;
> - }});
> -
> __PACKAGE__->register_method ({
> name => 'terminal',
> path => 'terminal',
> @@ -640,17 +584,21 @@ __PACKAGE__->register_method ({
> $conf->{cores} = $parsed->{qm}->{cores} if defined($parsed->{qm}->{cores});
>
> eval {
> - # order matters, as do_import() will load_config() internally
> + # order matters, as $convert_and_attach_disk will load_config() internally
> $conf->{vmgenid} = PVE::QemuServer::generate_uuid();
> $conf->{smbios1} = PVE::QemuServer::generate_smbios1_uuid();
> PVE::QemuConfig->write_config($vmid, $conf);
>
> foreach my $disk (@{ $parsed->{disks} }) {
> my ($file, $drive) = ($disk->{backing_file}, $disk->{disk_address});
> - PVE::QemuServer::ImportDisk::do_import($file, $vmid, $storeid, {
> - drive_name => $drive,
> + $PVE::API2::Qemu::convert_and_attach_disk->({
> + node => $nodename,
> + vmid => $vmid,
> + source_as_path => $file,
> + storage => $storeid,
> + device => $drive,
> format => $format,
> - skiplock => 1,
> + skiplock => 1, # Required to update VM configs
> });
> }
>
> @@ -984,7 +932,7 @@ our $cmddef = {
>
> terminal => [ __PACKAGE__, 'terminal', ['vmid']],
>
> - importdisk => [ __PACKAGE__, 'importdisk', ['vmid', 'source', 'storage']],
> + importdisk => [ "PVE::API2::Qemu", 'importdisk', ['vmid', 'source', 'storage'], { node => $nodename }],
>
> importovf => [ __PACKAGE__, 'importovf', ['vmid', 'manifest', 'storage']],
>
> diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
> index 2747c66..715c507 100644
> --- a/PVE/QemuServer.pm
> +++ b/PVE/QemuServer.pm
> @@ -6613,7 +6613,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 for LVM images for example
this is not really related to this patch, right?
> $src_path = $src_volid;
> if ($src_path =~ m/\.($PVE::QemuServer::Drive::QEMU_FORMAT_RE)$/) {
> $src_format = $1;
> diff --git a/PVE/QemuServer/Drive.pm b/PVE/QemuServer/Drive.pm
> index 91c33f8..af52035 100644
> --- a/PVE/QemuServer/Drive.pm
> +++ b/PVE/QemuServer/Drive.pm
> @@ -308,6 +308,19 @@ my $alldrive_fmt = {
> %wwn_fmt,
> };
>
> +my %optional_file_drivedesc_base = %drivedesc_base;
> +$optional_file_drivedesc_base{file}{optional} = 1;
> +my $drive_options_fmt = {
> + %optional_file_drivedesc_base,
> + %iothread_fmt,
> + %model_fmt,
> + %queues_fmt,
> + %scsiblock_fmt,
> + %ssd_fmt,
> + %wwn_fmt,
> +};
> +PVE::JSONSchema::register_format('drive_options', $drive_options_fmt);
> +
> my $efidisk_fmt = {
> volume => { alias => 'file' },
> file => {
> diff --git a/PVE/QemuServer/ImportDisk.pm b/PVE/QemuServer/ImportDisk.pm
> deleted file mode 100755
> index 51ad52e..0000000
> --- a/PVE/QemuServer/ImportDisk.pm
> +++ /dev/null
> @@ -1,85 +0,0 @@
> -package PVE::QemuServer::ImportDisk;
> -
> -use strict;
> -use warnings;
> -
> -use PVE::Storage;
> -use PVE::QemuServer;
> -use PVE::Tools qw(run_command extract_param);
> -
> -# imports an external disk image to an existing VM
> -# and creates by default a drive entry unused[n] pointing to the created volume
> -# $params->{drive_name} may be used to specify ide0, scsi1, etc ...
> -# $params->{format} may be used to specify qcow2, raw, etc ...
> -sub do_import {
> - my ($src_path, $vmid, $storage_id, $params) = @_;
> -
> - my $drive_name = extract_param($params, 'drive_name');
> - my $format = extract_param($params, 'format');
> - if ($drive_name && !(PVE::QemuServer::is_valid_drivename($drive_name))) {
> - die "invalid drive name: $drive_name\n";
> - }
> -
> - # get the needed size from source disk
> - my $src_size = PVE::Storage::file_size_info($src_path);
> -
> - # get target format, target image's path, and whether it's possible to sparseinit
> - my $storecfg = PVE::Storage::config();
> - my $dst_format = PVE::QemuServer::resolve_dst_disk_format($storecfg, $storage_id, undef, $format);
> -
> - my $dst_volid = PVE::Storage::vdisk_alloc($storecfg, $storage_id, $vmid, $dst_format, undef, $src_size / 1024);
> -
> - my $zeroinit = PVE::Storage::volume_has_feature($storecfg, 'sparseinit', $dst_volid);
> -
> - my $create_drive = sub {
> - my $vm_conf = PVE::QemuConfig->load_config($vmid);
> - if (!$params->{skiplock}) {
> - PVE::QemuConfig->check_lock($vm_conf);
> - }
> -
> - if ($drive_name) {
> - # should never happen as setting $drive_name is not exposed to public interface
> - die "cowardly refusing to overwrite existing entry: $drive_name\n" if $vm_conf->{$drive_name};
> -
> - my $modified = {}; # record what $option we modify
> - $modified->{$drive_name} = 1;
> - $vm_conf->{pending}->{$drive_name} = $dst_volid;
> - PVE::QemuConfig->write_config($vmid, $vm_conf);
> -
> - my $running = PVE::QemuServer::check_running($vmid);
> - if ($running) {
> - my $errors = {};
> - PVE::QemuServer::vmconfig_hotplug_pending($vmid, $vm_conf, $storecfg, $modified, $errors);
> - warn "hotplugging imported disk '$_' failed: $errors->{$_}\n" for keys %$errors;
> - } else {
> - PVE::QemuServer::vmconfig_apply_pending($vmid, $vm_conf, $storecfg);
> - }
> - } else {
> - $drive_name = PVE::QemuConfig->add_unused_volume($vm_conf, $dst_volid);
> - PVE::QemuConfig->write_config($vmid, $vm_conf);
> - }
> - };
> -
> - eval {
> - # trap interrupts so we have a chance to clean up
> - local $SIG{INT} =
> - local $SIG{TERM} =
> - local $SIG{QUIT} =
> - local $SIG{HUP} =
> - local $SIG{PIPE} = sub { die "interrupted by signal $!\n"; };
> -
> - PVE::Storage::activate_volumes($storecfg, [$dst_volid]);
> - PVE::QemuServer::qemu_img_convert($src_path, $dst_volid, $src_size, undef, $zeroinit);
> - PVE::Storage::deactivate_volumes($storecfg, [$dst_volid]);
> - PVE::QemuConfig->lock_config($vmid, $create_drive);
> - };
> - if (my $err = $@) {
> - eval { PVE::Storage::vdisk_free($storecfg, $dst_volid) };
> - warn "cleanup of $dst_volid failed: $@\n" if $@;
> - die $err;
> - }
> -
> - return ($drive_name, $dst_volid);
> -}
> -
> -1;
> diff --git a/PVE/QemuServer/Makefile b/PVE/QemuServer/Makefile
> index fd8cfbb..ecdab56 100644
> --- a/PVE/QemuServer/Makefile
> +++ b/PVE/QemuServer/Makefile
> @@ -1,7 +1,6 @@
> SOURCES=PCI.pm \
> USB.pm \
> Memory.pm \
> - ImportDisk.pm \
> OVF.pm \
> Cloudinit.pm \
> Agent.pm \
> --
> 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
* [pve-devel] [PATCH manager v4 2/2] Hardware View: Add GUI for importdisk
@ 2020-09-15 15:37 Alexandre - H3TI
0 siblings, 0 replies; 7+ messages in thread
From: Alexandre - H3TI @ 2020-09-15 15:37 UTC (permalink / raw)
To: pve-devel
Hello good day !
First of all, I would like to congratulate Proxmox for the excellent
evolution of the solution, the possibility of being able to have via GUI
the option of being able to importdisk from other platforms such as ovf,
ova .......... will also be possible. browser button to perform disk image
search? of course if it works.
Thankful !
Att.
--
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2020-10-28 13:02 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-15 11:33 [pve-devel] [PATCH v4 0/2] Close #2886: Add GUI for importdisk Dominic Jäger
2020-09-15 11:33 ` [pve-devel] [PATCH qemu-server v4 1/2] Move importdisk from qm to API Dominic Jäger
2020-10-28 13:01 ` Fabian Grünbichler
2020-09-15 11:33 ` [pve-devel] [PATCH manager v4 2/2] Hardware View: Add GUI for importdisk Dominic Jäger
2020-09-15 12:17 ` Gilberto Nunes
2020-09-16 7:43 ` Dominic Jäger
2020-09-15 15:37 Alexandre - H3TI
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox