[pve-devel] [PATCH guest-common] fix #2834: skip refs in config_with_pending_array

[pve-devel] [PATCH guest-common] fix #2834: skip refs in config_with_pending_array

[Stoiko Ivanov]

With the refactoring of config_with_pending_array in
daf8fca57a34417365c873ed91f3a52bf0002a4f a few sanity checks on parsed configs
were dropped.

One case where a config value should be skipped, instead of parsed and added
is when the value is not scalar. This is the case for the raw lxc keys
(e.g. lxc.init.cmd, lxc.apparmor.profile) - which get added as array to the
'lxc' key.

This patch reintroduces the skipping of non-scalar values, when parsing the
config but not for the pending values.
From a short look through the commit history the sanity checks were in place
since 2014 (introduced in qemu-server for handling pending configuration
changes), and their removal did not cause any other regressions.
To my knowledge only the raw lxc config keys are parsed into a non-scalar
value.

Tested by adding a 'lxc.init.cmd' key to a container config.

Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
---
I hope that I did not miss another case where the sanity-checks are necessary,
and can gladly send a v2 reintroducing all of them.

 PVE/GuestHelpers.pm | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/PVE/GuestHelpers.pm b/PVE/GuestHelpers.pm
index 16ab3ac..69a05d9 100644
--- a/PVE/GuestHelpers.pm
+++ b/PVE/GuestHelpers.pm
@@ -214,6 +214,8 @@ sub config_with_pending_array {
 
     my $res = [];
     foreach my $opt (keys %$conf) {
+	next if ref($conf->{$opt});
+
 	my $item = {
 	    key => $opt,
 	    value => $conf->{$opt},
-- 
2.20.1

[pve-devel] applied: Re: [PATCH guest-common] fix #2834: skip refs in config_with_pending_array

[Thomas Lamprecht]

On 10.07.20 20:11, Stoiko Ivanov wrote:
> With the refactoring of config_with_pending_array in
> daf8fca57a34417365c873ed91f3a52bf0002a4f a few sanity checks on parsed configs
> were dropped.
> 
> One case where a config value should be skipped, instead of parsed and added
> is when the value is not scalar. This is the case for the raw lxc keys
> (e.g. lxc.init.cmd, lxc.apparmor.profile) - which get added as array to the
> 'lxc' key.
> 
> This patch reintroduces the skipping of non-scalar values, when parsing the
> config but not for the pending values.
> From a short look through the commit history the sanity checks were in place
> since 2014 (introduced in qemu-server for handling pending configuration
> changes), and their removal did not cause any other regressions.
> To my knowledge only the raw lxc config keys are parsed into a non-scalar
> value.
> 
> Tested by adding a 'lxc.init.cmd' key to a container config.
> 
> Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
> ---
> I hope that I did not miss another case where the sanity-checks are necessary,
> and can gladly send a v2 reintroducing all of them.
> 
>  PVE/GuestHelpers.pm | 2 ++
>  1 file changed, 2 insertions(+)
> 
>

applied, many thanks! Added a comment as followup, as during the refactoring Oguz had
some feeling that there was something else besides snapshots but could not come
up with anything, so I've thrown it out. Vetter safe than sorry (twice).