[pve-devel] [PATCH qemu-server v5 09/16] config: make special section handling generic

[Fiona Ebner <f.ebner@proxmox.com>]

Collect special sections below a common 'special-sections' key in
preparation to introduce a new special section.

The special 'cloudinit' section was added in the top-level of the
configuration structure, but it's cleaner to group special sections
more similar to snapshots.

The 'cloudinit' key was already initialized, so having the new
'special-sections' key be always initialized should not cause issues
after checking and adapting all usages of 'cloudinit' which this patch
attempts to do.

Add compat handling for remote migration which might receive the
configuration hash from a node that does not yet have the changes.

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
 PVE/API2/Qemu.pm            | 19 ++++++++++++++++---
 PVE/QemuConfig.pm           |  2 +-
 PVE/QemuServer.pm           | 30 +++++++++++++++++-------------
 PVE/QemuServer/Cloudinit.pm |  8 ++++++--
 4 files changed, 40 insertions(+), 19 deletions(-)

diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
index 295260e7..084e4efb 100644
--- a/PVE/API2/Qemu.pm
+++ b/PVE/API2/Qemu.pm
@@ -1654,7 +1654,7 @@ __PACKAGE__->register_method({
 	my $vmid = $param->{vmid};
 	my $conf = PVE::QemuConfig->load_config($vmid);
 
-	my $ci = $conf->{cloudinit};
+	my $ci = $conf->{'special-sections'}->{cloudinit};
 
 	$conf->{cipassword} = '**********' if exists $conf->{cipassword};
 	$ci->{cipassword} = '**********' if exists $ci->{cipassword};
@@ -5952,9 +5952,22 @@ __PACKAGE__->register_method({
 		    # not handled by update_vm_api
 		    my $vmgenid = delete $new_conf->{vmgenid};
 		    my $meta = delete $new_conf->{meta};
-		    my $cloudinit = delete $new_conf->{cloudinit}; # this is informational only
+
+		    my $special_sections = delete $new_conf->{'special-sections'} // {};
+
 		    $new_conf->{skip_cloud_init} = 1; # re-use image from source side
 
+		    # TODO PVE 10 - remove backwards-compat handling?
+		    my $cloudinit = delete $new_conf->{cloudinit};
+		    if ($cloudinit) {
+			if ($special_sections->{cloudinit}) {
+			    warn "config has duplicate special 'cloudinit' sections - skipping"
+				." legacy variant\n";
+			} else {
+			    $special_sections->{cloudinit} = $cloudinit;
+			}
+		    }
+
 		    $new_conf->{vmid} = $state->{vmid};
 		    $new_conf->{node} = $node;
 
@@ -5976,7 +5989,7 @@ __PACKAGE__->register_method({
 		    $conf->{lock} = 'migrate';
 		    $conf->{vmgenid} = $vmgenid if defined($vmgenid);
 		    $conf->{meta} = $meta if defined($meta);
-		    $conf->{cloudinit} = $cloudinit if defined($cloudinit);
+		    $conf->{'special-sections'} = $special_sections;
 		    PVE::QemuConfig->write_config($state->{vmid}, $conf);
 
 		    $state->{lock} = 'migrate';
diff --git a/PVE/QemuConfig.pm b/PVE/QemuConfig.pm
index ffdf9f03..3d57a0a8 100644
--- a/PVE/QemuConfig.pm
+++ b/PVE/QemuConfig.pm
@@ -541,7 +541,7 @@ sub load_current_config {
     my ($class, $vmid, $current) = @_;
 
     my $conf = $class->SUPER::load_current_config($vmid, $current);
-    delete $conf->{cloudinit};
+    delete $conf->{'special-sections'};
     return $conf;
 }
 
diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
index e1c9d3f5..5a55e70e 100644
--- a/PVE/QemuServer.pm
+++ b/PVE/QemuServer.pm
@@ -1849,7 +1849,7 @@ sub vmconfig_register_unused_drive {
     if (drive_is_cloudinit($drive)) {
 	eval { PVE::Storage::vdisk_free($storecfg, $drive->{file}) };
 	warn $@ if $@;
-	delete $conf->{cloudinit};
+	delete $conf->{'special-sections'}->{cloudinit};
     } elsif (!drive_is_cdrom($drive)) {
 	my $volid = $drive->{file};
 	if (vm_is_volid_owner($storecfg, $vmid, $volid)) {
@@ -2203,7 +2203,7 @@ sub parse_vm_config {
 	digest => Digest::SHA::sha1_hex($raw),
 	snapshots => {},
 	pending => {},
-	cloudinit => {},
+	'special-sections' => {},
     };
 
     my $handle_error = sub {
@@ -2230,6 +2230,9 @@ sub parse_vm_config {
 	}
 	$descr = undef;
     };
+
+    my $special_sections_re_1 = qr/(cloudinit)/;
+
     my $section = { name => '', type => 'main' };
 
     my @lines = split(/\n/, $raw);
@@ -2241,10 +2244,10 @@ sub parse_vm_config {
 	    $finish_description->();
 	    $conf = $res->{$section->{name}} = {};
 	    next;
-	} elsif ($line =~ m/^\[special:cloudinit\]\s*$/i) {
-	    $section = { name => 'cloudinit', type => 'special' };
+	} elsif ($line =~ m/^\[special:$special_sections_re_1\]\s*$/i) {
+	    $section = { name => $1, type => 'special' };
 	    $finish_description->();
-	    $conf = $res->{$section->{name}} = {};
+	    $conf = $res->{'special-sections'}->{$section->{name}} = {};
 	    next;
 
 	} elsif ($line =~ m/^\[([a-z][a-z0-9_\-]+)\]\s*$/i) {
@@ -2349,7 +2352,7 @@ sub write_vm_config {
 
 	foreach my $key (keys %$cref) {
 	    next if $key eq 'digest' || $key eq 'description' || $key eq 'snapshots' ||
-		$key eq 'snapstate' || $key eq 'pending' || $key eq 'cloudinit';
+		$key eq 'snapstate' || $key eq 'pending' || $key eq 'special-sections';
 	    my $value = $cref->{$key};
 	    if ($key eq 'delete') {
 		die "propertry 'delete' is only allowed in [PENDING]\n"
@@ -2403,7 +2406,7 @@ sub write_vm_config {
 	}
 
 	foreach my $key (sort keys %$conf) {
-	    next if $key =~ /^(digest|description|pending|cloudinit|snapshots)$/;
+	    next if $key =~ /^(digest|description|pending|snapshots|special-sections)$/;
 	    $raw .= "$key: $conf->{$key}\n";
 	}
 	return $raw;
@@ -2416,9 +2419,10 @@ sub write_vm_config {
 	$raw .= &$generate_raw_config($conf->{pending}, 1);
     }
 
-    if (scalar(keys %{$conf->{cloudinit}}) && PVE::QemuConfig->has_cloudinit($conf)){
-	$raw .= "\n[special:cloudinit]\n";
-	$raw .= &$generate_raw_config($conf->{cloudinit});
+    for my $special (sort keys $conf->{'special-sections'}->%*) {
+	next if $special eq 'cloudinit' && !PVE::QemuConfig->has_cloudinit($conf);
+	$raw .= "\n[special:$special]\n";
+	$raw .= &$generate_raw_config($conf->{'special-sections'}->{$special});
     }
 
     foreach my $snapname (sort keys %{$conf->{snapshots}}) {
@@ -4767,7 +4771,7 @@ sub vmconfig_hotplug_pending {
 	my ($conf, $opt, $old, $new) = @_;
 	return if !$cloudinit_pending_properties->{$opt};
 
-	my $ci = ($conf->{cloudinit} //= {});
+	my $ci = ($conf->{'special-sections'}->{cloudinit} //= {});
 
 	my $recorded = $ci->{$opt};
 	my %added = map { $_ => 1 } PVE::Tools::split_list(delete($ci->{added}) // '');
@@ -5157,7 +5161,7 @@ sub vmconfig_apply_pending {
     if ($generate_cloudinit) {
 	if (PVE::QemuServer::Cloudinit::apply_cloudinit_config($conf, $vmid)) {
 	    # After successful generation and if there were changes to be applied, update the
-	    # config to drop the {cloudinit} entry.
+	    # config to drop the 'cloudinit' special section.
 	    PVE::QemuConfig->write_config($vmid, $conf);
 	}
     }
@@ -5588,7 +5592,7 @@ sub vm_start_nolock {
     if (!$migratedfrom) {
 	if (PVE::QemuServer::Cloudinit::apply_cloudinit_config($conf, $vmid)) {
 	    # FIXME: apply_cloudinit_config updates $conf in this case, and it would only drop
-	    # $conf->{cloudinit}, so we could just not do this?
+	    # $conf->{'special-sections'}->{cloudinit}, so we could just not do this?
 	    # But we do it above, so for now let's be consistent.
 	    $conf = PVE::QemuConfig->load_config($vmid); # update/reload
 	}
diff --git a/PVE/QemuServer/Cloudinit.pm b/PVE/QemuServer/Cloudinit.pm
index f1143aeb..001022e6 100644
--- a/PVE/QemuServer/Cloudinit.pm
+++ b/PVE/QemuServer/Cloudinit.pm
@@ -657,7 +657,11 @@ my $cloudinit_methods = {
 sub has_changes {
     my ($conf) = @_;
 
-    return !!$conf->{cloudinit}->%*;
+    if (my $cloudinit = $conf->{'special-sections'}->{cloudinit}) {
+	return !!$cloudinit->%*;
+    }
+
+    return;
 }
 
 sub generate_cloudinit_config {
@@ -689,7 +693,7 @@ sub apply_cloudinit_config {
     my $has_changes = generate_cloudinit_config($conf, $vmid);
 
     if ($has_changes) {
-	delete $conf->{cloudinit};
+	delete $conf->{'special-sections'}->{cloudinit};
 	PVE::QemuConfig->write_config($vmid, $conf);
 	return 1;
     }
-- 
2.39.5



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