Change in vdsm[master]: jobs: Fix abort semantics

[alitke]

Adam Litke has posted comments on this change.

Change subject: jobs: Fix abort semantics
..


Patch Set 6: Verified+1

Verified with unit tests.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I801082c50b10cf0571210d65cd3a5cec0d282a5c
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Shahar Havivi 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: jobs: Fix abort semantics

[nsoffer]

Nir Soffer has posted comments on this change.

Change subject: jobs: Fix abort semantics
..


Patch Set 6: Code-Review+2

Lets make some progress.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I801082c50b10cf0571210d65cd3a5cec0d282a5c
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Shahar Havivi 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: qemuimg: Explicit run semantics for QemuImgOperation

[automation]

gerrit-hooks has posted comments on this change.

Change subject: qemuimg: Explicit run semantics for QemuImgOperation
..


Patch Set 6:

* Update Tracker::IGNORE, no bug url/s found
* Check Bug-Url::IGNORE, not relevant for branch: master
* Check Public Bug::WARN, no public bug url found
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 
'ovirt-4.0'])

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I8e427c909a8dffc3ba2a5c838c7d1e7ce7cce55d
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: storage: Fix abort race in SDM.copy_data

[automation]

gerrit-hooks has posted comments on this change.

Change subject: storage: Fix abort race in SDM.copy_data
..


Patch Set 6:

* Update Tracker::IGNORE, no bug url/s found
* Check Bug-Url::IGNORE, not relevant for branch: master
* Check Public Bug::WARN, no public bug url found
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 
'ovirt-4.0'])

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I0c7d9aadb981e8b851a70c7ad0fdc0d75b46dc51
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: jobs: Fix abort semantics

[automation]

gerrit-hooks has posted comments on this change.

Change subject: jobs: Fix abort semantics
..


Patch Set 6:

* Update Tracker::IGNORE, no bug url/s found
* Check Bug-Url::IGNORE, not relevant for branch: master
* Check Public Bug::WARN, no public bug url found
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 
'ovirt-4.0'])

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I801082c50b10cf0571210d65cd3a5cec0d282a5c
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Shahar Havivi 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: jobs: Fix abort semantics

[alitke]

Adam Litke has posted comments on this change.

Change subject: jobs: Fix abort semantics
..


Patch Set 5:

(1 comment)

https://gerrit.ovirt.org/#/c/65102/5/lib/vdsm/jobs.py
File lib/vdsm/jobs.py:

Line 237: """
Line 238: raise NotImplementedError()
Line 239: 
Line 240: def _autodelete_if_required(self):
Line 241: if self.autodelete:
> We need a log here:
Done
Line 242: timeout = config.getint("jobs", "autodelete_delay")
Line 243: if timeout >= 0:
Line 244: _scheduler.schedule(timeout, self._delete)
Line 245: 


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I801082c50b10cf0571210d65cd3a5cec0d282a5c
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Shahar Havivi 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: [WIP] use libvirt domain xml prepared in advance

[ahadas]

Arik Hadas has posted comments on this change.

Change subject: [WIP] use libvirt domain xml prepared in advance
..


Patch Set 2:

(1 comment)

https://gerrit.ovirt.org/#/c/65182/2/vdsm/virt/vmxml.py
File vdsm/virt/vmxml.py:

Line 219: self._devices = Element('devices')
Line 220: self.dom.appendChild(self._devices)
Line 221: 
Line 222: self.appendMetadata()
Line 223: self.doc.appendChild(self.dom)
> I think this is redundant because minidom implicitely refers to the main Do
frankly, I don't remember.. I'll try without it
Line 224: 
Line 225: def appendClock(self):
Line 226: """
Line 227: Add  element to domain:


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I469fad3ca14a6b7f4675ef5c200175053f6dd4af
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Arik Hadas 
Gerrit-Reviewer: Arik Hadas 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Michal Skrivanek 
Gerrit-Reviewer: Milan Zamazal 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: qemuimg: Expose API for qemuimg map

[automation]

gerrit-hooks has posted comments on this change.

Change subject: qemuimg: Expose API for qemuimg map
..


Patch Set 7:

* Update Tracker::IGNORE, no bug url/s found
* Set MODIFIED::IGNORE, no Bug-Url found.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I54d6c936239d3dcc7a3d236dbaab0a93501ada8c
Gerrit-PatchSet: 7
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ala Hino 
Gerrit-Reviewer: Ala Hino 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: qemuimg: Expose API for qemuimg map

[nsoffer]

Nir Soffer has submitted this change and it was merged.

Change subject: qemuimg: Expose API for qemuimg map
..


qemuimg: Expose API for qemuimg map

Change-Id: I54d6c936239d3dcc7a3d236dbaab0a93501ada8c
Signed-off-by: Nir Soffer 
Signed-off-by: Ala Hino 
Reviewed-on: https://gerrit.ovirt.org/65112
Continuous-Integration: Jenkins CI
---
M lib/vdsm/qemuimg.py
M tests/qemuimg_test.py
2 files changed, 134 insertions(+), 0 deletions(-)

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



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

Gerrit-MessageType: merged
Gerrit-Change-Id: I54d6c936239d3dcc7a3d236dbaab0a93501ada8c
Gerrit-PatchSet: 7
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ala Hino 
Gerrit-Reviewer: Ala Hino 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: qemuimg: Expose API for qemuimg map

[nsoffer]

Nir Soffer has posted comments on this change.

Change subject: qemuimg: Expose API for qemuimg map
..


Patch Set 6: Verified+1

Tests pass, so this must verified.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I54d6c936239d3dcc7a3d236dbaab0a93501ada8c
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ala Hino 
Gerrit-Reviewer: Ala Hino 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: qemuimg: Expose API for qemuimg map

[nsoffer]

Nir Soffer has posted comments on this change.

Change subject: qemuimg: Expose API for qemuimg map
..


Patch Set 6: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I54d6c936239d3dcc7a3d236dbaab0a93501ada8c
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ala Hino 
Gerrit-Reviewer: Ala Hino 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: virt: Make DomainDescriptor use XML helpers

[nsoffer]

Nir Soffer has posted comments on this change.

Change subject: virt: Make DomainDescriptor use XML helpers
..


Patch Set 9:

(1 comment)

Very partial review, will look at it again next week.

https://gerrit.ovirt.org/#/c/55769/9/vdsm/virt/domain_descriptor.py
File vdsm/virt/domain_descriptor.py:

Line 25: def __init__(self, xmlStr):
Line 26: self._xml = xmlStr
Line 27: self._dom = vmxml.parse_xml(xmlStr)
Line 28: self._devices = vmxml.find_first(self._dom, 'devices', None)
Line 29: self._devices_hash = hash(vmxml.export_xml(self._devices)
parse/export? litle strange. How about parse/format?
Line 30:   if self._devices is not None else '')
Line 31: 
Line 32: @classmethod
Line 33: def from_id(cls, uuid):


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib169735936d19171ff8b8d127666d7258c308f34
Gerrit-PatchSet: 9
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Milan Zamazal 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Polednik 
Gerrit-Reviewer: Milan Zamazal 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: tests: Remove cPopenTests.py

[igoihman]

Irit Goihman has posted comments on this change.

Change subject: tests: Remove cPopenTests.py
..


Patch Set 2: Code-Review+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I9dc384d6e84ce5f09573be32ea6247c6ef834836
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Edward Haas 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Irit Goihman 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: Yaniv Bronhaim 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: build: Run the tests with tox

[igoihman]

Irit Goihman has posted comments on this change.

Change subject: build: Run the tests with tox
..


Patch Set 8: Code-Review+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I245a171940a5e869fd719a6410024ee77e8ad86c
Gerrit-PatchSet: 8
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Edward Haas 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Irit Goihman 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: Yaniv Bronhaim 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: virt: Try to detect non guest iniated shutdowns

[vfeenstr]

Vinzenz Feenstra has posted comments on this change.

Change subject: virt: Try to detect non guest iniated shutdowns
..


Patch Set 7:

Verification:

EL7 guest with guest agent supporting the messages:


When VM gets killed on the host with `kill -15 $PID`:

2016-10-13 16:49:17,105 INFO(libvirtEventLoop) [virt.vm] 
vmId=`03cfa007-f94c-4762-b497-a3c5fcd1b716`::Changed state to Down: VM has been 
terminated on the host (code=11) (vm:1166)

When shutdown from within the vm: 

libvirtEventLoop::INFO::2016-10-13 
16:36:41,423::vm::1150::virt.vm::(setDownStatus) 
vmId=`03cfa007-f94c-4762-b497-a3c5fcd1b716`::Changed state to Down: User shut 
down from within the guest (code=7)

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie04b9806fbf0a81dc576aa28cfdda5edb079ce29
Gerrit-PatchSet: 7
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Vinzenz Feenstra 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Michal Skrivanek 
Gerrit-Reviewer: Milan Zamazal 
Gerrit-Reviewer: Vinzenz Feenstra 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: vdsm: virt: add optional container support

[fromani]

Francesco Romani has posted comments on this change.

Change subject: vdsm: virt: add optional container support
..


Patch Set 51:

(1 comment)

https://gerrit.ovirt.org/#/c/53820/51/lib/vdsm/containerslib.py
File lib/vdsm/containerslib.py:

PS51, Line 99: def _is_cmd(argv, reqv):
> Yeah I'm slightly torn. I understand only coupling state dependent function
Looks slightly better if is a method, because of the functionality link.

A nested function would probably be the better tool, but makes
_execute too hard to read. let's stick with the method.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Id236a30a5c875994c037b8d00c7463bceaab143f
Gerrit-PatchSet: 51
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Polednik 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: virt: Try to detect non guest iniated shutdowns

[vfeenstr]

Vinzenz Feenstra has posted comments on this change.

Change subject: virt: Try to detect non guest iniated shutdowns
..


Patch Set 7: Verified+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie04b9806fbf0a81dc576aa28cfdda5edb079ce29
Gerrit-PatchSet: 7
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Vinzenz Feenstra 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Michal Skrivanek 
Gerrit-Reviewer: Milan Zamazal 
Gerrit-Reviewer: Vinzenz Feenstra 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: vdsm: virt: add optional container support

[mpolednik]

Martin Polednik has posted comments on this change.

Change subject: vdsm: virt: add optional container support
..


Patch Set 51:

(1 comment)

https://gerrit.ovirt.org/#/c/53820/51/lib/vdsm/containerslib.py
File lib/vdsm/containerslib.py:

PS51, Line 99: def _is_cmd(argv, reqv):
> true, but OTOH this code doesn't use the SuperVdsmCommand state at all, so 
Yeah I'm slightly torn. I understand only coupling state dependent 
functionality, but still dislike top-level (although "protected") functions 
that do share the "hidden" functionality link to some object. Not sure if it 
doesn't cross the edge of being personal dislike rather than proper improvement.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Id236a30a5c875994c037b8d00c7463bceaab143f
Gerrit-PatchSet: 51
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Polednik 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: build: Run the tests with tox

[fromani]

Francesco Romani has posted comments on this change.

Change subject: build: Run the tests with tox
..


Patch Set 8: Code-Review+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I245a171940a5e869fd719a6410024ee77e8ad86c
Gerrit-PatchSet: 8
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Edward Haas 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Irit Goihman 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: Yaniv Bronhaim 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: automation: Create coverage report sooner

[fromani]

Francesco Romani has posted comments on this change.

Change subject: automation: Create coverage report sooner
..


Patch Set 1: Code-Review+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I8688383eb5e99fdd0ee48815cc71afd6483819b4
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer 
Gerrit-Reviewer: Edward Haas 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: build: Replace pep8 and pyflakes with flake8

[fromani]

Francesco Romani has posted comments on this change.

Change subject: build: Replace pep8 and pyflakes with flake8
..


Patch Set 2: Code-Review+1

good idea, I like this.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib42430f045e4475c31cd370b4196214094c29cd1
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Edward Haas 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Irit Goihman 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: Yaniv Bronhaim 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: config: add tunables for container support

[mpolednik]

Martin Polednik has posted comments on this change.

Change subject: config: add tunables for container support
..


Patch Set 4: Code-Review+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I30e4372fc88ebb7e68cc3b982af3c2eefacef7ce
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Polednik 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: tests: Remove cPopenTests.py

[fromani]

Francesco Romani has posted comments on this change.

Change subject: tests: Remove cPopenTests.py
..


Patch Set 2: Code-Review+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I9dc384d6e84ce5f09573be32ea6247c6ef834836
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Edward Haas 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Irit Goihman 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: Yaniv Bronhaim 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: tests: Refine TerminationTests names

[fromani]

Francesco Romani has posted comments on this change.

Change subject: tests: Refine TerminationTests names
..


Patch Set 6: Code-Review+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Id085d7bbc65fe985454627e0958eb03d0e413b4d
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: Yaniv Bronhaim 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: utils: Raise detectable error if termination fail

[fromani]

Francesco Romani has posted comments on this change.

Change subject: utils: Raise detectable error if termination fail
..


Patch Set 6: Code-Review+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I5cfa5434ce011b9185550884233b5b233026d13c
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: Yaniv Bronhaim 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: qemuimg: Expose API for qemuimg map

[ahino]

Ala Hino has posted comments on this change.

Change subject: qemuimg: Expose API for qemuimg map
..


Patch Set 5:

(7 comments)

https://gerrit.ovirt.org/#/c/65112/5/tests/qemuimg_test.py
File tests/qemuimg_test.py:

Line 459: # Empty run
Line 460: run = img_map[0]
Line 461: self.assertEqual(run["length"], size)
Line 462: self.assertEqual(run["data"], False)
Line 463: self.assertEqual(run["zero"], True)
> Lets use the same format as in the other tests, and call the new shiny help
Done
Line 464: 
Line 465: @permutations([
Line 466: # offset, qcow2_compat
Line 467: (4 * 1024, "0.10"),


Line 462: self.assertEqual(run["data"], False)
Line 463: self.assertEqual(run["zero"], True)
Line 464: 
Line 465: @permutations([
Line 466: # offset, qcow2_compat
> This is length, not offset.
Done
Line 467: (4 * 1024, "0.10"),
Line 468: (4 * 1024, "1.1"),
Line 469: (64 * 1024, "0.10"),
Line 470: (64 * 1024, "1.1"),


Line 479: qemu_pattern_write(image, fmt, offset=offset, len=length,
Line 480:pattern=0xf0)
Line 481: 
Line 482: img_map = qemuimg.map(image)
Line 483: self.assertEqual(3, len(img_map))
> We can move this check into the helper:
Done
Line 484: 
Line 485: expected = [
Line 486: # run 1 - empty
Line 487: {


Line 508: ]
Line 509: 
Line 510: for actual, expected in zip(img_map, expected):
Line 511: for key in expected:
Line 512: self.assertEqual(actual[key], expected[key])
> We have same verification code twice, and we actually want to use it for th
Done
Line 513: 
Line 514: @permutations([
Line 515: # offset, length, expected_length, expected_start, 
qcow2_compat
Line 516: (64 * 1024, 4 * 1024, 65536, "0.10"),


Line 528: qemu_pattern_write(image, fmt, offset=offset, len=length,
Line 529:pattern=0xf0)
Line 530: 
Line 531: img_map = qemuimg.map(image)
Line 532: self.assertEqual(3, len(img_map))
> Move to check_map helper
Done
Line 533: 
Line 534: expected = [
Line 535: # run 1 - empty
Line 536: {


Line 541: },
Line 542: # run 2 - data
Line 543: {
Line 544: "start": offset,
Line 545: "offset": 327680,
> Lets add a comment abut this (no clue what is this value), or just remove i
Removed ...
Line 546: "length": expected_length,
Line 547: "data": True,
Line 548: "zero": False,
Line 549: },


Line 557: ]
Line 558: 
Line 559: for actual, expected in zip(img_map, expected):
Line 560: for key in expected:
Line 561: self.assertEqual(actual[key], expected[key])
> Repalce wiith call to check_map
Done
Line 562: 
Line 563: 
Line 564: def make_image(path, size, format, index, qcow2_compat, backing=None):
Line 565: qemuimg.create(path, size=size, format=format, 
qcow2Compat=qcow2_compat,


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I54d6c936239d3dcc7a3d236dbaab0a93501ada8c
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ala Hino 
Gerrit-Reviewer: Ala Hino 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: qemuimg: Expose API for qemuimg map

[automation]

gerrit-hooks has posted comments on this change.

Change subject: qemuimg: Expose API for qemuimg map
..


Patch Set 6:

* Update Tracker::IGNORE, no bug url/s found
* Check Bug-Url::IGNORE, not relevant for branch: master
* Check Public Bug::WARN, no public bug url found
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 
'ovirt-4.0'])

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I54d6c936239d3dcc7a3d236dbaab0a93501ada8c
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ala Hino 
Gerrit-Reviewer: Ala Hino 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: virt: Make DomainDescriptor use XML helpers

[fromani]

Francesco Romani has posted comments on this change.

Change subject: virt: Make DomainDescriptor use XML helpers
..


Patch Set 9: Code-Review+2

*Really* nice improvements. I like this.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib169735936d19171ff8b8d127666d7258c308f34
Gerrit-PatchSet: 9
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Milan Zamazal 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Polednik 
Gerrit-Reviewer: Milan Zamazal 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: virt: Try to detect non guest iniated shutdowns

[fromani]

Francesco Romani has posted comments on this change.

Change subject: virt: Try to detect non guest iniated shutdowns
..


Patch Set 7: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie04b9806fbf0a81dc576aa28cfdda5edb079ce29
Gerrit-PatchSet: 7
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Vinzenz Feenstra 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Michal Skrivanek 
Gerrit-Reviewer: Milan Zamazal 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: [WIP] use libvirt domain xml prepared in advance

[fromani]

Francesco Romani has posted comments on this change.

Change subject: [WIP] use libvirt domain xml prepared in advance
..


Patch Set 2: -Code-Review

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I469fad3ca14a6b7f4675ef5c200175053f6dd4af
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Arik Hadas 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Michal Skrivanek 
Gerrit-Reviewer: Milan Zamazal 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: [WIP] use libvirt domain xml prepared in advance

[fromani]

Francesco Romani has posted comments on this change.

Change subject: [WIP] use libvirt domain xml prepared in advance
..


Patch Set 2: Code-Review+1

(1 comment)

https://gerrit.ovirt.org/#/c/65182/2/vdsm/virt/vmxml.py
File vdsm/virt/vmxml.py:

Line 219: self._devices = Element('devices')
Line 220: self.dom.appendChild(self._devices)
Line 221: 
Line 222: self.appendMetadata()
Line 223: self.doc.appendChild(self.dom)
I think this is redundant because minidom implicitely refers to the main 
Document.

Or this line fixes an issue you actually found?
Line 224: 
Line 225: def appendClock(self):
Line 226: """
Line 227: Add  element to domain:


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I469fad3ca14a6b7f4675ef5c200175053f6dd4af
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Arik Hadas 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Michal Skrivanek 
Gerrit-Reviewer: Milan Zamazal 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: tests: use assertXMLEqual in vmTests

[nsoffer]

Nir Soffer has posted comments on this change.

Change subject: tests: use assertXMLEqual in vmTests
..


Patch Set 1: Code-Review+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I6516404548eb000f9501818818c929df09c406e2
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: tests: use assertXMLEqual in vmTests

[nsoffer]

Nir Soffer has posted comments on this change.

Change subject: tests: use assertXMLEqual in vmTests
..


Patch Set 1:

Looks good, need to make pep8 happy.

./tests/vmTests.py:636:80: E501 line too long (81 > 79 characters)
./tests/vmTests.py:832:80: E501 line too long (81 > 79 characters)

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I6516404548eb000f9501818818c929df09c406e2
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: static: move man under static

[fromani]

Francesco Romani has posted comments on this change.

Change subject: static: move man under static
..


Patch Set 5: Code-Review+2

same answer as per parent patch, no reason to hold +2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I91fa36b38904d85fc60734ce966b8d993911b490
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Martin Polednik 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Polednik 
Gerrit-Reviewer: Yaniv Bronhaim 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: static: move vdsm bonding defaults under static

[fromani]

Francesco Romani has posted comments on this change.

Change subject: static: move vdsm bonding defaults under static
..


Patch Set 6: Code-Review+2

(1 comment)

https://gerrit.ovirt.org/#/c/62537/6/static/Makefile.am
File static/Makefile.am:

PS6, Line 24: /usr/share
> It's referring to a source path, not the target. Target is vdsmdir.
Right, I was too hasty reading.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I9cbd81e59aec49876e15337d7cd617e51316a604
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Martin Polednik 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Polednik 
Gerrit-Reviewer: Ondřej Svoboda 
Gerrit-Reviewer: Petr Horáček 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: network: filter out 'veth' devices

[automation]

gerrit-hooks has posted comments on this change.

Change subject: network: filter out 'veth' devices
..


Patch Set 20:

* Update Tracker::IGNORE, no bug url/s found
* Check Bug-Url::IGNORE, not relevant for branch: master
* Check Public Bug::WARN, no public bug url found
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 
'ovirt-4.0'])

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I859c4bc885c0afd99fdaf741706d9bd1538850e6
Gerrit-PatchSet: 20
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Edward Haas 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: core: containers: add the container support module

[automation]

gerrit-hooks has posted comments on this change.

Change subject: core: containers: add the container support module
..


Patch Set 35:

* Update Tracker::IGNORE, no bug url/s found
* Check Bug-Url::IGNORE, not relevant for branch: master
* Check Public Bug::WARN, no public bug url found
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 
'ovirt-4.0'])

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I0fb768ea97dd719cde9bd5e57e1b7cabe4b0f0ae
Gerrit-PatchSet: 35
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Polednik 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: vdsm: virt: add optional container support

[automation]

gerrit-hooks has posted comments on this change.

Change subject: vdsm: virt: add optional container support
..


Patch Set 53:

* Update Tracker::IGNORE, no bug url/s found
* Check Bug-Url::IGNORE, not relevant for branch: master
* Check Public Bug::WARN, no public bug url found
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 
'ovirt-4.0'])

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Id236a30a5c875994c037b8d00c7463bceaab143f
Gerrit-PatchSet: 53
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Polednik 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: supervdsm: expose systemd utilities

[automation]

gerrit-hooks has posted comments on this change.

Change subject: supervdsm: expose systemd utilities
..


Patch Set 33:

* Update Tracker::IGNORE, no bug url/s found
* Check Bug-Url::IGNORE, not relevant for branch: master
* Check Public Bug::WARN, no public bug url found
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 
'ovirt-4.0'])

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I38e9a346da784fc200a82d9e5d9fdf665e752987
Gerrit-PatchSet: 33
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Polednik 
Gerrit-Reviewer: Milan Zamazal 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: network: supervdsm: configure container networks

[automation]

gerrit-hooks has posted comments on this change.

Change subject: network: supervdsm: configure container networks
..


Patch Set 46:

* Update Tracker::IGNORE, no bug url/s found
* Check Bug-Url::IGNORE, not relevant for branch: master
* Check Public Bug::WARN, no public bug url found
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 
'ovirt-4.0'])

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I338ca2d3abb0b1447c5a18c97afb9e14314f4107
Gerrit-PatchSet: 46
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Polednik 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: **WIP** tool: reconfigure containers networks

[automation]

gerrit-hooks has posted comments on this change.

Change subject: **WIP** tool: reconfigure containers networks
..


Patch Set 19:

* Update Tracker::IGNORE, no bug url/s found
* Check Bug-Url::IGNORE, not relevant for branch: master
* Check Public Bug::WARN, no public bug url found
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 
'ovirt-4.0'])

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I6053e283c004cd61ba7727cea22ba73a631180ba
Gerrit-PatchSet: 19
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: WIP: docs: add tutorial

[automation]

gerrit-hooks has posted comments on this change.

Change subject: WIP: docs: add tutorial
..


Patch Set 2:

* Update Tracker::IGNORE, no bug url/s found
* Check Bug-Url::IGNORE, not relevant for branch: master
* Check Public Bug::WARN, no public bug url found
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 
'ovirt-4.0'])

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I3a9e947c30f2978fd9670fd64d99bf380181aa9c
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: config: add tunables for container support

[automation]

gerrit-hooks has posted comments on this change.

Change subject: config: add tunables for container support
..


Patch Set 4:

* Update Tracker::IGNORE, no bug url/s found
* Check Bug-Url::IGNORE, not relevant for branch: master
* Check Public Bug::WARN, no public bug url found
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 
'ovirt-4.0'])

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I30e4372fc88ebb7e68cc3b982af3c2eefacef7ce
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Polednik 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: vmxml: export container metadata

[automation]

gerrit-hooks has posted comments on this change.

Change subject: vmxml: export container metadata
..


Patch Set 25:

* Update Tracker::IGNORE, no bug url/s found
* Check Bug-Url::IGNORE, not relevant for branch: master
* Check Public Bug::WARN, no public bug url found
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 
'ovirt-4.0'])

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I1ade3c0c7d300c5ce33cb23723c3d0e59e4af664
Gerrit-PatchSet: 25
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Polednik 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: tests: containers: add testsuite

[automation]

gerrit-hooks has posted comments on this change.

Change subject: tests: containers: add testsuite
..


Patch Set 23:

* Update Tracker::IGNORE, no bug url/s found
* Check Bug-Url::IGNORE, not relevant for branch: master
* Check Public Bug::WARN, no public bug url found
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 
'ovirt-4.0'])

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I27ba3cecbd71b7bbba94992d6bc63ca29333e313
Gerrit-PatchSet: 23
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: tests: Add tests for poll and wait failures

[nsoffer]

Nir Soffer has posted comments on this change.

Change subject: tests: Add tests for poll and wait failures
..


Patch Set 8:

(1 comment)

https://gerrit.ovirt.org/#/c/65294/8/tests/utilsTests.py
File tests/utilsTests.py:

Line 132: self.check_failure()
Line 133: 
Line 134: def test_kill_failure(self):
Line 135: def fail():
Line 136: raise ExpectedFailure("Fake kill failure")
> Why not to have fail method as top level and use it 138 and in 145 line. We
Because it raises different exceptions with different message?
Line 137: 
Line 138: self.proc.kill = fail
Line 139: self.check_failure()
Line 140: 


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I8674c9c3c2118041c74213cd8ce0d383086d6cbf
Gerrit-PatchSet: 8
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: Yaniv Bronhaim 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: tests: Add tests for waiting on a zombie process

[nsoffer]

Nir Soffer has posted comments on this change.

Change subject: tests: Add tests for waiting on a zombie process
..


Patch Set 6:

(1 comment)

https://gerrit.ovirt.org/#/c/65327/6/tests/utilsTests.py
File tests/utilsTests.py:

Line 102: self.proc.terminate()
Line 103: wait_for_zombie(self.proc, 1)
Line 104: 
Line 105: def fail():
Line 106: raise RuntimeError("Attempt to kill a zombie process")
> In patch #65328 we use RuntimeError as well. We mentioned in commit message
Yes, but with different error text - this should make debugging the tests 
easier in case of errors.

We can do something like this:

def fail(exception, msg):
raise exception(msg)

And then n each test we can do:

self.proc.kill = partial(fail, RuntimeError,
"Attempt to kill a zombie process")

But I find this much harder to understand compared to:

def fail():
raise RuntimeError("Attempt to kill a zombie process")
   
self.proc.kill = fail

The best would be to use lambda:

   self.proc.kill = lambda: raise RuntimeError("Attempt to kill a zombie 
process")

But lambda does not support this syntax.
Line 107: 
Line 108: self.proc.kill = fail
Line 109: with utils.terminating(self.proc):
Line 110: pass


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I238d77213153b1fa98bb3f52b33a6fd78bb57bca
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: Yaniv Bronhaim 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: utils: Raise detectable error if termination fail

[nsoffer]

Nir Soffer has posted comments on this change.

Change subject: utils: Raise detectable error if termination fail
..


Patch Set 5:

(1 comment)

https://gerrit.ovirt.org/#/c/65325/5//COMMIT_MSG
Commit Message:

Line 14: job running the process must stay in running state to prevent engine
Line 15: from starting the same operation on another machine.
Line 16: 
Line 17: Typically when we cannot handle an error we should not handle it. But
Line 18: in this special case, letting the caller handle the error is not useful
> OK, using your text.
With some modifications, hopefully this is more clear in the current version.
Line 19: since it does not have any context.
Line 20: 
Line 21: To make it possible to handle this critical error, we raise now a new
Line 22: TerminatingFallure exception with process pid and the original error.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I5cfa5434ce011b9185550884233b5b233026d13c
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: Yaniv Bronhaim 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: utils: Raise detectable error if termination fail

[nsoffer]

Nir Soffer has posted comments on this change.

Change subject: utils: Raise detectable error if termination fail
..


Patch Set 5:

(1 comment)

https://gerrit.ovirt.org/#/c/65325/5//COMMIT_MSG
Commit Message:

Line 14: job running the process must stay in running state to prevent engine
Line 15: from starting the same operation on another machine.
Line 16: 
Line 17: Typically when we cannot handle an error we should not handle it. But
Line 18: in this special case, letting the caller handle the error is not useful
> My confusion is that we say handling error is not useful but later we say t
OK, using your text.
Line 19: since it does not have any context.
Line 20: 
Line 21: To make it possible to handle this critical error, we raise now a new
Line 22: TerminatingFallure exception with process pid and the original error.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I5cfa5434ce011b9185550884233b5b233026d13c
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: Yaniv Bronhaim 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: network: supervdsm: configure container networks

[fromani]

Francesco Romani has posted comments on this change.

Change subject: network: supervdsm: configure container networks
..


Patch Set 44:

> The code is somewhat fine considering the TODOs, but question remains: if I 
> install vdsm-containers AFTER already using VDSM, do I explicitly have to 
> setup_networks again? How is user informed?

This is material for another TODO :)

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I338ca2d3abb0b1447c5a18c97afb9e14314f4107
Gerrit-PatchSet: 44
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Polednik 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: copy_data: Add qcow2_compat on convert.

[nsoffer]

Nir Soffer has posted comments on this change.

Change subject: copy_data: Add qcow2_compat on convert.
..


Patch Set 24:

(3 comments)

Nice!

https://gerrit.ovirt.org/#/c/64373/24/tests/storage_sdm_copy_data_test.py
File tests/storage_sdm_copy_data_test.py:

Line 76: 
Line 77: @contextmanager
Line 78: def get_vols(self, storage_type, src_fmt, dst_fmt, chain_length=1,
Line 79:  size=DEFAULT_SIZE, sd_version=3, 
src_qcow2_compat='0.10'):
Line 80: with fake_env(storage_type, sd_version) as env:
sd_version is a kwarg, please always call it sd_version=value
Line 81: rm = FakeResourceManager()
Line 82: with MonkeyPatchScope([
Line 83: (guarded, 'context', fake_guarded_context()),
Line 84: (storage.sdm.api.copy_data, 'sdCache', env.sdcache),


Line 197: ('block', 'cow', 'cow', '1.1', 4),
Line 198: ('block', 'cow', 'cow', '0.10', 3),
Line 199: ))
Line 200: def test_volume_qcow2v3(self, env_type, src_fmt, dst_fmt, 
src_qcow2_compat,
Line 201: dst_sd_version):
In this test we are actually testing coping in the same domain, so the source 
sd compat and destination sd compat are the same.

It would be more interesting to test copying form old domain (version=3) to a 
new domain (version=4), or the opposite (from 4 to 3), but we don't have 
infrastructure for this yet.

Please add a TODO for these tests.
Line 202: src_fmt = sc.name2type(src_fmt)
Line 203: dst_fmt = sc.name2type(dst_fmt)
Line 204: job_id = make_uuid()
Line 205: 


Line 220: job = storage.sdm.api.copy_data.Job(job_id, 0, source, 
dest)
Line 221: 
Line 222: job.run()
Line 223: wait_for_job(job)
Line 224: self.assertEqual(jobs.STATUS.DONE, job.status)
Please add:

verify_qemu_chain(dst_chain)

And also check that the destination images are using the correct compat. You 
can do this using qemuimg.info(path)["compat"]
Line 225: 
Line 226: def test_bad_vm_configuration_volume(self):
Line 227: """
Line 228: When copying a volume containing VM configuration information 
the


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie26e5dcba6fc493b32ea7764889df2918c4dfdd3
Gerrit-PatchSet: 24
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Maor Lipchuk 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Maor Lipchuk 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: vdsm: virt: add optional container support

[fromani]

Francesco Romani has posted comments on this change.

Change subject: vdsm: virt: add optional container support
..


Patch Set 51:

(9 comments)

https://gerrit.ovirt.org/#/c/53820/51/lib/vdsm/containersconnection.py
File lib/vdsm/containersconnection.py:

PS51, Line 33: _runtimes = frozenset()
> I have inner hate for how this is further "calculated" (also considering th
no, we don't really need a frozenset, we can probably kill it.


https://gerrit.ovirt.org/#/c/53820/51/lib/vdsm/containerslib.py
File lib/vdsm/containerslib.py:

PS51, Line 99: def _is_cmd(argv, reqv):
> Why this isn't attribute of SuperVdsmCommand class? It's slight implementat
true, but OTOH this code doesn't use the SuperVdsmCommand state at all, so 
could also be a free function


PS51, Line 113: def _call(ret):
> Same as above, I believe it should be __call__ on top of that (internally c
SubProcCommand.__call__ already does that!

Again, this is a free function because it doesn't use any of the 
SuperVdsmCommand state.
Anyway it's open for discussion, I'm not entirely happy with both designs. If 
you think it's clearer as method, I'll make this a method (same for _is_cmd 
above)


PS51, Line 153:  
> container connection
Done


PS51, Line 161: ev
> s/ev/event
Done


PS51, Line 177: For clearing connections during the tests.
> Only? Runtime code shouldn't be polluted by test helpers.
And for consistency with libvirtconnection. But we don't really care here, so 
let's kill this.


https://gerrit.ovirt.org/#/c/53820/51/vdsm/virt/recovery.py
File vdsm/virt/recovery.py:

PS51, Line 167: all_vms
> s/vms/domains ?
yes, a bit better. Changed.


PS51, Line 177: def _all_vms_from_runtime(cif):
> s/vms/domains ?
_all_running_domains(cif) is probably clearer and a better name


https://gerrit.ovirt.org/#/c/53820/51/vdsm/virt/vm.py
File vdsm/virt/vm.py:

PS51, Line 1657: if is_kvm(self.conf):
> My vote for split or helper function remains.
Let's retry with a different partiotioning.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Id236a30a5c875994c037b8d00c7463bceaab143f
Gerrit-PatchSet: 51
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Polednik 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: build: Replace pep8 and pyflakes with flake8

[edwardh]

Edward Haas has posted comments on this change.

Change subject: build: Replace pep8 and pyflakes with flake8
..


Patch Set 2:

What do you mean it is not relevant?
We need it installed once for py2 and once for py3 so tests will pass, no? Does 
tox take care of that somehow?

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib42430f045e4475c31cd370b4196214094c29cd1
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Edward Haas 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Irit Goihman 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: Yaniv Bronhaim 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: after_vm_destroy.py: migrate to jsonrpcvdscli

[automation]

gerrit-hooks has posted comments on this change.

Change subject: after_vm_destroy.py: migrate to jsonrpcvdscli
..


Patch Set 7:

* Update Tracker::IGNORE, no bug url/s found
* Check Bug-Url::IGNORE, not relevant for branch: master
* Check Public Bug::WARN, no public bug url found
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 
'ovirt-4.0'])

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I3f3fa6479dde2c4a1298d0ae167d888d9f7e020a
Gerrit-PatchSet: 7
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Irit Goihman 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Irit Goihman 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Marcin Mirecki 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: Yaniv Bronhaim 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: hsm:Use sd compat instead of qemu conf compat.

[nsoffer]

Nir Soffer has posted comments on this change.

Change subject: hsm:Use sd compat instead of qemu conf compat.
..


Patch Set 10:

(4 comments)

https://gerrit.ovirt.org/#/c/64951/10/tests/storage_hsm_test.py
File tests/storage_hsm_test.py:

Line 98
Line 99
Line 100
Line 101
Line 102
But you check that the code raises?!


Line 88: ('0.10', '0.10', 4),
Line 89: ('1.1', '0.10', 4),
Line 90: ('0.10', '0.10', 3),
Line 91: ))
Line 92: def test_disabled_compat_raises(self, hsm_compat, qemu_config, 
sd_version):
This test check the invalid compat raises...
Line 93: with self.fake_volume(sc.COW_FORMAT, sd_version) as vol:
Line 94: create_conf = make_config([('irs', 'qcow2_compat', 
qemu_config)])
Line 95: info = {"format": qemuimg.FORMAT.QCOW2, "compat": 
hsm_compat}
Line 96: with MonkeyPatchScope([(qemuimg, 'config', create_conf),


Line 98: qemuimg.create(vol.volumePath, size=self.SIZE,
Line 99:format=qemuimg.FORMAT.QCOW2)
Line 100: h = FakeHSM()
Line 101: self.assertNotRaises(h.verify_untrusted_volume, 'sp',
Line 102:  vol.sdUUID, vol.imgUUID, 
vol.volUUID)
But you check that the code does not raise?!
Line 103: 
Line 104: @permutations((
Line 105: ('1.1', '0.10', 3),
Line 106: ))


Line 103: 
Line 104: @permutations((
Line 105: ('1.1', '0.10', 3),
Line 106: ))
Line 107: def test_valid_compat(self, hsm_compat, qemu_config, sd_version):
This test should test valid compats...
Line 108: with self.fake_volume(sc.COW_FORMAT, sd_version) as vol:
Line 109: create_conf = make_config([('irs', 'qcow2_compat', 
qemu_config)])
Line 110: info = {"format": qemuimg.FORMAT.QCOW2, "compat": 
hsm_compat}
Line 111: with MonkeyPatchScope([(qemuimg, 'config', create_conf),


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie4b19a99e5d9a73c011bf6d8079e3855298561b9
Gerrit-PatchSet: 10
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Maor Lipchuk 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Amit Aviram 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Maor Lipchuk 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: tests: Add tests for poll and wait failures

[piotr . kliczewski]

Piotr Kliczewski has posted comments on this change.

Change subject: tests: Add tests for poll and wait failures
..


Patch Set 8:

(1 comment)

https://gerrit.ovirt.org/#/c/65294/8/tests/utilsTests.py
File tests/utilsTests.py:

Line 132: self.check_failure()
Line 133: 
Line 134: def test_kill_failure(self):
Line 135: def fail():
Line 136: raise ExpectedFailure("Fake kill failure")
> 3 test, 3 fail texts, please suggest a simple and more clear code.
Why not to have fail method as top level and use it 138 and in 145 line. We do 
not need to have it in two places.
Line 137: 
Line 138: self.proc.kill = fail
Line 139: self.check_failure()
Line 140: 


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I8674c9c3c2118041c74213cd8ce0d383086d6cbf
Gerrit-PatchSet: 8
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: Yaniv Bronhaim 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: storagetestlib: Add sdVersion param to the test API.

[nsoffer]

Nir Soffer has posted comments on this change.

Change subject: storagetestlib: Add sdVersion param to the test API.
..


Patch Set 3:

(1 comment)

https://gerrit.ovirt.org/#/c/65308/3/tests/storagetestlib_test.py
File tests/storagetestlib_test.py:

Line 91: self.assertEquals(3, env.sd_manifest.getVersion())
Line 92: 
Line 93: def test_block_domain_version_4(self):
Line 94: with fake_env('block', sd_version=4) as env:
Line 95: self.assertEquals(4, env.sd_manifest.getVersion())
Nice!

But wouldn't it be even better if we merge to one test with permutations?

@permutations([
# env_type, sd_version
("file", 3),
("file", 4),
("block", 3),
("block", 4),
])
def test_domain_version(self, env_type, sd_version):
with fake_env(env_type, sd_version=sd_version) as env:
self.assertEquals(sd_version, env.sd_manifest.getVersion())

And we can have another test for the default.
Line 96: 
Line 97: def test_volume_structure(self):
Line 98: with fake_file_env() as env:
Line 99: img_id = make_uuid()


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I9759fa6d8b2bd7c359fd188d1e18ffbba5004941
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Maor Lipchuk 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Maor Lipchuk 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: tests: Add tests for waiting on a zombie process

[piotr . kliczewski]

Piotr Kliczewski has posted comments on this change.

Change subject: tests: Add tests for waiting on a zombie process
..


Patch Set 6:

(1 comment)

https://gerrit.ovirt.org/#/c/65327/6/tests/utilsTests.py
File tests/utilsTests.py:

Line 102: self.proc.terminate()
Line 103: wait_for_zombie(self.proc, 1)
Line 104: 
Line 105: def fail():
Line 106: raise RuntimeError("Attempt to kill a zombie process")
> This raises rumtime error and will fail the test if raised.
In patch #65328 we use RuntimeError as well. We mentioned in commit message 
reuse of this method so let's do it.
Line 107: 
Line 108: self.proc.kill = fail
Line 109: with utils.terminating(self.proc):
Line 110: pass


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I238d77213153b1fa98bb3f52b33a6fd78bb57bca
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: Yaniv Bronhaim 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: virt: Try to detect non guest iniated shutdowns

[mzamazal]

Milan Zamazal has posted comments on this change.

Change subject: virt: Try to detect non guest iniated shutdowns
..


Patch Set 7: Code-Review+1

Maybe _shutdownLock is no longer necessary?

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie04b9806fbf0a81dc576aa28cfdda5edb079ce29
Gerrit-PatchSet: 7
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Vinzenz Feenstra 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Michal Skrivanek 
Gerrit-Reviewer: Milan Zamazal 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: net test: fix ovs_test:test_dry_run

[edwardh]

Edward Haas has posted comments on this change.

Change subject: net test: fix ovs_test:test_dry_run
..


Patch Set 1: Code-Review+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ieb5c70c1498bbe85c5fdd9b0cec63761f2cf7c0d
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Petr Horáček 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Edward Haas 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Petr Horáček 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: tests: Refine TerminationTests names

[piotr . kliczewski]

Piotr Kliczewski has posted comments on this change.

Change subject: tests: Refine TerminationTests names
..


Patch Set 6: Code-Review+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Id085d7bbc65fe985454627e0958eb03d0e413b4d
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: Yaniv Bronhaim 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: utils: Raise detectable error if termination fail

[piotr . kliczewski]

Piotr Kliczewski has posted comments on this change.

Change subject: utils: Raise detectable error if termination fail
..


Patch Set 5:

(1 comment)

https://gerrit.ovirt.org/#/c/65325/5//COMMIT_MSG
Commit Message:

Line 14: job running the process must stay in running state to prevent engine
Line 15: from starting the same operation on another machine.
Line 16: 
Line 17: Typically when we cannot handle an error we should not handle it. But
Line 18: in this special case, letting the caller handle the error is not useful
> Tried, I cannot see what is confusing and how to improve this. Please sugge
My confusion is that we say handling error is not useful but later we say that 
by creating new Exception and passing pid is fine. So I would go with:

But in this special case, letting the caller to handle the error without 
context is not useful. By adding new TrminatingFailure exception with pid we 
solve missing context issue.
Line 19: since it does not have any context.
Line 20: 
Line 21: To make it possible to handle this critical error, we raise now a new
Line 22: TerminatingFallure exception with process pid and the original error.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I5cfa5434ce011b9185550884233b5b233026d13c
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: Yaniv Bronhaim 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: qemuimg: Expose API for qemuimg map

[nsoffer]

Nir Soffer has posted comments on this change.

Change subject: qemuimg: Expose API for qemuimg map
..


Patch Set 5:

(7 comments)

Very nice!

https://gerrit.ovirt.org/#/c/65112/5/tests/qemuimg_test.py
File tests/qemuimg_test.py:

Line 459: # Empty run
Line 460: run = img_map[0]
Line 461: self.assertEqual(run["length"], size)
Line 462: self.assertEqual(run["data"], False)
Line 463: self.assertEqual(run["zero"], True)
Lets use the same format as in the other tests, and call the new shiny helper 
to verify.
Line 464: 
Line 465: @permutations([
Line 466: # offset, qcow2_compat
Line 467: (4 * 1024, "0.10"),


Line 462: self.assertEqual(run["data"], False)
Line 463: self.assertEqual(run["zero"], True)
Line 464: 
Line 465: @permutations([
Line 466: # offset, qcow2_compat
This is length, not offset.
Line 467: (4 * 1024, "0.10"),
Line 468: (4 * 1024, "1.1"),
Line 469: (64 * 1024, "0.10"),
Line 470: (64 * 1024, "1.1"),


Line 479: qemu_pattern_write(image, fmt, offset=offset, len=length,
Line 480:pattern=0xf0)
Line 481: 
Line 482: img_map = qemuimg.map(image)
Line 483: self.assertEqual(3, len(img_map))
We can move this check into the helper:

self.assertEqual(len(actual), len(expected))
Line 484: 
Line 485: expected = [
Line 486: # run 1 - empty
Line 487: {


Line 508: ]
Line 509: 
Line 510: for actual, expected in zip(img_map, expected):
Line 511: for key in expected:
Line 512: self.assertEqual(actual[key], expected[key])
We have same verification code twice, and we actually want to use it for the 
first test. Less move this into a helper and use it in all tests.

Something like:

self.check_map(actual, expected)
Line 513: 
Line 514: @permutations([
Line 515: # offset, length, expected_length, expected_start, 
qcow2_compat
Line 516: (64 * 1024, 4 * 1024, 65536, "0.10"),


Line 528: qemu_pattern_write(image, fmt, offset=offset, len=length,
Line 529:pattern=0xf0)
Line 530: 
Line 531: img_map = qemuimg.map(image)
Line 532: self.assertEqual(3, len(img_map))
Move to check_map helper
Line 533: 
Line 534: expected = [
Line 535: # run 1 - empty
Line 536: {


Line 541: },
Line 542: # run 2 - data
Line 543: {
Line 544: "start": offset,
Line 545: "offset": 327680,
Lets add a comment abut this (no clue what is this value), or just remove it 
since we don't depend on this value.
Line 546: "length": expected_length,
Line 547: "data": True,
Line 548: "zero": False,
Line 549: },


Line 557: ]
Line 558: 
Line 559: for actual, expected in zip(img_map, expected):
Line 560: for key in expected:
Line 561: self.assertEqual(actual[key], expected[key])
Repalce wiith call to check_map
Line 562: 
Line 563: 
Line 564: def make_image(path, size, format, index, qcow2_compat, backing=None):
Line 565: qemuimg.create(path, size=size, format=format, 
qcow2Compat=qcow2_compat,


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I54d6c936239d3dcc7a3d236dbaab0a93501ada8c
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ala Hino 
Gerrit-Reviewer: Ala Hino 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: osinfo: un-nest kernel version gathering function

[automation]

gerrit-hooks has posted comments on this change.

Change subject: osinfo: un-nest kernel version gathering function
..


Patch Set 3:

* Update Tracker::IGNORE, no bug url/s found
* Set MODIFIED::IGNORE, no Bug-Url found.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I77f78882973855cc971bbdae66cc96c8068e8230
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Martin Polednik 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Polednik 
Gerrit-Reviewer: Milan Zamazal 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: osinfo: un-nest kernel version gathering function

[danken]

Dan Kenigsberg has submitted this change and it was merged.

Change subject: osinfo: un-nest kernel version gathering function
..


osinfo: un-nest kernel version gathering function

Previous function, kernelDict, had unclear naming and was needlessly
nested within package_versions function. This patch un-nests the
function and renames it to _runtime_kernel_version. The new name
hints why we don't look up the version via rpmdb.

Change-Id: I77f78882973855cc971bbdae66cc96c8068e8230
Signed-off-by: Martin Polednik 
Reviewed-on: https://gerrit.ovirt.org/65379
Reviewed-by: Francesco Romani 
Continuous-Integration: Jenkins CI
---
M lib/vdsm/osinfo.py
1 file changed, 12 insertions(+), 11 deletions(-)

Approvals:
  Jenkins CI: Passed CI tests
  Francesco Romani: Looks good to me, approved
  Martin Polednik: Verified



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

Gerrit-MessageType: merged
Gerrit-Change-Id: I77f78882973855cc971bbdae66cc96c8068e8230
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Martin Polednik 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Polednik 
Gerrit-Reviewer: Milan Zamazal 
Gerrit-Reviewer: gerrit-hooks 
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: osinfo: drop package buildtime

[automation]

gerrit-hooks has posted comments on this change.

Change subject: osinfo: drop package buildtime
..


Patch Set 5:

* Update Tracker::IGNORE, no bug url/s found
* Set MODIFIED::IGNORE, no Bug-Url found.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Iacc9b0fe5446abd5405db6efff6b7f9f16468bc9
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Martin Polednik 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Polednik 
Gerrit-Reviewer: Michal Skrivanek 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: Yaniv Bronhaim 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: osinfo: drop package buildtime

[danken]

Dan Kenigsberg has submitted this change and it was merged.

Change subject: osinfo: drop package buildtime
..


osinfo: drop package buildtime

Buildtime of host packages was never used and only causes us problems
w/ parsing kernel cmdline. We are safe to drop it.

Signed-off-by: Martin Polednik 
Change-Id: Iacc9b0fe5446abd5405db6efff6b7f9f16468bc9
Reviewed-on: https://gerrit.ovirt.org/65059
Reviewed-by: Michal Skrivanek 
Continuous-Integration: Jenkins CI
Reviewed-by: Piotr Kliczewski 
Reviewed-by: Dan Kenigsberg 
---
M lib/api/vdsm-api.yml
M lib/vdsm/osinfo.py
2 files changed, 3 insertions(+), 15 deletions(-)

Approvals:
  Piotr Kliczewski: Looks good to me, but someone else must approve
  Jenkins CI: Passed CI tests
  Dan Kenigsberg: Looks good to me, approved
  Michal Skrivanek: Looks good to me, but someone else must approve
  Martin Polednik: Verified



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

Gerrit-MessageType: merged
Gerrit-Change-Id: Iacc9b0fe5446abd5405db6efff6b7f9f16468bc9
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Martin Polednik 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Polednik 
Gerrit-Reviewer: Michal Skrivanek 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: Yaniv Bronhaim 
Gerrit-Reviewer: gerrit-hooks 
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: osinfo: drop package buildtime

[danken]

Dan Kenigsberg has posted comments on this change.

Change subject: osinfo: drop package buildtime
..


Patch Set 4: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Iacc9b0fe5446abd5405db6efff6b7f9f16468bc9
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Martin Polednik 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Polednik 
Gerrit-Reviewer: Michal Skrivanek 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: Yaniv Bronhaim 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: py3: make gluster_cli_tests pass

[danken]

Dan Kenigsberg has posted comments on this change.

Change subject: py3: make gluster_cli_tests pass
..


Patch Set 5: Code-Review+2

copying dropped scores, and raising.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I7e212e247f057f2988debaf3b2de923851b90b24
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dan Kenigsberg 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Petr Horáček 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: Ramesh N 
Gerrit-Reviewer: Sahina Bose 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: py3: make gluster_cli_tests pass

[automation]

gerrit-hooks has posted comments on this change.

Change subject: py3: make gluster_cli_tests pass
..


Patch Set 6:

* Update Tracker::IGNORE, no bug url/s found
* Set MODIFIED::IGNORE, no Bug-Url found.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I7e212e247f057f2988debaf3b2de923851b90b24
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dan Kenigsberg 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Petr Horáček 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: Ramesh N 
Gerrit-Reviewer: Sahina Bose 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: py3: make gluster_cli_tests pass

[danken]

Dan Kenigsberg has submitted this change and it was merged.

Change subject: py3: make gluster_cli_tests pass
..


py3: make gluster_cli_tests pass

Change-Id: I7e212e247f057f2988debaf3b2de923851b90b24
Signed-off-by: Dan Kenigsberg 
Reviewed-on: https://gerrit.ovirt.org/65008
Continuous-Integration: Jenkins CI
Reviewed-by: Piotr Kliczewski 
---
M lib/vdsm/kaxmlrpclib.py
M lib/vdsm/network/netlink/addr.py
M tests/Makefile.am
M tests/gluster_cli_tests.py
M vdsm/gluster/storagedev.py
5 files changed, 14 insertions(+), 13 deletions(-)

Approvals:
  Piotr Kliczewski: Looks good to me, but someone else must approve
  Jenkins CI: Passed CI tests
  Dan Kenigsberg: Verified; Looks good to me, approved



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

Gerrit-MessageType: merged
Gerrit-Change-Id: I7e212e247f057f2988debaf3b2de923851b90b24
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dan Kenigsberg 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Petr Horáček 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: Ramesh N 
Gerrit-Reviewer: Sahina Bose 
Gerrit-Reviewer: gerrit-hooks 
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: build: Run the tests with tox

[nsoffer]

Nir Soffer has posted comments on this change.

Change subject: build: Run the tests with tox
..


Patch Set 8:

Edward, this works with python 3 as you can see in the jenkins and travis runs. 
But in python 3 run, we are using the distro packages.

In the distant future we will have a separate python 3 env running all the 
tests that can run with python 3, and then we would require nose for python 3.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I245a171940a5e869fd719a6410024ee77e8ad86c
Gerrit-PatchSet: 8
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Edward Haas 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Irit Goihman 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: Yaniv Bronhaim 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: build: Replace pep8 and pyflakes with flake8

[nsoffer]

Nir Soffer has posted comments on this change.

Change subject: build: Replace pep8 and pyflakes with flake8
..


Patch Set 2:

Edward, flake8 works on both python 2 and 3, but this is not relevant to our 
tests.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib42430f045e4475c31cd370b4196214094c29cd1
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Edward Haas 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Irit Goihman 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: Yaniv Bronhaim 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: tests: Refine TerminationTests names

[nsoffer]

Nir Soffer has posted comments on this change.

Change subject: tests: Refine TerminationTests names
..


Patch Set 6: Verified+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Id085d7bbc65fe985454627e0958eb03d0e413b4d
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: Yaniv Bronhaim 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: utils: Raise detectable error if termination fail

[nsoffer]

Nir Soffer has posted comments on this change.

Change subject: utils: Raise detectable error if termination fail
..


Patch Set 6: Verified+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I5cfa5434ce011b9185550884233b5b233026d13c
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: Yaniv Bronhaim 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: tests: Add tests for poll and wait failures

[automation]

gerrit-hooks has posted comments on this change.

Change subject: tests: Add tests for poll and wait failures
..


Patch Set 9:

* Update Tracker::IGNORE, not relevant for branch: master
* Check Bug-Url::IGNORE, not relevant for branch: master
* Check Public Bug::WARN, no public bug url found
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 
'ovirt-4.0'])

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I8674c9c3c2118041c74213cd8ce0d383086d6cbf
Gerrit-PatchSet: 9
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: Yaniv Bronhaim 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: utils: Wait for terminated process

[automation]

gerrit-hooks has posted comments on this change.

Change subject: utils: Wait for terminated process
..


Patch Set 5:

* Update Tracker::IGNORE, not relevant for branch: master
* Check Bug-Url::IGNORE, not relevant for branch: master
* Check Public Bug::WARN, no public bug url found
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 
'ovirt-4.0'])

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ida04e2c7ba092efdc2927ed9f460b0098ba2ad94
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: Yaniv Bronhaim 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: utils: Add missing Popen methods

[automation]

gerrit-hooks has posted comments on this change.

Change subject: utils: Add missing Popen methods
..


Patch Set 5:

* Update Tracker::IGNORE, not relevant for branch: master
* Check Bug-Url::IGNORE, not relevant for branch: master
* Check Public Bug::WARN, no public bug url found
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 
'ovirt-4.0'])

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I5d562663698d86b98b55d139e2691a0f51566ccf
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: Yaniv Bronhaim 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: tests: Add tests for waiting on a zombie process

[automation]

gerrit-hooks has posted comments on this change.

Change subject: tests: Add tests for waiting on a zombie process
..


Patch Set 7:

* Update Tracker::IGNORE, not relevant for branch: master
* Check Bug-Url::IGNORE, not relevant for branch: master
* Check Public Bug::WARN, no public bug url found
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 
'ovirt-4.0'])

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I238d77213153b1fa98bb3f52b33a6fd78bb57bca
Gerrit-PatchSet: 7
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: Yaniv Bronhaim 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: utils: Raise detectable error if termination fail

[automation]

gerrit-hooks has posted comments on this change.

Change subject: utils: Raise detectable error if termination fail
..


Patch Set 6:

* Update Tracker::IGNORE, not relevant for branch: master
* Check Bug-Url::IGNORE, not relevant for branch: master
* Check Public Bug::WARN, no public bug url found
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 
'ovirt-4.0'])

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I5cfa5434ce011b9185550884233b5b233026d13c
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: Yaniv Bronhaim 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: tests: Refine TerminationTests names

[automation]

gerrit-hooks has posted comments on this change.

Change subject: tests: Refine TerminationTests names
..


Patch Set 6:

* Update Tracker::IGNORE, not relevant for branch: master
* Check Bug-Url::IGNORE, not relevant for branch: master
* Check Public Bug::WARN, no public bug url found
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 
'ovirt-4.0'])

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Id085d7bbc65fe985454627e0958eb03d0e413b4d
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: Yaniv Bronhaim 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: tests: Wait for child process in tearDown

[automation]

gerrit-hooks has posted comments on this change.

Change subject: tests: Wait for child process in tearDown
..


Patch Set 5:

* Update Tracker::IGNORE, not relevant for branch: master
* Check Bug-Url::IGNORE, not relevant for branch: master
* Check Public Bug::WARN, no public bug url found
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 
'ovirt-4.0'])

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I248fa0b1c2129f69164be447402276d7908ed768
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: Yaniv Bronhaim 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: tests: Add test for terminating a terminated process

[automation]

gerrit-hooks has posted comments on this change.

Change subject: tests: Add test for terminating a terminated process
..


Patch Set 7:

* Update Tracker::IGNORE, not relevant for branch: master
* Check Bug-Url::IGNORE, not relevant for branch: master
* Check Public Bug::WARN, no public bug url found
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 
'ovirt-4.0'])

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ic0f06c28857664cc49beb938f33ac3c9d07ca3b6
Gerrit-PatchSet: 7
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: Yaniv Bronhaim 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: tests: Use Popen.poll() for running state

[automation]

gerrit-hooks has posted comments on this change.

Change subject: tests: Use Popen.poll() for running state
..


Patch Set 5:

* Update Tracker::IGNORE, not relevant for branch: master
* Check Bug-Url::IGNORE, not relevant for branch: master
* Check Public Bug::WARN, no public bug url found
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 
'ovirt-4.0'])

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I086e9317b5f8e5324d86bedeb04b485c7a09ad16
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: Yaniv Bronhaim 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: tests: Refine TerminationTests names

[nsoffer]

Nir Soffer has posted comments on this change.

Change subject: tests: Refine TerminationTests names
..


Patch Set 5:

(3 comments)

https://gerrit.ovirt.org/#/c/65326/5//COMMIT_MSG
Commit Message:

Line 5: CommitDate: 2016-10-13 03:07:01 +0300
Line 6: 
Line 7: tests: Refine TerminationTests names
Line 8: 
Line 9: - Al the test are testing termination, there is no point in repeating
> For all the tests that test termination there is no need to repeat...
Done
Line 10:   the class name in the test names.
Line 11: - Use the tests name to describe the situation tested; terminating a
Line 12:   running process, or kill failure.
Line 13: - Rename fake_kill to fail; this name can be used later for other


Line 7: tests: Refine TerminationTests names
Line 8: 
Line 9: - Al the test are testing termination, there is no point in repeating
Line 10:   the class name in the test names.
Line 11: - Use the tests name to describe the situation tested; terminating a
> test names
Done
Line 12:   running process, or kill failure.
Line 13: - Rename fake_kill to fail; this name can be used later for other
Line 14:   failure tests.
Line 15: 


Line 10:   the class name in the test names.
Line 11: - Use the tests name to describe the situation tested; terminating a
Line 12:   running process, or kill failure.
Line 13: - Rename fake_kill to fail; this name can be used later for other
Line 14:   failure tests.
> what do you mean? how do you want to do it? (my questions based on the code
I was planning to move this to a module function, and reuse the same function 
all the tests. 

When I added the next tests, I discovered that we cannot reuse it since it 
should raise different exception and different text, so the best way is to have 
a nested function in the test.

Hopefully more clear in the next version.
Line 15: 
Line 16: Change-Id: Id085d7bbc65fe985454627e0958eb03d0e413b4d


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Id085d7bbc65fe985454627e0958eb03d0e413b4d
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: Yaniv Bronhaim 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: utils: Raise detectable error if termination fail

[nsoffer]

Nir Soffer has posted comments on this change.

Change subject: utils: Raise detectable error if termination fail
..


Patch Set 5:

(1 comment)

https://gerrit.ovirt.org/#/c/65325/5//COMMIT_MSG
Commit Message:

Line 14: job running the process must stay in running state to prevent engine
Line 15: from starting the same operation on another machine.
Line 16: 
Line 17: Typically when we cannot handle an error we should not handle it. But
Line 18: in this special case, letting the caller handle the error is not useful
> Trying to make it more clear in the next version.
Tried, I cannot see what is confusing and how to improve this. Please suggest 
an improved text.
Line 19: since it does not have any context.
Line 20: 
Line 21: To make it possible to handle this critical error, we raise now a new
Line 22: TerminatingFallure exception with process pid and the original error.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I5cfa5434ce011b9185550884233b5b233026d13c
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: Yaniv Bronhaim 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: vmxml: export container metadata

[fromani]

Francesco Romani has posted comments on this change.

Change subject: vmxml: export container metadata
..


Patch Set 23:

(6 comments)

https://gerrit.ovirt.org/#/c/60481/23/vdsm/virt/vmxml.py
File vdsm/virt/vmxml.py:

PS23, Line 29: from vdsm.containersconnection import XML as xmlcont
> This is added in the following patch, why is it here?
rebase glitch, will fix.


Line 28: from vdsm import constants
Line 29: from vdsm.containersconnection import XML as xmlcont
Line 30: from vdsm import cpuarch
Line 31: from vdsm import utils
Line 32: 
> unrelated and unneeded
will remove
Line 33: 
Line 34: METADATA_VM_TUNE_URI = 'http://ovirt.org/vm/tune/1.0'
Line 35: METADATA_VM_TUNE_ELEMENT = 'qos'
Line 36: METADATA_VM_TUNE_PREFIX = 'ovirt'


Line 33: 
Line 34: METADATA_VM_TUNE_URI = 'http://ovirt.org/vm/tune/1.0'
Line 35: METADATA_VM_TUNE_ELEMENT = 'qos'
Line 36: METADATA_VM_TUNE_PREFIX = 'ovirt'
Line 37: 
> unrelated and unneeded
will remove
Line 38: 
Line 39: _BOOT_MENU_TIMEOUT = 1  # milliseconds
Line 40: 
Line 41: 


PS23, Line 296: six.iteritems(drive_map)
> same as below
Done


PS23, Line 612: six.iteritems(custom)
> Do you expect custom to be ever accessed concurrently? itemview seems reaso
no concurrent access is expected in this flow, so I'll switch to items()


PS23, Line 616: except ValueError:
> If key starts with 'volume:', you'll always have 2 values e.g.
Actually I don't, will just remove the try block


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I1ade3c0c7d300c5ce33cb23723c3d0e59e4af664
Gerrit-PatchSet: 23
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Polednik 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: config: add tunables for container support

[fromani]

Francesco Romani has posted comments on this change.

Change subject: config: add tunables for container support
..


Patch Set 2:

(1 comment)

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

Line 5: CommitDate: 2016-10-04 10:07:37 +0200
Line 6: 
Line 7: config: add tunables for container support
Line 8: 
Line 9: Add the config items we'll need later on
> Sentence regarding how were the defaults picked would be nice.
Done
Line 10: 
Line 11: Change-Id: I30e4372fc88ebb7e68cc3b982af3c2eefacef7ce


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I30e4372fc88ebb7e68cc3b982af3c2eefacef7ce
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Polednik 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: utils: Raise detectable error if termination fail

[nsoffer]

Nir Soffer has posted comments on this change.

Change subject: utils: Raise detectable error if termination fail
..


Patch Set 5:

(1 comment)

https://gerrit.ovirt.org/#/c/65325/5//COMMIT_MSG
Commit Message:

Line 14: job running the process must stay in running state to prevent engine
Line 15: from starting the same operation on another machine.
Line 16: 
Line 17: Typically when we cannot handle an error we should not handle it. But
Line 18: in this special case, letting the caller handle the error is not useful
> handling the error without knowing the context (pid) is not useful. That is
Trying to make it more clear in the next version.
Line 19: since it does not have any context.
Line 20: 
Line 21: To make it possible to handle this critical error, we raise now a new
Line 22: TerminatingFallure exception with process pid and the original error.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I5cfa5434ce011b9185550884233b5b233026d13c
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: Yaniv Bronhaim 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: tests: Use Popen.poll() for running state

[nsoffer]

Nir Soffer has posted comments on this change.

Change subject: tests: Use Popen.poll() for running state
..


Patch Set 4:

(1 comment)

https://gerrit.ovirt.org/#/c/65323/4//COMMIT_MSG
Commit Message:

Line 5: CommitDate: 2016-10-13 03:07:01 +0300
Line 6: 
Line 7: tests: Use Popen.poll() for running state
Line 8: 
Line 9: Checking if /proc/pid exists works, but using poll() is simpler and more
> `exits` should be enough or we could use `exists/works`
Currently we check if /proc/pid exists, this checks works but we can use 
Popen.poll() instead. I son't see how I can remove the word "works" or how 
"exists/works" is corect.
Line 10: clear.
Line 11: 
Line 12: Since proc_path is used only in one test, move it the test using it.
Line 13: 


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I086e9317b5f8e5324d86bedeb04b485c7a09ad16
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: Yaniv Bronhaim 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: qemuimg: Expose API for qemuimg map

[ahino]

Ala Hino has posted comments on this change.

Change subject: qemuimg: Expose API for qemuimg map
..


Patch Set 4:

(4 comments)

https://gerrit.ovirt.org/#/c/65112/4/tests/qemuimg_test.py
File tests/qemuimg_test.py:

Line 497: # Empty run
Line 498: run = img_map[2]
Line 499: self.assertEqual(run["length"], size - offset - length)
Line 500: self.assertEqual(run["data"], False)
Line 501: self.assertEqual(run["zero"], True)
> Please use the same style as in the qcow2 test for verifying.
Done
Line 502: 
Line 503: @permutations([
Line 504: # offset, length, expected_length, expected_start, 
qcow2_compat
Line 505: (64 * 1024, 4 * 1024, 65536, 131072, "0.10"),


Line 530: },
Line 531: # run 2 - data
Line 532: {
Line 533: "start": offset,
Line 534: "offset": 327680,
> Do you have a clue about this value?
No idea :(
Line 535: "length": expected_length,
Line 536: "data": True,
Line 537: "zero": False,
Line 538: },


Line 537: "zero": False,
Line 538: },
Line 539: # run 3 - empty
Line 540: {
Line 541: "start": expected_start,
> Isn't this offset + expected_length?
Fixed
Line 542: "length": size - offset - expected_length,
Line 543: "data": False,
Line 544: "zero": True,
Line 545: },


Line 540: {
Line 541: "start": expected_start,
Line 542: "length": size - offset - expected_length,
Line 543: "data": False,
Line 544: "zero": True,
> Do we have an offsect in this run? what is the value?
No offset here
Line 545: },
Line 546: ]
Line 547: 
Line 548: for actual, expected in zip(img_map, expected):


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I54d6c936239d3dcc7a3d236dbaab0a93501ada8c
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ala Hino 
Gerrit-Reviewer: Ala Hino 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: qemuimg: Expose API for qemuimg map

[automation]

gerrit-hooks has posted comments on this change.

Change subject: qemuimg: Expose API for qemuimg map
..


Patch Set 5:

* Update Tracker::IGNORE, not relevant for branch: master
* Check Bug-Url::IGNORE, not relevant for branch: master
* Check Public Bug::WARN, no public bug url found
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 
'ovirt-4.0'])

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I54d6c936239d3dcc7a3d236dbaab0a93501ada8c
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ala Hino 
Gerrit-Reviewer: Ala Hino 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: py3: make gluster_cli_tests pass

[piotr . kliczewski]

Piotr Kliczewski has posted comments on this change.

Change subject: py3: make gluster_cli_tests pass
..


Patch Set 5: Code-Review+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I7e212e247f057f2988debaf3b2de923851b90b24
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dan Kenigsberg 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Petr Horáček 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: Ramesh N 
Gerrit-Reviewer: Sahina Bose 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: osinfo: drop package buildtime

[piotr . kliczewski]

Piotr Kliczewski has posted comments on this change.

Change subject: osinfo: drop package buildtime
..


Patch Set 4: Code-Review+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Iacc9b0fe5446abd5405db6efff6b7f9f16468bc9
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Martin Polednik 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Polednik 
Gerrit-Reviewer: Michal Skrivanek 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: Yaniv Bronhaim 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: tests: Add tests for poll and wait failures

[nsoffer]

Nir Soffer has posted comments on this change.

Change subject: tests: Add tests for poll and wait failures
..


Patch Set 8:

(2 comments)

https://gerrit.ovirt.org/#/c/65294/8/tests/utilsTests.py
File tests/utilsTests.py:

Line 132: self.check_failure()
Line 133: 
Line 134: def test_kill_failure(self):
Line 135: def fail():
Line 136: raise ExpectedFailure("Fake kill failure")
> Now we have it 3 times.
3 test, 3 fail texts, please suggest a simple and more clear code.
Line 137: 
Line 138: self.proc.kill = fail
Line 139: self.check_failure()
Line 140: 


Line 154: self.assertEqual(type(e.exception.error), ExpectedFailure)
Line 155: 
Line 156: # Note: We cannot check return code since 
AsyncProc.returncode is a
Line 157: # property calling poll(). The return code here may be None 
or -9,
Line 158: # depeending on timing.
> Wouldn't be better to wait to be sure that timing is correct instead of rem
There is no way to test this with AsyncProc, check how it imlements returncode.
Line 159: 
Line 160: 
Line 161: class ExpectedFailure(Exception):
Line 162: pass


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I8674c9c3c2118041c74213cd8ce0d383086d6cbf
Gerrit-PatchSet: 8
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: Yaniv Bronhaim 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: tests: Add test for terminating a terminated process

[nsoffer]

Nir Soffer has posted comments on this change.

Change subject: tests: Add test for terminating a terminated process
..


Patch Set 6:

(1 comment)

https://gerrit.ovirt.org/#/c/65328/6/tests/utilsTests.py
File tests/utilsTests.py:

Line 114: self.proc.terminate()
Line 115: self.proc.wait()
Line 116: 
Line 117: def fail():
Line 118: raise RuntimeError("Attempt to kill a terminated process")
> I see that copy and paste are our friends :)
Did you notice that each exception has different text?

Please suggest how to improve this while keeping the code clear.
Line 119: 
Line 120: self.proc.kill = fail
Line 121: with utils.terminating(self.proc):
Line 122: pass


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ic0f06c28857664cc49beb938f33ac3c9d07ca3b6
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: Yaniv Bronhaim 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: tests: Add tests for waiting on a zombie process

[nsoffer]

Nir Soffer has posted comments on this change.

Change subject: tests: Add tests for waiting on a zombie process
..


Patch Set 6:

(1 comment)

https://gerrit.ovirt.org/#/c/65327/6/tests/utilsTests.py
File tests/utilsTests.py:

Line 102: self.proc.terminate()
Line 103: wait_for_zombie(self.proc, 1)
Line 104: 
Line 105: def fail():
Line 106: raise RuntimeError("Attempt to kill a zombie process")
> in previous patch I though we wanted to reuse similar code but now I see th
This raises rumtime error and will fail the test if raised.

The other fail helper raise expected error and the test will fail if not raised.

Please see the next patches for the complete picture.
Line 107: 
Line 108: self.proc.kill = fail
Line 109: with utils.terminating(self.proc):
Line 110: pass


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I238d77213153b1fa98bb3f52b33a6fd78bb57bca
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: Yaniv Bronhaim 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: osinfo: un-nest kernel version gathering function

[mpolednik]

Martin Polednik has posted comments on this change.

Change subject: osinfo: un-nest kernel version gathering function
..


Patch Set 1:

(1 comment)

https://gerrit.ovirt.org/#/c/65379/1/lib/vdsm/osinfo.py
File lib/vdsm/osinfo.py:

Line 245: 
Line 246: def _runtime_kernel_version():
Line 247: ret = os.uname()
Line 248: try:
Line 249: ver, rel = ret[2].split('-', 1)
> I wonder if we can get this for free from the platform module
Define free. We can get the exact same information with

 platform.uname()[2].split('-', 1)

Unfortunately, as uname's implementation does roughly

...
if release == 'unknown':
release = ''
...

if it cannot find the kernel release, it wouldn't really help us much.
Line 250: except ValueError:
Line 251: logging.error('kernel release not found', exc_info=True)
Line 252: ver, rel = '0', '0'
Line 253: 


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I77f78882973855cc971bbdae66cc96c8068e8230
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Martin Polednik 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Polednik 
Gerrit-Reviewer: Milan Zamazal 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: tests: Add tests for poll and wait failures

[piotr . kliczewski]

Piotr Kliczewski has posted comments on this change.

Change subject: tests: Add tests for poll and wait failures
..


Patch Set 8: Code-Review-1

(2 comments)

https://gerrit.ovirt.org/#/c/65294/8/tests/utilsTests.py
File tests/utilsTests.py:

PS8, Line 135: def fail():
 : raise ExpectedFailure("Fake kill failure")
Now we have it 3 times.


PS8, Line 158: timing
Wouldn't be better to wait to be sure that timing is correct instead of 
removing it?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I8674c9c3c2118041c74213cd8ce0d383086d6cbf
Gerrit-PatchSet: 8
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: Yaniv Bronhaim 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: osinfo: drop package buildtime

[mpolednik]

Martin Polednik has posted comments on this change.

Change subject: osinfo: drop package buildtime
..


Patch Set 4: Verified+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Iacc9b0fe5446abd5405db6efff6b7f9f16468bc9
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Martin Polednik 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Polednik 
Gerrit-Reviewer: Michal Skrivanek 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: Yaniv Bronhaim 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org