* [pve-devel] [PATCH pve-common] JSONSchema: Increase vmid option minimum to 100
@ 2020-12-14 10:20 Dominic Jäger
2020-12-14 11:52 ` Thomas Lamprecht
0 siblings, 1 reply; 5+ messages in thread
From: Dominic Jäger @ 2020-12-14 10:20 UTC (permalink / raw)
To: pve-devel
The API standard_option pve-vmid always checks the format pve-vmid. The format
pve-vmid requires the vmid to be >=100. This is correct, because IDs 1-99 are
reserved.
To display this correctly in the API documentation (API viewer, qm man pages),
we have to set the minimum to 100 in the API standard_option.
Signed-off-by: Dominic Jäger <d.jaeger@proxmox.com>
---
https://forum.proxmox.com/threads/qm-importovf-man-page.80632/
src/PVE/JSONSchema.pm | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
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,
});
register_standard_option('pve-node', {
--
2.20.1
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [pve-devel] [PATCH pve-common] JSONSchema: Increase vmid option minimum to 100
2020-12-14 10:20 [pve-devel] [PATCH pve-common] JSONSchema: Increase vmid option minimum to 100 Dominic Jäger
@ 2020-12-14 11:52 ` Thomas Lamprecht
2020-12-14 12:33 ` Fabian Grünbichler
0 siblings, 1 reply; 5+ messages in thread
From: Thomas Lamprecht @ 2020-12-14 11:52 UTC (permalink / raw)
To: Proxmox VE development discussion, Dominic Jäger
On 14.12.20 11:20, Dominic Jäger wrote:
> The API standard_option pve-vmid always checks the format pve-vmid. The format
> pve-vmid requires the vmid to be >=100. This is correct, because IDs 1-99 are
> reserved.
>
> To display this correctly in the API documentation (API viewer, qm man pages),
> we have to set the minimum to 100 in the API standard_option.
>
> Signed-off-by: Dominic Jäger <d.jaeger@proxmox.com>
> ---
> https://forum.proxmox.com/threads/qm-importovf-man-page.80632/
IIRC, we use this somewhere, albeit I'm currently not remembering where.
In any case, both Dietmar and I have the feeling that this will break some
things. I'd like to avoid that, maybe you could take another look if you find
some case where values <100 are used for some special handling?
>
> src/PVE/JSONSchema.pm | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> 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,
> });
>
> register_standard_option('pve-node', {
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [pve-devel] [PATCH pve-common] JSONSchema: Increase vmid option minimum to 100
2020-12-14 11:52 ` Thomas Lamprecht
@ 2020-12-14 12:33 ` Fabian Grünbichler
2020-12-15 8:11 ` Dominic Jäger
0 siblings, 1 reply; 5+ messages in thread
From: Fabian Grünbichler @ 2020-12-14 12:33 UTC (permalink / raw)
To: Dominic Jäger, Proxmox VE development discussion
On December 14, 2020 12:52 pm, Thomas Lamprecht wrote:
> On 14.12.20 11:20, Dominic Jäger wrote:
>> The API standard_option pve-vmid always checks the format pve-vmid. The format
>> pve-vmid requires the vmid to be >=100. This is correct, because IDs 1-99 are
>> reserved.
>>
>> To display this correctly in the API documentation (API viewer, qm man pages),
>> we have to set the minimum to 100 in the API standard_option.
>>
>> Signed-off-by: Dominic Jäger <d.jaeger@proxmox.com>
>> ---
>> https://forum.proxmox.com/threads/qm-importovf-man-page.80632/
>
> IIRC, we use this somewhere, albeit I'm currently not remembering where.
>
> In any case, both Dietmar and I have the feeling that this will break some
> things. I'd like to avoid that, maybe you could take another look if you find
> some case where values <100 are used for some special handling?
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.
we use special reserved VMIDs sometimes when parsing a temp config file,
or when querying the kvm binary by starting a fake VM, but neither the
API schema nor the format are involved in that at all AFAICT.
>> src/PVE/JSONSchema.pm | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> 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,
>> });
>>
>> register_standard_option('pve-node', {
>>
>
>
>
>
> _______________________________________________
> pve-devel mailing list
> pve-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [pve-devel] [PATCH pve-common] JSONSchema: Increase vmid option minimum to 100
2020-12-14 12:33 ` Fabian Grünbichler
@ 2020-12-15 8:11 ` Dominic Jäger
2020-12-15 8:55 ` Thomas Lamprecht
0 siblings, 1 reply; 5+ messages in thread
From: Dominic Jäger @ 2020-12-15 8:11 UTC (permalink / raw)
To: Fabian Grünbichler, Thomas Lamprecht
Cc: Proxmox VE development discussion
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
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,
> >> });
> >>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [pve-devel] [PATCH pve-common] JSONSchema: Increase vmid option minimum to 100
2020-12-15 8:11 ` Dominic Jäger
@ 2020-12-15 8:55 ` Thomas Lamprecht
0 siblings, 0 replies; 5+ messages in thread
From: Thomas Lamprecht @ 2020-12-15 8:55 UTC (permalink / raw)
To: Dominic Jäger, Fabian Grünbichler
Cc: Proxmox VE development discussion
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,
>>>> });
>>>>
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2020-12-15 8:55 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-12-14 10:20 [pve-devel] [PATCH pve-common] JSONSchema: Increase vmid option minimum to 100 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 is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox