Re: [pve-devel] [PATCH guest-common 1/4] vzdump: loosen mailto pattern to allow whitespaces
I'm going to try and come up with a v2 that re-uses the more complete
'email' pattern from pve-common.
On 12.02.21 13:23, Fabian Ebner wrote:
to make it more compatible to what we had previously. In pve-manger,
split_list() is called on the parameter, so any whitespaces and even stand-alone
',' and ';' are taken care of, leaving only the real addresses. This also fixes
creating a backup job with empty mailto in the UI.
For example,
vzdump 153 --mailto " ,,,develo...@proxmox.com; ad...@proxmox.com ; "
was valid and worked in PVE 6.2.
Signed-off-by: Fabian Ebner
---
PVE/VZDump/Common.pm | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/PVE/VZDump/Common.pm b/PVE/VZDump/Common.pm
index af141de..fb11b3d 100644
--- a/PVE/VZDump/Common.pm
+++ b/PVE/VZDump/Common.pm
@@ -72,7 +72,7 @@ sub parse_dow {
};
my $mailto_pattern = '[a-zA-Z0-9+._@][-a-zA-Z0-9+._@]*';
-my $mailto_list_pattern = "($mailto_pattern)([;,]$mailto_pattern)*";
+my $mailto_list_pattern = qr/([;,]?(?:$mailto_pattern|\s*))*/;
my $confdesc = {
vmid => {
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [RFC 2/2 manager] proxy: allow setting BIND_IP for pveproxy
hi,
thanks for responding!
On Wed, Feb 10, 2021 at 05:20:59PM +0100, Stoiko Ivanov wrote:
> Thanks for looking into this!
>
> On Wed, 10 Feb 2021 17:01:42 +0100
> Oguz Bektas wrote:
>
> > default to 0.0.0.0 to preserve backwards behavior
> >
> > Signed-off-by: Oguz Bektas
> > ---
> > PVE/Service/pveproxy.pm | 3 ++-
> > 1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/PVE/Service/pveproxy.pm b/PVE/Service/pveproxy.pm
> > index 571a6bf5..ce1d42a6 100755
> > --- a/PVE/Service/pveproxy.pm
> > +++ b/PVE/Service/pveproxy.pm
> > @@ -70,7 +70,8 @@ sub init {
> > die "unable to open lock file '${accept_lock_fn}' - $!\n";
> >
> > my $family = PVE::Tools::get_host_address_family($self->{nodename});
> > -my $socket = $self->create_reusable_socket(8006, undef, $family);
> > +my $bind_ip = $proxyconf->{BIND_IP} // '0.0.0.0'; # default
> any reason why the '0.0.0.0' is necessary? (the socket got created with
> undef before after all) - Given that I find the inner workings of perl
> IO::Socket::IP (which gets passed the arguments in create_reusable_socket
> eventually) a bit surprising in certain situations I think leaving it as
> it was might have its merit
after looking at it more it looks like `undef` might be better indeed.
>
> did you test it in a few different scenarios? - e.g.:
> * ipv6 only host
> * dual-stacked host
> * host with multiple interfaces and IPs
no, i've only tested ipv4 -- i'll take a look at these too
>
> > +my $socket = $self->create_reusable_socket(8006, $bind_ip, $family);
> >
> > my $dirs = {};
> >
>
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH v2 common 2/8] register email-or-username format
To be used for the mailto vzdump parameter.
Signed-off-by: Fabian Ebner
---
New in v2.
src/PVE/JSONSchema.pm | 12
1 file changed, 12 insertions(+)
diff --git a/src/PVE/JSONSchema.pm b/src/PVE/JSONSchema.pm
index 5870b69..20d72b3 100644
--- a/src/PVE/JSONSchema.pm
+++ b/src/PVE/JSONSchema.pm
@@ -478,6 +478,18 @@ sub pve_verify_email {
return $email;
}
+register_format('email-or-username', \&pve_verify_email_or_username);
+sub pve_verify_email_or_username {
+my ($email, $noerr) = @_;
+
+if ($email !~ /^$PVE::Tools::EMAIL_RE$/ &&
+ $email !~ /^$PVE::Tools::EMAIL_USER_RE$/) {
+ return undef if $noerr;
+ die "value does not look like a valid email address or user name\n";
+}
+return $email;
+}
+
register_format('dns-name', \&pve_verify_dns_name);
sub pve_verify_dns_name {
my ($name, $noerr) = @_;
--
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 v2 guest-common 3/8] vzdump: command line: refactor handling prune-backups
to re-use a line here and with the next patch.
Signed-off-by: Fabian Ebner
---
New in v2.
PVE/VZDump/Common.pm | 7 +++
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/PVE/VZDump/Common.pm b/PVE/VZDump/Common.pm
index af141de..eceea7f 100644
--- a/PVE/VZDump/Common.pm
+++ b/PVE/VZDump/Common.pm
@@ -393,11 +393,10 @@ sub command_line {
foreach my $path (split(/\0/, $v || '')) {
$cmd .= " --$p " . PVE::Tools::shellquote($path);
}
- } elsif ($p eq 'prune-backups') {
- my $property_string = PVE::JSONSchema::print_property_string($v,
'prune-backups');
- $cmd .= " --$p " . PVE::Tools::shellquote($property_string)
- if defined($property_string) && $property_string ne '';
} else {
+ $v = PVE::JSONSchema::print_property_string($v, 'prune-backups')
+ if $p eq 'prune-backups';
+
$cmd .= " --$p " . PVE::Tools::shellquote($v) if defined($v) && $v
ne '';
}
}
--
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 v2 manager 8/8] test: vzdump: rename vzdump_new_retention_test.pl to vzdump_new_test.pl
as it now also includes tests for new() with non-retention options.
Signed-off-by: Fabian Ebner
---
No changes from v1.
test/Makefile | 8
test/{vzdump_new_retention_test.pl => vzdump_new_test.pl} | 0
2 files changed, 4 insertions(+), 4 deletions(-)
rename test/{vzdump_new_retention_test.pl => vzdump_new_test.pl} (100%)
diff --git a/test/Makefile b/test/Makefile
index d383e89f..eb6f38f1 100644
--- a/test/Makefile
+++ b/test/Makefile
@@ -18,14 +18,14 @@ replication%.t: replication_test%.pl
test-mail:
./mail_test.pl
-test-vzdump: test-vzdump-guest-included test-vzdump-retention
+test-vzdump: test-vzdump-guest-included test-vzdump-new
-.PHONY: test-vzdump-guest-included test-vzdump-retention
+.PHONY: test-vzdump-guest-included test-vzdump-new
test-vzdump-guest-included:
./vzdump_guest_included_test.pl
-test-vzdump-retention:
- ./vzdump_new_retention_test.pl
+test-vzdump-new:
+ ./vzdump_new_test.pl
.PHONY: install
install:
diff --git a/test/vzdump_new_retention_test.pl b/test/vzdump_new_test.pl
similarity index 100%
rename from test/vzdump_new_retention_test.pl
rename to test/vzdump_new_test.pl
--
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 v2 manager 6/8] vzdump: refactor parsing mailto so it can be mocked
Signed-off-by: Fabian Ebner
---
No changes from v1.
PVE/API2/VZDump.pm | 11 +--
PVE/VZDump.pm | 21 +
2 files changed, 22 insertions(+), 10 deletions(-)
diff --git a/PVE/API2/VZDump.pm b/PVE/API2/VZDump.pm
index 806ac7fd..44376106 100644
--- a/PVE/API2/VZDump.pm
+++ b/PVE/API2/VZDump.pm
@@ -88,16 +88,7 @@ __PACKAGE__->register_method ({
# silent exit if specified VMs run on other nodes
return "OK" if !scalar(@{$local_vmids}) && !$param->{all};
- # exclude-path list need to be 0 separated
- if (defined($param->{'exclude-path'})) {
- my @expaths = split(/\0/, $param->{'exclude-path'} || '');
- $param->{'exclude-path'} = [ @expaths ];
- }
-
- if (defined($param->{mailto})) {
- my @mailto = PVE::Tools::split_list(extract_param($param,
'mailto'));
- $param->{mailto} = [ @mailto ];
- }
+ PVE::VZDump::parse_mailto_exclude_path($param);
die "you can only backup a single VM with option --stdout\n"
if $param->{stdout} && scalar(@{$local_vmids}) != 1;
diff --git a/PVE/VZDump.pm b/PVE/VZDump.pm
index a99d0565..2ddfa851 100644
--- a/PVE/VZDump.pm
+++ b/PVE/VZDump.pm
@@ -1177,6 +1177,27 @@ sub option_exists {
return defined($confdesc->{$key});
}
+# NOTE it might make sense to merge this and verify_vzdump_parameters(), but
one
+# needs to adapt command_line() in guest-common's PVE/VZDump/Common.pm and
detect
+# a second parsing attempt, because verify_vzdump_parameters() is called twice
+# during the update_job API call.
+sub parse_mailto_exclude_path {
+my ($param) = @_;
+
+# exclude-path list need to be 0 separated
+if (defined($param->{'exclude-path'})) {
+ my @expaths = split(/\0/, $param->{'exclude-path'} || '');
+ $param->{'exclude-path'} = [ @expaths ];
+}
+
+if (defined($param->{mailto})) {
+ my @mailto = PVE::Tools::split_list(extract_param($param, 'mailto'));
+ $param->{mailto} = [ @mailto ];
+}
+
+return;
+}
+
sub verify_vzdump_parameters {
my ($param, $check_missing) = @_;
--
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-SERIES v2] loosen up mailto regex for backwards compatibility
especially regarding the whitespace-agnostic behavior. And while we're at it,
also use the more complete email regex from pve-common.
Changes from v1:
* re-use the email regex from pve-common
* improve printing out mailto parameters to the cron file
common:
Fabian Ebner (2):
sendmail: use more complete email regex and shellquote
register email-or-username format
src/PVE/JSONSchema.pm | 14 +-
src/PVE/Tools.pm | 17 -
2 files changed, 25 insertions(+), 6 deletions(-)
guest-common:
Fabian Ebner (3):
vzdump: command line: refactor handling prune-backups
vzdump: command line: make sure mailto is comma-separated
vzdump: mailto: use email-or-username-list format
PVE/VZDump/Common.pm | 14 +-
1 file changed, 5 insertions(+), 9 deletions(-)
manager:
Fabian Ebner (3):
vzdump: refactor parsing mailto so it can be mocked
test: vzdump: add tests for mailto
test: vzdump: rename vzdump_new_retention_test.pl to
vzdump_new_test.pl
PVE/API2/VZDump.pm| 11 +-
PVE/VZDump.pm | 21 +++
test/Makefile | 8 +-
...w_retention_test.pl => vzdump_new_test.pl} | 174 +-
4 files changed, 198 insertions(+), 16 deletions(-)
rename test/{vzdump_new_retention_test.pl => vzdump_new_test.pl} (74%)
--
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 v2 guest-common 4/8] vzdump: command line: make sure mailto is comma-separated
In addition to relying on shellquote(), it's still nice to avoid printing out
unnecessary whitespaces, especially newlines.
Signed-off-by: Fabian Ebner
---
New in v2.
PVE/VZDump/Common.pm | 1 +
1 file changed, 1 insertion(+)
diff --git a/PVE/VZDump/Common.pm b/PVE/VZDump/Common.pm
index eceea7f..5d93b51 100644
--- a/PVE/VZDump/Common.pm
+++ b/PVE/VZDump/Common.pm
@@ -394,6 +394,7 @@ sub command_line {
$cmd .= " --$p " . PVE::Tools::shellquote($path);
}
} else {
+ $v = join(",", PVE::Tools::split_list($v)) if $p eq 'mailto';
$v = PVE::JSONSchema::print_property_string($v, 'prune-backups')
if $p eq 'prune-backups';
--
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 v2 common 1/8] sendmail: use more complete email regex and shellquote
Shellquote is needed for '~', and while it doesn't help with '-', there should
be no problem, because options are separated from mailto since commit
216a3f4f131693dc4bbad5e06e96a61baef5f5e9.
Signed-off-by: Fabian Ebner
---
New in v2.
Since JSONSchema already uses Tools, the pattern has to live in Tools.
src/PVE/JSONSchema.pm | 2 +-
src/PVE/Tools.pm | 17 -
2 files changed, 13 insertions(+), 6 deletions(-)
diff --git a/src/PVE/JSONSchema.pm b/src/PVE/JSONSchema.pm
index 29ada5b..5870b69 100644
--- a/src/PVE/JSONSchema.pm
+++ b/src/PVE/JSONSchema.pm
@@ -471,7 +471,7 @@ register_format('email', \&pve_verify_email);
sub pve_verify_email {
my ($email, $noerr) = @_;
-if ($email !~
/^[\w\+\-\~]+(\.[\w\+\-\~]+)*@[a-zA-Z0-9\-]+(\.[a-zA-Z0-9\-]+)*$/) {
+if ($email !~ /^$PVE::Tools::EMAIL_RE$/) {
return undef if $noerr;
die "value does not look like a valid email address\n";
}
diff --git a/src/PVE/Tools.pm b/src/PVE/Tools.pm
index 7fefa52..fc4a367 100644
--- a/src/PVE/Tools.pm
+++ b/src/PVE/Tools.pm
@@ -87,6 +87,9 @@ our $IPV6RE = "(?:" .
our $IPRE = "(?:$IPV4RE|$IPV6RE)";
+our $EMAIL_USER_RE = qr/[\w\+\-\~]+(\.[\w\+\-\~]+)*/;
+our $EMAIL_RE = qr/$EMAIL_USER_RE@[a-zA-Z0-9\-]+(\.[a-zA-Z0-9\-]+)*/;
+
use constant {CLONE_NEWNS => 0x0002,
CLONE_NEWUTS => 0x0400,
CLONE_NEWIPC => 0x0800,
@@ -1469,23 +1472,27 @@ sub sync_mountpoint {
# mailto may be a single email string or an array of receivers
sub sendmail {
my ($mailto, $subject, $text, $html, $mailfrom, $author) = @_;
-my $mail_re = qr/[^-a-zA-Z0-9+._@]/;
$mailto = [ $mailto ] if !ref($mailto);
+my $mailto_quoted = [];
for my $to (@$mailto) {
- die "illegal character in mailto address\n" if $to =~ $mail_re;
+ die "mailto does not look like a valid email address or username\n"
+ if $to !~ /^$EMAIL_RE$/ && $to !~ /^$EMAIL_USER_RE$/;
+ push @$mailto_quoted, shellquote($to);
}
my $rcvrtxt = join (', ', @$mailto);
$mailfrom = $mailfrom || "root";
-die "illegal character in mailfrom address\n" if $mailfrom =~ $mail_re;
+die "mailfrom does not look like a valid email address or username\n"
+ if $mailfrom !~ /^$EMAIL_RE$/ && $mailfrom !~ /^$EMAIL_USER_RE$/;
+my $mailfrom_quoted = shellquote($mailfrom);
$author = $author // 'Proxmox VE';
-open (MAIL, "|-", "sendmail", "-B", "8BITMIME", "-f", $mailfrom, "--",
@$mailto) ||
- die "unable to open 'sendmail' - $!";
+open (MAIL, "|-", "sendmail", "-B", "8BITMIME", "-f", $mailfrom_quoted,
+ "--", @$mailto_quoted) || die "unable to open 'sendmail' - $!";
my $date = time2str('%a, %d %b %Y %H:%M:%S %z', time());
--
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 v2 guest-common 5/8] vzdump: mailto: use email-or-username-list format
because it is a more complete pattern. Also, 'mailto' was a '-list' format in
PVE 6.2 and earlier, so this also fixes whitespace-related backwards
compatibility. In particular, this fixes creating a backup job in the GUI
without setting an address, which passes along ''.
For example,
> vzdump 153 --mailto " ,,,ad...@proxmox.com;;; develo...@proxmox.com , ; "
was valid and worked in PVE 6.2.
Signed-off-by: Fabian Ebner
---
Changes from v1:
* re-use the more complete pattern from pve-common
Dependency bump for pve-common is needed.
PVE/VZDump/Common.pm | 6 +-
1 file changed, 1 insertion(+), 5 deletions(-)
diff --git a/PVE/VZDump/Common.pm b/PVE/VZDump/Common.pm
index 5d93b51..86cb7bd 100644
--- a/PVE/VZDump/Common.pm
+++ b/PVE/VZDump/Common.pm
@@ -71,9 +71,6 @@ sub parse_dow {
return $res;
};
-my $mailto_pattern = '[a-zA-Z0-9+._@][-a-zA-Z0-9+._@]*';
-my $mailto_list_pattern = "($mailto_pattern)([;,]$mailto_pattern)*";
-
my $confdesc = {
vmid => {
type => 'string', format => 'pve-vmid-list',
@@ -146,8 +143,7 @@ my $confdesc = {
},
mailto => {
type => 'string',
- pattern => $mailto_list_pattern,
- format_description => 'email-or-username-list',
+ format => 'email-or-username-list',
description => "Comma-separated list of email addresses or users that
should" .
" receive email notifications.",
optional => 1,
--
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 v2 manager 7/8] test: vzdump: add tests for mailto
Re-use the existing code, by allowing special kinds of 'tests' that just set
the options that are tested for.
Signed-off-by: Fabian Ebner
---
No changes from v1.
Dependency bump for pve-guest-common is needed.
test/vzdump_new_retention_test.pl | 174 +-
1 file changed, 172 insertions(+), 2 deletions(-)
diff --git a/test/vzdump_new_retention_test.pl
b/test/vzdump_new_retention_test.pl
index 569419fb..a8f70d62 100755
--- a/test/vzdump_new_retention_test.pl
+++ b/test/vzdump_new_retention_test.pl
@@ -73,7 +73,7 @@ $pve_tools_module->mock(
},
);
-my @tested_options = qw(prune-backups remove);
+my $tested_options;
# each test consists of the following:
# name - unique name for the test
@@ -81,7 +81,13 @@ my @tested_options = qw(prune-backups remove);
# storage_param - parameters for the mocked storage configuration
# vzdump_param - parameters for the mocked /etc/vzdump.conf
# expected - expected options
+#
+# To begin testing for different options, use a fake test like the first one
my @tests = (
+{
+ description => 'BEGIN RETENTION TESTS',
+ tested_options => ['prune-backups', 'remove'],
+},
{
description => 'no params',
expected => {
@@ -464,11 +470,174 @@ my @tests = (
remove => 0,
},
},
+{
+ description => 'BEGIN MAILTO TESTS',
+ tested_options => ['mailto'],
+},
+{
+ description => 'mailto vzdump 1',
+ vzdump_param => {
+ 'mailto' => 'develo...@proxmox.com',
+ },
+ expected => {
+ 'mailto' => [
+ 'develo...@proxmox.com',
+ ],
+ },
+},
+{
+ description => 'mailto vzdump 2',
+ vzdump_param => {
+ 'mailto' => 'develo...@proxmox.com ad...@proxmox.com',
+ },
+ expected => {
+ 'mailto' => [
+ 'develo...@proxmox.com',
+ 'ad...@proxmox.com',
+ ],
+ },
+},
+{
+ description => 'mailto vzdump 3',
+ vzdump_param => {
+ 'mailto' => 'develo...@proxmox.com,ad...@proxmox.com',
+ },
+ expected => {
+ 'mailto' => [
+ 'develo...@proxmox.com',
+ 'ad...@proxmox.com',
+ ],
+ },
+},
+{
+ description => 'mailto vzdump 4',
+ vzdump_param => {
+ 'mailto' => 'develo...@proxmox.com, ad...@proxmox.com',
+ },
+ expected => {
+ 'mailto' => [
+ 'develo...@proxmox.com',
+ 'ad...@proxmox.com',
+ ],
+ },
+},
+{
+ description => 'mailto vzdump 5',
+ vzdump_param => {
+ 'mailto' => ' ,,; develo...@proxmox.com, ; ad...@proxmox.com ',
+ },
+ expected => {
+ 'mailto' => [
+ 'develo...@proxmox.com',
+ 'ad...@proxmox.com',
+ ],
+ },
+},
+{
+ description => 'mailto vzdump 6',
+ vzdump_param => {
+ 'mailto' => '',
+ },
+ expected => {
+ 'mailto' => [],
+ },
+},
+{
+ description => 'mailto CLI 1',
+ cli_param => {
+ 'mailto' => 'develo...@proxmox.com',
+ },
+ expected => {
+ 'mailto' => [
+ 'develo...@proxmox.com',
+ ],
+ },
+},
+{
+ description => 'mailto CLI 2',
+ cli_param => {
+ 'mailto' => 'develo...@proxmox.com ad...@proxmox.com',
+ },
+ expected => {
+ 'mailto' => [
+ 'develo...@proxmox.com',
+ 'ad...@proxmox.com',
+ ],
+ },
+},
+{
+ description => 'mailto CLI 3',
+ cli_param => {
+ 'mailto' => 'develo...@proxmox.com,ad...@proxmox.com',
+ },
+ expected => {
+ 'mailto' => [
+ 'develo...@proxmox.com',
+ 'ad...@proxmox.com',
+ ],
+ },
+},
+{
+ description => 'mailto CLI 4',
+ cli_param => {
+ 'mailto' => 'develo...@proxmox.com, ad...@proxmox.com',
+ },
+ expected => {
+ 'mailto' => [
+ 'develo...@proxmox.com',
+ 'ad...@proxmox.com',
+ ],
+ },
+},
+{
+ description => 'mailto CLI 5',
+ cli_param => {
+ 'mailto' => ' ,,; develo...@proxmox.com, ; ad...@proxmox.com ',
+ },
+ expected => {
+ 'mailto' => [
+ 'develo...@proxmox.com',
+ 'ad...@proxmox.com',
+ ],
+ },
+},
+{
+ description => 'mailto both 1',
+ vzdump_param => {
+ 'mailto' => 'develo...@proxmox.com',
+ },
+ cli_param => {
+ 'mailto' => 'ad...@proxmox.com',
+ },
+ expected => {
+ 'mailto' => [
+ 'ad...@proxmox.com',
+ ],
+ },
+},
+{
+ description => 'mailto b
Re: [pve-devel] [PATCH v5 series 0/5] disk reassign: add new feature
Bump in case this has been missed. :) The patches should still apply fine. (just tested) On 12/15/20 1:48 PM, Aaron Lauterer wrote: This series implements a new feature which allows users to easily reassign disks between VMs. Currently this is only possible with one of the following manual steps: * rename the disk image/file and do a `qm rescan` * configure the disk manually and use the old image name, having an image for VM A assigned to VM B The latter can cause unexpected behavior because PVE expects that the VMID in a disk name always corresponds to the VM it is assigned to. Thus when a disk, original from VM A was manually configured as disk for VM B it happens that, when deleting VM A, the disk in question will be deleted as well because it still had the VMID of VM A in it's name. To issue a reassign from the CLI run: qm reassign_disk where is the config key of the disk, e.g. ide0, scsi1 and so on. The following storage types are implemented at the moment: * dir based ones * directory * NFS * CIFS * gluster * ZFS * (thin) LVM * Ceph RBD v4 -> v5: * rebase on current master * reorder patches * rename `drive_key` to `drive_name` thanks @Dominic for pointing out that there already are a lot of different names in use for this [0] and not to invent another one ;) * implemented suggested changes from Fabian [1][2]. More directly in the patches themselves v3 -> v4: * revert intermediate storage plugin for directory based plugins * add a `die "not supported"` method in Plugin.pm * dir based plugins now call the file_reassign_volume method in Plugin.pm as the generic file/directory based method * restored old `volume_has_feature` method in Plugin.pm and override it in directory based plugins to check against the new `reassign` feature (not too happy about the repetition for each plugin) * task description mapping has been moved from widget-toolkit to pve-manager/utils v2 -> v3: * change locking approach * add more checks * add intermedia storage plugin for directory based plugins * use feature flags * split up the reassign method to have a dedicated method for the renaming itself * handle linked clones * clean up if disk used to be replicated I hope I didn't forget anything major. v1 -> v2: print info about the new disk volid and key at the end of the job so it shows up in the CLI output and task log Changes from RFC -> V1: * support to reassign unused disks * digest for target vm config * reorder the checks a bit * adding another one to check if the given key for the disk even exists in the config. [0] https://lists.proxmox.com/pipermail/pve-devel/2020-November/045986.html [1] https://lists.proxmox.com/pipermail/pve-devel/2020-November/046031.html [2] https://lists.proxmox.com/pipermail/pve-devel/2020-November/046030.html storage: Aaron Lauterer (1): add disk reassign feature PVE/Storage.pm | 15 -- PVE/Storage/CIFSPlugin.pm | 19 + PVE/Storage/DirPlugin.pm | 19 + PVE/Storage/GlusterfsPlugin.pm | 19 + PVE/Storage/LVMPlugin.pm | 24 PVE/Storage/LvmThinPlugin.pm | 1 + PVE/Storage/NFSPlugin.pm | 19 + PVE/Storage/Plugin.pm | 51 ++ PVE/Storage/RBDPlugin.pm | 31 + PVE/Storage/ZFSPoolPlugin.pm | 28 +++ 10 files changed, 224 insertions(+), 2 deletions(-) qemu-server: Aaron Lauterer (2): disk reassign: add API endpoint cli: disk reassign: add reassign_disk to qm command PVE/API2/Qemu.pm| 151 PVE/CLI/qm.pm | 2 + PVE/QemuServer/Drive.pm | 4 ++ 3 files changed, 157 insertions(+) guest-common: Aaron Lauterer (1): Replication: mention disk reassign in comment of possible reasons PVE/Replication.pm | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) manager: Aaron Lauterer (1): ui: tasks: add qmreassign task description www/manager6/Utils.js | 1 + 1 file changed, 1 insertion(+) ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH v5 series 0/3] Add GUI for disk reassignment
This patch series adds the GUI to the recent patch series [0] which enables the reassignment of disks between VMs. For this to work, the backend patch series [0] needs to be applied and installed. v4 -> v5: * rebased * removed whitespace patch as these issues have been addressed in the meantime v3 -> v4: * rebased * added check to not add template VMs to dropdown * renamed disk parameter to drive_name v2 -> v3: * fixed check if same VMID for dropdown * renamed disk parameter to drive_key v1 -> v2: linter fixes [0] https://lists.proxmox.com/pipermail/pve-devel/2020-December/046479.html widget-toolkit: Aaron Lauterer (1): window/edit: add option to disable reset button src/window/Edit.js | 5 - 1 file changed, 4 insertions(+), 1 deletion(-) manager: Aaron Lauterer (2): ui: utils: add method to get VM data from resource store ui: qemu: Add disk reassign dialog www/manager6/Makefile | 1 + www/manager6/Utils.js | 13 + www/manager6/qemu/HDReassign.js | 80 +++ www/manager6/qemu/HardwareView.js | 30 4 files changed, 124 insertions(+) create mode 100644 www/manager6/qemu/HDReassign.js -- 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 v5 manager 2/3] ui: utils: add method to get VM data from resource store
Signed-off-by: Aaron Lauterer
---
v3 -> v4i -> v5: rebased
v2 -> v3: nothing changed
v1 -> v2: fixed linter errors
www/manager6/Utils.js | 13 +
1 file changed, 13 insertions(+)
diff --git a/www/manager6/Utils.js b/www/manager6/Utils.js
index 7a1a7fb6..a298aac0 100644
--- a/www/manager6/Utils.js
+++ b/www/manager6/Utils.js
@@ -1833,4 +1833,17 @@ Ext.define('PVE.Utils', {
});
},
+getNodeVMs: function(nodename) {
+ let rstore = PVE.data.ResourceStore;
+ let vms = {};
+ rstore.data.items.forEach((item) => {
+ if (!item.id.startsWith('qemu/')) { return; }
+ let vmdata = item.data;
+ if (vmdata.node !== nodename) { return; }
+
+ vms[vmdata.vmid] = vmdata;
+ });
+ return vms;
+},
+
});
--
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 v5 widget-toolkit 1/3] window/edit: add option to disable reset button
Sometimes the reset button does not make sense and the isCreate option
does not fit as well because with it, the submit button will be enabled
right away instead of waiting for the form to be valid.
Signed-off-by: Aaron Lauterer
---
v1 -> v2 -> v3 -> v4 -> v5: nothing changed
This helps to reuse the PVE.window.Edit component in a modern viewModel
and controller way instead of building it all by hand the old way with a
large initComponent method.
A possible candidate for refactoring after this would be the
qemu/HDMove.js component.
src/window/Edit.js | 5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/src/window/Edit.js b/src/window/Edit.js
index e952362..548beed 100644
--- a/src/window/Edit.js
+++ b/src/window/Edit.js
@@ -25,6 +25,9 @@ Ext.define('Proxmox.window.Edit', {
// set to true if you want an Remove button (instead of Create)
isRemove: false,
+// set to false, if you don't want the reset button present
+showReset: true,
+
// custom submitText
submitText: undefined,
@@ -350,7 +353,7 @@ Ext.define('Proxmox.window.Edit', {
me.title = Proxmox.Utils.dialog_title(me.subject, me.isCreate,
me.isAdd);
}
- if (me.isCreate) {
+ if (me.isCreate || !me.showReset) {
me.buttons = [submitBtn];
} else {
me.buttons = [submitBtn, resetBtn];
--
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 v5 manager 3/3] ui: qemu: Add disk reassign dialog
Adds a new button to the hardware panel labeled 'Reassign disk' and
enables a user to reassign a disk to another VM.
Signed-off-by: Aaron Lauterer
---
v4 -> v5: rebased
v3 -> v4:
* added check to not show template VMs in dropdown
* renamed disk parameter to `drive_name`
v2 -> v3:
* fixed check to omit the current vmid in the target dropdown
* renamed parameter disk to drive_key
* added missing comma
v1 -> v2: fixed linter errors
This patch needs the previous patch series [0] applied which adds the
backend for disk reassignments.
[0] https://lists.proxmox.com/pipermail/pve-devel/2020-December/046479.html
www/manager6/Makefile | 1 +
www/manager6/qemu/HDReassign.js | 80 +++
www/manager6/qemu/HardwareView.js | 30
3 files changed, 111 insertions(+)
create mode 100644 www/manager6/qemu/HDReassign.js
diff --git a/www/manager6/Makefile b/www/manager6/Makefile
index 85f90ecd..21debfcb 100644
--- a/www/manager6/Makefile
+++ b/www/manager6/Makefile
@@ -200,6 +200,7 @@ JSSRC=
\
qemu/HDEdit.js \
qemu/HDEfi.js \
qemu/HDMove.js \
+ qemu/HDReassign.js \
qemu/HDResize.js\
qemu/HardwareView.js\
qemu/IPConfigEdit.js\
diff --git a/www/manager6/qemu/HDReassign.js b/www/manager6/qemu/HDReassign.js
new file mode 100644
index ..d0aa97f8
--- /dev/null
+++ b/www/manager6/qemu/HDReassign.js
@@ -0,0 +1,80 @@
+Ext.define('PVE.window.HDReassign', {
+extend: 'Proxmox.window.Edit',
+
+resizeable: false,
+title: gettext('Reassign disk'),
+submitText: gettext('Reassign disk'),
+showReset: false,
+method: 'POST',
+showProgress: true,
+width: 350,
+
+viewModel: {
+ data: {
+ targetVMData: {},
+ show_running_hint: false,
+ },
+},
+
+items: [
+ {
+ xtype: 'combobox',
+ name: 'target_vmid',
+ reference: 'target_vmid',
+ fieldLabel: gettext('Target VMID'),
+ bind: {
+ store: '{targetVMData}',
+ },
+ queryMode: 'local',
+ displayField: 'name',
+ valueField: 'vmid',
+ anyMatch: true,
+ emptyText: gettext('Select VM'),
+ },
+ {
+ xtype: 'displayfield',
+ padding: '5 0 0 0',
+ userCls: 'pmx-hint',
+ value: gettext('This disk cannot be reassigned while the VM is
running'),
+ bind: {
+ hidden: '{!show_running_hint}',
+ },
+ },
+],
+
+controller: {
+ xclass: 'Ext.app.ViewController',
+ init: function(view) {
+ let me = view;
+ let vm = me.getViewModel();
+ let vms = PVE.Utils.getNodeVMs(me.nodename);
+
+ let show_running_hint = vms[me.vmid].running &&
!me.drive_name.startsWith('unused');
+ vm.set('show_running_hint', show_running_hint);
+
+ if (show_running_hint) {
+ me.lookup('target_vmid').setDisabled(true);
+ }
+
+ let dropdownData = [];
+ for (const [vmid, data] of Object.entries(vms)) {
+ if (parseInt(vmid, 10) === parseInt(me.vmid, 10)) { continue; }
+ if (data.template) { continue; }
+ dropdownData.push({
+ vmid: vmid,
+ name: `${vmid} ${data.name}`,
+ });
+ }
+
+ vm.set('targetVMData', { data: dropdownData });
+ },
+},
+
+getValues: function() {
+ let me = this;
+ let values = me.callParent(arguments);
+ values.drive_name = me.drive_name;
+
+ return values;
+},
+});
diff --git a/www/manager6/qemu/HardwareView.js
b/www/manager6/qemu/HardwareView.js
index 77640e53..d129deda 100644
--- a/www/manager6/qemu/HardwareView.js
+++ b/www/manager6/qemu/HardwareView.js
@@ -415,6 +415,25 @@ Ext.define('PVE.qemu.HardwareView', {
win.on('destroy', me.reload, me);
};
+ var run_reassign = function() {
+ let rec = sm.getSelection()[0];
+ if (!rec) {
+ return;
+ }
+
+ let win = Ext.create('PVE.window.HDReassign', {
+ drive_name: rec.data.key,
+ vmid: vmid,
+ nodename: nodename,
+ title: gettext('Reassign disk') + ': ' + rec.data.key,
+ url: '/api2/extjs/nodes/' + nodename + '/qemu/' + vmid +
'/reassign_disk',
+ });
+
+ win.show();
+
+ win.on('destroy', me.reload, me);
+ };
+
var edit_btn = new Proxmox.button.Button({
text: gettext('Edit'),
selModel: sm,
@@ -436,6 +455,13 @@ Ext.defi
[pve-devel] applied: [PATCH zfsonlinux] update submodule and patches to zfs-2.0.3
On 12.02.21 18:28, Stoiko Ivanov wrote: > Signed-off-by: Stoiko Ivanov > --- > did a quick test on my zfs storage-replication testcluster: > * both systems failed to import their zfs pools upon boot (I'm quite sure > it's related to upstream commit 642d86af0d91b2bf88d5ea34cb6888b03c39c459) > * both systems were used for running the zfs testsuite - so probably don't > really represent a clean production-ready state > * importing the pool worked after a manual import (consistently, even > after reboots) > * did not have similiar issues on 4 other systems I tested this on > > ...ith-d-dev-disk-by-id-in-scan-service.patch | 2 +- > .../0010-Set-file-mode-during-zfs_write.patch | 39 --- > debian/patches/series | 1 - > upstream | 2 +- > 4 files changed, 2 insertions(+), 42 deletions(-) > delete mode 100644 debian/patches/0010-Set-file-mode-during-zfs_write.patch > > applied, thanks! ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
