[pve-devel] [PATCH pve-common] JSONSchema: Increase vmid option minimum to 100

[pve-devel] [PATCH pve-common] JSONSchema: Increase vmid option minimum to 100

[Dominic Jäger]

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

Re: [pve-devel] [PATCH pve-common] JSONSchema: Increase vmid option minimum to 100

[Thomas Lamprecht]

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', {
> 

Re: [pve-devel] [PATCH pve-common] JSONSchema: Increase vmid option minimum to 100

[Fabian Grünbichler]

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
> 

Re: [pve-devel] [PATCH pve-common] JSONSchema: Increase vmid option minimum to 100

[Dominic Jäger]

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,
> >>  });
> >>  

Re: [pve-devel] [PATCH pve-common] JSONSchema: Increase vmid option minimum to 100

[Thomas Lamprecht]

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,
>>>>  });
>>>>