[pve-devel] [PATCH manager 1/3] GUI: Allow passing the node to BackupConfig directly.

[pve-devel] [PATCH manager 1/3] GUI: Allow passing the node to BackupConfig directly.

[Matthias Heiserer]

Signed-off-by: Matthias Heiserer <m.heiserer@proxmox.com>
---
 www/manager6/window/BackupConfig.js | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/www/manager6/window/BackupConfig.js b/www/manager6/window/BackupConfig.js
index ca61b1e4..9609fe34 100644
--- a/www/manager6/window/BackupConfig.js
+++ b/www/manager6/window/BackupConfig.js
@@ -24,7 +24,7 @@ Ext.define('PVE.window.BackupConfig', {
 	    throw "no volume specified";
 	}
 
-	var nodename = me.pveSelNode.data.node;
+	var nodename = me.node ?? me.pveSelNode.data.node;
 	if (!nodename) {
 	    throw "no node name specified";
 	}
-- 
2.30.2

[pve-devel] [PATCH manager 2/3] GUI: Utils: Helpers for backup type and icon

[Matthias Heiserer]

Signed-off-by: Matthias Heiserer <m.heiserer@proxmox.com>
---
 www/manager6/Utils.js | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/www/manager6/Utils.js b/www/manager6/Utils.js
index aafe359a..650eeee9 100644
--- a/www/manager6/Utils.js
+++ b/www/manager6/Utils.js
@@ -1803,6 +1803,26 @@ Ext.define('PVE.Utils', {
 
 	return undefined;
     },
+
+    get_type_icon_cls: function(volid, format) {
+	var cls = '';
+	if (PVE.Utils.volume_is_qemu_backup(volid, format)) {
+	    cls = 'fa-desktop';
+	} else if (PVE.Utils.volume_is_lxc_backup(volid, format)) {
+	    cls = 'fa-cube';
+	}
+	return cls;
+    },
+
+    get_backup_type: function(volid, format) {
+	if (PVE.Utils.volume_is_qemu_backup(volid, format)) {
+	    return 'qemu';
+	} else if (PVE.Utils.volume_is_lxc_backup(volid, format)) {
+	    return 'lxc';
+	} else {
+	    throw 'Unknown backup type';
+	}
+    },
 },
 
     singleton: true,
-- 
2.30.2

[pve-devel] [PATCH manager 3/3] Storage GUI: Rewrite backup content view as TreePanel.

[Matthias Heiserer]

Should be easier to read/use than the current flat list.
Backups are grouped by ID and type, so in case there are backups
with ID 100 for both CT and VM, this would create two separate
groups in the UI.
Date and size of group are taken from the latest backup.
Notes, Protection, Encrypted, and Verify State stay as default
value empty, empty, No, and None, respectively.

Code adapted from the existing backup view and the pbs
datastore content, where appropriate.

Signed-off-by: Matthias Heiserer <m.heiserer@proxmox.com>
---
 www/manager6/storage/BackupView.js | 620 ++++++++++++++++++++---------
 1 file changed, 433 insertions(+), 187 deletions(-)

diff --git a/www/manager6/storage/BackupView.js b/www/manager6/storage/BackupView.js
index 2328c0fc..124a57c9 100644
--- a/www/manager6/storage/BackupView.js
+++ b/www/manager6/storage/BackupView.js
@@ -1,36 +1,384 @@
-Ext.define('PVE.storage.BackupView', {
-    extend: 'PVE.storage.ContentView',
+Ext.define('pve-data-store-backups', {
+    extend: 'Ext.data.Model',
+    fields: [
+	{
+	    name: 'ctime',
+	    date: 'date',
+	    dateFormat: 'timestamp',
+	},
+	'format',
+	'volid',
+	'content',
+	'vmid',
+	'size',
+	'protected',
+	'notes',
+    ],
+});
+
 
+Ext.define('PVE.storage.BackupView', {
+    extend: 'Ext.tree.Panel',
     alias: 'widget.pveStorageBackupView',
+    mixins: ['Proxmox.Mixin.CBind'],
+    rootVisible: false,
+
+    title: gettext('Content'),
+
+    cbindData: function(initialCfg) {
+	return {
+	    notPBS: initialCfg.pluginType !== 'pbs',
+	};
+    },
+
+    controller: {
+	xclass: 'Ext.app.ViewController',
+
+	init: function(view) {
+	    let me = this;
+	    me.storage = view.pveSelNode.data.storage;
+	    if (!me.storage) {
+		throw "No datastore specificed";
+	    }
+	    me.nodename = view.pveSelNode.data.node;
 
-    showColumns: ['name', 'notes', 'protected', 'date', 'format', 'size'],
+	    me.store = Ext.create('Ext.data.Store', {
+		model: 'pve-data-store-backups',
+		groupField: 'vmid',
+		filterer: 'bottomup',
+	    });
+	    me.store.on('load', me.onLoad, me);
+
+	    view.getStore().setSorters([
+		'vmid',
+		'text',
+		'backup-time',
+	    ]);
+	    view.getStore().setConfig('filterer', 'bottomup');
+	    Proxmox.Utils.monStoreErrors(view, me.store);
+	},
+
+	onLoad: function(store, records, success, operation) {
+	    let me = this;
+	    let view = me.getView();
 
-    initComponent: function() {
-	var me = this;
+	    let expanded = {};
+	    view.getRootNode().cascadeBy({
+		before: item => {
+		    if (item.isExpanded() && !item.data.leaf) {
+			let id = item.data.text;
+			expanded[id] = true;
+			return true;
+		    }
+		    return false;
+		},
+		after: Ext.emptyFn,
+	    });
+	    let groups = me.getRecordGroups(records, expanded);
 
-	var nodename = me.nodename = me.pveSelNode.data.node;
-	if (!nodename) {
-	    throw "no node name specified";
-	}
+	    for (const item of records.map(i => i.data)) {
+		item.text = item.volid;
+		item.leaf = true;
+		item.ctime = new Date(item.ctime * 1000);
+		item.iconCls = 'fa-file-o';
+		groups[`${item.format}` + item.vmid].children.push(item);
+	    }
+
+	    for (let [_name, group] of Object.entries(groups)) {
+		let c = group.children;
+		let latest = c.reduce((l, r) => l.ctime > r.ctime ? l : r);
+		let num_verified = c.reduce((l, r) => l + r.verification === 'ok', 0);
+		group.ctime = latest.ctime;
+		group.size = latest.size;
+		group.verified = num_verified / c.length;
+	    }
+
+	    let children = [];
+	    Object.entries(groups).forEach(e => children.push(e[1]));
+	    view.setRootNode({
+		expanded: true,
+		children: children,
+	    });
 
-	var storage = me.storage = me.pveSelNode.data.storage;
-	if (!storage) {
-	    throw "no storage ID specified";
-	}
+	    Proxmox.Utils.setErrorMask(view, false);
+	},
 
-	me.content = 'backup';
+	reload: function() {
+	    let me = this;
+	    let view = me.getView();
+	    if (!view.store || !me.store) {
+		console.warn('cannot reload, no store(s)');
+		return;
+	    }
 
-	var sm = me.sm = Ext.create('Ext.selection.RowModel', {});
+	    let url = `/api2/json/nodes/${me.nodename}/storage/${me.storage}/content`;
+	    me.store.setProxy({
+		type: 'proxmox',
+		timeout: 60*1000,
+		url: url,
+		extraParams: {
+		    content: 'backup',
+		},
+	    });
 
-	var reload = function() {
 	    me.store.load();
-	};
+	    Proxmox.Utils.monStoreErrors(view, me.store);
+	},
 
-	let pruneButton = Ext.create('Proxmox.button.Button', {
-	    text: gettext('Prune group'),
+	getRecordGroups: function(records, expanded) {
+	    let groups = {};
+	    for (const item of records) {
+		groups[`${item.data.format}` + item.data.vmid] = {
+		    vmid: item.data.vmid,
+		    leaf: false,
+		    children: [],
+		    expanded: !!expanded[item.data.vmid],
+		    text: item.data.vmid,
+		    ctime: 0,
+		    format: item.data.format,
+		    volid: "volid",
+		    content: "content",
+		    size: 0,
+		    iconCls: PVE.Utils.get_type_icon_cls(item.data.volid, item.data.format),
+		};
+	    }
+	    return groups;
+	},
+
+	restoreHandler: function(button, event, rec) {
+	    let me = this;
+	    let vmtype = PVE.Utils.get_backup_type(rec.data.volid, rec.data.format);
+	    let win = Ext.create('PVE.window.Restore', {
+		nodename: me.nodename,
+		volid: rec.data.volid,
+		volidText: PVE.Utils.render_storage_content(rec.data.volid, {}, rec),
+		vmtype: vmtype,
+		isPBS: me.isPBS,
+		view: me.view,
+	    });
+	    win.on('destroy', () => me.reload());
+	    win.show();
+	},
+
+	restoreFilesHandler: function(button, event, rec) {
+	    let me = this;
+	    let isVMArchive = PVE.Utils.volume_is_qemu_backup(rec.data.volid, rec.data.format);
+	    Ext.create('Proxmox.window.FileBrowser', {
+		title: gettext('File Restore') + " - " + rec.data.text,
+		listURL: `/api2/json/nodes/localhost/storage/${me.storage}/file-restore/list`,
+		downloadURL: `/api2/json/nodes/localhost/storage/${me.storage}/file-restore/download`,
+		extraParams: {
+		    volume: rec.data.volid,
+		},
+		archive: isVMArchive ? 'all' : undefined,
+		autoShow: true,
+	    });
+	},
+
+	showConfigurationHandler: function(button, event, rec) {
+	    let win = Ext.create('PVE.window.BackupConfig', {
+		volume: rec.data.volid,
+		node: this.nodename,
+	    });
+	    win.show();
+	},
+
+	editNotesHandler: function(button, event, rec) {
+	    let me = this;
+	    let volid = rec.data.volid;
+	    Ext.create('Proxmox.window.Edit', {
+		autoLoad: true,
+		width: 600,
+		height: 400,
+		resizable: true,
+		title: gettext('Notes'),
+		url: `/api2/extjs/nodes/${me.nodename}/storage/${me.storage}/content/${volid}`,
+		layout: 'fit',
+		items: [
+		    {
+			xtype: 'textarea',
+			layout: 'fit',
+			name: 'notes',
+			height: '100%',
+		    },
+		],
+		listeners: {
+		    destroy: () => me.reload(),
+		},
+	    }).show();
+	},
+
+	changeProtectionHandler: function(button, event, rec) {
+	    let me = this;
+	    const volid = rec.data.volid;
+	    Proxmox.Utils.API2Request({
+		url: `/api2/extjs/nodes/${me.nodename}/storage/${me.storage}/content/${volid}`,
+		method: 'PUT',
+		waitMsgTarget: button,
+		params: { 'protected': rec.data.protected ? 0 : 1 },
+		failure: (response) => Ext.Msg.alert('Error', response.htmlStatus),
+		success: (_) => me.reload(),
+	    });
+	},
+
+	pruneGroupHandler: function(button, event, rec) {
+	    let me = this;
+	    let vmtype = PVE.Utils.get_backup_type(rec.data.volid, rec.data.format);
+	    Ext.create('PVE.window.Prune', {
+		nodename: me.nodename,
+		storage: me.storage,
+		backup_id: rec.data.vmid,
+		backup_type: vmtype,
+		rec: rec,
+		listeners: {
+		    destroy: () => me.reload(),
+		},
+	    }).show();
+	},
+
+	removeHandler: function(button, event, rec) {
+	    let me = this;
+	    const volid = rec.data.volid;
+	    Proxmox.Utils.API2Request({
+		url: `/nodes/${me.nodename}/storage/${me.storage}/content//${volid}`,
+		method: 'DELETE',
+		callback: () => me.reload(),
+		failure: response => Ext.Msg.alert(gettext('Error'), response.htmlStatus),
+	    });
+	},
+
+	searchKeyupFn: function(field) {
+	    this.getView().getStore().clearFilter(true);
+	    this.getView().getStore().filter([
+		{
+		    property: 'volid',
+		    value: field.getValue(),
+		    anyMatch: true,
+		    caseSensitive: false,
+		},
+	    ]);
+	},
+
+	searchClearHandler: function(field) {
+	    field.triggers.clear.setVisible(false);
+	    field.setValue(this.originalValue);
+	    this.getView().getStore().clearFilter();
+	},
+
+	searchChangeFn: function(field, newValue, oldValue) {
+	    if (newValue !== field.originalValue) {
+		field.triggers.clear.setVisible(true);
+	    }
+	},
+    },
+
+    columns: [
+	{
+	    xtype: 'treecolumn',
+	    header: gettext("Backup Group"),
+	    dataIndex: 'text',
+	    flex: 2,
+	},
+	{
+	    header: gettext('Notes'),
+	    flex: 1,
+	    renderer: Ext.htmlEncode,
+	    dataIndex: 'notes',
+	},
+	{
+	    header: `<i class="fa fa-shield"></i>`,
+	    tooltip: gettext('Protected'),
+	    width: 30,
+	    renderer: v => v ? `<i data-qtip="${gettext('Protected')}" class="fa fa-shield"></i>` : '',
+	    sorter: (a, b) => (b.data.protected || 0) - (a.data.protected || 0),
+	    dataIndex: 'protected',
+	},
+	{
+	    header: gettext('Date'),
+	    width: 150,
+	    dataIndex: 'ctime',
+	    xtype: 'datecolumn',
+	    format: 'Y-m-d H:i:s',
+	},
+	{
+	    header: gettext('Format'),
+	    width: 100,
+	    dataIndex: 'format',
+	},
+	{
+	    header: gettext('Size'),
+	    width: 100,
+	    renderer: Proxmox.Utils.format_size,
+	    dataIndex: 'size',
+	},
+	{
+	    header: gettext('Encrypted'),
+	    dataIndex: 'encrypted',
+	    renderer: PVE.Utils.render_backup_encryption,
+	    cbind: {
+		hidden: '{notPBS}',
+	    },
+	},
+	{
+	    header: gettext('Verify State'),
+	    dataIndex: 'verification',
+	    renderer: PVE.Utils.render_backup_verification,
+	    cbind: {
+		hidden: '{notPBS}',
+	    },
+	},
+    ],
+
+    tbar: [
+	{
+	    xtype: 'proxmoxButton',
+	    text: gettext('Restore'),
+	    handler: 'restoreHandler',
+	    parentXType: "treepanel",
+	    disabled: true,
+	    enableFn: record => record.phantom === false,
+	},
+	{
+	    xtype: 'proxmoxButton',
+	    text: gettext('File Restore'),
+	    handler: 'restoreFilesHandler',
+	    cbind: {
+		hidden: '{notPBS}',
+	    },
+	    parentXType: "treepanel",
+	    disabled: true,
+	    enableFn: record => record.phantom === false,
+	},
+	{
+	    xtype: 'proxmoxButton',
+	    text: gettext('Show Configuration'),
+	    handler: 'showConfigurationHandler',
+	    parentXType: "treepanel",
+	    disabled: true,
+	    enableFn: record => record.phantom === false,
+	},
+	{
+	    xtype: 'proxmoxButton',
+	    text: gettext('Edit Notes'),
+	    handler: 'editNotesHandler',
+	    parentXType: "treepanel",
 	    disabled: true,
-	    selModel: sm,
+	    enableFn: record => record.phantom === false,
+	},
+	{
+	    xtype: 'proxmoxButton',
+	    text: gettext('Change Protection'),
+	    handler: 'changeProtectionHandler',
+	    parentXType: "treepanel",
+	    disabled: true,
+	    enableFn: record => record.phantom === false,
+	},
+	'-',
+	{
+	    xtype: 'proxmoxButton',
+	    text: gettext('Prune group'),
 	    setBackupGroup: function(backup) {
+		let me = this;
 		if (backup) {
 		    let name = backup.text;
 		    let vmid = backup.vmid;
@@ -38,186 +386,84 @@ Ext.define('PVE.storage.BackupView', {
 
 		    let vmtype;
 		    if (name.startsWith('vzdump-lxc-') || format === "pbs-ct") {
-			vmtype = 'lxc';
+		        vmtype = 'lxc';
 		    } else if (name.startsWith('vzdump-qemu-') || format === "pbs-vm") {
-			vmtype = 'qemu';
+		        vmtype = 'qemu';
 		    }
-
 		    if (vmid && vmtype) {
-			this.setText(gettext('Prune group') + ` ${vmtype}/${vmid}`);
-			this.vmid = vmid;
-			this.vmtype = vmtype;
-			this.setDisabled(false);
+			me.setText(gettext('Prune group') + ` ${vmtype}/${vmid}`);
+			me.vmid = vmid;
+			me.vmtype = vmtype;
+			me.setDisabled(false);
 			return;
 		    }
 		}
-		this.setText(gettext('Prune group'));
-		this.vmid = null;
-		this.vmtype = null;
-		this.setDisabled(true);
+		me.setText(gettext('Prune group'));
+		me.vmid = null;
+		me.vmtype = null;
+		me.setDisabled(true);
+	    },
+	    handler: 'pruneGroupHandler',
+	    parentXType: "treepanel",
+	    disabled: true,
+	    reference: 'pruneButton',
+	    enableFn: () => true,
+	},
+	{
+	    xtype: 'proxmoxButton',
+	    text: gettext('Remove'),
+	    handler: 'removeHandler',
+	    parentXType: 'treepanel',
+	    disabled: true,
+	    enableFn: record => record.phantom === false && !record?.data?.protected,
+	    confirmMsg: function(rec) {
+		console.log("controller:", this.getController());
+		let name = rec.data.text;
+		return Ext.String.format(gettext('Are you sure you want to remove entry {0}'), `'${name}'`);
+	    },
+	},
+	'->',
+	gettext('Search') + ':',
+	' ',
+	{
+	    xtype: 'textfield',
+	    width: 200,
+	    enableKeyEvents: true,
+	    emptyText: gettext('Name, Format'),
+	    listeners: {
+		keyup: {
+		    buffer: 500,
+		    fn: 'searchKeyupFn',
+		},
+		change: 'searchChangeFn',
 	    },
-	    handler: function(b, e, rec) {
-		let win = Ext.create('PVE.window.Prune', {
-		    nodename: nodename,
-		    storage: storage,
-		    backup_id: this.vmid,
-		    backup_type: this.vmtype,
-		});
-		win.show();
-		win.on('destroy', reload);
+	    triggers: {
+		clear: {
+		    cls: 'pmx-clear-trigger',
+		    weight: -1,
+		    hidden: true,
+		    handler: 'searchClearHandler',
+		},
 	    },
-	});
+	},
+	],
 
-	me.on('selectionchange', function(model, srecords, eOpts) {
+    listeners: {
+	activate: function() {
+	    let me = this;
+	    // only load on first activate to not load every tab switch
+	    if (!me.firstLoad) {
+		me.getController().reload();
+		me.firstLoad = true;
+	    }
+	},
+	selectionchange: function(model, srecords, eOpts) {
+	    let pruneButton = this.getController().lookup('pruneButton');
 	    if (srecords.length === 1) {
 		pruneButton.setBackupGroup(srecords[0].data);
 	    } else {
 		pruneButton.setBackupGroup(null);
 	    }
-	});
-
-	let isPBS = me.pluginType === 'pbs';
-
-	me.tbar = [
-	    {
-		xtype: 'proxmoxButton',
-		text: gettext('Restore'),
-		selModel: sm,
-		disabled: true,
-		handler: function(b, e, rec) {
-		    var vmtype;
-		    if (PVE.Utils.volume_is_qemu_backup(rec.data.volid, rec.data.format)) {
-			vmtype = 'qemu';
-		    } else if (PVE.Utils.volume_is_lxc_backup(rec.data.volid, rec.data.format)) {
-			vmtype = 'lxc';
-		    } else {
-			return;
-		    }
-
-		    var win = Ext.create('PVE.window.Restore', {
-			nodename: nodename,
-			volid: rec.data.volid,
-			volidText: PVE.Utils.render_storage_content(rec.data.volid, {}, rec),
-			vmtype: vmtype,
-			isPBS: isPBS,
-		    });
-		    win.show();
-		    win.on('destroy', reload);
-		},
-	    },
-	];
-	if (isPBS) {
-	    me.tbar.push({
-		xtype: 'proxmoxButton',
-		text: gettext('File Restore'),
-		disabled: true,
-		selModel: sm,
-		handler: function(b, e, rec) {
-		    let isVMArchive = PVE.Utils.volume_is_qemu_backup(rec.data.volid, rec.data.format);
-		    Ext.create('Proxmox.window.FileBrowser', {
-			title: gettext('File Restore') + " - " + rec.data.text,
-			listURL: `/api2/json/nodes/localhost/storage/${me.storage}/file-restore/list`,
-			downloadURL: `/api2/json/nodes/localhost/storage/${me.storage}/file-restore/download`,
-			extraParams: {
-			    volume: rec.data.volid,
-			},
-			archive: isVMArchive ? 'all' : undefined,
-			autoShow: true,
-		    });
-		},
-	    });
-	}
-	me.tbar.push(
-	    {
-		xtype: 'proxmoxButton',
-		text: gettext('Show Configuration'),
-		disabled: true,
-		selModel: sm,
-		handler: function(b, e, rec) {
-		    var win = Ext.create('PVE.window.BackupConfig', {
-			volume: rec.data.volid,
-			pveSelNode: me.pveSelNode,
-		    });
-
-		    win.show();
-		},
-	    },
-	    {
-		xtype: 'proxmoxButton',
-		text: gettext('Edit Notes'),
-		disabled: true,
-		selModel: sm,
-		handler: function(b, e, rec) {
-		    let volid = rec.data.volid;
-		    Ext.create('Proxmox.window.Edit', {
-			autoLoad: true,
-			width: 600,
-			height: 400,
-			resizable: true,
-			title: gettext('Notes'),
-			url: `/api2/extjs/nodes/${nodename}/storage/${me.storage}/content/${volid}`,
-			layout: 'fit',
-			items: [
-			    {
-				xtype: 'textarea',
-				layout: 'fit',
-				name: 'notes',
-				height: '100%',
-			    },
-			],
-			listeners: {
-			    destroy: () => reload(),
-			},
-		    }).show();
-		},
-	    },
-	    {
-		xtype: 'proxmoxButton',
-		text: gettext('Change Protection'),
-		disabled: true,
-		handler: function(button, event, record) {
-		    const volid = record.data.volid;
-		    Proxmox.Utils.API2Request({
-			url: `/api2/extjs/nodes/${nodename}/storage/${me.storage}/content/${volid}`,
-			method: 'PUT',
-			waitMsgTarget: me,
-			params: { 'protected': record.data.protected ? 0 : 1 },
-			failure: (response) => Ext.Msg.alert('Error', response.htmlStatus),
-			success: (response) => reload(),
-		    });
-		},
-	    },
-	    '-',
-	    pruneButton,
-	);
-
-	if (isPBS) {
-	    me.extraColumns = {
-		encrypted: {
-		    header: gettext('Encrypted'),
-		    dataIndex: 'encrypted',
-		    renderer: PVE.Utils.render_backup_encryption,
-		},
-		verification: {
-		    header: gettext('Verify State'),
-		    dataIndex: 'verification',
-		    renderer: PVE.Utils.render_backup_verification,
-		},
-	    };
-	}
-
-	me.callParent();
-
-	me.store.getSorters().clear();
-	me.store.setSorters([
-	    {
-		property: 'vmid',
-		direction: 'ASC',
-	    },
-	    {
-		property: 'vdate',
-		direction: 'DESC',
-	    },
-	]);
+	},
     },
 });
-- 
2.30.2

Re: [pve-devel] [PATCH manager 1/3] GUI: Allow passing the node to BackupConfig directly.

[Fabian Ebner]

Am 04.03.22 um 12:52 schrieb Matthias Heiserer:
> Signed-off-by: Matthias Heiserer <m.heiserer@proxmox.com>
> ---

Some rationale for the change would be nice to have in the commit
message. Is there a scenario where me.pveSelNode is not the correct node?

>  www/manager6/window/BackupConfig.js | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/www/manager6/window/BackupConfig.js b/www/manager6/window/BackupConfig.js
> index ca61b1e4..9609fe34 100644
> --- a/www/manager6/window/BackupConfig.js
> +++ b/www/manager6/window/BackupConfig.js
> @@ -24,7 +24,7 @@ Ext.define('PVE.window.BackupConfig', {
>  	    throw "no volume specified";
>  	}
>  
> -	var nodename = me.pveSelNode.data.node;
> +	var nodename = me.node ?? me.pveSelNode.data.node;
>  	if (!nodename) {
>  	    throw "no node name specified";
>  	}

Re: [pve-devel] [PATCH manager 2/3] GUI: Utils: Helpers for backup type and icon

[Fabian Ebner]

Am 04.03.22 um 12:52 schrieb Matthias Heiserer:
> Signed-off-by: Matthias Heiserer <m.heiserer@proxmox.com>
> ---
>  www/manager6/Utils.js | 20 ++++++++++++++++++++
>  1 file changed, 20 insertions(+)
> 
> diff --git a/www/manager6/Utils.js b/www/manager6/Utils.js
> index aafe359a..650eeee9 100644
> --- a/www/manager6/Utils.js
> +++ b/www/manager6/Utils.js
> @@ -1803,6 +1803,26 @@ Ext.define('PVE.Utils', {
>  
>  	return undefined;
>      },
> +
> +    get_type_icon_cls: function(volid, format) {

This name is too generic if it doesn't mention 'backup'.

> +	var cls = '';
> +	if (PVE.Utils.volume_is_qemu_backup(volid, format)) {
> +	    cls = 'fa-desktop';
> +	} else if (PVE.Utils.volume_is_lxc_backup(volid, format)) {
> +	    cls = 'fa-cube';
> +	}
> +	return cls;
> +    },
> +
> +    get_backup_type: function(volid, format) {
> +	if (PVE.Utils.volume_is_qemu_backup(volid, format)) {
> +	    return 'qemu';
> +	} else if (PVE.Utils.volume_is_lxc_backup(volid, format)) {
> +	    return 'lxc';
> +	} else {
> +	    throw 'Unknown backup type';
> +	}
> +    },
>  },
>  
>      singleton: true,

Re: [pve-devel] [PATCH manager 3/3] Storage GUI: Rewrite backup content view as TreePanel.

[Fabian Ebner]

Am 04.03.22 um 12:52 schrieb Matthias Heiserer:
> Should be easier to read/use than the current flat list.
> Backups are grouped by ID and type, so in case there are backups
> with ID 100 for both CT and VM, this would create two separate
> groups in the UI.

It might make sense to include qemu/lxc (for PBS either vm/ct or
qemu/lxc, not too sure) for displaying. Like that all containers backups
would come first like in PBS. Since we don't have notes for groups and
some people rely on notes to find VMs rather than the ID, it might be
better to start with a fully expanded tree.

If I have a manually renamed backup, it'll show below a node with name
'Root', maybe explicitly grouping them as 'Other' or 'None' would be better.

One other thing I noticed is that the selection is not remembered after
changing protection/editing notes. Not a big deal, but would be nice if
not too complicated.

> Date and size of group are taken from the latest backup.

It's a bit tricky. PBS has deduplication so doing that there makes
sense, but I'm sure many people would expect to see the sum of sizes
when dealing with non-PBS storages.

> Notes, Protection, Encrypted, and Verify State stay as default
> value empty, empty, No, and None, respectively.
> 
> Code adapted from the existing backup view and the pbs
> datastore content, where appropriate.
> 
> Signed-off-by: Matthias Heiserer <m.heiserer@proxmox.com>
> ---
>  www/manager6/storage/BackupView.js | 620 ++++++++++++++++++++---------
>  1 file changed, 433 insertions(+), 187 deletions(-)
> 
> diff --git a/www/manager6/storage/BackupView.js b/www/manager6/storage/BackupView.js
> index 2328c0fc..124a57c9 100644
> --- a/www/manager6/storage/BackupView.js
> +++ b/www/manager6/storage/BackupView.js
> @@ -1,36 +1,384 @@
> -Ext.define('PVE.storage.BackupView', {
> -    extend: 'PVE.storage.ContentView',

It'd be great if you could also remove the now unused stuff from the
ContentView base class.

Did you think about a way to re-use this new class for the guest's
backup view already?

> +Ext.define('pve-data-store-backups', {

Nit: datastore is not typical for PVE, and I'd prefer singular 'backup'.

> +    extend: 'Ext.data.Model',
> +    fields: [
> +	{
> +	    name: 'ctime',
> +	    date: 'date',
> +	    dateFormat: 'timestamp',
> +	},
> +	'format',
> +	'volid',
> +	'content',
> +	'vmid',
> +	'size',
> +	'protected',
> +	'notes',

Does not mention 'text' which is assigned later. And ContentView has
special handling to not display the full volid. Please also use
PVE.Utils.render_storage_content() for that here. Or we might want to
switch to using only the date for the leafs? But that needs some
consideration for filtering, and renamed backups which we currently
support..

> +    ],
> +});
> +
>  
> +Ext.define('PVE.storage.BackupView', {
> +    extend: 'Ext.tree.Panel',
>      alias: 'widget.pveStorageBackupView',
> +    mixins: ['Proxmox.Mixin.CBind'],
> +    rootVisible: false,
> +
> +    title: gettext('Content'),
> +
> +    cbindData: function(initialCfg) {
> +	return {
> +	    notPBS: initialCfg.pluginType !== 'pbs',
> +	};
> +    },
> +
> +    controller: {
> +	xclass: 'Ext.app.ViewController',
> +
> +	init: function(view) {
> +	    let me = this;
> +	    me.storage = view.pveSelNode.data.storage;
> +	    if (!me.storage) {
> +		throw "No datastore specificed";
> +	    }
> +	    me.nodename = view.pveSelNode.data.node;
>  
> -    showColumns: ['name', 'notes', 'protected', 'date', 'format', 'size'],
> +	    me.store = Ext.create('Ext.data.Store', {
> +		model: 'pve-data-store-backups',
> +		groupField: 'vmid',
> +		filterer: 'bottomup',
> +	    });
> +	    me.store.on('load', me.onLoad, me);
> +
> +	    view.getStore().setSorters([
> +		'vmid',
> +		'text',

Now it's ascending by date again.

@Thomas: in PBS it's also ascending by date. Should that be changed or
do we switch back in PVE?

> +		'backup-time',

Isn't it ctime? That said, ContentView has some special handling for the
'vdate' column, so maybe we should continue using that. It seems like
the back-end automatically sets the ctime to be the one from the archive
name via archive_info() in case it's a standard name, so maybe it
doesn't actually matter.

> +	    ]);
> +	    view.getStore().setConfig('filterer', 'bottomup');
> +	    Proxmox.Utils.monStoreErrors(view, me.store);
> +	},
> +
> +	onLoad: function(store, records, success, operation) {
> +	    let me = this;
> +	    let view = me.getView();
>  
> -    initComponent: function() {
> -	var me = this;
> +	    let expanded = {};
> +	    view.getRootNode().cascadeBy({
> +		before: item => {
> +		    if (item.isExpanded() && !item.data.leaf) {
> +			let id = item.data.text;
> +			expanded[id] = true;
> +			return true;
> +		    }
> +		    return false;
> +		},
> +		after: Ext.emptyFn,
> +	    });
> +	    let groups = me.getRecordGroups(records, expanded);
>  
> -	var nodename = me.nodename = me.pveSelNode.data.node;
> -	if (!nodename) {
> -	    throw "no node name specified";
> -	}
> +	    for (const item of records.map(i => i.data)) {
> +		item.text = item.volid;
> +		item.leaf = true;
> +		item.ctime = new Date(item.ctime * 1000);
> +		item.iconCls = 'fa-file-o';
> +		groups[`${item.format}` + item.vmid].children.push(item);
> +	    }
> +
> +	    for (let [_name, group] of Object.entries(groups)) {
> +		let c = group.children;

Style nit: please no one character variable names (except for closures).

> +		let latest = c.reduce((l, r) => l.ctime > r.ctime ? l : r);
> +		let num_verified = c.reduce((l, r) => l + r.verification === 'ok', 0);

Does adding a boolean work reliably? Feels hacky.

> +		group.ctime = latest.ctime;
> +		group.size = latest.size;
> +		group.verified = num_verified / c.length;
> +	    }
> +
> +	    let children = [];
> +	    Object.entries(groups).forEach(e => children.push(e[1]));
> +	    view.setRootNode({
> +		expanded: true,
> +		children: children,
> +	    });
>  
> -	var storage = me.storage = me.pveSelNode.data.storage;
> -	if (!storage) {
> -	    throw "no storage ID specified";
> -	}
> +	    Proxmox.Utils.setErrorMask(view, false);
> +	},
>  
> -	me.content = 'backup';
> +	reload: function() {
> +	    let me = this;
> +	    let view = me.getView();
> +	    if (!view.store || !me.store) {
> +		console.warn('cannot reload, no store(s)');
> +		return;
> +	    }
>  
> -	var sm = me.sm = Ext.create('Ext.selection.RowModel', {});
> +	    let url = `/api2/json/nodes/${me.nodename}/storage/${me.storage}/content`;
> +	    me.store.setProxy({
> +		type: 'proxmox',
> +		timeout: 60*1000,

I don't think you need to set this. API calls have a 30 second timeout IIRC.

> +		url: url,
> +		extraParams: {
> +		    content: 'backup',
> +		},
> +	    });
>  
> -	var reload = function() {
>  	    me.store.load();
> -	};
> +	    Proxmox.Utils.monStoreErrors(view, me.store);
> +	},
>  
> -	let pruneButton = Ext.create('Proxmox.button.Button', {
> -	    text: gettext('Prune group'),
> +	getRecordGroups: function(records, expanded) {
> +	    let groups = {};
> +	    for (const item of records) {
> +		groups[`${item.data.format}` + item.data.vmid] = {

Format can be things like 'tar', 'tar.zst', etc. when it's not PBS.

> +		    vmid: item.data.vmid,
> +		    leaf: false,
> +		    children: [],
> +		    expanded: !!expanded[item.data.vmid],
> +		    text: item.data.vmid,
> +		    ctime: 0,
> +		    format: item.data.format,
> +		    volid: "volid",
> +		    content: "content",
> +		    size: 0,
> +		    iconCls: PVE.Utils.get_type_icon_cls(item.data.volid, item.data.format),
> +		};
> +	    }
> +	    return groups;
> +	},
> +
> +	restoreHandler: function(button, event, rec) {
> +	    let me = this;
> +	    let vmtype = PVE.Utils.get_backup_type(rec.data.volid, rec.data.format);
> +	    let win = Ext.create('PVE.window.Restore', {
> +		nodename: me.nodename,
> +		volid: rec.data.volid,
> +		volidText: PVE.Utils.render_storage_content(rec.data.volid, {}, rec),
> +		vmtype: vmtype,
> +		isPBS: me.isPBS,

me.isPBS doesn't seem to be set anywhere.

> +		view: me.view,
> +	    });
> +	    win.on('destroy', () => me.reload());
> +	    win.show();
> +	},
> +

----8<----

> @@ -38,186 +386,84 @@ Ext.define('PVE.storage.BackupView', {
>  
>  		    let vmtype;
>  		    if (name.startsWith('vzdump-lxc-') || format === "pbs-ct") {
> -			vmtype = 'lxc';
> +		        vmtype = 'lxc';
>  		    } else if (name.startsWith('vzdump-qemu-') || format === "pbs-vm") {
> -			vmtype = 'qemu';
> +		        vmtype = 'qemu';

Introduces whitespace errors and could use the helper from patch #2.

>  		    }
> -
>  		    if (vmid && vmtype) {
> -			this.setText(gettext('Prune group') + ` ${vmtype}/${vmid}`);
> -			this.vmid = vmid;
> -			this.vmtype = vmtype;
> -			this.setDisabled(false);
> +			me.setText(gettext('Prune group') + ` ${vmtype}/${vmid}`);
> +			me.vmid = vmid;
> +			me.vmtype = vmtype;
> +			me.setDisabled(false);
>  			return;
>  		    }
>  		}
> -		this.setText(gettext('Prune group'));
> -		this.vmid = null;
> -		this.vmtype = null;
> -		this.setDisabled(true);
> +		me.setText(gettext('Prune group'));
> +		me.vmid = null;
> +		me.vmtype = null;
> +		me.setDisabled(true);
> +	    },
> +	    handler: 'pruneGroupHandler',
> +	    parentXType: "treepanel",
> +	    disabled: true,
> +	    reference: 'pruneButton',
> +	    enableFn: () => true,
> +	},
> +	{
> +	    xtype: 'proxmoxButton',
> +	    text: gettext('Remove'),
> +	    handler: 'removeHandler',
> +	    parentXType: 'treepanel',
> +	    disabled: true,
> +	    enableFn: record => record.phantom === false && !record?.data?.protected,
> +	    confirmMsg: function(rec) {
> +		console.log("controller:", this.getController());

Left-over debug log.

> +		let name = rec.data.text;
> +		return Ext.String.format(gettext('Are you sure you want to remove entry {0}'), `'${name}'`);
> +	    },
> +	},
> +	'->',

Re: [pve-devel] [PATCH manager 3/3] Storage GUI: Rewrite backup content view as TreePanel.

[Fabian Ebner]

Am 11.03.22 um 12:33 schrieb Matthias Heiserer:
> On 09.03.2022 13:39, Fabian Ebner wrote:
>> Am 04.03.22 um 12:52 schrieb Matthias Heiserer:
> 8< --------->
>> It'd be great if you could also remove the now unused stuff from the
>> ContentView base class.
>>
>> Did you think about a way to re-use this new class for the guest's
>> backup view already?
> Not yet, as I wasn't certain whether there is a need. Will do both.

If it's possible to do without too much overhead, it's certainly nicer
than the current approach of having duplicated code to keep in sync.

>>
>>> +Ext.define('pve-data-store-backups', {
>>
>> Nit: datastore is not typical for PVE, and I'd prefer singular 'backup'.
>>
>>> +    extend: 'Ext.data.Model',
>>> +    fields: [
>>> +    {
>>> +        name: 'ctime',
>>> +        date: 'date',
>>> +        dateFormat: 'timestamp',
>>> +    },
>>> +    'format',
>>> +    'volid',
>>> +    'content',
>>> +    'vmid',
>>> +    'size',
>>> +    'protected',
>>> +    'notes',
>>
>> Does not mention 'text' which is assigned later. And ContentView has
>> special handling to not display the full volid. Please also use
>> PVE.Utils.render_storage_content() for that here. Or we might want to
>> switch to using only the date for the leafs? But that needs some
>> consideration for filtering, and renamed backups which we currently
>> support..
>>
> For now, I would stay with displaying the full date and time, as it is
> the simplest solution.

I meant 'date and time' ;) Just not happy with displaying the full volid.

> 8< ---------
>> @Thomas: in PBS it's also ascending by date. Should that be changed or
>> do we switch back in PVE?
>>
>>> +        'backup-time',
>>
>> Isn't it ctime? That said, ContentView has some special handling for the
>> 'vdate' column, so maybe we should continue using that. It seems like
>> the back-end automatically sets the ctime to be the one from the archive
>> name via archive_info() in case it's a standard name, so maybe it
>> doesn't actually matter.
>>
> Yes, should be ctime.
> From what I understand, vdate in ContentView isn't something from the
> API, but just some convoluted way of displaying ctime. Functionally,
> there shouldn't be any differences.

Before pve-storage commit 9c629b3e76a6c70314a385982944fe7ca051947c, the
API didn't return the ctime calculated from the backup name. The code
for the vdate calculation is older. Using ctime from the API should be fine.

> 
> 8< ---------
>>
>>> +        let latest = c.reduce((l, r) => l.ctime > r.ctime ? l : r);
>>> +        let num_verified = c.reduce((l, r) => l + r.verification ===
>>> 'ok', 0);
>>
>> Does adding a boolean work reliably? Feels hacky.
> According to ECMA standard 7.1.4, which I believe is the relevant
> section, it works just intended and converts true to 1 and false to 0.
> 
> 8< ---------
> 
> Agree with everything else. Thanks!