[pve-devel] [PATCH qemu-server 1/2] restore: refactor archive parsing
to avoid duplicate work, always set 'volid' to the backup volume's volid, if it
was successfully parsed as such.
Signed-off-by: Fabian Grünbichler
---
PVE/API2/Qemu.pm | 16
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
index d0c199bf..b7832d88 100644
--- a/PVE/API2/Qemu.pm
+++ b/PVE/API2/Qemu.pm
@@ -750,20 +750,20 @@ my $parse_restore_archive = sub {
my ($archive_storeid, $archive_volname) =
PVE::Storage::parse_volume_id($archive, 1);
+my $res = {};
+
if (defined($archive_storeid)) {
my $scfg = PVE::Storage::storage_config($storecfg, $archive_storeid);
+ $res->{volid} = $archive;
if ($scfg->{type} eq 'pbs') {
- return {
- type => 'pbs',
- volid => $archive,
- };
+ $res->{type} = 'pbs';
+ return $res;
}
}
my $path = PVE::Storage::abs_filesystem_path($storecfg, $archive);
-return {
- type => 'file',
- path => $path,
-};
+$res->{type} = 'file';
+$res->{path} = $path;
+return $res;
};
--
2.39.2
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH qemu-server 2/2] restore: extend permissions checks
to allow early checking of the merged config, if the backup archive passed in
is a proper volume where extraction is possible.
Signed-off-by: Fabian Grünbichler
---
Notes:
this check needs to be inside the worker since we don't know how long
extracting the config takes..
for pipe (doesn't allow early extraction at all) or absolute path (not
supported by the extraction helper) we don't need to check, since those are
limited to root anyhow.
PVE/API2/Qemu.pm | 13 +
PVE/QemuServer.pm | 11 +--
2 files changed, 18 insertions(+), 6 deletions(-)
diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
index b7832d88..59307133 100644
--- a/PVE/API2/Qemu.pm
+++ b/PVE/API2/Qemu.pm
@@ -958,6 +958,19 @@ __PACKAGE__->register_method({
live => $live_restore,
override_conf => $param,
};
+ if (my $volid = $archive->{volid}) {
+ # best effort, real check is after restoring!
+ my $merged = eval {
+ my $old_conf =
PVE::Storage::extract_vzdump_config($storecfg, $volid);
+
PVE::QemuServer::restore_merge_config("backup/qemu-server/$vmid.conf",
$old_conf, $param);
+ };
+ if ($@) {
+ warn "Could not extract backed up config: $@\n";
+ warn "Skipping early checks!\n";
+ } else {
+ PVE::QemuServer::check_restore_permissions($rpcenv,
$authuser, $merged);
+ }
+ }
if ($archive->{type} eq 'file' || $archive->{type} eq 'pipe') {
die "live-restore is only compatible with backup images
from a Proxmox Backup Server\n"
if $live_restore;
diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
index 940cdacd..0fa43a74 100644
--- a/PVE/QemuServer.pm
+++ b/PVE/QemuServer.pm
@@ -6542,10 +6542,9 @@ sub check_mapping_access {
}
};
-# FIXME: improve checks on restore by checking before actually extracing and
-# merging the new config
sub check_restore_permissions {
my ($rpcenv, $user, $conf) = @_;
+
check_bridge_access($rpcenv, $user, $conf);
check_mapping_access($rpcenv, $user, $conf);
}
@@ -6865,7 +6864,7 @@ my $restore_destroy_volumes = sub {
}
};
-my $restore_merge_config = sub {
+sub restore_merge_config {
my ($filename, $backup_conf_raw, $override_conf) = @_;
my $backup_conf = parse_vm_config($filename, $backup_conf_raw);
@@ -6874,7 +6873,7 @@ my $restore_merge_config = sub {
}
return $backup_conf;
-};
+}
sub scan_volids {
my ($cfg, $vmid) = @_;
@@ -7192,7 +7191,7 @@ sub restore_proxmox_backup_archive {
$new_conf_raw .= "\nlock: create";
}
-my $new_conf = $restore_merge_config->($conffile, $new_conf_raw,
$options->{override_conf});
+my $new_conf = restore_merge_config($conffile, $new_conf_raw,
$options->{override_conf});
check_restore_permissions($rpcenv, $user, $new_conf);
PVE::QemuConfig->write_config($vmid, $new_conf);
@@ -7506,7 +7505,7 @@ sub restore_vma_archive {
die $err;
}
-my $new_conf = $restore_merge_config->($conffile, $new_conf_raw,
$opts->{override_conf});
+my $new_conf = restore_merge_config($conffile, $new_conf_raw,
$opts->{override_conf});
check_restore_permissions($rpcenv, $user, $new_conf);
PVE::QemuConfig->write_config($vmid, $new_conf);
--
2.39.2
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH v2 pve-docs] qemu: add cpu models documentation
Am 19.06.23 um 12:03 schrieb Fiona Ebner: > Am 19.06.23 um 10:38 schrieb Alexandre Derumier: >> add doc for differents cpu models including >> new x86-64-vX models >> >> changelog v2: x86-64-v2 is compatible with >= opteron_g3 >> I'm applying this with follow-ups for my suggestions, so no need to send a v3 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH manager 1/1] api: nodes: allow setting HA shutdown policy during shutdown/reboot
On June 16, 2023 1:33 pm, Fiona Ebner wrote:
> Increases flexibility/user-friendliness.
>
> Suggested-by: Thomas Lamprecht
> Signed-off-by: Fiona Ebner
> ---
>
> (Build-)dependency bump for libpve-cluster-perl needed.
>
> Dependency bump for ha-manager needed (to have the runtime dir exist
> and LRM honor the new param).
>
> PVE/API2/Nodes.pm | 11 ++-
> 1 file changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/PVE/API2/Nodes.pm b/PVE/API2/Nodes.pm
> index 9269694d..1cb83af5 100644
> --- a/PVE/API2/Nodes.pm
> +++ b/PVE/API2/Nodes.pm
> @@ -8,7 +8,7 @@ use Digest::SHA;
> use Filesys::Df;
> use HTTP::Status qw(:constants);
> use JSON;
> -use POSIX qw(LONG_MAX);
> +use POSIX qw(ENOENT LONG_MAX);
> use Time::Local qw(timegm_nocheck);
> use Socket;
> use IO::Socket::SSL;
> @@ -557,12 +557,21 @@ __PACKAGE__->register_method({
> type => 'string',
> enum => [qw(reboot shutdown)],
> },
> + 'shutdown-policy' => get_standard_option('pve-ha-shutdown-policy',
> { optional => 1 }),
> },
> },
> returns => { type => "null" },
> code => sub {
> my ($param) = @_;
>
> + my $sp_override_fn = '/run/pve-ha-lrm/shutdown-policy.local-override';
> + if ($param->{'shutdown-policy'}) {
> + eval { PVE::Tools::file_set_contents($sp_override_fn,
> $param->{'shutdown-policy'}); };
> + die "could not write shutdown policy override to $sp_override_fn -
> $@" if $@;
> + } else {
> + unlink $sp_override_fn or $! == ENOENT or die "unable to remove
> $sp_override_fn - $!";
> + }
should this part (setting or removing the override if one is set)
require `Sys.Modify` (like setting the policy via datacenter.cfg would)
in addition to `Sys.PowerMgmt` that the endpoint itself requires?
> +
> if ($param->{command} eq 'reboot') {
> system ("(sleep 2;/sbin/reboot)&");
> } elsif ($param->{command} eq 'shutdown') {
> --
> 2.39.2
>
>
>
> ___
> pve-devel mailing list
> pve-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
>
>
>
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH installer] remove nonexistent file from makefile
Signed-off-by: Maximiliano Sandoval --- Proxmox/Makefile | 1 - 1 file changed, 1 deletion(-) diff --git a/Proxmox/Makefile b/Proxmox/Makefile index d49da80..6360064 100644 --- a/Proxmox/Makefile +++ b/Proxmox/Makefile @@ -6,7 +6,6 @@ PERL5DIR=$(DESTDIR)/usr/share/perl5/Proxmox #PERL_MODULES=$(shell git ls-files . | grep '.pm$') PERL_MODULES=\ -Install.pm \ Install/Config.pm \ Install/ISOEnv.pm \ Install/RunEnv.pm \ -- 2.39.2 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH installer] fix #4643: show a confirmation dialog when clicking abort
Note that unlike the rest of the file, we connect to the response signal
instead of using Gtk3::Dialog->run, the reason is that run blocks the
main loop used by GTK and this undesirable to the point where
Gtk3::Dialog->run was removed for GTK 4.
Signed-off-by: Maximiliano Sandoval
---
proxinstall | 12 +++-
1 file changed, 11 insertions(+), 1 deletion(-)
diff --git a/proxinstall b/proxinstall
index fb365d4..015496b 100755
--- a/proxinstall
+++ b/proxinstall
@@ -1353,7 +1353,17 @@ sub create_main_window {
my $abort = Gtk3::Button->new('_Abort');
$abort->set_can_focus(0);
$cmdbox->pack_start($abort, 0, 0, 10);
-$abort->signal_connect(clicked => sub { app_quit(-1); });
+$abort->signal_connect(clicked => sub {
+ my $msg = 'Are you sure you want to abort the installation?';
+ my $dialog = Gtk3::MessageDialog->new($window, 'modal', 'question',
'yes-no', $msg);
+ $dialog->signal_connect(response => sub {
+ my ($dialog, $response) = @_;
+
+ $dialog->close();
+ app_quit(-1) if $response eq 'yes';
+ });
+ $dialog->present();
+});
my $vbox2 = Gtk3::Box->new('vertical', 0);
$hbox->add($vbox2);
--
2.39.2
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH installer 1/7] move create_basic_grid subroutine definition up
This will be used in future commits to create grids so we need it to be defined.
Signed-off-by: Maximiliano Sandoval
---
proxinstall | 30 +++---
1 file changed, 15 insertions(+), 15 deletions(-)
diff --git a/proxinstall b/proxinstall
index fb365d4..fbd260b 100755
--- a/proxinstall
+++ b/proxinstall
@@ -1482,6 +1482,21 @@ sub create_cidr_inputs {
my $ipconf_first_view = 1;
+my $create_basic_grid = sub {
+my $grid = Gtk3::Grid->new();
+$grid->set_visible(1);
+$grid->set_column_spacing(10);
+$grid->set_row_spacing(10);
+$grid->set_hexpand(1);
+
+$grid->set_margin_start(10);
+$grid->set_margin_end(20);
+$grid->set_margin_top(5);
+$grid->set_margin_bottom(5);
+
+return $grid;
+};
+
sub create_ipconf_view {
cleanup_view();
@@ -2076,21 +2091,6 @@ my $target_hd_label;
my $hdoption_first_setup = 1;
-my $create_basic_grid = sub {
-my $grid = Gtk3::Grid->new();
-$grid->set_visible(1);
-$grid->set_column_spacing(10);
-$grid->set_row_spacing(10);
-$grid->set_hexpand(1);
-
-$grid->set_margin_start(10);
-$grid->set_margin_end(20);
-$grid->set_margin_top(5);
-$grid->set_margin_bottom(5);
-
-return $grid;
-};
-
my $create_label_widget_grid = sub {
my ($labeled_widgets) = @_;
--
2.39.2
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH installer 0/7] use gtk grids when possible
We replace the use of nested boxes with grids when possible to ensure correct margins and alignment of widgets. At the end we also remove trailing spaces and colons from labels preceding an input widget, e.g. `DNS Server: `. Maximiliano Sandoval (7): move create_basic_grid subroutine definition up use basic grid in the network panel expand ip address Gtk3::Entry use basic grid in password panel use basic grid in country/timezone panel change margins in create_basic_grid remove trailing spaces and colons from labels proxinstall | 164 1 file changed, 74 insertions(+), 90 deletions(-) -- 2.39.2 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH installer 6/7] change margins in create_basic_grid
Previously the grids were inserted in a succession of boxes each with
its own set of margins and spacing. We define the margins now
exclusively in the grid and account for previous values.
Note that we match the top and bottom margins of the 'Target Harddisk'
panel which does not need to use a grid.
Signed-off-by: Maximiliano Sandoval
---
proxinstall | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/proxinstall b/proxinstall
index 29ed8dc..86922e2 100755
--- a/proxinstall
+++ b/proxinstall
@@ -1484,10 +1484,10 @@ my $create_basic_grid = sub {
$grid->set_row_spacing(10);
$grid->set_hexpand(1);
-$grid->set_margin_start(10);
+$grid->set_margin_start(20);
$grid->set_margin_end(20);
-$grid->set_margin_top(5);
-$grid->set_margin_bottom(5);
+$grid->set_margin_top(10);
+$grid->set_margin_bottom(10);
return $grid;
};
--
2.39.2
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH installer 5/7] use basic grid in country/timezone panel
Signed-off-by: Maximiliano Sandoval
---
proxinstall | 34 --
1 file changed, 12 insertions(+), 22 deletions(-)
diff --git a/proxinstall b/proxinstall
index faebdec..29ed8dc 100755
--- a/proxinstall
+++ b/proxinstall
@@ -1781,12 +1781,12 @@ sub update_layout {
my $lastzonecb;
sub update_zonelist {
-my ($box, $cc) = @_;
+my ($grid, $cc) = @_;
my $sel = Proxmox::Install::Config::get_timezone(); # initial default
if ($lastzonecb) {
$sel = $lastzonecb->get_active_text();
- $box->remove($lastzonecb);
+ $grid->remove($lastzonecb);
}
my $cb = $lastzonecb = Gtk3::ComboBoxText->new();
@@ -1812,7 +1812,7 @@ sub update_zonelist {
$cb->set_active($selected_index || 0);
$cb->show;
-$box->pack_start($cb, 0, 0, 0);
+$grid->attach($cb, 1, 1, 1, 1);
}
sub create_password_view {
@@ -1911,10 +1911,8 @@ sub create_country_view {
my $locales = $iso_env->{locales};
-my $vbox2 = Gtk3::Box->new('vertical', 0);
-$gtk_state->{inbox}->pack_start($vbox2, 1, 0, 0);
-my $vbox = Gtk3::Box->new('vertical', 0);
-$vbox2->pack_start($vbox, 0, 0, 10);
+my $grid = &$create_basic_grid();
+$gtk_state->{inbox}->pack_start($grid, 0, 0, 0);
my $w = Gtk3::Entry->new();
$w->set_size_request(200, -1);
@@ -1925,18 +1923,16 @@ sub create_country_view {
$c->set_popup_set_width(1);
$c->set_inline_completion(1);
-my $hbox2 = Gtk3::Box->new('horizontal', 0);
my $label = Gtk3::Label->new("Time zone");
$label->set_size_request(150, -1);
$label->set_xalign(1.0);
-$hbox2->pack_start($label, 0, 0, 10);
-update_zonelist ($hbox2);
+$grid->attach($label, 0, 1, 1, 1);
+update_zonelist ($grid);
-my $hbox3 = Gtk3::Box->new('horizontal', 0);
$label = Gtk3::Label->new("Keyboard Layout");
$label->set_size_request(150, -1);
$label->set_xalign(1.0);
-$hbox3->pack_start($label, 0, 0, 10);
+$grid->attach($label, 0, 2, 1, 1);
my $kmapcb = Gtk3::ComboBoxText->new();
$kmapcb->set_size_request (200, -1);
@@ -1945,7 +1941,7 @@ sub create_country_view {
}
update_layout($kmapcb);
-$hbox3->pack_start ($kmapcb, 0, 0, 0);
+$grid->attach($kmapcb, 1, 2, 1, 1);
$kmapcb->signal_connect ('changed' => sub {
my $sel = $kmapcb->get_active_text();
@@ -1982,7 +1978,7 @@ sub create_country_view {
my $text = $entry->get_text;
if (my $cc = $locales->{countryhash}->{lc($text)}) {
- update_zonelist($hbox2, $cc);
+ update_zonelist($grid, $cc);
my $kmap = $locales->{country}->{$cc}->{kmap} || 'en-us';
update_layout($kmapcb, $kmap);
}
@@ -2042,17 +2038,11 @@ sub create_country_view {
$w->set_completion ($c);
-my $hbox = Gtk3::Box->new('horizontal', 0);
-
$label = Gtk3::Label->new("Country");
$label->set_xalign(1.0);
$label->set_size_request(150, -1);
-$hbox->pack_start($label, 0, 0, 10);
-$hbox->pack_start($w, 0, 0, 0);
-
-$vbox->pack_start($hbox, 0, 0, 5);
-$vbox->pack_start($hbox2, 0, 0, 5);
-$vbox->pack_start($hbox3, 0, 0, 5);
+$grid->attach($label, 0, 0, 1, 1);
+$grid->attach($w, 1, 0, 1, 1);
my $country = Proxmox::Install::Config::get_country();
if ($country && (my $entry = $locales->{country}->{$country})) {
--
2.39.2
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH installer 2/7] use basic grid in the network panel
Using boxes causes the labels to not align correctly in certain
circumstances. In the following commits we replace the use of boxes with
grids and set the margins and spacing directly on the respective grid.
Signed-off-by: Maximiliano Sandoval
---
proxinstall | 56 ++---
1 file changed, 27 insertions(+), 29 deletions(-)
diff --git a/proxinstall b/proxinstall
index fbd260b..72802c0 100755
--- a/proxinstall
+++ b/proxinstall
@@ -1440,18 +1440,14 @@ sub check_number {
sub create_text_input {
my ($default, $text) = @_;
-my $hbox = Gtk3::Box->new('horizontal', 0);
-
my $label = Gtk3::Label->new($text);
$label->set_size_request(150, -1);
$label->set_xalign(1.0);
-$hbox->pack_start($label, 0, 0, 10);
my $e1 = Gtk3::Entry->new();
$e1->set_width_chars(35);
-$hbox->pack_start($e1, 0, 0, 0);
$e1->set_text($default);
-return ($hbox, $e1);
+return ($label, $e1);
}
sub create_cidr_inputs {
my ($default_ip, $default_mask) = @_;
@@ -1461,23 +1457,22 @@ sub create_cidr_inputs {
my $label = Gtk3::Label->new('IP Address (CIDR)');
$label->set_size_request(150, -1);
$label->set_xalign(1.0);
-$hbox->pack_start($label, 0, 0, 10);
my $ip_el = Gtk3::Entry->new();
$ip_el->set_width_chars(28);
$hbox->pack_start($ip_el, 0, 0, 0);
$ip_el->set_text($default_ip);
-$label = Gtk3::Label->new('/');
-$label->set_size_request(10, -1);
-$hbox->pack_start($label, 0, 0, 2);
+my $dash_label = Gtk3::Label->new('/');
+$dash_label->set_size_request(10, -1);
+$hbox->pack_start($dash_label, 0, 0, 2);
my $cidr_el = Gtk3::Entry->new();
$cidr_el->set_width_chars(3);
$hbox->pack_start($cidr_el, 0, 0, 0);
$cidr_el->set_text($default_mask);
-return ($hbox, $ip_el, $cidr_el);
+return ($label, $hbox, $ip_el, $cidr_el);
}
my $ipconf_first_view = 1;
@@ -1502,17 +1497,17 @@ sub create_ipconf_view {
cleanup_view();
Proxmox::UI::display_html('ipconf.htm');
-my $vcontainer = Gtk3::Box->new('vertical', 0);
-$gtk_state->{inbox}->pack_start($vcontainer, 1, 0, 0);
-my $hcontainer = Gtk3::Box->new('horizontal', 0);
-$vcontainer->pack_start($hcontainer, 0, 0, 10);
-my $vbox = Gtk3::Box->new('vertical', 0);
-$hcontainer->add($vbox);
+my $grid = &$create_basic_grid();
+$grid->set_row_spacing(10);
+$grid->set_column_spacing(10);
+
+$gtk_state->{inbox}->pack_start($grid, 0, 0, 0);
my $ipaddr_text = $config->{ipaddress} // "192.168.100.2";
my $netmask_text = $config->{netmask} // "24";
my $cidr_box;
-($cidr_box, $ipconf_entry_addr, $ipconf_entry_mask) =
+my $cidr_label;
+($cidr_label, $cidr_box, $ipconf_entry_addr, $ipconf_entry_mask) =
create_cidr_inputs($ipaddr_text, $netmask_text);
my $device_cb = Gtk3::ComboBoxText->new();
@@ -1567,34 +1562,37 @@ sub create_ipconf_view {
my $label = Gtk3::Label->new("Management Interface:");
$label->set_size_request(150, -1);
$label->set_xalign(1.0);
-$devicebox->pack_start($label, 0, 0, 10);
-$devicebox->pack_start($device_cb, 0, 0, 0);
-$vbox->pack_start($devicebox, 0, 0, 2);
+$grid->attach($label, 0, 0, 1, 1);
+$grid->attach($device_cb, 1, 0, 1, 1);
my $fqdn = Proxmox::Install::Config::get_fqdn();
my $hn = $fqdn // "$iso_env->{product}." . ($ipconf->{domain} //
"example.invalid");
-my ($hostbox, $hostentry) = create_text_input($hn, 'Hostname (FQDN):');
-$vbox->pack_start($hostbox, 0, 0, 2);
+my ($hostlabel, $hostentry) = create_text_input($hn, 'Hostname (FQDN):');
+$grid->attach($hostlabel, 0, 1, 1, 1);
+$grid->attach($hostentry, 1, 1, 1, 1);
-$vbox->pack_start($cidr_box, 0, 0, 2);
+$grid->attach($cidr_label, 0, 2, 1, 1);
+$grid->attach($cidr_box, 1, 2, 1, 1);
$gateway = $config->{gateway} // $ipconf->{gateway} || '192.168.100.1';
-my $gwbox;
-($gwbox, $ipconf_entry_gw) =
+my $gwlabel;
+($gwlabel, $ipconf_entry_gw) =
create_text_input($gateway, 'Gateway:');
-$vbox->pack_start($gwbox, 0, 0, 2);
+$grid->attach($gwlabel, 0, 3, 1, 1);
+$grid->attach($ipconf_entry_gw, 1, 3, 1, 1);
$dnsserver = $config->{dnsserver} // $ipconf->{dnsserver} || $gateway;
-my $dnsbox;
-($dnsbox, $ipconf_entry_dns) =
+my $dns_label;
+($dns_label, $ipconf_entry_dns) =
create_text_input($dnsserver, 'DNS Server:');
-$vbox->pack_start($dnsbox, 0, 0, 0);
+$grid->attach($dns_label, 0, 4, 1, 1);
+$grid->attach($ipconf_entry_dns, 1, 4, 1, 1);
$gtk_state->{inbox}->show_all;
set_next(undef, sub {
--
2.39.2
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH installer 3/7] expand ip address Gtk3::Entry
This accounts for the different layout set in the previous commit.
Signed-off-by: Maximiliano Sandoval
---
proxinstall | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/proxinstall b/proxinstall
index 72802c0..7e1bcd4 100755
--- a/proxinstall
+++ b/proxinstall
@@ -1460,7 +1460,7 @@ sub create_cidr_inputs {
my $ip_el = Gtk3::Entry->new();
$ip_el->set_width_chars(28);
-$hbox->pack_start($ip_el, 0, 0, 0);
+$hbox->pack_start($ip_el, 1, 1, 0);
$ip_el->set_text($default_ip);
my $dash_label = Gtk3::Label->new('/');
--
2.39.2
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH installer 7/7] remove trailing spaces and colons from labels
For consistency sake, all colons and trailing spaces in labels were
removed, this matches other panels such as the password and
country/timezone panels.
Signed-off-by: Maximiliano Sandoval
---
proxinstall | 12 ++--
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/proxinstall b/proxinstall
index 86922e2..b2c214b 100755
--- a/proxinstall
+++ b/proxinstall
@@ -1559,7 +1559,7 @@ sub create_ipconf_view {
}
my $devicebox = Gtk3::Box->new('horizontal', 0);
-my $label = Gtk3::Label->new("Management Interface:");
+my $label = Gtk3::Label->new("Management Interface");
$label->set_size_request(150, -1);
$label->set_xalign(1.0);
@@ -1569,7 +1569,7 @@ sub create_ipconf_view {
my $fqdn = Proxmox::Install::Config::get_fqdn();
my $hn = $fqdn // "$iso_env->{product}." . ($ipconf->{domain} //
"example.invalid");
-my ($hostlabel, $hostentry) = create_text_input($hn, 'Hostname (FQDN):');
+my ($hostlabel, $hostentry) = create_text_input($hn, 'Hostname (FQDN)');
$grid->attach($hostlabel, 0, 1, 1, 1);
$grid->attach($hostentry, 1, 1, 1, 1);
@@ -1580,7 +1580,7 @@ sub create_ipconf_view {
my $gwlabel;
($gwlabel, $ipconf_entry_gw) =
- create_text_input($gateway, 'Gateway:');
+ create_text_input($gateway, 'Gateway');
$grid->attach($gwlabel, 0, 3, 1, 1);
$grid->attach($ipconf_entry_gw, 1, 3, 1, 1);
@@ -1589,7 +1589,7 @@ sub create_ipconf_view {
my $dns_label;
($dns_label, $ipconf_entry_dns) =
- create_text_input($dnsserver, 'DNS Server:');
+ create_text_input($dnsserver, 'DNS Server');
$grid->attach($dns_label, 0, 4, 1, 1);
$grid->attach($ipconf_entry_dns, 1, 4, 1, 1);
@@ -2452,7 +2452,7 @@ sub create_hdoption_view {
$target_hd_label->set_text("Target: $filesys ");
$options_stack->set_visible_child_name("raiddisk");
} else {
- $target_hd_label->set_text("Target Harddisk: ");
+ $target_hd_label->set_text("Target Harddisk");
}
if ($raid) {
@@ -2665,7 +2665,7 @@ sub create_hdsel_view {
my ($disk, $devname, $size, $model, $logical_bsize) =
$cached_disks->[0]->@*;
$target_hd = $devname if !defined($target_hd);
-$target_hd_label = Gtk3::Label->new("Target Harddisk: ");
+$target_hd_label = Gtk3::Label->new("Target Harddisk");
$hbox->pack_start($target_hd_label, 0, 0, 0);
$target_hd_combo = Gtk3::ComboBoxText->new();
--
2.39.2
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH installer 4/7] use basic grid in password panel
The extra 10px margin on the email row was added to account for the
removed line:
$vbox->pack_start($hbox3, 0, 0, 15);
Signed-off-by: Maximiliano Sandoval
---
proxinstall | 32 ++--
1 file changed, 14 insertions(+), 18 deletions(-)
diff --git a/proxinstall b/proxinstall
index 7e1bcd4..faebdec 100755
--- a/proxinstall
+++ b/proxinstall
@@ -1821,47 +1821,43 @@ sub create_password_view {
my $password = Proxmox::Install::Config::get_password();
-my $vbox2 = Gtk3::Box->new('vertical', 0);
-$gtk_state->{inbox}->pack_start($vbox2, 1, 0, 0);
-my $vbox = Gtk3::Box->new('vertical', 0);
-$vbox2->pack_start($vbox, 0, 0, 10);
+my $grid = &$create_basic_grid();
+$gtk_state->{inbox}->pack_start($grid, 0, 0, 0);
-my $hbox1 = Gtk3::Box->new('horizontal', 0);
my $label = Gtk3::Label->new("Password");
$label->set_size_request(150, -1);
$label->set_xalign(1.0);
-$hbox1->pack_start($label, 0, 0, 10);
+$grid->attach($label, 0, 0, 1, 1);
my $pwe1 = Gtk3::Entry->new();
$pwe1->set_visibility(0);
$pwe1->set_text($password) if $password;
$pwe1->set_size_request(200, -1);
-$hbox1->pack_start($pwe1, 0, 0, 0);
+$grid->attach($pwe1, 1, 0, 1, 1);
-my $hbox2 = Gtk3::Box->new('horizontal', 0);
$label = Gtk3::Label->new("Confirm");
$label->set_size_request(150, -1);
$label->set_xalign(1.0);
-$hbox2->pack_start($label, 0, 0, 10);
+$grid->attach($label, 0, 1, 1, 1);
my $pwe2 = Gtk3::Entry->new();
$pwe2->set_visibility(0);
$pwe2->set_text($password) if $password;
$pwe2->set_size_request(200, -1);
-$hbox2->pack_start($pwe2, 0, 0, 0);
+$grid->attach($pwe2, 1, 1, 1, 1);
-my $hbox3 = Gtk3::Box->new('horizontal', 0);
$label = Gtk3::Label->new("Email");
$label->set_size_request(150, -1);
$label->set_xalign(1.0);
-$hbox3->pack_start($label, 0, 0, 10);
+$label->set_margin_top(10);
+$grid->attach($label, 0, 2, 1, 1);
+
+my $mailto = Proxmox::Install::Config::get_mailto();
+
my $eme = Gtk3::Entry->new();
$eme->set_size_request(200, -1);
-$eme->set_text(Proxmox::Install::Config::get_mailto());
-$hbox3->pack_start($eme, 0, 0, 0);
-
+$eme->set_text($mailto);
+$eme->set_margin_top(10);
-$vbox->pack_start($hbox1, 0, 0, 5);
-$vbox->pack_start($hbox2, 0, 0, 5);
-$vbox->pack_start($hbox3, 0, 0, 15);
+$grid->attach($eme, 1, 2, 1, 1);
$gtk_state->{inbox}->show_all;
--
2.39.2
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH installer] make harddisk options dialog modal
Modal dialogs block the user from interacting with the main window.
We already do this with message dialogs so for consistency sake, we set
the harddisk options dialog as modal. Note that one can see weird
behaviors when interacting with the window behind the dialog in certain
scenarios.
Signed-off-by: Maximiliano Sandoval
---
proxinstall | 2 ++
1 file changed, 2 insertions(+)
diff --git a/proxinstall b/proxinstall
index fe0239f..f32abd2 100755
--- a/proxinstall
+++ b/proxinstall
@@ -2291,6 +2291,8 @@ sub create_hdoption_view {
my $dialog = Gtk3::Dialog->new();
$dialog->set_title("Harddisk options");
+$dialog->set_modal(1);
+$dialog->set_transient_for($window);
$dialog->add_button("_OK", 1);
--
2.39.2
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [RFC PATCH manager 4/4] ui: pci mapping: rework mapping panel for better user experience
I like the approach as it cleans up the overloaded tbar that has items that are only valid in certain contexts. Two small nits from a UX POV: - double clicking any PCI device should open the edit dialog for the node, similar to double clicking the node itself - the Action Column should probably be further left and not on the far right side by default. I personally like it to be the second column from the left as all other columns are rather informal. I know it is kinda late, but would it be hard to add the "Device" column from the PCI device selection grid to the overview as well? This way one can easily verify that they got the right devices by name. But probably it is a bit harder to gather the info from the other nodes? On 6/19/23 16:13, Dominik Csapak wrote: by removing the confusing buttons in the toolbar and adding them as actions in an actioncolumn. There a only relevant actions are visible and get a more expressive tooltip with this, we now differentiate between 4 modes of the edit window: * create a new mapping altogether - shows all fields * edit existing mapping on top level - show only 'global' fields (comment+mdev), so no mappings * add new host mapping - shows nodeselector, mapping and mdev, but mdev is disabled (informational only) * edit existing host mapping - show selected node (displayfield) mdev and mappings, but only mappings are editable we have to split the nodeselector into two fields, since the disabling cbind does not pass through to the editconfig (and thus makes the form invalid if we try that) Signed-off-by: Dominik Csapak --- this is not intended to be applied as is, rather i'd like some feedback on the approach (@thomas, @aaron ?) so that if we want to do it this way i can also do it for the usb mappings the other approach mentioned off-list can still be done (having a full grid with all mappings regardless of the node) maybe only for usb devices (there it makes imho more sense) but then we'd have two interfaces for the mappings instead of one ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] applied: [PATCH v2 pve-docs] qemu: add cpu models documentation
Am 19.06.23 um 10:38 schrieb Alexandre Derumier: > add doc for differents cpu models including > new x86-64-vX models > > changelog v2: x86-64-v2 is compatible with >= opteron_g3 > > Signed-off-by: Alexandre Derumier applied, thanks! Made some follow-ups for typo/style/consistency fixes and moved the list of AMD and Intel CPU types to the end of the chapter to avoid cluttering the CPU types section. ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [RFC PATCH manager 4/4] ui: pci mapping: rework mapping panel for better user experience
On 6/20/23 11:35, Aaron Lauterer wrote: I like the approach as it cleans up the overloaded tbar that has items that are only valid in certain contexts. Two small nits from a UX POV: - double clicking any PCI device should open the edit dialog for the node, similar to double clicking the node itself makes sense imo - the Action Column should probably be further left and not on the far right side by default. I personally like it to be the second column from the left as all other columns are rather informal. mhmm can do that, but how i refactored that seems to be a bit hacky to inject an actioncolumn at a certain position, but technically not a problem I know it is kinda late, but would it be hard to add the "Device" column from the PCI device selection grid to the overview as well? This way one can easily verify that they got the right devices by name. But probably it is a bit harder to gather the info from the other nodes? we already query the pci list of each node, so we could extract that from there but this only works if the user has Sys.Audit which may not be the case. then the column would be empty On 6/19/23 16:13, Dominik Csapak wrote: by removing the confusing buttons in the toolbar and adding them as actions in an actioncolumn. There a only relevant actions are visible and get a more expressive tooltip with this, we now differentiate between 4 modes of the edit window: * create a new mapping altogether - shows all fields * edit existing mapping on top level - show only 'global' fields (comment+mdev), so no mappings * add new host mapping - shows nodeselector, mapping and mdev, but mdev is disabled (informational only) * edit existing host mapping - show selected node (displayfield) mdev and mappings, but only mappings are editable we have to split the nodeselector into two fields, since the disabling cbind does not pass through to the editconfig (and thus makes the form invalid if we try that) Signed-off-by: Dominik Csapak --- this is not intended to be applied as is, rather i'd like some feedback on the approach (@thomas, @aaron ?) so that if we want to do it this way i can also do it for the usb mappings the other approach mentioned off-list can still be done (having a full grid with all mappings regardless of the node) maybe only for usb devices (there it makes imho more sense) but then we'd have two interfaces for the mappings instead of one ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [RFC PATCH manager 4/4] ui: pci mapping: rework mapping panel for better user experience
On 6/20/23 11:57, Dominik Csapak wrote: On 6/20/23 11:35, Aaron Lauterer wrote: I like the approach as it cleans up the overloaded tbar that has items that are only valid in certain contexts. Two small nits from a UX POV: - double clicking any PCI device should open the edit dialog for the node, similar to double clicking the node itself makes sense imo - the Action Column should probably be further left and not on the far right side by default. I personally like it to be the second column from the left as all other columns are rather informal. mhmm can do that, but how i refactored that seems to be a bit hacky to inject an actioncolumn at a certain position, but technically not a problem But aren't we doing that already in the content view of PBS? AFAIK it is the 3rd column there. I know it is kinda late, but would it be hard to add the "Device" column from the PCI device selection grid to the overview as well? This way one can easily verify that they got the right devices by name. But probably it is a bit harder to gather the info from the other nodes? we already query the pci list of each node, so we could extract that from there but this only works if the user has Sys.Audit which may not be the case. then the column would be empty On 6/19/23 16:13, Dominik Csapak wrote: by removing the confusing buttons in the toolbar and adding them as actions in an actioncolumn. There a only relevant actions are visible and get a more expressive tooltip with this, we now differentiate between 4 modes of the edit window: * create a new mapping altogether - shows all fields * edit existing mapping on top level - show only 'global' fields (comment+mdev), so no mappings * add new host mapping - shows nodeselector, mapping and mdev, but mdev is disabled (informational only) * edit existing host mapping - show selected node (displayfield) mdev and mappings, but only mappings are editable we have to split the nodeselector into two fields, since the disabling cbind does not pass through to the editconfig (and thus makes the form invalid if we try that) Signed-off-by: Dominik Csapak --- this is not intended to be applied as is, rather i'd like some feedback on the approach (@thomas, @aaron ?) so that if we want to do it this way i can also do it for the usb mappings the other approach mentioned off-list can still be done (having a full grid with all mappings regardless of the node) maybe only for usb devices (there it makes imho more sense) but then we'd have two interfaces for the mappings instead of one ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [RFC PATCH manager 4/4] ui: pci mapping: rework mapping panel for better user experience
On 6/20/23 12:18, Aaron Lauterer wrote: On 6/20/23 11:57, Dominik Csapak wrote: On 6/20/23 11:35, Aaron Lauterer wrote: I like the approach as it cleans up the overloaded tbar that has items that are only valid in certain contexts. Two small nits from a UX POV: - double clicking any PCI device should open the edit dialog for the node, similar to double clicking the node itself makes sense imo - the Action Column should probably be further left and not on the far right side by default. I personally like it to be the second column from the left as all other columns are rather informal. mhmm can do that, but how i refactored that seems to be a bit hacky to inject an actioncolumn at a certain position, but technically not a problem But aren't we doing that already in the content view of PBS? AFAIK it is the 3rd column there. sorry i didn't describe it properly. ofc it's possible to have the actioncolumn at any position it's just that the way i have structured the code makes it slightly weird to do (the columns are defined in the subclasses, but the actioncolumn would be injected in the base class, so i'd have to split the columns array before inserting it) ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [RFC v2 ha-manager 2/7] lrm service: move PID file to service's runtime directory
it's arguably cleaner to put it there, now that there is one.
Signed-off-by: Fiona Ebner
---
No changes in v2.
debian/pve-ha-lrm.service | 2 +-
src/PVE/Service/pve_ha_lrm.pm | 5 -
2 files changed, 5 insertions(+), 2 deletions(-)
diff --git a/debian/pve-ha-lrm.service b/debian/pve-ha-lrm.service
index 9f3f73d..d9fcf87 100644
--- a/debian/pve-ha-lrm.service
+++ b/debian/pve-ha-lrm.service
@@ -21,7 +21,7 @@ After=watchdog-mux.service
[Service]
ExecStart=/usr/sbin/pve-ha-lrm start
ExecStop=/usr/sbin/pve-ha-lrm stop
-PIDFile=/run/pve-ha-lrm.pid
+PIDFile=/run/pve-ha-lrm/pve-ha-lrm.pid
RuntimeDirectory=pve-ha-lrm
TimeoutStopSec=infinity
KillMode=process
diff --git a/src/PVE/Service/pve_ha_lrm.pm b/src/PVE/Service/pve_ha_lrm.pm
index 91a9409..0790215 100644
--- a/src/PVE/Service/pve_ha_lrm.pm
+++ b/src/PVE/Service/pve_ha_lrm.pm
@@ -13,7 +13,10 @@ use base qw(PVE::Daemon);
my $cmdline = [$0, @ARGV];
-my %daemon_options = (stop_wait_time => 60*60);
+my %daemon_options = (
+stop_wait_time => 60*60,
+pidfile => '/run/pve-ha-lrm/pve-ha-lrm.pid',
+);
my $daemon = __PACKAGE__->new('pve-ha-lrm', $cmdline, %daemon_options);
--
2.39.2
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH v2 ha-manager 1/7] lrm service: add runtime directory
in preparation to allow overriding the shutdown policy with a file there. Signed-off-by: Fiona Ebner --- No changes in v2. debian/pve-ha-lrm.service | 1 + 1 file changed, 1 insertion(+) diff --git a/debian/pve-ha-lrm.service b/debian/pve-ha-lrm.service index fb1b5db..9f3f73d 100644 --- a/debian/pve-ha-lrm.service +++ b/debian/pve-ha-lrm.service @@ -22,6 +22,7 @@ After=watchdog-mux.service ExecStart=/usr/sbin/pve-ha-lrm start ExecStop=/usr/sbin/pve-ha-lrm stop PIDFile=/run/pve-ha-lrm.pid +RuntimeDirectory=pve-ha-lrm TimeoutStopSec=infinity KillMode=process Type=forking -- 2.39.2 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH v2 ha-manager 3/7] pve2 env: get shutdown policy override set by node's shutdown API endpoint
Signed-off-by: Fiona Ebner
---
No changes in v2.
src/PVE/HA/Env/PVE2.pm | 8
1 file changed, 8 insertions(+)
diff --git a/src/PVE/HA/Env/PVE2.pm b/src/PVE/HA/Env/PVE2.pm
index f6ebfeb..6a75bd4 100644
--- a/src/PVE/HA/Env/PVE2.pm
+++ b/src/PVE/HA/Env/PVE2.pm
@@ -458,9 +458,17 @@ sub get_datacenter_settings {
my $datacenterconfig = eval { cfs_read_file('datacenter.cfg') };
$self->log('err', "unable to get HA settings from datacenter.cfg - $@") if
$@;
+my $sp_override_fn = '/run/pve-ha-lrm/shutdown-policy.local-override';
+my $shutdown_policy_override = eval {
PVE::Tools::file_read_firstline($sp_override_fn); };
+$self->log('warning', "error reading shutdown policy override from
$sp_override_fn - $@") if $@;
+
+my $overrides = {};
+$overrides->{ha}->{shutdown_policy} = $shutdown_policy_override if
$shutdown_policy_override;
+
return {
ha => $datacenterconfig->{ha} // {},
crs => $datacenterconfig->{crs} // {},
+ 'local-overrides' => $overrides,
};
}
--
2.39.2
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [RFC v2 ha-manager 5/7] pve2 env: validate shutdown policy from override file
for future-proofing.
Signed-off-by: Fiona Ebner
---
Not sure if this is worth it.
(Build-)dependency bump for libpve-cluster-perl needed
No changes in v2.
src/PVE/HA/Env/PVE2.pm | 13 -
1 file changed, 12 insertions(+), 1 deletion(-)
diff --git a/src/PVE/HA/Env/PVE2.pm b/src/PVE/HA/Env/PVE2.pm
index 6a75bd4..6eb4aa7 100644
--- a/src/PVE/HA/Env/PVE2.pm
+++ b/src/PVE/HA/Env/PVE2.pm
@@ -12,6 +12,7 @@ use PVE::Tools;
use PVE::Cluster qw(cfs_register_file cfs_read_file cfs_write_file
cfs_lock_file);
use PVE::DataCenterConfig;
use PVE::INotify;
+use PVE::JSONSchema;
use PVE::RPCEnvironment;
use PVE::HA::Tools ':exit_codes';
@@ -463,7 +464,17 @@ sub get_datacenter_settings {
$self->log('warning', "error reading shutdown policy override from
$sp_override_fn - $@") if $@;
my $overrides = {};
-$overrides->{ha}->{shutdown_policy} = $shutdown_policy_override if
$shutdown_policy_override;
+
+if ($shutdown_policy_override) {
+ eval {
+ PVE::JSONSchema::validate(
+ $shutdown_policy_override,
+ PVE::JSONSchema::get_standard_option('pve-ha-shutdown-policy'),
+ );
+ $overrides->{ha}->{shutdown_policy} = $shutdown_policy_override;
+ };
+ $self->log('warning', "error validating shutdown policy override - $@")
if $@;
+}
return {
ha => $datacenterconfig->{ha} // {},
--
2.39.2
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH v2 ha-manager 4/7] lrm: honor shutdown policy override set by node's shutdown API endpoint
The /nodes//status API endpoint in pve-manager allows to specify
the shutdown policy now. This change is required for it to have any
effect of course.
Being able to choose the shutdown policy on a per-node/per-shutdown
is more flexible and user-friendly.
Signed-off-by: Fiona Ebner
---
No changes in v2.
src/PVE/HA/LRM.pm | 5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/src/PVE/HA/LRM.pm b/src/PVE/HA/LRM.pm
index b6ac0fe..b8b842f 100644
--- a/src/PVE/HA/LRM.pm
+++ b/src/PVE/HA/LRM.pm
@@ -60,7 +60,10 @@ sub shutdown_request {
my ($shutdown, $reboot) = $haenv->is_node_shutdown();
my $dc_cfg = $haenv->get_datacenter_settings();
-my $shutdown_policy = $dc_cfg->{ha}->{shutdown_policy} // 'conditional';
+my $dc_ha_overrides = $dc_cfg->{'local-overrides'}->{ha} // {};
+
+my $shutdown_policy =
+ $dc_ha_overrides->{shutdown_policy} // $dc_cfg->{ha}->{shutdown_policy}
// 'conditional';
if ($shutdown) { # don't log this on service restart, only on node shutdown
$haenv->log('info', "got shutdown request with shutdown policy
'$shutdown_policy'");
--
2.39.2
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH v2 manager 1/2] api: nodes: allow setting HA shutdown policy during shutdown/reboot
Increases flexibility/user-friendliness.
In the edge case that an override is already present, but the user
doesn't have Sys.Modify privilege, just proceed with the existing
override. Could in principle happen when the requests from a
privileged user with a policy and an unprivileged user without a
policy arrive at the same time.
Suggested-by: Thomas Lamprecht
Signed-off-by: Fiona Ebner
---
(Build-)dependency bump for libpve-cluster-perl needed.
Changes in v2:
* Also check for Sys.Modify privilege when parameter is specified.
PVE/API2/Nodes.pm | 18 +-
1 file changed, 17 insertions(+), 1 deletion(-)
diff --git a/PVE/API2/Nodes.pm b/PVE/API2/Nodes.pm
index 9269694d..b8f0c6ce 100644
--- a/PVE/API2/Nodes.pm
+++ b/PVE/API2/Nodes.pm
@@ -8,7 +8,7 @@ use Digest::SHA;
use Filesys::Df;
use HTTP::Status qw(:constants);
use JSON;
-use POSIX qw(LONG_MAX);
+use POSIX qw(ENOENT LONG_MAX);
use Time::Local qw(timegm_nocheck);
use Socket;
use IO::Socket::SSL;
@@ -544,6 +544,7 @@ __PACKAGE__->register_method({
method => 'POST',
permissions => {
check => ['perm', '/nodes/{node}', [ 'Sys.PowerMgmt' ]],
+ description => "The 'shutdown-policy' parameter additionally requires
'Sys.Modify'.",
},
protected => 1,
description => "Reboot or shutdown a node.",
@@ -557,12 +558,27 @@ __PACKAGE__->register_method({
type => 'string',
enum => [qw(reboot shutdown)],
},
+ 'shutdown-policy' => get_standard_option('pve-ha-shutdown-policy',
{ optional => 1 }),
},
},
returns => { type => "null" },
code => sub {
my ($param) = @_;
+ my $rpcenv = PVE::RPCEnvironment::get();
+ my $user = $rpcenv->get_user();
+ my $node = $param->{node};
+
+ my $sp_override_fn = '/run/pve-ha-lrm/shutdown-policy.local-override';
+
+ if ($param->{'shutdown-policy'}) {
+ $rpcenv->check($user, "/nodes/$node", ['Sys.Modify']);
+ eval { PVE::Tools::file_set_contents($sp_override_fn,
$param->{'shutdown-policy'}); };
+ die "could not write shutdown policy override to $sp_override_fn -
$@" if $@;
+ } elsif (-e $sp_override_fn && $rpcenv->check($user, "/nodes/$node",
['Sys.Modify'], 1)) {
+ unlink $sp_override_fn or die "unable to remove $sp_override_fn -
$!";
+ }
+
if ($param->{command} eq 'reboot') {
system ("(sleep 2;/sbin/reboot)&");
} elsif ($param->{command} eq 'shutdown') {
--
2.39.2
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH v2 ha-manager 6/7] sim env: add support for datacenter config overrides
to model the recent change in the pve2 environment.
Signed-off-by: Fiona Ebner
---
No changes in v2.
src/PVE/HA/Sim/Env.pm | 1 +
1 file changed, 1 insertion(+)
diff --git a/src/PVE/HA/Sim/Env.pm b/src/PVE/HA/Sim/Env.pm
index c6ea73c..1b8399a 100644
--- a/src/PVE/HA/Sim/Env.pm
+++ b/src/PVE/HA/Sim/Env.pm
@@ -433,6 +433,7 @@ sub get_datacenter_settings {
return {
ha => $datacenterconfig->{ha} // {},
crs => $datacenterconfig->{crs} // {},
+ 'local-overrides' => $datacenterconfig->{'local-overrides'} // {},
};
}
--
2.39.2
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH v2 manager 2/2] ui: allow specifying shutdown policy during node shutdown/reboot
Signed-off-by: Fiona Ebner
---
New in v2, was sent separately before (no changes to that).
Used a new window, because it's small and couldn't find a good
fit with the existing ones. Maybe SafeDestroy, but not in name and
would require a few modifications too.
Not sure about using gettext() for the option names either.
www/manager6/Makefile | 1 +
www/manager6/node/Config.js | 32 +++
www/manager6/window/NodeShutdown.js | 126
3 files changed, 139 insertions(+), 20 deletions(-)
create mode 100644 www/manager6/window/NodeShutdown.js
diff --git a/www/manager6/Makefile b/www/manager6/Makefile
index 9b6dd13b..94c8c05e 100644
--- a/www/manager6/Makefile
+++ b/www/manager6/Makefile
@@ -107,6 +107,7 @@ JSSRC=
\
window/FirewallLograteEdit.js \
window/LoginWindow.js \
window/Migrate.js \
+ window/NodeShutdown.js \
window/Prune.js \
window/Restore.js \
window/SafeDestroyGuest.js \
diff --git a/www/manager6/node/Config.js b/www/manager6/node/Config.js
index 6ed2172a..7a074879 100644
--- a/www/manager6/node/Config.js
+++ b/www/manager6/node/Config.js
@@ -19,18 +19,6 @@ Ext.define('PVE.node.Config', {
interval: 5000,
});
- var node_command = function(cmd) {
- Proxmox.Utils.API2Request({
- params: { command: cmd },
- url: '/nodes/' + nodename + '/status',
- method: 'POST',
- waitMsgTarget: me,
- failure: function(response, opts) {
- Ext.Msg.alert(gettext('Error'), response.htmlStatus);
- },
- });
- };
-
var actionBtn = Ext.create('Ext.Button', {
text: gettext('Bulk Actions'),
iconCls: 'fa fa-fw fa-ellipsis-v',
@@ -83,24 +71,28 @@ Ext.define('PVE.node.Config', {
}),
});
- let restartBtn = Ext.create('Proxmox.button.Button', {
+ let restartBtn = Ext.create('Ext.button.Button', {
text: gettext('Reboot'),
disabled: !caps.nodes['Sys.PowerMgmt'],
- dangerous: true,
- confirmMsg: Ext.String.format(gettext("Reboot node '{0}'?"),
nodename),
handler: function() {
- node_command('reboot');
+ Ext.create('PVE.window.NodeShutdown', {
+ confirmMsg: Ext.String.format(gettext("Reboot node
'{0}'?"), nodename),
+ url: '/nodes/' + nodename + '/status',
+ command: 'reboot',
+ }).show();
},
iconCls: 'fa fa-undo',
});
- var shutdownBtn = Ext.create('Proxmox.button.Button', {
+ let shutdownBtn = Ext.create('Ext.button.Button', {
text: gettext('Shutdown'),
disabled: !caps.nodes['Sys.PowerMgmt'],
- dangerous: true,
- confirmMsg: Ext.String.format(gettext("Shutdown node '{0}'?"),
nodename),
handler: function() {
- node_command('shutdown');
+ Ext.create('PVE.window.NodeShutdown', {
+ confirmMsg: Ext.String.format(gettext("Shutdown node
'{0}'?"), nodename),
+ url: '/nodes/' + nodename + '/status',
+ command: 'shutdown',
+ }).show();
},
iconCls: 'fa fa-power-off',
});
diff --git a/www/manager6/window/NodeShutdown.js
b/www/manager6/window/NodeShutdown.js
new file mode 100644
index ..4cdc541c
--- /dev/null
+++ b/www/manager6/window/NodeShutdown.js
@@ -0,0 +1,126 @@
+Ext.define('PVE.window.NodeShutdown', {
+extend: 'Ext.window.Window',
+alias: 'widget.pveNodeShutdown',
+mixins: ['Proxmox.Mixin.CBind'],
+
+title: gettext('Confirm'),
+modal: true,
+buttonAlign: 'center',
+bodyPadding: 10,
+width: 450,
+layout: { type: 'hbox' },
+defaultFocus: 'nodeShutdownNoButton',
+
+config: {
+ url: undefined,
+ confirmMsg: undefined,
+ command: undefined,
+},
+
+getParams: function() {
+ let me = this;
+ let params = { command: me.getCommand(), };
+
+ let shutdownPolicy = me.lookup('shutdownPolicy').getValue();
+ if (shutdownPolicy && shutdownPolicy !== '__default__') {
+ params['shutdown-policy'] = shutdownPolicy;
+ }
+
+ return params;
+},
+
+controller: {
+ xclass: 'Ext.app.ViewController',
+
+ control: {
+ 'button[reference=yesButton]': {
+ click: function() {
+ const view = this.getView();
+ Proxmox.Utils.API2Request({
+ params: view.getParams(),
+ url: view.getUrl(),
+
[pve-devel] [PATCH-SERIES v2 (ha-)manager] allow node HA shutdown policy override
Make it possible to specify the HA shutdown policy for the
/nodes/{node}/status POST API enpoint for user flexibilty and
convenience.
The override is written to the LRM service's (new dedicated) runtime
directory. The LRM will check and honor the override when it receives
a shutdown/reboot request.
(Build-)depedency bump pve-manager -> libpve-cluster-perl needed.
If patch ha-manager 5/7 is applied: (build-)depedency bump
pve-ha-manager -> libpve-cluster-perl needed.
Dependency bump pve-manager -> pve-ha-manager needed (to have the
runtime directory exist and LRM honor the new param).
Changes in v2:
* Also check for Sys.Modify permisson when shutdown policy
parameter is used.
* Add UI patch.
* Drop already applied cluster patch.
ha-manager:
Fiona Ebner (7):
lrm service: add runtime directory
lrm service: move PID file to service's runtime directory
pve2 env: get shutdown policy override set by node's shutdown API
endpoint
lrm: honor shutdown policy override set by node's shutdown API
endpoint
pve2 env: validate shutdown policy from override file
sim env: add support for datacenter config overrides
tests: add test for shutdown policy override
debian/pve-ha-lrm.service | 3 +-
src/PVE/HA/Env/PVE2.pm| 19 +++
src/PVE/HA/LRM.pm | 5 +-
src/PVE/HA/Sim/Env.pm | 1 +
src/PVE/Service/pve_ha_lrm.pm | 5 +-
src/test/test-shutdown-policy-override/README | 3 +
.../test-shutdown-policy-override/cmdlist | 4 ++
.../datacenter.cfg| 10
.../hardware_status | 5 ++
.../test-shutdown-policy-override/log.expect | 57 +++
.../manager_status| 1 +
.../service_config| 6 ++
12 files changed, 116 insertions(+), 3 deletions(-)
create mode 100644 src/test/test-shutdown-policy-override/README
create mode 100644 src/test/test-shutdown-policy-override/cmdlist
create mode 100644 src/test/test-shutdown-policy-override/datacenter.cfg
create mode 100644 src/test/test-shutdown-policy-override/hardware_status
create mode 100644 src/test/test-shutdown-policy-override/log.expect
create mode 100644 src/test/test-shutdown-policy-override/manager_status
create mode 100644 src/test/test-shutdown-policy-override/service_config
manager:
Fiona Ebner (2):
api: nodes: allow setting HA shutdown policy during shutdown/reboot
ui: allow specifying shutdown policy during node shutdown/reboot
PVE/API2/Nodes.pm | 18 +++-
www/manager6/Makefile | 1 +
www/manager6/node/Config.js | 32 +++
www/manager6/window/NodeShutdown.js | 126
4 files changed, 156 insertions(+), 21 deletions(-)
create mode 100644 www/manager6/window/NodeShutdown.js
--
2.39.2
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH v2 ha-manager 7/7] tests: add test for shutdown policy override
Signed-off-by: Fiona Ebner
---
No changes in v2.
src/test/test-shutdown-policy-override/README | 3 +
.../test-shutdown-policy-override/cmdlist | 4 ++
.../datacenter.cfg| 10
.../hardware_status | 5 ++
.../test-shutdown-policy-override/log.expect | 57 +++
.../manager_status| 1 +
.../service_config| 6 ++
7 files changed, 86 insertions(+)
create mode 100644 src/test/test-shutdown-policy-override/README
create mode 100644 src/test/test-shutdown-policy-override/cmdlist
create mode 100644 src/test/test-shutdown-policy-override/datacenter.cfg
create mode 100644 src/test/test-shutdown-policy-override/hardware_status
create mode 100644 src/test/test-shutdown-policy-override/log.expect
create mode 100644 src/test/test-shutdown-policy-override/manager_status
create mode 100644 src/test/test-shutdown-policy-override/service_config
diff --git a/src/test/test-shutdown-policy-override/README
b/src/test/test-shutdown-policy-override/README
new file mode 100644
index 000..b9c71ce
--- /dev/null
+++ b/src/test/test-shutdown-policy-override/README
@@ -0,0 +1,3 @@
+Test shutdown policy override.
+
+Expect that the policy from the override is used.
diff --git a/src/test/test-shutdown-policy-override/cmdlist
b/src/test/test-shutdown-policy-override/cmdlist
new file mode 100644
index 000..a86b9e2
--- /dev/null
+++ b/src/test/test-shutdown-policy-override/cmdlist
@@ -0,0 +1,4 @@
+[
+[ "power node1 on", "power node2 on", "power node3 on"],
+[ "shutdown node3" ]
+]
diff --git a/src/test/test-shutdown-policy-override/datacenter.cfg
b/src/test/test-shutdown-policy-override/datacenter.cfg
new file mode 100644
index 000..0a188bd
--- /dev/null
+++ b/src/test/test-shutdown-policy-override/datacenter.cfg
@@ -0,0 +1,10 @@
+{
+"ha": {
+"shutdown_policy": "migrate"
+},
+"local-overrides": {
+"ha": {
+"shutdown_policy": "freeze"
+}
+}
+}
diff --git a/src/test/test-shutdown-policy-override/hardware_status
b/src/test/test-shutdown-policy-override/hardware_status
new file mode 100644
index 000..451beb1
--- /dev/null
+++ b/src/test/test-shutdown-policy-override/hardware_status
@@ -0,0 +1,5 @@
+{
+ "node1": { "power": "off", "network": "off" },
+ "node2": { "power": "off", "network": "off" },
+ "node3": { "power": "off", "network": "off" }
+}
diff --git a/src/test/test-shutdown-policy-override/log.expect
b/src/test/test-shutdown-policy-override/log.expect
new file mode 100644
index 000..4aecc4f
--- /dev/null
+++ b/src/test/test-shutdown-policy-override/log.expect
@@ -0,0 +1,57 @@
+info 0 hardware: starting simulation
+info 20 cmdlist: execute power node1 on
+info 20node1/crm: status change startup => wait_for_quorum
+info 20node1/lrm: status change startup => wait_for_agent_lock
+info 20 cmdlist: execute power node2 on
+info 20node2/crm: status change startup => wait_for_quorum
+info 20node2/lrm: status change startup => wait_for_agent_lock
+info 20 cmdlist: execute power node3 on
+info 20node3/crm: status change startup => wait_for_quorum
+info 20node3/lrm: status change startup => wait_for_agent_lock
+info 20node1/crm: got lock 'ha_manager_lock'
+info 20node1/crm: status change wait_for_quorum => master
+info 20node1/crm: node 'node1': state changed from 'unknown' =>
'online'
+info 20node1/crm: node 'node2': state changed from 'unknown' =>
'online'
+info 20node1/crm: node 'node3': state changed from 'unknown' =>
'online'
+info 20node1/crm: adding new service 'vm:103' on node 'node3'
+info 20node1/crm: adding new service 'vm:104' on node 'node3'
+info 20node1/crm: adding new service 'vm:105' on node 'node3'
+info 20node1/crm: adding new service 'vm:106' on node 'node3'
+info 20node1/crm: service 'vm:103': state changed from 'request_start'
to 'started' (node = node3)
+info 20node1/crm: service 'vm:104': state changed from 'request_start'
to 'started' (node = node3)
+info 20node1/crm: service 'vm:105': state changed from 'request_start'
to 'started' (node = node3)
+info 20node1/crm: service 'vm:106': state changed from 'request_start'
to 'started' (node = node3)
+info 22node2/crm: status change wait_for_quorum => slave
+info 24node3/crm: status change wait_for_quorum => slave
+info 25node3/lrm: got lock 'ha_agent_node3_lock'
+info 25node3/lrm: status change wait_for_agent_lock => active
+info 25node3/lrm: starting service vm:103
+info 25node3/lrm: service status vm:103 started
+info 25node3/lrm: starting service vm:104
+info 25node3/lrm: service status vm:104 started
+info 25node3/lrm: starting service vm:105
+info 25node3/lrm: serv
[pve-devel] [PATCH pve-docs v7 0/1] update the PCI(e) docs
This is a followup of Dominiks review: https://lists.proxmox.com/pipermail/pve-devel/2023-June/057295.html With the public wiki having been updated, I fixed the links. Noel Ullreich (1): update the PCI(e) docs qm-pci-passthrough.adoc | 155 +++- qm.adoc | 2 +- system-booting.adoc | 8 +++ 3 files changed, 130 insertions(+), 35 deletions(-) -- 2.30.2 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH pve-docs v7 1/1] update the PCI(e) docs
A little update to the PCI(e) docs with the plan of reworking the PCI
wiki as well.
Along some minor grammar fixes added:
* how to check if kernelmodules are being loaded
* how to check which drivers to blacklist
* how to add softdeps for module loading
* where to find kernel params
Signed-off-by: Noel Ullreich
changes from v1:
* fixed spelling mistakes
* reduced code snippets of how to check iommu groupings to one
* moved where to find kernel params to kernel cmdline section
* removed wrong info on display output. will add correct info to
Examples-Wiki
* changed module names to variable-names, so that people can't
blindly copy-paste.
* restructured commit message ;)
changes from v2:
* while moving where to find the kernel params to the kernel
cmdline section, I forgot to remove it from the pci(e) section
* fixed typo in the link to the kernel param section
changes from v3:
* Some restructuring of the layout as well as moving parts of the
PCI examples wiki to the docs here. This should lead to well-
structured, concise docs that are independent from the PCI wiki.
* found some more minor grammar errors
* found a spelling mistake in qm.adoc
changes from v4:
* formatted the git message wrong again :/
changes from v5:
* fixed links to wiki
* moved where to find kernel params to end of its chapter
* the `vfio_virqfd` does not need to be loaded anymore with kernel 6.2
changes from v6: the public wiki was updated -> fixed the links
Signed-off-by: Noel Ullreich
---
qm-pci-passthrough.adoc | 155 +++-
qm.adoc | 2 +-
system-booting.adoc | 8 +++
3 files changed, 130 insertions(+), 35 deletions(-)
diff --git a/qm-pci-passthrough.adoc b/qm-pci-passthrough.adoc
index df6cf21..204bd4f 100644
--- a/qm-pci-passthrough.adoc
+++ b/qm-pci-passthrough.adoc
@@ -13,19 +13,27 @@ features (e.g., offloading).
But, if you pass through a device to a virtual machine, you cannot use that
device anymore on the host or in any other VM.
+Note that, while PCI passthrough is available for i440fx and q35 machines, PCIe
+passthrough is only available on q35 machines. This does not mean that
+PCIe capable devices that are passed through as PCI devices will only run at
+PCI speeds. Passing through devices as PCIe just sets a flag for the guest to
+tell it that the device is a PCIe device instead of a "really fast legacy PCI
+device". Some guest applications benefit from this.
+
General Requirements
-Since passthrough is a feature which also needs hardware support, there are
-some requirements to check and preparations to be done to make it work.
-
+Since passthrough is performed on real hardware, it needs to fulfill some
+requirements. A brief overview of these requirements is given below, for more
+information on specific devices, see
+https://pve.proxmox.com/wiki/PCI_Passthrough[PCI Passthrough Examples].
Hardware
Your hardware needs to support `IOMMU` (*I*/*O* **M**emory **M**anagement
**U**nit) interrupt remapping, this includes the CPU and the mainboard.
-Generally, Intel systems with VT-d, and AMD systems with AMD-Vi support this.
+Generally, Intel systems with VT-d and AMD systems with AMD-Vi support this.
But it is not guaranteed that everything will work out of the box, due
to bad hardware implementation and missing or low quality drivers.
@@ -35,6 +43,17 @@ hardware, but even then, many modern system can support this.
Please refer to your hardware vendor to check if they support this feature
under Linux for your specific setup.
+Determining PCI Card Address
+
+
+The easiest way is to use the GUI to add a device of type "Host PCI" in the
VM's
+hardware tab. Alternatively, you can use the command line.
+
+You can locate your card using
+
+
+ lspci
+
Configuration
^
@@ -44,8 +63,8 @@ some configuration to enable PCI(e) passthrough.
.IOMMU
-First, you have to enable IOMMU support in your BIOS/UEFI. Usually the
-corresponding setting is called `IOMMU` or `VT-d`,but you should find the exact
+First, you will have to enable IOMMU support in your BIOS/UEFI. Usually the
+corresponding setting is called `IOMMU` or `VT-d`, but you should find the
exact
option name in the manual of your motherboard.
For Intel CPUs, you may also need to enable the IOMMU on the
@@ -75,13 +94,15 @@ to the xref:sysboot_edit_kernel_cmdline[kernel commandline].
.Kernel Modules
You have to make sure the following modules are loaded. This can be achieved by
-adding them to `'/etc/modules''
+adding them to `'/etc/modules''. In kernels newer than 6.2 ({pve} 8 and onward)
+the 'vfio_virqfd' module is part of the 'vfio' module, therefore loading
+'vfio_virqfd' in {pve} 8 and newer is not necessary.
vfio
vfio_iommu_type1
vfio_pci
- vfio_virqfd
+ vfio_virqfd #not needed if on kernel 6.2 or newer
[[qm_pci_passthrough_update_initram
Re: [pve-devel] [PATCH manager 2/4] ui: pci map edit: reintroduce warnings checks
Am 19.06.23 um 16:13 schrieb Dominik Csapak:
> diff --git a/www/manager6/window/PCIMapEdit.js
> b/www/manager6/window/PCIMapEdit.js
> index 516678e0..cd2dbfbe 100644
> --- a/www/manager6/window/PCIMapEdit.js
> +++ b/www/manager6/window/PCIMapEdit.js
> @@ -70,6 +70,44 @@ Ext.define('PVE.window.PCIMapEditWindow', {
> me.lookup('iommu_warning').setVisible(
> records.every((val) => val.data.iommugroup === -1),
> );
> +
> + let value = me.lookup('pciselector').getValue();
> + me.checkIsolated(value);
> + },
> +
> + checkIsolated: function(value) {
> + let me = this;
> +
> + let isIsolated = function(entry) {
> + let isolated = true;
> + let parsed = PVE.Parser.parsePropertyString(entry);
> + parsed.iommugroup = parseInt(parsed.iommugroup, 10);
Nit: is there a simpler way to get the selected elements directly from
the store instead of going via getValue() above and then do the parsing
here?
> + if (!parsed.iommugroup) {
> + return isolated;
> + }
> + me.lookup('pciselector').getStore().each(({ data }) => {
Nit: Feel a bit out of place to do the lookup here every time. Maybe
pass this in the store data as an argument to the function already?
> + let isSubDevice = data.id.startsWith(parsed.path);
> + if (data.iommugroup === parsed.iommugroup && data.id !==
> parsed.path && !isSubDevice) {
> + isolated = false;
> + return false;
> + }
> + return true;
> + });
> + return isolated;
> + };
> +
> + let showWarning = false;
> + if (Ext.isArray(value)) {
> + for (const entry of value) {
> + if (!isIsolated(entry)) {
> + showWarning = true;
> + break;
> + }
> + }
> + } else {
> + showWarning = isIsolated(value);
> + }
> + me.lookup('group_warning').setVisible(showWarning);
> },
>
Not in this patch, but the warning is
> The selected Device is not in a seperate IOMMU group, make sure this is
> intended.
which has two typos:
s/Device/device/
s/seperate/separate/
and I'd use s/The/A/ because multiple ones can be selected.
And the warnings could/should use gettext().
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH manager 3/4] ui: pci map edit: improve new host mappings dialog
Am 19.06.23 um 16:13 schrieb Dominik Csapak: > by disallowing nodes to be selected where a mapping already exists > and not preselecting a node > Two UX nits, not sure how hard they would be to address: - if all nodes already have a mapping, disable the button (or after the RFC, hide the action element) - maybe pre-select the first valid node that has no mapping yet instead of none? ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [RFC PATCH manager 4/4] ui: pci mapping: rework mapping panel for better user experience
Am 19.06.23 um 16:13 schrieb Dominik Csapak:
> by removing the confusing buttons in the toolbar and adding them as
> actions in an actioncolumn. There a only relevant actions are visible
> and get a more expressive tooltip
I agree with Aaron that the actioncolumn is too far right at the moment.
> with this, we now differentiate between 4 modes of the edit window:
> * create a new mapping altogether
> - shows all fields
> * edit existing mapping on top level
> - show only 'global' fields (comment+mdev), so no mappings
This one feels slightly surprising to me from a user perspective as I
can't edit the actual mapping here. But it is cleaner and I guess one
could argue in the opposite direction too.
> * add new host mapping
> - shows nodeselector, mapping and mdev, but mdev is disabled
> (informational only)
> * edit existing host mapping
> - show selected node (displayfield) mdev and mappings, but only
> mappings are editable
>
> we have to split the nodeselector into two fields, since the disabling
> cbind does not pass through to the editconfig (and thus makes the form
> invalid if we try that)
>
> Signed-off-by: Dominik Csapak
> ---
> this is not intended to be applied as is, rather i'd like some feedback
> on the approach (@thomas, @aaron ?) so that if we want to do it this way
> i can also do it for the usb mappings
>
> the other approach mentioned off-list can still be done
> (having a full grid with all mappings regardless of the node)
> maybe only for usb devices (there it makes imho more sense) but then
> we'd have two interfaces for the mappings instead of one
It does involve a bit of clicking when it's only possible to add one
node entry at a time, but I'm not generally opposed to the current RFC.
I can image the action column takes a bit of getting used to as a
Proxmox VE user, because we don't really have those there yet.
The full grid might become quite big/confusing and involve lots of
scrolling or how would the grouping by node be done?
Maybe a third alternative would be to have a tab for each node and show
basic meta-info like how many devices are already selected on that node
and a warn/error indicator if that node is affected?
Would the full grid and tabs approach even be feasible with many nodes
or require too many API calls?
>
> www/manager6/tree/ResourceMapTree.js | 166 ---
> www/manager6/window/PCIMapEdit.js| 42 ---
> 2 files changed, 130 insertions(+), 78 deletions(-)
>
> diff --git a/www/manager6/tree/ResourceMapTree.js
> b/www/manager6/tree/ResourceMapTree.js
> index 02717042..cd24923e 100644
> --- a/www/manager6/tree/ResourceMapTree.js
> +++ b/www/manager6/tree/ResourceMapTree.js
> @@ -49,44 +49,89 @@ Ext.define('PVE.tree.ResourceMapTree', {
> });
> },
>
> - addHost: function() {
> + add: function(_grid, _rI, _cI, _item, _e, rec) {
> let me = this;
> - me.edit(false);
> + if (!rec.data.type === 'entry') {
AFAICT, this always evaluates to false, because of the misplaced '!'.
> + return;
> + }
> +
> + me.openMapEditWindow(rec.data.name);
> },
>
(...)
> @@ -254,63 +299,56 @@ Ext.define('PVE.tree.ResourceMapTree', {
>
> tbar: [
> {
> - text: gettext('Add mapping'),
> + text: gettext('Add'),
IMHO, Add mapping was/is better
> handler: 'addMapping',
> cbind: {
> disabled: '{!canConfigure}',
> },
> },
(...)
> diff --git a/www/manager6/window/PCIMapEdit.js
> b/www/manager6/window/PCIMapEdit.js
> index 0da2bae7..a0b42758 100644
> --- a/www/manager6/window/PCIMapEdit.js
> +++ b/www/manager6/window/PCIMapEdit.js
> @@ -13,8 +13,12 @@ Ext.define('PVE.window.PCIMapEditWindow', {
>
> cbindData: function(initialConfig) {
> let me = this;
> - me.isCreate = !me.name || !me.nodename;
> + me.isCreate = (!me.name || !me.nodename) && !me.entryOnly;
> me.method = me.name ? 'PUT' : 'POST';
> + me.hideMapping = !!me.entryOnly;
> + me.hideComment = me.name && !me.entryOnly;
> + me.hideNodeSelector = me.nodename || me.entryOnly;
> + me.hideNode = !me.nodename || !me.hideNodeSelector;
> return {
> name: me.name,
> nodename: me.nodename,
Nit: Is it even necessary to return these two as they are already
persistent properties?
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [RFC PATCH manager 4/4] ui: pci mapping: rework mapping panel for better user experience
thanks for the review!
i agree with all of the points of the answers in your other mails
some comments here:
On 6/20/23 15:25, Fiona Ebner wrote:
Am 19.06.23 um 16:13 schrieb Dominik Csapak:
by removing the confusing buttons in the toolbar and adding them as
actions in an actioncolumn. There a only relevant actions are visible
and get a more expressive tooltip
I agree with Aaron that the actioncolumn is too far right at the moment.
yes makes sense to me too
with this, we now differentiate between 4 modes of the edit window:
* create a new mapping altogether
- shows all fields
* edit existing mapping on top level
- show only 'global' fields (comment+mdev), so no mappings
This one feels slightly surprising to me from a user perspective as I
can't edit the actual mapping here. But it is cleaner and I guess one
could argue in the opposite direction too.
the question would be "which mapping" if there is more than one,
so i left it out, this way the config of the overall entry
is a bit seperated from the mappings themselves
* add new host mapping
- shows nodeselector, mapping and mdev, but mdev is disabled
(informational only)
* edit existing host mapping
- show selected node (displayfield) mdev and mappings, but only
mappings are editable
we have to split the nodeselector into two fields, since the disabling
cbind does not pass through to the editconfig (and thus makes the form
invalid if we try that)
Signed-off-by: Dominik Csapak
---
this is not intended to be applied as is, rather i'd like some feedback
on the approach (@thomas, @aaron ?) so that if we want to do it this way
i can also do it for the usb mappings
the other approach mentioned off-list can still be done
(having a full grid with all mappings regardless of the node)
maybe only for usb devices (there it makes imho more sense) but then
we'd have two interfaces for the mappings instead of one
It does involve a bit of clicking when it's only possible to add one
node entry at a time, but I'm not generally opposed to the current RFC.
I can image the action column takes a bit of getting used to as a
Proxmox VE user, because we don't really have those there yet.
we have it in pbs and pmg, but yes it's a new pattern for pve
i however find it less confusing than the toolbar buttons
The full grid might become quite big/confusing and involve lots of
scrolling or how would the grouping by node be done?
grouping/treestyle in such grids is always a bit tricky
i am currently working on this approach for the usb mapping
(as a demo of the other solution) but am running into quite
a few weird extjs related thing regarding widget columns and field
event handlers :( i'll see if i can send it tomorrow before lunch
Maybe a third alternative would be to have a tab for each node and show
basic meta-info like how many devices are already selected on that node
and a warn/error indicator if that node is affected?
mhmm that could be done, it's also a pattern we don't currently have:
a dynamic amount of tabs for each node
this could also become complex/confusing in a cluster with many nodes,
especially the hint in the tab title would probably not be visible
for all if there are many
i'll think about it
Would the full grid and tabs approach even be feasible with many nodes
or require too many API calls?
depends how we implement it, in my naive grid solution from above, the
api calls would be only done when a user selects a node
i.e it would look like:
-
| node | device | port | |
-
| [nodeselector] | [usbselector]| [usbselector] | [X] |
| | | | |
| | | | |
-
[add]
for pci it would be probably only one drop down + 'all functions' checkbox
the dropdowns only make an api call for the listing when the
nodeselector in the line changes its value
if we make more than one tab, we could defer the api call when the
user changes to the tab
www/manager6/tree/ResourceMapTree.js | 166 ---
www/manager6/window/PCIMapEdit.js| 42 ---
2 files changed, 130 insertions(+), 78 deletions(-)
diff --git a/www/manager6/tree/ResourceMapTree.js
b/www/manager6/tree/ResourceMapTree.js
index 02717042..cd24923e 100644
--- a/www/manager6/tree/ResourceMapTree.js
+++ b/www/manager6/tree/ResourceMapTree.js
@@ -49,44 +49,89 @@ Ext.define('PVE.tree.ResourceMapTree', {
});
},
- addHost: function() {
+ add: function(_grid, _rI, _cI, _item, _e, rec) {
let me = this;
- me.edit(false);
+ if (!rec.data.type === 'entry') {
AFAICT, this always evaluates to false, because of the misplaced '!'.
oops ;)
+ re
[pve-devel] [PATCH qemu-server] vm start: switch to new cleanup_transient_unit systemd helper
which also runs a reset-failed command for the unit first, to ensure
it is also cleaned up properly if in a failed state (e.g. after being
OOM-killed). Previuosly, users in that situation would only see the
less than ideal error message 'timeout waiting on systemd'.
Signed-off-by: Fiona Ebner
---
Dependency bump for libpve-common-perl needed.
PVE/QemuServer.pm| 7 +--
test/MigrationTest/Shared.pm | 2 +-
2 files changed, 2 insertions(+), 7 deletions(-)
diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
index 940cdacd..34d9b0b1 100644
--- a/PVE/QemuServer.pm
+++ b/PVE/QemuServer.pm
@@ -5871,12 +5871,7 @@ sub vm_start_nolock {
PVE::Storage::activate_volumes($storecfg, $vollist);
-eval {
- run_command(['/bin/systemctl', 'stop', "$vmid.scope"], outfunc =>
sub{}, errfunc => sub{});
-};
-# Issues with the above 'stop' not being fully completed are extremely
rare, a very low
-# timeout should be more than enough here...
-PVE::Systemd::wait_for_unit_removed("$vmid.scope", 20);
+PVE::Systemd::cleanup_transient_unit("$vmid.scope", 20);
my $cpuunits = PVE::CGroup::clamp_cpu_shares($conf->{cpuunits});
diff --git a/test/MigrationTest/Shared.pm b/test/MigrationTest/Shared.pm
index aa7203d1..44624f7f 100644
--- a/test/MigrationTest/Shared.pm
+++ b/test/MigrationTest/Shared.pm
@@ -191,7 +191,7 @@ $storage_plugin_module->mock(
our $systemd_module = Test::MockModule->new("PVE::Systemd");
$systemd_module->mock(
-wait_for_unit_removed => sub {
+cleanup_transient_unit => sub {
return;
},
enter_systemd_scope => sub {
--
2.39.2
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH common] systemd: add helper to cleanup transient unit
which combines the stop+wait logic previously present at the single
call site of wait_for_unit_removed() in QemuServer.pm. It also does a
reset-failed call first, to ensure a unit in a failed state is also
cleaned up properly.
Signed-off-by: Fiona Ebner
---
src/PVE/Systemd.pm | 16 +++-
1 file changed, 15 insertions(+), 1 deletion(-)
diff --git a/src/PVE/Systemd.pm b/src/PVE/Systemd.pm
index 2517d31..327106f 100644
--- a/src/PVE/Systemd.pm
+++ b/src/PVE/Systemd.pm
@@ -7,7 +7,7 @@ use Net::DBus qw(dbus_uint32 dbus_uint64 dbus_boolean);
use Net::DBus::Callback;
use Net::DBus::Reactor;
-use PVE::Tools qw(file_set_contents file_get_contents trim);
+use PVE::Tools qw(file_set_contents file_get_contents run_command trim);
sub escape_unit {
my ($val, $is_path) = @_;
@@ -167,6 +167,20 @@ sub wait_for_unit_removed($;$) {
}, $timeout);
}
+sub cleanup_transient_unit($;$) {
+my ($unit, $timeout) = @_;
+
+eval {
+ my %param = ( outfunc => sub {}, errfunc => sub {} );
+ # If the unit is in a failed state (e.g. after being OOM-killed),
stopping is not enough.
+ run_command(['/bin/systemctl', 'reset-failed', $unit], %param);
+ run_command(['/bin/systemctl', 'stop', $unit], %param);
+};
+
+# Issues with the above not being fully completed are rare, but not
impossible, see bug #3733.
+wait_for_unit_removed($unit, $timeout);
+}
+
sub read_ini {
my ($filename) = @_;
--
2.39.2
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH container] fix #4765: lxc: report cpu usage correctly
When running `pct status VMID` the variable
$last_proc_vmid_stat->{$vmid} is not set and pct reports no cpu usage.
We address this by computing the used cpu time over the total uptime of
the container.
Signed-off-by: Maximiliano Sandoval
---
src/PVE/LXC.pm | 5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/src/PVE/LXC.pm b/src/PVE/LXC.pm
index a531ea5..17a7cd7 100644
--- a/src/PVE/LXC.pm
+++ b/src/PVE/LXC.pm
@@ -276,10 +276,13 @@ sub vmstatus {
my $old = $last_proc_vmid_stat->{$vmid};
if (!$old) {
+ my $uptime_ms = $d->{uptime} * $cpucount * 1000.0;
+ $d->{cpu} = (($used_ms / $uptime_ms) * $cpucount) / $d->{cpus};
+
$last_proc_vmid_stat->{$vmid} = {
time => $cdtime,
used => $used_ms,
- cpu => 0,
+ cpu => $d->{cpu},
};
next;
}
--
2.39.2
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH installer] remove nonexistent file from makefile
Am 20/06/2023 um 10:42 schrieb Maximiliano Sandoval: > Signed-off-by: Maximiliano Sandoval > --- > Proxmox/Makefile | 1 - > 1 file changed, 1 deletion(-) > > diff --git a/Proxmox/Makefile b/Proxmox/Makefile > index d49da80..6360064 100644 > --- a/Proxmox/Makefile > +++ b/Proxmox/Makefile > @@ -6,7 +6,6 @@ PERL5DIR=$(DESTDIR)/usr/share/perl5/Proxmox > > #PERL_MODULES=$(shell git ls-files . | grep '.pm$') > PERL_MODULES=\ > -Install.pm \ > Install/Config.pm \ > Install/ISOEnv.pm \ > Install/RunEnv.pm \ sorry, committed that by accident, but it exists now: https://git.proxmox.com/?p=pve-installer.git;a=commit;h=a677c773e9856c7c8cbd20916870cea3d86c48b2 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] applied: [PATCH qemu-server 1/2] restore: refactor archive parsing
Am 20/06/2023 um 09:41 schrieb Fabian Grünbichler: > to avoid duplicate work, always set 'volid' to the backup volume's volid, if > it > was successfully parsed as such. > > Signed-off-by: Fabian Grünbichler > --- > PVE/API2/Qemu.pm | 16 > 1 file changed, 8 insertions(+), 8 deletions(-) > > applied series, thanks! ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
