From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by lists.proxmox.com (Postfix) with ESMTPS id 16A9B8E80 for ; Wed, 16 Nov 2022 18:14:43 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id EC46F232AC for ; Wed, 16 Nov 2022 18:14:12 +0100 (CET) Received: from proxmox-new.maurer-it.com (proxmox-new.maurer-it.com [94.136.29.106]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by firstgate.proxmox.com (Proxmox) with ESMTPS for ; Wed, 16 Nov 2022 18:14:09 +0100 (CET) Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1]) by proxmox-new.maurer-it.com (Proxmox) with ESMTP id A867544C03; Wed, 16 Nov 2022 18:14:09 +0100 (CET) From: Wolfgang Bumiller To: pve-devel@lists.proxmox.com Date: Wed, 16 Nov 2022 18:14:05 +0100 Message-Id: <20221116171408.216775-4-w.bumiller@proxmox.com> X-Mailer: git-send-email 2.30.2 In-Reply-To: <20221116171408.216775-1-w.bumiller@proxmox.com> References: <20221116171408.216775-1-w.bumiller@proxmox.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-SPAM-LEVEL: Spam detection results: =?UTF-8?Q?0=0A=09?=AWL 0.231 Adjusted score from AWL reputation of From: =?UTF-8?Q?address=0A=09?=BAYES_00 -1.9 Bayes spam probability is 0 to 1% KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict =?UTF-8?Q?Alignment=0A=09?=SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF =?UTF-8?Q?Record=0A=09?=SPF_PASS -0.001 SPF: sender matches SPF =?UTF-8?Q?record=0A=09?=URIBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to URIBL was blocked. See http://wiki.apache.org/spamassassin/DnsBlocklists#dnsbl-block for more information. [cloudinit.pm, qemuserver.pm] Subject: [pve-devel] [PATCH qemu-server 3/6] delay cloudinit generation in hotplug X-BeenThere: pve-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox VE development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 16 Nov 2022 17:14:43 -0000 Hotpluggieg generated a cloudinit image based on old values in order to attach the device and later update it again, but the update was only done if cloudinit hotplug was enabled. This is weird, let's not. Also introduce 'apply_cloudinit_config' which also write the config, which, as it turns out, is the only thing we actually need anyway, currently. Signed-off-by: Wolfgang Bumiller --- PVE/QemuServer.pm | 53 +++++++++++++++++++++++++++++-------- PVE/QemuServer/Cloudinit.pm | 26 +++++++++++++++++- 2 files changed, 67 insertions(+), 12 deletions(-) diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm index a585680..d8bcfff 100644 --- a/PVE/QemuServer.pm +++ b/PVE/QemuServer.pm @@ -4972,6 +4972,7 @@ sub vmconfig_hotplug_pending { } } + my $cloudinit_opt; foreach my $opt (keys %{$conf->{pending}}) { next if $selection && !$selection->{$opt}; my $value = $conf->{pending}->{$opt}; @@ -5020,7 +5021,9 @@ sub vmconfig_hotplug_pending { # some changes can be done without hotplug my $drive = parse_drive($opt, $value); if (drive_is_cloudinit($drive)) { - PVE::QemuServer::Cloudinit::generate_cloudinitconfig($conf, $vmid); + $cloudinit_opt = [$opt, $drive]; + # apply all the other changes first, then generate the cloudinit disk + die "skip\n"; } vmconfig_update_disk($storecfg, $conf, $hotplug_features->{disk}, $vmid, $opt, $value, $arch, $machine_type); @@ -5039,6 +5042,23 @@ sub vmconfig_hotplug_pending { die "skip\n"; # skip non-hot-pluggable options } }; + if (my $err = $@) { + &$add_error($opt, $err) if $err ne "skip\n"; + } else { + $cloudinit_record_changed->($conf, $opt, $conf->{$opt}, $value); + $conf->{$opt} = $value; + delete $conf->{pending}->{$opt}; + } + } + + if (defined($cloudinit_opt)) { + my ($opt, $drive) = @$cloudinit_opt; + my $value = $conf->{pending}->{$opt}; + eval { + PVE::QemuServer::Cloudinit::apply_cloudinit_config($conf, $vmid); + vmconfig_update_disk($storecfg, $conf, $hotplug_features->{disk}, + $vmid, $opt, $value, $arch, $machine_type); + }; if (my $err = $@) { &$add_error($opt, $err) if $err ne "skip\n"; } else { @@ -5062,13 +5082,8 @@ sub vmconfig_hotplug_pending { 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; + if ($hotplug_features->{cloudinit} && PVE::QemuServer::Cloudinit::has_changes($conf)) { + PVE::QemuServer::vmconfig_update_cloudinit_drive($storecfg, $conf, $vmid); } } @@ -5171,7 +5186,13 @@ sub vmconfig_apply_pending { # 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; + if ($generate_cloudnit) { + 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. + PVE::QemuConfig->write_config($vmid, $conf); + } + } } sub vmconfig_update_net { @@ -5375,7 +5396,10 @@ sub vmconfig_update_cloudinit_drive { return if !$cloudinit_drive; - PVE::QemuServer::Cloudinit::generate_cloudinitconfig($conf, $vmid); + if (PVE::QemuServer::Cloudinit::apply_cloudinit_config($conf, $vmid)) { + PVE::QemuConfig->write_config($vmid, $conf); + } + my $running = PVE::QemuServer::check_running($vmid); if ($running) { @@ -5561,7 +5585,14 @@ sub vm_start_nolock { # don't regenerate the ISO if the VM is started as part of a live migration # this way we can reuse the old ISO with the correct config - PVE::QemuServer::Cloudinit::generate_cloudinitconfig($conf, $vmid) if !$migratedfrom; + 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? + # But we do it above, so for now let's be consistent. + $conf = PVE::QemuConfig->load_config($vmid); # update/reload + } + } # override offline migrated volumes, conf is out of date still if (my $offline_volumes = $migrate_opts->{offline_volumes}) { diff --git a/PVE/QemuServer/Cloudinit.pm b/PVE/QemuServer/Cloudinit.pm index f9bcbbc..24725e7 100644 --- a/PVE/QemuServer/Cloudinit.pm +++ b/PVE/QemuServer/Cloudinit.pm @@ -559,11 +559,19 @@ my $cloudinit_methods = { opennebula => \&generate_opennebula, }; -sub generate_cloudinitconfig { +sub has_changes { + my ($conf) = @_; + + return !!$conf->{cloudinit}->%*; +} + +sub generate_cloudinit_config { my ($conf, $vmid) = @_; my $format = get_cloudinit_format($conf); + my $has_changes = has_changes($conf); + PVE::QemuConfig->foreach_volume($conf, sub { my ($ds, $drive) = @_; @@ -576,6 +584,22 @@ sub generate_cloudinitconfig { $generator->($conf, $vmid, $drive, $volname, $storeid); }); + + return $has_changes; +} + +sub apply_cloudinit_config { + my ($conf, $vmid) = @_; + + my $has_changes = generate_cloudinit_config($conf, $vmid); + + if ($has_changes) { + delete $conf->{cloudinit}; + PVE::QemuConfig->write_config($vmid, $conf); + return 1; + } + + return $has_changes; } sub dump_cloudinit_config { -- 2.30.2