Re: [PATCH qemu-server] fix #7173: config: do not write out an empty tag list

["Fabian Grünbichler" <f.gruenbichler@proxmox.com>]

On November 9, 2026 2:56 pm, Lucas Barros wrote:
> Removing the last tag of a VM, for example in the web UI, sets 'tags'
> to an empty string, which then ends up as an empty 'tags:' line in the
> VM config and in the output of 'qm config'. This carries no
> information, is inconsistent with how unset options are handled and
> confuses tools that consume the config, as they need to special case
> an empty tag list.

We could consider changing this and rejecting empty `-list` parameters,
and instead require properly deleting? e.g., switch over to use
`--delete tags` if the last to-be-set tag list is empty, make the parser
ignore keys without a value, and then with 10.0 make empty (either by
virtue of being an empty string, or just delimiters) list parameters
invalid?

It seems like these two patches just add a hacky workaround on top (with
rather subtle semantics about adding the invalid value to pending but
not to the `current` config)..

> Drop the key when writing the config if the tag list is empty, which
> also cleans up configs that already contain such a line on the next
> write. The pending section is left alone, as an empty value there is
> still needed to apply the change.
> 
> Signed-off-by: Lucas Barros <lucasbarros.kam@gmail.com>
> ---
> A matching patch for pve-container is sent separately.
> 
> Only the main section is touched. In [PENDING] the empty value is still
> needed so that applying the pending change removes the tags, and it is
> dropped from the main section on the write that follows.
> 
> The new parse-config test input contains the 'tags: ' line with the
> trailing space that the writer produces today, so 'git am' warns about
> trailing whitespace there. Without the trailing space the line would
> not match the config parser at all.
> 
> An alternative would be to turn an empty 'tags' value into a deletion
> in the update API, but that would not cover other paths that end up
> with an empty value (create, clone, restore, 'qm set --tags ""').
> 
> Tested on a PVE 9 node (qemu-server 9.2.15, pve-container 6.1.18),
> adding and then removing the only tag of a VM and of a container:
> 
> without the patches:
>   $ qm set 9901 --tags foo; pvesh set /nodes/<node>/qemu/9901/config --tags ''
>   $ grep -n tags /etc/pve/qemu-server/9901.conf | cat -A
>   6:tags:  $
>   $ pct set 9902 --tags foo; pct set 9902 --tags ''
>   $ grep -n tags /etc/pve/lxc/9902.conf | cat -A
>   7:tags: $
>   $ pvesh get /nodes/<node>/qemu/9901/config --output-format json | jq -c '{tags}'
>   {"tags":" "}
> 
> with the patches:
>   $ grep -c tags /etc/pve/qemu-server/9901.conf /etc/pve/lxc/9902.conf
>   /etc/pve/qemu-server/9901.conf:0
>   /etc/pve/lxc/9902.conf:0
>   $ pvesh get /nodes/<node>/qemu/9901/config --output-format json | jq -c '{tags}'
>   {"tags":null}
> 
> An existing empty 'tags:' line goes away on the next write of the
> config, removing the tag of a running VM leaves no line and no
> [PENDING] entry, and 'tags: a;b' is still written as before.
> 
> 'make test' (run with 16 CPUs in /proc/cpuinfo, the cfg2cmd tests need
> more vCPUs than the test machine has) passes before and after; the
> parse-config tests go from 20 to 22, and the new empty-tags case fails
> without the change to QemuServer.pm. proxmox-perltidy reports no
> changes.
> 
> This patch was prepared with the help of an LLM based coding assistant;
> I reviewed the change and ran the tests mentioned above myself.
> 
>  src/PVE/QemuServer.pm                          |  3 +++
>  src/test/parse-config-expected/empty-tags.conf | 11 +++++++++++
>  src/test/parse-config-input/empty-tags.conf    | 12 ++++++++++++
>  src/test/run_parse_config_tests.pl             |  2 +-
>  4 files changed, 27 insertions(+), 1 deletion(-)
>  create mode 100644 src/test/parse-config-expected/empty-tags.conf
>  create mode 100644 src/test/parse-config-input/empty-tags.conf
> 
> diff --git a/src/PVE/QemuServer.pm b/src/PVE/QemuServer.pm
> index 8045c07e..22621e63 100644
> --- a/src/PVE/QemuServer.pm
> +++ b/src/PVE/QemuServer.pm
> @@ -2067,6 +2067,9 @@ sub write_vm_config {
>          delete $conf->{smp};
>      }
>  
> +    # an empty tag list carries no information, so do not write out the key
> +    delete $conf->{tags} if defined($conf->{tags}) && $conf->{tags} =~ m/^[\s,;]*$/;

nit: if we keep this approach, this should `split_list` and check that
the result is empty, else it risks going out of sync (it already is
w.r.t. \0-delimited lists).

> +
>      my $used_volids = {};
>  
>      my $cleanup_config = sub {
> diff --git a/src/test/parse-config-expected/empty-tags.conf b/src/test/parse-config-expected/empty-tags.conf
> new file mode 100644
> index 00000000..a5b02f2c
> --- /dev/null
> +++ b/src/test/parse-config-expected/empty-tags.conf
> @@ -0,0 +1,11 @@
> +# VM with an empty tag list left behind after removing the last tag
> +bootdisk: scsi0
> +cores: 1
> +memory: 512
> +name: apache
> +ostype: l26
> +scsi0: mydir:142/vm-142-disk-0.qcow2,size=4G
> +scsihw: virtio-scsi-pci
> +smbios1: uuid=ddf91b3f-a597-42be-9a7e-fb6421dcd5cd
> +sockets: 1
> +vmgenid: 0
> diff --git a/src/test/parse-config-input/empty-tags.conf b/src/test/parse-config-input/empty-tags.conf
> new file mode 100644
> index 00000000..a37a9c2d
> --- /dev/null
> +++ b/src/test/parse-config-input/empty-tags.conf
> @@ -0,0 +1,12 @@
> +# VM with an empty tag list left behind after removing the last tag
> +bootdisk: scsi0
> +cores: 1
> +memory: 512
> +name: apache
> +ostype: l26
> +scsi0: mydir:142/vm-142-disk-0.qcow2,size=4G
> +scsihw: virtio-scsi-pci
> +smbios1: uuid=ddf91b3f-a597-42be-9a7e-fb6421dcd5cd
> +sockets: 1
> +tags: 
> +vmgenid: 0
> diff --git a/src/test/run_parse_config_tests.pl b/src/test/run_parse_config_tests.pl
> index f68abc06..c30aad99 100755
> --- a/src/test/run_parse_config_tests.pl
> +++ b/src/test/run_parse_config_tests.pl
> @@ -25,7 +25,7 @@ my $INPUT_DIR = './parse-config-input';
>  my $EXPECTED_DIR = './parse-config-expected';
>  
>  # NOTE update when you add/remove tests
> -plan tests => 2 * 10;
> +plan tests => 2 * 11;
>  
>  sub run_tests {
>      my ($strict) = @_;
> -- 
> 2.47.3
> 
> 
> 
> 
>