[pve-devel] [PATCH storage 3/3] pvesm: encryption key parameter should load files

2020-07-09 Thread Wolfgang Bumiller
also `pvesm set` and `pvesm add` should behave the same with
respect to how configuration options are treated

Signed-off-by: Wolfgang Bumiller 
---
 PVE/CLI/pvesm.pm | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/PVE/CLI/pvesm.pm b/PVE/CLI/pvesm.pm
index 4f934d6..f223c92 100755
--- a/PVE/CLI/pvesm.pm
+++ b/PVE/CLI/pvesm.pm
@@ -36,9 +36,11 @@ sub param_mapping {
return PVE::PTY::read_password("Enter Password: ");
},
 });
+
 my $mapping = {
'cifsscan' => [ $password_map ],
-   'create' => [ $password_map ],
+   'create' => [ $password_map, 'encryption_key' ],
+   'update' => [ $password_map, 'encryption_key' ],
 };
 return $mapping->{$name};
 }
-- 
2.20.1



___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel



[pve-devel] [PATCH storage 2/3] refactor sensitive parameter handling

2020-07-09 Thread Wolfgang Bumiller
Signed-off-by: Wolfgang Bumiller 
---
 PVE/API2/Storage/Config.pm | 61 --
 PVE/Storage/CIFSPlugin.pm  |  6 
 PVE/Storage/PBSPlugin.pm   |  6 ++--
 3 files changed, 41 insertions(+), 32 deletions(-)

diff --git a/PVE/API2/Storage/Config.pm b/PVE/API2/Storage/Config.pm
index 09c5d59..251b4af 100755
--- a/PVE/API2/Storage/Config.pm
+++ b/PVE/API2/Storage/Config.pm
@@ -112,6 +112,29 @@ __PACKAGE__->register_method ({
return &$api_storage_config($cfg, $param->{storage});
 }});
 
+my sub extract_sensitive_params :prototype($$) {
+my ($param, $delete_list) = @_;
+
+my $sensitive;
+
+my %delete = map { $_ => 1 } ($delete_list || [])->@*;
+
+# always extract pw and keys, so they don't get written to the www-data 
readable scfg
+for my $opt (qw(password encryption_key)) {
+   # First handle deletions as explicitly setting `undef`, afterwards new 
values may override
+   # it.
+   if (exists($delete{$opt})) {
+   $sensitive->{$opt} = undef;
+   }
+
+   if (defined(my $value = extract_param($param, $opt))) {
+   $sensitive->{$opt} = $value;
+   }
+}
+
+return $sensitive;
+}
+
 __PACKAGE__->register_method ({
 name => 'create',
 protected => 1,
@@ -133,15 +156,7 @@ __PACKAGE__->register_method ({
# fix me in section config create never need an empty entity.
delete $param->{nodes} if !$param->{nodes};
 
-   my $password;
-   # always extract pw, else it gets written to the www-data readable scfg
-   if (my $tmp_pw = extract_param($param, 'password')) {
-   if (($type eq 'pbs') || ($type eq 'cifs' && $param->{username})) {
-   $password = $tmp_pw;
-   } else {
-   warn "ignore password parameter\n";
-   }
-   }
+   my $sensitive = extract_sensitive_params($param, []);
 
my $plugin = PVE::Storage::Plugin->lookup($type);
my $opts = $plugin->check_config($storeid, $param, 1, 1);
@@ -157,7 +172,7 @@ __PACKAGE__->register_method ({
 
$cfg->{ids}->{$storeid} = $opts;
 
-   $plugin->on_add_hook($storeid, $opts, password => $password);
+   $plugin->on_add_hook($storeid, $opts, %$sensitive);
 
eval {
# try to activate if enabled on local node,
@@ -197,6 +212,10 @@ __PACKAGE__->register_method ({
my $digest = extract_param($param, 'digest');
my $delete = extract_param($param, 'delete');
 
+   if ($delete) {
+   $delete = [ PVE::Tools::split_list($delete) ];
+   }
+
 PVE::Storage::lock_storage_config(sub {
 
my $cfg = PVE::Storage::config();
@@ -206,24 +225,14 @@ __PACKAGE__->register_method ({
my $scfg = PVE::Storage::storage_config($cfg, $storeid);
my $type = $scfg->{type};
 
-   my $password;
-   # always extract pw, else it gets written to the www-data readable 
scfg
-   if (my $tmp_pw = extract_param($param, 'password')) {
-   if (($type eq 'pbs') || ($type eq 'cifs' && 
$param->{username})) {
-   $password = $tmp_pw;
-   } else {
-   warn "ignore password parameter\n";
-   }
-   }
+   my $sensitive = extract_sensitive_params($param, $delete);
 
my $plugin = PVE::Storage::Plugin->lookup($type);
my $opts = $plugin->check_config($storeid, $param, 0, 1);
 
-   my $delete_password = 0;
-
if ($delete) {
my $options = $plugin->private()->{options}->{$type};
-   foreach my $k (PVE::Tools::split_list($delete)) {
+   foreach my $k (@$delete) {
my $d = $options->{$k} || die "no such option '$k'\n";
die "unable to delete required option '$k'\n" if 
!$d->{optional};
die "unable to delete fixed option '$k'\n" if $d->{fixed};
@@ -231,16 +240,10 @@ __PACKAGE__->register_method ({
if defined($opts->{$k});
 
delete $scfg->{$k};
-
-   $delete_password = 1 if $k eq 'password';
}
}
 
-   if ($delete_password || defined($password)) {
-   $plugin->on_update_hook($storeid, $opts, password => $password);
-   } else {
-   $plugin->on_update_hook($storeid, $opts);
-   }
+   $plugin->on_update_hook($storeid, $opts, %$sensitive);
 
for my $k (keys %$opts) {
$scfg->{$k} = $opts->{$k};
diff --git a/PVE/Storage/CIFSPlugin.pm b/PVE/Storage/CIFSPlugin.pm
index 0eb1722..8de234b 100644
--- a/PVE/Storage/CIFSPlugin.pm
+++ b/PVE/Storage/CIFSPlugin.pm
@@ -161,6 +161,9 @@ sub on_add_hook {
 
 if (defined($param{password})) {
cifs_set_credentials($param{password}, $storeid);
+   if (!exists($scfg->{username})) {
+   warn "ignoring password parameter\n";

[pve-devel] [PATCH storage 1/3] pbs: encryption support, split "raw client command" API

2020-07-09 Thread Wolfgang Bumiller
(And deprecate it...)

Signed-off-by: Wolfgang Bumiller 
---
 PVE/Storage/PBSPlugin.pm | 126 +--
 1 file changed, 109 insertions(+), 17 deletions(-)

diff --git a/PVE/Storage/PBSPlugin.pm b/PVE/Storage/PBSPlugin.pm
index 092d800..31af450 100644
--- a/PVE/Storage/PBSPlugin.pm
+++ b/PVE/Storage/PBSPlugin.pm
@@ -4,12 +4,12 @@ package PVE::Storage::PBSPlugin;
 
 use strict;
 use warnings;
-use POSIX qw(strftime);
-use IO::File;
+use Fcntl qw(F_GETFD F_SETFD FD_CLOEXEC);
 use HTTP::Request;
-use LWP::UserAgent;
+use IO::File;
 use JSON;
-use Data::Dumper; # fixme: remove
+use LWP::UserAgent;
+use POSIX qw(strftime ENOENT);
 
 use PVE::Tools qw(run_command file_read_firstline trim dir_glob_regex 
dir_glob_foreach);
 use PVE::Storage::Plugin;
@@ -37,6 +37,10 @@ sub properties {
},
# openssl s_client -connect :8007 2>&1 |openssl x509 -fingerprint 
-sha256
fingerprint => get_standard_option('fingerprint-sha256'),
+   encryption_key => {
+   description => "Encryption key.",
+   type => 'string',
+   },
 };
 }
 
@@ -49,6 +53,7 @@ sub options {
content => { optional => 1},
username => { optional => 1 },
password => { optional => 1},
+   encryption_key => { optional => 1 },
maxfiles => { optional => 1 },
fingerprint => { optional => 1 },
 };
@@ -87,6 +92,52 @@ sub pbs_get_password {
 return PVE::Tools::file_read_firstline($pwfile);
 }
 
+sub pbs_encryption_key_file_name {
+my ($scfg, $storeid) = @_;
+
+return "/etc/pve/priv/storage/${storeid}.enc";
+}
+
+sub pbs_set_encryption_key {
+my ($scfg, $storeid, $key) = @_;
+
+my $pwfile = pbs_encryption_key_file_name($scfg, $storeid);
+mkdir "/etc/pve/priv/storage";
+
+PVE::Tools::file_set_contents($pwfile, "$key\n");
+}
+
+sub pbs_delete_encryption_key {
+my ($scfg, $storeid) = @_;
+
+my $pwfile = pbs_encryption_key_file_name($scfg, $storeid);
+
+unlink $pwfile;
+}
+
+sub pbs_get_encryption_key {
+my ($scfg, $storeid) = @_;
+
+my $pwfile = pbs_encryption_key_file_name($scfg, $storeid);
+
+return PVE::Tools::file_get_contents($pwfile);
+}
+
+# Returns a file handle if there is an encryption key, or `undef` if there is 
not. Dies on error.
+sub pbs_open_encryption_key {
+my ($scfg, $storeid) = @_;
+
+my $encryption_key_file = pbs_encryption_key_file_name($scfg, $storeid);
+
+my $keyfd;
+if (!open($keyfd, '<', $encryption_key_file)) {
+   return undef if $! == ENOENT;
+   die "failed to open encryption key: $encryption_key_file: $!\n";
+}
+
+return $keyfd;
+}
+
 sub print_volid {
 my ($storeid, $btype, $bid, $btime) = @_;
 
@@ -96,8 +147,8 @@ sub print_volid {
 return "${storeid}:${volname}";
 }
 
-sub run_raw_client_cmd {
-my ($scfg, $storeid, $client_cmd, $param, %opts) = @_;
+my sub do_raw_client_cmd {
+my ($scfg, $storeid, $client_cmd, $param, $can_encrypt, %opts) = @_;
 
 my $client_exe = '/usr/bin/proxmox-backup-client';
 die "executable not found '$client_exe'! Proxmox backup client not 
installed?\n"
@@ -115,6 +166,20 @@ sub run_raw_client_cmd {
 
 push @$cmd, $client_exe, $client_cmd;
 
+# This must live in the top scope to not get closed before the 
`run_command`
+my $keyfd;
+if ($can_encrypt) {
+   if (defined($keyfd = pbs_open_encryption_key($scfg, $storeid))) {
+   my $flags = fcntl($keyfd, F_GETFD, 0)
+   // die "failed to get file descriptor flags: $!\n";
+   fcntl($keyfd, F_SETFD, $flags & ~FD_CLOEXEC)
+   or die "failed to remove FD_CLOEXEC from encryption key file 
descriptor\n";
+   push @$cmd, '--crypt-mode=encrypt', '--keyfd='.fileno($keyfd);
+   } else {
+   push @$cmd, '--crypt-mode=none';
+   }
+}
+
 push @$cmd, @$param if defined($param);
 
 push @$cmd, "--repository", "$username\@$server:$datastore";
@@ -134,6 +199,18 @@ sub run_raw_client_cmd {
 run_command($cmd, %opts);
 }
 
+# FIXME: External perl code should NOT have access to this.
+#
+# There should be separate functions to
+# - make backups
+# - restore backups
+# - restore files
+# with a sane API
+sub run_raw_client_cmd{
+my ($scfg, $storeid, $client_cmd, $param, %opts) = @_;
+return do_raw_client_cmd($scfg, $storeid, $client_cmd, $param, 1, %opts);
+}
+
 sub run_client_cmd {
 my ($scfg, $storeid, $client_cmd, $param, $no_output) = @_;
 
@@ -145,8 +222,8 @@ sub run_client_cmd {
 
 $param = [@$param, '--output-format=json'] if !$no_output;
 
-run_raw_client_cmd($scfg, $storeid, $client_cmd, $param,
-  outfunc => $outfunc, errmsg => 'proxmox-backup-client 
failed');
+do_raw_client_cmd($scfg, $storeid, $client_cmd, $param, 0,
+ outfunc => $outfunc, errmsg => 'proxmox-backup-client 
failed');
 
 return undef if $no_output;
 
@@ -174,8 +251,8 @@ sub extract_vzdump_config {
die "unable 

[pve-devel] [PATCH manager] vzdump: included_guest: return empty hash if no guests selected

2020-07-09 Thread Aaron Lauterer
This will fix the behaviour when calling `vzdump --stop` to cause all
local guests to be backed up.

When refactoring this logic in commit be308647, the assumption was that
every call will have one of the following parameters set: pool, list of
VMIDs or all (intentional or when exclude is used).

There is an addtional possibility, that vzdump is called with only
--stop. Thus there are no other parameters that would indicate which
VMIDs to include.

In this case we want to return the empty hash.

Signed-off-by: Aaron Lauterer 
---
 PVE/VZDump.pm | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/PVE/VZDump.pm b/PVE/VZDump.pm
index 1f0eb246..601cd56e 100644
--- a/PVE/VZDump.pm
+++ b/PVE/VZDump.pm
@@ -1170,7 +1170,7 @@ sub get_included_guests {
$vmids = PVE::API2Tools::get_resource_pool_guest_members($job->{pool});
 } elsif ($job->{vmid}) {
$vmids = [ split_list($job->{vmid}) ];
-} else {
+} elsif ($job->{all}) {
# all or exclude
my $exclude = check_vmids(split_list($job->{exclude}));
my $excludehash = { map { $_ => 1 } @$exclude };
@@ -1179,6 +1179,8 @@ sub get_included_guests {
next if $excludehash->{$id};
push @$vmids, $id;
}
+} else {
+   return $vmids_per_node;
 }
 $vmids = check_vmids(@$vmids);
 
-- 
2.20.1



___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel



[pve-devel] applied: Re: [PATCH manager] vzdump: included_guest: return empty hash if no guests selected

2020-07-09 Thread Thomas Lamprecht
On 09.07.20 13:21, Aaron Lauterer wrote:
> This will fix the behaviour when calling `vzdump --stop` to cause all
> local guests to be backed up.
> 
> When refactoring this logic in commit be308647, the assumption was that
> every call will have one of the following parameters set: pool, list of
> VMIDs or all (intentional or when exclude is used).
> 
> There is an addtional possibility, that vzdump is called with only
> --stop. Thus there are no other parameters that would indicate which
> VMIDs to include.
> 
> In this case we want to return the empty hash.
> 
> Signed-off-by: Aaron Lauterer 
> ---
>  PVE/VZDump.pm | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
>

applied, thanks!


___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel



[pve-devel] applied-series: Re: [PATCH storage 1/3] pbs: encryption support, split "raw client command" API

2020-07-09 Thread Thomas Lamprecht
On 09.07.20 10:25, Wolfgang Bumiller wrote:
> (And deprecate it...)
> 
> Signed-off-by: Wolfgang Bumiller 
> ---
>  PVE/Storage/PBSPlugin.pm | 126 +--
>  1 file changed, 109 insertions(+), 17 deletions(-)
> 
>

applied series, thanks! Renamed the "encryption_key" parameter to 
"encryption-key"
and added support for a "autogen" magic value.


___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel



[pve-devel] applied: Re: [PATCH v3 qemu 1/2] PVE: add query_proxmox_support QMP command

2020-07-09 Thread Thomas Lamprecht
On 08.07.20 11:57, Stefan Reiter wrote:
> Generic interface for future use, currently used for PBS and
> dirty-bitmap backup support.
> 
> Signed-off-by: Stefan Reiter 
> ---
> 
> v2:
> * include 'pbs' feature
> 
>  pve-backup.c |  8 
>  qapi/block-core.json | 24 
>  2 files changed, 32 insertions(+)
> 
>

applied, thanks! but dropped the "pbs" one, makes no sense... We want to have
a proxmox-backup library version and soname info field here in the future 
though.


___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel



[pve-devel] applied: Re: [PATCH v3 container 1/4] fix #1423: add timezone config option

2020-07-09 Thread Thomas Lamprecht
On 02.07.20 14:49, Oguz Bektas wrote:
> optionally enabled.
> 
> adds the 'timezone' option to config, which takes a valid timezone (i.e.
> Europe/Vienna) to set in the container.
> 
> if nothing is selected, then it will show as 'container managed' in
> GUI, and nothing will be done.
> 
> if set to 'host', the /etc/localtime symlink from the host node will be
> cached and set in the container rootfs.
> 
> Signed-off-by: Oguz Bektas 
> ---
> v2->v3:
> no change
> 
> 
> 
> 
>  src/PVE/LXC/Config.pm | 14 ++
>  src/PVE/LXC/Setup.pm  | 13 +
>  src/PVE/LXC/Setup/Base.pm | 33 ++---
>  3 files changed, 57 insertions(+), 3 deletions(-)
> 
>

applied, thanks!


___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel



[pve-devel] applied: Re: [PATCH widget-toolkit] Add focusable pseudo class to edit windows defaultFocus

2020-07-09 Thread Thomas Lamprecht
On 08.07.20 10:42, Fabian Möller wrote:
> Restricting the defaultFocus of the edit windows to only focusable fields
> ensures that windows like "PVE -> Virtual Machine -> Manage HA", which
> has a first field of xtype "displayfield", receive focus upon opening.
> 
> This allows those windows to be closed with the ESC key, which only
> works when an element inside has focus.
> 
> In newer versions of ExtJS (>= 6.2.0) this filter could be reduced to
> "field:canfocus" or maybe even ":canfocus".
> ---
>  src/window/Edit.js | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
>

applied, thanks!



___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel


Re: [pve-devel] [PATCH v3 qemu-server 3/3] fix #2671: include CPU format in man page again

2020-07-09 Thread Thomas Lamprecht
On 25.06.20 13:35, Stefan Reiter wrote:
> Use the new register_format(3) call to use a validator (instead of a
> parser) for 'pve-(vm-)?cpu-conf'. This way the $cpu_fmt hash can be used for
> generating the documentation, while still applying the same verification
> rules as before.
> 
> Since the function no longer parses but only verifies, the parsing in
> print_cpu_device/get_cpu_options has to go via JSONSchema directly.
> 
> Signed-off-by: Stefan Reiter 
> ---
>  PVE/QemuServer/CPUConfig.pm | 56 -
>  1 file changed, 18 insertions(+), 38 deletions(-)
> 
> diff --git a/PVE/QemuServer/CPUConfig.pm b/PVE/QemuServer/CPUConfig.pm
> index 6250591..8ed898c 100644
> --- a/PVE/QemuServer/CPUConfig.pm
> +++ b/PVE/QemuServer/CPUConfig.pm
> @@ -153,6 +153,7 @@ my $cpu_fmt = {
>  'phys-bits' => {
>   type => 'string',
>   format => 'pve-phys-bits',
> + format_description => '8-64|host',
>   description => "The physical memory address bits that are reported to"
>. " the guest OS. Should be smaller or equal to the 
> host's."
>. " Set to 'host' to use value from host CPU, but note 
> that"
> @@ -182,57 +183,36 @@ sub parse_phys_bits {
>  
>  # $cpu_fmt describes both the CPU config passed as part of a VM config, as 
> well
>  # as the definition of a custom CPU model. There are some slight differences
> -# though, which we catch in the custom verification function below.
> -PVE::JSONSchema::register_format('pve-cpu-conf', \&parse_cpu_conf_basic);
> -sub parse_cpu_conf_basic {

This method is still used in PVE/QemuMigrate.pm:236 and breaks live-migration 
with
cpu type = host..



___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel



[pve-devel] [PATCH qemu-server] fixup: use parse_property_string instead of parse_cpu_conf_basic

2020-07-09 Thread Stefan Reiter
The latter was removed and replaced with a validator.

Signed-off-by: Stefan Reiter 
---

Too much Rust and static compilation...

 PVE/QemuMigrate.pm | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/PVE/QemuMigrate.pm b/PVE/QemuMigrate.pm
index 96de0db..b699b67 100644
--- a/PVE/QemuMigrate.pm
+++ b/PVE/QemuMigrate.pm
@@ -233,7 +233,7 @@ sub prepare {
# Since the parameter itself contains no reference to a custom model,
# this makes migration independent of changes to "cpu-models.conf".
if ($conf->{cpu}) {
-   my $cpuconf = 
PVE::QemuServer::CPUConfig::parse_cpu_conf_basic($conf->{cpu});
+   my $cpuconf = 
PVE::JSONSchema::parse_property_string('pve-cpu-conf', $conf->{cpu});
if ($cpuconf && 
PVE::QemuServer::CPUConfig::is_custom_model($cpuconf->{cputype})) {
$self->{forcecpu} = 
PVE::QemuServer::CPUConfig::get_cpu_from_running_vm($pid);
}
-- 
2.20.1



___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel



[pve-devel] [PATCH v4 manager 5/7] make use of archive_info and archive_remove

2020-07-09 Thread Fabian Ebner
to avoid some code duplication.

Signed-off-by: Fabian Ebner 
---
 PVE/VZDump.pm | 23 +--
 1 file changed, 13 insertions(+), 10 deletions(-)

diff --git a/PVE/VZDump.pm b/PVE/VZDump.pm
index 601cd56e..2f509555 100644
--- a/PVE/VZDump.pm
+++ b/PVE/VZDump.pm
@@ -631,10 +631,15 @@ sub get_backup_file_list {
 my $bklist = [];
 foreach my $fn (<$dir/${bkname}-*>) {
next if $exclude_fn && $fn eq $exclude_fn;
-   if ($fn =~ 
m!/(${bkname}-(\d{4})_(\d{2})_(\d{2})-(\d{2})_(\d{2})_(\d{2})\.(tgz|((tar|vma)(\.(${\PVE::Storage::Plugin::COMPRESSOR_RE}))?)))$!)
 {
-   $fn = "$dir/$1"; # untaint
-   my $t = timelocal ($7, $6, $5, $4, $3 - 1, $2);
-   push @$bklist, [$fn, $t];
+
+   my $archive_info = eval { PVE::Storage::archive_info($fn) } // {};
+   if ($archive_info->{is_std_name}) {
+   my $filename = $archive_info->{filename};
+   my $backup = {
+   'path' => "$dir/$filename",
+   'ctime' => $archive_info->{ctime},
+   };
+   push @{$bklist}, $backup;
}
 }
 
@@ -927,15 +932,13 @@ sub exec_backup_task {
$opts->{scfg}, $opts->{storage}, 'prune', $args, logfunc => 
$logfunc);
} else {
my $bklist = get_backup_file_list($opts->{dumpdir}, $bkname, 
$task->{target});
-   $bklist = [ sort { $b->[1] <=> $a->[1] } @$bklist ];
+   $bklist = [ sort { $b->{ctime} <=> $a->{ctime} } @$bklist ];
 
while (scalar (@$bklist) >= $maxfiles) {
my $d = pop @$bklist;
-   debugmsg ('info', "delete old backup '$d->[0]'", $logfd);
-   unlink $d->[0];
-   my $logfn = $d->[0];
-   $logfn =~ 
s/\.(tgz|((tar|vma)(\.(${\PVE::Storage::Plugin::COMPRESSOR_RE}))?))$/\.log/;
-   unlink $logfn;
+   my $archive_path = $d->{path};
+   debugmsg ('info', "delete old backup '$archive_path'", 
$logfd);
+   PVE::Storage::archive_remove($archive_path);
}
}
}
-- 
2.20.1



___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel



[pve-devel] [RFC container] fix #2762: add recursive bindmount option

2020-07-09 Thread Oguz Bektas
allows to mount bindmounts recursively. useful for mounting ZFS datasets
in containers.

Signed-off-by: Oguz Bektas 
---

@wobu is unsure about the security implications of this (bindmount +
symlink + recursion), so i'm sending this first version as RFC to get
reviewed.


 src/PVE/LXC.pm| 14 --
 src/PVE/LXC/Config.pm |  5 +
 2 files changed, 13 insertions(+), 6 deletions(-)

diff --git a/src/PVE/LXC.pm b/src/PVE/LXC.pm
index f3aca7a..aaf9594 100644
--- a/src/PVE/LXC.pm
+++ b/src/PVE/LXC.pm
@@ -1452,10 +1452,11 @@ sub __bindmount_verify {
 
 # Perform the actual bind mounting:
 sub __bindmount_do {
-my ($dir, $dest, $ro, @extra_opts) = @_;
-PVE::Tools::run_command(['mount', '-o', 'bind', @extra_opts, $dir, $dest]);
+my ($dir, $dest, $ro, $recursive_bind, @extra_opts) = @_;
+my $bind_method = $recursive_bind ? 'rbind' : 'bind';
+PVE::Tools::run_command(['mount', '-o', $bind_method, @extra_opts, $dir, 
$dest]);
 if ($ro) {
-   eval { PVE::Tools::run_command(['mount', '-o', 'bind,remount,ro', 
$dest]); };
+   eval { PVE::Tools::run_command(['mount', '-o', 
"$bind_method,remount,ro", $dest]); };
if (my $err = $@) {
warn "bindmount error\n";
# don't leave writable bind-mounts behind...
@@ -1466,11 +1467,11 @@ sub __bindmount_do {
 }
 
 sub bindmount {
-my ($dir, $parentfd, $last_dir, $dest, $ro, @extra_opts) = @_;
+my ($dir, $parentfd, $last_dir, $dest, $ro, $recursive_bind, @extra_opts) 
= @_;
 
 my $srcdh = __bindmount_prepare('/', $dir);
 
-__bindmount_do($dir, $dest, $ro, @extra_opts);
+__bindmount_do($dir, $dest, $ro, $recursive_bind, @extra_opts);
 
 if (!__bindmount_verify($srcdh, $parentfd, $last_dir, $ro)) {
PVE::Tools::run_command(['umount', $dest]);
@@ -1591,6 +1592,7 @@ sub __mountpoint_mount {
 
 my $optstring = join(',', @$optlist);
 my $readonly = $mountpoint->{ro};
+my $recursive_bind = $mountpoint->{recursive};
 
 my @extra_opts;
 @extra_opts = ('-o', $optstring) if $optstring;
@@ -1676,7 +1678,7 @@ sub __mountpoint_mount {
return wantarray ? ($volid, 0, $devpath) : $volid;
 } elsif ($type eq 'bind') {
die "directory '$volid' does not exist\n" if ! -d $volid;
-   bindmount($volid, $parentfd, $last_dir//$rootdir, $mount_path, 
$readonly, @extra_opts) if $mount_path;
+   bindmount($volid, $parentfd, $last_dir//$rootdir, $mount_path, 
$readonly, $recursive_bind, @extra_opts) if $mount_path;
warn "cannot enable quota control for bind mounts\n" if $quota;
return wantarray ? ($volid, 0, undef) : $volid;
 }
diff --git a/src/PVE/LXC/Config.pm b/src/PVE/LXC/Config.pm
index edd587b..0b8c35d 100644
--- a/src/PVE/LXC/Config.pm
+++ b/src/PVE/LXC/Config.pm
@@ -766,6 +766,11 @@ my $mp_desc = {
verbose_description => "Path to the mount point as seen from inside the 
container.\n\n".
   "NOTE: Must not contain any symlinks for 
security reasons."
 },
+recursive => {
+   type => 'boolean',
+   description => 'Mount recursively. Useful for mounting ZFS dataset 
trees.',
+   optional => 1
+}
 };
 PVE::JSONSchema::register_format('pve-ct-mountpoint', $mp_desc);
 
-- 
2.20.1


___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel



[pve-devel] [PATCH v4 storage 3/7] Add API and pvesm call for prune_backups

2020-07-09 Thread Fabian Ebner
For the pvesm call use a wrapper and a --dry-run option to redirect
to the correct API call.

Signed-off-by: Fabian Ebner 
---

Changes from v3:
* allow filtering by type
* pvesm: redirect to correct API call according to --dry-run

 PVE/API2/Storage/Makefile|   2 +-
 PVE/API2/Storage/PruneBackups.pm | 164 +++
 PVE/API2/Storage/Status.pm   |   7 ++
 PVE/CLI/pvesm.pm | 126 
 4 files changed, 298 insertions(+), 1 deletion(-)
 create mode 100644 PVE/API2/Storage/PruneBackups.pm

diff --git a/PVE/API2/Storage/Makefile b/PVE/API2/Storage/Makefile
index a33525b..3f667e8 100644
--- a/PVE/API2/Storage/Makefile
+++ b/PVE/API2/Storage/Makefile
@@ -1,5 +1,5 @@
 
-SOURCES= Content.pm Status.pm Config.pm
+SOURCES= Content.pm Status.pm Config.pm PruneBackups.pm
 
 .PHONY: install
 install:
diff --git a/PVE/API2/Storage/PruneBackups.pm b/PVE/API2/Storage/PruneBackups.pm
new file mode 100644
index 000..a84d1c8
--- /dev/null
+++ b/PVE/API2/Storage/PruneBackups.pm
@@ -0,0 +1,164 @@
+package PVE::API2::Storage::PruneBackups;
+
+use strict;
+use warnings;
+
+use PVE::Cluster;
+use PVE::JSONSchema qw(get_standard_option);
+use PVE::RESTHandler;
+use PVE::RPCEnvironment;
+use PVE::Storage;
+use PVE::Tools qw(extract_param);
+
+use base qw(PVE::RESTHandler);
+
+__PACKAGE__->register_method ({
+name => 'dryrun',
+path => '',
+method => 'GET',
+description => "Get prune information for backups. NOTE: this is only a 
preview and might not be exactly " .
+  "what a subsequent prune call does, if the hour changes or 
if backups are removed/added " .
+  "in the meantime.",
+permissions => {
+   check => ['perm', '/storage/{storage}', ['Datastore.Audit', 
'Datastore.AllocateSpace'], any => 1],
+},
+protected => 1,
+proxyto => 'node',
+parameters => {
+   additionalProperties => 0,
+   properties => {
+   node => get_standard_option('pve-node'),
+   storage => get_standard_option('pve-storage-id', {
+   completion => \&PVE::Storage::complete_storage_enabled,
+}),
+   'prune-backups' => get_standard_option('prune-backups', {
+   description => "Use these retention options instead of those 
from the storage configuration.",
+   optional => 1,
+   }),
+   type => {
+   description => "Either 'qemu' or 'lxc'. Only consider backups 
for guests of this type.",
+   type => 'string',
+   optional => 1,
+   enum => ['qemu', 'lxc'],
+   },
+   vmid => get_standard_option('pve-vmid', {
+   description => "Only consider backups for this guest.",
+   optional => 1,
+   completion => \&PVE::Cluster::complete_vmid,
+   }),
+   },
+},
+returns => {
+   type => 'array',
+   items => {
+   type => 'object',
+   properties => {
+   volid => {
+   description => "Backup volume ID.",
+   type => 'string',
+   },
+   'ctime' => {
+   description => "Creation time of the backup (seconds since 
the UNIX epoch).",
+   type => 'integer',
+   },
+   'mark' => {
+   description => "Whether the backup would be kept or 
removed. For backups that don't " .
+  "use the standard naming scheme, it's 
'protected'.",
+   type => 'string',
+   },
+   type => {
+   description => "One of 'qemu', 'lxc', 'openvz' or 
'unknown'.",
+   type => 'string',
+   },
+   'vmid' => {
+   description => "The VM the backup belongs to.",
+   type => 'integer',
+   optional => 1,
+   },
+   },
+   },
+},
+code => sub {
+   my ($param) = @_;
+
+   my $cfg = PVE::Storage::config();
+
+   my $vmid = extract_param($param, 'vmid');
+   my $type = extract_param($param, 'type');
+   my $storeid = extract_param($param, 'storage');
+
+   my $prune_backups = extract_param($param, 'prune-backups');
+   $prune_backups = 
PVE::JSONSchema::parse_property_string('prune-backups', $prune_backups)
+   if defined($prune_backups);
+
+   return PVE::Storage::prune_backups($cfg, $storeid, $prune_backups, 
$vmid, $type, 1);
+}});
+
+__PACKAGE__->register_method ({
+name => 'delete',
+path => '',
+method => 'DELETE',
+description => "Prune backups. Only those using the standard naming scheme 
are considered.",
+permissions => {
+   description => "You need the 'Datastore.Allocate' privilege on the 
storage " .
+  "(or if a VM ID is specified, 'Datastore.AllocateSpace' 
and 'VM.Backup' for 

[pve-devel] [PATCH v4 storage 2/7] Add prune_backups to storage API

2020-07-09 Thread Fabian Ebner
Implement it for generic storages supporting backups
(i.e. directory-based storages) and add a wrapper for PBS.

Signed-off-by: Fabian Ebner 
---

Changes from v3:
* prune_mark: merge loops
* allow filtering by type
* more error handling
* allow passing along a log function
* add more tests

 PVE/Storage.pm |  91 +-
 PVE/Storage/PBSPlugin.pm   |  68 
 PVE/Storage/Plugin.pm  |  65 +++
 test/prune_backups_test.pm | 342 +
 test/run_plugin_tests.pl   |   1 +
 5 files changed, 565 insertions(+), 2 deletions(-)
 create mode 100644 test/prune_backups_test.pm

diff --git a/PVE/Storage.pm b/PVE/Storage.pm
index edf9a2e..5c44386 100755
--- a/PVE/Storage.pm
+++ b/PVE/Storage.pm
@@ -40,11 +40,11 @@ use PVE::Storage::DRBDPlugin;
 use PVE::Storage::PBSPlugin;
 
 # Storage API version. Icrement it on changes in storage API interface.
-use constant APIVER => 5;
+use constant APIVER => 6;
 # Age is the number of versions we're backward compatible with.
 # This is like having 'current=APIVER' and age='APIAGE' in libtool,
 # see 
https://www.gnu.org/software/libtool/manual/html_node/Libtool-versioning.html
-use constant APIAGE => 4;
+use constant APIAGE => 5;
 
 # load standard plugins
 PVE::Storage::DirPlugin->register();
@@ -1540,6 +1540,93 @@ sub extract_vzdump_config {
 }
 }
 
+sub prune_backups {
+my ($cfg, $storeid, $keep, $vmid, $type, $dryrun, $logfunc) = @_;
+
+my $scfg = storage_config($cfg, $storeid);
+die "storage '$storeid' does not support backups\n" if 
!$scfg->{content}->{backup};
+
+if (!defined($keep)) {
+   die "no prune-backups options configured for storage '$storeid'\n"
+   if !defined($scfg->{'prune-backups'});
+   $keep = PVE::JSONSchema::parse_property_string('prune-backups', 
$scfg->{'prune-backups'});
+}
+
+my $plugin = PVE::Storage::Plugin->lookup($scfg->{type});
+return $plugin->prune_backups($scfg, $storeid, $keep, $vmid, $type, 
$dryrun, $logfunc);
+}
+
+my $prune_mark = sub {
+my ($prune_entries, $keep_count, $id_func) = @_;
+
+return if !$keep_count;
+
+my $already_included = {};
+my $newly_included = {};
+
+foreach my $prune_entry (@{$prune_entries}) {
+   my $mark = $prune_entry->{mark};
+   my $id = $id_func->($prune_entry->{ctime});
+
+   next if $already_included->{$id};
+
+   if (defined($mark)) {
+   $already_included->{$id} = 1 if $mark eq 'keep';
+   next;
+   }
+
+   if (!$newly_included->{$id}) {
+   last if scalar(keys %{$newly_included}) >= $keep_count;
+   $newly_included->{$id} = 1;
+   $prune_entry->{mark} = 'keep';
+   } else {
+   $prune_entry->{mark} = 'remove';
+   }
+}
+};
+
+sub prune_mark_backup_group {
+my ($backup_group, $keep) = @_;
+
+my $prune_list = [ sort { $b->{ctime} <=> $a->{ctime} } @{$backup_group} ];
+
+$prune_mark->($prune_list, $keep->{'keep-last'}, sub {
+   my ($ctime) = @_;
+   return $ctime;
+});
+$prune_mark->($prune_list, $keep->{'keep-hourly'}, sub {
+   my ($ctime) = @_;
+   my (undef, undef, $hour, $day, $month, $year) = localtime($ctime);
+   return "$hour/$day/$month/$year";
+});
+$prune_mark->($prune_list, $keep->{'keep-daily'}, sub {
+   my ($ctime) = @_;
+   my (undef, undef, undef, $day, $month, $year) = localtime($ctime);
+   return "$day/$month/$year";
+});
+$prune_mark->($prune_list, $keep->{'keep-weekly'}, sub {
+   my ($ctime) = @_;
+   my ($sec, $min, $hour, $day, $month, $year) = localtime($ctime);
+   my $iso_week = int(strftime("%V", $sec, $min, $hour, $day, $month - 1, 
$year - 1900));
+   my $iso_week_year = int(strftime("%G", $sec, $min, $hour, $day, $month 
- 1, $year - 1900));
+   return "$iso_week/$iso_week_year";
+});
+$prune_mark->($prune_list, $keep->{'keep-monthly'}, sub {
+   my ($ctime) = @_;
+   my (undef, undef, undef, undef, $month, $year) = localtime($ctime);
+   return "$month/$year";
+});
+$prune_mark->($prune_list, $keep->{'keep-yearly'}, sub {
+   my ($ctime) = @_;
+   my $year = (localtime($ctime))[5];
+   return "$year";
+});
+
+foreach my $prune_entry (@{$prune_list}) {
+   $prune_entry->{mark} //= 'remove';
+}
+}
+
 sub volume_export {
 my ($cfg, $fh, $volid, $format, $snapshot, $base_snapshot, 
$with_snapshots) = @_;
 
diff --git a/PVE/Storage/PBSPlugin.pm b/PVE/Storage/PBSPlugin.pm
index 87b09f2..12f56a9 100644
--- a/PVE/Storage/PBSPlugin.pm
+++ b/PVE/Storage/PBSPlugin.pm
@@ -258,6 +258,74 @@ sub extract_vzdump_config {
 return $config;
 }
 
+sub prune_backups {
+my ($class, $scfg, $storeid, $keep, $vmid, $type, $dryrun, $logfunc) = @_;
+
+$logfunc //= sub { print "$_[1]\n" };
+
+my $backups = $class->list_volumes($storeid, $scfg, $vmid, ['backup']);
+
+$type = 'vm' if defined($type) && $type eq 'qemu'

[pve-devel] [PATCH v4 storage 1/7] Introduce prune-backups property for directory-based storages

2020-07-09 Thread Fabian Ebner
Signed-off-by: Fabian Ebner 
---

Changes from v3:
* use property string validator

 PVE/Storage/CIFSPlugin.pm  |  1 +
 PVE/Storage/CephFSPlugin.pm|  1 +
 PVE/Storage/DirPlugin.pm   |  5 +--
 PVE/Storage/GlusterfsPlugin.pm |  5 +--
 PVE/Storage/NFSPlugin.pm   |  5 +--
 PVE/Storage/PBSPlugin.pm   |  1 +
 PVE/Storage/Plugin.pm  | 59 +-
 7 files changed, 70 insertions(+), 7 deletions(-)

diff --git a/PVE/Storage/CIFSPlugin.pm b/PVE/Storage/CIFSPlugin.pm
index 72ec757..6edbc9d 100644
--- a/PVE/Storage/CIFSPlugin.pm
+++ b/PVE/Storage/CIFSPlugin.pm
@@ -134,6 +134,7 @@ sub options {
nodes => { optional => 1 },
disable => { optional => 1 },
maxfiles => { optional => 1 },
+   'prune-backups' => { optional => 1 },
content => { optional => 1 },
format => { optional => 1 },
username => { optional => 1 },
diff --git a/PVE/Storage/CephFSPlugin.pm b/PVE/Storage/CephFSPlugin.pm
index 6575f4f..880ec05 100644
--- a/PVE/Storage/CephFSPlugin.pm
+++ b/PVE/Storage/CephFSPlugin.pm
@@ -150,6 +150,7 @@ sub options {
fuse => { optional => 1 },
bwlimit => { optional => 1 },
maxfiles => { optional => 1 },
+   'prune-backups' => { optional => 1 },
 };
 }
 
diff --git a/PVE/Storage/DirPlugin.pm b/PVE/Storage/DirPlugin.pm
index 39760a8..3c81d24 100644
--- a/PVE/Storage/DirPlugin.pm
+++ b/PVE/Storage/DirPlugin.pm
@@ -49,10 +49,11 @@ sub properties {
 sub options {
 return {
path => { fixed => 1 },
-nodes => { optional => 1 },
+   nodes => { optional => 1 },
shared => { optional => 1 },
disable => { optional => 1 },
-maxfiles => { optional => 1 },
+   maxfiles => { optional => 1 },
+   'prune-backups' => { optional => 1 },
content => { optional => 1 },
format => { optional => 1 },
mkdir => { optional => 1 },
diff --git a/PVE/Storage/GlusterfsPlugin.pm b/PVE/Storage/GlusterfsPlugin.pm
index 70ea4fc..2dd414d 100644
--- a/PVE/Storage/GlusterfsPlugin.pm
+++ b/PVE/Storage/GlusterfsPlugin.pm
@@ -129,9 +129,10 @@ sub options {
server2 => { optional => 1 },
volume => { fixed => 1 },
transport => { optional => 1 },
-nodes => { optional => 1 },
+   nodes => { optional => 1 },
disable => { optional => 1 },
-maxfiles => { optional => 1 },
+   maxfiles => { optional => 1 },
+   'prune-backups' => { optional => 1 },
content => { optional => 1 },
format => { optional => 1 },
mkdir => { optional => 1 },
diff --git a/PVE/Storage/NFSPlugin.pm b/PVE/Storage/NFSPlugin.pm
index 82b0c5f..6abb24b 100644
--- a/PVE/Storage/NFSPlugin.pm
+++ b/PVE/Storage/NFSPlugin.pm
@@ -79,9 +79,10 @@ sub options {
path => { fixed => 1 },
server => { fixed => 1 },
export => { fixed => 1 },
-nodes => { optional => 1 },
+   nodes => { optional => 1 },
disable => { optional => 1 },
-maxfiles => { optional => 1 },
+   maxfiles => { optional => 1 },
+   'prune-backups' => { optional => 1 },
options => { optional => 1 },
content => { optional => 1 },
format => { optional => 1 },
diff --git a/PVE/Storage/PBSPlugin.pm b/PVE/Storage/PBSPlugin.pm
index b236f6c..87b09f2 100644
--- a/PVE/Storage/PBSPlugin.pm
+++ b/PVE/Storage/PBSPlugin.pm
@@ -55,6 +55,7 @@ sub options {
password => { optional => 1 },
'encryption-key' => { optional => 1 },
maxfiles => { optional => 1 },
+   'prune-backups' => { optional => 1 },
fingerprint => { optional => 1 },
 };
 }
diff --git a/PVE/Storage/Plugin.pm b/PVE/Storage/Plugin.pm
index 7f04e85..6e321ae 100644
--- a/PVE/Storage/Plugin.pm
+++ b/PVE/Storage/Plugin.pm
@@ -10,7 +10,7 @@ use File::Basename;
 use File::stat qw();
 
 use PVE::Tools qw(run_command);
-use PVE::JSONSchema qw(get_standard_option);
+use PVE::JSONSchema qw(get_standard_option register_standard_option);
 use PVE::Cluster qw(cfs_register_file);
 
 use JSON;
@@ -43,6 +43,62 @@ cfs_register_file ('storage.cfg',
   sub { __PACKAGE__->parse_config(@_); },
   sub { __PACKAGE__->write_config(@_); });
 
+my %prune_option = (
+optional => 1,
+type => 'integer', minimum => '0',
+format_description => 'N',
+);
+
+my $prune_backups_format = {
+   'keep-last' => {
+   %prune_option,
+   description => 'Keep the last  backups.',
+   },
+   'keep-hourly' => {
+   %prune_option,
+   description => 'Keep backups for the last  different hours. If 
there is more' .
+  'than one backup for a single hour, only the latest 
one is kept.'
+   },
+   'keep-daily' => {
+   %prune_option,
+   description => 'Keep backups for the last  different days. If 
there is more' .
+  'than one backup for a single day, only the latest 
one is kept

[pve-devel] [PATCH v4 manager 6/7] Allow prune-backups as an alternative to maxfiles

2020-07-09 Thread Fabian Ebner
and make the two options mutally exclusive as long
as they are specified on the same level (e.g. both
from the storage configuration). Otherwise prefer
option > storage config > default (only maxfiles has a default currently).

Defines the backup limit for prune-backups as the sum of all
keep-values.

There is no perfect way to determine whether a
new backup would trigger a removal with prune later:
1. we would need a way to include the not yet existing backup
   in a 'prune --dry-run' check.
2. even if we had that check, if it's executed right before
   a full hour, and the actual backup happens after the full
   hour, the information from the check is not correct.

So in some cases, we allow backup jobs with remove=0, that
will lead to a removal when the next prune is executed.
Still, the job with remove=0 does not execute a prune, so:
1. There is a well-defined limit.
2. A job with remove=0 never removes an old backup.

Signed-off-by: Fabian Ebner 
---

Changes from v3:
* allow prune-backups as a vzdump parameter
* use log function for output when pruning
* only make maxfiles and prune-backups conflict when they
  have the same preference level (e.g. both are passed as an option).


 PVE/API2/VZDump.pm |  4 +--
 PVE/VZDump.pm  | 88 --
 2 files changed, 64 insertions(+), 28 deletions(-)

diff --git a/PVE/API2/VZDump.pm b/PVE/API2/VZDump.pm
index 2eda973e..19fa1e3b 100644
--- a/PVE/API2/VZDump.pm
+++ b/PVE/API2/VZDump.pm
@@ -25,7 +25,7 @@ __PACKAGE__->register_method ({
 method => 'POST',
 description => "Create backup.",
 permissions => {
-   description => "The user needs 'VM.Backup' permissions on any VM, and 
'Datastore.AllocateSpace' on the backup storage. The 'maxfiles', 'tmpdir', 
'dumpdir', 'script', 'bwlimit' and 'ionice' parameters are restricted to the 
'root\@pam' user.",
+   description => "The user needs 'VM.Backup' permissions on any VM, and 
'Datastore.AllocateSpace' on the backup storage. The 'maxfiles', 
'prune-backups', 'tmpdir', 'dumpdir', 'script', 'bwlimit' and 'ionice' 
parameters are restricted to the 'root\@pam' user.",
user => 'all',
 },
 protected => 1,
@@ -58,7 +58,7 @@ __PACKAGE__->register_method ({
if $param->{stdout};
}
 
-   foreach my $key (qw(maxfiles tmpdir dumpdir script bwlimit ionice)) {
+   foreach my $key (qw(maxfiles prune-backups tmpdir dumpdir script 
bwlimit ionice)) {
raise_param_exc({ $key => "Only root may set this option."})
if defined($param->{$key}) && ($user ne 'root@pam');
}
diff --git a/PVE/VZDump.pm b/PVE/VZDump.pm
index 2f509555..17153fe4 100644
--- a/PVE/VZDump.pm
+++ b/PVE/VZDump.pm
@@ -89,6 +89,12 @@ sub storage_info {
maxfiles => $scfg->{maxfiles},
 };
 
+$info->{'prune-backups'} = 
PVE::JSONSchema::parse_property_string('prune-backups', 
$scfg->{'prune-backups'})
+   if defined($scfg->{'prune-backups'});
+
+die "cannot have 'maxfiles' and 'prune-backups' configured at the same 
time"
+   if defined($info->{'prune-backups'}) && defined($info->{maxfiles});
+
 if ($type eq 'pbs') {
$info->{pbs} = 1;
 } else {
@@ -459,12 +465,18 @@ sub new {
 
 if ($opts->{storage}) {
my $info = eval { storage_info ($opts->{storage}) };
-   $errors .= "could not get storage information for '$opts->{storage}': 
$@"
-   if ($@);
-   $opts->{dumpdir} = $info->{dumpdir};
-   $opts->{scfg} = $info->{scfg};
-   $opts->{pbs} = $info->{pbs};
-   $opts->{maxfiles} //= $info->{maxfiles};
+   if (my $err = $@) {
+   $errors .= "could not get storage information for 
'$opts->{storage}': $err";
+   } else {
+   $opts->{dumpdir} = $info->{dumpdir};
+   $opts->{scfg} = $info->{scfg};
+   $opts->{pbs} = $info->{pbs};
+
+   if (!defined($opts->{'prune-backups'}) && 
!defined($opts->{maxfiles})) {
+   $opts->{'prune-backups'} = $info->{'prune-backups'};
+   $opts->{maxfiles} = $info->{maxfiles};
+   }
+   }
 } elsif ($opts->{dumpdir}) {
$errors .= "dumpdir '$opts->{dumpdir}' does not exist"
if ! -d $opts->{dumpdir};
@@ -472,7 +484,9 @@ sub new {
die "internal error";
 }
 
-$opts->{maxfiles} //= $defaults->{maxfiles};
+if (!defined($opts->{'prune-backups'}) && !defined($opts->{maxfiles})) {
+   $opts->{maxfiles} = $defaults->{maxfiles};
+}
 
 if ($opts->{tmpdir} && ! -d $opts->{tmpdir}) {
$errors .= "\n" if $errors;
@@ -651,6 +665,7 @@ sub exec_backup_task {
 
 my $opts = $self->{opts};
 
+my $cfg = PVE::Storage::config();
 my $vmid = $task->{vmid};
 my $plugin = $task->{plugin};
 my $vmtype = $plugin->type();
@@ -703,8 +718,18 @@ sub exec_backup_task {
my $basename = $bkname . strftime("-%Y_%m_%d-%H_%M_%S", 
localtime($task->{backup_time}));
 
my $maxfiles

[pve-devel] [PATCH-SERIES v4] fix #2649: introduce prune-backups property for storages supporting backups

2020-07-09 Thread Fabian Ebner
A new 'prune-backups' option is introduced for storages as well as for vzdump

Patches #1-#3 introduces prune-backups in the backend
Patch #4 introduces the new vzdump parameter
Patch #6 is the 'real' change needed for vzdump.
Patch #7 gets rid of 'maxfiles' internally, re-using 'prune-backups' instead.


Changes from v3:
* Error handling for the prune_backup functions, to allow
  pruning other groups if one fails
* Allow passing a logfunc in prune_backups (useful for vzdump)
* Use new property string validation
* Make prune-backups also a vzdump option
* Introduce a wrapper for pvesm prune-backups and redirect call
  to API according to --dry-run
* Also allow filtering by type
* prune_mark: merge loops
* Add more tests

Non-standard backups are ignored when pruning. I chose 'protected'
to mark those, 'ignore' could also be used. Backups are grouped
by type+ID.

The backup type from PBS is translated: ct->lxc, vm->qemu, so that the
possible values for the API result don't depend on the backend.

Dependency bumps 'manager -> storage' and 'manager -> guest-common -> storage' 
are needed


storage:

Fabian Ebner (3):
  Introduce prune-backups property for directory-based storages
  Add prune_backups to storage API
  Add API and pvesm call for prune_backups

 PVE/API2/Storage/Makefile|   2 +-
 PVE/API2/Storage/PruneBackups.pm | 164 +++
 PVE/API2/Storage/Status.pm   |   7 +
 PVE/CLI/pvesm.pm | 126 
 PVE/Storage.pm   |  91 +++-
 PVE/Storage/CIFSPlugin.pm|   1 +
 PVE/Storage/CephFSPlugin.pm  |   1 +
 PVE/Storage/DirPlugin.pm |   5 +-
 PVE/Storage/GlusterfsPlugin.pm   |   5 +-
 PVE/Storage/NFSPlugin.pm |   5 +-
 PVE/Storage/PBSPlugin.pm |  69 +++
 PVE/Storage/Plugin.pm| 124 ++-
 test/prune_backups_test.pm   | 342 +++
 test/run_plugin_tests.pl |   1 +
 14 files changed, 933 insertions(+), 10 deletions(-)
 create mode 100644 PVE/API2/Storage/PruneBackups.pm
 create mode 100644 test/prune_backups_test.pm


guest-common:

Fabian Ebner (1):
  Add prune-backups option to vzdump parameters

 PVE/VZDump/Common.pm | 4 
 1 file changed, 4 insertions(+)


manager:

Fabian Ebner (3):
  make use of archive_info and archive_remove
  Allow prune-backups as an alternative to maxfiles
  Always use prune-backups instead of maxfiles internally

 PVE/API2/VZDump.pm |  4 +-
 PVE/VZDump.pm  | 93 ++
 2 files changed, 63 insertions(+), 34 deletions(-)

-- 
2.20.1



___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel



[pve-devel] [PATCH v4 manager 7/7] Always use prune-backups instead of maxfiles internally

2020-07-09 Thread Fabian Ebner
For the use case with '--dumpdir', it's not possible to call prune_backups
directly, so a little bit of special handling is required there.

Signed-off-by: Fabian Ebner 
---
 PVE/VZDump.pm | 42 --
 1 file changed, 16 insertions(+), 26 deletions(-)

diff --git a/PVE/VZDump.pm b/PVE/VZDump.pm
index 17153fe4..d87ef857 100644
--- a/PVE/VZDump.pm
+++ b/PVE/VZDump.pm
@@ -484,8 +484,10 @@ sub new {
die "internal error";
 }
 
-if (!defined($opts->{'prune-backups'}) && !defined($opts->{maxfiles})) {
-   $opts->{maxfiles} = $defaults->{maxfiles};
+if (!defined($opts->{'prune-backups'})) {
+   $opts->{maxfiles} //= $defaults->{maxfiles};
+   $opts->{'prune_backups'} = { 'keep-last' => $opts->{maxfiles} };
+   delete $opts->{maxfiles};
 }
 
 if ($opts->{tmpdir} && ! -d $opts->{tmpdir}) {
@@ -717,16 +719,11 @@ sub exec_backup_task {
my $bkname = "vzdump-$vmtype-$vmid";
my $basename = $bkname . strftime("-%Y_%m_%d-%H_%M_%S", 
localtime($task->{backup_time}));
 
-   my $maxfiles = $opts->{maxfiles};
my $prune_options = $opts->{'prune-backups'};
 
my $backup_limit = 0;
-   if (defined($maxfiles)) {
-   $backup_limit = $maxfiles;
-   } elsif (defined($prune_options)) {
-   foreach my $keep (values %{$prune_options}) {
-   $backup_limit += $keep;
-   }
+   foreach my $keep (values %{$prune_options}) {
+   $backup_limit += $keep;
}
 
if ($backup_limit && !$opts->{remove}) {
@@ -949,25 +946,18 @@ sub exec_backup_task {
 
# purge older backup
if ($opts->{remove}) {
-   if ($maxfiles) {
+   if (!defined($opts->{storage})) {
+   my $bklist = get_backup_file_list($opts->{dumpdir}, $bkname, 
$task->{target});
+   PVE::Storage::prune_mark_backup_group($bklist, $prune_options);
 
-   if ($self->{opts}->{pbs}) {
-   my $args = [$pbs_group_name, '--quiet', '1', '--keep-last', 
$maxfiles];
-   my $logfunc = sub { my $line = shift; debugmsg ('info', 
$line, $logfd); };
-   PVE::Storage::PBSPlugin::run_raw_client_cmd(
-   $opts->{scfg}, $opts->{storage}, 'prune', $args, 
logfunc => $logfunc);
-   } else {
-   my $bklist = get_backup_file_list($opts->{dumpdir}, 
$bkname, $task->{target});
-   $bklist = [ sort { $b->{ctime} <=> $a->{ctime} } @$bklist ];
-
-   while (scalar (@$bklist) >= $maxfiles) {
-   my $d = pop @$bklist;
-   my $archive_path = $d->{path};
-   debugmsg ('info', "delete old backup '$archive_path'", 
$logfd);
-   PVE::Storage::archive_remove($archive_path);
-   }
+   foreach my $prune_entry (@{$bklist}) {
+   next if $prune_entry->{mark} ne 'remove';
+
+   my $archive_path = $prune_entry->{path};
+   debugmsg ('info', "delete old backup '$archive_path'", 
$logfd);
+   PVE::Storage::archive_remove($archive_path);
}
-   } elsif (defined($prune_options)) {
+   } else {
my $logfunc = sub { debugmsg($_[0], $_[1], $logfd) };
PVE::Storage::prune_backups($cfg, $opts->{storage}, 
$prune_options, $vmid, $vmtype, 0, $logfunc);
}
-- 
2.20.1



___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel



[pve-devel] [PATCH v4 guest-common 4/7] Add prune-backups option to vzdump parameters

2020-07-09 Thread Fabian Ebner
Signed-off-by: Fabian Ebner 
---

New in v4

 PVE/VZDump/Common.pm | 4 
 1 file changed, 4 insertions(+)

diff --git a/PVE/VZDump/Common.pm b/PVE/VZDump/Common.pm
index 909e3af..2f3c16e 100644
--- a/PVE/VZDump/Common.pm
+++ b/PVE/VZDump/Common.pm
@@ -210,6 +210,10 @@ my $confdesc = {
minimum => 1,
default => 1,
 },
+'prune-backups' => get_standard_option('prune-backups', {
+   description => "Use these retention options instead of those from the 
storage configuration.",
+   optional => 1,
+}),
 remove => {
type => 'boolean',
description => "Remove old backup files if there are more than 
'maxfiles' backup files.",
-- 
2.20.1



___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel



[pve-devel] applied: Re: [PATCH qemu-server] fixup: use parse_property_string instead of parse_cpu_conf_basic

2020-07-09 Thread Thomas Lamprecht
On 09.07.20 14:41, Stefan Reiter wrote:
> The latter was removed and replaced with a validator.
> 
> Signed-off-by: Stefan Reiter 
> ---
> 
> Too much Rust and static compilation...
> 
>  PVE/QemuMigrate.pm | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
>

applied, thanks!


___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel



[pve-devel] applied: [PATCH storage] vdisk_list: skip scanning storages which cannot have images/rootdisks

2020-07-09 Thread Thomas Lamprecht
Do not try to scan (and thus activate) storages which aren't
configured to support (or cannot support) "vdisks" anyway.

Avoids seemingly strange failures of VM migrations due to a backup storage
not being currently online - even if that storage isn't referenced in
the VM config anywhere..

Signed-off-by: Thomas Lamprecht 
---
 PVE/Storage.pm | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/PVE/Storage.pm b/PVE/Storage.pm
index edf9a2e..8375a91 100755
--- a/PVE/Storage.pm
+++ b/PVE/Storage.pm
@@ -882,6 +882,8 @@ sub vdisk_list {
foreach my $sid (keys %$ids) {
next if $storeid && $storeid ne $sid;
next if !storage_check_enabled($cfg, $sid, undef, 1);
+   my $content = $ids->{$sid}->{content};
+   next if !($content->{rootdir} || $content->{images});
push @$storage_list, $sid;
}
 }
-- 
2.20.1



___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel



[pve-devel] applied: [PATCH container] vzdump: fix passing mountpoints for the PBS case

2020-07-09 Thread Thomas Lamprecht
The stop-mode case only worked by luck as then $snapdir == $rootdir.
But for snapshots we rsync over a clean state to a separate
directory, so this has to be used as base for the backup (just like
tar does).

Signed-off-by: Thomas Lamprecht 
---
 src/PVE/VZDump/LXC.pm | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/PVE/VZDump/LXC.pm b/src/PVE/VZDump/LXC.pm
index 29e1982..5038d3a 100644
--- a/src/PVE/VZDump/LXC.pm
+++ b/src/PVE/VZDump/LXC.pm
@@ -364,7 +364,7 @@ sub archive {
 
 if ($self->{vzdump}->{opts}->{pbs}) {
 
-   my $rootdir = $default_mount_point;
+   my $rootdir = $snapdir;
my $param = [];
 
push @$param, "pct.conf:$tmpdir/etc/vzdump/pct.conf";
@@ -376,8 +376,8 @@ sub archive {
 
push @$param, "root.pxar:$rootdir";
 
-   foreach my $disk (@$disks) {
-   push @$param, '--include-dev', $disk->{dir};
+   foreach my $disk (@sources) {
+   push @$param, '--include-dev', "$snapdir/$disk";
}
 
push @$param, '--skip-lost-and-found' if $userns_cmd;
-- 
2.20.1



___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel



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

2020-07-09 Thread 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.


___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel