[pve-devel] [PATCH-SERIES widget-toolkit/manager] Add checkbox for removing unreferenced disks on guest removal

[pve-devel] [PATCH-SERIES widget-toolkit/manager] Add checkbox for removing unreferenced disks on guest removal

[Fabian Ebner]

by allowing generic additional items for the SafeDestroy window in
widget-toolkit and deriving a SafeDestroyGuest child class from it.

Also switch over the other SafeDestroy users in PVE.


proxmox-widget-toolkit:

Fabian Ebner (2):
  safe destroy: remove purge checkbox
  safe destroy: allow specifing additional items

 src/window/SafeDestroy.js | 127 +++++++++++++++++---------------------
 1 file changed, 55 insertions(+), 72 deletions(-)


pve-manager:

Fabian Ebner (5):
  ui: add SafeDestroyGuest window
  ui: use new SafeDestroyGuest window
  ui: safe destroy guest: add checkbox for removal of unreferenced disks
  ui: use safe destroy window from proxmox-widget-toolkit
  ui: remove SafeDestroy window

 www/manager6/Makefile                   |   2 +-
 www/manager6/ceph/Pool.js               |   3 +-
 www/manager6/lxc/Config.js              |   3 +-
 www/manager6/qemu/Config.js             |   3 +-
 www/manager6/storage/ImageView.js       |   3 +-
 www/manager6/window/SafeDestroy.js      | 194 ------------------------
 www/manager6/window/SafeDestroyGuest.js |  44 ++++++
 7 files changed, 53 insertions(+), 199 deletions(-)
 delete mode 100644 www/manager6/window/SafeDestroy.js
 create mode 100644 www/manager6/window/SafeDestroyGuest.js

-- 
2.20.1

[pve-devel] [PATCH proxmox-widget-toolkit 1/7] safe destroy: remove purge checkbox

[Fabian Ebner]

To be replaced by generic additional items specified by the user of the window
or child class. AFAICS there are no existing users of this widget yet, so this
shouldn't break anything.

Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
---
 src/window/SafeDestroy.js | 23 +----------------------
 1 file changed, 1 insertion(+), 22 deletions(-)

diff --git a/src/window/SafeDestroy.js b/src/window/SafeDestroy.js
index 87f81b7..91e0f2c 100644
--- a/src/window/SafeDestroy.js
+++ b/src/window/SafeDestroy.js
@@ -18,7 +18,6 @@ Ext.define('Proxmox.window.SafeDestroy', {
     config: {
 	item: {
 	    id: undefined,
-	    purgeable: false,
 	},
 	url: undefined,
 	note: undefined,
@@ -28,10 +27,7 @@ Ext.define('Proxmox.window.SafeDestroy', {
 
     getParams: function() {
 	let me = this;
-	const purgeCheckbox = me.lookupReference('purgeCheckbox');
-	if (purgeCheckbox.checked) {
-	    me.params.purge = 1;
-	}
+
 	if (Ext.Object.isEmpty(me.params)) {
 	    return '';
 	}
@@ -128,17 +124,6 @@ Ext.define('Proxmox.window.SafeDestroy', {
 		    hideTrigger: true,
 		    allowBlank: false,
 		},
-		{
-		    xtype: 'proxmoxcheckbox',
-		    name: 'purge',
-		    reference: 'purgeCheckbox',
-		    boxLabel: gettext('Purge'),
-		    checked: false,
-		    autoEl: {
-			tag: 'div',
-			'data-qtip': gettext('Remove from replication and backup jobs'),
-		    },
-		},
 		{
 		    xtype: 'container',
 		    reference: 'noteContainer',
@@ -198,12 +183,6 @@ Ext.define('Proxmox.window.SafeDestroy', {
 	    throw "no task name specified";
 	}
 
-	if (!item.purgeable) {
-	    const purgeCheckbox = me.lookupReference('purgeCheckbox');
-	    purgeCheckbox.setDisabled(true);
-	    purgeCheckbox.setHidden(true);
-	}
-
 	const confirmField = me.lookupReference('confirmField');
 	msg = gettext('Please enter the ID to confirm') +
 	    ' (' + item.id + ')';
-- 
2.20.1

[pve-devel] [PATCH proxmox-widget-toolkit 2/7] safe destroy: allow specifing additional items

[Fabian Ebner]

to be used to add more checkboxes for PVE's guest destroy use case.

Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
---
 src/window/SafeDestroy.js | 104 ++++++++++++++++++++------------------
 1 file changed, 54 insertions(+), 50 deletions(-)

diff --git a/src/window/SafeDestroy.js b/src/window/SafeDestroy.js
index 91e0f2c..72e7e19 100644
--- a/src/window/SafeDestroy.js
+++ b/src/window/SafeDestroy.js
@@ -15,6 +15,8 @@ Ext.define('Proxmox.window.SafeDestroy', {
     defaultFocus: 'confirmField',
     showProgress: false,
 
+    additionalItems: [],
+
     config: {
 	item: {
 	    id: undefined,
@@ -96,56 +98,6 @@ Ext.define('Proxmox.window.SafeDestroy', {
 	},
     },
 
-    items: [
-	{
-	    xtype: 'component',
-	    cls: [Ext.baseCSSPrefix + 'message-box-icon',
-		   Ext.baseCSSPrefix + 'message-box-warning',
-		   Ext.baseCSSPrefix + 'dlg-icon'],
-	},
-	{
-	    xtype: 'container',
-	    flex: 1,
-	    layout: {
-		type: 'vbox',
-		align: 'stretch',
-	    },
-	    items: [
-		{
-		    xtype: 'component',
-		    reference: 'messageCmp',
-		},
-		{
-		    itemId: 'confirmField',
-		    reference: 'confirmField',
-		    xtype: 'textfield',
-		    name: 'confirm',
-		    labelWidth: 300,
-		    hideTrigger: true,
-		    allowBlank: false,
-		},
-		{
-		    xtype: 'container',
-		    reference: 'noteContainer',
-		    flex: 1,
-		    hidden: true,
-		    layout: {
-			type: 'vbox',
-			align: 'middle',
-		    },
-		    height: 25,
-		    items: [
-			{
-			    xtype: 'component',
-			    reference: 'noteCmp',
-			    width: '300px',
-			    style: 'font-size: smaller; overflow: hidden; white-space: nowrap; text-overflow: ellipsis;',
-			},
-		    ],
-		},
-	    ],
-	},
-    ],
     buttons: [
 	{
 	    reference: 'removeButton',
@@ -157,6 +109,58 @@ Ext.define('Proxmox.window.SafeDestroy', {
     initComponent: function() {
 	let me = this;
 
+	me.items = [
+	    {
+		xtype: 'component',
+		cls: [Ext.baseCSSPrefix + 'message-box-icon',
+		       Ext.baseCSSPrefix + 'message-box-warning',
+		       Ext.baseCSSPrefix + 'dlg-icon'],
+	    },
+	    {
+		xtype: 'container',
+		flex: 1,
+		layout: {
+		    type: 'vbox',
+		    align: 'stretch',
+		},
+		items: [
+		    {
+			xtype: 'component',
+			reference: 'messageCmp',
+		    },
+		    {
+			itemId: 'confirmField',
+			reference: 'confirmField',
+			xtype: 'textfield',
+			name: 'confirm',
+			labelWidth: 300,
+			hideTrigger: true,
+			allowBlank: false,
+		    },
+		].concat(me.additionalItems).concat([
+		    {
+			xtype: 'container',
+			reference: 'noteContainer',
+			flex: 1,
+			hidden: true,
+			layout: {
+			    type: 'vbox',
+			    align: 'middle',
+			},
+			height: 25,
+			items: [
+			    {
+				xtype: 'component',
+				reference: 'noteCmp',
+				width: '300px',
+				style: 'font-size: smaller; overflow: hidden; white-space: nowrap; text-overflow: ellipsis;',
+			    },
+			],
+		    },
+		]),
+	    },
+	];
+
 	me.callParent();
 
 	const item = me.getItem();
-- 
2.20.1

[pve-devel] [PATCH manager 3/7] ui: add SafeDestroyGuest window

[Fabian Ebner]

The purge parameter is always explicitly set, which is different from the
existing behavior, but it does not rely on what the default in the backend is.

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

Dependency bump for proxmox-widget-toolkit is needed.

 www/manager6/Makefile                   |  1 +
 www/manager6/window/SafeDestroyGuest.js | 30 +++++++++++++++++++++++++
 2 files changed, 31 insertions(+)
 create mode 100644 www/manager6/window/SafeDestroyGuest.js

diff --git a/www/manager6/Makefile b/www/manager6/Makefile
index a2f7be6d..6fc4cde5 100644
--- a/www/manager6/Makefile
+++ b/www/manager6/Makefile
@@ -100,6 +100,7 @@ JSSRC= 							\
 	window/Prune.js					\
 	window/Restore.js				\
 	window/SafeDestroy.js				\
+	window/SafeDestroyGuest.js			\
 	window/Settings.js				\
 	window/Snapshot.js				\
 	window/StartupEdit.js				\
diff --git a/www/manager6/window/SafeDestroyGuest.js b/www/manager6/window/SafeDestroyGuest.js
new file mode 100644
index 00000000..d6121594
--- /dev/null
+++ b/www/manager6/window/SafeDestroyGuest.js
@@ -0,0 +1,30 @@
+/*
+ * SafeDestroy window with additional checkboxes for removing guests
+ */
+Ext.define('PVE.window.SafeDestroyGuest', {
+    extend: 'Proxmox.window.SafeDestroy',
+    alias: 'proxmoxSafeDestroy',
+
+    additionalItems: [
+	{
+	    xtype: 'proxmoxcheckbox',
+	    name: 'purge',
+	    reference: 'purgeCheckbox',
+	    boxLabel: gettext('Purge'),
+	    checked: false,
+	    autoEl: {
+		tag: 'div',
+		'data-qtip': gettext('Remove from replication and backup jobs'),
+	    },
+	},
+    ],
+
+    getParams: function() {
+	let me = this;
+
+	const purgeCheckbox = me.lookupReference('purgeCheckbox');
+	me.params.purge = purgeCheckbox.checked ? 1 : 0;
+
+	return me.callParent();
+    },
+});
-- 
2.20.1

[pve-devel] [PATCH manager 4/7] ui: use new SafeDestroyGuest window

[Fabian Ebner]

Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
---
 www/manager6/lxc/Config.js  | 3 ++-
 www/manager6/qemu/Config.js | 3 ++-
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/www/manager6/lxc/Config.js b/www/manager6/lxc/Config.js
index 73e34614..f996bad1 100644
--- a/www/manager6/lxc/Config.js
+++ b/www/manager6/lxc/Config.js
@@ -150,9 +150,10 @@ Ext.define('PVE.lxc.Config', {
 		    disabled: !caps.vms['VM.Allocate'],
 		    itemId: 'removeBtn',
 		    handler: function() {
-			Ext.create('PVE.window.SafeDestroy', {
+			Ext.create('PVE.window.SafeDestroyGuest', {
 			    url: base_url,
 			    item: { type: 'CT', id: vmid },
+			    taskName: 'vzdestroy',
 			}).show();
 		    },
 		    iconCls: 'fa fa-trash-o',
diff --git a/www/manager6/qemu/Config.js b/www/manager6/qemu/Config.js
index 10bf10a4..21bf3749 100644
--- a/www/manager6/qemu/Config.js
+++ b/www/manager6/qemu/Config.js
@@ -125,9 +125,10 @@ Ext.define('PVE.qemu.Config', {
 		    itemId: 'removeBtn',
 		    disabled: !caps.vms['VM.Allocate'],
 		    handler: function() {
-			Ext.create('PVE.window.SafeDestroy', {
+			Ext.create('PVE.window.SafeDestroyGuest', {
 			    url: base_url,
 			    item: { type: 'VM', id: vmid },
+			    taskName: 'qmdestroy',
 			}).show();
 		    },
 		    iconCls: 'fa fa-trash-o',
-- 
2.20.1

[pve-devel] [PATCH manager 5/7] ui: safe destroy guest: add checkbox for removal of unreferenced disks

[Fabian Ebner]

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

Dependency bump for pve-container and qemu-server is needed. Ideally, the
dependency bump for qemu-server also includes [0], so people with snapshots
don't run into left-over VM state disks.

[0]: https://lists.proxmox.com/pipermail/pve-devel/2021-April/047685.html

 www/manager6/window/SafeDestroyGuest.js | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/www/manager6/window/SafeDestroyGuest.js b/www/manager6/window/SafeDestroyGuest.js
index d6121594..a8a86d07 100644
--- a/www/manager6/window/SafeDestroyGuest.js
+++ b/www/manager6/window/SafeDestroyGuest.js
@@ -17,6 +17,17 @@ Ext.define('PVE.window.SafeDestroyGuest', {
 		'data-qtip': gettext('Remove from replication and backup jobs'),
 	    },
 	},
+	{
+	    xtype: 'proxmoxcheckbox',
+	    name: 'destroyUnreferenced',
+	    reference: 'destroyUnreferencedCheckbox',
+	    boxLabel: gettext('Destroy unreferenced disks'),
+	    checked: false,
+	    autoEl: {
+		tag: 'div',
+		'data-qtip': gettext('Scan all storages for unreferenced disks'),
+	    },
+	},
     ],
 
     getParams: function() {
@@ -25,6 +36,9 @@ Ext.define('PVE.window.SafeDestroyGuest', {
 	const purgeCheckbox = me.lookupReference('purgeCheckbox');
 	me.params.purge = purgeCheckbox.checked ? 1 : 0;
 
+	const destroyUnreferencedCheckbox = me.lookupReference('destroyUnreferencedCheckbox');
+	me.params["destroy-unreferenced-disks"] = destroyUnreferencedCheckbox.checked ? 1 : 0;
+
 	return me.callParent();
     },
 });
-- 
2.20.1

[pve-devel] [PATCH manager 6/7] ui: use safe destroy window from proxmox-widget-toolkit

[Fabian Ebner]

for the remaining (i.e. non-guests) users.

Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
---
 www/manager6/ceph/Pool.js         | 3 ++-
 www/manager6/storage/ImageView.js | 3 ++-
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/www/manager6/ceph/Pool.js b/www/manager6/ceph/Pool.js
index 5dabd4e6..910fcb65 100644
--- a/www/manager6/ceph/Pool.js
+++ b/www/manager6/ceph/Pool.js
@@ -238,13 +238,14 @@ Ext.define('PVE.node.CephPoolList', {
 		var base_url = '/nodes/' + nodename + '/ceph/pools/' +
 		    rec.data.pool_name;
 
-		var win = Ext.create('PVE.window.SafeDestroy', {
+		var win = Ext.create('Proxmox.window.SafeDestroy', {
 		    showProgress: true,
 		    url: base_url,
 		    params: {
 			remove_storages: 1,
 		    },
 		    item: { type: 'CephPool', id: rec.data.pool_name },
+		    taskName: 'cephdestroypool',
 		}).show();
 		win.on('destroy', function() {
 		    rstore.load();
diff --git a/www/manager6/storage/ImageView.js b/www/manager6/storage/ImageView.js
index c828116d..612c419b 100644
--- a/www/manager6/storage/ImageView.js
+++ b/www/manager6/storage/ImageView.js
@@ -57,11 +57,12 @@ Ext.define('PVE.storage.ImageView', {
 			    return;
 			}
 		    }
-		    var win = Ext.create('PVE.window.SafeDestroy', {
+		    var win = Ext.create('Proxmox.window.SafeDestroy', {
 			title: Ext.String.format(gettext("Destroy '{0}'"), rec.data.volid),
 			showProgress: true,
 			url: url,
 			item: { type: 'Image', id: vmid },
+			taskName: 'unknownimgdel',
 		    }).show();
 		    win.on('destroy', reload);
 		},
-- 
2.20.1

[pve-devel] [PATCH manager 7/7] ui: remove SafeDestroy window

[Fabian Ebner]

which has been moved to proxmox-widget-toolkit.

Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
---
 www/manager6/Makefile              |   1 -
 www/manager6/window/SafeDestroy.js | 194 -----------------------------
 2 files changed, 195 deletions(-)
 delete mode 100644 www/manager6/window/SafeDestroy.js

diff --git a/www/manager6/Makefile b/www/manager6/Makefile
index 6fc4cde5..e3b8d78e 100644
--- a/www/manager6/Makefile
+++ b/www/manager6/Makefile
@@ -99,7 +99,6 @@ JSSRC= 							\
 	window/NotesEdit.js				\
 	window/Prune.js					\
 	window/Restore.js				\
-	window/SafeDestroy.js				\
 	window/SafeDestroyGuest.js			\
 	window/Settings.js				\
 	window/Snapshot.js				\
diff --git a/www/manager6/window/SafeDestroy.js b/www/manager6/window/SafeDestroy.js
deleted file mode 100644
index 5acb6e32..00000000
--- a/www/manager6/window/SafeDestroy.js
+++ /dev/null
@@ -1,194 +0,0 @@
-/* Popup a message window
- * where the user has to manually enter the resource ID
- * to enable the destroy button
- */
-Ext.define('PVE.window.SafeDestroy', {
-    extend: 'Ext.window.Window',
-    alias: 'widget.pveSafeDestroy',
-
-    title: gettext('Confirm'),
-    modal: true,
-    buttonAlign: 'center',
-    bodyPadding: 10,
-    width: 450,
-    layout: { type: 'hbox' },
-    defaultFocus: 'confirmField',
-    showProgress: false,
-
-    config: {
-	item: {
-	    id: undefined,
-	    type: undefined,
-	},
-	url: undefined,
-	params: {},
-    },
-
-    getParams: function() {
-	var me = this;
-	var purgeCheckbox = me.lookupReference('purgeCheckbox');
-	if (purgeCheckbox.checked) {
-	    me.params.purge = 1;
-	}
-	if (Ext.Object.isEmpty(me.params)) {
-	    return '';
-	}
-	return '?' + Ext.Object.toQueryString(me.params);
-    },
-
-    controller: {
-
-	xclass: 'Ext.app.ViewController',
-
-	control: {
-	    'field[name=confirm]': {
-		change: function(f, value) {
-		    var view = this.getView();
-		    var removeButton = this.lookupReference('removeButton');
-		    if (value === view.getItem().id.toString()) {
-			removeButton.enable();
-		    } else {
-			removeButton.disable();
-		    }
-		},
-		specialkey: function(field, event) {
-		    var removeButton = this.lookupReference('removeButton');
-		    if (!removeButton.isDisabled() && event.getKey() == event.ENTER) {
-			removeButton.fireEvent('click', removeButton, event);
-		    }
-		},
-	    },
-           'button[reference=removeButton]': {
-		click: function() {
-		    var view = this.getView();
-		    Proxmox.Utils.API2Request({
-			url: view.getUrl() + view.getParams(),
-			method: 'DELETE',
-			waitMsgTarget: view,
-			failure: function(response, opts) {
-			    view.close();
-			    Ext.Msg.alert('Error', response.htmlStatus);
-			},
-			success: function(response, options) {
-			    var hasProgressBar = !!(view.showProgress &&
-				response.result.data);
-
-			    if (hasProgressBar) {
-				// stay around so we can trigger our close events
-				// when background action is completed
-				view.hide();
-
-				var upid = response.result.data;
-				var win = Ext.create('Proxmox.window.TaskProgress', {
-				    upid: upid,
-				    listeners: {
-					destroy: function() {
-					    view.close();
-					},
-				    },
-				});
-				win.show();
-			    } else {
-				view.close();
-			    }
-			},
-		    });
-		},
-            },
-	},
-    },
-
-    items: [
-	{
-	    xtype: 'component',
-	    cls: [Ext.baseCSSPrefix + 'message-box-icon',
-		   Ext.baseCSSPrefix + 'message-box-warning',
-		   Ext.baseCSSPrefix + 'dlg-icon'],
-	},
-	{
-	    xtype: 'container',
-	    flex: 1,
-	    layout: {
-		type: 'vbox',
-		align: 'stretch',
-	    },
-	    items: [
-		{
-		    xtype: 'component',
-		    reference: 'messageCmp',
-		},
-		{
-		    itemId: 'confirmField',
-		    reference: 'confirmField',
-		    xtype: 'textfield',
-		    name: 'confirm',
-		    labelWidth: 300,
-		    hideTrigger: true,
-		    allowBlank: false,
-		},
-		{
-		    xtype: 'proxmoxcheckbox',
-		    name: 'purge',
-		    reference: 'purgeCheckbox',
-		    boxLabel: gettext('Purge'),
-		    checked: false,
-		    autoEl: {
-			tag: 'div',
-			'data-qtip': gettext('Remove from replication & backup jobs and HA resource configuration.'),
-		    },
-		},
-	    ],
-	},
-    ],
-    buttons: [
-	{
-	    reference: 'removeButton',
-	    text: gettext('Remove'),
-	    disabled: true,
-	},
-    ],
-
-    initComponent: function() {
-	var me = this;
-
-	me.callParent();
-
-	var item = me.getItem();
-
-	if (!Ext.isDefined(item.id)) {
-	    throw "no ID specified";
-	}
-
-	if (!Ext.isDefined(item.type)) {
-	    throw "no VM type specified";
-	}
-
-	var messageCmp = me.lookupReference('messageCmp');
-	var msg;
-
-	if (item.type === 'VM') {
-	    msg = Proxmox.Utils.format_task_description('qmdestroy', item.id);
-	} else if (item.type === 'CT') {
-	    msg = Proxmox.Utils.format_task_description('vzdestroy', item.id);
-	} else if (item.type === 'CephPool') {
-	    msg = Proxmox.Utils.format_task_description('cephdestroypool', item.id);
-	} else if (item.type === 'Image') {
-	    msg = Proxmox.Utils.format_task_description('unknownimgdel', item.id);
-	} else {
-	    throw "unknown item type specified";
-	}
-
-	messageCmp.setHtml(msg);
-
-	if (!(item.type === 'VM' || item.type === 'CT')) {
-	    let purgeCheckbox = me.lookupReference('purgeCheckbox');
-	    purgeCheckbox.setDisabled(true);
-	    purgeCheckbox.setHidden(true);
-	}
-
-	var confirmField = me.lookupReference('confirmField');
-	msg = gettext('Please enter the ID to confirm') +
-	    ' (' + item.id + ')';
-	confirmField.setFieldLabel(msg);
-    },
-});
-- 
2.20.1

[pve-devel] applied: [PATCH-SERIES widget-toolkit/manager] Add checkbox for removing unreferenced disks on guest removal

[Thomas Lamprecht]

On 19.04.21 15:14, Fabian Ebner wrote:
> by allowing generic additional items for the SafeDestroy window in
> widget-toolkit and deriving a SafeDestroyGuest child class from it.
> 
> Also switch over the other SafeDestroy users in PVE.
> 
> 
> proxmox-widget-toolkit:
> 
> Fabian Ebner (2):
>   safe destroy: remove purge checkbox
>   safe destroy: allow specifing additional items

> pve-manager:
> 
> Fabian Ebner (5):
>   ui: add SafeDestroyGuest window
>   ui: use new SafeDestroyGuest window
>   ui: safe destroy guest: add checkbox for removal of unreferenced disks
>   ui: use safe destroy window from proxmox-widget-toolkit
>   ui: remove SafeDestroy window


applied, much thanks!