Change in vdsm[master]: storage: add support for blkdiscard command

2016-05-05 Thread nsoffer 
Nir Soffer has posted comments on this change.

Change subject: storage: add support for blkdiscard command
..


Patch Set 6:

(2 comments)

https://gerrit.ovirt.org/#/c/35629/6/lib/vdsm/storage/blkdiscard.py
File lib/vdsm/storage/blkdiscard.py:

Line 43: def run_blkdiscard(device):
Line 44: cmd = [_blkdiscard.cmd]
Line 45: cmd.append(device)
Line 46: 
Line 47: rc, out, err = commands.execCmd(cmd, deathSignal=signal.SIGKILL)
> Since I'm hoping to see this in 4.0.x, not sure we can do that right now. I
Right, we want this change *now*, we don't have time for additional work.
Line 48: 
Line 49: if rc != 0:


https://gerrit.ovirt.org/#/c/35629/6/tests/storage_blkdiscard_test.py
File tests/storage_blkdiscard_test.py:

Line 25: 
Line 26: BLKDISCARD = blkdiscard._blkdiscard.cmd
Line 27: 
Line 28: 
Line 29: class FakeCmd(object):
> This class looks like a general purpose utility that should appear in testl
I agree, good idea for later patch.
Line 30: 
Line 31: def __init__(self, name, *calls):
Line 32: self.patch = monkeypatch.Patch([(commands, name, self)])
Line 33: self.calls = list(calls)


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I2ea7dd19fadc600b8fe78fb436ae430d35f52165
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Federico Simoncelli 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Ala Hino 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Freddy Rolland 
Gerrit-Reviewer: Idan Shaby 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Jenkins CI RO
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Tal Nisan 
Gerrit-Reviewer: Yaniv Kaul 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: storage: add support for blkdiscard command

2016-05-05 Thread ykaul 
Yaniv Kaul has posted comments on this change.

Change subject: storage: add support for blkdiscard command
..


Patch Set 6:

(1 comment)

https://gerrit.ovirt.org/#/c/35629/6/lib/vdsm/storage/blkdiscard.py
File lib/vdsm/storage/blkdiscard.py:

Line 43: def run_blkdiscard(device):
Line 44: cmd = [_blkdiscard.cmd]
Line 45: cmd.append(device)
Line 46: 
Line 47: rc, out, err = commands.execCmd(cmd, deathSignal=signal.SIGKILL)
> This module seems like a perfect candidate for StorageJob integration.  Fro
Since I'm hoping to see this in 4.0.x, not sure we can do that right now. I 
also hope that there won't be a need for progress - I expect blkdiscard to run 
fast (Gigs/sec - faster than line speed) in most storages.
Line 48: 
Line 49: if rc != 0:


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I2ea7dd19fadc600b8fe78fb436ae430d35f52165
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Federico Simoncelli 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Ala Hino 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Freddy Rolland 
Gerrit-Reviewer: Idan Shaby 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Jenkins CI RO
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Tal Nisan 
Gerrit-Reviewer: Yaniv Kaul 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: storage: add support for blkdiscard command

2016-05-05 Thread alitke 
Adam Litke has posted comments on this change.

Change subject: storage: add support for blkdiscard command
..


Patch Set 6:

(2 comments)

https://gerrit.ovirt.org/#/c/35629/6/lib/vdsm/storage/blkdiscard.py
File lib/vdsm/storage/blkdiscard.py:

Line 43: def run_blkdiscard(device):
Line 44: cmd = [_blkdiscard.cmd]
Line 45: cmd.append(device)
Line 46: 
Line 47: rc, out, err = commands.execCmd(cmd, deathSignal=signal.SIGKILL)
This module seems like a perfect candidate for StorageJob integration.  From 
the blkdiscard man page I see that we can get progress info which can be 
reported via the  Jobs API.
Line 48: 
Line 49: if rc != 0:


https://gerrit.ovirt.org/#/c/35629/6/tests/storage_blkdiscard_test.py
File tests/storage_blkdiscard_test.py:

Line 25: 
Line 26: BLKDISCARD = blkdiscard._blkdiscard.cmd
Line 27: 
Line 28: 
Line 29: class FakeCmd(object):
This class looks like a general purpose utility that should appear in 
testlib.py.
Line 30: 
Line 31: def __init__(self, name, *calls):
Line 32: self.patch = monkeypatch.Patch([(commands, name, self)])
Line 33: self.calls = list(calls)


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I2ea7dd19fadc600b8fe78fb436ae430d35f52165
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Federico Simoncelli 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Ala Hino 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Freddy Rolland 
Gerrit-Reviewer: Idan Shaby 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Jenkins CI RO
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Tal Nisan 
Gerrit-Reviewer: Yaniv Kaul 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: storage: add support for blkdiscard command

2016-05-04 Thread nsoffer 
Nir Soffer has posted comments on this change.

Change subject: storage: add support for blkdiscard command
..


Patch Set 6:

(2 comments)

https://gerrit.ovirt.org/#/c/35629/6/lib/vdsm/storage/blkdiscard.py
File lib/vdsm/storage/blkdiscard.py:

Line 28: _blkdiscard = utils.CommandPath("blkdiscard",
Line 29: "/sbin/blkdiscard",)
Line 30: 
Line 31: 
Line 32: class BlkDiscardError(Exception):
Not needed, use cmdutils.Error instead.
Line 33: def __init__(self, ecode, stdout, stderr):
Line 34: self.ecode = ecode
Line 35: self.stdout = stdout
Line 36: self.stderr = stderr


Line 39: return "ecode={0}, stdout={1}, stderr={2}".format(
Line 40: self.ecode, self.stdout, self.stderr)
Line 41: 
Line 42: 
Line 43: def run_blkdiscard(device):
Should be named blkdiscard.
Line 44: cmd = [_blkdiscard.cmd]
Line 45: cmd.append(device)
Line 46: 
Line 47: rc, out, err = commands.execCmd(cmd, deathSignal=signal.SIGKILL)


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I2ea7dd19fadc600b8fe78fb436ae430d35f52165
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Federico Simoncelli 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Ala Hino 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Freddy Rolland 
Gerrit-Reviewer: Idan Shaby 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Jenkins CI RO
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Tal Nisan 
Gerrit-Reviewer: Yaniv Kaul 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: storage: add support for blkdiscard command

2016-05-04 Thread ishaby 
Idan Shaby has posted comments on this change.

Change subject: storage: add support for blkdiscard command
..


Patch Set 6: Code-Review-1 -Verified

Yaniv, a parameter can always be added to the method, and then appended to the 
command.
Since I don't want to add "dead code", I'll add it only when I need it.

Regardless, I am making a few more changes so please don't review it yet.

Thanks

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I2ea7dd19fadc600b8fe78fb436ae430d35f52165
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Federico Simoncelli 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Ala Hino 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Freddy Rolland 
Gerrit-Reviewer: Idan Shaby 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Jenkins CI RO
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Tal Nisan 
Gerrit-Reviewer: Yaniv Kaul 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: storage: add support for blkdiscard command

2016-05-04 Thread ykaul 
Yaniv Kaul has posted comments on this change.

Change subject: storage: add support for blkdiscard command
..


Patch Set 6:

How do you pass parameters (such as --zero) to blkdiscard?

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I2ea7dd19fadc600b8fe78fb436ae430d35f52165
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Federico Simoncelli 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Ala Hino 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Freddy Rolland 
Gerrit-Reviewer: Idan Shaby 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Jenkins CI RO
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Tal Nisan 
Gerrit-Reviewer: Yaniv Kaul 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: storage: add support for blkdiscard command

2016-05-03 Thread ishaby 
Idan Shaby has posted comments on this change.

Change subject: storage: add support for blkdiscard command
..


Patch Set 6: Verified+1

(1 comment)

https://gerrit.ovirt.org/#/c/35629/5/tests/Makefile.am
File tests/Makefile.am:

Line 69: 
Line 70: test_modules = \
Line 71:alignmentScanTests.py \
Line 72:virt_api_test.py \
Line 73:blocksdTests.py \
> Please name this storage_blkdiscard_test.py - this is our new convention.
Done
Line 74:blockVolumeTests.py \
Line 75:bridgeTests.py \
Line 76:bulk_sampling_test.py \
Line 77:cPopenTests.py \


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I2ea7dd19fadc600b8fe78fb436ae430d35f52165
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Federico Simoncelli 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Ala Hino 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Freddy Rolland 
Gerrit-Reviewer: Idan Shaby 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Jenkins CI RO
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Tal Nisan 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: storage: add support for blkdiscard command

2016-05-03 Thread automation 
gerrit-hooks has posted comments on this change.

Change subject: storage: add support for blkdiscard command
..


Patch Set 6:

* 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'])

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I2ea7dd19fadc600b8fe78fb436ae430d35f52165
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Federico Simoncelli 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Ala Hino 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Freddy Rolland 
Gerrit-Reviewer: Idan Shaby 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Jenkins CI RO
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Tal Nisan 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: storage: add support for blkdiscard command

2016-05-03 Thread nsoffer 
Nir Soffer has posted comments on this change.

Change subject: storage: add support for blkdiscard command
..


Patch Set 5:

(1 comment)

https://gerrit.ovirt.org/#/c/35629/5/tests/Makefile.am
File tests/Makefile.am:

Line 69: 
Line 70: test_modules = \
Line 71:alignmentScanTests.py \
Line 72:virt_api_test.py \
Line 73:blkdiscardTests.py \
Please name this storage_blkdiscard_test.py - this is our new convention.
Line 74:blocksdTests.py \
Line 75:blockVolumeTests.py \
Line 76:bridgeTests.py \
Line 77:bulk_sampling_test.py \


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I2ea7dd19fadc600b8fe78fb436ae430d35f52165
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Federico Simoncelli 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Ala Hino 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Freddy Rolland 
Gerrit-Reviewer: Idan Shaby 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Jenkins CI RO
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Tal Nisan 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: storage: add support for blkdiscard command

2016-05-03 Thread automation 
gerrit-hooks has posted comments on this change.

Change subject: storage: add support for blkdiscard command
..


Patch Set 5:

* 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'])

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I2ea7dd19fadc600b8fe78fb436ae430d35f52165
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Federico Simoncelli 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Ala Hino 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Freddy Rolland 
Gerrit-Reviewer: Idan Shaby 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Jenkins CI RO
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Tal Nisan 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: storage: add support for blkdiscard command

2016-05-03 Thread ishaby 
Idan Shaby has posted comments on this change.

Change subject: storage: add support for blkdiscard command
..


Patch Set 4: Verified+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I2ea7dd19fadc600b8fe78fb436ae430d35f52165
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Federico Simoncelli 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Ala Hino 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Freddy Rolland 
Gerrit-Reviewer: Idan Shaby 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Jenkins CI RO
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Tal Nisan 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: storage: add support for blkdiscard command

2016-05-03 Thread automation 
gerrit-hooks has posted comments on this change.

Change subject: storage: add support for blkdiscard command
..


Patch Set 4:

* 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'])

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I2ea7dd19fadc600b8fe78fb436ae430d35f52165
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Federico Simoncelli 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Idan Shaby 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Jenkins CI RO
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches