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

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

[Lucas Barros]

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.

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,;]*$/;
+
     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

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

[Fabian Grünbichler]

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
> 
> 
> 
> 
>