From: Thomas Lamprecht <t.lamprecht@proxmox.com>
To: "Dominic Jäger" <d.jaeger@proxmox.com>,
"Fabian Grünbichler" <f.gruenbichler@proxmox.com>
Cc: Proxmox VE development discussion <pve-devel@lists.proxmox.com>
Subject: Re: [pve-devel] [PATCH pve-common] JSONSchema: Increase vmid option minimum to 100
Date: Tue, 15 Dec 2020 09:55:06 +0100 [thread overview]
Message-ID: <680c7504-9b6d-08f2-3b72-a8a2bcec16b6@proxmox.com> (raw)
In-Reply-To: <20201215081139.GA383434@mala.proxmox.com>
On 15.12.20 09:11, Dominic Jäger wrote:
> On Mon, Dec 14, 2020 at 01:33:25PM +0100, Fabian Grünbichler wrote:
>> I think the argument is that the associated format already enforces that
>> the ID is >= 100, so that the API allows less has no practical effect
>> except confusing users that read the man page/help output/api dump.
>
> This is exactly what I meant. But I'm not 100% sure if some schema magic could
> happen that circumvents the pve-vmid format, so I can certainly start
>
FWIW; I never questioned that and this was 100% clear to me, but it still
does not help if we have some use here, even if only legacy or restore wise
or what not, touch such things can easily break things in a subtle way, and
only get noticed in months or even years, we had such stuff a few times already,
so I'd be really cautious here - adding the "actual enforced limit" into the
description can also help.
I mean, I really do not think this crucial limit slipped and was left as is
for years...
> On Mon, Dec 14, 2020 at 12:52:35PM +0100, Thomas Lamprecht wrote:
>> maybe you could take another look if you find
>> some case where values <100 are used for some special handling?
>
> looking for this.
>
>>>> diff --git a/src/PVE/JSONSchema.pm b/src/PVE/JSONSchema.pm
>>>> index 29ada5b..b4596d3 100644
>>>> --- a/src/PVE/JSONSchema.pm
>>>> +++ b/src/PVE/JSONSchema.pm
>>>> @@ -58,7 +58,7 @@ sub get_standard_option {
>>>> register_standard_option('pve-vmid', {
>>>> description => "The (unique) ID of the VM.",
>>>> type => 'integer', format => 'pve-vmid',
>>>> - minimum => 1
>>>> + minimum => 100,
>>>> });
>>>>
prev parent reply other threads:[~2020-12-15 8:55 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-12-14 10:20 Dominic Jäger
2020-12-14 11:52 ` Thomas Lamprecht
2020-12-14 12:33 ` Fabian Grünbichler
2020-12-15 8:11 ` Dominic Jäger
2020-12-15 8:55 ` Thomas Lamprecht [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=680c7504-9b6d-08f2-3b72-a8a2bcec16b6@proxmox.com \
--to=t.lamprecht@proxmox.com \
--cc=d.jaeger@proxmox.com \
--cc=f.gruenbichler@proxmox.com \
--cc=pve-devel@lists.proxmox.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox