Change in vdsm[master]: multipath: getDeviceDiscardMaxBytes return type

Idan Shaby has uploaded a new change for review.

Change subject: multipath: getDeviceDiscardMaxBytes return type
......................................................................

multipath: getDeviceDiscardMaxBytes return type

Since /sys/block/dm-X/queue/discard_max_bytes may return more than 4GB,
uint might not be enough.
Thus, this patch chenges getDeviceDiscardMaxBytes to read and return it
as a long value.

Change-Id: Icc45c708df0e93ddf94165eb6f3e3630ce8430a4
Signed-off-by: Idan Shaby <ishaby(a)redhat.com&gt;
---
M lib/api/vdsm-api.yml
M vdsm/storage/multipath.py
2 files changed, 12 insertions(+), 5 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/99/62799/1

diff --git a/lib/api/vdsm-api.yml b/lib/api/vdsm-api.yml
index 8ec6f9f..6c0401a 100644
--- a/lib/api/vdsm-api.yml
+++ b/lib/api/vdsm-api.yml
@@ -221,7 +221,7 @@
                 unmapped in a single discard operation. 0 means that the device
                 does not support discard functionality.
             name: discard_max_bytes
-            type: uint
+            type: long
 
         -   description: Indicates whether or not the device deterministically
                 returns zeroes when a discarded area is read.
diff --git a/vdsm/storage/multipath.py b/vdsm/storage/multipath.py
index 27f720e..6b43bfc 100644
--- a/vdsm/storage/multipath.py
+++ b/vdsm/storage/multipath.py
@@ -168,7 +168,7 @@
 
 def getDeviceDiscardMaxBytes(physDev):
     path = os.path.join(SYS_BLOCK, physDev, QUEUE, "discard_max_bytes")
-    return read_int(path)
+    return read_long(path)
 
 
 def getDeviceDiscardZeroesData(physDev):
@@ -176,10 +176,17 @@
     return read_int(path)
 
 
-def read_int(path):
+def read_line(path):
     with open(path, "r") as f:
-        data = f.readline()
-    return int(data)
+        return f.readline()
+
+
+def read_int(path):
+    return int(read_line(path))
+
+
+def read_long(path):
+    return long(read_line(path))
 
 
 def getScsiSerial(physdev):


-- 
To view, visit https://gerrit.ovirt.org/62799
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: Icc45c708df0e93ddf94165eb6f3e3630ce8430a4
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Idan Shaby <ishaby(a)redhat.com&gt;

gerrit-hooks has posted comments on this change.

Change subject: multipath: getDeviceDiscardMaxBytes return type
......................................................................


Patch Set 1:

* Update tracker: IGNORE, no Bug-Url found
* Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and
is a valid url.
* Check merged to previous::IGNORE,         Not in stable branch (['ovirt-3.6',
'ovirt-4.0'])

-- 
To view, visit https://gerrit.ovirt.org/62799
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Icc45c708df0e93ddf94165eb6f3e3630ce8430a4
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Idan Shaby <ishaby(a)redhat.com&gt;
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org&gt;
Gerrit-HasComments: No

gerrit-hooks has posted comments on this change.

Change subject: multipath: getDeviceDiscardMaxBytes return type
......................................................................


Patch Set 2:

* Update tracker: IGNORE, no Bug-Url found
* Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and
is a valid url.
* Check merged to previous::IGNORE,         Not in stable branch (['ovirt-3.6',
'ovirt-4.0'])

-- 
To view, visit https://gerrit.ovirt.org/62799
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Icc45c708df0e93ddf94165eb6f3e3630ce8430a4
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Idan Shaby <ishaby(a)redhat.com&gt;
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org&gt;
Gerrit-HasComments: No

Idan Shaby has posted comments on this change.

Change subject: multipath: getDeviceDiscardMaxBytes return type
......................................................................


Patch Set 2: Verified+1

-- 
To view, visit https://gerrit.ovirt.org/62799
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Icc45c708df0e93ddf94165eb6f3e3630ce8430a4
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Idan Shaby <ishaby(a)redhat.com&gt;
Gerrit-Reviewer: Idan Shaby <ishaby(a)redhat.com&gt;
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org&gt;
Gerrit-HasComments: No

[Jenkins CI RO]

Jenkins CI has posted comments on this change.

Change subject: multipath: getDeviceDiscardMaxBytes return type
......................................................................


Patch Set 2: Continuous-Integration+1

Propagate review hook: Continuous Integration value inherited from patch 1

-- 
To view, visit https://gerrit.ovirt.org/62799
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Icc45c708df0e93ddf94165eb6f3e3630ce8430a4
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Idan Shaby <ishaby(a)redhat.com&gt;
Gerrit-Reviewer: Adam Litke <alitke(a)redhat.com&gt;
Gerrit-Reviewer: Allon Mureinik <amureini(a)redhat.com&gt;
Gerrit-Reviewer: Idan Shaby <ishaby(a)redhat.com&gt;
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: Tal Nisan <tnisan(a)redhat.com&gt;
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org&gt;
Gerrit-HasComments: No

[Allon Mureinik]

Allon Mureinik has posted comments on this change.

Change subject: multipath: getDeviceDiscardMaxBytes return type
......................................................................


Patch Set 2:

(1 comment)

https://gerrit.ovirt.org/#/c/62799/2//COMMIT_MSG
Commit Message:

Line 9: Since /sys/block/dm-X/queue/discard_max_bytes may return more than 4GB,
Line 10: uint might not be enough.
Line 11: Thus, this patch changes getDeviceDiscardMaxBytes to read and return it
Line 12: as a long value.
Line 13: 
Just to be sure - this was added with THIS VERSION, so we don't have a backwards
compatibility issue, right?
Line 14: Change-Id: Icc45c708df0e93ddf94165eb6f3e3630ce8430a4


-- 
To view, visit https://gerrit.ovirt.org/62799
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Icc45c708df0e93ddf94165eb6f3e3630ce8430a4
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Idan Shaby <ishaby(a)redhat.com&gt;
Gerrit-Reviewer: Adam Litke <alitke(a)redhat.com&gt;
Gerrit-Reviewer: Ala Hino <ahino(a)redhat.com&gt;
Gerrit-Reviewer: Allon Mureinik <amureini(a)redhat.com&gt;
Gerrit-Reviewer: Idan Shaby <ishaby(a)redhat.com&gt;
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Liron Aravot <laravot(a)redhat.com&gt;
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: Tal Nisan <tnisan(a)redhat.com&gt;
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org&gt;
Gerrit-HasComments: Yes

Idan Shaby has posted comments on this change.

Change subject: multipath: getDeviceDiscardMaxBytes return type
......................................................................


Patch Set 2:

(1 comment)

https://gerrit.ovirt.org/#/c/62799/2//COMMIT_MSG
Commit Message:

Line 9: Since /sys/block/dm-X/queue/discard_max_bytes may return more than 4GB,
Line 10: uint might not be enough.
Line 11: Thus, this patch changes getDeviceDiscardMaxBytes to read and return it
Line 12: as a long value.
Line 13: 
>  Just to be sure - this was added with THIS VERSION, so we don't
> have a back 
Right, it was added only to master after the branch for ovirt-4.0 was
created and was not backported.
No backward compatibility issues here.
Line 14: Change-Id: Icc45c708df0e93ddf94165eb6f3e3630ce8430a4


-- 
To view, visit https://gerrit.ovirt.org/62799
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Icc45c708df0e93ddf94165eb6f3e3630ce8430a4
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Idan Shaby <ishaby(a)redhat.com&gt;
Gerrit-Reviewer: Adam Litke <alitke(a)redhat.com&gt;
Gerrit-Reviewer: Ala Hino <ahino(a)redhat.com&gt;
Gerrit-Reviewer: Allon Mureinik <amureini(a)redhat.com&gt;
Gerrit-Reviewer: Idan Shaby <ishaby(a)redhat.com&gt;
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Liron Aravot <laravot(a)redhat.com&gt;
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: Tal Nisan <tnisan(a)redhat.com&gt;
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org&gt;
Gerrit-HasComments: Yes

[Nir Soffer]

Nir Soffer has posted comments on this change.

Change subject: multipath: getDeviceDiscardMaxBytes return type
......................................................................


Patch Set 2: Code-Review-1

(3 comments)

https://gerrit.ovirt.org/#/c/62799/2//COMMIT_MSG
Commit Message:

Line 8: 
Line 9: Since /sys/block/dm-X/queue/discard_max_bytes may return more than 4GB,
Line 10: uint might not be enough.
Line 11: Thus, this patch changes getDeviceDiscardMaxBytes to read and return it
Line 12: as a long value.
This should only fix the schema, the code is already returning long value when the kernel
reports such value.
Line 13: 
Line 14: Change-Id: Icc45c708df0e93ddf94165eb6f3e3630ce8430a4


https://gerrit.ovirt.org/#/c/62799/2/lib/api/vdsm-api.yml
File lib/api/vdsm-api.yml:

Line 220:         -   description: The maximal number of bytes that can be trimmed or
Line 221:                 unmapped in a single discard operation. 0 means that the device
Line 222:                 does not support discard functionality.
Line 223:             name: discard_max_bytes
Line 224:             type: long
Should be ulong, unless you think the old type should have been int.
Line 225: 
Line 226:         -   description: Indicates whether or not the device deterministically
Line 227:                 returns zeroes when a discarded area is read.
Line 228:                 1 means it does, 0 mesans that the result is undefined (can be


https://gerrit.ovirt.org/#/c/62799/2/vdsm/storage/multipath.py
File vdsm/storage/multipath.py:

Line 185:     return int(read_line(path))
Line 186: 
Line 187: 
Line 188: def read_long(path):
Line 189:     return long(read_line(path))
int() and long() are the same since Python 2.4, and there is no need to create different
types or different apis to parse integer values.

> >> int(str(2**64 + 1)) 
    18446744073709551617L

This module does not need any change, documenting the type of the value in the schema is
good enough.

See https://www.python.org/dev/peps/pep-0237/
Line 190: 
Line 191: 
Line 192: def getScsiSerial(physdev):
Line 193:     blkdev = os.path.join("/dev", physdev)


-- 
To view, visit https://gerrit.ovirt.org/62799
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Icc45c708df0e93ddf94165eb6f3e3630ce8430a4
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Idan Shaby <ishaby(a)redhat.com&gt;
Gerrit-Reviewer: Adam Litke <alitke(a)redhat.com&gt;
Gerrit-Reviewer: Ala Hino <ahino(a)redhat.com&gt;
Gerrit-Reviewer: Allon Mureinik <amureini(a)redhat.com&gt;
Gerrit-Reviewer: Idan Shaby <ishaby(a)redhat.com&gt;
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Liron Aravot <laravot(a)redhat.com&gt;
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: Tal Nisan <tnisan(a)redhat.com&gt;
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org&gt;
Gerrit-HasComments: Yes

gerrit-hooks has posted comments on this change.

Change subject: multipath: getDeviceDiscardMaxBytes return type
......................................................................


Patch Set 3:

* Update tracker: IGNORE, no Bug-Url found
* Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and
is a valid url.
* Check merged to previous::IGNORE,         Not in stable branch (['ovirt-3.6',
'ovirt-4.0'])

-- 
To view, visit https://gerrit.ovirt.org/62799
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Icc45c708df0e93ddf94165eb6f3e3630ce8430a4
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Idan Shaby <ishaby(a)redhat.com&gt;
Gerrit-Reviewer: Adam Litke <alitke(a)redhat.com&gt;
Gerrit-Reviewer: Ala Hino <ahino(a)redhat.com&gt;
Gerrit-Reviewer: Allon Mureinik <amureini(a)redhat.com&gt;
Gerrit-Reviewer: Idan Shaby <ishaby(a)redhat.com&gt;
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Liron Aravot <laravot(a)redhat.com&gt;
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: Tal Nisan <tnisan(a)redhat.com&gt;
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org&gt;
Gerrit-HasComments: No

Idan Shaby has posted comments on this change.

Change subject: multipath: getDeviceDiscardMaxBytes return type
......................................................................


Patch Set 3: Verified+1

(3 comments)

https://gerrit.ovirt.org/#/c/62799/2//COMMIT_MSG
Commit Message:

Line 8: 
Line 9: Since /sys/block/dm-X/queue/discard_max_bytes may return more than 4GB,
Line 10: uint might not be enough.
Line 11: Thus, this patch fixes the schema to state on a ulong instead of uint
Line 12: value for discard_max_bytes.
>  This should only fix the schema, the code is already returning long
> value w 
Done
Line 13: 
Line 14: Change-Id: Icc45c708df0e93ddf94165eb6f3e3630ce8430a4


https://gerrit.ovirt.org/#/c/62799/2/lib/api/vdsm-api.yml
File lib/api/vdsm-api.yml:

Line 220:         -   description: The maximal number of bytes that can be trimmed or
Line 221:                 unmapped in a single discard operation. 0 means that the device
Line 222:                 does not support discard functionality.
Line 223:             name: discard_max_bytes
Line 224:             type: ulong
>  Should be ulong, unless you think the old type should have been int.
Done
Line 225: 
Line 226:         -   description: Indicates whether or not the device deterministically
Line 227:                 returns zeroes when a discarded area is read.
Line 228:                 1 means it does, 0 mesans that the result is undefined (can be


https://gerrit.ovirt.org/#/c/62799/2/vdsm/storage/multipath.py
File vdsm/storage/multipath.py:

Line 185: def getScsiSerial(physdev):
Line 186:     blkdev = os.path.join("/dev", physdev)
Line 187:     cmd = [_SCSI_ID.cmd,
Line 188:            "--page=0x80",
Line 189:            "--whitelisted",
>  int() and long() are the same since Python 2.4, and there is no need
> to cre 
Done, thanks for the explanation, Nir.
Line 190:            "--export",
Line 191:            "--replace-whitespace",
Line 192:            "--device=" + blkdev]
Line 193:     (rc, out, err) = misc.execCmd(cmd)


-- 
To view, visit https://gerrit.ovirt.org/62799
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Icc45c708df0e93ddf94165eb6f3e3630ce8430a4
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Idan Shaby <ishaby(a)redhat.com&gt;
Gerrit-Reviewer: Adam Litke <alitke(a)redhat.com&gt;
Gerrit-Reviewer: Ala Hino <ahino(a)redhat.com&gt;
Gerrit-Reviewer: Allon Mureinik <amureini(a)redhat.com&gt;
Gerrit-Reviewer: Idan Shaby <ishaby(a)redhat.com&gt;
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Liron Aravot <laravot(a)redhat.com&gt;
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: Tal Nisan <tnisan(a)redhat.com&gt;
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org&gt;
Gerrit-HasComments: Yes

[Nir Soffer]

Nir Soffer has posted comments on this change.

Change subject: multipath: getDeviceDiscardMaxBytes return type
......................................................................


Patch Set 3: Code-Review+2

-- 
To view, visit https://gerrit.ovirt.org/62799
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Icc45c708df0e93ddf94165eb6f3e3630ce8430a4
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Idan Shaby <ishaby(a)redhat.com&gt;
Gerrit-Reviewer: Adam Litke <alitke(a)redhat.com&gt;
Gerrit-Reviewer: Ala Hino <ahino(a)redhat.com&gt;
Gerrit-Reviewer: Allon Mureinik <amureini(a)redhat.com&gt;
Gerrit-Reviewer: Idan Shaby <ishaby(a)redhat.com&gt;
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Liron Aravot <laravot(a)redhat.com&gt;
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: Tal Nisan <tnisan(a)redhat.com&gt;
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org&gt;
Gerrit-HasComments: No

[Nir Soffer]

Nir Soffer has submitted this change and it was merged.

Change subject: multipath: getDeviceDiscardMaxBytes return type
......................................................................


multipath: getDeviceDiscardMaxBytes return type

Since /sys/block/dm-X/queue/discard_max_bytes may return more than 4GB,
uint might not be enough.
Thus, this patch fixes the schema to state on a ulong instead of uint
value for discard_max_bytes.

Change-Id: Icc45c708df0e93ddf94165eb6f3e3630ce8430a4
Signed-off-by: Idan Shaby <ishaby(a)redhat.com&gt;
Reviewed-on: https://gerrit.ovirt.org/62799
Continuous-Integration: Jenkins CI
Reviewed-by: Nir Soffer <nsoffer(a)redhat.com&gt;
---
M lib/api/vdsm-api.yml
1 file changed, 1 insertion(+), 1 deletion(-)

Approvals:
  Nir Soffer: Looks good to me, approved
  Jenkins CI: Passed CI tests
  Idan Shaby: Verified



-- 
To view, visit https://gerrit.ovirt.org/62799
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: Icc45c708df0e93ddf94165eb6f3e3630ce8430a4
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Idan Shaby <ishaby(a)redhat.com&gt;
Gerrit-Reviewer: Adam Litke <alitke(a)redhat.com&gt;
Gerrit-Reviewer: Ala Hino <ahino(a)redhat.com&gt;
Gerrit-Reviewer: Allon Mureinik <amureini(a)redhat.com&gt;
Gerrit-Reviewer: Idan Shaby <ishaby(a)redhat.com&gt;
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Liron Aravot <laravot(a)redhat.com&gt;
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: Tal Nisan <tnisan(a)redhat.com&gt;
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org&gt;

gerrit-hooks has posted comments on this change.

Change subject: multipath: getDeviceDiscardMaxBytes return type
......................................................................


Patch Set 4:

* Update tracker: IGNORE, no Bug-Url found
* Set MODIFIED::IGNORE, no Bug-Url found.

-- 
To view, visit https://gerrit.ovirt.org/62799
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Icc45c708df0e93ddf94165eb6f3e3630ce8430a4
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Idan Shaby <ishaby(a)redhat.com&gt;
Gerrit-Reviewer: Adam Litke <alitke(a)redhat.com&gt;
Gerrit-Reviewer: Ala Hino <ahino(a)redhat.com&gt;
Gerrit-Reviewer: Allon Mureinik <amureini(a)redhat.com&gt;
Gerrit-Reviewer: Idan Shaby <ishaby(a)redhat.com&gt;
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Liron Aravot <laravot(a)redhat.com&gt;
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: Tal Nisan <tnisan(a)redhat.com&gt;
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org&gt;
Gerrit-HasComments: No