[pve-devel] [PATCH v7 qemu-server 00/10] cloudinit pending behaviour change

[pve-devel] [PATCH v7 qemu-server 00/10] cloudinit pending behaviour change

[Alexandre Derumier]

Hi,

This is an attempt to cleanup current behaviour of cloudinit online changes.

Currently, we setup cloudinit options as pending, until we generate the config drive.

This is not 100% true, because some option like vm name, nic mac address can be changed,
without going to pending, so user can't known if it need to regenerated it.

Also, some can are very difficult to handle, if you hotplug a nic but it's failing,so pending,
then you defined an ipconfig, and then you revert hotplug.
or if you delete a nic, the ipconfig is no more displayed in the gui.


So, instead of setting cloudinit values in pending,
this patch serie copy the current cloudinit config in a new section [special:cloudinit],
when the config drive is generated.
This is only an hint, to allow to display diff between the generated cloudinit
drive, and the current vm config.

A new specific cloudinit config api is added too displaying the diff between current and generated config.

Reminder: This need pve-manager depency bump first to check the version for live migration.
pve-manager patches series is available here : https://lists.proxmox.com/pipermail/pve-devel/2021-June/048542.html

(I need to small fix to display nic/mac info, I'll try to fix it today)


Changelog V1:

- use [special:cloudinit] instead [CLOUDINIT] for section
- delete config section on drive removal
- config api: move code to new PVE::QemuServer::Cloudinit::get_pending_config
- config api: add "qm cloudinit pending" cli
- add update api to regenerate drive with 1 api call
- add cloudinit hotplug option

Changelog v2:

- fix trailing whitespace in first patch
- revert previous "cloudinit" check in snapshot name (":" character is already forbidden)

Changelog v3:

- extract the current conf from cloudinit drive instead write the special cloudinit section

Changelog v4:

- rebase on v2, keep current cloudinit config in vm configuration
- pending api: display mac address change on netX
- cleanup && fix from Fabian comments

Changelog v5:

- move cloudinit fast_plug_option generation outside vmconfig_hotplug_pending
- remove cloudinit section from vm_config api
- vzdump : skip cloudinit section 
- migration: check target node version && forbid migration if too old && cloudinit section exist.

Changelog v6:

- move last 3 patches at beginning of the patch series
- api config: remove cloudinit in load_current_config
- migration: move the 2 helpers in PVE::QemuServer::Helpers + code cleanup


Changelog v7:

- code cleanup
- add an extra patch for debian control to break pve-manager <= 7.2.4

Alexandre Derumier (10):
  qemuconfig: load_current_config : delete cloudinit value
  vzdump : skip special:cloudinit section
  migration: test targetnode min version for cloudinit section
  cloudinit: add cloudinit section for current generated config.
  generate cloudinit drive on offline plug
  cloudinit: make cloudnit options fastplug
  api2: add cloudinit config api
  api2: add cloudinit_update
  add cloudinit hotplug
  debian : control : add a break on pve-manager <= 7.2.4

 PVE/API2/Qemu.pm            | 111 ++++++++++++++++++++++++++++++++++++
 PVE/CLI/qm.pm               |   2 +
 PVE/QemuConfig.pm           |   8 +++
 PVE/QemuMigrate.pm          |   8 +++
 PVE/QemuServer.pm           | 100 ++++++++++++++++++++++----------
 PVE/QemuServer/Cloudinit.pm | 110 +++++++++++++++++++++++++++++++++++
 PVE/QemuServer/Helpers.pm   |  24 ++++++++
 PVE/VZDump/QemuServer.pm    |   5 +-
 debian/control              |   2 +-
 9 files changed, 337 insertions(+), 33 deletions(-)

-- 
2.30.2

[pve-devel] [PATCH v7 qemu-server 01/10] qemuconfig: load_current_config : delete cloudinit value

[Alexandre Derumier]

Signed-off-by: Alexandre Derumier <aderumier@odiso.com>
---
 PVE/QemuConfig.pm | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/PVE/QemuConfig.pm b/PVE/QemuConfig.pm
index cfef8d3..d912456 100644
--- a/PVE/QemuConfig.pm
+++ b/PVE/QemuConfig.pm
@@ -516,6 +516,14 @@ sub __snapshot_rollback_get_unused {
     return $unused;
 }
 
+sub load_current_config {
+    my ($class, $vmid, $current) = @_;
+
+    my $conf = $class->SUPER::load_current_config($vmid, $current);
+    delete $conf->{cloudinit};
+    return $conf;
+}
+
 # END implemented abstract methods from PVE::AbstractConfig
 
 1;
-- 
2.30.2

[pve-devel] [PATCH v7 qemu-server 02/10] vzdump : skip special:cloudinit section

[Alexandre Derumier]

Signed-off-by: Alexandre Derumier <aderumier@odiso.com>
---
 PVE/VZDump/QemuServer.pm | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/PVE/VZDump/QemuServer.pm b/PVE/VZDump/QemuServer.pm
index 7429508..1d38e63 100644
--- a/PVE/VZDump/QemuServer.pm
+++ b/PVE/VZDump/QemuServer.pm
@@ -214,17 +214,20 @@ sub assemble {
 
     my $found_snapshot;
     my $found_pending;
+    my $found_cloudinit;
     while (defined (my $line = <$conffd>)) {
 	next if $line =~ m/^\#vzdump\#/; # just to be sure
 	next if $line =~ m/^\#qmdump\#/; # just to be sure
 	if ($line =~ m/^\[(.*)\]\s*$/) {
 	    if ($1 =~ m/PENDING/i) {
 		$found_pending = 1;
+	    } elsif ($1 =~ m/special:cloudinit/) {
+		$found_cloudinit = 1;
 	    } else {
 		$found_snapshot = 1;
 	    }
 	}
-	next if $found_snapshot || $found_pending; # skip all snapshots and pending changes config data
+	next if $found_snapshot || $found_pending || $found_cloudinit; # skip all snapshots,pending changes and cloudinit config data
 
 	if ($line =~ m/^unused\d+:\s*(\S+)\s*/) {
 	    $self->loginfo("skip unused drive '$1' (not included into backup)");
-- 
2.30.2

[pve-devel] [PATCH v7 qemu-server 03/10] migration: test targetnode min version for cloudinit section

[Alexandre Derumier]

Signed-off-by: Alexandre Derumier <aderumier@odiso.com>
---
 PVE/QemuMigrate.pm        |  8 ++++++++
 PVE/QemuServer/Helpers.pm | 24 ++++++++++++++++++++++++
 2 files changed, 32 insertions(+)

diff --git a/PVE/QemuMigrate.pm b/PVE/QemuMigrate.pm
index d52dc8d..b7e3d30 100644
--- a/PVE/QemuMigrate.pm
+++ b/PVE/QemuMigrate.pm
@@ -122,6 +122,14 @@ sub prepare {
     # test if VM exists
     my $conf = $self->{vmconf} = PVE::QemuConfig->load_config($vmid);
 
+    my $version = PVE::QemuServer::Helpers::get_node_pvecfg_version($self->{node});
+    my $cloudinit_config = $conf->{cloudinit};
+
+    if (defined($cloudinit_config) && keys %$cloudinit_config &&
+	!PVE::QemuServer::Helpers::pvecfg_min_version($version, 7, 2, 5)) {
+	die "target node is too old and doesn't support new cloudinit section\n";
+    }
+
     my $repl_conf = PVE::ReplicationConfig->new();
     $self->{replication_jobcfg} = $repl_conf->find_local_replication_job($vmid, $self->{node});
     $self->{is_replicated} = $repl_conf->check_for_existing_jobs($vmid, 1);
diff --git a/PVE/QemuServer/Helpers.pm b/PVE/QemuServer/Helpers.pm
index c10d842..a2ba20a 100644
--- a/PVE/QemuServer/Helpers.pm
+++ b/PVE/QemuServer/Helpers.pm
@@ -4,6 +4,7 @@ use strict;
 use warnings;
 
 use File::stat;
+use JSON;
 
 use PVE::INotify;
 use PVE::ProcFSTools;
@@ -161,4 +162,27 @@ sub config_aware_timeout {
     return $timeout;
 }
 
+sub get_node_pvecfg_version {
+    my ($node) = @_;
+
+    my $nodes_version_info = PVE::Cluster::get_node_kv('version-info', $node);
+    return if !$nodes_version_info->{$node};
+
+    my $version_info = decode_json($nodes_version_info->{$node});
+    return $version_info->{version};
+}
+
+sub pvecfg_min_version {
+    my ($verstr, $major, $minor, $release) = @_;
+
+    return 0 if !$verstr;
+
+    if ($verstr =~ m/^(\d+)\.(\d+)-(\d+)/) {
+	return 1 if version_cmp($1, $major, $2, $minor, $3, $release) >= 0;
+	return 0;
+    }
+
+    die "internal error: cannot check version of invalid string '$verstr'";
+}
+
 1;
-- 
2.30.2

[pve-devel] [PATCH v7 qemu-server 04/10] cloudinit: add cloudinit section for current generated config.

[Alexandre Derumier]

Instead using vm pending options for pending cloudinit generated config,

write current generated cloudinit config in a new [special:cloudinit] SECTION.

Currently, some options like vm name, nic mac address can be hotplugged,
so they are not way to know if the cloud-init disk is already updated.

Signed-off-by: Alexandre Derumier <aderumier@odiso.com>
---
 PVE/QemuServer.pm           | 18 ++++++++++++++++--
 PVE/QemuServer/Cloudinit.pm | 31 +++++++++++++++++++++++++++++++
 2 files changed, 47 insertions(+), 2 deletions(-)

diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
index b3964bc..6cecf04 100644
--- a/PVE/QemuServer.pm
+++ b/PVE/QemuServer.pm
@@ -2003,6 +2003,7 @@ sub vmconfig_register_unused_drive {
     if (drive_is_cloudinit($drive)) {
 	eval { PVE::Storage::vdisk_free($storecfg, $drive->{file}) };
 	warn $@ if $@;
+	delete $conf->{cloudinit};
     } elsif (!drive_is_cdrom($drive)) {
 	my $volid = $drive->{file};
 	if (vm_is_volid_owner($storecfg, $vmid, $volid)) {
@@ -2373,6 +2374,7 @@ sub parse_vm_config {
 	digest => Digest::SHA::sha1_hex($raw),
 	snapshots => {},
 	pending => {},
+	cloudinit => {},
     };
 
     my $handle_error = sub {
@@ -2407,6 +2409,11 @@ sub parse_vm_config {
 	    $descr = undef;
 	    $conf = $res->{$section} = {};
 	    next;
+	} elsif ($line =~ m/^\[special:cloudinit\]\s*$/i) {
+	    $section = 'cloudinit';
+	    $descr = undef;
+	    $conf = $res->{$section} = {};
+	    next;
 
 	} elsif ($line =~ m/^\[([a-z][a-z0-9_\-]+)\]\s*$/i) {
 	    $section = $1;
@@ -2504,7 +2511,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 'snapstate' || $key eq 'pending' || $key eq 'cloudinit';
 	    my $value = $cref->{$key};
 	    if ($key eq 'delete') {
 		die "propertry 'delete' is only allowed in [PENDING]\n"
@@ -2528,6 +2535,8 @@ sub write_vm_config {
 
     &$cleanup_config($conf->{pending}, 1);
 
+    &$cleanup_config($conf->{cloudinit});
+
     foreach my $snapname (keys %{$conf->{snapshots}}) {
 	die "internal error: snapshot name '$snapname' is forbidden" if lc($snapname) eq 'pending';
 	&$cleanup_config($conf->{snapshots}->{$snapname}, undef, $snapname);
@@ -2558,7 +2567,7 @@ sub write_vm_config {
 	}
 
 	foreach my $key (sort keys %$conf) {
-	    next if $key =~ /^(digest|description|pending|snapshots)$/;
+	    next if $key =~ /^(digest|description|pending|cloudinit|snapshots)$/;
 	    $raw .= "$key: $conf->{$key}\n";
 	}
 	return $raw;
@@ -2571,6 +2580,11 @@ sub write_vm_config {
 	$raw .= &$generate_raw_config($conf->{pending}, 1);
     }
 
+    if (scalar(keys %{$conf->{cloudinit}})){
+	$raw .= "\n[special:cloudinit]\n";
+	$raw .= &$generate_raw_config($conf->{cloudinit});
+    }
+
     foreach my $snapname (sort keys %{$conf->{snapshots}}) {
 	$raw .= "\n[$snapname]\n";
 	$raw .= &$generate_raw_config($conf->{snapshots}->{$snapname});
diff --git a/PVE/QemuServer/Cloudinit.pm b/PVE/QemuServer/Cloudinit.pm
index b7daa2a..cdaf4e5 100644
--- a/PVE/QemuServer/Cloudinit.pm
+++ b/PVE/QemuServer/Cloudinit.pm
@@ -574,6 +574,37 @@ sub generate_cloudinitconfig {
 
 	$generator->($conf, $vmid, $drive, $volname, $storeid);
     });
+
+    my $cloudinitconf = delete $conf->{cloudinit};
+    $cloudinitconf = {};
+
+    my @cloudinit_opts = keys %{PVE::QemuServer::cloudinit_config_properties()};
+    push @cloudinit_opts, 'name';
+
+    for my $opt (@cloudinit_opts) {
+
+	if ($opt =~ m/^ipconfig(\d+)/) {
+	    my $netid = "net$1";
+	    next if !defined($conf->{$netid});
+	    $conf->{cloudinit}->{$netid} = $conf->{$netid};
+	}
+
+	$conf->{cloudinit}->{$opt} = $conf->{$opt} if $conf->{$opt};
+    }
+
+    $conf->{cloudinit}->{name} = "VM$vmid" if !$conf->{cloudinit}->{name};
+
+    for my $opt (keys %{$conf}) {
+	if (PVE::QemuServer::is_valid_drivename($opt)) {
+	    my $drive = PVE::QemuServer::parse_drive($opt, $conf->{$opt});
+	    if (PVE::QemuServer::drive_is_cloudinit($drive)) {
+		$conf->{cloudinit}->{$opt} = $conf->{$opt};
+	    }
+	}
+    }
+
+    PVE::QemuConfig->write_config($vmid, $conf);
+
 }
 
 sub dump_cloudinit_config {
-- 
2.30.2

[pve-devel] [PATCH v7 qemu-server 05/10] generate cloudinit drive on offline plug

[Alexandre Derumier]

Currently when only generate it at vm start

Signed-off-by: Alexandre Derumier <aderumier@odiso.com>
---
 PVE/QemuServer.pm | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
index 6cecf04..6310a41 100644
--- a/PVE/QemuServer.pm
+++ b/PVE/QemuServer.pm
@@ -5103,6 +5103,8 @@ sub vmconfig_apply_pending {
 
     PVE::QemuConfig->cleanup_pending($conf);
 
+    my $generate_cloudnit = undef;
+
     foreach my $opt (keys %{$conf->{pending}}) { # add/change
 	next if $opt eq 'delete'; # just to be sure
 	eval {
@@ -5113,12 +5115,19 @@ sub vmconfig_apply_pending {
 	if (my $err = $@) {
 	    $add_apply_error->($opt, $err);
 	} else {
+
+	    if (is_valid_drivename($opt)) {
+		my $drive = parse_drive($opt, $conf->{pending}->{$opt});
+		$generate_cloudnit = 1 if drive_is_cloudinit($drive);
+	    }
+
 	    $conf->{$opt} = delete $conf->{pending}->{$opt};
 	}
     }
 
     # write all changes at once to avoid unnecessary i/o
     PVE::QemuConfig->write_config($vmid, $conf);
+    PVE::QemuServer::Cloudinit::generate_cloudinitconfig($conf, $vmid) if $generate_cloudnit;
 }
 
 sub vmconfig_update_net {
-- 
2.30.2

[pve-devel] [PATCH v7 qemu-server 06/10] cloudinit: make cloudnit options fastplug

[Alexandre Derumier]

Signed-off-by: Alexandre Derumier <aderumier@odiso.com>
---
 PVE/QemuServer.pm | 31 +++++--------------------------
 1 file changed, 5 insertions(+), 26 deletions(-)

diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
index 6310a41..818285b 100644
--- a/PVE/QemuServer.pm
+++ b/PVE/QemuServer.pm
@@ -4832,6 +4832,10 @@ my $fast_plug_option = {
     'tags' => 1,
 };
 
+for my $opt (keys %$confdesc_cloudinit) {
+    $fast_plug_option->{$opt} = 1;
+};
+
 # hotplug changes in [PENDING]
 # $selection hash can be used to only apply specified options, for
 # example: { cores => 1 } (only apply changed 'cores')
@@ -4927,31 +4931,6 @@ sub vmconfig_hotplug_pending {
 	}
     }
 
-    my ($apply_pending_cloudinit, $apply_pending_cloudinit_done);
-    $apply_pending_cloudinit = sub {
-	return if $apply_pending_cloudinit_done; # once is enough
-	$apply_pending_cloudinit_done = 1; # once is enough
-
-	my ($key, $value) = @_;
-
-	my @cloudinit_opts = keys %$confdesc_cloudinit;
-	foreach my $opt (keys %{$conf->{pending}}) {
-	    next if !grep { $_ eq $opt } @cloudinit_opts;
-	    $conf->{$opt} = delete $conf->{pending}->{$opt};
-	}
-
-	my $pending_delete_hash = PVE::QemuConfig->parse_pending_delete($conf->{pending}->{delete});
-	foreach my $opt (sort keys %$pending_delete_hash) {
-	    next if !grep { $_ eq $opt } @cloudinit_opts;
-	    PVE::QemuConfig->remove_from_pending_delete($conf, $opt);
-	    delete $conf->{$opt};
-	}
-
-	my $new_conf = { %$conf };
-	$new_conf->{$key} = $value;
-	PVE::QemuServer::Cloudinit::generate_cloudinitconfig($new_conf, $vmid);
-    };
-
     foreach my $opt (keys %{$conf->{pending}}) {
 	next if $selection && !$selection->{$opt};
 	my $value = $conf->{pending}->{$opt};
@@ -4998,7 +4977,7 @@ sub vmconfig_hotplug_pending {
 		# some changes can be done without hotplug
 		my $drive = parse_drive($opt, $value);
 		if (drive_is_cloudinit($drive)) {
-		    &$apply_pending_cloudinit($opt, $value);
+		    PVE::QemuServer::Cloudinit::generate_cloudinitconfig($conf, $vmid);
 		}
 		vmconfig_update_disk($storecfg, $conf, $hotplug_features->{disk},
 				     $vmid, $opt, $value, $arch, $machine_type);
-- 
2.30.2

[pve-devel] [PATCH v7 qemu-server 07/10] api2: add cloudinit config api

[Alexandre Derumier]

Signed-off-by: Alexandre Derumier <aderumier@odiso.com>
---
 PVE/API2/Qemu.pm            | 68 +++++++++++++++++++++++++++++++
 PVE/CLI/qm.pm               |  1 +
 PVE/QemuServer/Cloudinit.pm | 79 +++++++++++++++++++++++++++++++++++++
 3 files changed, 148 insertions(+)

diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
index 99b426e..17d912c 100644
--- a/PVE/API2/Qemu.pm
+++ b/PVE/API2/Qemu.pm
@@ -21,6 +21,7 @@ use PVE::ReplicationConfig;
 use PVE::GuestHelpers;
 use PVE::QemuConfig;
 use PVE::QemuServer;
+use PVE::QemuServer::Cloudinit;
 use PVE::QemuServer::CPUConfig;
 use PVE::QemuServer::Drive;
 use PVE::QemuServer::ImportDisk;
@@ -1289,6 +1290,73 @@ __PACKAGE__->register_method({
 	return PVE::GuestHelpers::config_with_pending_array($conf, $pending_delete_hash);
    }});
 
+__PACKAGE__->register_method({
+    name => 'cloudinit_pending',
+    path => '{vmid}/cloudinit',
+    method => 'GET',
+    proxyto => 'node',
+    description => "Get the cloudinit configuration with both current and pending values.",
+    permissions => {
+	check => ['perm', '/vms/{vmid}', [ 'VM.Audit' ]],
+    },
+    parameters => {
+	additionalProperties => 0,
+	properties => {
+	    node => get_standard_option('pve-node'),
+	    vmid => get_standard_option('pve-vmid', { completion => \&PVE::QemuServer::complete_vmid }),
+	},
+    },
+    returns => {
+	type => "array",
+	items => {
+	    type => "object",
+	    properties => {
+		key => {
+		    description => "Configuration option name.",
+		    type => 'string',
+		},
+		value => {
+		    description => "Current value.",
+		    type => 'string',
+		    optional => 1,
+		},
+		pending => {
+		    description => "Pending value.",
+		    type => 'string',
+		    optional => 1,
+		},
+		delete => {
+		    description => "Indicates a pending delete request if present and not 0. " .
+		                   "The value 2 indicates a force-delete request.",
+		    type => 'integer',
+		    minimum => 0,
+		    maximum => 2,
+		    optional => 1,
+		},
+	    },
+	},
+    },
+    code => sub {
+	my ($param) = @_;
+
+	my $vmid = $param->{vmid};
+	my $conf = PVE::QemuConfig->load_config($vmid);
+
+	if (defined($conf->{cipassword}) &&
+	    defined($conf->{cloudinit}->{cipassword}) &&
+	    $conf->{cipassword} ne $conf->{cloudinit}->{cipassword}) {
+	    $conf->{cipassword} = '********** ';
+	} elsif (defined($conf->{cipassword})) {
+	    $conf->{cipassword} = '**********';
+	}
+
+	$conf->{cloudinit}->{cipassword} = '**********' if defined($conf->{cloudinit}->{cipassword});
+
+	my $res = PVE::QemuServer::Cloudinit::get_pending_config($conf, $vmid);
+
+	return $res;
+   }});
+
 # POST/PUT {vmid}/config implementation
 #
 # The original API used PUT (idempotent) an we assumed that all operations
diff --git a/PVE/CLI/qm.pm b/PVE/CLI/qm.pm
index 6a2e161..f7336a7 100755
--- a/PVE/CLI/qm.pm
+++ b/PVE/CLI/qm.pm
@@ -986,6 +986,7 @@ our $cmddef = {
 		my $data = shift;
 		print "$data\n";
 	    }],
+	pending => [ "PVE::API2::Qemu", 'cloudinit_pending', ['vmid'], { node => $nodename }, \&PVE::GuestHelpers::format_pending ]
     },
 
 };
diff --git a/PVE/QemuServer/Cloudinit.pm b/PVE/QemuServer/Cloudinit.pm
index cdaf4e5..3e93692 100644
--- a/PVE/QemuServer/Cloudinit.pm
+++ b/PVE/QemuServer/Cloudinit.pm
@@ -7,6 +7,7 @@ use File::Path;
 use Digest::SHA;
 use URI::Escape;
 use MIME::Base64 qw(encode_base64);
+use Storable qw(dclone);
 
 use PVE::Tools qw(run_command file_set_contents);
 use PVE::Storage;
@@ -632,4 +633,82 @@ sub dump_cloudinit_config {
     }
 }
 
+sub get_pending_config {
+    my ($conf, $vmid) = @_;
+
+    my $newconf = dclone($conf);
+
+    my $cloudinit_current = $newconf->{cloudinit};
+    my @cloudinit_opts = keys %{PVE::QemuServer::cloudinit_config_properties()};
+    push @cloudinit_opts, 'name';
+
+    #add cloud-init drive
+    my $drives = {};
+    PVE::QemuConfig->foreach_volume($newconf, sub {
+	my ($ds, $drive) = @_;
+	$drives->{$ds} = 1 if PVE::QemuServer::drive_is_cloudinit($drive);
+    });
+
+    PVE::QemuConfig->foreach_volume($cloudinit_current, sub {
+	my ($ds, $drive) = @_;
+	$drives->{$ds} = 1 if PVE::QemuServer::drive_is_cloudinit($drive);
+    });
+    for my $ds (keys %{$drives}) {
+	push @cloudinit_opts, $ds;
+    }
+
+    $newconf->{name} = "VM$vmid" if !$newconf->{name};
+    $cloudinit_current->{name} = "VM$vmid" if !$cloudinit_current->{name};
+
+    #only mac-address is used in cloud-init config.
+    #We don't want to display other pending net changes.
+    my $print_cloudinit_net = sub {
+	my ($conf, $opt) = @_;
+
+	if (defined($conf->{$opt})) {
+	    my $net = PVE::QemuServer::parse_net($conf->{$opt});
+	    $conf->{$opt} = "macaddr=".$net->{macaddr} if $net->{macaddr};
+	}
+    };
+
+    my $cloudinit_options = {};
+    for my $opt (@cloudinit_opts) {
+	if ($opt =~ m/^ipconfig(\d+)/) {
+	    my $netid = "net$1";
+
+	    next if !defined($newconf->{$netid}) && !defined($cloudinit_current->{$netid}) &&
+		    !defined($newconf->{$opt}) && !defined($cloudinit_current->{$opt});
+
+	    &$print_cloudinit_net($newconf, $netid);
+	    &$print_cloudinit_net($cloudinit_current, $netid);
+	    $cloudinit_options->{$netid} = 1;
+	}
+	$cloudinit_options->{$opt} = 1;
+    }
+
+    my $res = [];
+
+    for my $opt (keys %{$cloudinit_options}) {
+
+	my $item = {
+	    key => $opt,
+	};
+	if ($cloudinit_current->{$opt}) {
+	    $item->{value} = $cloudinit_current->{$opt};
+	    if (defined($newconf->{$opt})) {
+		$item->{pending} = $newconf->{$opt} 
+		    if $newconf->{$opt} ne $cloudinit_current->{$opt};
+	    } else {
+		$item->{delete} = 1;
+	    }
+	} else {
+	    $item->{pending} = $newconf->{$opt} if $newconf->{$opt}
+	}
+
+	push @$res, $item;
+   }
+
+   return $res;
+}
+
 1;
-- 
2.30.2

[pve-devel] [PATCH v7 qemu-server 08/10] api2: add cloudinit_update

[Alexandre Derumier]

This allow to regenerate the config drive with 1 api call.

This also avoid to delete drive first, and recreate it again.

As it's a readonly drive, we can simply live update it,
and eject/replace it with qemu monitor

Signed-off-by: Alexandre Derumier <aderumier@odiso.com>
---
 PVE/API2/Qemu.pm  | 43 +++++++++++++++++++++++++++++++++++++++++++
 PVE/CLI/qm.pm     |  3 ++-
 PVE/QemuServer.pm | 28 ++++++++++++++++++++++++++++
 3 files changed, 73 insertions(+), 1 deletion(-)

diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
index 17d912c..6142504 100644
--- a/PVE/API2/Qemu.pm
+++ b/PVE/API2/Qemu.pm
@@ -1357,6 +1357,49 @@ __PACKAGE__->register_method({
 	return $res;
    }});
 
+__PACKAGE__->register_method({
+    name => 'cloudinit_update',
+    path => '{vmid}/cloudinit',
+    method => 'PUT',
+    protected => 1,
+    proxyto => 'node',
+    description => "Regenerate and change cloudinit config drive.",
+    permissions => {
+	check => ['perm', '/vms/{vmid}', 'VM.Config.Cloudinit'],
+    },
+    parameters => {
+	additionalProperties => 0,
+	properties => {
+	    node => get_standard_option('pve-node'),
+	    vmid => get_standard_option('pve-vmid'),
+	},
+    },
+    returns => { type => 'null' },
+    code => sub {
+	my ($param) = @_;
+
+	my $rpcenv = PVE::RPCEnvironment::get();
+
+	my $authuser = $rpcenv->get_user();
+
+	my $vmid = extract_param($param, 'vmid');
+
+	my $updatefn =  sub {
+
+	    my $conf = PVE::QemuConfig->load_config($vmid);
+
+	    PVE::QemuConfig->check_lock($conf);
+
+	    my $storecfg = PVE::Storage::config();
+
+	    PVE::QemuServer::vmconfig_update_cloudinit_drive($storecfg, $conf, $vmid);
+	};
+
+	PVE::QemuConfig->lock_config($vmid, $updatefn);
+
+	return;
+    }});
+
 # POST/PUT {vmid}/config implementation
 #
 # The original API used PUT (idempotent) an we assumed that all operations
diff --git a/PVE/CLI/qm.pm b/PVE/CLI/qm.pm
index f7336a7..0818105 100755
--- a/PVE/CLI/qm.pm
+++ b/PVE/CLI/qm.pm
@@ -986,7 +986,8 @@ our $cmddef = {
 		my $data = shift;
 		print "$data\n";
 	    }],
-	pending => [ "PVE::API2::Qemu", 'cloudinit_pending', ['vmid'], { node => $nodename }, \&PVE::GuestHelpers::format_pending ]
+	pending => [ "PVE::API2::Qemu", 'cloudinit_pending', ['vmid'], { node => $nodename }, \&PVE::GuestHelpers::format_pending ],
+	update => [ "PVE::API2::Qemu", 'cloudinit_update', ['vmid'], { node => $nodename }],
     },
 
 };
diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
index 818285b..278a7b6 100644
--- a/PVE/QemuServer.pm
+++ b/PVE/QemuServer.pm
@@ -5293,6 +5293,34 @@ sub vmconfig_update_disk {
     vm_deviceplug($storecfg, $conf, $vmid, $opt, $drive, $arch, $machine_type);
 }
 
+sub vmconfig_update_cloudinit_drive {
+    my ($storecfg, $conf, $vmid) = @_;
+
+    my $cloudinit_ds = undef;
+    my $cloudinit_drive = undef;
+
+    PVE::QemuConfig->foreach_volume($conf, sub {
+	my ($ds, $drive) = @_;
+	if (PVE::QemuServer::drive_is_cloudinit($drive)) {
+	    $cloudinit_ds = $ds;
+	    $cloudinit_drive = $drive;
+	}
+    });
+
+    return if !$cloudinit_drive;
+
+    PVE::QemuServer::Cloudinit::generate_cloudinitconfig($conf, $vmid);
+    my $running = PVE::QemuServer::check_running($vmid);
+
+    if ($running) {
+	my $path = PVE::Storage::path($storecfg, $cloudinit_drive->{file});
+	if ($path) {
+	    mon_cmd($vmid, "eject", force => JSON::true, id => "$cloudinit_ds");
+	    mon_cmd($vmid, "blockdev-change-medium", id => "$cloudinit_ds", filename => "$path");
+	}
+    }
+}
+
 # called in locked context by incoming migration
 sub vm_migrate_get_nbd_disks {
     my ($storecfg, $conf, $replicated_volumes) = @_;
-- 
2.30.2

[pve-devel] [PATCH v7 qemu-server 09/10] add cloudinit hotplug

[Alexandre Derumier]

This allow to regenerate config drive if pending values exist
when we change vm options.

Signed-off-by: Alexandre Derumier <aderumier@odiso.com>
---
 PVE/QemuServer.pm | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
index 278a7b6..ded9812 100644
--- a/PVE/QemuServer.pm
+++ b/PVE/QemuServer.pm
@@ -303,7 +303,7 @@ my $confdesc = {
 	optional => 1,
 	type => 'string', format => 'pve-hotplug-features',
 	description => "Selectively enable hotplug features. This is a comma separated list of"
-	    ." hotplug features: 'network', 'disk', 'cpu', 'memory' and 'usb'. Use '0' to disable"
+	    ." hotplug features: 'network', 'disk', 'cpu', 'memory', 'usb' and 'cloudinit'. Use '0' to disable"
 	    ." hotplug completely. Using '1' as value is an alias for the default `network,disk,usb`.",
         default => 'network,disk,usb',
     },
@@ -1356,7 +1356,7 @@ sub parse_hotplug_features {
     $data = $confdesc->{hotplug}->{default} if $data eq '1';
 
     foreach my $feature (PVE::Tools::split_list($data)) {
-	if ($feature =~ m/^(network|disk|cpu|memory|usb)$/) {
+	if ($feature =~ m/^(network|disk|cpu|memory|usb|cloudinit)$/) {
 	    $res->{$1} = 1;
 	} else {
 	    die "invalid hotplug feature '$feature'\n";
@@ -5003,8 +5003,16 @@ sub vmconfig_hotplug_pending {
 	    delete $conf->{pending}->{$opt};
 	}
     }
-
     PVE::QemuConfig->write_config($vmid, $conf);
+
+    if($hotplug_features->{cloudinit}) {
+	my $pending = PVE::QemuServer::Cloudinit::get_pending_config($conf, $vmid);
+	my $regenerate = undef;
+	for my $item (@$pending) {
+	    $regenerate = 1 if defined($item->{delete}) or defined($item->{pending});
+	}
+	PVE::QemuServer::vmconfig_update_cloudinit_drive($storecfg, $conf, $vmid) if $regenerate;
+    }
 }
 
 sub try_deallocate_drive {
-- 
2.30.2

[pve-devel] [PATCH v7 qemu-server 10/10] debian : control : add a break on pve-manager <= 7.2.4

[Alexandre Derumier]

Signed-off-by: Alexandre Derumier <aderumier@odiso.com>
---
 debian/control | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/debian/control b/debian/control
index af258db..eff8165 100644
--- a/debian/control
+++ b/debian/control
@@ -54,6 +54,6 @@ Depends: dbus,
          ${perl:Depends},
          ${shlibs:Depends},
 Recommends: proxmox-backup-file-restore (>= 2.1.9-2)
-Breaks: pve-ha-manager (<= 3.0-4), pve-manager (<= 6.0-13)
+Breaks: pve-ha-manager (<= 3.0-4), pve-manager (<= 7.2-4)
 Description: Qemu Server Tools
  This package contains the Qemu Server tools used by Proxmox VE
-- 
2.30.2

Re: [pve-devel] [PATCH v7 qemu-server 10/10] debian : control : add a break on pve-manager <= 7.2.4

[Fabian Ebner]

Am 22.06.22 um 13:52 schrieb Alexandre Derumier:
> Signed-off-by: Alexandre Derumier <aderumier@odiso.com>
> ---
>  debian/control | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/debian/control b/debian/control
> index af258db..eff8165 100644
> --- a/debian/control
> +++ b/debian/control
> @@ -54,6 +54,6 @@ Depends: dbus,
>           ${perl:Depends},
>           ${shlibs:Depends},
>  Recommends: proxmox-backup-file-restore (>= 2.1.9-2)
> -Breaks: pve-ha-manager (<= 3.0-4), pve-manager (<= 6.0-13)
> +Breaks: pve-ha-manager (<= 3.0-4), pve-manager (<= 7.2-4)

Is there any special reason for this? What exactly breaks?

What's important is that new pve-manager on the target (which is what
the migration patch tests for - needs to be adapted to the proper
version when applying of course) implies new qemu-server on the target
(so it can parse the new section correctly). And that is done via a
normal dependency bump pve-manger -> qemu-server.

>  Description: Qemu Server Tools
>   This package contains the Qemu Server tools used by Proxmox VE

Re: [pve-devel] [PATCH v7 qemu-server 10/10] debian : control : add a break on pve-manager <= 7.2.4

[DERUMIER, Alexandre]

mmm, sorry.

I thinked I was breaking current gui,
but it's still working fine.
(it's don't call the new cloudinit api to display pending change,
 but the display is still like before)


For migration, indeed, it's need the new normal dependency bump pve-
manger -> qemu-server.

you can just skip this patch.


Le jeudi 23 juin 2022 à 09:22 +0200, Fabian Ebner a écrit :
> Am 22.06.22 um 13:52 schrieb Alexandre Derumier:
> > Signed-off-by: Alexandre Derumier <aderumier@odiso.com>
> > ---
> >  debian/control | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/debian/control b/debian/control
> > index af258db..eff8165 100644
> > --- a/debian/control
> > +++ b/debian/control
> > @@ -54,6 +54,6 @@ Depends: dbus,
> >           ${perl:Depends},
> >           ${shlibs:Depends},
> >  Recommends: proxmox-backup-file-restore (>= 2.1.9-2)
> > -Breaks: pve-ha-manager (<= 3.0-4), pve-manager (<= 6.0-13)
> > +Breaks: pve-ha-manager (<= 3.0-4), pve-manager (<= 7.2-4)
> 
> Is there any special reason for this? What exactly breaks?
> 
> What's important is that new pve-manager on the target (which is what
> the migration patch tests for - needs to be adapted to the proper
> version when applying of course) implies new qemu-server on the
> target
> (so it can parse the new section correctly). And that is done via a
> normal dependency bump pve-manger -> qemu-server.
> 
> >  Description: Qemu Server Tools
> >   This package contains the Qemu Server tools used by Proxmox VE
> 

Re: [pve-devel] [PATCH v7 qemu-server 00/10] cloudinit pending behaviour change

[Fiona Ebner]

Am 22.06.22 um 13:51 schrieb Alexandre Derumier:
> Hi,
> 
> This is an attempt to cleanup current behaviour of cloudinit online changes.
> 
> Currently, we setup cloudinit options as pending, until we generate the config drive.
> 
> This is not 100% true, because some option like vm name, nic mac address can be changed,
> without going to pending, so user can't known if it need to regenerated it.
> 
> Also, some can are very difficult to handle, if you hotplug a nic but it's failing,so pending,
> then you defined an ipconfig, and then you revert hotplug.
> or if you delete a nic, the ipconfig is no more displayed in the gui.
> 
> 
> So, instead of setting cloudinit values in pending,
> this patch serie copy the current cloudinit config in a new section [special:cloudinit],
> when the config drive is generated.
> This is only an hint, to allow to display diff between the generated cloudinit
> drive, and the current vm config.
> 
> A new specific cloudinit config api is added too displaying the diff between current and generated config.
> 
> Reminder: This need pve-manager depency bump first to check the version for live migration.

Without 10/10 and the version check in 03/10 adapted in accordance with
the above-mentioned bump (best done when applying):

Reviewed-by: Fiona Ebner <f.ebner@proxmox.com>

Re: [pve-devel] [PATCH v7 qemu-server 00/10] cloudinit pending behaviour change

[DERUMIER, Alexandre]

Hi,

Any news to apply this patch serie  ? Now that qemu 7.0 is pushed in repo.

Do you want a new patch serie with version check updated in 03/10 patch ?


Le 27/07/22 à 13:19, Fiona Ebner a écrit :
> Am 22.06.22 um 13:51 schrieb Alexandre Derumier:
>> Hi,
>>
>> This is an attempt to cleanup current behaviour of cloudinit online changes.
>>
>> Currently, we setup cloudinit options as pending, until we generate the config drive.
>>
>> This is not 100% true, because some option like vm name, nic mac address can be changed,
>> without going to pending, so user can't known if it need to regenerated it.
>>
>> Also, some can are very difficult to handle, if you hotplug a nic but it's failing,so pending,
>> then you defined an ipconfig, and then you revert hotplug.
>> or if you delete a nic, the ipconfig is no more displayed in the gui.
>>
>>
>> So, instead of setting cloudinit values in pending,
>> this patch serie copy the current cloudinit config in a new section [special:cloudinit],
>> when the config drive is generated.
>> This is only an hint, to allow to display diff between the generated cloudinit
>> drive, and the current vm config.
>>
>> A new specific cloudinit config api is added too displaying the diff between current and generated config.
>>
>> Reminder: This need pve-manager depency bump first to check the version for live migration.
> 
> Without 10/10 and the version check in 03/10 adapted in accordance with
> the above-mentioned bump (best done when applying):
> 
> Reviewed-by: Fiona Ebner <f.ebner@proxmox.com>
> 

Re: [pve-devel] [PATCH v7 qemu-server 00/10] cloudinit pending behaviour change

[Fiona Ebner]

Hi,

Am 31.08.22 um 11:40 schrieb DERUMIER, Alexandre:
> Hi,
> 
> Any news to apply this patch serie  ? Now that qemu 7.0 is pushed in repo.
> 

While the series looks fine to me, I'd feel better if a member of the
release team could take a look for sanity-checking as well, especially
the config-related parts.

> Do you want a new patch serie with version check updated in 03/10 patch ?
> 

I think that is best done by the person applying, together with the
package version bump.

Re: [pve-devel] [PATCH v7 qemu-server 00/10] cloudinit pending behaviour change

[DERUMIER, Alexandre]

Le 2/09/22 à 14:54, Fiona Ebner a écrit :
> Hi,
> 
> Am 31.08.22 um 11:40 schrieb DERUMIER, Alexandre:
>> Hi,
>>
>> Any news to apply this patch serie  ? Now that qemu 7.0 is pushed in repo.
>>
> 
> While the series looks fine to me, I'd feel better if a member of the
> release team could take a look for sanity-checking as well, especially
> the config-related parts.
> 
>> Do you want a new patch serie with version check updated in 03/10 patch ?
>>
> 
> I think that is best done by the person applying, together with the
> package version bump.
> 

ok, thanks Fionna !

Re: [pve-devel] [PATCH v7 qemu-server 00/10] cloudinit pending behaviour change

[DERUMIER, Alexandre]

Le 5/09/22 à 09:29, Mark Schouten via pve-devel a écrit :
> _______________________________________________
> pve-devel mailing list
> pve-devel@lists.proxmox.com
>Hi,

>>Not trying to break this thread, feel free to poke me off-list if this message is not supposed to be sent in this thread.
>>
>>Concerning cloud-init. There is something with Proxmox cloud-init that if you keep the cloud-init drive, after every boot, the sshd host-keys change. Is that something fixable? Or am I doing it wrong? 😄


Hi, Mark.

The main problem is the cloud-init has not been created to change ip 
configuration after the vm create.  It's really created for vm first 
init, it's "by design" like this.  (yes, that's sucks)


The only way to change ip address, is to regenerated a new cdrom id, to 
get cloudinit launch is early service.
But this also regenerated ssh host-keys.

I have done tests last year, I never was able to skip this.



I known that very last version of cloudinit have support for nic 
hotplug, to add ip on new hotplug iface, but I don't think it's working 
for already existing interface.



Personally, I'm use opennebula guest tools instead cloudinit,
they are simple bash script, easily extendable, udev hook for hotplug is 
working,...

I have added support last year to generate config drive format, it's not 
available in the gui, but can be added in a template manually


https://www.mail-archive.com/pve-devel@lists.proxmox.com/msg02621.html


the guest daemon are available here:

https://github.com/OpenNebula/addon-context-linux
https://github.com/OpenNebula/addon-context-windows

[pve-devel] applied-series: [PATCH v7 qemu-server 00/10] cloudinit pending behaviour change

[Thomas Lamprecht]

Am 22/06/2022 um 13:51 schrieb Alexandre Derumier:
> Alexandre Derumier (10):
>   qemuconfig: load_current_config : delete cloudinit value
>   vzdump : skip special:cloudinit section
>   migration: test targetnode min version for cloudinit section
>   cloudinit: add cloudinit section for current generated config.
>   generate cloudinit drive on offline plug
>   cloudinit: make cloudnit options fastplug
>   api2: add cloudinit config api
>   api2: add cloudinit_update
>   add cloudinit hotplug
>   debian : control : add a break on pve-manager <= 7.2.4
> 
>  PVE/API2/Qemu.pm            | 111 ++++++++++++++++++++++++++++++++++++
>  PVE/CLI/qm.pm               |   2 +
>  PVE/QemuConfig.pm           |   8 +++
>  PVE/QemuMigrate.pm          |   8 +++
>  PVE/QemuServer.pm           | 100 ++++++++++++++++++++++----------
>  PVE/QemuServer/Cloudinit.pm | 110 +++++++++++++++++++++++++++++++++++
>  PVE/QemuServer/Helpers.pm   |  24 ++++++++
>  PVE/VZDump/QemuServer.pm    |   5 +-
>  debian/control              |   2 +-
>  9 files changed, 337 insertions(+), 33 deletions(-)
> 

applied all but the manager one with Fiona's R-b tag.

> pve-manager patches series is available here : https://lists.proxmox.com/pipermail/pve-devel/2021-June/048542.html
> 

Do you have a new version for the GUI with the mentioned net issue addressed
or should I go for the available one at the list for now?