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.laute...@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};
_______________________________________________
pve-devel mailing list
pve-devel@pve.proxmox.com
https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
