On Mon, Oct 30, 2023 at 05:30:15PM +0100, Thomas Lamprecht wrote:
> 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.du...@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.
AFAICT that's how it is for the other existing `format => 'urlencoded'`
options.
Our schema validation isn't smart enough for this currently.
We *could* add this sort of thing, though, if we wanted to?
The registered format verifiers could get the value's schema passed as
an additional parameter, then the `pve_verify_urlencoded()` sub in
JSONSchema.pm could check the decoded length against
`$schema->{maxLength} / 3` or (or a custom "extension"
`decodedLength => <int>`, but that's actually just superfluous in a
way... Unfortunately we can't put the lower number in 'maxLength' since
that's checked independently of the validation sub).
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
