[pve-devel] [PATCH manager] ui: restore: improve warning for restoring container with same ID

[pve-devel] [PATCH manager] ui: restore: improve warning for restoring container with same ID

[Fabian Ebner]

It's not clear to users that the "VM data" includes mount point
volumes, including those that are not marked for backup. This is
different from VM restore, where volumes attached at drives not
present in the backup will be kept around as unused volumes.

Several (supposedly newer) users got tripped up by this over the
years, the latest report being [0]. The long term plan is to make the
restore dialog more flexible to be able to select actions for disks
individually, but that will take a bit of time. In the mean time, make
the warning more explicit.

[0]: https://forum.proxmox.com/threads/111760/#post-482045

Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
---
 www/manager6/window/Restore.js | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/www/manager6/window/Restore.js b/www/manager6/window/Restore.js
index 6738d26f..be2efa3b 100644
--- a/www/manager6/window/Restore.js
+++ b/www/manager6/window/Restore.js
@@ -92,7 +92,13 @@ Ext.define('PVE.window.Restore', {
 	    };
 
 	    if (view.vmid) {
-		confirmMsg += '. ' + gettext('This will permanently erase current VM data.');
+		if (view.vmtype === 'lxc') {
+		    confirmMsg += `. ${gettext(
+			'This will permanently erase current CT data including mountpoint volumes.',
+		    )}`;
+		} else {
+		    confirmMsg += `. ${gettext('This will permanently erase current VM data.')}`;
+		}
 		Ext.Msg.confirm(gettext('Confirm'), confirmMsg, function(btn) {
 		    if (btn === 'yes') {
 			executeRestore();
-- 
2.30.2

Re: [pve-devel] [PATCH manager] ui: restore: improve warning for restoring container with same ID

[Fabian Ebner]

Am 05.07.22 um 10:08 schrieb Fabian Ebner:
> It's not clear to users that the "VM data" includes mount point
> volumes, including those that are not marked for backup. This is
> different from VM restore, where volumes attached at drives not
> present in the backup will be kept around as unused volumes.
> 
> Several (supposedly newer) users got tripped up by this over the
> years, the latest report being [0]. The long term plan is to make the
> restore dialog more flexible to be able to select actions for disks
> individually, but that will take a bit of time. In the mean time, make
> the warning more explicit.
> 
> [0]: https://forum.proxmox.com/threads/111760/#post-482045
> 
> Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
> ---
>  www/manager6/window/Restore.js | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/www/manager6/window/Restore.js b/www/manager6/window/Restore.js
> index 6738d26f..be2efa3b 100644
> --- a/www/manager6/window/Restore.js
> +++ b/www/manager6/window/Restore.js
> @@ -92,7 +92,13 @@ Ext.define('PVE.window.Restore', {
>  	    };
>  
>  	    if (view.vmid) {
> -		confirmMsg += '. ' + gettext('This will permanently erase current VM data.');
> +		if (view.vmtype === 'lxc') {
> +		    confirmMsg += `. ${gettext(
> +			'This will permanently erase current CT data including mountpoint volumes.',
> +		    )}`;

Sorry, forgot that the text needs to be on the very same line for
gettext. Will send a v2 with that reorganized.

> +		} else {
> +		    confirmMsg += `. ${gettext('This will permanently erase current VM data.')}`;
> +		}
>  		Ext.Msg.confirm(gettext('Confirm'), confirmMsg, function(btn) {
>  		    if (btn === 'yes') {
>  			executeRestore();