[pve-devel] [PATCH v4 installer 31/30 follow-up] auto-installer: answer: deny unknown fields
This way, serde will throw errors if fields are not known.
This can help to reduce frustration if one might think to have set an
option, but for example a small type has happened.
Signed-off-by: Aaron Lauterer
---
Since Christoph mentioned it I tried to implement it. Tested quickly
with the proxmox-autoinst-helper tool.
proxmox-auto-installer/src/answer.rs | 16 +++-
1 file changed, 11 insertions(+), 5 deletions(-)
diff --git a/proxmox-auto-installer/src/answer.rs
b/proxmox-auto-installer/src/answer.rs
index 94cebb3..57c2602 100644
--- a/proxmox-auto-installer/src/answer.rs
+++ b/proxmox-auto-installer/src/answer.rs
@@ -10,7 +10,7 @@ use std::{collections::BTreeMap, net::IpAddr};
/// storing them in a HashMap
#[derive(Clone, Deserialize, Debug)]
-#[serde(rename_all = "kebab-case")]
+#[serde(rename_all = "kebab-case", deny_unknown_fields)]
pub struct Answer {
pub global: Global,
pub network: Network,
@@ -19,6 +19,7 @@ pub struct Answer {
}
#[derive(Clone, Deserialize, Debug)]
+#[serde(deny_unknown_fields)]
pub struct Global {
pub country: String,
pub fqdn: Fqdn,
@@ -33,6 +34,7 @@ pub struct Global {
}
#[derive(Clone, Deserialize, Debug)]
+#[serde(deny_unknown_fields)]
struct NetworkInAnswer {
#[serde(default)]
pub use_dhcp: bool,
@@ -43,7 +45,7 @@ struct NetworkInAnswer {
}
#[derive(Clone, Deserialize, Debug)]
-#[serde(try_from = "NetworkInAnswer")]
+#[serde(try_from = "NetworkInAnswer", deny_unknown_fields)]
pub struct Network {
pub network_settings: NetworkSettings,
}
@@ -97,6 +99,7 @@ pub struct NetworkManual {
}
#[derive(Clone, Debug, Deserialize)]
+#[serde(deny_unknown_fields)]
pub struct DiskSetup {
pub filesystem: Filesystem,
#[serde(default)]
@@ -109,7 +112,7 @@ pub struct DiskSetup {
}
#[derive(Clone, Debug, Deserialize)]
-#[serde(try_from = "DiskSetup")]
+#[serde(try_from = "DiskSetup", deny_unknown_fields)]
pub struct Disks {
pub fs_type: FsType,
pub disk_selection: DiskSelection,
@@ -207,14 +210,14 @@ pub enum DiskSelection {
Filter(BTreeMap),
}
#[derive(Clone, Deserialize, Debug, PartialEq, ValueEnum)]
-#[serde(rename_all = "lowercase")]
+#[serde(rename_all = "lowercase", deny_unknown_fields)]
pub enum FilterMatch {
Any,
All,
}
#[derive(Clone, Deserialize, Serialize, Debug, PartialEq)]
-#[serde(rename_all = "lowercase")]
+#[serde(rename_all = "lowercase", deny_unknown_fields)]
pub enum Filesystem {
Ext4,
Xfs,
@@ -223,6 +226,7 @@ pub enum Filesystem {
}
#[derive(Clone, Copy, Default, Deserialize, Debug)]
+#[serde(deny_unknown_fields)]
pub struct ZfsOptions {
pub raid: Option,
pub ashift: Option,
@@ -234,6 +238,7 @@ pub struct ZfsOptions {
}
#[derive(Clone, Copy, Default, Deserialize, Serialize, Debug)]
+#[serde(deny_unknown_fields)]
pub struct LvmOptions {
pub hdsize: Option,
pub swapsize: Option,
@@ -243,6 +248,7 @@ pub struct LvmOptions {
}
#[derive(Clone, Copy, Default, Deserialize, Debug)]
+#[serde(deny_unknown_fields)]
pub struct BtrfsOptions {
pub hdsize: Option,
pub raid: Option,
--
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-network 0/9] SDN: Testing VNets as a blackbox.
sent v3 which fixes also the broken tests. ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH v3 pve-network 08/12] sdn: dnsmasq: extract function that updates dnsmasq lease via dbus
Extract the dbus based interactions with dnsmasq so that it can be
mocked in tests.
Signed-off-by: Stefan Lendl
Reviewed-by: Max Carrara
Tested-by: Max Carrara
Reviewed-by: Stefan Hanreich
Tested-by: Stefan Hanreich
---
src/PVE/Network/SDN/Dhcp/Dnsmasq.pm | 22 --
1 file changed, 12 insertions(+), 10 deletions(-)
diff --git a/src/PVE/Network/SDN/Dhcp/Dnsmasq.pm
b/src/PVE/Network/SDN/Dhcp/Dnsmasq.pm
index 5a227ba..c14f5d7 100644
--- a/src/PVE/Network/SDN/Dhcp/Dnsmasq.pm
+++ b/src/PVE/Network/SDN/Dhcp/Dnsmasq.pm
@@ -38,6 +38,17 @@ sub ethers_file {
return "$DNSMASQ_CONFIG_ROOT/$dhcpid/ethers";
}
+sub update_lease {
+my ($dhcpid, $ip4, $mac) = @_;
+#update lease as ip could still be associated to an old removed mac
+my $bus = Net::DBus->system();
+my $dnsmasq = $bus->get_service("uk.org.thekelleys.dnsmasq.$dhcpid");
+my $manager =
$dnsmasq->get_object("/uk/org/thekelleys/dnsmasq","uk.org.thekelleys.dnsmasq.$dhcpid");
+
+my @hostname = unpack("C*", "*");
+$manager->AddDhcpLease($ip4, $mac, \@hostname, undef, 0, 0, 0) if $ip4;
+}
+
sub add_ip_mapping {
my ($class, $dhcpid, $macdb, $mac, $ip4, $ip6) = @_;
@@ -107,16 +118,7 @@ sub add_ip_mapping {
my $service_name = "dnsmasq\@$dhcpid";
systemctl_service('reload', $service_name) if $reload;
-
-#update lease as ip could still be associated to an old removed mac
-my $bus = Net::DBus->system();
-my $dnsmasq = $bus->get_service("uk.org.thekelleys.dnsmasq.$dhcpid");
-my $manager =
$dnsmasq->get_object("/uk/org/thekelleys/dnsmasq","uk.org.thekelleys.dnsmasq.$dhcpid");
-
-my @hostname = unpack("C*", "*");
-$manager->AddDhcpLease($ip4, $mac, \@hostname, undef, 0, 0, 0) if $ip4;
-#$manager->AddDhcpLease($ip6, $mac, \@hostname, undef, 0, 0, 0) if $ip6;
-
+update_lease($dhcpid, $ip4, $mac);
}
sub configure_subnet {
--
2.44.0
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH v3 pve-network 11/12] tests: test VNets functionality as a blackbox
Add several tests for Vnets in test_vnets_blackbox. State setup as well
as testing results is done only via the API to test on the API
boundaries not not against the internal state. Internal state is mocked
to avoid requiring access to system files or pmxcfs.
Mocking is done by reading and writing to a hash that holds the entire
state of SDN. The state is reset after every test run.
Testing is done via helper functions: nic_join and nic_start.
When a nic joins a Vnet, currently it always - and only - calls
add_next_free_cidr(). The same is true if a nic starts on Vnet, which
only calles add_dhcp_mapping.
These test functions homogenize the parameter list in contrast to the
current calls to the current functions. The intention for the functions
is that they can be moved to Vnets.pm to be called from QemuServer and
LXC!
The tests are composed of a test function which can be parameterized. To
call the test function, the run_test function takes the function pointer
and passes the rest of the arguments to the test functions. It also
takes care of resetting the test state.
This allows fine-grained parameterization per-test directly in the code
instead of separated files that require the entire state to be passed
in.
The tests setup the SDN by creating a simple zone and a simple vnet. The
nic_join and nic_start function is called with different subnet
configuration wiht and without a dhcp-range configured and with or
without an already present IP in the IPAM.
Signed-off-by: Stefan Lendl
Reviewed-by: Max Carrara
Tested-by: Max Carrara
Reviewed-by: Stefan Hanreich
Tested-by: Stefan Hanreich
---
src/test/Makefile | 5 +-
src/test/run_test_vnets_blackbox.pl | 894
2 files changed, 898 insertions(+), 1 deletion(-)
create mode 100755 src/test/run_test_vnets_blackbox.pl
diff --git a/src/test/Makefile b/src/test/Makefile
index eb59d5f..5a937a4 100644
--- a/src/test/Makefile
+++ b/src/test/Makefile
@@ -1,6 +1,6 @@
all: test
-test: test_zones test_ipams test_dns test_subnets
+test: test_zones test_ipams test_dns test_subnets test_vnets_blackbox
test_zones: run_test_zones.pl
./run_test_zones.pl
@@ -14,4 +14,7 @@ test_dns: run_test_dns.pl
test_subnets: run_test_subnets.pl
./run_test_subnets.pl
+test_vnets_blackbox: run_test_vnets_blackbox.pl
+ ./run_test_vnets_blackbox.pl
+
clean:
diff --git a/src/test/run_test_vnets_blackbox.pl
b/src/test/run_test_vnets_blackbox.pl
new file mode 100755
index 000..f7caca2
--- /dev/null
+++ b/src/test/run_test_vnets_blackbox.pl
@@ -0,0 +1,894 @@
+#!/usr/bin/perl
+
+use strict;
+use warnings;
+
+use lib qw(..);
+use File::Slurp;
+use List::Util qw(first all);
+use NetAddr::IP qw(:lower);
+
+use Test::More;
+use Test::MockModule;
+
+use PVE::Tools qw(extract_param file_set_contents);
+
+use PVE::Network::SDN;
+use PVE::Network::SDN::Zones;
+use PVE::Network::SDN::Zones::Plugin;
+use PVE::Network::SDN::Controllers;
+use PVE::Network::SDN::Dns;
+use PVE::Network::SDN::Vnets;
+
+use PVE::RESTEnvironment;
+
+use PVE::API2::Network::SDN::Zones;
+use PVE::API2::Network::SDN::Subnets;
+use PVE::API2::Network::SDN::Vnets;
+use PVE::API2::Network::SDN::Ipams;
+
+my $TMP_ETHERS_FILE = "/tmp/ethers";
+
+my $test_state = undef;
+sub clear_test_state {
+$test_state = {
+ locks => {},
+ datacenter_config => {},
+ subnets_config => {},
+ controller_config => {},
+ dns_config => {},
+ zones_config => {},
+ vnets_config => {},
+ macdb => {},
+ ipamdb => {},
+ ipam_config => {
+ 'ids' => {
+ 'pve' => {
+ 'type' => 'pve'
+ },
+ }
+ },
+};
+PVE::Tools::file_set_contents($TMP_ETHERS_FILE, "\n");
+}
+clear_test_state();
+
+my $mocked_cfs_lock_file = sub {
+my ($filename, $timeout, $code, @param) = @_;
+
+die "$filename already locked\n" if ($test_state->{locks}->{$filename});
+
+$test_state->{locks}->{$filename} = 1;
+
+my $res = eval { $code->(@param); };
+
+delete $test_state->{locks}->{$filename};
+
+return $res;
+};
+
+sub read_sdn_config {
+my ($file) = @_;
+# Read structure back in again
+open my $in, '<', $file or die $!;
+my $sdn_config;
+{
+ local $/;# slurp mode
+ $sdn_config = eval <$in>;
+}
+close $in;
+return $sdn_config;
+}
+
+my $mocked_pve_sdn;
+$mocked_pve_sdn = Test::MockModule->new('PVE::Network::SDN');
+$mocked_pve_sdn->mock(
+cfs_lock_file => $mocked_cfs_lock_file,
+);
+
+my $mocked_pve_tools = Test::MockModule->new('PVE::Tools');
+$mocked_pve_tools->mock(
+lock_file => $mocked_cfs_lock_file,
+);
+
+my $mocked_sdn_zones;
+$mocked_sdn_zones = Test::MockModule->new('PVE::Network::SDN::Zones');
+$mocked_sdn_zones->mock(
+config => sub {
+ return $test_state->{zones_config};
+},
+write_config => sub {
+ my ($cfg) = @_;
+ $test_state->{zones_config} =
[pve-devel] [PATCH v3 pve-network 12/12] tests: remove old Vnets tests
The did not work and were primarily testing against internal state.
Signed-off-by: Stefan Lendl
Reviewed-by: Max Carrara
Tested-by: Max Carrara
Reviewed-by: Stefan Hanreich
Tested-by: Stefan Hanreich
---
src/test/run_test_vnets.pl | 343 -
1 file changed, 343 deletions(-)
delete mode 100755 src/test/run_test_vnets.pl
diff --git a/src/test/run_test_vnets.pl b/src/test/run_test_vnets.pl
deleted file mode 100755
index 65fbfb7..000
--- a/src/test/run_test_vnets.pl
+++ /dev/null
@@ -1,343 +0,0 @@
-#!/usr/bin/perl
-
-use strict;
-use warnings;
-
-use lib qw(..);
-use File::Slurp;
-use NetAddr::IP qw(:lower);
-
-use Test::More;
-use Test::MockModule;
-
-use PVE::Network::SDN;
-use PVE::Network::SDN::Zones;
-use PVE::Network::SDN::Controllers;
-use PVE::INotify;
-use JSON;
-
-use Data::Dumper qw(Dumper);
-$Data::Dumper::Sortkeys = 1;
-
-sub read_sdn_config {
-my ($file) = @_;
-
-# Read structure back in again
-open my $in, '<', $file or die $!;
-my $sdn_config;
-{
- local $/; # slurp mode
- $sdn_config = eval <$in>;
-}
-close $in;
-return $sdn_config;
-}
-
-my @plugins = read_dir('./vnets/', prefix => 1);
-
-foreach my $path (@plugins) {
-
-my (undef, $testid) = split(/\//, $path);
-
-print "test: $testid\n";
-my $sdn_config = read_sdn_config("$path/sdn_config");
-
-my $pve_sdn_zones;
-$pve_sdn_zones = Test::MockModule->new('PVE::Network::SDN::Zones');
-$pve_sdn_zones->mock(
- config => sub {
- return $sdn_config->{zones};
- },
-);
-
-my $pve_sdn_vnets;
-$pve_sdn_vnets = Test::MockModule->new('PVE::Network::SDN::Vnets');
-$pve_sdn_vnets->mock(
- config => sub {
- return $sdn_config->{vnets};
- },
-);
-
-my $pve_sdn_subnets;
-$pve_sdn_subnets = Test::MockModule->new('PVE::Network::SDN::Subnets');
-$pve_sdn_subnets->mock(
- config => sub {
- return $sdn_config->{subnets};
- },
- verify_dns_zone => sub {
- return;
- },
- add_dns_record => sub {
- return;
- },
-);
-
-my $js = JSON->new;
-$js->canonical(1);
-
-#test params;
-#test params;
-my $subnets = $sdn_config->{subnets}->{ids};
-
-my $subnetid = (sort keys %{$subnets})[0];
-my $subnet =
- PVE::Network::SDN::Subnets::sdn_subnets_config($sdn_config->{subnets},
$subnetid, 1);
-my $subnet_cidr = $subnet->{cidr};
-my $iplist = NetAddr::IP->new($subnet_cidr);
-my $mask = $iplist->masklen();
-my $ipversion = undef;
-
-if (Net::IP::ip_is_ipv4($iplist->canon())) {
- $iplist++; #skip network address for ipv4
- $ipversion = 4;
-} else {
- $ipversion = 6;
-}
-
-my $cidr1 = $iplist->canon() . "/$mask";
-$iplist++;
-my $cidr2 = $iplist->canon() . "/$mask";
-my $cidr_outofrange = '8.8.8.8/8';
-
-my $subnetid2 = (sort keys %{$subnets})[1];
-my $subnet2 =
- PVE::Network::SDN::Subnets::sdn_subnets_config($sdn_config->{subnets},
$subnetid2, 1);
-my $subnet2_cidr = $subnet2->{cidr};
-my $iplist2 = NetAddr::IP->new($subnet2_cidr);
-$iplist2++;
-my $cidr3 = $iplist2->canon() . "/$mask";
-$iplist2++;
-my $cidr4 = $iplist2->canon() . "/$mask";
-
-my $hostname = "myhostname";
-my $mac = "da:65:8f:18:9b:6f";
-my $description = "mydescription";
-my $ipamdb = read_sdn_config("$path/ipam.db");
-
-my $zone = $sdn_config->{zones}->{ids}->{"myzone"};
-my $ipam = $zone->{ipam};
-
-my $plugin;
-my $sdn_ipam_plugin;
-if ($ipam) {
- $plugin = PVE::Network::SDN::Ipams::Plugin->lookup($ipam);
- $sdn_ipam_plugin = Test::MockModule->new($plugin);
- $sdn_ipam_plugin->mock(
- read_db => sub {
- return $ipamdb;
- },
- write_db => sub {
- my ($cfg) = @_;
- $ipamdb = $cfg;
- },
- );
-}
-
-my $pve_sdn_ipams;
-$pve_sdn_ipams = Test::MockModule->new('PVE::Network::SDN::Ipams');
-$pve_sdn_ipams->mock(
- config => sub {
- my $ipam_config = read_sdn_config("$path/ipam_config");
- return $ipam_config;
- },
-);
-
-my $vnetid = "myvnet";
-
-## add_ip
-my $test = "add_cidr $cidr1";
-my $name = "$testid $test";
-my $result = undef;
-my $expected = '';
-
-eval { PVE::Network::SDN::Vnets::add_cidr($vnetid, $cidr1, $hostname,
$mac, $description); };
-
-if ($@) {
- fail("$name : $@");
-} else {
- is(undef, undef, $name);
-}
-
-## add_ip
-$test = "add_already_exist_cidr $cidr1";
-$name = "$testid $test";
-$result = undef;
-$expected = '';
-
-eval { PVE::Network::SDN::Vnets::add_cidr($vnetid, $cidr1, $hostname,
$mac, $description); };
-
-if ($@) {
- is(undef, undef, $name);
-} elsif ($ipam) {
- fail("$name : $@");
-} else {
- is(undef,
[pve-devel] [PATCH v3 pve-network 10/12] debian: blackbox tests depend on libpve-access-control at build
For mocking RPCEnvironment in sbuild. Signed-off-by: Stefan Lendl Reviewed-by: Stefan Hanreich Tested-by: Stefan Hanreich --- debian/control | 1 + 1 file changed, 1 insertion(+) diff --git a/debian/control b/debian/control index 6f0bbeb..1f97196 100644 --- a/debian/control +++ b/debian/control @@ -10,6 +10,7 @@ Build-Depends: debhelper-compat (= 13), perl, pve-cluster (>= 8.0.5), pve-doc-generator (>= 5.3-3), + libpve-access-control, Standards-Version: 4.6.1 Homepage: https://www.proxmox.com -- 2.44.0 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH v3 pve-network 06/12] dns: dnsmasq: extract function to systemctl command.
systemctl_service() is a wrapper around PVE::Tools::run_command to allow
mocking the systemctl interactions in tests.
Signed-off-by: Stefan Lendl
Reviewed-by: Max Carrara
Tested-by: Max Carrara
Reviewed-by: Stefan Hanreich
Tested-by: Stefan Hanreich
---
src/PVE/Network/SDN/Dhcp/Dnsmasq.pm | 18 --
1 file changed, 12 insertions(+), 6 deletions(-)
diff --git a/src/PVE/Network/SDN/Dhcp/Dnsmasq.pm
b/src/PVE/Network/SDN/Dhcp/Dnsmasq.pm
index 2844943..f9f1c39 100644
--- a/src/PVE/Network/SDN/Dhcp/Dnsmasq.pm
+++ b/src/PVE/Network/SDN/Dhcp/Dnsmasq.pm
@@ -101,7 +101,7 @@ sub add_ip_mapping {
}
my $service_name = "dnsmasq\@$dhcpid";
-PVE::Tools::run_command(['systemctl', 'reload', $service_name]) if $reload;
+systemctl_service('reload', $service_name) if $reload;
#update lease as ip could still be associated to an old removed mac
my $bus = Net::DBus->system();
@@ -163,6 +163,12 @@ sub configure_vnet {
);
}
+sub systemctl_service {
+my ($action, $service) = @_;
+
+PVE::Tools::run_command(['systemctl', $action, $service]);
+}
+
sub before_configure {
my ($class, $dhcpid) = @_;
@@ -250,9 +256,9 @@ sub after_configure {
my $service_name = "dnsmasq\@$dhcpid";
-PVE::Tools::run_command(['systemctl', 'reload', 'dbus']);
-PVE::Tools::run_command(['systemctl', 'enable', $service_name]);
-PVE::Tools::run_command(['systemctl', 'restart', $service_name]);
+systemctl_service('reload', 'dbus');
+systemctl_service('enable', $service_name);
+systemctl_service('restart', $service_name);
}
sub before_regenerate {
@@ -260,8 +266,8 @@ sub before_regenerate {
return if !assert_dnsmasq_installed($noerr);
-PVE::Tools::run_command(['systemctl', 'stop', "dnsmasq@*"]);
-PVE::Tools::run_command(['systemctl', 'disable', 'dnsmasq@']);
+systemctl_service('stop', "dnsmasq@*");
+systemctl_service('disable', 'dnsmasq@');
}
sub after_regenerate {
--
2.44.0
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH v3 pve-network 00/12] SDN: Testing VNets as a blackbox and fixing found bugs
This add several tests for SDN VNets. State setup as well as testing results is done only via the API to test on the API boundaries and not against the internal state. Internal state and config files are mocked to avoid requiring access to system files or pmxcfs. The first 4 commits fix functionality, identified as bugs thanks to the tests. The next 7 commits extract various functions to allow mocking them in the tests. The tests are then added as the test_vnets_blackbox test. The last commit removes the old vnets tests which are not working anyway. Tests validate the events of a nic joining a VNet or a nic staring on a VNet. These events are tested with with different subnet configurations. Mainly for IPv4 and/or IPv6 configurations and odd combinations. Further descriptions in the commit. Differences v2 -> v3: * Fix functionalitiy in VNet and Subnet so all tests pass Thanks @s.hanreich for lots of testing * Update and add more tests * Make it build in sbuild Differences v1 -> v2: * Add tests that expect a failure when no IP can be allocated * Removed commented out debug stuff Stefan Hanreich (2): sdn: dhcp: only consider subnets that have dhcp-range configured sdn: dhcp: rollback allocated ips on failure Stefan Lendl (10): sdn: dhcp: get next free ip for a specific IP version sdn: dhcp: request both IPv4 and IPv6 addresses on VM start sdn: zones: extract function that reads datacenter config dns: dnsmasq: extract function to systemctl command. sdn: dnsmasq: extract function that generates the ethers file path sdn: dnsmasq: extract function that updates dnsmasq lease via dbus sdn: api: extract function that creates the sdn directory. debian: blackbox tests depend on libpve-access-control at build tests: test VNets functionality as a blackbox tests: remove old Vnets tests debian/control| 1 + src/PVE/API2/Network/SDN/Zones.pm | 6 +- src/PVE/Network/SDN/Dhcp/Dnsmasq.pm | 47 +- src/PVE/Network/SDN/Subnets.pm| 2 +- src/PVE/Network/SDN/Vnets.pm | 29 +- src/PVE/Network/SDN/Zones/EvpnPlugin.pm | 3 +- src/PVE/Network/SDN/Zones/Plugin.pm | 5 + src/PVE/Network/SDN/Zones/SimplePlugin.pm | 2 +- src/test/Makefile | 5 +- src/test/run_test_vnets.pl| 343 - src/test/run_test_vnets_blackbox.pl | 894 ++ 11 files changed, 962 insertions(+), 375 deletions(-) delete mode 100755 src/test/run_test_vnets.pl create mode 100755 src/test/run_test_vnets_blackbox.pl -- 2.44.0 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH v3 pve-network 01/12] sdn: dhcp: get next free ip for a specific IP version
Specify the IP version (4|6) for which an IP shall be requested from the IPAM.
Signed-off-by: Stefan Lendl
Reviewed-by: Stefan Hanreich
Tested-by: Stefan Hanreich
---
src/PVE/Network/SDN/Subnets.pm | 2 +-
src/PVE/Network/SDN/Vnets.pm | 6 +++---
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/src/PVE/Network/SDN/Subnets.pm b/src/PVE/Network/SDN/Subnets.pm
index 3b08dcd..e2c8c9c 100644
--- a/src/PVE/Network/SDN/Subnets.pm
+++ b/src/PVE/Network/SDN/Subnets.pm
@@ -206,7 +206,7 @@ sub del_subnet {
}
sub add_next_free_ip {
-my ($zone, $subnetid, $subnet, $hostname, $mac, $vmid, $skipdns,
$dhcprange) = @_;
+my ($zone, $subnetid, $subnet, $hostname, $mac, $vmid, $skipdns,
$dhcprange, $ipversion) = @_;
my $cidr = undef;
my $ip = undef;
diff --git a/src/PVE/Network/SDN/Vnets.pm b/src/PVE/Network/SDN/Vnets.pm
index 0dfdfd7..03609b7 100644
--- a/src/PVE/Network/SDN/Vnets.pm
+++ b/src/PVE/Network/SDN/Vnets.pm
@@ -95,7 +95,7 @@ sub get_subnet_from_vnet_ip {
}
sub add_next_free_cidr {
-my ($vnetid, $hostname, $mac, $vmid, $skipdns, $dhcprange) = @_;
+my ($vnetid, $hostname, $mac, $vmid, $skipdns, $dhcprange, $ipversion) =
@_;
my $vnet = PVE::Network::SDN::Vnets::get_vnet($vnetid);
return if !$vnet;
@@ -109,7 +109,7 @@ sub add_next_free_cidr {
my $ips = {};
-my @ipversions = qw/ 4 6 /;
+my @ipversions = defined($ipversion) ? ($ipversion) : qw/ 4 6 /;
for my $ipversion (@ipversions) {
my $ip = undef;
my $subnetcount = 0;
@@ -125,7 +125,7 @@ sub add_next_free_cidr {
};
die $@ if $@;
-if ($ip) {
+ if ($ip) {
$ips->{$ipversion} = $ip;
last;
}
--
2.44.0
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH v3 pve-network 09/12] sdn: api: extract function that creates the sdn directory.
create_etc_interfaces_sdn_dir creates the /etc/pve/sdn directory.
This allows mocking in tests to prevent system fs access in tests
Signed-off-by: Stefan Lendl
Reviewed-by: Max Carrara
Tested-by: Max Carrara
Reviewed-by: Stefan Hanreich
Tested-by: Stefan Hanreich
---
src/PVE/API2/Network/SDN/Zones.pm | 6 +-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/src/PVE/API2/Network/SDN/Zones.pm
b/src/PVE/API2/Network/SDN/Zones.pm
index b09c9ad..35e2f7f 100644
--- a/src/PVE/API2/Network/SDN/Zones.pm
+++ b/src/PVE/API2/Network/SDN/Zones.pm
@@ -186,6 +186,10 @@ __PACKAGE__->register_method ({
return &$api_sdn_zones_config($cfg, $param->{zone});
}});
+sub create_etc_interfaces_sdn_dir {
+mkdir("/etc/pve/sdn");
+}
+
__PACKAGE__->register_method ({
name => 'create',
protected => 1,
@@ -207,7 +211,7 @@ __PACKAGE__->register_method ({
my $opts = $plugin->check_config($id, $param, 1, 1);
PVE::Cluster::check_cfs_quorum();
- mkdir("/etc/pve/sdn");
+ create_etc_interfaces_sdn_dir();
PVE::Network::SDN::lock_sdn_config(sub {
my $zone_cfg = PVE::Network::SDN::Zones::config();
--
2.44.0
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH v3 pve-network 05/12] sdn: zones: extract function that reads datacenter config
The datacenter_config() functions in SDN::Zones::Plugin is a simple
wrapper that reads datacenter.cfg via cfs.
This allows mocking datacenter.cfg in tests.
Signed-off-by: Stefan Lendl
Reviewed-by: Max Carrara
Tested-by: Max Carrara
Reviewed-by: Stefan Hanreich
Tested-by: Stefan Hanreich
---
src/PVE/Network/SDN/Zones/EvpnPlugin.pm | 3 ++-
src/PVE/Network/SDN/Zones/Plugin.pm | 5 +
src/PVE/Network/SDN/Zones/SimplePlugin.pm | 2 +-
3 files changed, 8 insertions(+), 2 deletions(-)
diff --git a/src/PVE/Network/SDN/Zones/EvpnPlugin.pm
b/src/PVE/Network/SDN/Zones/EvpnPlugin.pm
index 655a9f0..4843756 100644
--- a/src/PVE/Network/SDN/Zones/EvpnPlugin.pm
+++ b/src/PVE/Network/SDN/Zones/EvpnPlugin.pm
@@ -3,6 +3,7 @@ package PVE::Network::SDN::Zones::EvpnPlugin;
use strict;
use warnings;
use PVE::Network::SDN::Zones::VxlanPlugin;
+use PVE::Network::SDN::Zones::Plugin;
use PVE::Exception qw(raise raise_param_exc);
use PVE::JSONSchema qw(get_standard_option);
use PVE::Tools qw($IPV4RE);
@@ -294,7 +295,7 @@ sub on_update_hook {
}
if (!defined($zone_cfg->{ids}->{$zoneid}->{'mac'})) {
- my $dc = PVE::Cluster::cfs_read_file('datacenter.cfg');
+my $dc = PVE::Network::SDN::Zones::Plugin->datacenter_config();
$zone_cfg->{ids}->{$zoneid}->{'mac'} =
PVE::Tools::random_ether_addr($dc->{mac_prefix});
}
}
diff --git a/src/PVE/Network/SDN/Zones/Plugin.pm
b/src/PVE/Network/SDN/Zones/Plugin.pm
index b55b967..247d0b2 100644
--- a/src/PVE/Network/SDN/Zones/Plugin.pm
+++ b/src/PVE/Network/SDN/Zones/Plugin.pm
@@ -356,4 +356,9 @@ sub get_bridge_ifaces {
return @bridge_ifaces;
}
+
+sub datacenter_config {
+return PVE::Cluster::cfs_read_file('datacenter.cfg');
+}
+
1;
diff --git a/src/PVE/Network/SDN/Zones/SimplePlugin.pm
b/src/PVE/Network/SDN/Zones/SimplePlugin.pm
index c996bf3..65e9ad4 100644
--- a/src/PVE/Network/SDN/Zones/SimplePlugin.pm
+++ b/src/PVE/Network/SDN/Zones/SimplePlugin.pm
@@ -139,7 +139,7 @@ sub vnet_update_hook {
raise_param_exc({ tag => "vlan tag is not allowed on simple zone"}) if
defined($tag);
if (!defined($vnet->{mac})) {
-my $dc = PVE::Cluster::cfs_read_file('datacenter.cfg');
+my $dc = PVE::Network::SDN::Zones::Plugin::datacenter_config();
$vnet->{mac} = PVE::Tools::random_ether_addr($dc->{mac_prefix});
}
}
--
2.44.0
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH v3 pve-network 07/12] sdn: dnsmasq: extract function that generates the ethers file path
Extracted to a function so it can be mocked in tests.
Signed-off-by: Stefan Lendl
Reviewed-by: Max Carrara
Tested-by: Max Carrara
Reviewed-by: Stefan Hanreich
Tested-by: Stefan Hanreich
---
src/PVE/Network/SDN/Dhcp/Dnsmasq.pm | 7 ++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/src/PVE/Network/SDN/Dhcp/Dnsmasq.pm
b/src/PVE/Network/SDN/Dhcp/Dnsmasq.pm
index f9f1c39..5a227ba 100644
--- a/src/PVE/Network/SDN/Dhcp/Dnsmasq.pm
+++ b/src/PVE/Network/SDN/Dhcp/Dnsmasq.pm
@@ -33,10 +33,15 @@ my sub assert_dnsmasq_installed {
return 1;
}
+sub ethers_file {
+my ($dhcpid) = @_;
+return "$DNSMASQ_CONFIG_ROOT/$dhcpid/ethers";
+}
+
sub add_ip_mapping {
my ($class, $dhcpid, $macdb, $mac, $ip4, $ip6) = @_;
-my $ethers_file = "$DNSMASQ_CONFIG_ROOT/$dhcpid/ethers";
+my $ethers_file = ethers_file($dhcpid);
my $ethers_tmp_file = "$ethers_file.tmp";
my $reload = undef;
--
2.44.0
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH v3 pve-network 04/12] sdn: dhcp: rollback allocated ips on failure
From: Stefan Hanreich
If DHCP is configured for IPv4 and IPv6, failing to obtain an IPv6 IP
does not roll back the allocation made for IPv4. This patch rolls back
any changes made in case of failure, so that IP allocation is actually
atomic.
Signed-off-by: Stefan Hanreich
Reviewed-by: Stefan Lendl
Tested-by: Stefan Lendl
Signed-off-by: Stefan Lendl
---
src/PVE/Network/SDN/Vnets.pm | 12 +++-
1 file changed, 11 insertions(+), 1 deletion(-)
diff --git a/src/PVE/Network/SDN/Vnets.pm b/src/PVE/Network/SDN/Vnets.pm
index cbf0a07..45292e3 100644
--- a/src/PVE/Network/SDN/Vnets.pm
+++ b/src/PVE/Network/SDN/Vnets.pm
@@ -131,7 +131,17 @@ sub add_next_free_cidr {
last;
}
}
- die "can't find any free ip" if !$ip && $subnetcount > 0;
+
+ if (!$ip && $subnetcount > 0) {
+ foreach my $version (sort keys %{$ips}) {
+ my $ip = $ips->{$version};
+ my ($subnetid, $subnet) =
PVE::Network::SDN::Subnets::find_ip_subnet($ip, $subnets);
+
+ PVE::Network::SDN::Subnets::del_ip($zone, $subnetid, $subnet,
$ip, $hostname, $mac, $skipdns);
+ }
+
+ die "can't find any free ip in zone $zoneid for IPv$ipversion";
+ }
}
}
--
2.44.0
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH v3 pve-network 03/12] sdn: dhcp: only consider subnets that have dhcp-range configured
From: Stefan Hanreich
If DHCP is enabled on a zone with subnets, but no subnet has a
dhcp-range configured, then starting a VM will fail because no IP can
be allocated. This patch fixes this by only considering subnets that
have a dhcp-range configured and only failing if there is at least one
subnet with a dhcp-range configured.
Signed-off-by: Stefan Hanreich
Reviewed-by: Stefan Lendl
Tested-by: Stefan Lendl
Signed-off-by: Stefan Lendl
---
src/PVE/Network/SDN/Vnets.pm | 1 +
1 file changed, 1 insertion(+)
diff --git a/src/PVE/Network/SDN/Vnets.pm b/src/PVE/Network/SDN/Vnets.pm
index 4542b70..cbf0a07 100644
--- a/src/PVE/Network/SDN/Vnets.pm
+++ b/src/PVE/Network/SDN/Vnets.pm
@@ -118,6 +118,7 @@ sub add_next_free_cidr {
my $network = $subnet->{network};
next if Net::IP::ip_get_version($network) != $ipversion ||
$ips->{$ipversion};
+ next if !$subnet->{'dhcp-range'};
$subnetcount++;
eval {
--
2.44.0
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH v3 pve-network 02/12] sdn: dhcp: request both IPv4 and IPv6 addresses on VM start
If previously an IP was allocated in the IPAM, but a new subnet added
for the other IP version, we need to allocate an IP in the new subnet.
Signed-off-by: Stefan Lendl
Reviewed-by: Stefan Hanreich
Tested-by: Stefan Hanreich
---
src/PVE/Network/SDN/Vnets.pm | 10 --
1 file changed, 4 insertions(+), 6 deletions(-)
diff --git a/src/PVE/Network/SDN/Vnets.pm b/src/PVE/Network/SDN/Vnets.pm
index 03609b7..4542b70 100644
--- a/src/PVE/Network/SDN/Vnets.pm
+++ b/src/PVE/Network/SDN/Vnets.pm
@@ -196,12 +196,10 @@ sub add_dhcp_mapping {
return if !$zone->{ipam} || !$zone->{dhcp};
my ($ip4, $ip6) = PVE::Network::SDN::Vnets::get_ips_from_mac($vnetid,
$mac);
-if ( ! ($ip4 || $ip6) ) {
- print "No IP found for MAC: $mac for VMID:$vmid\n";
- add_next_free_cidr($vnetid, $name, $mac, "$vmid", undef, 1);
- ($ip4, $ip6) = PVE::Network::SDN::Vnets::get_ips_from_mac($vnetid,
$mac);
- print "got new IP from IPAM: $ip4 $ip6\n";
-}
+add_next_free_cidr($vnetid, $name, $mac, "$vmid", undef, 1, 4) if ! $ip4;
+add_next_free_cidr($vnetid, $name, $mac, "$vmid", undef, 1, 6) if ! $ip6;
+
+($ip4, $ip6) = PVE::Network::SDN::Vnets::get_ips_from_mac($vnetid, $mac);
PVE::Network::SDN::Dhcp::add_mapping($vnetid, $mac, $ip4, $ip6) if $ip4 ||
$ip6;
}
--
2.44.0
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] applied: [PATCH container v2 2/6] api: status: move config locking from API handler into worker
On 04/04/2024 17:26, Thomas Lamprecht wrote: > Oh, and it might be worth mentioning explicitly in the next release notes, > as it's a change in behavior that could theoretically throw up some > tooling that depends on the $action not failing due to locking if the > adapted endpoints returned – albeit IMO a bit silly to assume that (as the > actions can fail in other ways inside the worker anyway). True, I've made a note to mention this in the release notes. ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH guest-common v2 1/6] guest helpers: add helper to overrule active tasks of a specific type
Thanks for the review!
On 04/04/2024 17:20, Thomas Lamprecht wrote:
> Am 30/01/2024 um 18:10 schrieb Friedrich Weber:
>
> Maybe start of with "Add a helper to abort all tasks from a specific
> (type, user, vmid) tuple. It will be used ...
Will do.
>> This helper is used to abort any active qmshutdown/vzshutdown tasks
>> before attempting to stop a VM/CT (if requested).
>>
>> Signed-off-by: Friedrich Weber
>> ---
>>
>> Notes:
>> no changes v1 -> v2
>>
>> src/PVE/GuestHelpers.pm | 18 ++
>> 1 file changed, 18 insertions(+)
>>
>> diff --git a/src/PVE/GuestHelpers.pm b/src/PVE/GuestHelpers.pm
>> index 961a7b8..bd94ed2 100644
>> --- a/src/PVE/GuestHelpers.pm
>> +++ b/src/PVE/GuestHelpers.pm
>> @@ -416,4 +416,22 @@ sub check_vnet_access {
>> if !($tag || $trunks);
>> }
>>
>> +sub overrule_tasks {
>
> hmm, overruling is the thing you want to do now, but one might
> be use this for other reasons, so maybe naming it about what it
> does would be a bit better compared to what is used for (now).
>
> From top of my head that could be "abort_guest_tasks", but maybe
> you got a better idea.
Yeah, that's true, I'll rename it to `abort_guest_tasks` or some variation.
>> +my ($type, $user, $vmid) = @_;
>> +
>> +my $active_tasks = PVE::INotify::read_file('active');
>> +my $res = [];
>> +for my $task (@$active_tasks) {
>> + if (!$task->{saved}
>> + && $task->{type} eq $type
>> + && $task->{user} eq $user
>
> This also means that e.g. root@pam cannot overrule a task started by
> apprentice@pam, which might be something admins what to do (they like
> overruling users after all ;-P). Or some automation triggering the
> shutdown (using a token with a separate user) might be also a good
> examples of things I'd like to be able to overrule.
Good point. I see the usecase for being able to override other user's
tasks. My original motivation for making the user check that tight was
to avoid privilege escalation.
> Or does it even make sense to check this at all?
> As long as the user has the rights to execute a stop they probably
> should also be able to force it at any time, even for other users?
The usecase sounds reasonable, but would technically amount to a small
privilege escalation: Currently, if user A and user B both have
PVEVMUser and user A starts a `vzshutdown` task, user B must have
`Sys.Modify` to kill the task.
> Maybe make this optional and only enforce it for users that do not
> have some more powerful priv?
We could introduce a similar check as for the DELETE
/nodes/{node}/tasks/{upid} endpoint: Users can always overrule their own
guest tasks, and with `Sys.Modify` they can overrule any guest task (for
guests for which they have the necessary permission).
Still, right now I think the primary motivation for this overruling
feature is to save GUI users some frustration and/or clicks. In this
scenario, a user will overrule only their own tasks, which is possible
with the current check. What do you think about keeping the check as it
is for now, and making it more permissive once the need arises?
>> + && $task->{id} eq $vmid
>> + ) {
>
> meh, the pre-existing $killit param is way too subtle for my taste...
> Some %param that takes a `kill => "reason"` property for this would be
> much more telling.
>
> But changing that is a bit out of scope, a comment would be great for
> now though.
Makes sense, will add a comment.
>
>> + PVE::RPCEnvironment->check_worker($task->{upid}, 1);
>> + push @$res, $task->{upid};
>
> renaming $res to $killed_tasks would also help in reading this code out
> of further context.
Makes sense, will rename $res.
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH installer v4 00/30] add automated/unattended installation
I've tested mostly the same things as for v3 [0], to confirm nothing broke since that: - Using a few different values for `global` options - Install on ext4, xfs, Btrfs RAID1 and ZFS RAID10 (with different values in multiple runs) - Using DHCP and static IP - Fetching answer from a partition - Fetching answer from a HTTP source, getting the URL through DHCP or DNS - Trying out the `proxmox-autoinst-helper` tool for assembling udev rules and validating files. - Using the `post_command` to create some files in the newly installed system. - Tested with PVE, PMG and PBS, each w/ BIOS & UEFI (latter also w/ SB) One small thing I noticed: unknown/undefined options in the answer file are currently silently ignored - in the installer as well as by `proxmox-autoinst-helper validate-answer`. Something to implement in the future though definitely, but for now IMHO a rather mundane issue. Really just noting it here for reference. I can also confirm now that a small bug I found in [0] is now fixed, such that LVM configurations only allows a single disk now. The other things from [0] (and more) were also talked over again with Aaron directly, off-list. Also quickly skimmed over the actual changes again, looks fine overall. At least nothing to really note of; that would impact functionality and aren't some low-hanging fruit for the future (as e.g. noted above). So please consider this whole series: Tested-by: Christoph Heiss Reviewed-by: Christoph Heiss [0] https://lists.proxmox.com/pipermail/pve-devel/2024-April/062485.html On Thu, Apr 04, 2024 at 04:48:32PM +0200, Aaron Lauterer wrote: > This patch series adds the possibility to do an automated / unattended > installation of Proxmox VE. > > The overall idea is that we will have a dedicated ISO for the unattended > installation. It should be configured in such a way that it will start > the installation without any user interaction. Therefore, the GRUB > config should automatically start it (after a timeout). > > The information for the installer that is usually gathered interactively > from the user is provided via an `answer.toml` file. > > The answer file allows to select disks and the network card via filters. > > The installer also allows to run custom commands pre and post > installation. This should give users plenty of possibilities to either > further customize/prepare the installation or integrate it into a larger > automated installation setup. > For example, one could issue HTTP requests to signal the status and > progress of the installation. > > When the installer is called with 'proxauto' in the kernel cmdline, the > 'proxmox-fetch-answer' binary is called. It tries to find the answer > file and once found, will start the 'proxmox-auto-installer' binary and > pass the contents to it via stdin. > > The auto-installer then parses the answer file and determines what > parameters need to be passed to the low-level installer. For example, > which disks and NIC to use, network IP settings and so forth. > > The current status reporting of the actual installation is kept rather > simple. > > Both binaries log into the tmp directory. > > There is a third binary, the 'proxmox-autoinst-helper'. It provides a > few subcommands, from the help: > answerValidate if an answer file is formatted correctly > device-match Test which devices the given filter matches against > device-info Show device information that can be used for filters > identifiers Show identifiers for the current machine. This information is > part of the POST request to fetch an answer file > > The fetch-answer binary is trying to get an answer file. It does so by > first searching for a partition/FS labeled `proxmoxinst`, or all upper > case, and an `answer.toml` in there. This could be provided by another > USB flash drive. > If that is not successful, the next step is to send an HTTP POST request > to a URL to get the TOML contents in return. A POST request was chosen > because we also send information to identify the host in JSON format. > > The question then is, where to get that URL from. Right now, there are > two options implemented. The first is looking for a custom DHCP option > and the second is querying for a TXT record in the `proxmoxinst` > subdomain of the search domain. > > It is possible to provide a SHA256 fingerprint of the SSL cert used by > the answer server. The safest option is to place a > `cert_fingerprint.txt` file in the same `proxmoxinst` partition as where > you alternatively would place the `answer.toml`. > If that is not found, then it can be provided by a second custom DHCP > option or placed as TXT record in the subdomain `proxmoxinst-fp`. > > This patch series now also separates the 3 binaries into their own > crate. The 'proxmox-fetch-answer' to keep the OpenSSL dependency as > localized as possible, and the 'proxmox-autoinst-helper' to make it easy > to compile just that binary. > > The new `proxmox-chroot` utility helps to pr
[pve-devel] applied: [PATCH pve-kernel] revert 2 changes in thermal driver causing an early kernel Oops.
Am 05/04/2024 um 11:27 schrieb Stoiko Ivanov: > The second patch, that is reverted (first): > `thermal: trip: Drop lockdep assertion from thermal_zone_trip_id()` > only touches code introduced by the first patch. > The first patch causes the following Oops (reproduced on an old > HP DL380 G8): > ``` > [2.960519] ACPI: button: Power Button [PWRF] > [2.963126] BUG: kernel NULL pointer dereference, address: 000c > [2.965667] #PF: supervisor read access in kernel mode > [2.966954] #PF: error_code(0x) - not-present page > [2.966954] PGD 0 P4D 0 > [2.966954] Oops: [#1] PREEMPT SMP PTI > [2.966954] CPU: 0 PID: 1 Comm: swapper/0 Tainted: G I > 6.5.13-4-pve #1 > [2.966954] Hardware name: HP ProLiant DL380p Gen8, BIOS P70 05/24/2019 > [2.966954] RIP: 0010:step_wise_throttle+0x48/0x360 > [2.966954] Code: 04 25 28 00 00 00 48 89 45 d0 31 c0 48 63 c6 48 8d 14 40 > 48 8b 87 50 03 00 00 4c 8d 24 90 e8 cf d0 ff ff c6 45 bf 00 89 45 b4 <41> 8b > 04 24 41 39 85 78 03 00 00 0f 8d a9 02 00 00 0f 1f 44 00 00 > [2.966954] RSP: :9e2b8014bae8 EFLAGS: 00010246 > [2.966954] RAX: 0002 RBX: 0001 RCX: > > [2.966954] RDX: RSI: RDI: > > [2.966954] RBP: 9e2b8014bb40 R08: R09: > > [2.966954] R10: R11: R12: > 000c > [2.966954] R13: 8c7ac421d000 R14: 0001 R15: > > [2.966954] FS: () GS:8c7def60() > knlGS: > [2.966954] CS: 0010 DS: ES: CR0: 80050033 > [2.966954] CR2: 000c CR3: 000513a34001 CR4: > 000606f0 > [2.966954] Call Trace: > [2.966954] > ``` > > the relevant mainline kernels (6.6.15), corresponding to the > Ubuntu-patchset (which mixes changes from 6.6.15, with ones from > 6.1.76) [0] - also boot happily - so I strongly assume that the > changes depend on one of the many commits introduced in linux-upstream > between v6.5.1 and v6.6.1. > As it looks like a refactoring (upon which later changes are based), > and not a bug-fix in itself - simply dropping it seems sensible. > > Signed-off-by: Stoiko Ivanov > --- > ...rip-Drop-lockdep-assertion-from-ther.patch | 24 ++ > ...ore-Store-trip-pointer-in-struct-the.patch | 343 ++ > 2 files changed, 367 insertions(+) > create mode 100644 > patches/kernel/0014-Revert-thermal-trip-Drop-lockdep-assertion-from-ther.patch > create mode 100644 > patches/kernel/0015-Revert-thermal-core-Store-trip-pointer-in-struct-the.patch > > applied, thanks! ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH installer v4 13/30] auto-installer: add tests for answer file parsing
By matching the resulting json to be passed to the low level installer
against known good ones.
The environment info was gathered from one of our AMD Epyc Rome test
servers to have a realistic starting point.
Signed-off-by: Aaron Lauterer
---
proxmox-auto-installer/tests/parse-answer.rs | 106 ++
.../tests/resources/iso-info.json | 1 +
.../tests/resources/locales.json | 1 +
.../resources/parse_answer/disk_match.json| 29 +
.../resources/parse_answer/disk_match.toml| 17 +++
.../parse_answer/disk_match_all.json | 26 +
.../parse_answer/disk_match_all.toml | 17 +++
.../parse_answer/disk_match_any.json | 33 ++
.../parse_answer/disk_match_any.toml | 17 +++
.../tests/resources/parse_answer/minimal.json | 17 +++
.../tests/resources/parse_answer/minimal.toml | 14 +++
.../resources/parse_answer/nic_matching.json | 17 +++
.../resources/parse_answer/nic_matching.toml | 19
.../tests/resources/parse_answer/readme | 4 +
.../resources/parse_answer/specific_nic.json | 17 +++
.../resources/parse_answer/specific_nic.toml | 19
.../tests/resources/parse_answer/zfs.json | 27 +
.../tests/resources/parse_answer/zfs.toml | 20
.../tests/resources/run-env-info.json | 1 +
.../tests/resources/run-env-udev.json | 1 +
20 files changed, 403 insertions(+)
create mode 100644 proxmox-auto-installer/tests/parse-answer.rs
create mode 100644 proxmox-auto-installer/tests/resources/iso-info.json
create mode 100644 proxmox-auto-installer/tests/resources/locales.json
create mode 100644
proxmox-auto-installer/tests/resources/parse_answer/disk_match.json
create mode 100644
proxmox-auto-installer/tests/resources/parse_answer/disk_match.toml
create mode 100644
proxmox-auto-installer/tests/resources/parse_answer/disk_match_all.json
create mode 100644
proxmox-auto-installer/tests/resources/parse_answer/disk_match_all.toml
create mode 100644
proxmox-auto-installer/tests/resources/parse_answer/disk_match_any.json
create mode 100644
proxmox-auto-installer/tests/resources/parse_answer/disk_match_any.toml
create mode 100644
proxmox-auto-installer/tests/resources/parse_answer/minimal.json
create mode 100644
proxmox-auto-installer/tests/resources/parse_answer/minimal.toml
create mode 100644
proxmox-auto-installer/tests/resources/parse_answer/nic_matching.json
create mode 100644
proxmox-auto-installer/tests/resources/parse_answer/nic_matching.toml
create mode 100644 proxmox-auto-installer/tests/resources/parse_answer/readme
create mode 100644
proxmox-auto-installer/tests/resources/parse_answer/specific_nic.json
create mode 100644
proxmox-auto-installer/tests/resources/parse_answer/specific_nic.toml
create mode 100644 proxmox-auto-installer/tests/resources/parse_answer/zfs.json
create mode 100644 proxmox-auto-installer/tests/resources/parse_answer/zfs.toml
create mode 100644 proxmox-auto-installer/tests/resources/run-env-info.json
create mode 100644 proxmox-auto-installer/tests/resources/run-env-udev.json
diff --git a/proxmox-auto-installer/tests/parse-answer.rs
b/proxmox-auto-installer/tests/parse-answer.rs
new file mode 100644
index 000..c12520f
--- /dev/null
+++ b/proxmox-auto-installer/tests/parse-answer.rs
@@ -0,0 +1,106 @@
+use std::path::PathBuf;
+
+use serde_json::Value;
+use std::fs;
+
+use proxmox_auto_installer::answer;
+use proxmox_auto_installer::answer::Answer;
+use proxmox_auto_installer::udevinfo::UdevInfo;
+use proxmox_auto_installer::utils::parse_answer;
+
+use proxmox_installer_common::setup::{read_json, LocaleInfo, RuntimeInfo,
SetupInfo};
+
+fn get_test_resource_path() -> Result {
+Ok(std::env::current_dir()
+.expect("current dir failed")
+.join("tests/resources"))
+}
+fn get_answer(path: PathBuf) -> Result {
+let answer_raw = std::fs::read_to_string(&path).unwrap();
+let answer: answer::Answer = toml::from_str(&answer_raw)
+.map_err(|err| format!("error parsing answer.toml: {err}"))
+.unwrap();
+
+Ok(answer)
+}
+
+fn setup_test_basic(path: &PathBuf) -> (SetupInfo, LocaleInfo, RuntimeInfo,
UdevInfo) {
+let installer_info: SetupInfo = {
+let mut path = path.clone();
+path.push("iso-info.json");
+
+read_json(&path)
+.map_err(|err| format!("Failed to retrieve setup info: {err}"))
+.unwrap()
+};
+
+let locale_info = {
+let mut path = path.clone();
+path.push("locales.json");
+
+read_json(&path)
+.map_err(|err| format!("Failed to retrieve locale info: {err}"))
+.unwrap()
+};
+
+let mut runtime_info: RuntimeInfo = {
+let mut path = path.clone();
+path.push("run-env-info.json");
+
+read_json(&path)
+.map_err(|err| format!("Failed to retrieve runtime environment
info: {err}"))
+.unwrap()
+};
+
+let u
[pve-devel] [PATCH pve-kernel] revert 2 changes in thermal driver causing an early kernel Oops.
The second patch, that is reverted (first):
`thermal: trip: Drop lockdep assertion from thermal_zone_trip_id()`
only touches code introduced by the first patch.
The first patch causes the following Oops (reproduced on an old
HP DL380 G8):
```
[2.960519] ACPI: button: Power Button [PWRF]
[2.963126] BUG: kernel NULL pointer dereference, address: 000c
[2.965667] #PF: supervisor read access in kernel mode
[2.966954] #PF: error_code(0x) - not-present page
[2.966954] PGD 0 P4D 0
[2.966954] Oops: [#1] PREEMPT SMP PTI
[2.966954] CPU: 0 PID: 1 Comm: swapper/0 Tainted: G I
6.5.13-4-pve #1
[2.966954] Hardware name: HP ProLiant DL380p Gen8, BIOS P70 05/24/2019
[2.966954] RIP: 0010:step_wise_throttle+0x48/0x360
[2.966954] Code: 04 25 28 00 00 00 48 89 45 d0 31 c0 48 63 c6 48 8d 14 40
48 8b 87 50 03 00 00 4c 8d 24 90 e8 cf d0 ff ff c6 45 bf 00 89 45 b4 <41> 8b 04
24 41 39 85 78 03 00 00 0f 8d a9 02 00 00 0f 1f 44 00 00
[2.966954] RSP: :9e2b8014bae8 EFLAGS: 00010246
[2.966954] RAX: 0002 RBX: 0001 RCX:
[2.966954] RDX: RSI: RDI:
[2.966954] RBP: 9e2b8014bb40 R08: R09:
[2.966954] R10: R11: R12: 000c
[2.966954] R13: 8c7ac421d000 R14: 0001 R15:
[2.966954] FS: () GS:8c7def60()
knlGS:
[2.966954] CS: 0010 DS: ES: CR0: 80050033
[2.966954] CR2: 000c CR3: 000513a34001 CR4: 000606f0
[2.966954] Call Trace:
[2.966954]
```
the relevant mainline kernels (6.6.15), corresponding to the
Ubuntu-patchset (which mixes changes from 6.6.15, with ones from
6.1.76) [0] - also boot happily - so I strongly assume that the
changes depend on one of the many commits introduced in linux-upstream
between v6.5.1 and v6.6.1.
As it looks like a refactoring (upon which later changes are based),
and not a bug-fix in itself - simply dropping it seems sensible.
Signed-off-by: Stoiko Ivanov
---
...rip-Drop-lockdep-assertion-from-ther.patch | 24 ++
...ore-Store-trip-pointer-in-struct-the.patch | 343 ++
2 files changed, 367 insertions(+)
create mode 100644
patches/kernel/0014-Revert-thermal-trip-Drop-lockdep-assertion-from-ther.patch
create mode 100644
patches/kernel/0015-Revert-thermal-core-Store-trip-pointer-in-struct-the.patch
diff --git
a/patches/kernel/0014-Revert-thermal-trip-Drop-lockdep-assertion-from-ther.patch
b/patches/kernel/0014-Revert-thermal-trip-Drop-lockdep-assertion-from-ther.patch
new file mode 100644
index ..413b1641a4b1
--- /dev/null
+++
b/patches/kernel/0014-Revert-thermal-trip-Drop-lockdep-assertion-from-ther.patch
@@ -0,0 +1,24 @@
+From Mon Sep 17 00:00:00 2001
+From: Stoiko Ivanov
+Date: Thu, 4 Apr 2024 11:41:15 +0200
+Subject: [PATCH] Revert "thermal: trip: Drop lockdep assertion from
+ thermal_zone_trip_id()"
+
+This reverts commit c723c4fca6d2db3815623ff4dc0ea51667b56b89.
+---
+ drivers/thermal/thermal_trip.c | 2 ++
+ 1 file changed, 2 insertions(+)
+
+diff --git a/drivers/thermal/thermal_trip.c b/drivers/thermal/thermal_trip.c
+index 68bea8706c597..1d4fe63e09f77 100644
+--- a/drivers/thermal/thermal_trip.c
b/drivers/thermal/thermal_trip.c
+@@ -201,6 +201,8 @@ int thermal_zone_trip_id(struct thermal_zone_device *tz,
+ {
+ int i;
+
++ lockdep_assert_held(&tz->lock);
++
+ for (i = 0; i < tz->num_trips; i++) {
+ if (&tz->trips[i] == trip)
+ return i;
diff --git
a/patches/kernel/0015-Revert-thermal-core-Store-trip-pointer-in-struct-the.patch
b/patches/kernel/0015-Revert-thermal-core-Store-trip-pointer-in-struct-the.patch
new file mode 100644
index ..fe1ce3ed6632
--- /dev/null
+++
b/patches/kernel/0015-Revert-thermal-core-Store-trip-pointer-in-struct-the.patch
@@ -0,0 +1,343 @@
+From Mon Sep 17 00:00:00 2001
+From: Stoiko Ivanov
+Date: Thu, 4 Apr 2024 11:41:17 +0200
+Subject: [PATCH] Revert "thermal: core: Store trip pointer in struct
+ thermal_instance"
+
+This reverts commit 643b451957369f28b7770af387d14d4e4712074b.
+---
+ drivers/thermal/gov_bang_bang.c | 23 +++
+ drivers/thermal/gov_fair_share.c | 5 ++---
+ drivers/thermal/gov_power_allocator.c | 11 +++
+ drivers/thermal/gov_step_wise.c | 16 +---
+ drivers/thermal/thermal_core.c| 15 +--
+ drivers/thermal/thermal_core.h| 4 +---
+ drivers/thermal/thermal_helpers.c | 5 +
+ drivers/thermal/thermal_sysfs.c | 3 +--
+ drivers/thermal/thermal_trip.c| 15 ---
+ 9 files changed, 37 insertions(+), 60 deletions(-)
+
+diff --git a/drivers/thermal/gov_ban
