Change in vdsm[master]: misc: remove cp parameter

[automation]

automat...@ovirt.org has posted comments on this change.

Change subject: misc: remove cp parameter
..


Patch Set 2:

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

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I0906bfd7dfa128c323aa399810bbd75883618434
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yeela Kaplan 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Yeela Kaplan 
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: misc: remove cp parameter

[danken]

Dan Kenigsberg has posted comments on this change.

Change subject: misc: remove cp parameter
..


Patch Set 1:

(1 comment)

https://gerrit.ovirt.org/#/c/45613/1/vdsm/storage/misc.py
File vdsm/storage/misc.py:

Line 488: 
EXT_CP should be dropped from constants, too.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I0906bfd7dfa128c323aa399810bbd75883618434
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yeela Kaplan 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: schema: rpm for jsonrpc schema files

[piotr . kliczewski]

Piotr Kliczewski has posted comments on this change.

Change subject: schema: rpm for jsonrpc schema files
..


Patch Set 2:

Good catch will remove it.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I13d6291ddbf5bf7d8e6a0956db3300cd0c45e563
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Piotr Kliczewski 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: Yaniv Bronhaim 
Gerrit-Reviewer: Yeela Kaplan 
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: scheduler: use single instance

[piotr . kliczewski]

Piotr Kliczewski has posted comments on this change.

Change subject: scheduler: use single instance
..


Patch Set 7:

(4 comments)

https://gerrit.ovirt.org/#/c/43825/7/vdsm/virt/periodic.py
File vdsm/virt/periodic.py:

Line 62: 
Line 63: def per_vm_operation(func, period):
Line 64: disp = VmDispatcher(
Line 65: cif.getVMs, _executor, func, _timeout_from(period))
Line 66: return Operation(disp, period, scheduler, _executor)
> if we use the default executor, let's avoid to pass it. We'll make things m
Done
Line 67: 
Line 68: _operations = [
Line 69: # needs dispatching becuse updating the volume stats needs the
Line 70: # access the storage, thus can block.


Line 91: cif.getVMs,
Line 92: sampling.stats_cache),
Line 93: config.getint('vars', 'vm_sample_interval'),
Line 94: scheduler,
Line 95: _executor),
> same as per line 66. It's fine to pass scheduler, but let's keep this chang
Done
Line 96: 
Line 97: # we do this only until we get high water mark notifications
Line 98: # from qemu. Access storage and/or qemu monitor, so can block,
Line 99: # thus we need dispatching.


Line 111: for op in _operations:
Line 112: op.stop()
Line 113: 
Line 114: if _executor is not None:
Line 115: _executor.stop(wait=False)
> your intention here is good, but if this is None, it means that someone cal
Done
Line 116: 
Line 117: 
Line 118: class Operation(object):
Line 119: """


Line 125: """
Line 126: 
Line 127: _log = logging.getLogger("virt.periodic.Operation")
Line 128: 
Line 129: def __init__(self, func, period, scheduler, executor=None, 
timeout=0):
> please switch the order of "executor" and "timeout":
Done
Line 130: """
Line 131: parameters:
Line 132: 
Line 133: func: callable, without arguments (task interface).


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

Gerrit-MessageType: comment
Gerrit-Change-Id: If83eded458f8304d802fcd75839e7a916146918b
Gerrit-PatchSet: 7
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Piotr Kliczewski 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: Yaniv Bronhaim 
Gerrit-Reviewer: Yeela Kaplan 
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: lib: daemon: cpu affinity support using taskset

[msivak]

Martin Sivák has posted comments on this change.

Change subject: lib: daemon: cpu affinity support using taskset
..


Patch Set 1:

(1 comment)

https://gerrit.ovirt.org/#/c/45738/1//COMMIT_MSG
Commit Message:

Line 17: We need a patch which is simple to backport down to 3.5,
Line 18: so we use taskset just before the start of VDSM.
Line 19: 
Line 20: taskset is part of util-linux, so no additional dependency
Line 21: is needed.
> This is a good point - I agree that we should make this easy as possible to
Yep, I think he found the exact sweetspot there and I like it too.
Line 22: 
Line 23: Change-Id: I3f7f68d65eddb5a21afbc3809ea79cd1dee67984
Line 24: Bug-Url: https://bugzilla.redhat.com/1247075
Line 25: Backport-To: 3.6


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I3f7f68d65eddb5a21afbc3809ea79cd1dee67984
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Polednik 
Gerrit-Reviewer: Martin Sivák 
Gerrit-Reviewer: Michal Skrivanek 
Gerrit-Reviewer: Michal Skrivanek 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: Vinzenz Feenstra 
Gerrit-Reviewer: Yaniv Bronhaim 
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: misc: remove cp parameter

[danken]

Dan Kenigsberg has posted comments on this change.

Change subject: misc: remove cp parameter
..


Patch Set 1: Code-Review-1

(1 comment)

https://gerrit.ovirt.org/#/c/45613/1/vdsm/storage/misc.py
File vdsm/storage/misc.py:

Line 483: utils.unpersist(newName)
Line 484: except:
Line 485: pass
Line 486: 
Line 487: os.rename(oldName, newName)
the original except:pass was horrible, but you should not change the logic in 
such a patch, so please keep it.
Line 488: 
Line 489: if utils.isOvirtNode() and persist:
Line 490: try:
Line 491: utils.persist(newName)


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I0906bfd7dfa128c323aa399810bbd75883618434
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yeela Kaplan 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[ovirt-3.6]: volume: fix failing metadata parsing

[automation]

automat...@ovirt.org has posted comments on this change.

Change subject: volume: fix failing metadata parsing
..


Patch Set 2:

* Update tracker::#1258835::OK
* Check TR::#1258835::OK
* Set MODIFIED::bug 1258835#1258835OK

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie84bf5e42d2c4949944a5f348615d6fe80f72fa2
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-3.6
Gerrit-Owner: Idan Shaby 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Idan Shaby 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[ovirt-3.6]: volume: fix failing metadata parsing

[fromani]

Francesco Romani has posted comments on this change.

Change subject: volume: fix failing metadata parsing
..


Patch Set 1: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie84bf5e42d2c4949944a5f348615d6fe80f72fa2
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-3.6
Gerrit-Owner: Idan Shaby 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Idan Shaby 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Tal Nisan 
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[ovirt-3.6]: volume: fix failing metadata parsing

[fromani]

Francesco Romani has submitted this change and it was merged.

Change subject: volume: fix failing metadata parsing
..


volume: fix failing metadata parsing

The call to "split" in file and block volumes expects only one "=" per
line. This is not a good behavior since there might be fields that
contain some more "=" (like Description).
This patch limits the number of splits to 1, so that the fields values
can contains some other "=".

Change-Id: Ie84bf5e42d2c4949944a5f348615d6fe80f72fa2
Backport-To: 3.6
Bug-Url: https://bugzilla.redhat.com/1258835
Signed-off-by: Idan Shaby 
Reviewed-on: https://gerrit.ovirt.org/45585
Reviewed-by: Nir Soffer 
Reviewed-by: Allon Mureinik 
Continuous-Integration: Jenkins CI
Reviewed-on: https://gerrit.ovirt.org/45761
Reviewed-by: Francesco Romani 
---
M vdsm/storage/blockVolume.py
M vdsm/storage/fileVolume.py
2 files changed, 2 insertions(+), 2 deletions(-)

Approvals:
  Nir Soffer: Looks good to me, but someone else must approve
  Jenkins CI: Passed CI tests
  Allon Mureinik: Looks good to me, but someone else must approve
  Francesco Romani: Looks good to me, approved
  Idan Shaby: Verified



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

Gerrit-MessageType: merged
Gerrit-Change-Id: Ie84bf5e42d2c4949944a5f348615d6fe80f72fa2
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-3.6
Gerrit-Owner: Idan Shaby 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Idan Shaby 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: automat...@ovirt.org
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: schema: rpm for jsonrpc schema files

[piotr . kliczewski]

Piotr Kliczewski has posted comments on this change.

Change subject: schema: rpm for jsonrpc schema files
..


Patch Set 2:

vdsm-api.html is generated during the build from schema file. From where do you 
want me to remove it?

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I13d6291ddbf5bf7d8e6a0956db3300cd0c45e563
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Piotr Kliczewski 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: Yaniv Bronhaim 
Gerrit-Reviewer: Yeela Kaplan 
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: automation: enable pep8

[msivak]

Martin Sivák has posted comments on this change.

Change subject: automation: enable pep8
..


Patch Set 1: Code-Review+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: If2266725af30e45b414d040f7f8d6b47d46bf947
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dan Kenigsberg 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Sivák 
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: schema: rpm for jsonrpc schema files

[ykaplan]

Yeela Kaplan has posted comments on this change.

Change subject: schema: rpm for jsonrpc schema files
..


Patch Set 2:

If it is generated during the build, why is it added as a file to this patch?

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I13d6291ddbf5bf7d8e6a0956db3300cd0c45e563
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Piotr Kliczewski 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: Yaniv Bronhaim 
Gerrit-Reviewer: Yeela Kaplan 
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: multipath: Move all calls to multipath exe to a single method

[piotr . kliczewski]

Piotr Kliczewski has posted comments on this change.

Change subject: multipath: Move all calls to multipath exe to a single method
..


Patch Set 10:

Do we still want to have this patch?

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I52afc07a07a925ed7572eb369deb7c203edb04cd
Gerrit-PatchSet: 10
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Saggi Mizrahi 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Ayal Baron 
Gerrit-Reviewer: Barak Azulay 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Eduardo 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: Yaniv Bronhaim 
Gerrit-Reviewer: Yeela Kaplan 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: blockVolume: Fail if metadata overflows

[danken]

Dan Kenigsberg has posted comments on this change.

Change subject: blockVolume: Fail if metadata overflows
..


Patch Set 3:

/me is waiting for Engine-side approval of the fix in our semantics

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I9ac286c68307e4a1925b9430a0b3b9909cdd682a
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Ala Hino 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Freddy Rolland 
Gerrit-Reviewer: Idan Shaby 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Liron Aravot 
Gerrit-Reviewer: Maor Lipchuk 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: Add an empty metadata qos element to the created domain

[msivak]

Martin Sivák has posted comments on this change.

Change subject: Add an empty metadata qos element to the created domain
..


Patch Set 2:

(1 comment)

https://gerrit.ovirt.org/#/c/45664/2/vdsm/virt/vm.py
File vdsm/virt/vm.py:

Line 72: from .vmdevices.storage import DISK_TYPE
Line 73: from .vmtune import update_io_tune_dom, collect_inner_elements
Line 74: from .vmtune import io_tune_values_to_dom, io_tune_dom_to_values
Line 75: from . import vmxml
Line 76: from .vmxml import METADATA_VM_TUNE_URI, METADATA_VM_TUNE_ELEMENT, 
METADATA_VM_TUNE_PREFIX
> why didn't pep8 cry!? we must fix the CI job.
No it didn't.. I will shorten the line.
Line 77: 
Line 78: from .utils import isVdsmImage, cleanup_guest_socket
Line 79: from vmpowerdown import VmShutdown, VmReboot
Line 80: 


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ibc5db925b55ea8e583a11d548a27dc3fd0886fee
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Martin Sivák 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Sivák 
Gerrit-Reviewer: Michal Skrivanek 
Gerrit-Reviewer: Michal Skrivanek 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: zombiereaper: Add ability to wait on process

[piotr . kliczewski]

Piotr Kliczewski has posted comments on this change.

Change subject: zombiereaper: Add ability to wait on process
..


Patch Set 1:

Do we still see a need for this patch?

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Idcd69b6187bc1c412c11f6cba9af431557ea7077
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Saggi Mizrahi 
Gerrit-Reviewer: Barak Azulay 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: Saggi Mizrahi 
Gerrit-Reviewer: Yaniv Bronhaim 
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: misc: remove cp parameter

[ykaplan]

Yeela Kaplan has posted comments on this change.

Change subject: misc: remove cp parameter
..


Patch Set 1:

(2 comments)

https://gerrit.ovirt.org/#/c/45613/1/vdsm/storage/misc.py
File vdsm/storage/misc.py:

Line 488: 
> EXT_CP should be dropped from constants, too.
EXT_CP is still used in multipath configurator


Line 483: utils.unpersist(newName)
Line 484: except:
Line 485: pass
Line 486: 
Line 487: os.rename(oldName, newName)
> the original except:pass was horrible, but you should not change the logic 
Done
Line 488: 
Line 489: if utils.isOvirtNode() and persist:
Line 490: try:
Line 491: utils.persist(newName)


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I0906bfd7dfa128c323aa399810bbd75883618434
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yeela Kaplan 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Yeela Kaplan 
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[ovirt-3.5]: net: fix vdsmd service start

[automation]

automat...@ovirt.org has posted comments on this change.

Change subject: net: fix vdsmd service start
..


Patch Set 2:

* Update tracker::#1258551::OK
* Check Bug-Url::OK
* Check Public Bug::#1258551::OK, public bug
* Check Product::#1258551::OK, Correct product Red Hat Enterprise 
Virtualization Manager
* Check TR::#1258551::OK, correct target release 3.5.5
* warn_if_not_merged_to_previous_branch: OK

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I71342362f56b87bcb566c3343c90a69f95327ea6
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-3.5
Gerrit-Owner: Petr Horáček 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: scale: limit cpu usage using cpu-affinity

[fromani]

Francesco Romani has posted comments on this change.

Change subject: scale: limit cpu usage using cpu-affinity
..


Patch Set 5:

(5 comments)

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

Line 36: taskset to remove VDSM's cpu affinity and allow the child
Line 37: process to use any cpu.
Line 38: 
Line 39: taskset is part of util-linux, so no additional dependency
Line 40: is needed.
in this version (and in the half-backed before), I reverted the attempted use 
of CommandPath because
- utils needs taskset code in execCmd
- taskset needs utils code to actually run the external process.

Thus:
- taskset module cannot own the taskset command path (otherwise: circular 
dependency between taskset and utils)
- for the same reason I re-added back code in cmdutils
Line 41: 
Line 42: Change-Id: I3f7f68d65eddb5a21afbc3809ea79cd1dee67984
Line 43: Bug-Url: https://bugzilla.redhat.com/1247075
Line 44: Backport-To: 3.6


https://gerrit.ovirt.org/#/c/45738/5/lib/vdsm/taskset.py
File lib/vdsm/taskset.py:

Line 44: 
Line 45: if rc != 0:
Line 46: raise Error(rc, err)
Line 47: 
Line 48: return _expand_list(_extract_list(out))
taskset command can use range syntex on output as well, then we need this code 
to normalize it back to our plain list
Line 49: 
Line 50: 
Line 51: def set(pid, cpu_list, all_tasks=True):
Line 52: # TODO: check pid


https://gerrit.ovirt.org/#/c/45738/5/lib/vdsm/utils.py
File lib/vdsm/utils.py:

Line 626: self._poller.close()
Line 627: 
Line 628: 
Line 629: _MAX_CPU = 1023  # FIXME: lift this limit
Line 630: _ALL_CPU = range(_MAX_CPU)
I don't like this. Suggestions welcome.
Line 631: 
Line 632: 
Line 633: def execCmd(command, sudo=False, cwd=None, data=None, raw=False,
Line 634: printable=None, env=None, sync=True, nice=None, 
ioclass=None,


Line 644: """
Line 645: 
Line 646: if resetCpuAffinity and config.get('vars', 'cpu_affinity'):
Line 647: # only the main VDSM process whould be bound
Line 648: command = cmdutils.taskset(command, _ALL_CPU)
there is a tiny race on startup here. Code that inspects childs (utils.pidStat) 
can be now broken.

Please note that unlike nice/ionice, taskset will *always* added to execCmd - 
of course when affinity is enabled.
Line 649: 
Line 650: if ioclass is not None:
Line 651: command = cmdutils.ionice(command, ioclass=ioclass,
Line 652:   ioclassdata=ioclassdata)


https://gerrit.ovirt.org/#/c/45738/5/tests/tasksetTests.py
File tests/tasksetTests.py:

Line 23: import time
Line 24: 
Line 25: from vdsm import taskset
Line 26: 
Line 27: from testlib import VdsmTestCase
I know many important test cases are missing here. I'll add them in the coming 
revision(s).
Line 28: 
Line 29: 
Line 30: class AffinityTests(VdsmTestCase):
Line 31: 


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I3f7f68d65eddb5a21afbc3809ea79cd1dee67984
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Polednik 
Gerrit-Reviewer: Martin Sivák 
Gerrit-Reviewer: Michal Skrivanek 
Gerrit-Reviewer: Michal Skrivanek 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: Vinzenz Feenstra 
Gerrit-Reviewer: Yaniv Bronhaim 
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: tests: functional - convert to run over jsonrpc

[automation]

automat...@ovirt.org has posted comments on this change.

Change subject: tests: functional - convert to run over jsonrpc
..


Patch Set 1:

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

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Iaba1e2811f010e4509a658acef8040ad8f39cece
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yeela Kaplan 
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: tests: functional - convert to run over jsonrpc

[ykaplan]

Yeela Kaplan has uploaded a new change for review.

Change subject: tests: functional - convert to run over jsonrpc
..

tests: functional - convert to run over jsonrpc

will run functional tests by default over jsonrpc

Change-Id: Iaba1e2811f010e4509a658acef8040ad8f39cece
Signed-off-by: Yeela Kaplan 
---
M lib/vdsm/jsonrpcvdscli.py
M lib/vdsm/response.py
M tests/functional/utils.py
3 files changed, 58 insertions(+), 16 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/89/45789/1

diff --git a/lib/vdsm/jsonrpcvdscli.py b/lib/vdsm/jsonrpcvdscli.py
index 21e135c..606bbef 100644
--- a/lib/vdsm/jsonrpcvdscli.py
+++ b/lib/vdsm/jsonrpcvdscli.py
@@ -33,17 +33,34 @@
 
 
 _COMMAND_CONVERTER = {
-'ping': 'Host.ping',
+'addNetwork': 'Host.addNetwork',
+'create': 'VM.create',
+'delNetwork': 'Host.delNetwork',
 'destroy': 'VM.destroy',
+'editNetwork': 'Host.editNetwork',
+'fullList': 'Host.getVMFullList',
+'getAllVmStats': 'Host.getAllVmStats',
+'getVdsCapabilities': 'Host.getCapabilities',
+'getVdsStats': 'Host.getStats',
 'getVmStats': 'VM.getStats',
+'list': 'Host.getVMList',
 'migrationCreate': 'VM.migrationCreate',
+'ping': 'Host.ping',
+'setBalloonTarget': 'VM.setBalloonTarget',
+'setCpuTunePeriod': 'VM.setCpuTunePeriod',
+'setCpuTuneQuota': 'VM.setCpuTuneQuota',
+'setMOMPolicy': 'Host.setMOMPolicy',
+'setSafeNetworkConfig': 'Host.setSafeNetworkConfig',
+'setupNetworks': 'Host.setupNetworks',
+'updateVmPolicy': 'VM.updateVmPolicy',
 }
 
 
 class _Server(object):
 
-def __init__(self, client):
+def __init__(self, client, compat):
 self._client = client
+self._compat = compat
 
 def _callMethod(self, methodName, *args):
 try:
@@ -64,6 +81,9 @@
 return response.error_raw(resp.error["code"],
   resp.error["message"])
 
+if not self._compat:
+return response.success_raw(resp.result)
+
 if resp.result and resp.result is not True:
 # None is translated to True inside our JSONRPC implementation
 return response.success(**resp.result)
@@ -72,6 +92,11 @@
 
 def migrationCreate(self, params):
 return self._callMethod('migrationCreate',
+params['vmId'],
+params)
+
+def create(self, params):
+return self._callMethod('create',
 params['vmId'],
 params)
 
@@ -107,7 +132,7 @@
 def connect(requestQueue, stompClient=None,
 host=None, port=None,
 useSSL=None,
-responseQueue=None):
+responseQueue=None, compat=True):
 if not stompClient:
 client = _create(requestQueue,
  host, port, useSSL,
@@ -119,4 +144,4 @@
 str(uuid4())
 )
 
-return _Server(client)
+return _Server(client, compat)
diff --git a/lib/vdsm/response.py b/lib/vdsm/response.py
index 62ed49e..160898f 100644
--- a/lib/vdsm/response.py
+++ b/lib/vdsm/response.py
@@ -41,6 +41,21 @@
 return kwargs
 
 
+def success_raw(result=None, message=None):
+ret = {
+'status':
+{
+"code": doneCode["code"],
+"message": message or doneCode["message"],
+}
+}
+
+if result:
+ret['result'] = result
+
+return ret
+
+
 def error(name, message=None):
 status = errCode[name]["status"]
 return {
diff --git a/tests/functional/utils.py b/tests/functional/utils.py
index 6bd8dd9..b8bdce6 100644
--- a/tests/functional/utils.py
+++ b/tests/functional/utils.py
@@ -26,9 +26,9 @@
 from vdsm.config import config
 from vdsm.utils import retry
 from vdsm import ipwrapper
+from vdsm import jsonrpcvdscli
 from vdsm import netinfo
 from vdsm import supervdsm
-from vdsm import vdscli
 from vdsm.netconfpersistence import RunningConfig
 
 
@@ -71,7 +71,9 @@
 retry(self.start, (socket.error, KeyError), tries=30)
 
 def start(self):
-self.vdscli = vdscli.connect()
+requestQueues = config.get('addresses', 'request_queues')
+requestQueue = requestQueues.split(",")[0]
+self.vdscli = jsonrpcvdscli.connect(requestQueue, compat=False)
 self.netinfo = self._get_netinfo()
 if config.get('vars', 'net_persistence') == 'unified':
 self.config = RunningConfig()
@@ -204,40 +206,40 @@
 
 def getVdsStats(self):
 result = self.vdscli.getVdsStats()
-return _parse_result(result, 'info')
+return _parse_result(result, True)
 
 def getAllVmStats(self):
 result = self.vdscli.getAllVmStats()
-return _parse_result(result, 'statsList')
+return _parse_result(result, True)
 
 def getVmStats(self, vmId):
 result = 

Change in vdsm[master]: automation: enable pep8

[piotr . kliczewski]

Piotr Kliczewski has posted comments on this change.

Change subject: automation: enable pep8
..


Patch Set 1: Code-Review+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: If2266725af30e45b414d040f7f8d6b47d46bf947
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dan Kenigsberg 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Sivák 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[ovirt-3.5]: net: backport cmdutils.systemd_run

[danken]

Dan Kenigsberg has posted comments on this change.

Change subject: net: backport cmdutils.systemd_run
..


Patch Set 5:

this seems bad, albeit unrelated to the patch

ERROR: test_parseVolumeStatus (gluster_cli_tests.GlusterCliTests)
--
Traceback (most recent call last):
  File "/builddir/build/BUILD/vdsm-4.16.26/tests/gluster_cli_tests.py", line 
1083, in test_parseVolumeStatus
self._parseVolumeStatus_test()
  File "/builddir/build/BUILD/vdsm-4.16.26/tests/gluster_cli_tests.py", line 
268, in _parseVolumeStatus_test
status = gcli._parseVolumeStatus(tree)
  File "/builddir/build/BUILD/vdsm-4.16.26/vdsm/gluster/cli.py", line 137, in 
_parseVolumeStatus
hostname = _getLocalIpAddress() or _getGlusterHostName()
  File "/builddir/build/BUILD/vdsm-4.16.26/vdsm/gluster/cli.py", line 107, in 
_getLocalIpAddress
for ip in netinfo.getIpAddresses():
  File "/builddir/build/BUILD/vdsm-4.16.26/lib/vdsm/netinfo.py", line 784, in 
getIpAddresses
return filter(None, [getaddr(i) for i in ethtool.get_active_devices()])
  File "/builddir/build/BUILD/vdsm-4.16.26/lib/vdsm/netinfo.py", line 300, in 
getaddr
dev_info_list = ethtool.get_interfaces_info(dev.encode('utf8'))
UnicodeDecodeError: 'ascii' codec can't decode byte 0xe3 in position 5: ordinal 
not in range(128)

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I7686e80c0880dcc28fa735d1dd3ab658136889f9
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-3.5
Gerrit-Owner: Ido Barkan 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Ido Barkan 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Yaniv Bronhaim 
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: scale: limit cpu usage using cpu-affinity

[automation]

automat...@ovirt.org has posted comments on this change.

Change subject: scale: limit cpu usage using cpu-affinity
..


Patch Set 4:

* Update tracker::#1247075::OK
* Check Bug-Url::OK
* Check Public Bug::#1247075::OK, public bug
* Check Product::#1247075::OK, Correct product Red Hat Enterprise 
Virtualization Manager
* Check TR::SKIP, not in a monitored branch (ovirt-3.5 ovirt-3.4 ovirt-3.3 
ovirt-3.2)
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 
'ovirt-3.4', 'ovirt-3.3'])

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I3f7f68d65eddb5a21afbc3809ea79cd1dee67984
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Polednik 
Gerrit-Reviewer: Martin Sivák 
Gerrit-Reviewer: Michal Skrivanek 
Gerrit-Reviewer: Michal Skrivanek 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: Vinzenz Feenstra 
Gerrit-Reviewer: Yaniv Bronhaim 
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: scale: limit cpu usage using cpu-affinity

[nsoffer]

Nir Soffer has posted comments on this change.

Change subject: scale: limit cpu usage using cpu-affinity
..


Patch Set 5: Code-Review-1

(14 comments)

https://gerrit.ovirt.org/#/c/45738/5/lib/vdsm/cmdutils.py
File lib/vdsm/cmdutils.py:

Line 69: 
Line 70: def taskset(cmd, cpu_list, all_tasks=True):
Line 71: command = [constants.EXT_TASKSET,
Line 72:'-a' if all_tasks else '',
Line 73:'-c', ','.join(cpu_list)]
I think we should use the long options for better readability in the code and 
the logs. This is an edge case since the code is very clear because of the 
arguments names.

Adding -a or "" works, but is dirty. Lets use simpler code like the other 
functions around:

command = [taskset, '--cpu-list', ','.join(cpu_list)]
if all_tasks:
command.append('--all-tasks')
Line 74: command.extend(cmd)
Line 75: return command
Line 76: 
Line 77: 


https://gerrit.ovirt.org/#/c/45738/5/lib/vdsm/config.py.in
File lib/vdsm/config.py.in:

Line 209: 'Time in seconds defining how frequently we log transport 
stats'),
Line 210: 
Line 211: ('cpu_affinity', '',
Line 212: 'Comma separated whitelist of CPU cores on which VDSM is 
allowed '
Line 213: 'to run. Only the first 64 CPUs can be specified. '
Are you sure about 64 limit? taskset(1) does not mention this. Maybe this is a 
limit when using the mask format (which we don't use now)?
Line 214: 'Default is empty list, meaning VDSM can be scheduled by 
the OS '
Line 215: 'to run on any core. '
Line 216: 'Valid examples: "0,1", "0,2,3"')
Line 217: 


Line 210: 
Line 211: ('cpu_affinity', '',
Line 212: 'Comma separated whitelist of CPU cores on which VDSM is 
allowed '
Line 213: 'to run. Only the first 64 CPUs can be specified. '
Line 214: 'Default is empty list, meaning VDSM can be scheduled by 
the OS '
"empty list" is confusing - use ""?
Line 215: 'to run on any core. '
Line 216: 'Valid examples: "0,1", "0,2,3"')
Line 217: 
Line 218: ]),


https://gerrit.ovirt.org/#/c/45738/5/lib/vdsm/taskset.py
File lib/vdsm/taskset.py:

Line 26: 
Line 27: class Error(Exception):
Line 28: def __init__(self, ecode, stderr):
Line 29: self.ecode = ecode
Line 30: self.stderr = stderr
Lets use same attributes as in udevadm.Error, and include also the output of 
the command.

We probably need one such error in vdsm and reuse it everywhere, instead of 
inventing an error for each module or flow.
Line 31: 
Line 32: def __str__(self):
Line 33: return '[Error %d] %s' % (self.ecode, self.stderr)
Line 34: 


Line 35: 
Line 36: def get(pid, all_tasks=True):
Line 37: # TODO: check pid
Line 38: cmd = [constants.EXT_TASKSET,
Line 39:'-a' if all_tasks else '',
Too dirty, lets simplify
Line 40:'-c',
Line 41:'-p', str(pid)]
Line 42: 
Line 43: rc, out, err = utils.execCmd(cmd)


Line 37: # TODO: check pid
Line 38: cmd = [constants.EXT_TASKSET,
Line 39:'-a' if all_tasks else '',
Line 40:'-c',
Line 41:'-p', str(pid)]
Use long arguments for more clear code and logs?
Line 42: 
Line 43: rc, out, err = utils.execCmd(cmd)
Line 44: 
Line 45: if rc != 0:


Line 39:'-a' if all_tasks else '',
Line 40:'-c',
Line 41:'-p', str(pid)]
Line 42: 
Line 43: rc, out, err = utils.execCmd(cmd)
This will run taskset within taskset, I think we can use resetCpuAffinity=False 
for this call.
Line 44: 
Line 45: if rc != 0:
Line 46: raise Error(rc, err)
Line 47: 


Line 48: return _expand_list(_extract_list(out))
Line 49: 
Line 50: 
Line 51: def set(pid, cpu_list, all_tasks=True):
Line 52: # TODO: check pid
Why kind of check? If you provide junk, the call will fail, so I'm not sure we 
need any check here.
Line 53: # warning: the order of options is significant.
Line 54: cmd = [constants.EXT_TASKSET,
Line 55:'-a' if all_tasks else '',
Line 56:'-p',


Line 51: def set(pid, cpu_list, all_tasks=True):
Line 52: # TODO: check pid
Line 53: # warning: the order of options is significant.
Line 54: cmd = [constants.EXT_TASKSET,
Line 55:'-a' if all_tasks else '',
Dirty, simplify
Line 56:'-p',
Line 57:'-c', ','.join(cpu_list),
Line 58:str(pid)]
Line 59: 


Line 52: # TODO: check pid
Line 53: # warning: the order of options is significant.
Line 54: cmd = [constants.EXT_TASKSET,
Line 55:'-a' if all_tasks else '',
Line 56:'-p',
-p should be before -c?

Use long arguments for more clear code and logs?
Line 57:'-c', ','.join(cpu_list),
Line 58:str(pid)]
Line 59: 
Line 60: rc, _, err = utils.execCmd(cmd)


Line 56:'-p',

Change in vdsm[master]: automation: enable pep8

[piotr . kliczewski]

Piotr Kliczewski has posted comments on this change.

Change subject: automation: enable pep8
..


Patch Set 1: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: If2266725af30e45b414d040f7f8d6b47d46bf947
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dan Kenigsberg 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Sivák 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: scale: limit cpu usage using cpu-affinity

[fromani]

Francesco Romani has posted comments on this change.

Change subject: scale: limit cpu usage using cpu-affinity
..


Patch Set 3:

(1 comment)

https://gerrit.ovirt.org/#/c/45738/3/lib/vdsm/cmdutils.py
File lib/vdsm/cmdutils.py:

Line 92: if index < CPU_MIN or index > CPU_MAX:
Line 93: raise ValueError('cpu index %i outside limits [%i, %i]',
Line 94:  index, CPU_MIN, CPU_MAX)
Line 95: mask |= (1 << index)
Line 96: return mask
> When getting cpu list, I think it will be simpler not to use -c/--cpu-list 
I like bit twiddling. Let's do this way.
Line 97: 
Line 98: # This function returns truthy value if its argument contains unsafe 
characters
Line 99: # for including in a command passed to the shell. The safe characters 
were
Line 100: # stolen from pipes._safechars.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I3f7f68d65eddb5a21afbc3809ea79cd1dee67984
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Polednik 
Gerrit-Reviewer: Martin Sivák 
Gerrit-Reviewer: Michal Skrivanek 
Gerrit-Reviewer: Michal Skrivanek 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: Vinzenz Feenstra 
Gerrit-Reviewer: Yaniv Bronhaim 
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: virtTests: add way to query full stats via jsonrpc

[ykaplan]

Yeela Kaplan has posted comments on this change.

Change subject: virtTests: add way to query full stats via jsonrpc
..


Patch Set 2:

Martin, I have to take this 2-liner into the functional tests patch.
Getting either one of them in without the other will break the tests...
Is that OK by you?

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ic2c2f50d862bfd1447aa812a037b5b41f3efd6af
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Martin Polednik 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Polednik 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: Yeela Kaplan 
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[ovirt-3.5]: net: fix vdsmd service start

[danken]

Dan Kenigsberg has posted comments on this change.

Change subject: net: fix vdsmd service start
..


Patch Set 2:

(2 comments)

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

Line 13: VDSM networks uses DHCP and `service network start` tries to run
Line 14: it second time.
Line 15: 
Line 16: With `service network restart` we first put dhclients down and then up.
Line 17: 
please add

 Label: ovirt-3.5-only

and explain why this is not needed on 3.6/master.
Line 18: Change-Id: I71342362f56b87bcb566c3343c90a69f95327ea6
Line 19: Bug-Url: https://bugzilla.redhat.com/1258551


https://gerrit.ovirt.org/#/c/45787/2/init/sysvinit/vdsmd.init.in
File init/sysvinit/vdsmd.init.in:

Line 117: re
the down side of this is that we slow down startup quite a bit.

Can you please review the original need for calling start_network() function?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I71342362f56b87bcb566c3343c90a69f95327ea6
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-3.5
Gerrit-Owner: Petr Horáček 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Ido Barkan 
Gerrit-Reviewer: Ondřej Svoboda 
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: [WIP]Java Bindings: Proton support in Java Bindings

[automation]

automat...@ovirt.org has posted comments on this change.

Change subject: [WIP]Java Bindings: Proton support in Java Bindings
..


Patch Set 3:

* Update tracker::IGNORE, no Bug-Url found

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I94c52e118cb63d7df84b89a9b93da7b9e477be91
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Saggi Mizrahi 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Saggi Mizrahi 
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: [WIP]Java Bindings: Proton support in Java Bindings

[Piotr Kliczewski]

Piotr Kliczewski has abandoned this change.

Change subject: [WIP]Java Bindings: Proton support in Java Bindings
..


Abandoned

We do not use proton anymore so this pach is not needed.

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

Gerrit-MessageType: abandon
Gerrit-Change-Id: I94c52e118cb63d7df84b89a9b93da7b9e477be91
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Saggi Mizrahi 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Saggi Mizrahi 
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: supervdsm: log actual error in ProxyCaller

[automation]

automat...@ovirt.org has posted comments on this change.

Change subject: supervdsm: log actual error in ProxyCaller
..


Patch Set 1:

* Update tracker::IGNORE, no Bug-Url found

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Id8a1c83d2a963002d3c793398bb62679b77f5f47
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Saggi Mizrahi 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Yaniv Bronhaim 
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: supervdsm: log actual error in ProxyCaller

[Piotr Kliczewski]

Piotr Kliczewski has abandoned this change.

Change subject: supervdsm: log actual error in ProxyCaller
..


Abandoned

Yaniv has a point so abandoning this patch.

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

Gerrit-MessageType: abandon
Gerrit-Change-Id: Id8a1c83d2a963002d3c793398bb62679b77f5f47
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Saggi Mizrahi 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Yaniv Bronhaim 
Gerrit-Reviewer: automat...@ovirt.org
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: automation: enable pep8

[danken]

Dan Kenigsberg has submitted this change and it was merged.

Change subject: automation: enable pep8
..


automation: enable pep8

We now have a single check-patch job.

Change-Id: If2266725af30e45b414d040f7f8d6b47d46bf947
Signed-off-by: Dan Kenigsberg 
Reviewed-on: https://gerrit.ovirt.org/45753
Continuous-Integration: Jenkins CI
Reviewed-by: Martin Sivák 
Reviewed-by: Piotr Kliczewski 
Reviewed-by: Francesco Romani 
---
M automation/check-patch.sh
1 file changed, 0 insertions(+), 2 deletions(-)

Approvals:
  Piotr Kliczewski: Looks good to me, but someone else must approve
  Martin Sivák: Looks good to me, but someone else must approve
  Jenkins CI: Passed CI tests
  Dan Kenigsberg: Verified; Looks good to me, approved
  Francesco Romani: Looks good to me, approved



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

Gerrit-MessageType: merged
Gerrit-Change-Id: If2266725af30e45b414d040f7f8d6b47d46bf947
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dan Kenigsberg 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Sivák 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: automat...@ovirt.org
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: automation: enable pep8

[automation]

automat...@ovirt.org has posted comments on this change.

Change subject: automation: enable pep8
..


Patch Set 2:

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

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

Gerrit-MessageType: comment
Gerrit-Change-Id: If2266725af30e45b414d040f7f8d6b47d46bf947
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dan Kenigsberg 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Sivák 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: Add an empty metadata qos element to the created domain

[danken]

Dan Kenigsberg has submitted this change and it was merged.

Change subject: Add an empty metadata qos element to the created domain
..


Add an empty metadata qos element to the created domain

Libvirt reports an error every time VDSM queries for the metadata
element when there is none. This patch adds an empty default
element to get rid of those errors.

Bug-Url: https://bugzilla.redhat.com/show_bug.cgi?id=1219903
Change-Id: Ibc5db925b55ea8e583a11d548a27dc3fd0886fee
Signed-off-by: Martin Sivak 
Reviewed-on: https://gerrit.ovirt.org/45664
Continuous-Integration: Jenkins CI
Reviewed-by: Francesco Romani 
---
M tests/vmTests.py
M tests/vmTestsData.py
M vdsm/virt/vm.py
M vdsm/virt/vmxml.py
4 files changed, 55 insertions(+), 9 deletions(-)

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



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

Gerrit-MessageType: merged
Gerrit-Change-Id: Ibc5db925b55ea8e583a11d548a27dc3fd0886fee
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Martin Sivák 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Sivák 
Gerrit-Reviewer: Michal Skrivanek 
Gerrit-Reviewer: Michal Skrivanek 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: automat...@ovirt.org
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: automation: enable pep8

[fromani]

Francesco Romani has posted comments on this change.

Change subject: automation: enable pep8
..


Patch Set 1: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: If2266725af30e45b414d040f7f8d6b47d46bf947
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dan Kenigsberg 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Sivák 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: automation: enable pep8

[danken]

Dan Kenigsberg has posted comments on this change.

Change subject: automation: enable pep8
..


Patch Set 1: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: If2266725af30e45b414d040f7f8d6b47d46bf947
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dan Kenigsberg 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Sivák 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: Add an empty metadata qos element to the created domain

[fromani]

Francesco Romani has posted comments on this change.

Change subject: Add an empty metadata qos element to the created domain
..


Patch Set 4: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ibc5db925b55ea8e583a11d548a27dc3fd0886fee
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Martin Sivák 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Sivák 
Gerrit-Reviewer: Michal Skrivanek 
Gerrit-Reviewer: Michal Skrivanek 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: scale: limit cpu usage using cpu-affinity

[automation]

automat...@ovirt.org has posted comments on this change.

Change subject: scale: limit cpu usage using cpu-affinity
..


Patch Set 5:

* Update tracker::#1247075::OK
* Check Bug-Url::OK
* Check Public Bug::#1247075::OK, public bug
* Check Product::#1247075::OK, Correct product Red Hat Enterprise 
Virtualization Manager
* Check TR::SKIP, not in a monitored branch (ovirt-3.5 ovirt-3.4 ovirt-3.3 
ovirt-3.2)
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 
'ovirt-3.4', 'ovirt-3.3'])

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I3f7f68d65eddb5a21afbc3809ea79cd1dee67984
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Polednik 
Gerrit-Reviewer: Martin Sivák 
Gerrit-Reviewer: Michal Skrivanek 
Gerrit-Reviewer: Michal Skrivanek 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: Vinzenz Feenstra 
Gerrit-Reviewer: Yaniv Bronhaim 
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: scale: limit cpu usage using cpu-affinity

[nsoffer]

Nir Soffer has posted comments on this change.

Change subject: scale: limit cpu usage using cpu-affinity
..


Patch Set 5:

(1 comment)

https://gerrit.ovirt.org/#/c/45738/5/tests/tasksetTests.py
File tests/tasksetTests.py:

Line 30: class AffinityTests(VdsmTestCase):
Line 31: 
Line 32: def setUp(self):
Line 33: self.running = multiprocessing.Event()
Line 34: self.stop = multiprocessing.Event()
Nice!
Line 35: self.proc = None
Line 36: 
Line 37: def tearDown(self):
Line 38: if self.proc is not None:


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I3f7f68d65eddb5a21afbc3809ea79cd1dee67984
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Polednik 
Gerrit-Reviewer: Martin Sivák 
Gerrit-Reviewer: Michal Skrivanek 
Gerrit-Reviewer: Michal Skrivanek 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: Vinzenz Feenstra 
Gerrit-Reviewer: Yaniv Bronhaim 
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: scale: limit cpu usage using cpu-affinity

[nsoffer]

Nir Soffer has posted comments on this change.

Change subject: scale: limit cpu usage using cpu-affinity
..


Patch Set 5:

(1 comment)

https://gerrit.ovirt.org/#/c/45738/5/lib/vdsm/taskset.py
File lib/vdsm/taskset.py:

Line 68: extractst the cpu list from taskset output, in the form
Line 69:   pid 's current affinity list: CPU_LIST
Line 70: """
Line 71: text, cpu_list = taskset_output[0].split(': ', 1)
Line 72: return cpu_list
I think we can inline this into get()
Line 73: 
Line 74: 
Line 75: def _expand_list(cpu_list):
Line 76: """


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I3f7f68d65eddb5a21afbc3809ea79cd1dee67984
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Polednik 
Gerrit-Reviewer: Martin Sivák 
Gerrit-Reviewer: Michal Skrivanek 
Gerrit-Reviewer: Michal Skrivanek 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: Vinzenz Feenstra 
Gerrit-Reviewer: Yaniv Bronhaim 
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: Add an empty metadata qos element to the created domain

[automation]

automat...@ovirt.org has posted comments on this change.

Change subject: Add an empty metadata qos element to the created domain
..


Patch Set 5:

* Update tracker::#1219903::OK
* Set MODIFIED::bug 1219903#1219903IGNORE, not oVirt prod but Red Hat 
Enterprise Virtualization Manager

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ibc5db925b55ea8e583a11d548a27dc3fd0886fee
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Martin Sivák 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Sivák 
Gerrit-Reviewer: Michal Skrivanek 
Gerrit-Reviewer: Michal Skrivanek 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: volume: Support older engine or disks with long description

[laravot]

Liron Aravot has posted comments on this change.

Change subject: volume: Support older engine or disks with long description
..


Patch Set 3:

(1 comment)

https://gerrit.ovirt.org/#/c/45501/3/vdsm/rpc/vdsmapi-schema.json
File vdsm/rpc/vdsmapi-schema.json:

Line 8496: #
Line 8497: # @imageID:  The Image associated with the Volume
Line 8498: #
Line 8499: # @description:  A human-readable Volume description.
Line 8500: #Up to 210 bytes; longer values will be truncated
this was handled only in https://gerrit.ovirt.org/#/c/45502/4 as it seems to 
me, am i missing something?
Line 8501: #
Line 8502: # Since: 4.10.0
Line 8503: ##
Line 8504: {'command': {'class': 'Volume', 'name': 'setDescription'},


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie3b1d9da0084aa5db02b29295d4cd0d5d8178ca2
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Ala Hino 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Freddy Rolland 
Gerrit-Reviewer: Idan Shaby 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Liron Aravot 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: scale: limit cpu usage using cpu-affinity

[fromani]

Francesco Romani has posted comments on this change.

Change subject: scale: limit cpu usage using cpu-affinity
..


Patch Set 4: Code-Review-1

bad version, submitted too early.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I3f7f68d65eddb5a21afbc3809ea79cd1dee67984
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Polednik 
Gerrit-Reviewer: Martin Sivák 
Gerrit-Reviewer: Michal Skrivanek 
Gerrit-Reviewer: Michal Skrivanek 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: Vinzenz Feenstra 
Gerrit-Reviewer: Yaniv Bronhaim 
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: scale: limit cpu usage using cpu-affinity

[nsoffer]

Nir Soffer has posted comments on this change.

Change subject: scale: limit cpu usage using cpu-affinity
..


Patch Set 3:

(2 comments)

https://gerrit.ovirt.org/#/c/45738/3/lib/vdsm/cmdutils.py
File lib/vdsm/cmdutils.py:

Line 92: if index < CPU_MIN or index > CPU_MAX:
Line 93: raise ValueError('cpu index %i outside limits [%i, %i]',
Line 94:  index, CPU_MIN, CPU_MAX)
Line 95: mask |= (1 << index)
Line 96: return mask
> it is needed the other way around, when we use taskset to get the affinity 
When getting cpu list, I think it will be simpler not to use -c/--cpu-list and 
convert from bits to list of cpus. Something like:

mask = int(mask, 16)
cpus = [i for i in range(mask.bit_length()) if mask & (1 << i)]
Line 97: 
Line 98: # This function returns truthy value if its argument contains unsafe 
characters
Line 99: # for including in a command passed to the shell. The safe characters 
were
Line 100: # stolen from pipes._safechars.


https://gerrit.ovirt.org/#/c/45738/3/tests/cmdutilsTests.py
File tests/cmdutilsTests.py:

Line 86: self.assertEquals(cmdutils._list2cmdline([]), "")
Line 87: 
Line 88: 
Line 89: @expandPermutations
Line 90: class CpuMaskTests(VdsmTestCase):
> yep, but we'll need something similar for the range syntax (see comments in
See my reply in cmdutils.py
Line 91: 
Line 92: @permutations([
Line 93:   ['a'],
Line 94:   ['1,a'],


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I3f7f68d65eddb5a21afbc3809ea79cd1dee67984
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Polednik 
Gerrit-Reviewer: Martin Sivák 
Gerrit-Reviewer: Michal Skrivanek 
Gerrit-Reviewer: Michal Skrivanek 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: Vinzenz Feenstra 
Gerrit-Reviewer: Yaniv Bronhaim 
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: automation: enable pep8

[fromani]

Francesco Romani has posted comments on this change.

Change subject: automation: enable pep8
..


Patch Set 1: Code-Review+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: If2266725af30e45b414d040f7f8d6b47d46bf947
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dan Kenigsberg 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Sivák 
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: Add an empty metadata qos element to the created domain

[automation]

automat...@ovirt.org has posted comments on this change.

Change subject: Add an empty metadata qos element to the created domain
..


Patch Set 4:

* Update tracker::#1219903::OK
* Check Bug-Url::OK
* Check Public Bug::#1219903::OK, public bug
* Check Product::#1219903::OK, Correct product Red Hat Enterprise 
Virtualization Manager
* Check TR::SKIP, not in a monitored branch (ovirt-3.5 ovirt-3.4 ovirt-3.3 
ovirt-3.2)
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 
'ovirt-3.4', 'ovirt-3.3'])

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ibc5db925b55ea8e583a11d548a27dc3fd0886fee
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Martin Sivák 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Sivák 
Gerrit-Reviewer: Michal Skrivanek 
Gerrit-Reviewer: Michal Skrivanek 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: vdscli: map invocation params to dictionary

[osvoboda]

Ondřej Svoboda has posted comments on this change.

Change subject: vdscli: map invocation params to dictionary
..


Patch Set 4: Code-Review-1

(2 comments)

Thanks for picking this up! I have a request for a less cryptic variable name 
and also for removing of intermediate lists.

https://gerrit.ovirt.org/#/c/45429/4/lib/vdsm/jsonrpcvdscli.py
File lib/vdsm/jsonrpcvdscli.py:

Line 47: _vapi
Why not _vdsmapi?


Line 53: allargs
I think it is sufficient for allargs to be just a generator and that it should 
be constructed only once, with iterkeys().


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ibac6eab3c519becb29d2b355d671bbb79df5
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Piotr Kliczewski 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Polednik 
Gerrit-Reviewer: Ondřej Svoboda 
Gerrit-Reviewer: Petr Horáček 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: Yeela Kaplan 
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: Add an empty metadata qos element to the created domain

[msivak]

Martin Sivák has posted comments on this change.

Change subject: Add an empty metadata qos element to the created domain
..


Patch Set 4: Verified+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ibc5db925b55ea8e583a11d548a27dc3fd0886fee
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Martin Sivák 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Sivák 
Gerrit-Reviewer: Michal Skrivanek 
Gerrit-Reviewer: Michal Skrivanek 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: zombiereaper: Add ability to wait on process

[nsoffer]

Nir Soffer has posted comments on this change.

Change subject: zombiereaper: Add ability to wait on process
..


Patch Set 1: Code-Review-1

I don't think this is the right approach.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Idcd69b6187bc1c412c11f6cba9af431557ea7077
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Saggi Mizrahi 
Gerrit-Reviewer: Barak Azulay 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: Saggi Mizrahi 
Gerrit-Reviewer: Yaniv Bronhaim 
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: zombiereaper: Add ability to wait on process

[piotr . kliczewski]

Piotr Kliczewski has posted comments on this change.

Change subject: zombiereaper: Add ability to wait on process
..


Patch Set 1:

Do you suggest to abandon this patch and post new one to solve this issue or 
use this one to do it or just abandon it?

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Idcd69b6187bc1c412c11f6cba9af431557ea7077
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Saggi Mizrahi 
Gerrit-Reviewer: Barak Azulay 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: Saggi Mizrahi 
Gerrit-Reviewer: Yaniv Bronhaim 
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[ovirt-3.5]: net: backport cmdutils.systemd_run

[automation]

automat...@ovirt.org has posted comments on this change.

Change subject: net: backport cmdutils.systemd_run
..


Patch Set 5:

* Update tracker::#1228322::OK
* Check Bug-Url::OK
* Check Public Bug::#1228322::OK, public bug
* Check Product::#1228322::OK, Correct product oVirt
* Check TR::#1228322::OK, correct target release 3.5.5
* warn_if_not_merged_to_previous_branch: OK

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I7686e80c0880dcc28fa735d1dd3ab658136889f9
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-3.5
Gerrit-Owner: Ido Barkan 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Ido Barkan 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Yaniv Bronhaim 
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[ovirt-3.6]: Add an empty metadata qos element to the created domain

[msivak]

Martin Sivák has uploaded a new change for review.

Change subject: Add an empty metadata qos element to the created domain
..

Add an empty metadata qos element to the created domain

Libvirt reports an error every time VDSM queries for the metadata
element when there is none. This patch adds an empty default
element to get rid of those errors.

Bug-Url: https://bugzilla.redhat.com/show_bug.cgi?id=1219903
Change-Id: Ibc5db925b55ea8e583a11d548a27dc3fd0886fee
Signed-off-by: Martin Sivak 
Reviewed-on: https://gerrit.ovirt.org/45664
Continuous-Integration: Jenkins CI
Reviewed-by: Francesco Romani 
(cherry picked from commit deadb606de96fc84d8d912d56bbc042974074f54)
---
M tests/vmTests.py
M tests/vmTestsData.py
M vdsm/virt/vm.py
M vdsm/virt/vmxml.py
4 files changed, 55 insertions(+), 9 deletions(-)


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

diff --git a/tests/vmTests.py b/tests/vmTests.py
index 1ac1ea7..d48526b 100644
--- a/tests/vmTests.py
+++ b/tests/vmTests.py
@@ -113,13 +113,16 @@
 
 def testDomXML(self):
 expectedXML = """
-   
+   http://ovirt.org/vm/tune/1.0; type="kvm">
   testVm
   9ffe28b6-6134-4b1e-8804-1185f49c436f
   1048576
   1048576
   160
   
+  
+ 
+  
"""
 
 domxml = vmxml.Domain(self.conf, self.log, caps.Architecture.X86_64)
diff --git a/tests/vmTestsData.py b/tests/vmTestsData.py
index 9f17976..e1f53f3 100644
--- a/tests/vmTestsData.py
+++ b/tests/vmTestsData.py
@@ -32,7 +32,8 @@
 'guestNumaNodes': []},
 
 """
-
+http://ovirt.org/vm/tune/1.0;>
 testVm
 %(vmId)s
 1048576
@@ -51,6 +52,9 @@
 
 
 
+
+
+
 
 hvm
 
@@ -97,6 +101,7 @@
 
 """
 http://ovirt.org/vm/tune/1.0;
 xmlns:qemu="http://libvirt.org/schemas/domain/qemu/1.0;>
 testVm
 %(vmId)s
@@ -117,6 +122,9 @@
 
 /usr/bin/qemu-system-ppc64
 
+
+
+
 
 hvm
 
diff --git a/vdsm/virt/vm.py b/vdsm/virt/vm.py
index a398fe6..7b6dcfb 100644
--- a/vdsm/virt/vm.py
+++ b/vdsm/virt/vm.py
@@ -73,6 +73,8 @@
 from .vmtune import update_io_tune_dom, collect_inner_elements
 from .vmtune import io_tune_values_to_dom, io_tune_dom_to_values
 from . import vmxml
+from .vmxml import METADATA_VM_TUNE_URI, METADATA_VM_TUNE_ELEMENT
+from .vmxml import METADATA_VM_TUNE_PREFIX
 
 from .utils import isVdsmImage, cleanup_guest_socket
 from vmpowerdown import VmShutdown, VmReboot
@@ -84,8 +86,6 @@
 _AGENT_CHANNEL_DEVICES = (_VMCHANNEL_DEVICE_NAME, _QEMU_GA_DEVICE_NAME)
 
 DEFAULT_BRIDGE = config.get("vars", "default_bridge")
-
-METADATA_VM_TUNE_URI = 'http://ovirt.org/vm/tune/1.0'
 
 # A libvirt constant for undefined cpu quota
 _NO_CPU_QUOTA = 0
@@ -2368,7 +2368,7 @@
 
 try:
 self._dom.setMetadata(libvirt.VIR_DOMAIN_METADATA_ELEMENT,
-  metadata_xml, 'ovirt',
+  metadata_xml, METADATA_VM_TUNE_PREFIX,
   METADATA_VM_TUNE_URI, 0)
 except libvirt.libvirtError as e:
 self.log.exception("updateVmPolicy failed")
@@ -2389,7 +2389,8 @@
 :return: XML DOM object representing the root qos element
 """
 
-metadata_xml = ""
+metadata_xml = "<%s>" % (METADATA_VM_TUNE_ELEMENT,
+  METADATA_VM_TUNE_ELEMENT)
 
 try:
 metadata_xml = self._dom.metadata(
@@ -2401,7 +2402,7 @@
 return None
 
 metadata = _domParseStr(metadata_xml)
-return metadata.getElementsByTagName("qos")[0]
+return metadata.getElementsByTagName(METADATA_VM_TUNE_ELEMENT)[0]
 
 def _findDeviceByNameOrPath(self, device_name, device_path):
 for device in self._devices[hwclass.DISK]:
diff --git a/vdsm/virt/vmxml.py b/vdsm/virt/vmxml.py
index 2c51272..60e3357 100644
--- a/vdsm/virt/vmxml.py
+++ b/vdsm/virt/vmxml.py
@@ -28,6 +28,10 @@
 
 import caps
 
+METADATA_VM_TUNE_URI = 'http://ovirt.org/vm/tune/1.0'
+METADATA_VM_TUNE_ELEMENT = 'qos'
+METADATA_VM_TUNE_PREFIX = 'ovirt'
+
 
 def has_channel(domXML, name):
 domObj = etree.fromstring(domXML)
@@ -91,8 +95,12 @@
 
 class Element(object):
 
-def __init__(self, tagName, text=None, **attrs):
-self._elem = xml.dom.minidom.Document().createElement(tagName)
+def __init__(self, tagName, text=None, namespaceUri=None, **attrs):
+if namespaceUri is not None:
+   

Change in vdsm[ovirt-3.6]: Add an empty metadata qos element to the created domain

[automation]

automat...@ovirt.org has posted comments on this change.

Change subject: Add an empty metadata qos element to the created domain
..


Patch Set 1:

* Update tracker::#1219903::OK
* Check Bug-Url::OK
* Check Public Bug::#1219903::OK, public bug
* Check Product::#1219903::OK, Correct product Red Hat Enterprise 
Virtualization Manager
* Check TR::SKIP, not in a monitored branch (ovirt-3.5 ovirt-3.4 ovirt-3.3 
ovirt-3.2)
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 
'ovirt-3.4', 'ovirt-3.3'])

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ibc5db925b55ea8e583a11d548a27dc3fd0886fee
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-3.6
Gerrit-Owner: Martin Sivák 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: scale: limit cpu usage using cpu-affinity

[fromani]

Francesco Romani has posted comments on this change.

Change subject: scale: limit cpu usage using cpu-affinity
..


Patch Set 5:

(15 comments)

https://gerrit.ovirt.org/#/c/45738/5/lib/vdsm/cmdutils.py
File lib/vdsm/cmdutils.py:

Line 69: 
Line 70: def taskset(cmd, cpu_list, all_tasks=True):
Line 71: command = [constants.EXT_TASKSET,
Line 72:'-a' if all_tasks else '',
Line 73:'-c', ','.join(cpu_list)]
> I think we should use the long options for better readability in the code a
OK for long options. Unfortunately, taskset is quite picky about the order of 
the options:

  $ taskset --cpu-list 1,2 --all-tasks /bin/sleep 1m
taskset: failed to execute --all-tasks: No such file or directory
Line 74: command.extend(cmd)
Line 75: return command
Line 76: 
Line 77: 


https://gerrit.ovirt.org/#/c/45738/5/lib/vdsm/config.py.in
File lib/vdsm/config.py.in:

Line 209: 'Time in seconds defining how frequently we log transport 
stats'),
Line 210: 
Line 211: ('cpu_affinity', '',
Line 212: 'Comma separated whitelist of CPU cores on which VDSM is 
allowed '
Line 213: 'to run. Only the first 64 CPUs can be specified. '
> Are you sure about 64 limit? taskset(1) does not mention this. Maybe this i
right, let me drop this reference.
Line 214: 'Default is empty list, meaning VDSM can be scheduled by 
the OS '
Line 215: 'to run on any core. '
Line 216: 'Valid examples: "0,1", "0,2,3"')
Line 217: 


Line 210: 
Line 211: ('cpu_affinity', '',
Line 212: 'Comma separated whitelist of CPU cores on which VDSM is 
allowed '
Line 213: 'to run. Only the first 64 CPUs can be specified. '
Line 214: 'Default is empty list, meaning VDSM can be scheduled by 
the OS '
> "empty list" is confusing - use ""?
Done
Line 215: 'to run on any core. '
Line 216: 'Valid examples: "0,1", "0,2,3"')
Line 217: 
Line 218: ]),


https://gerrit.ovirt.org/#/c/45738/5/lib/vdsm/taskset.py
File lib/vdsm/taskset.py:

Line 26: 
Line 27: class Error(Exception):
Line 28: def __init__(self, ecode, stderr):
Line 29: self.ecode = ecode
Line 30: self.stderr = stderr
> Lets use same attributes as in udevadm.Error, and include also the output o
Done. I can count at least three nearly-identical clones (udevadm, taskset, 
virtsparsify). We'll remove this duplication in a later patch.
Line 31: 
Line 32: def __str__(self):
Line 33: return '[Error %d] %s' % (self.ecode, self.stderr)
Line 34: 


Line 35: 
Line 36: def get(pid, all_tasks=True):
Line 37: # TODO: check pid
Line 38: cmd = [constants.EXT_TASKSET,
Line 39:'-a' if all_tasks else '',
> Too dirty, lets simplify
I agree it is dirty, simplification is not trivial due to the taskset pickiness.
Line 40:'-c',
Line 41:'-p', str(pid)]
Line 42: 
Line 43: rc, out, err = utils.execCmd(cmd)


Line 37: # TODO: check pid
Line 38: cmd = [constants.EXT_TASKSET,
Line 39:'-a' if all_tasks else '',
Line 40:'-c',
Line 41:'-p', str(pid)]
> Use long arguments for more clear code and logs?
Done
Line 42: 
Line 43: rc, out, err = utils.execCmd(cmd)
Line 44: 
Line 45: if rc != 0:


Line 39:'-a' if all_tasks else '',
Line 40:'-c',
Line 41:'-p', str(pid)]
Line 42: 
Line 43: rc, out, err = utils.execCmd(cmd)
> This will run taskset within taskset, I think we can use resetCpuAffinity=F
OOps. stupid error. Thanks, fixed.
Line 44: 
Line 45: if rc != 0:
Line 46: raise Error(rc, err)
Line 47: 


Line 48: return _expand_list(_extract_list(out))
Line 49: 
Line 50: 
Line 51: def set(pid, cpu_list, all_tasks=True):
Line 52: # TODO: check pid
> Why kind of check? If you provide junk, the call will fail, so I'm not sure
Agreed. We should follow the GIGO (Garbage In, Garbage Out) rule. Let's remove 
it
Line 53: # warning: the order of options is significant.
Line 54: cmd = [constants.EXT_TASKSET,
Line 55:'-a' if all_tasks else '',
Line 56:'-p',


Line 51: def set(pid, cpu_list, all_tasks=True):
Line 52: # TODO: check pid
Line 53: # warning: the order of options is significant.
Line 54: cmd = [constants.EXT_TASKSET,
Line 55:'-a' if all_tasks else '',
> Dirty, simplify
I'd love to. See above/elsewhere.
Line 56:'-p',
Line 57:'-c', ','.join(cpu_list),
Line 58:str(pid)]
Line 59: 


Line 52: # TODO: check pid
Line 53: # warning: the order of options is significant.
Line 54: cmd = [constants.EXT_TASKSET,
Line 55:'-a' if all_tasks else '',
Line 56:'-p',
> -p should be before -c?
Yep. More taskset oddities. Example:

  68 14:30:55 fromani@musashi ~/Projects/upstream/vdsm $ taskset -a -p -c 1,2 

Change in vdsm[master]: net: fix test_setupNetworks_on_external_bond

[osvoboda]

Ondřej Svoboda has posted comments on this change.

Change subject: net: fix test_setupNetworks_on_external_bond
..


Patch Set 1: Code-Review+1 Verified+1

The test passes on Fedora 22 for me :-)

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia29761aa2ec920db4485bf704926b67e0d9851b9
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Petr Horáček 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Ido Barkan 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Ondřej Svoboda 
Gerrit-Reviewer: Petr Horáček 
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: volume: Support older engine or disks with long description

[laravot]

Liron Aravot has posted comments on this change.

Change subject: volume: Support older engine or disks with long description
..


Patch Set 3:

(3 comments)

https://gerrit.ovirt.org/#/c/45501/3/vdsm/storage/volume.py
File vdsm/storage/volume.py:

Line 884: cls.createMetadata(metaId, meta)
Line 885: return meta
Line 886: 
Line 887: @classmethod
Line 888: def validateDescription(cls, desc):
i'd suggest to rename that to "adjustDescription", we don't validate anything 
here.
Line 889: desc = str(desc)
Line 890: # We cannot fail when the description is too long, since we 
must
Line 891: # support older engine that may send such values, or old disks
Line 892: # with long description.


Line 890: # We cannot fail when the description is too long, since we 
must
Line 891: # support older engine that may send such values, or old disks
Line 892: # with long description.
Line 893: if len(desc) > DESCRIPTION_SIZE:
Line 894: cls.log.warning("Description it too long, truncating to 
%d bytes",
/s/it/is
Line 895: DESCRIPTION_SIZE)
Line 896: desc = desc[:DESCRIPTION_SIZE]
Line 897: return desc
Line 898: 


Line 891: # support older engine that may send such values, or old disks
Line 892: # with long description.
Line 893: if len(desc) > DESCRIPTION_SIZE:
Line 894: cls.log.warning("Description it too long, truncating to 
%d bytes",
Line 895: DESCRIPTION_SIZE)
consider to log the description before it was truncated.
Line 896: desc = desc[:DESCRIPTION_SIZE]
Line 897: return desc
Line 898: 
Line 899: def getInfo(self):


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie3b1d9da0084aa5db02b29295d4cd0d5d8178ca2
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Ala Hino 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Freddy Rolland 
Gerrit-Reviewer: Idan Shaby 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Liron Aravot 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: scale: limit cpu usage using cpu-affinity

[automation]

automat...@ovirt.org has posted comments on this change.

Change subject: scale: limit cpu usage using cpu-affinity
..


Patch Set 6:

* Update tracker::#1247075::OK
* Check Bug-Url::OK
* Check Public Bug::#1247075::OK, public bug
* Check Product::#1247075::OK, Correct product Red Hat Enterprise 
Virtualization Manager
* Check TR::SKIP, not in a monitored branch (ovirt-3.5 ovirt-3.4 ovirt-3.3 
ovirt-3.2)
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 
'ovirt-3.4', 'ovirt-3.3'])

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I3f7f68d65eddb5a21afbc3809ea79cd1dee67984
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Polednik 
Gerrit-Reviewer: Martin Sivák 
Gerrit-Reviewer: Michal Skrivanek 
Gerrit-Reviewer: Michal Skrivanek 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: Vinzenz Feenstra 
Gerrit-Reviewer: Yaniv Bronhaim 
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: scale: limit cpu usage using cpu-affinity

[fromani]

Francesco Romani has posted comments on this change.

Change subject: scale: limit cpu usage using cpu-affinity
..


Patch Set 6:

(2 comments)

https://gerrit.ovirt.org/#/c/45738/6/lib/vdsm/taskset.py
File lib/vdsm/taskset.py:

Line 36: self.rc, self.out, self.err)
Line 37: 
Line 38: 
Line 39: def get(pid, all_tasks=True):
Line 40: # TODO: check pid
stale comment, forgot to remove.
Line 41: cmd = [constants.EXT_TASKSET,
Line 42:'--all-tasks' if all_tasks else '',
Line 43:'--pid', str(pid)]
Line 44: 


Line 50: return _cpu_list_from_output(out[0])
Line 51: 
Line 52: 
Line 53: def set(pid, cpu_list, all_tasks=True):
Line 54: # TODO: check pid
ditto
Line 55: # warning: the order of options is significant.
Line 56: cmd = [constants.EXT_TASKSET,
Line 57:'--all-tasks' if all_tasks else '',
Line 58:'--pid',


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I3f7f68d65eddb5a21afbc3809ea79cd1dee67984
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Polednik 
Gerrit-Reviewer: Martin Sivák 
Gerrit-Reviewer: Michal Skrivanek 
Gerrit-Reviewer: Michal Skrivanek 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: Vinzenz Feenstra 
Gerrit-Reviewer: Yaniv Bronhaim 
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: scale: limit cpu usage using cpu-affinity

[fromani]

Francesco Romani has posted comments on this change.

Change subject: scale: limit cpu usage using cpu-affinity
..


Patch Set 6: Code-Review-1

addressed all the comments, but lacks tests.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I3f7f68d65eddb5a21afbc3809ea79cd1dee67984
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Polednik 
Gerrit-Reviewer: Martin Sivák 
Gerrit-Reviewer: Michal Skrivanek 
Gerrit-Reviewer: Michal Skrivanek 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: Vinzenz Feenstra 
Gerrit-Reviewer: Yaniv Bronhaim 
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: scheduler: use single instance

[piotr . kliczewski]

Piotr Kliczewski has posted comments on this change.

Change subject: scheduler: use single instance
..


Patch Set 8: Verified+1

Verified by host deploying and seeing that communication is OK.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: If83eded458f8304d802fcd75839e7a916146918b
Gerrit-PatchSet: 8
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Piotr Kliczewski 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: Yaniv Bronhaim 
Gerrit-Reviewer: Yeela Kaplan 
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[ovirt-3.6]: Add an empty metadata qos element to the created domain

[msivak]

Martin Sivák has posted comments on this change.

Change subject: Add an empty metadata qos element to the created domain
..


Patch Set 1: Verified+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ibc5db925b55ea8e583a11d548a27dc3fd0886fee
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-3.6
Gerrit-Owner: Martin Sivák 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Sivák 
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: vdscli: map invocation params to dictionary

[piotr . kliczewski]

Piotr Kliczewski has posted comments on this change.

Change subject: vdscli: map invocation params to dictionary
..


Patch Set 4:

(2 comments)

https://gerrit.ovirt.org/#/c/45429/4/lib/vdsm/jsonrpcvdscli.py
File lib/vdsm/jsonrpcvdscli.py:

Line 47: _vapi
> Why not _vdsmapi?
Done


Line 53: allargs
> I think it is sufficient for allargs to be just a generator and that it sho
I think that iterkeys are removed for python3. Please note that we get allargs 
for each class and method that we attempt to run. Please give an example what 
do you want to have it here.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ibac6eab3c519becb29d2b355d671bbb79df5
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Piotr Kliczewski 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Polednik 
Gerrit-Reviewer: Ondřej Svoboda 
Gerrit-Reviewer: Petr Horáček 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: Yeela Kaplan 
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: sp: startSpm - ignore InquireNotSupportedError()

[nsoffer]

Nir Soffer has posted comments on this change.

Change subject: sp: startSpm - ignore InquireNotSupportedError()
..


Patch Set 1:

(1 comment)

https://gerrit.ovirt.org/#/c/45764/1/vdsm/storage/sp.py
File vdsm/storage/sp.py:

Line 247: raise se.CurrentVersionTooAdvancedError(
Line 248: self.masterDomain.sdUUID, curVer=masterDomVersion,
Line 249: expVer=expectedDomVersion)
Line 250: 
Line 251: oldlver = LVER_INVALID
Init in the expect
Line 252: try:
Line 253: oldlver, oldid = self._backend.getSpmStatus()
Line 254: except se.InquireNotSupportedError:
Line 255: self.log.info("cluster lock inquire isn't supported. "


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I082dc83ea410768db3819e7259089c20c2614b07
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Liron Aravot 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: schema: rpm for jsonrpc schema files

[piotr . kliczewski]

Piotr Kliczewski has posted comments on this change.

Change subject: schema: rpm for jsonrpc schema files
..


Patch Set 3: Verified+1

Removed generated file, no other changes. Copying verification flag from 
previous patch set.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I13d6291ddbf5bf7d8e6a0956db3300cd0c45e563
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Piotr Kliczewski 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: Yaniv Bronhaim 
Gerrit-Reviewer: Yeela Kaplan 
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: vdscli: map invocation params to dictionary

[automation]

automat...@ovirt.org has posted comments on this change.

Change subject: vdscli: map invocation params to dictionary
..


Patch Set 5:

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

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ibac6eab3c519becb29d2b355d671bbb79df5
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Piotr Kliczewski 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Polednik 
Gerrit-Reviewer: Ondřej Svoboda 
Gerrit-Reviewer: Petr Horáček 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: Yeela Kaplan 
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: schema: rpm for jsonrpc schema files

[automation]

automat...@ovirt.org has posted comments on this change.

Change subject: schema: rpm for jsonrpc schema files
..


Patch Set 3:

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

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I13d6291ddbf5bf7d8e6a0956db3300cd0c45e563
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Piotr Kliczewski 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: Yaniv Bronhaim 
Gerrit-Reviewer: Yeela Kaplan 
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: scale: limit cpu usage using cpu-affinity

[nsoffer]

Nir Soffer has posted comments on this change.

Change subject: scale: limit cpu usage using cpu-affinity
..


Patch Set 6: Code-Review-1

(8 comments)

https://gerrit.ovirt.org/#/c/45738/6/lib/vdsm/cmdutils.py
File lib/vdsm/cmdutils.py:

Line 69: 
Line 70: def taskset(cmd, cpu_list, all_tasks=True):
Line 71: command = [constants.EXT_TASKSET,
Line 72:'--all-tasks' if all_tasks else '',
Line 73:'--cpu-list', ','.join(cpu_list)]
While taskset order requirements are annoying, we can write in a cleaner way 
easily:

command = [constants.EXT_TASKSET]
if all_tasks:
command.append("--all-tasks")
command.extend(("--cpu-list", ",".join(cpu_list)))
...
Line 74: command.extend(cmd)
Line 75: return command
Line 76: 
Line 77: 


https://gerrit.ovirt.org/#/c/45738/6/lib/vdsm/taskset.py
File lib/vdsm/taskset.py:

Line 35: return "Process failed with rc=%d out=%r err=%r" % (
Line 36: self.rc, self.out, self.err)
Line 37: 
Line 38: 
Line 39: def get(pid, all_tasks=True):
need to document the return value.
Line 40: # TODO: check pid
Line 41: cmd = [constants.EXT_TASKSET,
Line 42:'--all-tasks' if all_tasks else '',
Line 43:'--pid', str(pid)]


Line 38: 
Line 39: def get(pid, all_tasks=True):
Line 40: # TODO: check pid
Line 41: cmd = [constants.EXT_TASKSET,
Line 42:'--all-tasks' if all_tasks else '',
Can be cleaner, see comment in cmdutils.
Line 43:'--pid', str(pid)]
Line 44: 
Line 45: rc, out, err = utils.execCmd(cmd, resetCpuAffinity=False)
Line 46: 


Line 46: 
Line 47: if rc != 0:
Line 48: raise Error(rc, out, err)
Line 49: 
Line 50: return _cpu_list_from_output(out[0])
Are you sure that we always need the first line? Is it possible that we get a 
warning in the first line and the result in the second line? In this case, 
taking the last line is better.
Line 51: 
Line 52: 
Line 53: def set(pid, cpu_list, all_tasks=True):
Line 54: # TODO: check pid


Line 49: 
Line 50: return _cpu_list_from_output(out[0])
Line 51: 
Line 52: 
Line 53: def set(pid, cpu_list, all_tasks=True):
need to document that cpu_list is an iterable of strings, either cpu number 
("1") or range ("1-3").
Line 54: # TODO: check pid
Line 55: # warning: the order of options is significant.
Line 56: cmd = [constants.EXT_TASKSET,
Line 57:'--all-tasks' if all_tasks else '',


Line 53: def set(pid, cpu_list, all_tasks=True):
Line 54: # TODO: check pid
Line 55: # warning: the order of options is significant.
Line 56: cmd = [constants.EXT_TASKSET,
Line 57:'--all-tasks' if all_tasks else '',
Can be cleaner, see comment in cmdutils.
Line 58:'--pid',
Line 59:'--cpu-list', ','.join(cpu_list),
Line 60:str(pid)]
Line 61: 


Line 64: if rc != 0:
Line 65: raise Error(rc, out, err)
Line 66: 
Line 67: 
Line 68: def _cpu_list_from_output(line):
Can you show here the typical output of the command, making it easier to 
maintain?
Line 69: cpu_list = line.split(': ', 1)
Line 70: mask = int(cpu_list[1], 16)


Line 65: raise Error(rc, out, err)
Line 66: 
Line 67: 
Line 68: def _cpu_list_from_output(line):
Line 69: cpu_list = line.split(': ', 1)
I think we should use rsplit, in case the command format changes to something 
like "xxx: yyy: e".

Also, spliting on ": " is more fragile, better split on ":" and strip the 
result.

Also the result of the split is not cpu_list. We should use something like:

hexmask = line.rsplit(":", 1)[1].strip()
Line 70: mask = int(cpu_list[1], 16)


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I3f7f68d65eddb5a21afbc3809ea79cd1dee67984
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Polednik 
Gerrit-Reviewer: Martin Sivák 
Gerrit-Reviewer: Michal Skrivanek 
Gerrit-Reviewer: Michal Skrivanek 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: Vinzenz Feenstra 
Gerrit-Reviewer: Yaniv Bronhaim 
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: scale: limit cpu usage using cpu-affinity

[nsoffer]

Nir Soffer has posted comments on this change.

Change subject: scale: limit cpu usage using cpu-affinity
..


Patch Set 6:

(1 comment)

https://gerrit.ovirt.org/#/c/45738/6//COMMIT_MSG
Commit Message:

Line 11: 
Line 12: Initial results using taskset manually provided very
Line 13: interesting results (more details in rhbz#1247075):
Line 14: 
Line 15: ---clip here---
The --clip here--- comments are too smart. I would describe shortly what was 
tested.
Line 16: [...]
Line 17: tested pinning all vdsm threads to cores when running 110 idle VMs:
Line 18: [...]
Line 19: 


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I3f7f68d65eddb5a21afbc3809ea79cd1dee67984
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Polednik 
Gerrit-Reviewer: Martin Sivák 
Gerrit-Reviewer: Michal Skrivanek 
Gerrit-Reviewer: Michal Skrivanek 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: Vinzenz Feenstra 
Gerrit-Reviewer: Yaniv Bronhaim 
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: scale: limit cpu usage using cpu-affinity

[fromani]

Francesco Romani has posted comments on this change.

Change subject: scale: limit cpu usage using cpu-affinity
..


Patch Set 6:

(2 comments)

https://gerrit.ovirt.org/#/c/45738/6/lib/vdsm/taskset.py
File lib/vdsm/taskset.py:

Line 36: self.rc, self.out, self.err)
Line 37: 
Line 38: 
Line 39: def get(pid, all_tasks=True):
Line 40: # TODO: check pid
> stale comment, forgot to remove.
Done
Line 41: cmd = [constants.EXT_TASKSET,
Line 42:'--all-tasks' if all_tasks else '',
Line 43:'--pid', str(pid)]
Line 44: 


Line 50: return _cpu_list_from_output(out[0])
Line 51: 
Line 52: 
Line 53: def set(pid, cpu_list, all_tasks=True):
Line 54: # TODO: check pid
> ditto
Done
Line 55: # warning: the order of options is significant.
Line 56: cmd = [constants.EXT_TASKSET,
Line 57:'--all-tasks' if all_tasks else '',
Line 58:'--pid',


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I3f7f68d65eddb5a21afbc3809ea79cd1dee67984
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Polednik 
Gerrit-Reviewer: Martin Sivák 
Gerrit-Reviewer: Michal Skrivanek 
Gerrit-Reviewer: Michal Skrivanek 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: Vinzenz Feenstra 
Gerrit-Reviewer: Yaniv Bronhaim 
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: volume: Support older engine or disks with long description

[nsoffer]

Nir Soffer has posted comments on this change.

Change subject: volume: Support older engine or disks with long description
..


Patch Set 3:

(1 comment)

https://gerrit.ovirt.org/#/c/45501/3/vdsm/rpc/vdsmapi-schema.json
File vdsm/rpc/vdsmapi-schema.json:

Line 8496: #
Line 8497: # @imageID:  The Image associated with the Volume
Line 8498: #
Line 8499: # @description:  A human-readable Volume description.
Line 8500: #Up to 210 bytes; longer values will be truncated
> this was handled only in https://gerrit.ovirt.org/#/c/45502/4 as it seems t
I think you are missing line 636 in  
https://gerrit.ovirt.org/#/c/45501/3/vdsm/storage/volume.py.
Line 8501: #
Line 8502: # Since: 4.10.0
Line 8503: ##
Line 8504: {'command': {'class': 'Volume', 'name': 'setDescription'},


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie3b1d9da0084aa5db02b29295d4cd0d5d8178ca2
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Ala Hino 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Freddy Rolland 
Gerrit-Reviewer: Idan Shaby 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Liron Aravot 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: scale: limit cpu usage using cpu-affinity

[fromani]

Francesco Romani has posted comments on this change.

Change subject: scale: limit cpu usage using cpu-affinity
..


Patch Set 6:

(1 comment)

https://gerrit.ovirt.org/#/c/45738/6//COMMIT_MSG
Commit Message:

Line 11: 
Line 12: Initial results using taskset manually provided very
Line 13: interesting results (more details in rhbz#1247075):
Line 14: 
Line 15: ---clip here---
> The --clip here--- comments are too smart. I would describe shortly what wa
Done
Line 16: [...]
Line 17: tested pinning all vdsm threads to cores when running 110 idle VMs:
Line 18: [...]
Line 19: 


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I3f7f68d65eddb5a21afbc3809ea79cd1dee67984
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Polednik 
Gerrit-Reviewer: Martin Sivák 
Gerrit-Reviewer: Michal Skrivanek 
Gerrit-Reviewer: Michal Skrivanek 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: Vinzenz Feenstra 
Gerrit-Reviewer: Yaniv Bronhaim 
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: volume: Support older engine or disks with long description

[nsoffer]

Nir Soffer has posted comments on this change.

Change subject: volume: Support older engine or disks with long description
..


Patch Set 3:

(3 comments)

https://gerrit.ovirt.org/#/c/45501/3/vdsm/storage/volume.py
File vdsm/storage/volume.py:

Line 884: cls.createMetadata(metaId, meta)
Line 885: return meta
Line 886: 
Line 887: @classmethod
Line 888: def validateDescription(cls, desc):
> i'd suggest to rename that to "adjustDescription", we don't validate anythi
We validate that it is string. This will raise UnicodeEncodeError if desc is 
unicode.
Line 889: desc = str(desc)
Line 890: # We cannot fail when the description is too long, since we 
must
Line 891: # support older engine that may send such values, or old disks
Line 892: # with long description.


Line 890: # We cannot fail when the description is too long, since we 
must
Line 891: # support older engine that may send such values, or old disks
Line 892: # with long description.
Line 893: if len(desc) > DESCRIPTION_SIZE:
Line 894: cls.log.warning("Description it too long, truncating to 
%d bytes",
> /s/it/is
Thanks, will fix in next version.
Line 895: DESCRIPTION_SIZE)
Line 896: desc = desc[:DESCRIPTION_SIZE]
Line 897: return desc
Line 898: 


Line 891: # support older engine that may send such values, or old disks
Line 892: # with long description.
Line 893: if len(desc) > DESCRIPTION_SIZE:
Line 894: cls.log.warning("Description it too long, truncating to 
%d bytes",
Line 895: DESCRIPTION_SIZE)
> consider to log the description before it was truncated.
It is already logged in hsm - we log all input parameters to all vdsm verbs.
Line 896: desc = desc[:DESCRIPTION_SIZE]
Line 897: return desc
Line 898: 
Line 899: def getInfo(self):


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie3b1d9da0084aa5db02b29295d4cd0d5d8178ca2
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Ala Hino 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Freddy Rolland 
Gerrit-Reviewer: Idan Shaby 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Liron Aravot 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: scale: limit cpu usage using cpu-affinity

[fromani]

Francesco Romani has posted comments on this change.

Change subject: scale: limit cpu usage using cpu-affinity
..


Patch Set 6:

(8 comments)

https://gerrit.ovirt.org/#/c/45738/6/lib/vdsm/cmdutils.py
File lib/vdsm/cmdutils.py:

Line 69: 
Line 70: def taskset(cmd, cpu_list, all_tasks=True):
Line 71: command = [constants.EXT_TASKSET,
Line 72:'--all-tasks' if all_tasks else '',
Line 73:'--cpu-list', ','.join(cpu_list)]
> While taskset order requirements are annoying, we can write in a cleaner wa
OK, I kinda like the Python's ternary operator, but I see your point and will 
update in the next upload.
Line 74: command.extend(cmd)
Line 75: return command
Line 76: 
Line 77: 


https://gerrit.ovirt.org/#/c/45738/6/lib/vdsm/taskset.py
File lib/vdsm/taskset.py:

Line 35: return "Process failed with rc=%d out=%r err=%r" % (
Line 36: self.rc, self.out, self.err)
Line 37: 
Line 38: 
Line 39: def get(pid, all_tasks=True):
> need to document the return value.
will add proper docstring.
Line 40: # TODO: check pid
Line 41: cmd = [constants.EXT_TASKSET,
Line 42:'--all-tasks' if all_tasks else '',
Line 43:'--pid', str(pid)]


Line 38: 
Line 39: def get(pid, all_tasks=True):
Line 40: # TODO: check pid
Line 41: cmd = [constants.EXT_TASKSET,
Line 42:'--all-tasks' if all_tasks else '',
> Can be cleaner, see comment in cmdutils.
will fix using the same approach
Line 43:'--pid', str(pid)]
Line 44: 
Line 45: rc, out, err = utils.execCmd(cmd, resetCpuAffinity=False)
Line 46: 


Line 46: 
Line 47: if rc != 0:
Line 48: raise Error(rc, out, err)
Line 49: 
Line 50: return _cpu_list_from_output(out[0])
> Are you sure that we always need the first line? Is it possible that we get
Taking the last line seem safer. However, I cannot anticipate possible 
failures, especially considering that we want to set the affinity exactly once 
(no updates), so I'll keep an close eye on this.
Line 51: 
Line 52: 
Line 53: def set(pid, cpu_list, all_tasks=True):
Line 54: # TODO: check pid


Line 49: 
Line 50: return _cpu_list_from_output(out[0])
Line 51: 
Line 52: 
Line 53: def set(pid, cpu_list, all_tasks=True):
> need to document that cpu_list is an iterable of strings, either cpu number
will add proper docstring.
Line 54: # TODO: check pid
Line 55: # warning: the order of options is significant.
Line 56: cmd = [constants.EXT_TASKSET,
Line 57:'--all-tasks' if all_tasks else '',


Line 53: def set(pid, cpu_list, all_tasks=True):
Line 54: # TODO: check pid
Line 55: # warning: the order of options is significant.
Line 56: cmd = [constants.EXT_TASKSET,
Line 57:'--all-tasks' if all_tasks else '',
> Can be cleaner, see comment in cmdutils.
will fix in the same way.
Line 58:'--pid',
Line 59:'--cpu-list', ','.join(cpu_list),
Line 60:str(pid)]
Line 61: 


Line 64: if rc != 0:
Line 65: raise Error(rc, out, err)
Line 66: 
Line 67: 
Line 68: def _cpu_list_from_output(line):
> Can you show here the typical output of the command, making it easier to ma
sure, will add in a proper docstring.
Line 69: cpu_list = line.split(': ', 1)
Line 70: mask = int(cpu_list[1], 16)


Line 65: raise Error(rc, out, err)
Line 66: 
Line 67: 
Line 68: def _cpu_list_from_output(line):
Line 69: cpu_list = line.split(': ', 1)
> I think we should use rsplit, in case the command format changes to somethi
good points. will fix as you suggest.
Line 70: mask = int(cpu_list[1], 16)


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I3f7f68d65eddb5a21afbc3809ea79cd1dee67984
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Polednik 
Gerrit-Reviewer: Martin Sivák 
Gerrit-Reviewer: Michal Skrivanek 
Gerrit-Reviewer: Michal Skrivanek 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: Vinzenz Feenstra 
Gerrit-Reviewer: Yaniv Bronhaim 
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: net: fix test_setupNetworks_on_external_bond

[danken]

Dan Kenigsberg has posted comments on this change.

Change subject: net: fix test_setupNetworks_on_external_bond
..


Patch Set 1: Code-Review+2

thanks! I did wonder.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia29761aa2ec920db4485bf704926b67e0d9851b9
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Petr Horáček 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Ido Barkan 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Ondřej Svoboda 
Gerrit-Reviewer: Petr Horáček 
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: vdscli: map invocation params to dictionary

[piotr . kliczewski]

Piotr Kliczewski has posted comments on this change.

Change subject: vdscli: map invocation params to dictionary
..


Patch Set 5: Verified+1

Renamed variable only. No additional tests performed. Copying verification flag 
from previous patch set.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ibac6eab3c519becb29d2b355d671bbb79df5
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Piotr Kliczewski 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Polednik 
Gerrit-Reviewer: Ondřej Svoboda 
Gerrit-Reviewer: Petr Horáček 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: Yeela Kaplan 
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: jsonrpc: executor based thread factory

[automation]

automat...@ovirt.org has posted comments on this change.

Change subject: jsonrpc: executor based thread factory
..


Patch Set 10:

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

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I56b307633a8bf7e4aad8f87cc97a4129c9ed0970
Gerrit-PatchSet: 10
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Piotr Kliczewski 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: Yaniv Bronhaim 
Gerrit-Reviewer: Yeela Kaplan 
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: scheduler: use single instance

[automation]

automat...@ovirt.org has posted comments on this change.

Change subject: scheduler: use single instance
..


Patch Set 8:

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

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

Gerrit-MessageType: comment
Gerrit-Change-Id: If83eded458f8304d802fcd75839e7a916146918b
Gerrit-PatchSet: 8
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Piotr Kliczewski 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: Yaniv Bronhaim 
Gerrit-Reviewer: Yeela Kaplan 
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: scheduler: use single instance

[fromani]

Francesco Romani has posted comments on this change.

Change subject: scheduler: use single instance
..


Patch Set 8: Code-Review+1

thanks, at first glance looks good now. Preliminary ACK, deeper review later.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: If83eded458f8304d802fcd75839e7a916146918b
Gerrit-PatchSet: 8
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Piotr Kliczewski 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: Yaniv Bronhaim 
Gerrit-Reviewer: Yeela Kaplan 
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: core: moving InquireNotSupportedError to storage_exception.py

[nsoffer]

Nir Soffer has posted comments on this change.

Change subject: core: moving InquireNotSupportedError to storage_exception.py
..


Patch Set 1:

(2 comments)

https://gerrit.ovirt.org/#/c/45763/1/vdsm/storage/hsm.py
File vdsm/storage/hsm.py:

Line 599: # This happens when we cannot read the MD LV
Line 600: self.log.error("Can't read LV based metadata", 
exc_info=True)
Line 601: raise se.StorageDomainMasterError("Can't read LV based 
metadata")
Line 602: except se.InquireNotSupportedError:
Line 603: self.log.error("Inquire spm status isn't supported by the 
used cluster lock", exc_info=True)
- line too long
- remove exc_info
- used->current
Line 604: raise
Line 605: except se.StorageException as e:
Line 606: self.log.error("MD read error: %s", str(e), exc_info=True)
Line 607: raise se.StorageDomainMasterError("MD read error")


https://gerrit.ovirt.org/#/c/45763/1/vdsm/storage/storage_exception.py
File vdsm/storage/storage_exception.py:

Line 1618: message = "Could not initialize cluster lock"
Line 1619: 
Line 1620: class InquireNotSupportedError(StorageException):
Line 1621: code = 702
Line 1622: message = "Cluster lock inquire isnt supported"
Cluster lock does not support inquire?
Line 1623: 
Line 1624: #
Line 1625: #  Meta data related Exceptions
Line 1626: #


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I8201794dc96ee24dc9c0da5b7c3d71ab0b75e9f3
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Liron Aravot 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: virt: vm with port mirroring stuck in down state

[automation]

automat...@ovirt.org has posted comments on this change.

Change subject: virt: vm with port mirroring  stuck in down state
..


Patch Set 4:

* Update tracker::#1254713::OK
* Check Bug-Url::OK
* Check Public Bug::#1254713::OK, public bug
* Check Product::#1254713::OK, Correct product Red Hat Enterprise 
Virtualization Manager
* Check TR::SKIP, not in a monitored branch (ovirt-3.5 ovirt-3.4 ovirt-3.3 
ovirt-3.2)
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 
'ovirt-3.4', 'ovirt-3.3'])

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia43369f584f696f1387685ec8f4fc4b6f58d98a6
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Marcin Mirecki 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Marcin Mirecki 
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: ssl: runtime config to choose implementation

[automation]

automat...@ovirt.org has posted comments on this change.

Change subject: ssl: runtime config to choose implementation
..


Patch Set 6:

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

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I9881d11e30ced9c34bfe602bba3d968f57e0fe15
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Piotr Kliczewski 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: Sandro Bonazzola 
Gerrit-Reviewer: Simone Tiraboschi 
Gerrit-Reviewer: Yaniv Bronhaim 
Gerrit-Reviewer: Yeela Kaplan 
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: ssl: configurable implementation

[automation]

automat...@ovirt.org has posted comments on this change.

Change subject: ssl: configurable implementation
..


Patch Set 8:

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

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I6501981bbd5276c49731b0d9eba4794286b0f823
Gerrit-PatchSet: 8
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Piotr Kliczewski 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: Sandro Bonazzola 
Gerrit-Reviewer: Simone Tiraboschi 
Gerrit-Reviewer: Yaniv Bronhaim 
Gerrit-Reviewer: Yeela Kaplan 
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: ssl: runtime config to choose implementation

[piotr . kliczewski]

Piotr Kliczewski has posted comments on this change.

Change subject: ssl: runtime config to choose implementation
..


Patch Set 6: Verified+1

Updated description in config.py, no code changes. Copying verification flag 
from previous patch set.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I9881d11e30ced9c34bfe602bba3d968f57e0fe15
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Piotr Kliczewski 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: Sandro Bonazzola 
Gerrit-Reviewer: Simone Tiraboschi 
Gerrit-Reviewer: Yaniv Bronhaim 
Gerrit-Reviewer: Yeela Kaplan 
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: sp: startSpm - ignore InquireNotSupportedError()

[nsoffer]

Nir Soffer has posted comments on this change.

Change subject: sp: startSpm - ignore InquireNotSupportedError()
..


Patch Set 1:

(1 comment)

https://gerrit.ovirt.org/#/c/45764/1/vdsm/storage/sp.py
File vdsm/storage/sp.py:

Line 252: try:
Line 253: oldlver, oldid = self._backend.getSpmStatus()
Line 254: except se.InquireNotSupportedError:
Line 255: self.log.info("cluster lock inquire isn't supported. "
Line 256:   "proceeding with startSpm()")
> Should probably be log.warn
This is an expected condition during upgrade, but vdsm does not have any 
context about the upgrade, so I don't think we need to warn here.
Line 257: else:
Line 258: if int(oldlver) != int(prevLVER) or int(oldid) != 
int(prevID):
Line 259: self.log.info("expected previd:%s lver:%s got 
request for "
Line 260:   "previd:%s lver:%s" %


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I082dc83ea410768db3819e7259089c20c2614b07
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Liron Aravot 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: hsm: Report vg name in getDeviceList

[automation]

automat...@ovirt.org has posted comments on this change.

Change subject: hsm: Report vg name in getDeviceList
..


Patch Set 1:

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

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I116714cb5143ea92f5cb54c3f80f895c07ada536
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer 
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: hsm: Report vg name in getDeviceList

[nsoffer]

Nir Soffer has uploaded a new change for review.

Change subject: hsm: Report vg name in getDeviceList
..

hsm: Report vg name in getDeviceList

Hosted engine needs the iscsi session info used by the hosted engine
storage domain. getDeviceList() seems to include the needed info, but it
does not report the vg name for each device, making it hard to match the
iscsi session info and the hosted engine storage domain.

We return now the vg name of each device, which seems to be useful info
regardless of hosted engine needs, and can be used on the engine side
for reconstructing host state or validating engine view vs host view.

Change-Id: I116714cb5143ea92f5cb54c3f80f895c07ada536
Signed-off-by: Nir Soffer 
---
M vdsm/rpc/vdsmapi-schema.json
M vdsm/storage/hsm.py
2 files changed, 11 insertions(+), 2 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/23/45823/1

diff --git a/vdsm/rpc/vdsmapi-schema.json b/vdsm/rpc/vdsmapi-schema.json
index e0e95b9..1b2a171 100644
--- a/vdsm/rpc/vdsmapi-schema.json
+++ b/vdsm/rpc/vdsmapi-schema.json
@@ -1473,6 +1473,10 @@
 #
 # @status: The device status (free/used/unknown)
 #
+# @vgname: The LVM volume group name, if this device is used as
+#  a physical volume. This is typically a storage domain
+#  UUID.
+#
 # Since: 4.10.0
 #
 # Notes:  The value of @serial may be dependent on the current host so this
@@ -1490,7 +1494,8 @@
   'pathstatus': ['BlockDevicePathInfo'],
   'pathlist': ['IscsiSessionInfo'], 'logicalblocksize': 'uint',
   'physicalblocksize': 'uint', 'partitioned': 'bool',
-  'pvsize': 'uint', 'status': 'BlockDeviceStatus'}}
+  'pvsize': 'uint', 'status': 'BlockDeviceStatus',
+  'vgname': 'str'}}
 
 ##
 # @Host.getDeviceList:
diff --git a/vdsm/storage/hsm.py b/vdsm/storage/hsm.py
index 1b8c064..32e16c3 100644
--- a/vdsm/storage/hsm.py
+++ b/vdsm/storage/hsm.py
@@ -2019,14 +2019,18 @@
 pvuuid = pv.uuid
 pvsize = pv.size
 vguuid = pv.vg_uuid
+vgname = pv.vg_name
 else:
 pvuuid = ""
 pvsize = ""
 vguuid = ""
+vgname = ""
 
 devInfo = {'GUID': dev.get("guid", ""), 'pvUUID': pvuuid,
'pvsize': str(pvsize),
-   'vgUUID': vguuid, 'vendorID': dev.get("vendor", ""),
+   'vgUUID': vguuid,
+   'vgname': vgname,
+   'vendorID': dev.get("vendor", ""),
'productID': dev.get("product", ""),
'fwrev': dev.get("fwrev", ""),
"serial": dev.get("serial", ""),


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I116714cb5143ea92f5cb54c3f80f895c07ada536
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer 
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: hsm: Report vg name in getDeviceList

[automation]

automat...@ovirt.org has posted comments on this change.

Change subject: hsm: Report vg name in getDeviceList
..


Patch Set 2:

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

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I116714cb5143ea92f5cb54c3f80f895c07ada536
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer 
Gerrit-Reviewer: Ala Hino 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Daniel Erez 
Gerrit-Reviewer: Freddy Rolland 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Roy Golan 
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: hsm: Report vg name in getDeviceList

[nsoffer]

Nir Soffer has posted comments on this change.

Change subject: hsm: Report vg name in getDeviceList
..


Patch Set 2: Verified+1

This version adds "new in version" to the schema.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I116714cb5143ea92f5cb54c3f80f895c07ada536
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer 
Gerrit-Reviewer: Ala Hino 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Daniel Erez 
Gerrit-Reviewer: Freddy Rolland 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Roy Golan 
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: scsi: Scan only the required domain type

[nsoffer]

Nir Soffer has uploaded a new change for review.

Change subject: scsi: Scan only the required domain type
..

scsi: Scan only the required domain type

We used to perform both iSCSI and FCP rescan when creating or editing a
storage domain, connecting to storage server, getting vg and storage
domain list and more.

The unneeded rescan is typically fast, but if a storage server or device
is not accessible, a SCSI rescan may block for couple of minutes,
leading to unwanted blocking of unrelated storage threads. This is
particularly bad when you are interested only in one domain type, but
the host get stuck scanning the other type.

To improve storage domain isolation, we use the specified storage type
to perform a rescan only of the relevant type. If storage type was not
specified, we scan both ISCSI and FCP keeping the old behavior.

Change-Id: Ic32cd683020e94df016dd77b19ae3eb7317c5554
Signed-off-by: Nir Soffer 
---
M vdsm/storage/hsm.py
M vdsm/storage/multipath.py
M vdsm/storage/sdc.py
3 files changed, 25 insertions(+), 17 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/24/45824/1

diff --git a/vdsm/storage/hsm.py b/vdsm/storage/hsm.py
index 1b8c064..541b699 100644
--- a/vdsm/storage/hsm.py
+++ b/vdsm/storage/hsm.py
@@ -1984,15 +1984,16 @@
 return dict(devList=devices)
 
 def _getDeviceList(self, storageType=None, guids=(), checkStatus=True):
-sdCache.refreshStorage()
-typeFilter = lambda dev: True
-if storageType:
-if sd.storageType(storageType) == sd.type2name(sd.ISCSI_DOMAIN):
-typeFilter = \
-lambda dev: multipath.devIsiSCSI(dev.get("devtype"))
-elif sd.storageType(storageType) == sd.type2name(sd.FCP_DOMAIN):
-typeFilter = \
-lambda dev: multipath.devIsFCP(dev.get("devtype"))
+domType = sd.storageType(storageType) if storageType else None
+
+sdCache.refreshStorage(domType)
+
+if domType == sd.ISCSI_DOMAIN:
+typeFilter = lambda dev: multipath.devIsiSCSI(dev.get("devtype"))
+elif domType == sd.FCP_DOMAIN:
+typeFilter = lambda dev: multipath.devIsFCP(dev.get("devtype"))
+else:
+typeFilter = lambda dev: True
 
 devices = []
 pvs = {}
@@ -2470,7 +2471,7 @@
 # while the VDSM was not connected, we need to
 # call refreshStorage.
 if domType in (sd.FCP_DOMAIN, sd.ISCSI_DOMAIN):
-sdCache.refreshStorage()
+sdCache.refreshStorage(domType)
 try:
 doms = self.__prefetchDomains(domType, conObj)
 except:
@@ -2864,7 +2865,8 @@
 """
 vars.task.setDefaultException(
 se.StorageDomainActionError("spUUID: %s" % spUUID))
-sdCache.refreshStorage()
+domType = sd.storageType(storageType) if storageType else None
+sdCache.refreshStorage(domType)
 if spUUID and spUUID != volume.BLANK_UUID:
 domList = self.getPool(spUUID).getDomains()
 domains = domList.keys()
@@ -2925,7 +2927,8 @@
 :rtype: dict
 """
 vars.task.setDefaultException(se.VolumeGroupActionError())
-sdCache.refreshStorage()
+domType = sd.storageType(storageType) if storageType else None
+sdCache.refreshStorage(domType)
 # getSharedLock(connectionsResource...)
 vglist = []
 vgs = self.__getVGsInfo()
diff --git a/vdsm/storage/multipath.py b/vdsm/storage/multipath.py
index ad81d2d..32deb98 100644
--- a/vdsm/storage/multipath.py
+++ b/vdsm/storage/multipath.py
@@ -39,6 +39,7 @@
 import misc
 import iscsi
 import devicemapper
+import sd
 
 DEV_ISCSI = "iSCSI"
 DEV_FCP = "FCP"
@@ -61,7 +62,7 @@
 """ multipath operation failed """
 
 
-def rescan():
+def rescan(domType=None):
 """
 Forces multipath daemon to rescan the list of available devices and
 refresh the mapping table. New devices can be found under /dev/mapper
@@ -70,8 +71,12 @@
 """
 
 # First rescan iSCSI and FCP connections
-iscsi.rescan()
-hba.rescan()
+
+if domType in (None, sd.ISCSI_DOMAIN):
+iscsi.rescan()
+
+if domType in (None, sd.FCP_DOMAIN):
+hba.rescan()
 
 # Now let multipath daemon pick up new devices
 misc.execCmd([constants.EXT_MULTIPATH], sudo=True)
diff --git a/vdsm/storage/sdc.py b/vdsm/storage/sdc.py
index ecb9708..273c5c0 100644
--- a/vdsm/storage/sdc.py
+++ b/vdsm/storage/sdc.py
@@ -77,10 +77,10 @@
 self.__staleStatus = self.STORAGE_STALE
 
 @misc.samplingmethod
-def refreshStorage(self):
+def refreshStorage(self, domType=None):
 self.__staleStatus = self.STORAGE_REFRESHING
 
-multipath.rescan()
+multipath.rescan(domType)
 multipath.resize_devices()

Change in vdsm[master]: scsi: Scan only the required domain type

[automation]

automat...@ovirt.org has posted comments on this change.

Change subject: scsi: Scan only the required domain type
..


Patch Set 1:

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

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ic32cd683020e94df016dd77b19ae3eb7317c5554
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer 
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: hsm: Report vg name in getDeviceList

[nsoffer]

Nir Soffer has posted comments on this change.

Change subject: hsm: Report vg name in getDeviceList
..


Patch Set 1: Verified+1

Verified that we get the vgname for devices which are part of a storage domain, 
and empty string for other devices.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I116714cb5143ea92f5cb54c3f80f895c07ada536
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer 
Gerrit-Reviewer: Ala Hino 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Daniel Erez 
Gerrit-Reviewer: Freddy Rolland 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Roy Golan 
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: hsm: Reformat device info dict

[automation]

automat...@ovirt.org has posted comments on this change.

Change subject: hsm: Reformat device info dict
..


Patch Set 1:

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

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I0907e3c5cf27959e0989903162cf16c9add0406d
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer 
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: hsm: Reformat device info dict

[nsoffer]

Nir Soffer has uploaded a new change for review.

Change subject: hsm: Reformat device info dict
..

hsm: Reformat device info dict

Reformat device info dict in getDeviceList using one item per line and
sorted. This make it easier to search and modify the code and creates
nicer diffs.

Change-Id: I0907e3c5cf27959e0989903162cf16c9add0406d
Signed-off-by: Nir Soffer 
---
M vdsm/storage/hsm.py
1 file changed, 16 insertions(+), 12 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/22/45822/1

diff --git a/vdsm/storage/hsm.py b/vdsm/storage/hsm.py
index 1b8c064..d3405aa 100644
--- a/vdsm/storage/hsm.py
+++ b/vdsm/storage/hsm.py
@@ -2024,18 +2024,22 @@
 pvsize = ""
 vguuid = ""
 
-devInfo = {'GUID': dev.get("guid", ""), 'pvUUID': pvuuid,
-   'pvsize': str(pvsize),
-   'vgUUID': vguuid, 'vendorID': dev.get("vendor", ""),
-   'productID': dev.get("product", ""),
-   'fwrev': dev.get("fwrev", ""),
-   "serial": dev.get("serial", ""),
-   'capacity': dev.get("capacity", "0"),
-   'devtype': dev.get("devtype", ""),
-   'pathstatus': dev.get("paths", []),
-   'pathlist': dev.get("connections", []),
-   'logicalblocksize': dev.get("logicalblocksize", ""),
-   'physicalblocksize': dev.get("physicalblocksize", "")}
+devInfo = {
+"serial": dev.get("serial", ""),
+'GUID': dev.get("guid", ""),
+'capacity': dev.get("capacity", "0"),
+'devtype': dev.get("devtype", ""),
+'fwrev': dev.get("fwrev", ""),
+'logicalblocksize': dev.get("logicalblocksize", ""),
+'pathlist': dev.get("connections", []),
+'pathstatus': dev.get("paths", []),
+'physicalblocksize': dev.get("physicalblocksize", ""),
+'productID': dev.get("product", ""),
+'pvUUID': pvuuid,
+'pvsize': str(pvsize),
+'vendorID': dev.get("vendor", ""),
+'vgUUID': vguuid,
+}
 if not checkStatus:
 devInfo["status"] = "unknown"
 devices.append(devInfo)


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I0907e3c5cf27959e0989903162cf16c9add0406d
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer 
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: sp: deactivateSd - remove domain from pending for upgrade list

[nsoffer]

Nir Soffer has posted comments on this change.

Change subject: sp: deactivateSd - remove domain from pending for upgrade list
..


Patch Set 1:

(2 comments)

https://gerrit.ovirt.org/#/c/45773/1/vdsm/storage/sp.py
File vdsm/storage/sp.py:

Line 1094
Line 1095
Line 1096
Line 1097
Line 1098
> We should add the code above in new _cancelDomainUpgrade(sduuid) method, so
When removing uuid from _domainsToUpgrade, it may become empty. This is handled 
in _upgradePoolDomain, and must be also handled in _cancelDomainUpgrade - see 
sp.py line 186


Line 1113: except mount.MountError:
Line 1114: self.log.error("Can't umount masterDir 
%s for domain "
Line 1115: "%s", masterDir, dom)
Line 1116: self._deactivateSD(domList, sdUUID)
Line 1117: self._domainsToUpgrade.remove(sdUUID)
You don't handle the case when self._domainsToUpgrade becomes empty.
Line 1118: 
Line 1119: def _deactivateSD(self, domList, sdUUID):
Line 1120: domList[sdUUID] = sd.DOM_ATTACHED_STATUS
Line 1121: self._backend.setDomainsMap(domList)


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I4451b348b8837dd83d95aea2be4a4536b33cdd99
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Liron Aravot 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: scale: limit cpu usage using cpu-affinity

[nsoffer]

Nir Soffer has posted comments on this change.

Change subject: scale: limit cpu usage using cpu-affinity
..


Patch Set 6:

(1 comment)

https://gerrit.ovirt.org/#/c/45738/6/lib/vdsm/cmdutils.py
File lib/vdsm/cmdutils.py:

Line 69: 
Line 70: def taskset(cmd, cpu_list, all_tasks=True):
Line 71: command = [constants.EXT_TASKSET,
Line 72:'--all-tasks' if all_tasks else '',
Line 73:'--cpu-list', ','.join(cpu_list)]
> OK, I kinda like the Python's ternary operator, but I see your point and wi
You are using it to set argv[1] of the taskset program to empty string, and 
this is not our intent, and it also does not work:

$ taskset "" -c 1 sleep 60
taskset: failed to set pid 0's affinity: Invalid argument

But there is a simple solution - we do not need --all-tasks in this context, 
since we are setting cpu affinity before a new program is executed. --all-tasks 
is needed only in taskset module.
Line 74: command.extend(cmd)
Line 75: return command
Line 76: 
Line 77: 


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I3f7f68d65eddb5a21afbc3809ea79cd1dee67984
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Polednik 
Gerrit-Reviewer: Martin Sivák 
Gerrit-Reviewer: Michal Skrivanek 
Gerrit-Reviewer: Michal Skrivanek 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: Vinzenz Feenstra 
Gerrit-Reviewer: Yaniv Bronhaim 
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: scale: limit cpu usage using cpu-affinity

[fromani]

Francesco Romani has posted comments on this change.

Change subject: scale: limit cpu usage using cpu-affinity
..


Patch Set 3:

(22 comments)

https://gerrit.ovirt.org/#/c/45738/3//COMMIT_MSG
Commit Message:

Line 3: AuthorDate: 2015-09-03 19:44:46 +0200
Line 4: Commit: Francesco Romani 
Line 5: CommitDate: 2015-09-04 17:06:01 +0200
Line 6: 
Line 7: lib: daemon: cpu affinity support using taskset
> This looks like a change in the daemon module of the vdsm library, which is
Better, thanks.
Line 8: 
Line 9: To improve reduce the impact of the GIL, we want to pin
Line 10: VDSM threads to few cores, maybe just one.
Line 11: 


Line 6: 
Line 7: lib: daemon: cpu affinity support using taskset
Line 8: 
Line 9: To improve reduce the impact of the GIL, we want to pin
Line 10: VDSM threads to few cores, maybe just one.
> I think you should quote the impressing results from the bug here.
Done
Line 11: 
Line 12: The user can configure using vdsm.conf the cpu cores on
Line 13: which she or he wants to let VDSM run on.
Line 14: If nothing is specified, VDSM behaves as before and uses


Line 9: To improve reduce the impact of the GIL, we want to pin
Line 10: VDSM threads to few cores, maybe just one.
Line 11: 
Line 12: The user can configure using vdsm.conf the cpu cores on
Line 13: which she or he wants to let VDSM run on.
> which vdsm should run on?
Seems simpler and more correct. Changed.
Line 14: If nothing is specified, VDSM behaves as before and uses
Line 15: any CPU core.
Line 16: 
Line 17: We need a patch which is simple to backport down to 3.5,


Line 14: If nothing is specified, VDSM behaves as before and uses
Line 15: any CPU core.
Line 16: 
Line 17: We need a patch which is simple to backport down to 3.5,
Line 18: so we use taskset just before the start of VDSM.
> ... to set vdsm cpu affinity, and we start any child process using taskset 
Right. Fixed.
Line 19: 
Line 20: taskset is part of util-linux, so no additional dependency
Line 21: is needed.
Line 22: 


https://gerrit.ovirt.org/#/c/45738/3/configure.ac
File configure.ac:

Line 312: AC_PATH_PROG([SU_PATH], [su], [/bin/su])
Line 313: AC_PATH_PROG([SYSCTL_PATH], [sysctl], [/sbin/sysctl])
Line 314: AC_PATH_PROG([SYSTEMD_RUN_PATH], [systemd-run], 
[/usr/bin/systemd-run])
Line 315: AC_PATH_PROG([TAR_PATH], [tar], [/bin/tar])
Line 316: AC_PATH_PROG([TASKSET_PATH], [taskset], [/usr/bin/taskset])
> Why not use CommandPath? can avoid changes both here and in constants.py.in
will switch to CommandPath when creating the new taskset module.
Line 317: AC_PATH_PROG([TC_PATH], [tc], [/sbin/tc])
Line 318: AC_PATH_PROG([TEE_PATH], [tee], [/usr/bin/tee])
Line 319: AC_PATH_PROG([TOUCH_PATH], [touch], [/bin/touch])
Line 320: AC_PATH_PROG([TUNE2FS_PATH], [tune2fs], [/sbin/tune2fs])


https://gerrit.ovirt.org/#/c/45738/3/lib/vdsm/cmdutils.py
File lib/vdsm/cmdutils.py:

Line 73: command = [constants.EXT_TASKSET, cpumask]
Line 74: else:
Line 75: command = [constants.EXT_TASKSET, '-p', cpumask, '%s' % pid]
Line 76: command.extend(cmd)
Line 77: return command
> We should use --cpu-list for better readability.
will add as option (default to yes) - because I'm not yet 100% about all the 
usecases.
Line 78: 
Line 79: 
Line 80: CPU_MIN = 0
Line 81: CPU_MAX = 63  # TODO: lift this constraint


Line 73: command = [constants.EXT_TASKSET, cpumask]
Line 74: else:
Line 75: command = [constants.EXT_TASKSET, '-p', cpumask, '%s' % pid]
Line 76: command.extend(cmd)
Line 77: return command
> Too complicated, does too much stuff that we don't want to do here.
OK for the tiny taskset module. On it.
Line 78: 
Line 79: 
Line 80: CPU_MIN = 0
Line 81: CPU_MAX = 63  # TODO: lift this constraint


Line 92: if index < CPU_MIN or index > CPU_MAX:
Line 93: raise ValueError('cpu index %i outside limits [%i, %i]',
Line 94:  index, CPU_MIN, CPU_MAX)
Line 95: mask |= (1 << index)
Line 96: return mask
> All this is not needed if we use --cpu-list option (avilable in el6)
it is needed the other way around, when we use taskset to get the affinity mask 
of a process. Example:

  8 09:25:42 fromani@musashi ~/Projects/upstream/vdsm $ taskset -c -p 8537
pid 8537's current affinity list: 0-3

As we can see, the tool uses the range syntax, we want to expand it for 
consistency.
Line 97: 
Line 98: # This function returns truthy value if its argument contains unsafe 
characters
Line 99: # for including in a command passed to the shell. The safe characters 
were
Line 100: # stolen from pipes._safechars.


https://gerrit.ovirt.org/#/c/45738/3/lib/vdsm/config.py.in
File lib/vdsm/config.py.in:

Line 207: 
Line 208: ('connection_stats_timeout', '3600',
Line 209: 'Time in seconds defining how frequently we log transport 
stats'),
Line 210: 
Line 211: ('cpu_allowed', '',
> cpu_affinity?

Change in vdsm[ovirt-3.5]: net: fix vdsmd service start

[phoracek]

Petr Horáček has uploaded a new change for review.

Change subject: net: fix vdsmd service start
..

net: fix vdsmd service start

Change-Id: I71342362f56b87bcb566c3343c90a69f95327ea6
Bug-Url: https://bugzilla.redhat.com/1258551
Signed-off-by: Petr Horáček 
---
M init/sysvinit/vdsmd.init.in
1 file changed, 1 insertion(+), 1 deletion(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/87/45787/1

diff --git a/init/sysvinit/vdsmd.init.in b/init/sysvinit/vdsmd.init.in
index 9777306..2eefa7c 100755
--- a/init/sysvinit/vdsmd.init.in
+++ b/init/sysvinit/vdsmd.init.in
@@ -114,7 +114,7 @@
 
 for static_dev in $static_devs; do
 if [[ $activated_devs != *"$static_dev"* ]]; then
-service network start
+service network restart
 ret_val=$?
 if [ "${ret_val}" -ne 0 ]; then
 log_failure_msg "${prog}: Start dependent network"


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I71342362f56b87bcb566c3343c90a69f95327ea6
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-3.5
Gerrit-Owner: Petr Horáček 
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[ovirt-3.5]: net: fix vdsmd service start

[automation]

automat...@ovirt.org has posted comments on this change.

Change subject: net: fix vdsmd service start
..


Patch Set 1:

* Update tracker::#1258551::OK
* Check Bug-Url::OK
* Check Public Bug::#1258551::OK, public bug
* Check Product::#1258551::OK, Correct product Red Hat Enterprise 
Virtualization Manager
* Check TR::#1258551::OK, correct target release 3.5.5
* warn_if_not_merged_to_previous_branch: OK

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I71342362f56b87bcb566c3343c90a69f95327ea6
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-3.5
Gerrit-Owner: Petr Horáček 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches