[pve-devel] [PATCH qemu-server] fix #1934: add qemu fw_cfg variables via 'tags'

[Fabian Grünbichler]

On September 2, 2019 2:27 pm, Thomas Lamprecht wrote:
> On 9/2/19 2:14 PM, Fabian Grünbichler wrote:
>> On August 23, 2019 2:03 pm, Dominik Csapak wrote:
>>> this add the 'tags' property to vms, which has the format:
>> 
>> why 'tags'? seems rather generic for what it does ;)
> 
> Second, tags is a NAK from me. Maybe just "fw_cfg" (or a variation), the Webinterface
> can show a full name and link to docs anyway (if it'd be added there, someday), and for
> the CLI we have the CLI help, man pages and the fact that it's named the same as the QEMU
> option, which helps a lot to associated both together (even if the sematic how values are
> passed differs, which is IMO totally fine).
> 
>>>
>>> key=value(;key=value)*
>>>
>>> each value will be set as
>>>
>>> -fw_cfg 'name=opt/com.proxmox/$key,string=$value'
>>> (qemu recommends using a unique rfqdn)
>>>
>>> this way, users can tag the vm with that information available inside
>>> e.g. under linux the value can be read under
>>>
>>> /sys/firmware/qemu_fw_cfg/by_name/opt/com.proxmox./$key/raw
>>>
>>> see the file docs/specs/fw_cfg.txt in the qemu repository for more
>>> details
>> 
>> if we introduce this, wouldn't it also make sense to allow to pass in 
>> snippet files via this interface (via file=$path instead of string=$value)?
> 
> How do you want to differentiate between? Would need a special syntax to
> detect that "file" is not meant to be a key for fw_cfg but a hint for PVE..

yes, obviously that would require a different syntax (or maybe even an 
indexed property string? I don't know how many such values users would 
want to set for a single VM..)

> 
>> 
>>>
>>> maybe we can also use this in the future to show/set in the gui
>>> e.g. some grouping/ordering etc.
>>>
>>> Signed-off-by: Dominik Csapak <d.csapak at proxmox.com>
>>> ---
>>>  PVE/QemuServer.pm | 37 +++++++++++++++++++++++++++++++++++++
>>>  1 file changed, 37 insertions(+)
>>>
>>> diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
>>> index 9f5bf56..d55a1ae 100644
>>> --- a/PVE/QemuServer.pm
>>> +++ b/PVE/QemuServer.pm
>>> @@ -232,6 +232,29 @@ my $watchdog_fmt = {
>>>  };
>>>  PVE::JSONSchema::register_format('pve-qm-watchdog', $watchdog_fmt);
>>>  
>>> +PVE::JSONSchema::register_format('pve-qm-tags', \&verify_tag_format);
>>> +sub verify_tag_format {
>>> +    my ($tagstring, $noerr) = @_;
>>> +
>>> +    if (!$tagstring) {
>>> +	return '';
>>> +    }
>>> +
>>> +    # forbid all characters not in range 0x20-0x7E
>>> +    for my $tag (split(';', $tagstring)) {
>>> +	my ($key, $value) = ($tag =~ m/^(.*)=(.*)$/);
>>> +	if ($key =~ m/[\x00-\x1F\x7F-\xFF]/) {
>>> +	    die "invalid character in tag key\n" if !$noerr;
>>> +	    return undef;
>>> +	}
>>> +	if ($value =~ m/[\x00-\x1F\x7F-\xFF]/) {
>>> +	    die "invalid character in tag value\n" if !$noerr;
>>> +	    return undef;
>>> +	}
>>> +    }
>>> +
>>> +    return $tagstring;
>>> +}
>>>  my $agent_fmt = {
>>>      enabled => {
>>>  	description => "Enable/disable Qemu GuestAgent.",
>>> @@ -672,6 +695,13 @@ EODESCR
>>>  	description => "Configure a audio device, useful in combination with QXL/Spice.",
>>>  	optional => 1
>>>      },
>>> +    tags => {
>>> +	description => "Specify key/value pairs to be added to qemu fw_cfg.",
>>> +	type => 'string',
>>> +	maxLength => 4096,
>>> +	format => 'pve-qm-tags',
>>> +	optional => 1,
>>> +    },
>>>  };
>>>  
>>>  my $cicustom_fmt = {
>>> @@ -4152,6 +4182,13 @@ sub config_to_command {
>>>  	push @$cmd, '-loadstate', $statepath;
>>>      }
>>>  
>>> +    if ($conf->{tags}) {
>>> +	for my $tag (split(';', $conf->{tags})) {
>>> +	    my ($key, $value) = ($tag =~ m/^(.*)=(.*)$/);
> 
> what do I do when I need a ";" in a value or a "=" in a key?
> 
>>> +	    push @$cmd, '-fw_cfg', "name=opt/com.proxmox/$key,string=$value";
>>> +	}
>>> +    }
>>> +
>>>      # add custom args
>>>      if ($conf->{args}) {
>>>  	my $aa = PVE::Tools::split_args($conf->{args});
>>> -- 
>>> 2.20.1
>>>
>