From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <t.lamprecht@proxmox.com>
Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68])
 (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)
 key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256)
 (No client certificate requested)
 by lists.proxmox.com (Postfix) with ESMTPS id 2DF32632EE
 for <pve-devel@lists.proxmox.com>; Mon, 14 Feb 2022 15:27:42 +0100 (CET)
Received: from firstgate.proxmox.com (localhost [127.0.0.1])
 by firstgate.proxmox.com (Proxmox) with ESMTP id 228CD23E72
 for <pve-devel@lists.proxmox.com>; Mon, 14 Feb 2022 15:27:42 +0100 (CET)
Received: from proxmox-new.maurer-it.com (proxmox-new.maurer-it.com
 [94.136.29.106])
 (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)
 key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256)
 (No client certificate requested)
 by firstgate.proxmox.com (Proxmox) with ESMTPS id 5645923E67
 for <pve-devel@lists.proxmox.com>; Mon, 14 Feb 2022 15:27:41 +0100 (CET)
Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1])
 by proxmox-new.maurer-it.com (Proxmox) with ESMTP id 2E46D44F31
 for <pve-devel@lists.proxmox.com>; Mon, 14 Feb 2022 15:27:41 +0100 (CET)
Message-ID: <3134476c-c4e9-a79e-25fa-8136df26d5b3@proxmox.com>
Date: Mon, 14 Feb 2022 15:27:40 +0100
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:98.0) Gecko/20100101
 Thunderbird/98.0
Content-Language: en-US
To: Proxmox VE development discussion <pve-devel@lists.proxmox.com>,
 Matthias Heiserer <m.heiserer@proxmox.com>
References: <20220214140144.961041-1-m.heiserer@proxmox.com>
From: Thomas Lamprecht <t.lamprecht@proxmox.com>
In-Reply-To: <20220214140144.961041-1-m.heiserer@proxmox.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
X-SPAM-LEVEL: Spam detection results:  0
 AWL 0.058 Adjusted score from AWL reputation of From: address
 BAYES_00                 -1.9 Bayes spam probability is 0 to 1%
 KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment
 NICE_REPLY_A           -0.001 Looks like a legit reply (A)
 SPF_HELO_NONE           0.001 SPF: HELO does not publish an SPF Record
 SPF_PASS               -0.001 SPF: sender matches SPF record
 T_SCC_BODY_TEXT_LINE    -0.01 -
Subject: Re: [pve-devel] [RFC manager 0/5] GUI: Hardware comments
X-BeenThere: pve-devel@lists.proxmox.com
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: Proxmox VE development discussion <pve-devel.lists.proxmox.com>
List-Unsubscribe: <https://lists.proxmox.com/cgi-bin/mailman/options/pve-devel>, 
 <mailto:pve-devel-request@lists.proxmox.com?subject=unsubscribe>
List-Archive: <http://lists.proxmox.com/pipermail/pve-devel/>
List-Post: <mailto:pve-devel@lists.proxmox.com>
List-Help: <mailto:pve-devel-request@lists.proxmox.com?subject=help>
List-Subscribe: <https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel>, 
 <mailto:pve-devel-request@lists.proxmox.com?subject=subscribe>
X-List-Received-Date: Mon, 14 Feb 2022 14:27:42 -0000

On 14.02.22 15:01, Matthias Heiserer wrote:
> This series is a first attempt to fix 2672 by implementing editable comments
> that are displayed in the GUI and stored in the qemu config file. 

I know Dominik's comment suggests this features is "ack'd" but I'm really not
sure about that I'd ack it, at least not in such a overly general form that adds
such things every where. We already have the guest notes for general comments about
bios/machine/... settings, bug 2672 is actually only asking for giving nic's and
disks some tag-like property to allow humans easier to determine what it's used for.

> 
> It works, but there are several questions:
> 
> How should comments be displayed? (GUI)
>     I added a third column. IMO, this looks cleaner than the alternative of
>     appending the comment to the corresponding config values. However, it 
>     requires more space and some special logic.

No, I really would not go that way for now, especially as I don't want such
comments on every level, if they're more restricted and tag like they can
just be rendered with existing infra.

>     
> What to do when there is not enough space? (GUI)
>     Currently, this case is not handled. A possible solution would be to
>     wrap overflowing lines, but I'm not sure what opinions are on that.

see above, no special handling required.

>     
> Should comments of ineditable fields (EFI Disk) be editable? (GUI)
>     I suppose yes, but currently they are not.

no, one can only have one efidisk, so the admin cannot be confused by multiple
efi disk entries, so they don't require a comment, the general guest section
is already more than enough for this-

> 
> Currently missing are:
>     GUI: Comments in the wizard, i.e. when creating a VM, no comment can be set.

for disks maybe ok, at least since we can add multiple disks from the start now
already, for all else no, please don't overload the user with inputs fields which
most people never need.

>     Detaching disks voids their comment. Probably shouldn't happen, but seems
>         a tad complicated.

that is unrelated of comments, detaching disks voids a lot of stuff which isn't
ideal and is separate from such a series.

>     The GUI assumes comments (except memory, socket, bios,  machine, scsihw)
>         to be URIencoded. This could be verified server-side.

just restrict them to a simpler format and be done..

Something along [\w-]{,32} (check what we already use elsewhere first)

What's missing: container support.


patch submit tipp: if you group the filenames of the format-patch exported patches
by repo before letting git send-email process them, they'd be also ordered/grouped
by that in the MTA's thread view per default, making it easier to review/apply.