Bug 962442

Need to raise an error on this.
Probably a @Valid or @Range annotation on the command's params should do the trick

The (short) integer overflowing is a system wide issue.

E.g., consider the following PUT request to update a host's SPM priority:
<host>
<storage_manager priority="4294967298"/>
</host>

This will end up setting the host's SPM priority to 2.

@Michael - I'd like your input on this, I'm guessing this should be handled in a system-wide fashion.


Specifically for this bug, we need canDoAction validations on retrans and timeo to prevent the engine needlessly going to VDSM with a request that's bound to fail and possibly giving a nicer error message, but again, there is a larger underlying issue.

@Meital - why is this a test blocker?
Why aren't you just using values that are <10000 ?
Frankly, I can't image a real-world use case that needs a larger value.
What am I missing?

Sent a patch to improve the parameter validation so we don't send useless commands to VDSM (see external trackers).
I am not moving the bug to POST, as I don't feel this is really a solution, just a minor improvement.
Pending feedback from Michael and Meital (comment 4 and comment 5, respectively), to see how we proceed with this issue.

@Allon - we use values > 1000 because it is a part of the test plan: https://tcms.engineering.redhat.com/case/148670/?from_plan=5849

We want to make sure that RHEV-M can handle incorrect input in a reasonable way, without crashing and preferably with showing an appropriate error message. We don't expect any user to need such values and we don't expect RHEV-M to handle it in any other way than just returning an error "Value out of range".

Returning needinfo on Michael (Meital - I assume you remove it by mistake when you cleared the needinfo on you?).

Michael - please take a look at comment 4, I need your input on this.

(In reply to Allon Mureinik from comment #4)
> The (short) integer overflowing is a system wide issue.
> 
> E.g., consider the following PUT request to update a host's SPM priority:
> <host>
> <storage_manager priority="4294967298"/>
> </host>
> 
> This will end up setting the host's SPM priority to 2.
> 
> @Michael - I'd like your input on this, I'm guessing this should be handled
> in a system-wide fashion.

indeed, can be achieved via range/max validation.

> 
> 
> Specifically for this bug, we need canDoAction validations on retrans and
> timeo to prevent the engine needlessly going to VDSM with a request that's
> bound to fail and possibly giving a nicer error message, but again, there is
> a larger underlying issue.

Validation to NFS params was merged.
@Michael - If we want to handle the overflow, please clone.

From a user prespective, if the user sends an out of range value, then he should get an exception that it's out of range. 
It should not update the value to the out of range value. Also, it should not set a truncated value.

(In reply to Meital Bourvine from comment #17)
> From a user prespective, if the user sends an out of range value, then he
> should get an exception that it's out of range. 
> It should not update the value to the out of range value. Also, it should
> not set a truncated value.

Correct, and for really large numbers as you suggest you have bug 1000796 tracking that issue as explained in comment 15.

To avoid hitting that bug you can test with any value from -65536 to 65535
which includes a lot of out of range numbers and you'll get the canDoAction you're expecting.

Alon,

you should be using xml spec [1] to limit numbers in api.xsd,

[1] http://www.w3schools.com/schema/schema_dtypes_numeric.asp

<xs:element name="nfs_retrans">
  <xs:simpleType>
    <xs:restriction base="xs:unsignedShort">
      <xs:minInclusive value="0"/>
      <xs:maxInclusive value="..."/>
    </xs:restriction>
  </xs:simpleType>
</xs:element>

Moved to ON_QA for testing, as bug 1000796 should already cover this.

Verified on av2:

Succeeded on a positive value

<storage_domain>
     <name>test_148670_oor_retrans</name>
     <type>data</type>
     <storage>
         <address>10.35.160.108</address>
         <path>/RHEV/mbourvin1</path>
         <nfs_timeo>730</nfs_timeo>
         <nfs_retrans>65533</nfs_retrans>
         <nfs_version>v3</nfs_version>
         <type>nfs</type>
     </storage>
     <host>
         <name>green-vdsb</name>
     </host>
     <storage_format>v3</storage_format>
</storage_domain>


<fault>
<reason>Operation Failed</reason>
<detail>
[NFS Retransmissions should be between 0 and 32767]
</detail>
</fault>



Failed on a negative value:
(Succeeded creating the storage domain although it should fail!)

<storage_domain>
     <name>test_148670_oor_retrans</name>
     <type>data</type>
     <storage>
         <address>10.35.160.108</address>
         <path>/RHEV/mbourvin1</path>
         <nfs_timeo>730</nfs_timeo>
         <nfs_retrans>-65534</nfs_retrans>
         <nfs_version>v3</nfs_version>
         <type>nfs</type>
     </storage>
     <host>
         <name>green-vdsb</name>
     </host>
     <storage_format>v3</storage_format>
</storage_domain>



<storage_domain href= "/api/storagedomains/fc04227c-21fe-4b94-bdd2-034b9e3c6835" id="fc04227c-21fe-4b94-bdd2-034b9e3c6835">
<name>test_148670_oor_retrans</name>
<link href= "/api/storagedomains/fc04227c-21fe-4b94-bdd2-034b9e3c6835/permissions" rel="permissions"/>
<link href= "/api/storagedomains/fc04227c-21fe-4b94-bdd2-034b9e3c6835/disks" rel="disks"/>
<link href= "/api/storagedomains/fc04227c-21fe-4b94-bdd2-034b9e3c6835/storageconnections" rel="storageconnections"/>
<type>data</type>
<status>
<state>unknown</state>
</status>
<master>false</master>
<storage>
<address>10.35.160.108</address>
<type>nfs</type>
<path>/RHEV/mbourvin1</path>
<nfs_version>v3</nfs_version>
<nfs_timeo>730</nfs_timeo>
<nfs_retrans>2</nfs_retrans>
</storage>
<available>2028298305536</available>
<used>134217728000</used>
<committed>0</committed>
<storage_format>v3</storage_format>
</storage_domain>

This should be solved as part of bug 1000796.

*** This bug has been marked as a duplicate of bug 1000796 ***