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 A3E1B9E1A9
for <pve-devel@lists.proxmox.com>; Mon, 30 Oct 2023 17:30:19 +0100 (CET)
Received: from firstgate.proxmox.com (localhost [127.0.0.1])
by firstgate.proxmox.com (Proxmox) with ESMTP id 8B739122BB
for <pve-devel@lists.proxmox.com>; Mon, 30 Oct 2023 17:30:19 +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
for <pve-devel@lists.proxmox.com>; Mon, 30 Oct 2023 17:30:18 +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 A17104275F
for <pve-devel@lists.proxmox.com>; Mon, 30 Oct 2023 17:30:18 +0100 (CET)
Message-ID: <c0893733-091e-4bb4-9d43-a47b3c17c840@proxmox.com>
Date: Mon, 30 Oct 2023 17:30:15 +0100
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Content-Language: en-GB
To: Proxmox VE development discussion <pve-devel@lists.proxmox.com>,
Hannes Duerr <h.duerr@proxmox.com>
References: <20231025123719.38036-1-h.duerr@proxmox.com>
From: Thomas Lamprecht <t.lamprecht@proxmox.com>
In-Reply-To: <20231025123719.38036-1-h.duerr@proxmox.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
X-SPAM-LEVEL: Spam detection results: 0
AWL -0.074 Adjusted score from AWL reputation of From: address
BAYES_00 -1.9 Bayes spam probability is 0 to 1%
DMARC_MISSING 0.1 Missing DMARC policy
KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment
SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record
SPF_PASS -0.001 SPF: sender matches SPF record
Subject: Re: [pve-devel] [PATCH qemu-server] fix #4957: add vendor and
product information passthrough for SCSI-Disks
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, 30 Oct 2023 16:30:19 -0000
I mean, the properties are relatively straight forward, but some commit
message would be still nice to have – e.g., how did you check if the values
propagate into the guest, can this
On 25/10/2023 14:37, Hannes Duerr wrote:
> Signed-off-by: Hannes Duerr <h.duerr@proxmox.com>
> ---
> PVE/QemuServer.pm | 12 ++++++++++++
> PVE/QemuServer/Drive.pm | 26 ++++++++++++++++++++++++++
> 2 files changed, 38 insertions(+)
>
> diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
> index 2cd8948..69be3af 100644
> --- a/PVE/QemuServer.pm
> +++ b/PVE/QemuServer.pm
> @@ -1482,6 +1482,18 @@ sub print_drivedevice_full {
> }
> $device .= ",wwn=$drive->{wwn}" if $drive->{wwn};
>
> + # only scsi-hd supports passing vendor and product information
should we error out then if it's set to other types? Not here, as it's
already in the config, but erroring out on on config update/create could
be better UX than accepting it, but then not using it.
> + if ($devicetype eq 'hd') {
> + if (my $vendor = $drive->{vendor}) {
> + $vendor = URI::Escape::uri_unescape($vendor);
> + $device .= ",vendor=$vendor";
> + }
> + if (my $product = $drive->{product}) {
> + $product = URI::Escape::uri_unescape($product);
> + $device .= ",product=$product";
> + }
> + }
> +
> } elsif ($drive->{interface} eq 'ide' || $drive->{interface} eq 'sata') {
> my $maxdev = ($drive->{interface} eq 'sata') ? $PVE::QemuServer::Drive::MAX_SATA_DISKS : 2;
> my $controller = int($drive->{index} / $maxdev);
> diff --git a/PVE/QemuServer/Drive.pm b/PVE/QemuServer/Drive.pm
> index e24ba12..20efc2f 100644
> --- a/PVE/QemuServer/Drive.pm
> +++ b/PVE/QemuServer/Drive.pm
> @@ -159,6 +159,28 @@ my %iothread_fmt = ( iothread => {
> optional => 1,
> });
>
> +my %product_fmt = (
> + product => {
> + type => 'string',
> + format => 'urlencoded',
> + format_description => 'product',
> + maxLength => 40*3, # *3 since it's %xx url enoded
wouldn't that be for the worst case, e.g., if one would only enter spaces
or colons? And what about utf-8, that would be even bigger (not sure though
of we support that here).
> + description => "The drive's product name, url-encoded, up to 40 bytes long.",
the 40 bytesa aren't checked though? We would need to do so manually
after decoding it.
> + optional => 1,
> + },
> +);
> +
> +my %vendor_fmt = (
> + vendor => {
> + type => 'string',
> + format => 'urlencoded',
> + format_description => 'vendor',
> + maxLength => 40*3, # *3 since it's %xx url enoded
> + description => "The drive's vendor name, url-encoded, up to 40 bytes long.",
same here w.r.t. maxLength and 40 bytes max as above.
> + optional => 1,
> + },
> +);
> +
> my %model_fmt = (
> model => {
> type => 'string',