[pve-devel] [PATCH v4 manager 0/5] add backup detail and not backed up view

[pve-devel] [PATCH v4 manager 0/5] add backup detail and not backed up view

[Aaron Lauterer]

This series add a new detail view for backup jobs to show which volumes
of a guest would be included.

Additionally it adds a notification if there are any guests not covered
by any backup job with a new window showing these guests. This is to fix
#2609.

For the latter, a new API endpoint was needed because the already
existing `cluster/backup` expects a job ID on the next level. This makes
it hard to impossible to add other endpoints there. More details are in
the actual patch.


Changes from v3 -> v4:

* switched from summary view with its own button to notification if some
  guests are not backed up and changed the view to only include not
  backed up guests.
* added search boxes to both the detail tree view
* removed the "not permissions for all" notification. We don't do that
  anywhere else anyway and it makes the API return structure simpler and
  easier to deal with
* incorporated some code style changes such as creating the object to be
  pushed to the result array inline in the push operation instead of
  defining it way before


Aaron Lauterer (5):
  api: backup: add endpoint to list included guests and volumes
  gui: dc/backup: move renderers to Utils.js
  gui: dc/backup: add new backup job detail view
  fix #2609 api: backupinfo: add non job specific endpoint
  fix #2609 gui: backup: add window for not backed guests

 PVE/API2/Backup.pm        | 174 ++++++++++++
 PVE/API2/BackupInfo.pm    | 145 ++++++++++
 PVE/API2/Cluster.pm       |   6 +
 PVE/API2/Makefile         |   1 +
 www/manager6/Utils.js     |  94 +++++++
 www/manager6/dc/Backup.js | 574 ++++++++++++++++++++++++++++++++++----
 6 files changed, 936 insertions(+), 58 deletions(-)
 create mode 100644 PVE/API2/BackupInfo.pm

-- 
2.20.1

[pve-devel] [PATCH v4 manager 1/5] api: backup: add endpoint to list included guests and volumes

[Aaron Lauterer]

This patch adds a new API endpoint that returns a list of included
guests, their volumes and whether they are included in a backup.

The output is formatted to be used with the extJS tree panel.

Signed-off-by: Aaron Lauterer <a.lauterer@proxmox.com>
---
The return types are `qemu`, `lxc` and `unknown`. The latter is there on
purpose because it is possible that a deleted but not purged VM is still
configured on a backup job. While the backup job itself will fail, I
think it is good to show it in the job detail view so users can react to
it.

v3 -> v4:
* remove the "not all permissions" field as we never show such
  notifications anywhere else. This makes the returned data simpler
* define objects to be pushed in the return data directly in the push
  operation and not way ahead in the code.

v2 -> v3 (hopefully I got them all):
* incorporate feedback from thomas
    * changed double negative for permissions `not_all_permissions` to
      `permissions_for_all`
* adapted to latest changes to return values from `get_included_guests`
* define $guest only once
* return VMID as int
* renamed some vars to be more descriptive

v1 -> v2:
* simplified the code
* refactored according to feedback


 PVE/API2/Backup.pm | 174 +++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 174 insertions(+)

diff --git a/PVE/API2/Backup.pm b/PVE/API2/Backup.pm
index 86377c0a..6fbe2106 100644
--- a/PVE/API2/<F2>Backup.pm
+++ b/PVE/API2/Backup.pm
@@ -324,4 +324,178 @@ __PACKAGE__->register_method({
 	die "$@" if ($@);
     }});
 
+__PACKAGE__->register_method({
+    name => 'get_volume_backup_included',
+    path => '{id}/included_volumes',
+    method => 'GET',
+    protected => 1,
+    description => "Returns included guests and the backup status of their disks. Optimized to be used in ExtJS tree views.",
+    permissions => {
+	check => ['perm', '/', ['Sys.Audit']],
+    },
+    parameters => {
+	additionalProperties => 0,
+	properties => {
+	    id => $vzdump_job_id_prop
+	},
+    },
+    returns => {
+	type => 'object',
+	description => 'Root node of the tree object. Children represent guests, grandchildren represent volumes of that guest.',
+	properties => {
+	    children => {
+		type => 'array',
+		items => {
+		    type => 'object',
+		    properties => {
+			id => {
+			    type => 'integer',
+			    description => 'VMID of the guest.',
+			},
+			name => {
+			    type => 'string',
+			    description => 'Name of the guest',
+			    optional => 1,
+			},
+			type => {
+			    type => 'string',
+			    description => 'Type of the guest, VM, CT or unknown for removed but not purged guests.',
+			    enum => ['qemu', 'lxc', 'unknown'],
+			},
+			children => {
+			    type => 'array',
+			    optional => 1,
+			    description => 'The volumes of the guest with the information if they will be included in backups.',
+			    items => {
+				type => 'object',
+				properties => {
+				    id => {
+					type => 'string',
+					description => 'Configuration key of the volume.',
+				    },
+				    name => {
+					type => 'string',
+					description => 'Name of the volume.',
+				    },
+				    included => {
+					type => 'boolean',
+					description => 'Whether the volume is included in the backup or not.',
+				    },
+				    reason => {
+					type => 'string',
+					description => 'The reason why the volume is included (or excluded).',
+				    },
+				},
+			    },
+			},
+		    },
+		},
+	    },
+	},
+    },
+    code => sub {
+	my ($param) = @_;
+
+	my $rpcenv = PVE::RPCEnvironment::get();
+
+	my $user = $rpcenv->get_user();
+
+	my $vzconf = cfs_read_file('vzdump.cron');
+	my $all_jobs = $vzconf->{jobs} || [];
+	my $job;
+	my $rrd = PVE::Cluster::rrd_dump();
+
+	for my $j (@$all_jobs) {
+	    if ($j->{id} eq $param->{id}) {
+	       $job = $j;
+	       last;
+	    }
+	}
+	raise_param_exc({ id => "No such job '$param->{id}'" }) if !$job;
+
+	my $vmlist = PVE::Cluster::get_vmlist();
+
+	my @job_vmids;
+
+	my $included_guests = PVE::VZDump::get_included_guests($job);
+
+	for my $node (keys %{$included_guests}) {
+	    my $node_vmids = $included_guests->{$node};
+	    push(@job_vmids, @{$node_vmids});
+	}
+
+	# remove VMIDs to which the user has no permission to not leak infos
+	# like the guest name
+	my @allowed_vmids = grep {
+		$rpcenv->check($user, "/vms/$_", [ 'VM.Audit' ], 1);
+	} @job_vmids;
+
+	my $result = {
+	    children => [],
+	};
+
+	for my $vmid (@allowed_vmids) {
+
+	    my $children = [];
+
+	    # It's possible that a job has VMIDs configured that are not in
+	    # vmlist. This could be because a guest was removed but not purged.
+	    # Since there is no more data available we can only deliver the VMID
+	    # and no volumes.
+	    if (!defined $vmlist->{ids}->{$vmid}) {
+		push(@{$result->{children}}, {
+		    id => int($vmid),
+		    type => 'unknown',
+		    leaf => 1,
+		});
+		next;
+	    }
+
+	    my $type = $vmlist->{ids}->{$vmid}->{type};
+	    my $node = $vmlist->{ids}->{$vmid}->{node};
+
+	    my $conf;
+	    my $volumes;
+	    my $name = "";
+
+	    if ($type eq 'qemu') {
+		$conf = PVE::QemuConfig->load_config($vmid, $node);
+		$volumes = PVE::QemuConfig->get_backup_volumes($conf);
+		$name = $conf->{name};
+	    } elsif ($type eq 'lxc') {
+		$conf = PVE::LXC::Config->load_config($vmid, $node);
+		$volumes = PVE::LXC::Config->get_backup_volumes($conf);
+		$name = $conf->{hostname};
+	    } else {
+		die "VMID $vmid is neither Qemu nor LXC guest\n";
+	    }
+
+	    foreach my $volume (@$volumes) {
+		my $disk = {
+		    # id field must be unique for ExtJS tree view
+		    id => "$vmid:$volume->{key}",
+		    name => $volume->{volume_config}->{file} // $volume->{volume_config}->{volume},
+		    included=> $volume->{included},
+		    reason => $volume->{reason},
+		    leaf => 1,
+		};
+		push(@{$children}, $disk);
+	    }
+
+	    my $leaf = 0;
+	    # it's possible for a guest to have no volumes configured
+	    $leaf = 1 if !@{$children};
+
+	    push(@{$result->{children}}, {
+		    id => int($vmid),
+		    type => $type,
+		    name => $name,
+		    children => $children,
+		    leaf => $leaf,
+	    });
+	}
+
+	return $result;
+    }});
+
 1;
-- 
2.20.1

[pve-devel] [PATCH v4 manager 2/5] gui: dc/backup: move renderers to Utils.js

[Aaron Lauterer]

Moving the following renderers to Utils.js to be able to use them in
more than one place:
* render_backup_days_of_week
* render_backup_selection

Signed-off-by: Aaron Lauterer <a.lauterer@proxmox.com>
---
v1 -> v2 -> v3 -> v4: rebased

 www/manager6/Utils.js     | 60 +++++++++++++++++++++++++++++++++++++++
 www/manager6/dc/Backup.js | 59 ++------------------------------------
 2 files changed, 62 insertions(+), 57 deletions(-)

diff --git a/www/manager6/Utils.js b/www/manager6/Utils.js
index 56a0407d..158b370b 100644
--- a/www/manager6/Utils.js
+++ b/www/manager6/Utils.js
@@ -219,6 +219,66 @@ Ext.define('PVE.Utils', { utilities: {
 
     },
 
+    render_backup_days_of_week: function(val) {
+	var dows = ['sun', 'mon', 'tue', 'wed', 'thu', 'fri', 'sat'];
+	var selected = [];
+	var cur = -1;
+	val.split(',').forEach(function(day){
+	    cur++;
+	    var dow = (dows.indexOf(day)+6)%7;
+	    if (cur === dow) {
+		if (selected.length === 0 || selected[selected.length-1] === 0) {
+		    selected.push(1);
+		} else {
+		    selected[selected.length-1]++;
+		}
+	    } else {
+		while (cur < dow) {
+		    cur++;
+		    selected.push(0);
+		}
+		selected.push(1);
+	    }
+	});
+
+	cur = -1;
+	var days = [];
+	selected.forEach(function(item) {
+	    cur++;
+	    if (item > 2) {
+		days.push(Ext.Date.dayNames[(cur+1)] + '-' + Ext.Date.dayNames[(cur+item)%7]);
+		cur += item-1;
+	    } else if (item == 2) {
+		days.push(Ext.Date.dayNames[cur+1]);
+		days.push(Ext.Date.dayNames[(cur+2)%7]);
+		cur++;
+	    } else if (item == 1) {
+		days.push(Ext.Date.dayNames[(cur+1)%7]);
+	    }
+	});
+	return days.join(', ');
+    },
+
+    render_backup_selection: function(value, metaData, record) {
+	let allExceptText = gettext('All except {0}');
+	let allText = '-- ' + gettext('All') + ' --';
+	if (record.data.all) {
+	    if (record.data.exclude) {
+		return Ext.String.format(allExceptText, record.data.exclude);
+	    }
+	    return allText;
+	}
+	if (record.data.vmid) {
+	    return record.data.vmid;
+	}
+
+	if (record.data.pool) {
+	    return "Pool '"+ record.data.pool + "'";
+	}
+
+	return "-";
+    },
+
     get_kvm_osinfo: function(value) {
 	var info = { base: 'Other' }; // default
 	if (value) {
diff --git a/www/manager6/dc/Backup.js b/www/manager6/dc/Backup.js
index 13cd0528..1ef092c5 100644
--- a/www/manager6/dc/Backup.js
+++ b/www/manager6/dc/Backup.js
@@ -383,7 +383,6 @@ Ext.define('PVE.dc.BackupView', {
     onlineHelp: 'chapter_vzdump',
 
     allText: '-- ' + gettext('All') + ' --',
-    allExceptText: gettext('All except {0}'),
 
     initComponent : function() {
 	var me = this;
@@ -568,45 +567,7 @@ Ext.define('PVE.dc.BackupView', {
 		    width: 200,
 		    sortable: false,
 		    dataIndex: 'dow',
-		    renderer: function(val) {
-			var dows = ['sun', 'mon', 'tue', 'wed', 'thu', 'fri', 'sat'];
-			var selected = [];
-			var cur = -1;
-			val.split(',').forEach(function(day){
-			    cur++;
-			    var dow = (dows.indexOf(day)+6)%7;
-			    if (cur === dow) {
-				if (selected.length === 0 || selected[selected.length-1] === 0) {
-				    selected.push(1);
-				} else {
-				    selected[selected.length-1]++;
-				}
-			    } else {
-				while (cur < dow) {
-				    cur++;
-				    selected.push(0);
-				}
-				selected.push(1);
-			    }
-			});
-
-			cur = -1;
-			var days = [];
-			selected.forEach(function(item) {
-			    cur++;
-			    if (item > 2) {
-				days.push(Ext.Date.dayNames[(cur+1)] + '-' + Ext.Date.dayNames[(cur+item)%7]);
-				cur += item-1;
-			    } else if (item == 2) {
-				days.push(Ext.Date.dayNames[cur+1]);
-				days.push(Ext.Date.dayNames[(cur+2)%7]);
-				cur++;
-			    } else if (item == 1) {
-				days.push(Ext.Date.dayNames[(cur+1)%7]);
-			    }
-			});
-			return days.join(', ');
-		    }
+		    renderer: PVE.Utils.render_backup_days_of_week
 		},
 		{
 		    header: gettext('Start Time'),
@@ -625,23 +586,7 @@ Ext.define('PVE.dc.BackupView', {
 		    flex: 1,
 		    sortable: false,
 		    dataIndex: 'vmid',
-		    renderer: function(value, metaData, record) {
-			if (record.data.all) {
-			    if (record.data.exclude) {
-				return Ext.String.format(me.allExceptText, record.data.exclude);
-			    }
-			    return me.allText;
-			}
-			if (record.data.vmid) {
-			    return record.data.vmid;
-			}
-
-			if (record.data.pool) {
-			    return "Pool '"+ record.data.pool + "'";
-			}
-
-			return "-";
-		    }
+		    renderer: PVE.Utils.render_backup_selection
 		}
 	    ],
 	    listeners: {
-- 
2.20.1

[pve-devel] [PATCH v4 manager 3/5] gui: dc/backup: add new backup job detail view

[Aaron Lauterer]

The new detail view for backup jobs shows the settings similar to the
edit dialog but read only. Additionally it does show a list of all
included guests with their volumes and whether these volumes will be
included in the backup.

Signed-off-by: Aaron Lauterer <a.lauterer@proxmox.com>
---
v3 -> v4:
* added search box
* removed "not all permissions" notification as we don't show such a
  notifcation naywhere else. makes dealing with the API data esier
* fixed if clauses to show the correct selection mode. `exclude vmids`
  was overruled by `all` in previous version.
* added more returned reasons to the backup_reasons_table. The qemu side
  got new return values in the latest patches [0]


v2->v3:
* made the comment why we need to split the "vmid:key" ID disks
 more descriptive
* changed double negative for permissions `not_all_permissions` to
  `permissions_for_all`

v1->v2:
* made render_backup_status more generic
* reworked the reasons why a volume might not be included. turns out we
   cannot easily determine if a volume is in-/excluded by default or
   because the user actively wanted it

   [0] https://pve.proxmox.com/pipermail/pve-devel/2020-June/044052.html

 www/manager6/Utils.js     |  34 ++++
 www/manager6/dc/Backup.js | 372 +++++++++++++++++++++++++++++++++++++-
 2 files changed, 405 insertions(+), 1 deletion(-)

diff --git a/www/manager6/Utils.js b/www/manager6/Utils.js
index 158b370b..91e6238b 100644
--- a/www/manager6/Utils.js
+++ b/www/manager6/Utils.js
@@ -219,6 +219,31 @@ Ext.define('PVE.Utils', { utilities: {
 
     },
 
+    render_backup_status: function(value, meta, record) {
+	if (typeof value == 'undefined') {
+	    return "";
+	}
+
+	let iconCls = 'check-circle good';
+	let text = gettext('Yes');
+
+	if (!PVE.Parser.parseBoolean(value.toString())) {
+	    iconCls = 'times-circle critical';
+
+	    text = gettext('No');
+
+	    let reason = record.get('reason');
+	    if (typeof reason !== 'undefined') {
+		if (reason in PVE.Utils.backup_reasons_table) {
+		    reason = PVE.Utils.backup_reasons_table[record.get('reason')];
+		}
+		text = `${text} - ${reason}`;
+	    }
+	}
+
+	return `<i class="fa fa-${iconCls}"></i> ${text}`;
+    },
+
     render_backup_days_of_week: function(val) {
 	var dows = ['sun', 'mon', 'tue', 'wed', 'thu', 'fri', 'sat'];
 	var selected = [];
@@ -279,6 +304,15 @@ Ext.define('PVE.Utils', { utilities: {
 	return "-";
     },
 
+    backup_reasons_table: {
+	'backup=yes': gettext('Enabled'),
+	'backup=no': gettext('Disabled'),
+	'enabled': gettext('Enabled'),
+	'disabled': gettext('Disabled'),
+	'not a volume': gettext('Not a volume'),
+	'efidisk but no OMVF BIOS': gettext('EFI Disk without OMVF BIOS'),
+    },
+
     get_kvm_osinfo: function(value) {
 	var info = { base: 'Other' }; // default
 	if (value) {
diff --git a/www/manager6/dc/Backup.js b/www/manager6/dc/Backup.js
index 1ef092c5..1e070510 100644
--- a/www/manager6/dc/Backup.js
+++ b/www/manager6/dc/Backup.js
@@ -375,6 +375,327 @@ Ext.define('PVE.dc.BackupEdit', {
 });
 
 
+Ext.define('PVE.dc.BackupDiskTree', {
+    extend: 'Ext.tree.Panel',
+    alias: 'widget.pveBackupDiskTree',
+
+    folderSort: true,
+    rootVisible: false,
+
+    store: {
+	sorters: 'id',
+	data: {},
+    },
+
+    tools: [
+	{
+	    type: 'expand',
+	    tooltip: gettext('Expand All'),
+	    scope: this,
+	    callback: function(panel) {
+		panel.expandAll();
+	    },
+	},
+	{
+	    type: 'collapse',
+	    tooltip: gettext('Collapse All'),
+	    scope: this,
+	    callback: function(panel) {
+		panel.collapseAll();
+	    }
+	},
+    ],
+
+    columns: [
+	{
+	    xtype: 'treecolumn',
+	    text: gettext('Guest Image'),
+	    renderer: function(value, meta, record) {
+		if (record.data.type) {
+		    // guest level
+		    let ret = value;
+		    if (record.data.name) {
+			ret += " (" + record.data.name + ")";
+		    }
+		    return ret;
+		} else {
+		    // volume level
+		    // extJS needs unique IDs but we only want to show the
+		    // volumes key from "vmid:key"
+		    return value.split(':')[1] + " - " + record.data.name;
+		}
+	    },
+	    dataIndex: 'id',
+	    flex: 6,
+	},
+	{
+	    text: gettext('Type'),
+	    dataIndex: 'type',
+	    flex: 1,
+	},
+	{
+	    text: gettext('Backup Job'),
+	    renderer: PVE.Utils.render_backup_status,
+	    dataIndex: 'included',
+	    flex: 3,
+	},
+    ],
+
+    reload: function() {
+	var me = this;
+	var sm = me.getSelectionModel();
+
+	Proxmox.Utils.API2Request({
+	    url: "/cluster/backup/" + me.jobid + "/included_volumes",
+	    waitMsgTarget: me,
+	    method: 'GET',
+	    failure: function(response, opts) {
+		Proxmox.Utils.setErrorMask(me, response.htmlStatus);
+	    },
+	    success: function(response, opts) {
+		sm.deselectAll();
+		me.setRootNode(response.result.data);
+		me.expandAll();
+	    },
+	});
+    },
+
+    initComponent: function() {
+	var me = this;
+
+	if (!me.jobid) {
+	    throw "no job id specified";
+	}
+
+	var sm = Ext.create('Ext.selection.TreeModel', {});
+
+	Ext.apply(me, {
+	    selModel: sm,
+	    fields: ['id', 'type',
+		{
+		    type: 'string',
+		    name: 'iconCls',
+		    calculate: function(data) {
+			var txt = 'fa x-fa-tree fa-';
+			if (data.leaf && !data.type) {
+			    return txt + 'hdd-o';
+			} else if (data.type === 'qemu') {
+			    return txt + 'desktop';
+			} else if (data.type === 'lxc') {
+			    return txt + 'cube';
+			} else {
+			    return txt + 'question-circle';
+			}
+		    }
+		}
+	    ],
+	    tbar: [
+	        '->',
+		gettext('Search') + ':', ' ',
+		{
+		    xtype: 'textfield',
+		    width: 200,
+		    enableKeyEvents: true,
+		    listeners: {
+			buffer: 500,
+			keyup: function(field) {
+			    let searchValue = field.getValue();
+			    searchValue = searchValue.toLowerCase();
+
+			    me.store.clearFilter(true);
+			    me.store.filterBy(function(record) {
+				let match = false;
+
+				let data = '';
+				if (record.data.depth == 0) {
+				    return true;
+				} else if (record.data.depth == 1) {
+				    data = record.data;
+				} else if (record.data.depth == 2) {
+				    data = record.parentNode.data;
+				}
+
+				Ext.each(['name', 'id', 'type'], function(property) {
+				    if (data[property] == null) {
+					return;
+				    }
+
+				    let v = data[property].toString();
+				    if (v !== undefined) {
+					v = v.toLowerCase();
+					if (v.includes(searchValue)) {
+					    match = true;
+					    return;
+					}
+				    }
+				});
+				return match;
+			    });
+			}
+		    }
+		}
+	    ],
+	});
+
+	me.callParent();
+
+	me.reload();
+    }
+});
+
+Ext.define('PVE.dc.BackupInfo', {
+    extend: 'Proxmox.panel.InputPanel',
+    alias: 'widget.pveBackupInfo',
+
+    padding: 10,
+
+    column1: [
+	{
+	    name: 'node',
+	    fieldLabel: gettext('Node'),
+	    xtype: 'displayfield',
+	    renderer: function (value) {
+		if (!value) {
+		    return '-- ' + gettext('All') + ' --';
+		} else {
+		    return value;
+		}
+	    },
+	},
+	{
+	    name: 'storage',
+	    fieldLabel: gettext('Storage'),
+	    xtype: 'displayfield',
+	},
+	{
+	    name: 'dow',
+	    fieldLabel: gettext('Day of week'),
+	    xtype: 'displayfield',
+	    renderer: PVE.Utils.render_backup_days_of_week,
+	},
+	{
+	    name: 'starttime',
+	    fieldLabel: gettext('Start Time'),
+	    xtype: 'displayfield',
+	},
+	{
+	    name: 'selMode',
+	    fieldLabel: gettext('Selection mode'),
+	    xtype: 'displayfield',
+	},
+	{
+	    name: 'pool',
+	    fieldLabel: gettext('Pool to backup'),
+	    xtype: 'displayfield',
+	}
+    ],
+    column2: [
+	{
+	    name: 'mailto',
+	    fieldLabel: gettext('Send email to'),
+	    xtype: 'displayfield',
+	},
+	{
+	    name: 'mailnotification',
+	    fieldLabel: gettext('Email notification'),
+	    xtype: 'displayfield',
+	    renderer: function (value) {
+		let msg;
+		switch (value) {
+		    case 'always':
+			msg = gettext('Always');
+			break;
+		    case 'failure':
+			msg = gettext('On failure only');
+			break;
+		}
+		return msg;
+	    },
+	},
+	{
+	    name: 'compress',
+	    fieldLabel: gettext('Compression'),
+	    xtype: 'displayfield',
+	},
+	{
+	    name: 'mode',
+	    fieldLabel: gettext('Mode'),
+	    xtype: 'displayfield',
+	    renderer: function (value) {
+		let msg;
+		switch (value) {
+		    case 'snapshot':
+			msg = gettext('Snapshot');
+			break;
+		    case 'suspend':
+			msg = gettext('Suspend');
+			break;
+		    case 'stop':
+			msg = gettext('Stop');
+			break;
+		}
+		return msg;
+	    },
+	},
+	{
+	    name: 'enabled',
+	    fieldLabel: gettext('Enabled'),
+	    xtype: 'displayfield',
+	    renderer: function (value) {
+		if (PVE.Parser.parseBoolean(value.toString())) {
+		    return gettext('Yes');
+		} else {
+		    return gettext('No');
+		}
+	    },
+	},
+    ],
+
+    setValues: function(values) {
+	var me = this;
+
+        Ext.iterate(values, function(fieldId, val) {
+	    let field = me.query('[isFormField][name=' + fieldId + ']')[0];
+	    if (field) {
+		field.setValue(val);
+            }
+	});
+
+	// selection Mode depends on the presence/absence of several keys
+	let selModeField = me.query('[isFormField][name=selMode]')[0];
+	let selMode = 'none';
+	if (values.vmid) {
+	    selMode = gettext('Include selected VMs');
+	}
+	if (values.all) {
+	    selMode = gettext('All');
+	}
+	if (values.exclude) {
+	     selMode = gettext('Exclude selected VMs');
+	}
+	if (values.pool) {
+	    selMode = gettext('Pool based');
+	}
+	selModeField.setValue(selMode);
+
+	if (!values.pool) {
+	    let poolField = me.query('[isFormField][name=pool]')[0];
+	    poolField.setVisible(0);
+	}
+    },
+
+    initComponent: function() {
+	var me = this;
+
+	if (!me.record) {
+	    throw "no data provided";
+	}
+	me.callParent();
+
+	me.setValues(me.record);
+    }
+});
+
 Ext.define('PVE.dc.BackupView', {
     extend: 'Ext.grid.GridPanel',
 
@@ -414,6 +735,45 @@ Ext.define('PVE.dc.BackupView', {
 	    win.show();
 	};
 
+	var run_detail = function() {
+	    let record = sm.getSelection()[0]
+	    if (!record) {
+		return;
+	    }
+	    var me = this;
+	    var infoview = Ext.create('PVE.dc.BackupInfo', {
+		flex: 0,
+		layout: 'fit',
+		record: record.data,
+	    });
+	    var disktree = Ext.create('PVE.dc.BackupDiskTree', {
+		title: gettext('Included disks'),
+		flex: 1,
+		jobid: record.data.id,
+	    });
+
+	    var win = Ext.create('Ext.window.Window', {
+		modal: true,
+		width: 600,
+		height: 500,
+		stateful: true,
+		stateId: 'backup-detail-view',
+		resizable: true,
+		layout: 'fit',
+		title: gettext('Backup Details'),
+
+		items:[{
+		    xtype: 'panel',
+		    region: 'center',
+		    layout: {
+			type: 'vbox',
+			align: 'stretch'
+		    },
+		    items: [infoview, disktree],
+		}]
+	    }).show();
+	};
+
 	var run_backup_now = function(job) {
 	    job = Ext.clone(job);
 
@@ -514,6 +874,14 @@ Ext.define('PVE.dc.BackupView', {
 	    }
 	});
 
+	var detail_btn = new Proxmox.button.Button({
+	    text: gettext('Detail'),
+	    disabled: true,
+	    tooltip: gettext('Show job details and which guests and volumes are affected by the backup job'),
+	    selModel: sm,
+	    handler: run_detail,
+	});
+
 	Proxmox.Utils.monStoreErrors(me, store);
 
 	Ext.apply(me, {
@@ -536,8 +904,10 @@ Ext.define('PVE.dc.BackupView', {
 		'-',
 		remove_btn,
 		edit_btn,
+		detail_btn,
+		'-',
+		run_btn,
 		'-',
-		run_btn
 	    ],
 	    columns: [
 		{
-- 
2.20.1

[pve-devel] [PATCH v4 manager 4/5] fix #2609 api: backupinfo: add non job specific endpoint

[Aaron Lauterer]

Adds a new api endpoint at cluster/backupinfo for cluster wide backup
stuff. This is necessary because cluster/backup expects a backup job ID
at the next level and thus other endpoints are hard to impossible to
implement under that hierarchy.

The only api endpoint available for now is the `not_backed_up` which
returns a list of all guests which are not covered by any backup job.

The top level index endpoint is left unsused for now to be available for
a more generic summary endpoint in the future.

Signed-off-by: Aaron Lauterer <a.lauterer@proxmox.com>
---

v2 -> v4: (v3 was skipped to align version number to the rest of the
	   series)
* renamed from `backupsummary` to `backupinfo`
* changed endpoint from `included_status` to `not_backed_up` as we only
  need to show guests not covered by any job instead of all guests and
  their status
* incorporated a few code style changes such as creating the object to
  be pushed to the return array at the push operation instead of way
  before

v1->v2:
* incorporated feedback for the other API endpoint that is also valid
  here
    * changed double negative for permissions `not_all_permissions` to
      `permissions_for_all`
* adapted to latest changes in return value of `get_included_guests`
    * created two new method which merge and flatten the info from all
      backup jobs
* renamed some vars to more descriptive names
* return VMIDs as int
* reworded some API descriptions
* added stub endpoint for root endpoint

 PVE/API2/BackupInfo.pm | 145 +++++++++++++++++++++++++++++++++++++++++
 PVE/API2/Cluster.pm    |   6 ++
 PVE/API2/Makefile      |   1 +
 3 files changed, 152 insertions(+)
 create mode 100644 PVE/API2/BackupInfo.pm

diff --git a/PVE/API2/BackupInfo.pm b/PVE/API2/BackupInfo.pm
new file mode 100644
index 00000000..909a5de1
--- /dev/null
+++ b/PVE/API2/BackupInfo.pm
@@ -0,0 +1,145 @@
+package PVE::API2::BackupInfo;
+
+use strict;
+use warnings;
+use Digest::SHA;
+
+use PVE::SafeSyslog;
+use PVE::Tools qw(extract_param);
+use PVE::Cluster qw(cfs_lock_file cfs_read_file cfs_write_file);
+use PVE::RESTHandler;
+use PVE::RPCEnvironment;
+use PVE::JSONSchema;
+use PVE::Storage;
+use PVE::Exception qw(raise_param_exc);
+use PVE::VZDump;
+use PVE::VZDump::Common;
+
+use base qw(PVE::RESTHandler);
+
+sub map_job_vmids {
+    my ($job_included_guests, $included_vmids) = @_;
+
+    for my $node_vmids (values %{$job_included_guests}) {
+	for my $vmid (@{$node_vmids}) {
+	    $included_vmids->{$vmid} = 1;
+	}
+    }
+
+    return $included_vmids;
+}
+
+sub get_included_vmids {
+    my $included_vmids = {};
+    my $vzconf = cfs_read_file('vzdump.cron');
+
+    my $all_jobs = $vzconf->{jobs} || [];
+
+    for my $job (@$all_jobs) {
+	my $job_included_guests = PVE::VZDump::get_included_guests($job);
+	$included_vmids = map_job_vmids($job_included_guests, $included_vmids);
+    }
+
+    return $included_vmids;
+}
+
+__PACKAGE__->register_method({
+    name => 'get_backupinfo',
+    path => '',
+    method => 'GET',
+    protected => 1,
+    description => "Stub, waits for future use.",
+    parameters => {
+       additionalProperties => 0,
+       properties => {},
+    },
+    returns => {
+       type => 'string',
+       description => 'Shows stub message',
+    },
+    code => sub {
+       return "Stub endpoint. There is nothing here yet.";
+    }});
+
+__PACKAGE__->register_method({
+    name => 'get_guests_not_in_backup',
+    path => 'not_backed_up',
+    method => 'GET',
+    protected => 1,
+    description => "Shows all guests which are not covered by any backup job.",
+    permissions => {
+	check => ['perm', '/', ['Sys.Audit']],
+    },
+    parameters => {
+    	additionalProperties => 0,
+	properties => {},
+    },
+    returns => {
+	type => 'array',
+	description => 'Contains the guest objects.',
+	items => {
+	    type => 'object',
+	    properties => {
+		vmid => {
+		    type => 'integer',
+		    description => 'VMID of the guest.',
+		},
+		name => {
+		    type => 'string',
+		    description => 'Name of the guest',
+		    optional => 1,
+		},
+		type => {
+		    type => 'string',
+		    description => 'Type of the guest.',
+		    enum => ['qemu', 'lxc'],
+		},
+	    },
+	},
+    },
+    code => sub {
+	my $rpcenv = PVE::RPCEnvironment::get();
+	my $user = $rpcenv->get_user();
+	my $rrd = PVE::Cluster::rrd_dump();
+	my $included_vmids = get_included_vmids();
+	my $vmlist = PVE::Cluster::get_vmlist();
+	my @vmids = ( keys %{$vmlist->{ids}} );
+
+	# remove VMIDs to which the user has no permission to not leak infos
+	# like the guest name
+	my @allowed_vmids = grep {
+		$rpcenv->check($user, "/vms/$_", [ 'VM.Audit' ], 1);
+	} @vmids;
+
+	my $result = [];
+
+	for my $vmid (@allowed_vmids) {
+
+	    next if $included_vmids->{$vmid};
+
+	    my $type = $vmlist->{ids}->{$vmid}->{type};
+	    my $node = $vmlist->{ids}->{$vmid}->{node};
+
+	    my $conf;
+	    my $name = "";
+
+	    if ($type eq 'qemu') {
+		$conf = PVE::QemuConfig->load_config($vmid, $node);
+		$name = $conf->{name};
+	    } elsif ($type eq 'lxc') {
+		$conf = PVE::LXC::Config->load_config($vmid, $node);
+		$name = $conf->{hostname};
+	    } else {
+		die "VMID $vmid is neither Qemu nor LXC guest\n";
+	    }
+
+	    push @{$result}, {
+		vmid => int($vmid),
+		name => $name,
+		type => $type,
+	    };
+	}
+
+	return $result;
+    }});
+1;
diff --git a/PVE/API2/Cluster.pm b/PVE/API2/Cluster.pm
index 062ca849..e768cbc6 100644
--- a/PVE/API2/Cluster.pm
+++ b/PVE/API2/Cluster.pm
@@ -23,6 +23,7 @@ use PVE::Tools qw(extract_param);
 use PVE::API2::ACMEAccount;
 use PVE::API2::ACMEPlugin;
 use PVE::API2::Backup;
+use PVE::API2::BackupInfo;
 use PVE::API2::Cluster::Ceph;
 use PVE::API2::ClusterConfig;
 use PVE::API2::Firewall::Cluster;
@@ -57,6 +58,11 @@ __PACKAGE__->register_method ({
     path => 'backup',
 });
 
+__PACKAGE__->register_method ({
+    subclass => "PVE::API2::BackupInfo",
+    path => 'backupinfo',
+});
+
 __PACKAGE__->register_method ({
     subclass => "PVE::API2::HAConfig",
     path => 'ha',
diff --git a/PVE/API2/Makefile b/PVE/API2/Makefile
index 28ecc070..bc5ccc36 100644
--- a/PVE/API2/Makefile
+++ b/PVE/API2/Makefile
@@ -10,6 +10,7 @@ PERLSOURCE = 			\
 	Subscription.pm		\
 	VZDump.pm		\
 	Backup.pm		\
+	BackupInfo.pm		\
 	Cluster.pm		\
 	HAConfig.pm		\
 	Nodes.pm		\
-- 
2.20.1

[pve-devel] [PATCH v4 manager 5/5] fix #2609 gui: backup: add window for not backed guests

[Aaron Lauterer]

In case that there are guests which are not covered by any backup job, a
notification is shown and a window with a grid can be opened to view
these guests.

Signed-off-by: Aaron Lauterer <a.lauterer@proxmox.com>
---
v2 -> v4: (v3 was skipped to align version number to the rest of the
	   series)
* changed from having a dedicated `Summary` button to showing a
  notification if there are guests not covered by any job
* API data is now fetched in the main backup grid and pushed to the
  window showing the not covered guests
* code for the search box has been simplified

v1->v2:
* renamed ostore to cold_store
* changed double negative for permissions `not_all_permissions` to
  `permissions_for_all`


 www/manager6/dc/Backup.js | 145 +++++++++++++++++++++++++++++++++++++-
 1 file changed, 144 insertions(+), 1 deletion(-)

diff --git a/www/manager6/dc/Backup.js b/www/manager6/dc/Backup.js
index 1e070510..1c0b5c57 100644
--- a/www/manager6/dc/Backup.js
+++ b/www/manager6/dc/Backup.js
@@ -696,6 +696,89 @@ Ext.define('PVE.dc.BackupInfo', {
     }
 });
 
+
+Ext.define('PVE.dc.BackedGuests', {
+    extend: 'Ext.grid.GridPanel',
+    alias: 'widget.pveBackedGuests',
+
+    textfilter: '',
+
+    columns: [
+	{
+	    header: gettext('Type'),
+	    dataIndex: "type",
+	    renderer: PVE.Utils.render_resource_type,
+	    flex: 1,
+	    sortable: true,
+	},
+	{
+	    header: gettext('VMID'),
+	    dataIndex: 'vmid',
+	    flex: 1,
+	    sortable: true,
+	},
+	{
+	    header: gettext('Name'),
+	    dataIndex: 'name',
+	    flex: 2,
+	    sortable: true,
+	},
+    ],
+
+    initComponent: function() {
+	let me = this;
+
+	me.store.clearFilter(true);
+
+	Ext.apply(me, {
+	    stateful: true,
+	    stateId: 'grid-dc-backed-guests',
+	    tbar: [
+	        '->',
+		gettext('Search') + ':', ' ',
+		{
+		    xtype: 'textfield',
+		    width: 200,
+		    enableKeyEvents: true,
+		    listeners: {
+			buffer: 500,
+			keyup: function(field) {
+			    let searchValue = field.getValue();
+			    searchValue = searchValue.toLowerCase();
+
+			    me.store.clearFilter(true);
+			    me.store.filterBy(function(record) {
+				let match = false;
+
+				Ext.each(['name', 'vmid', 'type'], function(property) {
+				    if (record.data[property] == null) {
+					return;
+				    }
+
+				    let v = record.data[property].toString();
+				    if (v !== undefined) {
+					v = v.toLowerCase();
+					if (v.includes(searchValue)) {
+					    match = true;
+					    return;
+					}
+				    }
+				});
+				return match;
+			    });
+			}
+		    }
+		}
+	    ],
+	    viewConfig: {
+		stripeRows: true,
+		trackOver: false,
+            },
+	});
+	me.callParent();
+    },
+});
+
 Ext.define('PVE.dc.BackupView', {
     extend: 'Ext.grid.GridPanel',
 
@@ -716,8 +799,27 @@ Ext.define('PVE.dc.BackupView', {
 	    }
 	});
 
+	var not_backed_store = new Ext.data.Store({
+	    sorters: 'vmid',
+	    proxy:{
+		type: 'proxmox',
+		url: 'api2/json/cluster/backupinfo/not_backed_up',
+	    },
+	});
+
 	var reload = function() {
 	    store.load();
+	    not_backed_store.load({
+		callback: function(records, operation, success) {
+		    if (records.length) {
+			not_backed_warning.setVisible(true);
+			not_backed_btn.setVisible(true);
+		    } else {
+			not_backed_warning.setVisible(false);
+			not_backed_btn.setVisible(false);
+		    }
+		},
+	    });
 	};
 
 	var sm = Ext.create('Ext.selection.RowModel', {});
@@ -834,6 +936,34 @@ Ext.define('PVE.dc.BackupView', {
 	    }));
 	};
 
+	var run_show_not_backed = function() {
+	    var me = this;
+	    var backedinfo = Ext.create('PVE.dc.BackedGuests', {
+		flex: 1,
+		layout: 'fit',
+		store: not_backed_store,
+	    });
+
+	    var win = Ext.create('Ext.window.Window', {
+		modal: true,
+		width: 600,
+		height: 500,
+		resizable: true,
+		layout: 'fit',
+		title: gettext('Guests without backup job'),
+
+		items:[{
+		    xtype: 'panel',
+		    region: 'center',
+		    layout: {
+			type: 'vbox',
+			align: 'stretch'
+		    },
+		    items: [backedinfo],
+		}]
+	    }).show();
+	};
+
 	var edit_btn = new Proxmox.button.Button({
 	    text: gettext('Edit'),
 	    disabled: true,
@@ -882,6 +1012,17 @@ Ext.define('PVE.dc.BackupView', {
 	    handler: run_detail,
 	});
 
+	var not_backed_warning = Ext.create('Ext.toolbar.TextItem', {
+	    html: '<i class="fa fa-fw fa-exclamation-circle"></i>' + gettext('Some guests are not covered by any backup job.'),
+	    hidden: true,
+	});
+
+	var not_backed_btn = new Proxmox.button.Button({
+	    text: gettext('Show'),
+	    hidden: true,
+	    handler: run_show_not_backed,
+	});
+
 	Proxmox.Utils.monStoreErrors(me, store);
 
 	Ext.apply(me, {
@@ -907,7 +1048,9 @@ Ext.define('PVE.dc.BackupView', {
 		detail_btn,
 		'-',
 		run_btn,
-		'-',
+		'->',
+		not_backed_warning,
+		not_backed_btn,
 	    ],
 	    columns: [
 		{
-- 
2.20.1

Re: [pve-devel] [PATCH v4 manager 1/5] api: backup: add endpoint to list included guests and volumes

[Thomas Lamprecht]

On 07.07.20 11:48, Aaron Lauterer wrote:
> This patch adds a new API endpoint that returns a list of included
> guests, their volumes and whether they are included in a backup.
> 
> The output is formatted to be used with the extJS tree panel.
> 
> Signed-off-by: Aaron Lauterer <a.lauterer@proxmox.com>
> ---
> The return types are `qemu`, `lxc` and `unknown`. The latter is there on
> purpose because it is possible that a deleted but not purged VM is still
> configured on a backup job. While the backup job itself will fail, I
> think it is good to show it in the job detail view so users can react to
> it.
> 
> v3 -> v4:
> * remove the "not all permissions" field as we never show such
>   notifications anywhere else. This makes the returned data simpler
> * define objects to be pushed in the return data directly in the push
>   operation and not way ahead in the code.
> 
> v2 -> v3 (hopefully I got them all):
> * incorporate feedback from thomas
>     * changed double negative for permissions `not_all_permissions` to
>       `permissions_for_all`
> * adapted to latest changes to return values from `get_included_guests`
> * define $guest only once
> * return VMID as int
> * renamed some vars to be more descriptive
> 
> v1 -> v2:
> * simplified the code
> * refactored according to feedback
> 
> 
>  PVE/API2/Backup.pm | 174 +++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 174 insertions(+)
> 
> diff --git a/PVE/API2/Backup.pm b/PVE/API2/Backup.pm
> index 86377c0a..6fbe2106 100644
> --- a/PVE/API2/<F2>Backup.pm

A <F2> slipped into here, git am -3 handles it like a champ though..

Further, when applying this series I get:
> implement me - abstract method (500)

In the detail window, am I still missing some other part or package bump?

Re: [pve-devel] [PATCH v4 manager 1/5] api: backup: add endpoint to list included guests and volumes

[Aaron Lauterer]

On 7/8/20 9:11 AM, Thomas Lamprecht wrote:
> On 07.07.20 11:48, Aaron Lauterer wrote:
>> This patch adds a new API endpoint that returns a list of included
>> guests, their volumes and whether they are included in a backup.
>>
>> The output is formatted to be used with the extJS tree panel.
>>
>> Signed-off-by: Aaron Lauterer <a.lauterer@proxmox.com>
>> ---
>> The return types are `qemu`, `lxc` and `unknown`. The latter is there on
>> purpose because it is possible that a deleted but not purged VM is still
>> configured on a backup job. While the backup job itself will fail, I
>> think it is good to show it in the job detail view so users can react to
>> it.
>>
>> v3 -> v4:
>> * remove the "not all permissions" field as we never show such
>>    notifications anywhere else. This makes the returned data simpler
>> * define objects to be pushed in the return data directly in the push
>>    operation and not way ahead in the code.
>>
>> v2 -> v3 (hopefully I got them all):
>> * incorporate feedback from thomas
>>      * changed double negative for permissions `not_all_permissions` to
>>        `permissions_for_all`
>> * adapted to latest changes to return values from `get_included_guests`
>> * define $guest only once
>> * return VMID as int
>> * renamed some vars to be more descriptive
>>
>> v1 -> v2:
>> * simplified the code
>> * refactored according to feedback
>>
>>
>>   PVE/API2/Backup.pm | 174 +++++++++++++++++++++++++++++++++++++++++++++
>>   1 file changed, 174 insertions(+)
>>
>> diff --git a/PVE/API2/Backup.pm b/PVE/API2/Backup.pm
>> index 86377c0a..6fbe2106 100644
>> --- a/PVE/API2/<F2>Backup.pm
> 
> A <F2> slipped into here, git am -3 handles it like a champ though..

Oops :/
> 
> Further, when applying this series I get:
>> implement me - abstract method (500)
> 
> In the detail window, am I still missing some other part or package bump?

Turns out that pve-container hasn't gotten a version bump yet since the 
needed changes for this to work were applied. Should work if you build 
and install pve-container from the master branch.

Something to take a closer look at in the future :)

> 
> 

[pve-devel] applied-series: Re: [PATCH v4 manager 0/5] add backup detail and not backed up view

[Thomas Lamprecht]

On 07.07.20 11:48, Aaron Lauterer wrote:
> This series add a new detail view for backup jobs to show which volumes
> of a guest would be included.
> 
> Additionally it adds a notification if there are any guests not covered
> by any backup job with a new window showing these guests. This is to fix
> #2609.
> 
> For the latter, a new API endpoint was needed because the already
> existing `cluster/backup` expects a job ID on the next level. This makes
> it hard to impossible to add other endpoints there. More details are in
> the actual patch.
> 
> 
> Changes from v3 -> v4:
> 
> * switched from summary view with its own button to notification if some
>   guests are not backed up and changed the view to only include not
>   backed up guests.
> * added search boxes to both the detail tree view
> * removed the "not permissions for all" notification. We don't do that
>   anywhere else anyway and it makes the API return structure simpler and
>   easier to deal with
> * incorporated some code style changes such as creating the object to be
>   pushed to the result array inline in the push operation instead of
>   defining it way before
> 
> 
> Aaron Lauterer (5):
>   api: backup: add endpoint to list included guests and volumes
>   gui: dc/backup: move renderers to Utils.js
>   gui: dc/backup: add new backup job detail view
>   fix #2609 api: backupinfo: add non job specific endpoint
>   fix #2609 gui: backup: add window for not backed guests
> 
>  PVE/API2/Backup.pm        | 174 ++++++++++++
>  PVE/API2/BackupInfo.pm    | 145 ++++++++++
>  PVE/API2/Cluster.pm       |   6 +
>  PVE/API2/Makefile         |   1 +
>  www/manager6/Utils.js     |  94 +++++++
>  www/manager6/dc/Backup.js | 574 ++++++++++++++++++++++++++++++++++----
>  6 files changed, 936 insertions(+), 58 deletions(-)
>  create mode 100644 PVE/API2/BackupInfo.pm
> 



applied series, thanks! FYI: Slightly adapted default window size, renamed "Detail"
button to "Job Detail" moved the Job Detail and moved it's search to the panel
header to save some vertical space.