Bug 962442 - Advanced NFS options (nfs_retrans and nfs_timeo) are silently truncated to 16bit
Advanced NFS options (nfs_retrans and nfs_timeo) are silently truncated to 16bit
Status: CLOSED DUPLICATE of bug 1000796
Product: Red Hat Enterprise Virtualization Manager
Classification: Red Hat
Component: ovirt-engine (Show other bugs)
3.2.0
Unspecified Unspecified
unspecified Severity high
: ---
: 3.4.0
Assigned To: Allon Mureinik
Meital Bourvine
storage
:
Depends On: 1000796
Blocks: rhev3.4beta 1142926
  Show dependency treegraph
 
Reported: 2013-05-13 09:55 EDT by Katarzyna Jachim
Modified: 2016-02-10 13:57 EST (History)
15 users (show)

See Also:
Fixed In Version: av1
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2014-03-05 05:25:11 EST
Type: Bug
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: Storage
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
amureini: Triaged+


Attachments (Terms of Use)


External Trackers
Tracker ID Priority Status Summary Last Updated
oVirt gerrit 17182 None None None Never

  None (edit)
Description Katarzyna Jachim 2013-05-13 09:55:57 EDT
Description of problem:
If you pass number n > 32768 as nfs_retrans or nfs_timeo, it is silently changed to x, where -32768 < x < 32768 (probably one of these is <=, I haven't looked into it so deeply) and (n-x) is divisible by 65536. If x is positive, storage domain is added successfully, if it is negative, adding fails.

Happens both with CLI and REST API.

CLI logs:
2013-05-13 14:57:46,877 - MainThread - capabilities - DEBUG - CREATE cli command is: add storagedomain  --host-name '10.35.160.67' --name 'test_148670_oor_retrans' --storage-address '10.35.64.106' --storage-path '/fastpass/jenkins-vm-09_nfs_2013_05_13_14_47_7_494994' --storage-nfs_version 'v3' --storage-nfs_timeo 730 --storage-nfs_retrans 131077 --storage-type 'nfs' --type 'data' --storage_format 'v3' --expect '201-created' > /tmp/cli_output.tmp
2013-05-13 14:57:55,386 - MainThread - capabilities - DEBUG - CREATE cli command Debug and Error output: [?1h=[K[?1l>
2013-05-13 14:57:55,387 - MainThread - capabilities - DEBUG - CREATE cli command output: 
id                 : 92757af0-a3a1-4b68-b797-04402b046fe0
name               : test_148670_oor_retrans
available          : 906238099456
committed          : 0
master             : False
status-state       : unknown
storage-address    : 10.35.64.106
storage-nfs_retrans: 5
storage-nfs_timeo  : 730
storage-nfs_version: v3
storage-path       : /fastpass/jenkins-vm-09_nfs_2013_05_13_14_47_7_494994
storage-type       : nfs
storage_format     : v3
type               : data
used               : 175019917312

REST call:

<storage_domain>
    <name>test_148670_oor_retrans</name>
    <type>data</type>
    <storage>
        <nfs_timeo>730</nfs_timeo>
        <type>nfs</type>
        <nfs_retrans>131077</nfs_retrans>
        <address>10.35.64.106</address>
        <path>/fastpass/jenkins-vm-28_nfs_2013_05_13_15_7_26_537303</path>
        <nfs_version>v3</nfs_version>
    </storage>
    <host>
        <name>10.35.160.49</name>
    </host>
    <storage_format>v3</storage_format>
</storage_domain>

REST response:
<storage_domain href="/api/storagedomains/9b0e5051-05fa-4065-9003-52776c13da23" id="9b0e5051-05fa-4065-9003-52776c13da23">
    <name>test_148670_oor_retrans</name>
    <link href="/api/storagedomains/9b0e5051-05fa-4065-9003-52776c13da23/permissions" rel="permissions"/>
    <link href="/api/storagedomains/9b0e5051-05fa-4065-9003-52776c13da23/disks" rel="disks"/>
    <type>data</type>
    <status>
        <state>unknown</state>
    </status>
    <master>false</master>
    <storage>
        <type>nfs</type>
        <address>10.35.64.106</address>
        <path>/fastpass/jenkins-vm-28_nfs_2013_05_13_15_7_26_537303</path>
        <nfs_version>v3</nfs_version>
        <nfs_timeo>730</nfs_timeo>
        <nfs_retrans>5</nfs_retrans>
    </storage>
    <available>906238099456</available>
    <used>175019917312</used>
    <committed>0</committed>
    <storage_format>v3</storage_format>
</storage_domain>


engine.log (from the call by CLI):
2013-05-13 14:57:47,145 INFO  [org.ovirt.engine.core.vdsbroker.vdsbroker.ValidateStorageServerConnectionVDSCommand] (ajp-/127.0.0.1:8702-1) [37a2c4e] START, ValidateStorageServerConnectionVDSCommand(HostName = 10.35.160.67, HostId = 6a416251-c55a-4179-9792-dd7b5af7c7b0, storagePoolId = 00000000-0000-0000-0000-000000000000, storageType = NFS, connectionList = [{ id: null, connection: 10.35.64.106:/fastpass/jenkins-vm-09_nfs_2013_05_13_14_47_7_494994, iqn: null, vfsType: null, mountOptions: null, nfsVersion: V3, nfsRetrans: 5, nfsTimeo: 730 };]), log id: 138d2d7f


Version-Release number of selected component (if applicable):
rhevm-3.2.0-10.25.beta3.el6ev.noarch


How reproducible: 100%


Steps to Reproduce:
1. Try to add NFS storage domain with nfs_retrans = 131077
  
Actual results:
Storage domain added with nfs_retrans = 5

Expected results:
Should fail with appropriate error message

Additional info:
http://jenkins.qa.lab.tlv.redhat.com:8080/job/Developer_job/1148 - CLI
http://jenkins.qa.lab.tlv.redhat.com:8080/job/Developer_job/1150 - REST

You can find engine logs etc. on the above links, please let me know if you want me to copy them and attach to this bug.
Comment 1 Allon Mureinik 2013-05-29 05:06:59 EDT
Need to raise an error on this.
Probably a @Valid or @Range annotation on the command's params should do the trick
Comment 4 Allon Mureinik 2013-07-21 12:44:53 EDT
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.
Comment 5 Allon Mureinik 2013-07-21 12:46:51 EDT
@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?
Comment 6 Allon Mureinik 2013-07-22 01:36:44 EDT
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.
Comment 7 Katarzyna Jachim 2013-07-22 08:41:11 EDT
@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".
Comment 8 Allon Mureinik 2013-07-22 10:35:13 EDT
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.
Comment 9 Michael Pasternak 2013-07-30 02:58:25 EDT
(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.
Comment 10 Allon Mureinik 2013-07-31 06:40:10 EDT
Validation to NFS params was merged.
@Michael - If we want to handle the overflow, please clone.
Comment 17 Meital Bourvine 2013-08-27 08:57:42 EDT
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.
Comment 18 Ayal Baron 2013-08-28 04:01:46 EDT
(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.
Comment 20 Michael Pasternak 2013-09-02 09:34:55 EDT
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>
Comment 21 Allon Mureinik 2014-02-20 08:51:16 EST
Moved to ON_QA for testing, as bug 1000796 should already cover this.
Comment 22 Meital Bourvine 2014-03-05 05:14:45 EST
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>
Comment 23 Allon Mureinik 2014-03-05 05:25:11 EST
This should be solved as part of bug 1000796.

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

Note You need to log in before you can comment on or make changes to this bug.