public inbox for pve-devel@lists.proxmox.com
 help / color / mirror / Atom feed
* [pve-devel] [PATCH-SERIES v3 qemu-server/manager] more flexible restore
@ 2022-04-26 12:30 Fabian Ebner
  2022-04-26 12:30 ` [pve-devel] [PATCH v3 qemu-server 1/3] api: create: refactor parameter check logic Fabian Ebner
                   ` (5 more replies)
  0 siblings, 6 replies; 11+ messages in thread
From: Fabian Ebner @ 2022-04-26 12:30 UTC (permalink / raw)
  To: pve-devel

Allows preserving disks and overriding VM settings upon restore.
For containers, overriding settings was already possible, but managing
partial restore is more involved because of nested mount structure,
etc.

Exposes the functionality in the UI, allowing to set (host)name,
cores(+sockets), memory, and, for VMs, which action should be taken for
the drive.

Also includes the related improvement in the UI, to detect if a
storage needed by the restore is not available.


Changes from v2:
    * Dropped already applied patches.
    * Switch to a parameter with explicit drive actions, which also
      allows setting a per-drive target storage.
    * Adapt UI and improve JS style.


Necessary dependency bumps are pve-manager -> widget-toolkit
and pve-manager -> qemu-server -> qemu.


Still missing:
    * add documentation for the new restore functionality for VMs and
      existing restore functionality for containers.
    * add per-drive storage selection to UI (just adding a widgetcolumn
      with our storage selector means an API call for each disk, which
      is sub-optimal)


qemu-server:

Fabian Ebner (3):
  api: create: refactor parameter check logic
  api: create: allow overriding non-disk options during restore
  restore: allow specifying drive actions during restore

 PVE/API2/Qemu.pm  |  75 ++++++++++++++++++++++++--------
 PVE/QemuServer.pm | 107 ++++++++++++++++++++++++++++++++++++++++++----
 2 files changed, 156 insertions(+), 26 deletions(-)


manager:

Fabian Ebner (3):
  ui: restore: disallow empty storage selection if it wouldn't work
  ui: restore: allow override of some settings
  ui: restore: allow treating disks differently

 www/manager6/Makefile                |   1 +
 www/manager6/grid/RestoreDiskGrid.js | 151 +++++++++++++++++++++++++++
 www/manager6/window/Restore.js       | 130 ++++++++++++++++++++++-
 3 files changed, 280 insertions(+), 2 deletions(-)
 create mode 100644 www/manager6/grid/RestoreDiskGrid.js

-- 
2.30.2





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

* [pve-devel] [PATCH v3 qemu-server 1/3] api: create: refactor parameter check logic
  2022-04-26 12:30 [pve-devel] [PATCH-SERIES v3 qemu-server/manager] more flexible restore Fabian Ebner
@ 2022-04-26 12:30 ` Fabian Ebner
  2022-04-28  9:12   ` [pve-devel] applied: " Thomas Lamprecht
  2022-04-26 12:30 ` [pve-devel] [PATCH v3 qemu-server 2/3] api: create: allow overriding non-disk options during restore Fabian Ebner
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 11+ messages in thread
From: Fabian Ebner @ 2022-04-26 12:30 UTC (permalink / raw)
  To: pve-devel

In preparation to allow passing along certain parameters together with
'archive'. Moving the parameter checks to after the
conflicts-with-'archive' to ensure that the more telling error will
trigger first.

All check helpers should handle empty params fine, but check first
just to make sure and to avoid all the superfluous function calls.

Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
---

No changes from v2.

 PVE/API2/Qemu.pm | 34 ++++++++++++++++++----------------
 1 file changed, 18 insertions(+), 16 deletions(-)

diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
index 71db264a..61aee0ba 100644
--- a/PVE/API2/Qemu.pm
+++ b/PVE/API2/Qemu.pm
@@ -818,22 +818,7 @@ __PACKAGE__->register_method({
 	    raise_perm_exc();
 	}
 
-	if (!$archive) {
-	    &$resolve_cdrom_alias($param);
-
-	    &$check_storage_access($rpcenv, $authuser, $storecfg, $vmid, $param, $storage);
-
-	    &$check_vm_modify_config_perm($rpcenv, $authuser, $vmid, $pool, [ keys %$param]);
-
-	    &$check_vm_create_serial_perm($rpcenv, $authuser, $vmid, $pool, $param);
-	    &$check_vm_create_usb_perm($rpcenv, $authuser, $vmid, $pool, $param);
-
-	    &$check_cpu_model_access($rpcenv, $authuser, $param);
-
-	    $check_drive_param->($param, $storecfg);
-
-	    PVE::QemuServer::add_random_macs($param);
-	} else {
+	if ($archive) {
 	    my $keystr = join(' ', keys %$param);
 	    raise_param_exc({ archive => "option conflicts with other options ($keystr)"}) if $keystr;
 
@@ -854,6 +839,23 @@ __PACKAGE__->register_method({
 	    }
 	}
 
+	if (scalar(keys $param->%*) > 0) {
+	    &$resolve_cdrom_alias($param);
+
+	    &$check_storage_access($rpcenv, $authuser, $storecfg, $vmid, $param, $storage);
+
+	    &$check_vm_modify_config_perm($rpcenv, $authuser, $vmid, $pool, [ keys %$param]);
+
+	    &$check_vm_create_serial_perm($rpcenv, $authuser, $vmid, $pool, $param);
+	    &$check_vm_create_usb_perm($rpcenv, $authuser, $vmid, $pool, $param);
+
+	    &$check_cpu_model_access($rpcenv, $authuser, $param);
+
+	    $check_drive_param->($param, $storecfg);
+
+	    PVE::QemuServer::add_random_macs($param);
+	}
+
 	my $emsg = $is_restore ? "unable to restore VM $vmid -" : "unable to create VM $vmid -";
 
 	eval { PVE::QemuConfig->create_and_lock_config($vmid, $force) };
-- 
2.30.2





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

* [pve-devel] [PATCH v3 qemu-server 2/3] api: create: allow overriding non-disk options during restore
  2022-04-26 12:30 [pve-devel] [PATCH-SERIES v3 qemu-server/manager] more flexible restore Fabian Ebner
  2022-04-26 12:30 ` [pve-devel] [PATCH v3 qemu-server 1/3] api: create: refactor parameter check logic Fabian Ebner
@ 2022-04-26 12:30 ` Fabian Ebner
  2022-04-28  9:12   ` [pve-devel] applied: " Thomas Lamprecht
  2022-04-26 12:30 ` [pve-devel] [PATCH v3 qemu-server 3/3] restore: allow specifying drive actions " Fabian Ebner
                   ` (3 subsequent siblings)
  5 siblings, 1 reply; 11+ messages in thread
From: Fabian Ebner @ 2022-04-26 12:30 UTC (permalink / raw)
  To: pve-devel

Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
---

No changes from v2.

 PVE/API2/Qemu.pm  |  8 ++++++--
 PVE/QemuServer.pm | 26 ++++++++++++++++++++------
 2 files changed, 26 insertions(+), 8 deletions(-)

diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
index 61aee0ba..54af11a0 100644
--- a/PVE/API2/Qemu.pm
+++ b/PVE/API2/Qemu.pm
@@ -819,8 +819,11 @@ __PACKAGE__->register_method({
 	}
 
 	if ($archive) {
-	    my $keystr = join(' ', keys %$param);
-	    raise_param_exc({ archive => "option conflicts with other options ($keystr)"}) if $keystr;
+	    for my $opt (sort keys $param->%*) {
+		if (PVE::QemuServer::Drive::is_valid_drivename($opt)) {
+		    raise_param_exc({ $opt => "option conflicts with option 'archive'" });
+		}
+	    }
 
 	    if ($archive eq '-') {
 		die "pipe requires cli environment\n" if $rpcenv->{type} ne 'cli';
@@ -876,6 +879,7 @@ __PACKAGE__->register_method({
 		    unique => $unique,
 		    bwlimit => $bwlimit,
 		    live => $live_restore,
+		    override_conf => $param,
 		};
 		if ($archive->{type} eq 'file' || $archive->{type} eq 'pipe') {
 		    die "live-restore is only compatible with backup images from a Proxmox Backup Server\n"
diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
index 5db10fe6..2a3e6d58 100644
--- a/PVE/QemuServer.pm
+++ b/PVE/QemuServer.pm
@@ -6480,6 +6480,17 @@ my $restore_destroy_volumes = sub {
     }
 };
 
+my $restore_merge_config = sub {
+    my ($filename, $backup_conf_raw, $override_conf) = @_;
+
+    my $backup_conf = parse_vm_config($filename, $backup_conf_raw);
+    for my $key (keys $override_conf->%*) {
+	$backup_conf->{$key} = $override_conf->{$key};
+    }
+
+    return $backup_conf;
+};
+
 sub scan_volids {
     my ($cfg, $vmid) = @_;
 
@@ -6789,9 +6800,8 @@ sub restore_proxmox_backup_archive {
 	$new_conf_raw .= "\nlock: create";
     }
 
-    PVE::Tools::file_set_contents($conffile, $new_conf_raw);
-
-    PVE::Cluster::cfs_update(); # make sure we read new file
+    my $new_conf = $restore_merge_config->($conffile, $new_conf_raw, $options->{override_conf});
+    PVE::QemuConfig->write_config($vmid, $new_conf);
 
     eval { rescan($vmid, 1); };
     warn $@ if $@;
@@ -7095,9 +7105,8 @@ sub restore_vma_archive {
 	die $err;
     }
 
-    PVE::Tools::file_set_contents($conffile, $new_conf_raw);
-
-    PVE::Cluster::cfs_update(); # make sure we read new file
+    my $new_conf = $restore_merge_config->($conffile, $new_conf_raw, $opts->{override_conf});
+    PVE::QemuConfig->write_config($vmid, $new_conf);
 
     eval { rescan($vmid, 1); };
     warn $@ if $@;
@@ -7108,6 +7117,11 @@ sub restore_vma_archive {
 sub restore_tar_archive {
     my ($archive, $vmid, $user, $opts) = @_;
 
+    if (scalar(keys $opts->{override_conf}->%*) > 0) {
+	my $keystring = join(' ', keys $opts->{override_conf}->%*);
+	die "cannot pass along options ($keystring) when restoring from tar archive\n";
+    }
+
     if ($archive ne '-') {
 	my $firstfile = tar_archive_read_firstfile($archive);
 	die "ERROR: file '$archive' does not look like a QemuServer vzdump backup\n"
-- 
2.30.2





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

* [pve-devel] [PATCH v3 qemu-server 3/3] restore: allow specifying drive actions during restore
  2022-04-26 12:30 [pve-devel] [PATCH-SERIES v3 qemu-server/manager] more flexible restore Fabian Ebner
  2022-04-26 12:30 ` [pve-devel] [PATCH v3 qemu-server 1/3] api: create: refactor parameter check logic Fabian Ebner
  2022-04-26 12:30 ` [pve-devel] [PATCH v3 qemu-server 2/3] api: create: allow overriding non-disk options during restore Fabian Ebner
@ 2022-04-26 12:30 ` Fabian Ebner
  2022-04-26 12:30 ` [pve-devel] [PATCH v3 manager 1/3] ui: restore: disallow empty storage selection if it wouldn't work Fabian Ebner
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 11+ messages in thread
From: Fabian Ebner @ 2022-04-26 12:30 UTC (permalink / raw)
  To: pve-devel

Possible actions are restoring (with an optional target storage;
default, when drive is not part of the backup), keeping the disk as
unused (default, when drive is part of the backup) and keeping the
disk configured.

Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
---

Changes from v2:
    * Switch to a parameter that allows explicitly setting the action
      for each drive and in case of restore, the target storage. This
      avoids automagic, covers all cases explicitly and allows for a
      per-disk target storage setting too.

 PVE/API2/Qemu.pm  | 35 +++++++++++++++++++-
 PVE/QemuServer.pm | 83 +++++++++++++++++++++++++++++++++++++++++++++--
 2 files changed, 114 insertions(+), 4 deletions(-)

diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
index 54af11a0..7088e61f 100644
--- a/PVE/API2/Qemu.pm
+++ b/PVE/API2/Qemu.pm
@@ -745,6 +745,18 @@ __PACKAGE__->register_method({
 		    description => "Start the VM immediately from the backup and restore in background. PBS only.",
 		    requires => 'archive',
 		},
+		'restore-drive-actions' => {
+		    optional => 1,
+		    type => 'string',
+		    format => 'pve-restore-drive-action-list',
+		    description => "List of <drive>=<action> pairs where the action can be ".
+			"'restore:<storage ID>' (restore from backup to specified storage; if no ".
+			"storage is specified, the default storage is used), 'unused' (keep ".
+			"current disk as unused) or 'preserve' (keep current config, even if ".
+			"empty). Default is 'restore' for drives in the backup and 'unused' for ".
+			"others.",
+		    requires => 'archive',
+		},
 		pool => {
 		    optional => 1,
 		    type => 'string', format => 'pve-poolid',
@@ -790,6 +802,11 @@ __PACKAGE__->register_method({
 	my $unique = extract_param($param, 'unique');
 	my $live_restore = extract_param($param, 'live-restore');
 
+	my $restore_drive_actions = extract_param($param, 'restore-drive-actions');
+	$restore_drive_actions = PVE::QemuServer::parse_restore_drive_actions(
+	    $restore_drive_actions,
+	);
+
 	if (defined(my $ssh_keys = $param->{sshkeys})) {
 		$ssh_keys = URI::Escape::uri_unescape($ssh_keys);
 		PVE::Tools::validate_ssh_public_keys($ssh_keys);
@@ -821,7 +838,10 @@ __PACKAGE__->register_method({
 	if ($archive) {
 	    for my $opt (sort keys $param->%*) {
 		if (PVE::QemuServer::Drive::is_valid_drivename($opt)) {
-		    raise_param_exc({ $opt => "option conflicts with option 'archive'" });
+		    raise_param_exc({
+			$opt => "option conflicts with option 'archive' (do you mean to use ".
+			    "'restore-drive-actions'?)",
+		    });
 		}
 	    }
 
@@ -872,6 +892,17 @@ __PACKAGE__->register_method({
 
 	    die "$emsg vm is running\n" if PVE::QemuServer::check_running($vmid);
 
+	    my $restore_drives = [];
+	    for my $drive (sort keys $restore_drive_actions->%*) {
+		my $action = $restore_drive_actions->{$drive}->{action};
+
+		die "$drive - invalid drive action '$action' - drive not present in config\n"
+		    if !$conf->{$drive} && $action eq 'unused';
+
+		$param->{$drive} = $conf->{$drive} if $action eq 'preserve';
+		$param->{$drive} = undef if $action eq 'unused';
+	    }
+
 	    my $realcmd = sub {
 		my $restore_options = {
 		    storage => $storage,
@@ -880,7 +911,9 @@ __PACKAGE__->register_method({
 		    bwlimit => $bwlimit,
 		    live => $live_restore,
 		    override_conf => $param,
+		    drive_actions => $restore_drive_actions,
 		};
+
 		if ($archive->{type} eq 'file' || $archive->{type} eq 'pipe') {
 		    die "live-restore is only compatible with backup images from a Proxmox Backup Server\n"
 			if $live_restore;
diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
index 2a3e6d58..ad46741b 100644
--- a/PVE/QemuServer.pm
+++ b/PVE/QemuServer.pm
@@ -1206,6 +1206,46 @@ sub print_bootorder {
     return PVE::JSONSchema::print_property_string($data, $boot_fmt);
 }
 
+PVE::JSONSchema::register_format('pve-restore-drive-action', \&verify_restore_drive_action);
+sub verify_restore_drive_action {
+    my ($kv, $noerr) = @_;
+
+    my $actions = eval { parse_restore_drive_actions($kv) };
+    if (my $err = $@) {
+	die $err if !$noerr;
+	return;
+    }
+    return $actions;
+}
+
+sub parse_restore_drive_actions {
+    my ($string) = @_;
+
+    my $actions = {};
+
+    for my $kv (PVE::Tools::split_list($string)) {
+	die "expected a drive=action pair\n" if $kv !~ m/^([^=]+)=(.+)$/;
+	my ($drive, $action) = ($1, $2);
+	die "invalid drivename '$drive'\n" if !PVE::QemuServer::Drive::is_valid_drivename($drive);
+	die "drivename '$drive' specified multiple times\n" if $actions->{$drive};
+
+	if ($action =~ m/^restore(?::(.*))?$/) {
+	    $actions->{$drive} = { action => 'restore' };
+	    if (my $storage = $1) {
+		eval { PVE::JSONSchema::check_format('pve-storage-id', $storage, ''); };
+		die "invalid storage ID '$storage' - $@\n" if $@;
+		$actions->{$drive}->{storage} = $storage;
+	    }
+	} elsif ($action eq 'preserve' || $action eq 'unused') {
+	    $actions->{$drive} = { action => $action };
+	} else {
+	    die "invalid action '$action'\n";
+	}
+    }
+
+    return $actions;
+}
+
 my $kvm_api_version = 0;
 
 sub kvm_version {
@@ -6295,7 +6335,10 @@ my $parse_backup_hints = sub {
 	    if $user ne 'root@pam';
     };
 
+    my $drive_actions = $options->{drive_actions};
+
     my $virtdev_hash = {};
+    my $cdroms = {};
     while (defined(my $line = <$fh>)) {
 	if ($line =~ m/^\#qmdump\#map:(\S+):(\S+):(\S*):(\S*):$/) {
 	    my ($virtdev, $devname, $storeid, $format) = ($1, $2, $3, $4);
@@ -6311,6 +6354,12 @@ my $parse_backup_hints = sub {
 	    $devinfo->{$devname}->{devname} = $devname;
 	    $devinfo->{$devname}->{virtdev} = $virtdev;
 	    $devinfo->{$devname}->{format} = $format;
+
+	    if ($drive_actions->{$virtdev}) {
+		next if $drive_actions->{$virtdev}->{action} ne 'restore';
+		$storeid = $drive_actions->{$virtdev}->{storage} || $storeid;
+	    }
+
 	    $devinfo->{$devname}->{storeid} = $storeid;
 
 	    my $scfg = PVE::Storage::storage_config($storecfg, $storeid);
@@ -6336,9 +6385,17 @@ my $parse_backup_hints = sub {
 		    is_cloudinit => 1,
 		};
 	    }
+
+	    $cdroms->{$virtdev} = 1 if drive_is_cdrom($drive);
 	}
     }
 
+    for my $virtdev (sort keys $drive_actions->%*) {
+	my $action = $drive_actions->{$virtdev}->{action};
+	die "requested restore for drive '$virtdev', but not present in backup\n"
+	    if $action eq 'restore' && !$virtdev_hash->{$virtdev} && !$cdroms->{$virtdev};
+    }
+
     return $virtdev_hash;
 };
 
@@ -6485,7 +6542,11 @@ my $restore_merge_config = sub {
 
     my $backup_conf = parse_vm_config($filename, $backup_conf_raw);
     for my $key (keys $override_conf->%*) {
-	$backup_conf->{$key} = $override_conf->{$key};
+	if (defined($override_conf->{$key})) {
+	    $backup_conf->{$key} = $override_conf->{$key};
+	} else {
+	    delete $backup_conf->{$key};
+	}
     }
 
     return $backup_conf;
@@ -6822,6 +6883,12 @@ sub restore_proxmox_backup_archive {
 	# these special drives are already restored before start
 	delete $devinfo->{'drive-efidisk0'};
 	delete $devinfo->{'drive-tpmstate0-backup'};
+
+	for my $key (keys $options->{drive_actions}->%*) {
+	    delete $devinfo->{"drive-$key"}
+		if $options->{drive_actions}->{$key}->{action} ne 'restore';
+	}
+
 	pbs_live_restore($vmid, $conf, $storecfg, $devinfo, $repo, $keyfile, $pbs_backup_name);
 
 	PVE::QemuConfig->remove_lock($vmid, "create");
@@ -7014,8 +7081,15 @@ sub restore_vma_archive {
 	my $map = $restore_allocate_devices->($cfg, $virtdev_hash, $vmid);
 
 	# print restore information to $fifofh
-	foreach my $virtdev (sort keys %$virtdev_hash) {
-	    my $d = $virtdev_hash->{$virtdev};
+	for my $devname (sort keys $devinfo->%*) {
+	    my $d = $devinfo->{$devname};
+
+	    if (!$virtdev_hash->{$d->{virtdev}}) { # skipped
+		print $fifofh "skip=$d->{devname}\n";
+		print "not restoring '$d->{devname}', but keeping current disk\n";
+		next;
+	    }
+
 	    next if $d->{is_cloudinit}; # no need to restore cloudinit
 
 	    my $storeid = $d->{storeid};
@@ -7122,6 +7196,9 @@ sub restore_tar_archive {
 	die "cannot pass along options ($keystring) when restoring from tar archive\n";
     }
 
+    die "drive actions are not supported when restoring from tar archive\n"
+	if scalar(keys $opts->{drive_actions}->%*) > 0;
+
     if ($archive ne '-') {
 	my $firstfile = tar_archive_read_firstfile($archive);
 	die "ERROR: file '$archive' does not look like a QemuServer vzdump backup\n"
-- 
2.30.2





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

* [pve-devel] [PATCH v3 manager 1/3] ui: restore: disallow empty storage selection if it wouldn't work
  2022-04-26 12:30 [pve-devel] [PATCH-SERIES v3 qemu-server/manager] more flexible restore Fabian Ebner
                   ` (2 preceding siblings ...)
  2022-04-26 12:30 ` [pve-devel] [PATCH v3 qemu-server 3/3] restore: allow specifying drive actions " Fabian Ebner
@ 2022-04-26 12:30 ` Fabian Ebner
  2022-04-28  9:13   ` [pve-devel] applied: " Thomas Lamprecht
  2022-04-26 12:30 ` [pve-devel] [PATCH v3 manager 2/3] ui: restore: allow override of some settings Fabian Ebner
  2022-04-26 12:30 ` [pve-devel] [PATCH v3 manager 3/3] ui: restore: allow treating disks differently Fabian Ebner
  5 siblings, 1 reply; 11+ messages in thread
From: Fabian Ebner @ 2022-04-26 12:30 UTC (permalink / raw)
  To: pve-devel

Namely, if there is a storage in the backup configuration that's not
available on the current node.

Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
---

Changes from v2:
    * Improve style.
    * Handle when no storage hint is present correctly.

 www/manager6/window/Restore.js | 31 +++++++++++++++++++++++++++++++
 1 file changed, 31 insertions(+)

diff --git a/www/manager6/window/Restore.js b/www/manager6/window/Restore.js
index e48aac1a..51f7d063 100644
--- a/www/manager6/window/Restore.js
+++ b/www/manager6/window/Restore.js
@@ -94,6 +94,37 @@ Ext.define('PVE.window.Restore', {
 		executeRestore();
 	    }
 	},
+
+	afterRender: function() {
+	    let view = this.getView();
+
+	    Proxmox.Utils.API2Request({
+		url: `/nodes/${view.nodename}/vzdump/extractconfig`,
+		method: 'GET',
+		waitMsgTarget: view,
+		params: {
+		    volume: view.volid,
+		},
+		failure: response => Ext.Msg.alert('Error', response.htmlStatus),
+		success: function(response, options) {
+		    let allStoragesAvailable = response.result.data.split('\n').every(line => {
+			let match = line.match(/^#qmdump#map:(\S+):(\S+):(\S*):(\S*):$/);
+			if (!match) {
+			    return true;
+			}
+			// if a /dev/XYZ disk was backed up, ther is no storage hint
+			return !!match[3] && !!PVE.data.ResourceStore.getById(
+			    `storage/${view.nodename}/${match[3]}`);
+		    });
+
+		    if (!allStoragesAvailable) {
+			let storagesel = view.down('pveStorageSelector[name=storage]');
+			storagesel.allowBlank = false;
+			storagesel.setEmptyText('');
+		    }
+		},
+	    });
+	},
     },
 
     initComponent: function() {
-- 
2.30.2





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

* [pve-devel] [PATCH v3 manager 2/3] ui: restore: allow override of some settings
  2022-04-26 12:30 [pve-devel] [PATCH-SERIES v3 qemu-server/manager] more flexible restore Fabian Ebner
                   ` (3 preceding siblings ...)
  2022-04-26 12:30 ` [pve-devel] [PATCH v3 manager 1/3] ui: restore: disallow empty storage selection if it wouldn't work Fabian Ebner
@ 2022-04-26 12:30 ` Fabian Ebner
  2022-04-28  9:13   ` [pve-devel] applied: " Thomas Lamprecht
  2022-04-26 12:30 ` [pve-devel] [PATCH v3 manager 3/3] ui: restore: allow treating disks differently Fabian Ebner
  5 siblings, 1 reply; 11+ messages in thread
From: Fabian Ebner @ 2022-04-26 12:30 UTC (permalink / raw)
  To: pve-devel

Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
---

Dependency bump for qemu-server needed

Changes from v2:
    * Improve style.

 www/manager6/window/Restore.js | 80 ++++++++++++++++++++++++++++++----
 1 file changed, 71 insertions(+), 9 deletions(-)

diff --git a/www/manager6/window/Restore.js b/www/manager6/window/Restore.js
index 51f7d063..2d78eb56 100644
--- a/www/manager6/window/Restore.js
+++ b/www/manager6/window/Restore.js
@@ -47,8 +47,16 @@ Ext.define('PVE.window.Restore', {
 	    if (values.storage) {
 		params.storage = values.storage;
 	    }
-	    if (values.bwlimit !== undefined) {
-		params.bwlimit = values.bwlimit;
+
+	    ['bwlimit', 'cores', 'name', 'memory', 'sockets'].forEach(opt => {
+		if ((values[opt] ?? '') !== '') {
+		    params[opt] = values[opt];
+		}
+	    });
+
+	    if (params.name && view.vmtype === 'lxc') {
+		params.hostname = params.name;
+		delete params.name;
 	    }
 
 	    let confirmMsg;
@@ -107,14 +115,25 @@ Ext.define('PVE.window.Restore', {
 		},
 		failure: response => Ext.Msg.alert('Error', response.htmlStatus),
 		success: function(response, options) {
-		    let allStoragesAvailable = response.result.data.split('\n').every(line => {
-			let match = line.match(/^#qmdump#map:(\S+):(\S+):(\S*):(\S*):$/);
-			if (!match) {
-			    return true;
+		    let allStoragesAvailable = true;
+
+		    response.result.data.split('\n').forEach(line => {
+			let [_, key, value] = line.match(/^([^:]+):\s*(\S+)\s*$/) ?? [];
+
+			if (!key) {
+			    return;
+			}
+
+			if (key === '#qmdump#map') {
+			    let match = value.match(/^(\S+):(\S+):(\S*):(\S*):$/) ?? [];
+			    // if a /dev/XYZ disk was backed up, ther is no storage hint
+			    allStoragesAvailable &&= !!match[3] && !!PVE.data.ResourceStore.getById(
+				`storage/${view.nodename}/${match[3]}`);
+			} else if (key === 'name' || key === 'hostname') {
+			    view.lookupReference('nameField').setEmptyText(value);
+			} else if (key === 'memory' || key === 'cores' || key === 'sockets') {
+			    view.lookupReference(`${key}Field`).setEmptyText(value);
 			}
-			// if a /dev/XYZ disk was backed up, ther is no storage hint
-			return !!match[3] && !!PVE.data.ResourceStore.getById(
-			    `storage/${view.nodename}/${match[3]}`);
 		    });
 
 		    if (!allStoragesAvailable) {
@@ -274,6 +293,49 @@ Ext.define('PVE.window.Restore', {
 	    });
 	}
 
+	items.push(
+	    {
+		xtype: 'displayfield',
+		value: `${gettext('Override Settings')}:`,
+	    },
+	    {
+		xtype: 'textfield',
+		fieldLabel: gettext('Name'),
+		name: 'name',
+		reference: 'nameField',
+		allowBlank: true,
+	    },
+	    {
+		xtype: 'pveMemoryField',
+		fieldLabel: gettext('Memory'),
+		name: 'memory',
+		reference: 'memoryField',
+		value: '',
+		allowBlank: true,
+	    },
+	    {
+		xtype: 'proxmoxintegerfield',
+		fieldLabel: gettext('Cores'),
+		name: 'cores',
+		reference: 'coresField',
+		minValue: 1,
+		maxValue: 128,
+		allowBlank: true,
+	    },
+	);
+
+	if (me.vmtype === 'qemu') {
+	    items.push({
+		xtype: 'proxmoxintegerfield',
+		fieldLabel: gettext('Sockets'),
+		name: 'sockets',
+		reference: 'socketsField',
+		minValue: 1,
+		maxValue: 4,
+		allowBlank: true,
+	    });
+	}
+
 	let title = gettext('Restore') + ": " + (me.vmtype === 'lxc' ? 'CT' : 'VM');
 	if (me.vmid) {
 	    title = `${gettext('Overwrite')} ${title} ${me.vmid}`;
-- 
2.30.2





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

* [pve-devel] [PATCH v3 manager 3/3] ui: restore: allow treating disks differently
  2022-04-26 12:30 [pve-devel] [PATCH-SERIES v3 qemu-server/manager] more flexible restore Fabian Ebner
                   ` (4 preceding siblings ...)
  2022-04-26 12:30 ` [pve-devel] [PATCH v3 manager 2/3] ui: restore: allow override of some settings Fabian Ebner
@ 2022-04-26 12:30 ` Fabian Ebner
  5 siblings, 0 replies; 11+ messages in thread
From: Fabian Ebner @ 2022-04-26 12:30 UTC (permalink / raw)
  To: pve-devel

Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
---

Changes from v2:
    * Adapt to backend changes with explicit action for each drive.

 www/manager6/Makefile                |   1 +
 www/manager6/grid/RestoreDiskGrid.js | 151 +++++++++++++++++++++++++++
 www/manager6/window/Restore.js       |  35 ++++++-
 3 files changed, 186 insertions(+), 1 deletion(-)
 create mode 100644 www/manager6/grid/RestoreDiskGrid.js

diff --git a/www/manager6/Makefile b/www/manager6/Makefile
index 2c7b1e70..93f9f9c6 100644
--- a/www/manager6/Makefile
+++ b/www/manager6/Makefile
@@ -80,6 +80,7 @@ JSSRC= 							\
 	grid/PoolMembers.js				\
 	grid/Replication.js				\
 	grid/ResourceGrid.js				\
+	grid/RestoreDiskGrid.js				\
 	panel/ConfigPanel.js				\
 	panel/BackupJobPrune.js				\
 	panel/HealthWidget.js				\
diff --git a/www/manager6/grid/RestoreDiskGrid.js b/www/manager6/grid/RestoreDiskGrid.js
new file mode 100644
index 00000000..ded8331a
--- /dev/null
+++ b/www/manager6/grid/RestoreDiskGrid.js
@@ -0,0 +1,151 @@
+Ext.define('pve-restore-disk', {
+    extend: 'Ext.data.Model',
+    fields: [
+	{ name: 'action', type: 'string' },
+	{ name: 'drivename', type: 'string' },
+	{ name: 'vm', type: 'string' },
+	{ name: 'backup', type: 'string' },
+    ],
+});
+
+Ext.define('PVE.grid.RestoreDiskGrid', {
+    extend: 'Ext.grid.Panel',
+    alias: ['widget.pveRestoreDiskGrid'],
+
+    vmConfig: undefined,
+    backupConfig: undefined,
+
+    setVMConfig: function(config) {
+	let me = this;
+
+	if (me.vmConfig) {
+	    throw "vm config already set";
+	}
+	me.vmConfig = config;
+
+	if (me.vmConfig && me.backupConfig) {
+	    me.updateData();
+	}
+    },
+
+    setBackupConfig: function(config) {
+	let me = this;
+
+	if (me.backupConfig) {
+	    throw "backup config already set";
+	}
+	me.backupConfig = config;
+
+	if (me.vmConfig && me.backupConfig) {
+	    me.updateData();
+	}
+    },
+
+    updateData: function() {
+	let me = this;
+
+	let data = [];
+
+	Object.keys(me.vmConfig).concat(Object.keys(me.backupConfig)).forEach(function(key) {
+	    if (!key.match(PVE.Utils.bus_match) && !key.match(/^(efidisk|tpmstate)\d+$/)) {
+		return;
+	    }
+
+	    if (data.find(item => item.drivename === key)) {
+		return;
+	    }
+
+	    data.push({
+		drivename: key,
+		action: me.backupConfig[key] ? 'restore' : 'unused',
+		vm: me.vmConfig[key],
+		backup: me.backupConfig[key],
+	    });
+	});
+
+	me.getStore().setData(data);
+    },
+
+    getDriveActionsValue: function() {
+	let actions = [];
+
+	this.getStore().getData().each(item => {
+	    let action = `${item.data.drivename}=${item.data.action}`;
+	    if (item.data.action === 'restore' && item.data.storage) {
+		action += `:${item.data.storage}`;
+	    }
+	    actions.push(action);
+	});
+
+	return actions.join(',');
+    },
+
+
+    store: {
+	model: 'pve-restore-disk',
+	sorters: 'drivename',
+    },
+
+    columns: [
+	{
+	    xtype: 'widgetcolumn',
+	    header: gettext('Action'),
+	    dataIndex: 'action',
+	    width: 150,
+	    widget: {
+		xtype: 'proxmoxKVComboBox',
+		comboItems: [
+		    ['restore', gettext('Restore')],
+		    ['unused', gettext('Keep as unused')],
+		    ['preserve', gettext('Keep configured')],
+		],
+		listeners: {
+		    change: function(f, value) {
+			let rec = f.getWidgetRecord();
+
+			rec.set('action', value);
+
+			// shouldn't happen, but avoid potential infinite recursion below
+			if (!rec.data.backup && !rec.data.vm) {
+			    return;
+			}
+
+			// auto-correct invalid actions
+			if (value === 'restore' && !rec.data.backup) {
+			    f.setValue('unused');
+			} else if (value === 'unused' && !rec.data.vm) {
+			    f.setValue('restore');
+			}
+		    },
+		},
+	    },
+	},
+	{
+	    header: gettext('Drive'),
+	    dataIndex: 'drivename',
+	    width: 80,
+	},
+	{
+	    header: gettext('Current'),
+	    dataIndex: 'vm',
+	    flex: 1,
+	    renderer: function(drive, metaData, record) {
+		if (record.data.action !== 'restore') {
+		    metaData.tdCls = 'proxmox-good-row';
+		}
+		return drive;
+	    },
+	},
+	{
+	    header: gettext('Backup'),
+	    dataIndex: 'backup',
+	    flex: 1,
+	    renderer: function(drive, metaData, record) {
+		if (record.data.action === 'restore' && drive) {
+		    metaData.tdCls = 'proxmox-good-row';
+		}
+		return drive;
+	    },
+	},
+    ],
+});
diff --git a/www/manager6/window/Restore.js b/www/manager6/window/Restore.js
index 2d78eb56..fb7231c3 100644
--- a/www/manager6/window/Restore.js
+++ b/www/manager6/window/Restore.js
@@ -59,6 +59,11 @@ Ext.define('PVE.window.Restore', {
 		delete params.name;
 	    }
 
+	    let diskGrid = view.lookupReference('restoreDiskGrid');
+	    if (diskGrid) {
+		params['restore-drive-actions'] = diskGrid.getDriveActionsValue();
+	    }
+
 	    let confirmMsg;
 	    if (view.vmtype === 'lxc') {
 		params.ostemplate = view.volid;
@@ -105,6 +110,7 @@ Ext.define('PVE.window.Restore', {
 
 	afterRender: function() {
 	    let view = this.getView();
+	    let diskGrid = view.lookupReference('restoreDiskGrid');
 
 	    Proxmox.Utils.API2Request({
 		url: `/nodes/${view.nodename}/vzdump/extractconfig`,
@@ -116,6 +122,7 @@ Ext.define('PVE.window.Restore', {
 		failure: response => Ext.Msg.alert('Error', response.htmlStatus),
 		success: function(response, options) {
 		    let allStoragesAvailable = true;
+		    let config = {};
 
 		    response.result.data.split('\n').forEach(line => {
 			let [_, key, value] = line.match(/^([^:]+):\s*(\S+)\s*$/) ?? [];
@@ -129,7 +136,11 @@ Ext.define('PVE.window.Restore', {
 			    // if a /dev/XYZ disk was backed up, ther is no storage hint
 			    allStoragesAvailable &&= !!match[3] && !!PVE.data.ResourceStore.getById(
 				`storage/${view.nodename}/${match[3]}`);
-			} else if (key === 'name' || key === 'hostname') {
+			} else {
+			    config[key] = value;
+			}
+
+			if (key === 'name' || key === 'hostname') {
 			    view.lookupReference('nameField').setEmptyText(value);
 			} else if (key === 'memory' || key === 'cores' || key === 'sockets') {
 			    view.lookupReference(`${key}Field`).setEmptyText(value);
@@ -141,8 +152,22 @@ Ext.define('PVE.window.Restore', {
 			storagesel.allowBlank = false;
 			storagesel.setEmptyText('');
 		    }
+
+		    diskGrid?.setBackupConfig(config);
 		},
 	    });
+
+	    if (!diskGrid) {
+		return;
+	    }
+
+	    Proxmox.Utils.API2Request({
+		url: `/nodes/${view.nodename}/qemu/${view.vmid}/config`,
+		waitMsgTarget: diskGrid,
+		method: 'GET',
+		failure: response => Ext.Msg.alert(gettext('Error'), response.htmlStatus),
+		success: response => diskGrid.setVMConfig(response.result.data),
+	    });
 	},
     },
 
@@ -334,6 +359,13 @@ Ext.define('PVE.window.Restore', {
 		maxValue: 4,
 		allowBlank: true,
 	    });
+
+	    if (me.vmid) {
+		items.push({
+		    xtype: 'pveRestoreDiskGrid',
+		    reference: 'restoreDiskGrid',
+		});
+	    }
 	}
 
 	let title = gettext('Restore') + ": " + (me.vmtype === 'lxc' ? 'CT' : 'VM');
@@ -343,6 +375,7 @@ Ext.define('PVE.window.Restore', {
 
 	Ext.apply(me, {
 	    title: title,
+	    width: 700,
 	    items: [
 		{
 		    xtype: 'form',
-- 
2.30.2





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

* [pve-devel] applied: [PATCH v3 qemu-server 1/3] api: create: refactor parameter check logic
  2022-04-26 12:30 ` [pve-devel] [PATCH v3 qemu-server 1/3] api: create: refactor parameter check logic Fabian Ebner
@ 2022-04-28  9:12   ` Thomas Lamprecht
  0 siblings, 0 replies; 11+ messages in thread
From: Thomas Lamprecht @ 2022-04-28  9:12 UTC (permalink / raw)
  To: Proxmox VE development discussion, Fabian Ebner

On 26.04.22 14:30, Fabian Ebner wrote:
> In preparation to allow passing along certain parameters together with
> 'archive'. Moving the parameter checks to after the
> conflicts-with-'archive' to ensure that the more telling error will
> trigger first.
> 
> All check helpers should handle empty params fine, but check first
> just to make sure and to avoid all the superfluous function calls.
> 
> Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
> ---
> 
> No changes from v2.
> 
>  PVE/API2/Qemu.pm | 34 ++++++++++++++++++----------------
>  1 file changed, 18 insertions(+), 16 deletions(-)
> 
>

applied, thanks!




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

* [pve-devel] applied: [PATCH v3 qemu-server 2/3] api: create: allow overriding non-disk options during restore
  2022-04-26 12:30 ` [pve-devel] [PATCH v3 qemu-server 2/3] api: create: allow overriding non-disk options during restore Fabian Ebner
@ 2022-04-28  9:12   ` Thomas Lamprecht
  0 siblings, 0 replies; 11+ messages in thread
From: Thomas Lamprecht @ 2022-04-28  9:12 UTC (permalink / raw)
  To: Proxmox VE development discussion, Fabian Ebner

On 26.04.22 14:30, Fabian Ebner wrote:
> Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
> ---
> 
> No changes from v2.
> 
>  PVE/API2/Qemu.pm  |  8 ++++++--
>  PVE/QemuServer.pm | 26 ++++++++++++++++++++------
>  2 files changed, 26 insertions(+), 8 deletions(-)
> 
>

applied, thanks!




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

* [pve-devel] applied: [PATCH v3 manager 1/3] ui: restore: disallow empty storage selection if it wouldn't work
  2022-04-26 12:30 ` [pve-devel] [PATCH v3 manager 1/3] ui: restore: disallow empty storage selection if it wouldn't work Fabian Ebner
@ 2022-04-28  9:13   ` Thomas Lamprecht
  0 siblings, 0 replies; 11+ messages in thread
From: Thomas Lamprecht @ 2022-04-28  9:13 UTC (permalink / raw)
  To: Proxmox VE development discussion, Fabian Ebner

On 26.04.22 14:30, Fabian Ebner wrote:
> Namely, if there is a storage in the backup configuration that's not
> available on the current node.
> 
> Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
> ---
> 
> Changes from v2:
>     * Improve style.
>     * Handle when no storage hint is present correctly.
> 
>  www/manager6/window/Restore.js | 31 +++++++++++++++++++++++++++++++
>  1 file changed, 31 insertions(+)
> 
>

applied, thanks!




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

* [pve-devel] applied: [PATCH v3 manager 2/3] ui: restore: allow override of some settings
  2022-04-26 12:30 ` [pve-devel] [PATCH v3 manager 2/3] ui: restore: allow override of some settings Fabian Ebner
@ 2022-04-28  9:13   ` Thomas Lamprecht
  0 siblings, 0 replies; 11+ messages in thread
From: Thomas Lamprecht @ 2022-04-28  9:13 UTC (permalink / raw)
  To: Proxmox VE development discussion, Fabian Ebner

On 26.04.22 14:30, Fabian Ebner wrote:
> Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
> ---
> 
> Dependency bump for qemu-server needed
> 
> Changes from v2:
>     * Improve style.
> 
>  www/manager6/window/Restore.js | 80 ++++++++++++++++++++++++++++++----
>  1 file changed, 71 insertions(+), 9 deletions(-)
> 
>

applied, thanks!




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

end of thread, other threads:[~2022-04-28  9:13 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-04-26 12:30 [pve-devel] [PATCH-SERIES v3 qemu-server/manager] more flexible restore Fabian Ebner
2022-04-26 12:30 ` [pve-devel] [PATCH v3 qemu-server 1/3] api: create: refactor parameter check logic Fabian Ebner
2022-04-28  9:12   ` [pve-devel] applied: " Thomas Lamprecht
2022-04-26 12:30 ` [pve-devel] [PATCH v3 qemu-server 2/3] api: create: allow overriding non-disk options during restore Fabian Ebner
2022-04-28  9:12   ` [pve-devel] applied: " Thomas Lamprecht
2022-04-26 12:30 ` [pve-devel] [PATCH v3 qemu-server 3/3] restore: allow specifying drive actions " Fabian Ebner
2022-04-26 12:30 ` [pve-devel] [PATCH v3 manager 1/3] ui: restore: disallow empty storage selection if it wouldn't work Fabian Ebner
2022-04-28  9:13   ` [pve-devel] applied: " Thomas Lamprecht
2022-04-26 12:30 ` [pve-devel] [PATCH v3 manager 2/3] ui: restore: allow override of some settings Fabian Ebner
2022-04-28  9:13   ` [pve-devel] applied: " Thomas Lamprecht
2022-04-26 12:30 ` [pve-devel] [PATCH v3 manager 3/3] ui: restore: allow treating disks differently Fabian Ebner

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