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

[Thomas Lamprecht]

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

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