[pve-devel] [PATCH container] vmstatus: Align name if not set in config to VMs

[Aaron Lauterer]

I think this email only got sent to me and not the mailing list. Answer 
from my side inline.

On 1/16/20 7:05 PM, Thomas Lamprecht wrote:
> On 1/16/20 2:35 PM, Aaron Lauterer wrote:
>> VMs have a space in between VM and the VMID.
>>
>> Signed-off-by: Aaron Lauterer <a.lauterer at proxmox.com>
>> ---
>>
>> While this is a small optical nit pick we could also think about
>> replacing the whitespace between CT/VM and the ID with a dash. We do not
>> allow a space when setting the name in the config AFAICT.
> 
> Hmm, mixed feelings on that one. While I get where you come from and support
> that argument we then get into a situation where the name displayed in the
> tree differs from the one under CT -> Options, and is also one which would
> not be valid when entered in CT -> Options.
> I mean we can just ignore it, but it creates still a slight discrepancy.

I just realized that while the patch works as expected for VMs (when 
changing the same code in the qemu-server repo), the situation for 
containers is a bit different. That's what happens when I assume that 
things will be the same... :/

So with a bit more research the situation AFAICT is that this code in 
pve-container/src/PVE/LXC.pm never gets triggered because the hostname 
is set to "CT$vmid" when the container is created.

Should be want to make this nicer we would have to touch that code in 
the create_vm API endpoint and use a dash as delimiter to have a valid 
hostname.

> 
> VMs are more free in their naming as it does not functions as a real
> hostname set to the CT, we could
> 
>>
>> This could potentially break something down the line if the names are
>> used for more than just display.
> 
> If people use this to make some correlation between vmstatus and the config
> assuming name == hostname (quasi as index) it could maybe lead to unexpected
> results. A change using a dash or slash or some other character would always
> habe those issues too, independent of the fact if they would be a valid
> hostname (only dash would be anyway).
> 
> An option would be to changes this only on rendering, it would keep the API
> return semantics the same but still make "unamed" CTs and VMs more similar.
> 
> Maybe the breakage is really not an issue and this patch is OK as is, so other
> opinion are welcome.

So with the realization from above there are only two possibilities:
* change GUI rendering
* change naming behavior during container creation

The latter would mean that users who already have unnamed containers 
will see both, containers with a dash and without.

With this in mind I would actually argue to just leave it be since all 
other solutions are ugly in some way. :/

> 
>>
>>   src/PVE/LXC.pm | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/src/PVE/LXC.pm b/src/PVE/LXC.pm
>> index 34949c6..81d2dd4 100644
>> --- a/src/PVE/LXC.pm
>> +++ b/src/PVE/LXC.pm
>> @@ -197,7 +197,7 @@ sub vmstatus {
>>   
>>   	$unprivileged->{$vmid} = $conf->{unprivileged};
>>   
>> -	$d->{name} = $conf->{'hostname'} || "CT$vmid";
>> +	$d->{name} = $conf->{'hostname'} || "CT $vmid";
>>   	$d->{name} =~ s/[\s]//g;
>>   
>>   	$d->{cpus} = $conf->{cores} || $conf->{cpulimit};
>>
>