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

[Dominik Csapak]

On 9/2/19 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).

see my other mail for why i named it tags, but i am fine with 'fw_cfg' 
or something else also

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

see my other mail for a proposal

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

would not work as it is, but if we allow for files, a user
could do this there then

another approach would be to base64 encode it (or sthg similar), but
then the config is not that readable anymore, and we probably
would want to provide an api call that returns the tags in a
'formatted' way (when i think about this, we probably want such an api 
call anyway)

> 
>>> +	    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
>>>
> 
> _______________________________________________
> pve-devel mailing list
> pve-devel at pve.proxmox.com
> https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
>