Change in vdsm[master]: tests: prevent hook validation decorator from leaving script...

2013-11-03 Thread oVirt Jenkins CI Server
oVirt Jenkins CI Server has posted comments on this change.

Change subject: tests: prevent hook validation decorator from leaving scripts 
installed
..


Patch Set 6:

Build Successful 

http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/4401/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/5205/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/5281/ : SUCCESS

-- 
To view, visit http://gerrit.ovirt.org/20310
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I138c3eab77ea6d35d6a997049940923026e06d96
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Miguel Angel Ajo Pelayo 
Gerrit-Reviewer: Antoni Segura Puimedon 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Miguel Angel Ajo Pelayo 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: lvm: Prevent auto-actviation of logical volumes

2013-11-03 Thread ewarszaw
Eduardo has posted comments on this change.

Change subject: lvm: Prevent auto-actviation of logical volumes
..


Patch Set 3: Code-Review-1

(5 comments)


Commit Message
Line 8: 
Line 9: When using FC storage, physical volumes are connected during boot, and
Line 10: vdsm logical volumes are auto-activated by /etc/rc.sysinit and/or
Line 11: /etc/init.d/netfs.  This is abnormal situation that vdsm cannot handle,
Line 12: and leads to data corruption.
""" This is abnormal situation that vdsm cannot handle, 11
and leads to data corruption."""
May lead. Please remove this sentence.
Line 13: 
Line 14: This patch prevents auto-activation of vdsm volumes using new
Line 15: --setactivationskip and --ignoreactivationskip options, introcuded in
Line 16: lvm 2.02.100.


Line 16: lvm 2.02.100.
Line 17: 
Line 18: To make this change easy, lvm.changelv() was modified to accept 
variable
Line 19: argument tuples, instead of multiple non-related types. This also
Line 20: simplify the only caller that use multiple arguments.
Please remove.
Line 21: 
Line 22: Change-Id: Ie001c5a4c888bb1ca44bedaeeae6fb75e7cbacc0
Line 23: Bug-Url: https://bugzilla.redhat.com/1009812



File vdsm/storage/lvm.py
Line 748: raise se.VolumeGroupActionError(
Line 749: "vgchange on vg(s) %s failed. %d %s %s" % (vgs, rc, 
out, err))
Line 750: 
Line 751: 
Line 752: def changelv(vg, lvs, *attrs):
There is no reason for changing the interface of this function.
Line 753: """
Line 754: Change multiple attributes on multiple LVs.
Line 755: 
Line 756: vg: VG name


Line 769: cmd = ["lvchange"]
Line 770: cmd.extend(LVM_NOBACKUP)
Line 771: for attr, value in attrs:
Line 772: cmd.extend((attr, value))
Line 773: if attr in ("-a", "--available", "--activate") and value == 
"y":
The if is redundant as said before.
There is no need for more logic when it is not necessary. Let lvm do is best.
 And for log's sake will be better (TM) if --ignoreactivationskip is added in 
the invariant part of the command.
Line 774: cmd.append("--ignoreactivationskip")
Line 775: cmd.extend(lvnames)
Line 776: rc, out, err = _lvminfo.cmd(tuple(cmd), _lvminfo._getVGDevs((vg, 
)))
Line 777: _lvminfo._invalidatelvs(vg, lvs)


Line 1019: cont = {True: "y", False: "n"}[contiguous]
Line 1020: cmd = ["lvcreate"]
Line 1021: cmd.extend(LVM_NOBACKUP)
Line 1022: cmd.extend(("--contiguous", cont, "--size", "%sm" % size,
Line 1023: "--setactivationskip", "y", 
"--ignoreactivationskip"))
In the invariant part, as before.
Line 1024: if initialTag is not None:
Line 1025: cmd.extend(("--addtag", initialTag))
Line 1026: cmd.extend(("--name", lvName, vgName))
Line 1027: rc, out, err = _lvminfo.cmd(cmd, _lvminfo._getVGDevs((vgName, )))


-- 
To view, visit http://gerrit.ovirt.org/20832
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie001c5a4c888bb1ca44bedaeeae6fb75e7cbacc0
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Ayal Baron 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Eduardo 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Sergey Gotliv 
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: lvm: Prevent auto-actviation of logical volumes

2013-11-03 Thread sgotliv
Sergey Gotliv has posted comments on this change.

Change subject: lvm: Prevent auto-actviation of logical volumes
..


Patch Set 3:

Similar to http://gerrit.ovirt.org/#/c/20836/.

Eduardo mentioned that in one of his comments.

-- 
To view, visit http://gerrit.ovirt.org/20832
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie001c5a4c888bb1ca44bedaeeae6fb75e7cbacc0
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Ayal Baron 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Eduardo 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Sergey Gotliv 
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: lvm: Prevent auto-activation of logical volumes.

2013-11-03 Thread sgotliv
Sergey Gotliv has posted comments on this change.

Change subject: lvm: Prevent auto-activation of logical volumes.
..


Patch Set 2:

You and Nir are working on the same issue:

http://gerrit.ovirt.org/#/c/20832/

-- 
To view, visit http://gerrit.ovirt.org/20836
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Iab9b7579990d934c60999b4c603c3acf46557be1
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Eduardo 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Ayal Baron 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Eduardo 
Gerrit-Reviewer: Elad Ben Aharon 
Gerrit-Reviewer: Gadi Ickowicz 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Sergey Gotliv 
Gerrit-Reviewer: Yeela Kaplan 
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: Remove redundant supervsdm.validateAccess.

2013-11-03 Thread oVirt Jenkins CI Server
oVirt Jenkins CI Server has posted comments on this change.

Change subject: Remove redundant supervsdm.validateAccess.
..


Patch Set 1:

Build Successful 

http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/4400/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/5204/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/5280/ : SUCCESS

-- 
To view, visit http://gerrit.ovirt.org/20838
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Id37865225ea2d29361d6e588f9bb0acf7bf71cec
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Eduardo 
Gerrit-Reviewer: Ayal Baron 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Saggi Mizrahi 
Gerrit-Reviewer: Yaniv Bronhaim 
Gerrit-Reviewer: Yeela Kaplan 
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: Remove redundant supervsdm.validateAccess.

2013-11-03 Thread ewarszaw
Eduardo has uploaded a new change for review.

Change subject: Remove redundant supervsdm.validateAccess.
..

Remove redundant supervsdm.validateAccess.

Change-Id: Id37865225ea2d29361d6e588f9bb0acf7bf71cec
Signed-off-by: Eduardo 
---
M vdsm/storage/fileSD.py
M vdsm/supervdsmServer
2 files changed, 0 insertions(+), 11 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/38/20838/1

diff --git a/vdsm/storage/fileSD.py b/vdsm/storage/fileSD.py
index f9ff93b..47c5448 100644
--- a/vdsm/storage/fileSD.py
+++ b/vdsm/storage/fileSD.py
@@ -35,7 +35,6 @@
 from persistentDict import PersistentDict, DictValidator
 from vdsm import constants
 from vdsm.utils import stripNewLines
-import supervdsm
 import mount
 
 REMOTE_PATH = "REMOTE_PATH"
@@ -50,10 +49,6 @@
 def validateDirAccess(dirPath):
 try:
 getProcPool().fileUtils.validateAccess(dirPath)
-supervdsm.getProxy().validateAccess(
-constants.QEMU_PROCESS_USER,
-(constants.DISKIMAGE_GROUP, constants.METADATA_GROUP), dirPath,
-(os.R_OK | os.X_OK))
 except OSError as e:
 if e.errno == errno.EACCES:
 raise se.StorageServerAccessPermissionError(dirPath)
diff --git a/vdsm/supervdsmServer b/vdsm/supervdsmServer
index d42e320..7a9c3da 100755
--- a/vdsm/supervdsmServer
+++ b/vdsm/supervdsmServer
@@ -57,7 +57,6 @@
 from storage.iscsi import readSessionInfo as _readSessionInfo
 from supervdsm import _SuperVdsmManager
 from storage.fileUtils import chown, resolveGid, resolveUid
-from storage.fileUtils import validateAccess as _validateAccess
 from vdsm.constants import METADATA_GROUP, EXT_UDEVADM, \
 DISKIMAGE_USER, DISKIMAGE_GROUP, P_LIBVIRT_VMCHANNELS, VDSM_USER
 from storage.devicemapper import _removeMapping, _getPathsStatus
@@ -237,11 +236,6 @@
 raise err
 
 return res
-
-@logDecorator
-def validateAccess(self, user, groups, *args, **kwargs):
-return self._runAs(user, groups, _validateAccess, args=args,
-   kwargs=kwargs)
 
 @logDecorator
 def setSafeNetworkConfig(self):


-- 
To view, visit http://gerrit.ovirt.org/20838
To unsubscribe, visit http://gerrit.ovirt.org/settings

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


Change in vdsm[master]: lvm: Prevent auto-activation of logical volumes.

2013-11-03 Thread oVirt Jenkins CI Server
oVirt Jenkins CI Server has posted comments on this change.

Change subject: lvm: Prevent auto-activation of logical volumes.
..


Patch Set 2:

Build Successful 

http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/4399/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/5203/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/5279/ : SUCCESS

-- 
To view, visit http://gerrit.ovirt.org/20836
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Iab9b7579990d934c60999b4c603c3acf46557be1
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Eduardo 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Ayal Baron 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Eduardo 
Gerrit-Reviewer: Elad Ben Aharon 
Gerrit-Reviewer: Gadi Ickowicz 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Yeela Kaplan 
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: lvm: Prevent auto-activation of logical volumes.

2013-11-03 Thread ewarszaw
Eduardo has posted comments on this change.

Change subject: lvm: Prevent auto-activation of logical volumes.
..


Patch Set 1:

(1 comment)


Commit Message
Line 6: 
Line 7: lvm: Prevent auto-activation of logical volumes.
Line 8: 
Line 9: When using FC storage, physical volumes are connected during boot,
Line 10: and vdsm logical volumes are auto-activated by /etc/rc.sysinit
I'm agree with you that this is not my preferred API.
And I think that the opposite logic will be more convinient for us,
but this is the fine grain option that lvm provides by now.
Line 11: and/or /etc/init.d/netfs.
Line 12: 
Line 13: This patch prevents auto-activation of vdsm volumes using new
Line 14: --setactivationskip and --ignoreactivationskip options, introduced


-- 
To view, visit http://gerrit.ovirt.org/20836
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Iab9b7579990d934c60999b4c603c3acf46557be1
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Eduardo 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Ayal Baron 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Eduardo 
Gerrit-Reviewer: Elad Ben Aharon 
Gerrit-Reviewer: Gadi Ickowicz 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Yeela Kaplan 
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: vdsm: move watchdog default params to WatchdogDevice

2013-11-03 Thread danken
Dan Kenigsberg has submitted this change and it was merged.

Change subject: vdsm: move watchdog default params to WatchdogDevice
..


vdsm: move watchdog default params to WatchdogDevice

Watchdog device creation is currently handled in
buildConfDevices, creating unnecessary code pollution.
This patch moves the creation to WatchdogDevice class,
cleaning up buildConfDevices code. Side effect is that
we lose access to specParams if they are not passed from
engine.

Change-Id: Ic76f040bf78cbec3129569eb30fbf14447f742d6
Signed-off-by: Martin Polednik 
Reviewed-on: http://gerrit.ovirt.org/19331
Reviewed-by: Dan Kenigsberg 
---
M vdsm/vm.py
1 file changed, 8 insertions(+), 10 deletions(-)

Approvals:
  Dan Kenigsberg: Looks good to me, approved
  Martin Polednik: Verified



-- 
To view, visit http://gerrit.ovirt.org/19331
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: Ic76f040bf78cbec3129569eb30fbf14447f742d6
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Martin Polednik 
Gerrit-Reviewer: Ayal Baron 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: Martin Polednik 
Gerrit-Reviewer: Michal Skrivanek 
Gerrit-Reviewer: Peter V. Saveliev 
Gerrit-Reviewer: Vinzenz Feenstra 
Gerrit-Reviewer: oVirt Jenkins CI Server
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: vdsm: move watchdog default params to WatchdogDevice

2013-11-03 Thread danken
Dan Kenigsberg has posted comments on this change.

Change subject: vdsm: move watchdog default params to WatchdogDevice
..


Patch Set 2: Code-Review+2

(1 comment)


File vdsm/vm.py
Line 1631: 
Line 1632: 
Line 1633: class WatchdogDevice(VmDevice):
Line 1634: def __init__(self, *args, **kwargs):
Line 1635: super(WatchdogDevice, self).__init__(*args, **kwargs)
Ayal is right. VmDevice's conversion of a dict into object attribute is a piece 
of bad design. I wish we never had it that way.

However, this patch is not about big refactoring of VmDevice, but about 
splitting a huge buildConfDevices functioninto smaller pieces. In this task, I 
think this patch succeeds.
Line 1636: 
Line 1637: if not hasattr(self, 'specParams'):
Line 1638: self.specParams = {}
Line 1639: 


-- 
To view, visit http://gerrit.ovirt.org/19331
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ic76f040bf78cbec3129569eb30fbf14447f742d6
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Martin Polednik 
Gerrit-Reviewer: Ayal Baron 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: Martin Polednik 
Gerrit-Reviewer: Michal Skrivanek 
Gerrit-Reviewer: Peter V. Saveliev 
Gerrit-Reviewer: Vinzenz Feenstra 
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: lvm: Prevent auto-activation of logical volumes.

2013-11-03 Thread danken
Dan Kenigsberg has posted comments on this change.

Change subject: lvm: Prevent auto-activation of logical volumes.
..


Patch Set 1: Code-Review-1

(2 comments)


Commit Message
Line 6: 
Line 7: lvm: Prevent auto-activation of logical volumes.
Line 8: 
Line 9: When using FC storage, physical volumes are connected during boot,
Line 10: and vdsm logical volumes are auto-activated by /etc/rc.sysinit
which code exactly does the auto-activation? wouldn't it be simpler to ask 
sysv/systemd to be more selective? the new lvm logic of adding one flag to skip 
and another flag to ignore the first flag sounds crazy, begging for a third 
flag controlling whether the first flag has precedence over the second one. It 
reminds me too much of our

 llPrepare(self, rw=False, setrw=False).
Line 11: and/or /etc/init.d/netfs.
Line 12: 
Line 13: This patch prevents auto-activation of vdsm volumes using new
Line 14: --setactivationskip and --ignoreactivationskip options, introduced


Line 11: and/or /etc/init.d/netfs.
Line 12: 
Line 13: This patch prevents auto-activation of vdsm volumes using new
Line 14: --setactivationskip and --ignoreactivationskip options, introduced
Line 15: in lvm 2.02.100.
please put this explicitly into vdsm.spec.
Line 16: 
Line 17: Note: The change of existing volumes should be done with a little app
Line 18: running when installing or upgrading vdsm. In addition it should be
Line 19: executable from command line. Should not be part of the vdsm code.


-- 
To view, visit http://gerrit.ovirt.org/20836
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Iab9b7579990d934c60999b4c603c3acf46557be1
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Eduardo 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Ayal Baron 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Elad Ben Aharon 
Gerrit-Reviewer: Gadi Ickowicz 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Yeela Kaplan 
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: sampling.ImagePathStatus: drop unused code

2013-11-03 Thread oVirt Jenkins CI Server
oVirt Jenkins CI Server has posted comments on this change.

Change subject: sampling.ImagePathStatus: drop unused code
..


Patch Set 4:

Build Successful 

http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/4398/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/5202/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/5278/ : SUCCESS

-- 
To view, visit http://gerrit.ovirt.org/20812
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I682d9730ac087e0a24ba5c72dbac912c87a6452f
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dan Kenigsberg 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Petr Šebek 
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: cleanup: Improve lib/vdsm imports (PEP328)

2013-11-03 Thread danken
Dan Kenigsberg has posted comments on this change.

Change subject: cleanup: Improve lib/vdsm imports (PEP328)
..


Patch Set 4:

Saggi, why no prepare now for the (nearing) __future__?

-- 
To view, visit http://gerrit.ovirt.org/20555
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I249cfa0ad734ea45ecbbecbade9daeed6c3adc12
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Antoni Segura Puimedon 
Gerrit-Reviewer: Antoni Segura Puimedon 
Gerrit-Reviewer: Assaf Muller 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: Saggi Mizrahi 
Gerrit-Reviewer: Yaniv Bronhaim 
Gerrit-Reviewer: mooli tayer 
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: cleanup: Improve lib/vdsm imports (PEP328)

2013-11-03 Thread nsoffer
Nir Soffer has posted comments on this change.

Change subject: cleanup: Improve lib/vdsm imports (PEP328)
..


Patch Set 1:

(2 comments)


File vdsm/configNetwork.py
Line 32: from .neterrors import ConfigNetworkError
Line 33: from .netmodels import Bond, Bridge, IPv4, IPv6, IpConfig, Nic, Vlan
Line 34: from vdsm.config import config
Line 35: from vdsm import constants, netinfo, utils
Line 36: from vdsm.utils import execCmd
So this is actually a fix - the code was using the wrong execCmd?

If so I would separate this to another patch - it has nothing to to with import 
cleanup.
Line 37: 
Line 38: CONNECTIVITY_TIMEOUT_DEFAULT = 4
Line 39: 
Line 40: 



File vdsm/tc.py
Line 26: 
Line 27: import ethtool
Line 28: 
Line 29: from vdsm.constants import EXT_TC
Line 30: from vdsm.utils import execCmd
See my reply in the other file.
Line 31: 
Line 32: ERR_DEV_NOEXIST = 2
Line 33: 
Line 34: QDISC_INGRESS = ':'


-- 
To view, visit http://gerrit.ovirt.org/20555
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I249cfa0ad734ea45ecbbecbade9daeed6c3adc12
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Antoni Segura Puimedon 
Gerrit-Reviewer: Antoni Segura Puimedon 
Gerrit-Reviewer: Assaf Muller 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: Saggi Mizrahi 
Gerrit-Reviewer: Yaniv Bronhaim 
Gerrit-Reviewer: mooli tayer 
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: sampling.ImagePathStatus: drop dead thread

2013-11-03 Thread nsoffer
Nir Soffer has posted comments on this change.

Change subject: sampling.ImagePathStatus: drop dead thread
..


Patch Set 3:

(1 comment)

Minor title edit.


Commit Message
Line 3: AuthorDate: 2013-11-02 21:37:33 +
Line 4: Commit: Dan Kenigsberg 
Line 5: CommitDate: 2013-11-02 22:47:39 +
Line 6: 
Line 7: sampling.ImagePathStatus: drop dead thread
Maybe:

sampling: Drop unused ImagePathStatus thread
Line 8: 
Line 9: In pre-historic rhev-2.1 days, ImagePathStatus thread was used to
Line 10: collect and cache information about the image repository accessibility.
Line 11: However, this code was never used by ovirt-3.y.


-- 
To view, visit http://gerrit.ovirt.org/20812
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I682d9730ac087e0a24ba5c72dbac912c87a6452f
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dan Kenigsberg 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Petr Šebek 
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: sampling.ImagePathStatus: drop dead thread

2013-11-03 Thread nsoffer
Nir Soffer has posted comments on this change.

Change subject: sampling.ImagePathStatus: drop dead thread
..


Patch Set 3:

(1 comment)

Looks good expect unused variable.


File vdsm/sampling.py
Line 363: """
Line 364: AVERAGING_WINDOW = 5
Line 365: SAMPLE_INTERVAL_SEC = 2
Line 366: 
Line 367: def __init__(self, cif, log):
Isn't cif unused now? I would remove it as well.
Line 368: self.startTime = time.time()
Line 369: 
Line 370: threading.Thread.__init__(self)
Line 371: self._log = log


-- 
To view, visit http://gerrit.ovirt.org/20812
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I682d9730ac087e0a24ba5c72dbac912c87a6452f
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dan Kenigsberg 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Petr Šebek 
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: lvm: Prevent auto-actviation of logical volumes

2013-11-03 Thread oVirt Jenkins CI Server
oVirt Jenkins CI Server has posted comments on this change.

Change subject: lvm: Prevent auto-actviation of logical volumes
..


Patch Set 3:

Build Successful 

http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/4397/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/5201/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/5277/ : SUCCESS

-- 
To view, visit http://gerrit.ovirt.org/20832
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie001c5a4c888bb1ca44bedaeeae6fb75e7cbacc0
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Ayal Baron 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Eduardo 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: lvm: Prevent auto-actviation of logical volumes

2013-11-03 Thread nsoffer
Nir Soffer has posted comments on this change.

Change subject: lvm: Prevent auto-actviation of logical volumes
..


Patch Set 3:

Fix typo and lvm2 dependency.

-- 
To view, visit http://gerrit.ovirt.org/20832
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie001c5a4c888bb1ca44bedaeeae6fb75e7cbacc0
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Ayal Baron 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Eduardo 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: lvm: Prevent auto-actviation of logical volumes

2013-11-03 Thread nsoffer
Nir Soffer has posted comments on this change.

Change subject: lvm: Prevent auto-actviation of logical volumes
..


Patch Set 2:

Address Eduardo comments. Remove cleanups that are not required for this fix.

-- 
To view, visit http://gerrit.ovirt.org/20832
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie001c5a4c888bb1ca44bedaeeae6fb75e7cbacc0
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Ayal Baron 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Eduardo 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: lvm: Prevent auto-actviation of logical volumes

2013-11-03 Thread oVirt Jenkins CI Server
oVirt Jenkins CI Server has posted comments on this change.

Change subject: lvm: Prevent auto-actviation of logical volumes
..


Patch Set 2: Verified-1

Build Failed 

http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/4396/ : FAILURE

http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/5200/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/5276/ : FAILURE

-- 
To view, visit http://gerrit.ovirt.org/20832
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie001c5a4c888bb1ca44bedaeeae6fb75e7cbacc0
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Ayal Baron 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Eduardo 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: lvm: Prevent auto-actviation of logical volumes

2013-11-03 Thread nsoffer
Nir Soffer has posted comments on this change.

Change subject: lvm: Prevent auto-actviation of logical volumes
..


Patch Set 1:

(6 comments)


Commit Message
Line 16: lvm 2.02.100.
Line 17: 
Line 18: To make this change easy, lvm.changelv() was modified to accept 
variable
Line 19: argument tuples, instead of multiple non-related types. This also
Line 20: simplify the only caller that use multiple arguments.
It is required to add the --ignoreactivationskip only when needed. How does it 
give less functionality?
Line 21: 
Line 22: Note: upgrade of existing volumes is not implemented yet.
Line 23: 
Line 24: Change-Id: Ie001c5a4c888bb1ca44bedaeeae6fb75e7cbacc0


Line 18: To make this change easy, lvm.changelv() was modified to accept 
variable
Line 19: argument tuples, instead of multiple non-related types. This also
Line 20: simplify the only caller that use multiple arguments.
Line 21: 
Line 22: Note: upgrade of existing volumes is not implemented yet.
I also think so - the only problem is only the spm can invoke this upgrade. How 
do you suggest to trigger this tool from the spm?
Line 23: 
Line 24: Change-Id: Ie001c5a4c888bb1ca44bedaeeae6fb75e7cbacc0
Line 25: Bug-Url: https://bugzilla.redhat.com/1009812



File vdsm/storage/blockSD.py
Line 990: operation and is resilient to open LVs, etc.
Line 991: """
Line 992: prefix = blockVolume.TAG_PREFIX_IMAGE
Line 993: try:
Line 994: lvm.changelv(sdUUID, volUUIDs, ("--available", "y"),
This is indeed not required, as the underlying call does support both short and 
long options, but it is more readable to use only long options in this case.

How this change remove any functionality?
Line 995:  ("--deltag", prefix + imgUUID),
Line 996:  ("--addtag", prefix + opTag + imgUUID))
Line 997: except se.StorageException as e:
Line 998: log.error("Can't activate or change LV tags in SD %s. "



File vdsm/storage/lvm.py
Line 755: 
Line 756: vg: VG name
Line 757: lvs: a single LV name or iterable of LV names.
Line 758: attrs: lvchange argument tupples. Use None value for unary 
options.
Line 759:e.g. ('--attr', 'value'), ('--unary', None)
There are options that we do use, like --refresh. Adding this option allow 
fixing code calling lvchange directly to use changelv, removing duplicate code.

I agree that calling with None is not pretty, but this was the smallest change 
needed to easily detect a call that does activation, and include the required 
activation flag.
Line 760: 
Line 761: Note:
Line 762: You may activate an activated LV without error
Line 763: but lvchange returns an error (RC=5) when activating rw if 
already rw


Line 772: for attr, value in attrs:
Line 773: cmd.append(attr)
Line 774: if value is not None:
Line 775: cmd.append(value)
Line 776: if attr in ("-a", "--available", "--activate") and value == 
"y":
Why do you want to add unneeded command line option when it does not have any 
function?
Line 777: cmd.append('--ignoreactivationskip')
Line 778: cmd.extend(lvnames)
Line 779: rc, out, err = _lvminfo.cmd(tuple(cmd), _lvminfo._getVGDevs((vg, 
)))
Line 780: _lvminfo._invalidatelvs(vg, lvs)


Line 1022: cont = {True: "y", False: "n"}[contiguous]
Line 1023: cmd = ["lvcreate"]
Line 1024: cmd.extend(LVM_NOBACKUP)
Line 1025: cmd.extend(("--contiguous", cont, "--size", "%sm" % size,
Line 1026: "--setactivationskip", "y"))
I'll add --ignoreactivationskip.
Line 1027: if initialTag is not None:
Line 1028: cmd.extend(("--addtag", initialTag))
Line 1029: cmd.extend(("--name", lvName, vgName))
Line 1030: rc, out, err = _lvminfo.cmd(cmd, _lvminfo._getVGDevs((vgName, )))


-- 
To view, visit http://gerrit.ovirt.org/20832
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie001c5a4c888bb1ca44bedaeeae6fb75e7cbacc0
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Ayal Baron 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Eduardo 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: upgrade: fix v3ResetMetaVolSize argument

2013-11-03 Thread sgotliv
Sergey Gotliv has posted comments on this change.

Change subject: upgrade: fix v3ResetMetaVolSize argument
..


Patch Set 2: Code-Review+1

-- 
To view, visit http://gerrit.ovirt.org/20721
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Iddffa996652300e5a5daa19bef5bd7873f39bb15
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Federico Simoncelli 
Gerrit-Reviewer: Ayal Baron 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Eduardo 
Gerrit-Reviewer: Sergey Gotliv 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: lvm: Prevent auto-actviation of logical volumes

2013-11-03 Thread ewarszaw
Eduardo has posted comments on this change.

Change subject: lvm: Prevent auto-actviation of logical volumes
..


Patch Set 1:

Please look at:
Change-Id: Iab9b7579990d934c60999b4c603c3acf46557be1

-- 
To view, visit http://gerrit.ovirt.org/20832
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie001c5a4c888bb1ca44bedaeeae6fb75e7cbacc0
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Ayal Baron 
Gerrit-Reviewer: Eduardo 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: lvm: Prevent auto-activation of logical volumes.

2013-11-03 Thread oVirt Jenkins CI Server
oVirt Jenkins CI Server has posted comments on this change.

Change subject: lvm: Prevent auto-activation of logical volumes.
..


Patch Set 1:

Build Successful 

http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/4395/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/5199/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/5275/ : SUCCESS

-- 
To view, visit http://gerrit.ovirt.org/20836
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Iab9b7579990d934c60999b4c603c3acf46557be1
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Eduardo 
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: lvm: Prevent auto-activation of logical volumes.

2013-11-03 Thread ewarszaw
Eduardo has uploaded a new change for review.

Change subject: lvm: Prevent auto-activation of logical volumes.
..

lvm: Prevent auto-activation of logical volumes.

When using FC storage, physical volumes are connected during boot,
and vdsm logical volumes are auto-activated by /etc/rc.sysinit
and/or /etc/init.d/netfs.

This patch prevents auto-activation of vdsm volumes using new
--setactivationskip and --ignoreactivationskip options, introduced
in lvm 2.02.100.

Note: The change of existing volumes should be done with a little app
running when installing or upgrading vdsm. In addition it should be
executable from command line. Should not be part of the vdsm code.

Change-Id: Iab9b7579990d934c60999b4c603c3acf46557be1
Bug-Url: https://bugzilla.redhat.com/1009812
Signed-off-by: Eduardo 
---
M vdsm/storage/lvm.py
1 file changed, 3 insertions(+), 1 deletion(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/36/20836/1

diff --git a/vdsm/storage/lvm.py b/vdsm/storage/lvm.py
index 0c2964e..2561ae4 100644
--- a/vdsm/storage/lvm.py
+++ b/vdsm/storage/lvm.py
@@ -769,6 +769,7 @@
 lvnames = tuple("%s/%s" % (vg, lv) for lv in lvs)
 cmd = ["lvchange"]
 cmd.extend(LVM_NOBACKUP)
+cmd.append('--ignoreactivationskip')
 if isinstance(attrs[0], str):
 # ("--attribute", "value")
 cmd.extend(attrs)
@@ -1023,7 +1024,8 @@
 cont = {True: "y", False: "n"}[contiguous]
 cmd = ["lvcreate"]
 cmd.extend(LVM_NOBACKUP)
-cmd.extend(("--contiguous", cont, "--size", "%sm" % size))
+cmd.extend(("--setactivationskip", "y", "--ignoreactivationskip",
+"--contiguous", cont, "--size", "%sm" % size))
 if initialTag is not None:
 cmd.extend(("--addtag", initialTag))
 cmd.extend(("--name", lvName, vgName))


-- 
To view, visit http://gerrit.ovirt.org/20836
To unsubscribe, visit http://gerrit.ovirt.org/settings

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


Change in vdsm[master]: cleanup: Improve lib/vdsm imports (PEP328)

2013-11-03 Thread mtayer
mooli tayer has posted comments on this change.

Change subject: cleanup: Improve lib/vdsm imports (PEP328)
..


Patch Set 4:

(1 comment)


Commit Message
Line 29: from . import moduleX
Line 30: from ..subpackage2 import moduleZ
Line 31: 
Line 32: Which makes it much more evident where things are and avoids possible
Line 33: shadowing of packages in site-packages.
As a newb to python, I find these type of imports nicer to tell top level 
modules from package modules. If this forces us to have code base locations 
matching deployment locations that could be nice too.
Line 34: 
Line 35: Change-Id: I249cfa0ad734ea45ecbbecbade9daeed6c3adc12


-- 
To view, visit http://gerrit.ovirt.org/20555
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I249cfa0ad734ea45ecbbecbade9daeed6c3adc12
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Antoni Segura Puimedon 
Gerrit-Reviewer: Antoni Segura Puimedon 
Gerrit-Reviewer: Assaf Muller 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: Saggi Mizrahi 
Gerrit-Reviewer: Yaniv Bronhaim 
Gerrit-Reviewer: mooli tayer 
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: lvm: Prevent auto-actviation of logical volumes

2013-11-03 Thread ewarszaw
Eduardo has posted comments on this change.

Change subject: lvm: Prevent auto-actviation of logical volumes
..


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

(6 comments)


Commit Message
Line 16: lvm 2.02.100.
Line 17: 
Line 18: To make this change easy, lvm.changelv() was modified to accept 
variable
Line 19: argument tuples, instead of multiple non-related types. This also
Line 20: simplify the only caller that use multiple arguments.
This change is not required and gives less functionality.
Line 21: 
Line 22: Note: upgrade of existing volumes is not implemented yet.
Line 23: 
Line 24: Change-Id: Ie001c5a4c888bb1ca44bedaeeae6fb75e7cbacc0


Line 18: To make this change easy, lvm.changelv() was modified to accept 
variable
Line 19: argument tuples, instead of multiple non-related types. This also
Line 20: simplify the only caller that use multiple arguments.
Line 21: 
Line 22: Note: upgrade of existing volumes is not implemented yet.
The change of existing volumes should be changed with a little app running when 
installing or upgrading vdsm.
In addition should be runable from command line.
Should not be part of the vdsm code.
Line 23: 
Line 24: Change-Id: Ie001c5a4c888bb1ca44bedaeeae6fb75e7cbacc0
Line 25: Bug-Url: https://bugzilla.redhat.com/1009812



File vdsm/storage/blockSD.py
Line 990: operation and is resilient to open LVs, etc.
Line 991: """
Line 992: prefix = blockVolume.TAG_PREFIX_IMAGE
Line 993: try:
Line 994: lvm.changelv(sdUUID, volUUIDs, ("--available", "y"),
This change is not required and gives less functionality.
Line 995:  ("--deltag", prefix + imgUUID),
Line 996:  ("--addtag", prefix + opTag + imgUUID))
Line 997: except se.StorageException as e:
Line 998: log.error("Can't activate or change LV tags in SD %s. "



File vdsm/storage/lvm.py
Line 755: 
Line 756: vg: VG name
Line 757: lvs: a single LV name or iterable of LV names.
Line 758: attrs: lvchange argument tupples. Use None value for unary 
options.
Line 759:e.g. ('--attr', 'value'), ('--unary', None)
There is no vdsm use for unary operators.

The interface is cumbersome.
Line 760: 
Line 761: Note:
Line 762: You may activate an activated LV without error
Line 763: but lvchange returns an error (RC=5) when activating rw if 
already rw


Line 772: for attr, value in attrs:
Line 773: cmd.append(attr)
Line 774: if value is not None:
Line 775: cmd.append(value)
Line 776: if attr in ("-a", "--available", "--activate") and value == 
"y":
The if is redundant. Simply add --ignoreactivationskip for all the lvchanges.
Since vdsm is specifically activating the lvs, the modifier is required.
For other changes is a noop.
Line 777: cmd.append('--ignoreactivationskip')
Line 778: cmd.extend(lvnames)
Line 779: rc, out, err = _lvminfo.cmd(tuple(cmd), _lvminfo._getVGDevs((vg, 
)))
Line 780: _lvminfo._invalidatelvs(vg, lvs)


Line 1022: cont = {True: "y", False: "n"}[contiguous]
Line 1023: cmd = ["lvcreate"]
Line 1024: cmd.extend(LVM_NOBACKUP)
Line 1025: cmd.extend(("--contiguous", cont, "--size", "%sm" % size,
Line 1026: "--setactivationskip", "y"))
--setactivationskip y requires either --zero n or --ignoreactivationskip
Line 1027: if initialTag is not None:
Line 1028: cmd.extend(("--addtag", initialTag))
Line 1029: cmd.extend(("--name", lvName, vgName))
Line 1030: rc, out, err = _lvminfo.cmd(cmd, _lvminfo._getVGDevs((vgName, )))


-- 
To view, visit http://gerrit.ovirt.org/20832
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie001c5a4c888bb1ca44bedaeeae6fb75e7cbacc0
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Ayal Baron 
Gerrit-Reviewer: Eduardo 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: Introducing configurator package in vdsm-tool

2013-11-03 Thread Alon Bar-Lev
Alon Bar-Lev has posted comments on this change.

Change subject: Introducing configurator package in vdsm-tool
..


Patch Set 24:

I still do not follow how restart of sanlock cause it to run under different 
account.

The only possibility is that it is not 'configure' but 'upgrade', for example 
old sanlock ran under root, and newer under some other account and we want to 
restart the services to force upgrade... then we should explicitly comment 
this, including which version was the first to work correctly.

-- 
To view, visit http://gerrit.ovirt.org/20100
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I16bf5894e7e55a84b4c2a0caacde383ae7c19242
Gerrit-PatchSet: 24
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yaniv Bronhaim 
Gerrit-Reviewer: Alon Bar-Lev 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: Yaniv Bronhaim 
Gerrit-Reviewer: Zhou Zheng Sheng 
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: Introducing configurator package in vdsm-tool

2013-11-03 Thread Alon Bar-Lev
Alon Bar-Lev has posted comments on this change.

Change subject: Introducing configurator package in vdsm-tool
..


Patch Set 24:

No more comments, just the sanlock should not be abnormality.

-- 
To view, visit http://gerrit.ovirt.org/20100
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I16bf5894e7e55a84b4c2a0caacde383ae7c19242
Gerrit-PatchSet: 24
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yaniv Bronhaim 
Gerrit-Reviewer: Alon Bar-Lev 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: Yaniv Bronhaim 
Gerrit-Reviewer: Zhou Zheng Sheng 
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: vdsm-tool: vdsm-id: add force option to force generate id

2013-11-03 Thread ybronhei
Yaniv Bronhaim has posted comments on this change.

Change subject: vdsm-tool: vdsm-id: add force option to force generate id
..


Patch Set 7: Code-Review+1

-- 
To view, visit http://gerrit.ovirt.org/20808
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I89f1e29c9cdad0cadb32545fa27c1702ad2e116a
Gerrit-PatchSet: 7
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Alon Bar-Lev 
Gerrit-Reviewer: Alon Bar-Lev 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Yaniv Bronhaim 
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: Introducing configurator package in vdsm-tool

2013-11-03 Thread ybronhei
Yaniv Bronhaim has posted comments on this change.

Change subject: Introducing configurator package in vdsm-tool
..


Patch Set 24: Verified+1

Verified. although there is still libvirtd stop in libvirtd_sysv2upstart which 
I intend to remove, and I still check why in http://gerrit.ovirt.org/#/c/16742/ 
the sanlock check was called reconfigure, which confused me..

any more comments? please ack if not.

-- 
To view, visit http://gerrit.ovirt.org/20100
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I16bf5894e7e55a84b4c2a0caacde383ae7c19242
Gerrit-PatchSet: 24
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yaniv Bronhaim 
Gerrit-Reviewer: Alon Bar-Lev 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: Yaniv Bronhaim 
Gerrit-Reviewer: Zhou Zheng Sheng 
Gerrit-Reviewer: oVirt Jenkins CI Server
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: Add miniaml issciadm tests

2013-11-03 Thread smizrahi
Saggi Mizrahi has posted comments on this change.

Change subject: tests: Add miniaml issciadm tests
..


Patch Set 11:

I understand what you are testing I just don't understand why  you want to test 
that.
It can't catch any mistake, if you change the arguments because of a misstype 
someone will catch that. It is the kind of errors you don't guard against. If 
you change the arguments on purpose then you just added a new place to change 
it as well to make sure he is really sure that the interface changed.

And I understand what the fake module does I just don't think it's something 
that needs to be done since stubs need to simulate something. They can't be 
empty or they are useless and can only test useless cases like the cases in 
this patch.

-- 
To view, visit http://gerrit.ovirt.org/19856
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I9ed21408c7b496c384053ce02cdb66d47df9a14b
Gerrit-PatchSet: 11
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Daniel Erez 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Saggi Mizrahi 
Gerrit-Reviewer: Sergey Gotliv 
Gerrit-Reviewer: Vered Volansky 
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: lvm: Prevent auto-actviation of logical volumes

2013-11-03 Thread nsoffer
Nir Soffer has posted comments on this change.

Change subject: lvm: Prevent auto-actviation of logical volumes
..


Patch Set 1:

Missing also spec changes - depend on lvm 2.02.100.

-- 
To view, visit http://gerrit.ovirt.org/20832
To unsubscribe, visit http://gerrit.ovirt.org/settings

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


Change in vdsm[master]: Introducing configurator package in vdsm-tool

2013-11-03 Thread oVirt Jenkins CI Server
oVirt Jenkins CI Server has posted comments on this change.

Change subject: Introducing configurator package in vdsm-tool
..


Patch Set 24:

Build Successful 

http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/4394/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/5198/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/5274/ : SUCCESS

-- 
To view, visit http://gerrit.ovirt.org/20100
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I16bf5894e7e55a84b4c2a0caacde383ae7c19242
Gerrit-PatchSet: 24
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yaniv Bronhaim 
Gerrit-Reviewer: Alon Bar-Lev 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: Yaniv Bronhaim 
Gerrit-Reviewer: Zhou Zheng Sheng 
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: iscsi: Iscsi rescan cleanup

2013-11-03 Thread oVirt Jenkins CI Server
oVirt Jenkins CI Server has posted comments on this change.

Change subject: iscsi: Iscsi rescan cleanup
..


Patch Set 14:

Build Successful 

http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/4393/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/5197/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/5273/ : SUCCESS

-- 
To view, visit http://gerrit.ovirt.org/19256
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I842eb37cea3bd5e8cd357a310dd966081b242abd
Gerrit-PatchSet: 14
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Saggi Mizrahi 
Gerrit-Reviewer: Ayal Baron 
Gerrit-Reviewer: Barak Azulay 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Eduardo 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: Saggi Mizrahi 
Gerrit-Reviewer: Yaniv Bronhaim 
Gerrit-Reviewer: Yeela Kaplan 
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: lvm: Prevent auto-actviation of logical volumes

2013-11-03 Thread oVirt Jenkins CI Server
oVirt Jenkins CI Server has posted comments on this change.

Change subject: lvm: Prevent auto-actviation of logical volumes
..


Patch Set 1:

Build Successful 

http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/4392/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/5196/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/5272/ : SUCCESS

-- 
To view, visit http://gerrit.ovirt.org/20832
To unsubscribe, visit http://gerrit.ovirt.org/settings

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


Change in vdsm[master]: lvm: Prevent auto-actviation of logical volumes

2013-11-03 Thread nsoffer
Nir Soffer has uploaded a new change for review.

Change subject: lvm: Prevent auto-actviation of logical volumes
..

lvm: Prevent auto-actviation of logical volumes

When using FC storage, physical volumes are connected during boot, and
vdsm logical volumes are auto-activated by /etc/rc.sysinit and/or
/etc/init.d/netfs.  This is abnormal situation that vdsm cannot handle,
and leads to data corruption.

This patch prevents auto-activation of vdsm volumes using new
--setactivationskip and --ignoreactivationskip options, introcuded in
lvm 2.02.100.

To make this change easy, lvm.changelv() was modified to accept variable
argument tuples, instead of multiple non-related types. This also
simplify the only caller that use multiple arguments.

Note: upgrade of existing volumes is not implemented yet.

Change-Id: Ie001c5a4c888bb1ca44bedaeeae6fb75e7cbacc0
Bug-Url: https://bugzilla.redhat.com/1009812
Signed-off-by: Nir Soffer 
---
M vdsm/storage/blockSD.py
M vdsm/storage/lvm.py
2 files changed, 15 insertions(+), 15 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/32/20832/1

diff --git a/vdsm/storage/blockSD.py b/vdsm/storage/blockSD.py
index b95be21..70998c7 100644
--- a/vdsm/storage/blockSD.py
+++ b/vdsm/storage/blockSD.py
@@ -989,11 +989,11 @@
 Tagging is preferable to rename since it can be done in a single lvm
 operation and is resilient to open LVs, etc.
 """
+prefix = blockVolume.TAG_PREFIX_IMAGE
 try:
-lvm.changelv(sdUUID, volUUIDs, (("-a", "y"),
-("--deltag", blockVolume.TAG_PREFIX_IMAGE + imgUUID),
-("--addtag", blockVolume.TAG_PREFIX_IMAGE +
- opTag + imgUUID)))
+lvm.changelv(sdUUID, volUUIDs, ("--available", "y"),
+ ("--deltag", prefix + imgUUID),
+ ("--addtag", prefix + opTag + imgUUID))
 except se.StorageException as e:
 log.error("Can't activate or change LV tags in SD %s. "
   "failing Image %s %s operation for vols: %s. %s",
diff --git a/vdsm/storage/lvm.py b/vdsm/storage/lvm.py
index 0c2964e..f3baeb2 100644
--- a/vdsm/storage/lvm.py
+++ b/vdsm/storage/lvm.py
@@ -749,14 +749,14 @@
 "vgchange on vg(s) %s failed. %d %s %s" % (vgs, rc, out, err))
 
 
-def changelv(vg, lvs, attrs):
+def changelv(vg, lvs, *attrs):
 """
 Change multiple attributes on multiple LVs.
 
 vg: VG name
 lvs: a single LV name or iterable of LV names.
-attrs: an iterable of (attr, value) pairs),
-e.g. (('--available', 'y'), ('--permission', 'rw')
+attrs: lvchange argument tupples. Use None value for unary options.
+   e.g. ('--attr', 'value'), ('--unary', None)
 
 Note:
 You may activate an activated LV without error
@@ -769,13 +769,12 @@
 lvnames = tuple("%s/%s" % (vg, lv) for lv in lvs)
 cmd = ["lvchange"]
 cmd.extend(LVM_NOBACKUP)
-if isinstance(attrs[0], str):
-# ("--attribute", "value")
-cmd.extend(attrs)
-else:
-# (("--aa", "v1"), ("--ab", "v2"))
-for attr in attrs:
-cmd.extend(attr)
+for attr, value in attrs:
+cmd.append(attr)
+if value is not None:
+cmd.append(value)
+if attr in ("-a", "--available", "--activate") and value == "y":
+cmd.append('--ignoreactivationskip')
 cmd.extend(lvnames)
 rc, out, err = _lvminfo.cmd(tuple(cmd), _lvminfo._getVGDevs((vg, )))
 _lvminfo._invalidatelvs(vg, lvs)
@@ -1023,7 +1022,8 @@
 cont = {True: "y", False: "n"}[contiguous]
 cmd = ["lvcreate"]
 cmd.extend(LVM_NOBACKUP)
-cmd.extend(("--contiguous", cont, "--size", "%sm" % size))
+cmd.extend(("--contiguous", cont, "--size", "%sm" % size,
+"--setactivationskip", "y"))
 if initialTag is not None:
 cmd.extend(("--addtag", initialTag))
 cmd.extend(("--name", lvName, vgName))


-- 
To view, visit http://gerrit.ovirt.org/20832
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: Ie001c5a4c888bb1ca44bedaeeae6fb75e7cbacc0
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]: cleanup: Improve lib/vdsm imports (PEP328)

2013-11-03 Thread smizrahi
Saggi Mizrahi has posted comments on this change.

Change subject: cleanup: Improve lib/vdsm imports (PEP328)
..


Patch Set 4: Code-Review-1

Although relative imports are nice we will not use them until we don't need to 
import from __future__.

Also, some of the places you change are places where the files are actually 
moved to sitelib after installation.

In any case, I would much rather if we move things to relative imports bit by 
bit. This kind of change is very volatile, testing would be simpler if we do it 
in smaller chunks.

-- 
To view, visit http://gerrit.ovirt.org/20555
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I249cfa0ad734ea45ecbbecbade9daeed6c3adc12
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Antoni Segura Puimedon 
Gerrit-Reviewer: Antoni Segura Puimedon 
Gerrit-Reviewer: Assaf Muller 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: Saggi Mizrahi 
Gerrit-Reviewer: Yaniv Bronhaim 
Gerrit-Reviewer: mooli tayer 
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: cleanup: Improve lib/vdsm imports (PEP328)

2013-11-03 Thread danken
Dan Kenigsberg has posted comments on this change.

Change subject: cleanup: Improve lib/vdsm imports (PEP328)
..


Patch Set 4:

(1 comment)


Commit Message
Line 29: from . import moduleX
Line 30: from ..subpackage2 import moduleZ
Line 31: 
Line 32: Which makes it much more evident where things are and avoids possible
Line 33: shadowing of packages in site-packages.
The old syntax in nicer, but ambiguous. I think it's time we start moving; 
https://fedoraproject.org/wiki/Changes/Python_3_as_Default
Line 34: 
Line 35: Change-Id: I249cfa0ad734ea45ecbbecbade9daeed6c3adc12


-- 
To view, visit http://gerrit.ovirt.org/20555
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I249cfa0ad734ea45ecbbecbade9daeed6c3adc12
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Antoni Segura Puimedon 
Gerrit-Reviewer: Antoni Segura Puimedon 
Gerrit-Reviewer: Assaf Muller 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: Saggi Mizrahi 
Gerrit-Reviewer: Yaniv Bronhaim 
Gerrit-Reviewer: mooli tayer 
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: xmlrpc: Parsing error logging enhancement - vdsClient

2013-11-03 Thread ybronhei
Yaniv Bronhaim has posted comments on this change.

Change subject: xmlrpc: Parsing error logging enhancement - vdsClient
..


Patch Set 2:

(1 comment)


File lib/vdsm/vdscli.py.in
Line 37: class TransportWrapper:
Line 38: def __init__(self, transport):
Line 39: self._transport = transport
Line 40: 
Line 41: def __getattr__(self, name):
imo wrapper is the right way here. you just modify the getattr which is common 
for attributes, and wrap the call
Line 42: if hasattr(self._transport, name):
Line 43: func = getattr(self._transport, name)
Line 44: return lambda *args, **kwargs: self._wrap(name, func, 
args, kwargs)
Line 45: raise AttributeError(name)


-- 
To view, visit http://gerrit.ovirt.org/20627
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ife29c4f7749b9cd8a4ad892f486d91509e505ae4
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Piotr Kliczewski 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: Saggi Mizrahi 
Gerrit-Reviewer: Yaniv Bronhaim 
Gerrit-Reviewer: mooli tayer 
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: vdsm-tool: vdsm-id: add force option to force generate id

2013-11-03 Thread oVirt Jenkins CI Server
oVirt Jenkins CI Server has posted comments on this change.

Change subject: vdsm-tool: vdsm-id: add force option to force generate id
..


Patch Set 7:

Build Successful 

http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/4391/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/5195/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/5271/ : SUCCESS

-- 
To view, visit http://gerrit.ovirt.org/20808
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I89f1e29c9cdad0cadb32545fa27c1702ad2e116a
Gerrit-PatchSet: 7
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Alon Bar-Lev 
Gerrit-Reviewer: Alon Bar-Lev 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Yaniv Bronhaim 
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: vdsm-tool: vdsm-id: add force option to force generate id

2013-11-03 Thread Alon Bar-Lev
Alon Bar-Lev has posted comments on this change.

Change subject: vdsm-tool: vdsm-id: add force option to force generate id
..


Patch Set 7:

modified commit message

-- 
To view, visit http://gerrit.ovirt.org/20808
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I89f1e29c9cdad0cadb32545fa27c1702ad2e116a
Gerrit-PatchSet: 7
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Alon Bar-Lev 
Gerrit-Reviewer: Alon Bar-Lev 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Yaniv Bronhaim 
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: Introducing configurator package in vdsm-tool

2013-11-03 Thread Alon Bar-Lev
Alon Bar-Lev has posted comments on this change.

Change subject: Introducing configurator package in vdsm-tool
..


Patch Set 23:

I have no more comments except that we need to move the sanlock configuration 
here to have sane solution.

-- 
To view, visit http://gerrit.ovirt.org/20100
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I16bf5894e7e55a84b4c2a0caacde383ae7c19242
Gerrit-PatchSet: 23
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yaniv Bronhaim 
Gerrit-Reviewer: Alon Bar-Lev 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: Yaniv Bronhaim 
Gerrit-Reviewer: Zhou Zheng Sheng 
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: Introducing configurator package in vdsm-tool

2013-11-03 Thread ybronhei
Yaniv Bronhaim has posted comments on this change.

Change subject: Introducing configurator package in vdsm-tool
..


Patch Set 23:

Alon, lets continue with the review until I'll understand all the material 
about sanlock configure. I won't forget about your request to modify it. 
Is the patch accepted by you as it is?

-- 
To view, visit http://gerrit.ovirt.org/20100
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I16bf5894e7e55a84b4c2a0caacde383ae7c19242
Gerrit-PatchSet: 23
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yaniv Bronhaim 
Gerrit-Reviewer: Alon Bar-Lev 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: Yaniv Bronhaim 
Gerrit-Reviewer: Zhou Zheng Sheng 
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: vdsm-tool: vdsm-id: add force option to force generate id

2013-11-03 Thread ybronhei
Yaniv Bronhaim has posted comments on this change.

Change subject: vdsm-tool: vdsm-id: add force option to force generate id
..


Patch Set 6:

(1 comment)


File lib/vdsm/utils.py
Line 624: 
Line 625: if p.returncode == 0 and 'Not' not in out:
Line 626: #Avoid error string - 'Not Settable' or 'Not Present'
Line 627: __hostUUID = out.strip()
Line 628: elif force:
I will think about it . for now I accept this solution to enforce response from 
that function. the commit message should emphasize that this is to avoid cases 
where hardware uuid is not "easy" to gather imo.. it doesn't really relate to 
the existence of /etc/vdsm/vdsm.id file
Line 629: hostid = str(uuid.uuid4())
Line 630: with open(constants.P_VDSM_NODE_ID, 'w') as f:
Line 631: f.write("%s\n", hostid)
Line 632: ovirtNodePersist([constants.P_VDSM_NODE_ID])


-- 
To view, visit http://gerrit.ovirt.org/20808
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I89f1e29c9cdad0cadb32545fa27c1702ad2e116a
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Alon Bar-Lev 
Gerrit-Reviewer: Alon Bar-Lev 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Yaniv Bronhaim 
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: vdsm-tool: vdsm-id: add force option to force generate id

2013-11-03 Thread Alon Bar-Lev
Alon Bar-Lev has posted comments on this change.

Change subject: vdsm-tool: vdsm-id: add force option to force generate id
..


Patch Set 6:

(1 comment)


File lib/vdsm/utils.py
Line 624: 
Line 625: if p.returncode == 0 and 'Not' not in out:
Line 626: #Avoid error string - 'Not Settable' or 'Not Present'
Line 627: __hostUUID = out.strip()
Line 628: elif force:
the /etc/vdsm/vdsm.id feature is existing feature, we are not discussing 
extending this, but make it more easier to use.

if you have idea of hardware based solution it should be in separate patch... 
and separate idea... and even if you find a solution, you should always allow 
software based uuid, to allow workaround cases you cannot think of.
Line 629: hostid = str(uuid.uuid4())
Line 630: with open(constants.P_VDSM_NODE_ID, 'w') as f:
Line 631: f.write("%s\n", hostid)
Line 632: ovirtNodePersist([constants.P_VDSM_NODE_ID])


-- 
To view, visit http://gerrit.ovirt.org/20808
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I89f1e29c9cdad0cadb32545fa27c1702ad2e116a
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Alon Bar-Lev 
Gerrit-Reviewer: Alon Bar-Lev 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Yaniv Bronhaim 
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: vdsm-tool: vdsm-id: add force option to force generate id

2013-11-03 Thread ybronhei
Yaniv Bronhaim has posted comments on this change.

Change subject: vdsm-tool: vdsm-id: add force option to force generate id
..


Patch Set 6:

(1 comment)


File lib/vdsm/utils.py
Line 624: 
Line 625: if p.returncode == 0 and 'Not' not in out:
Line 626: #Avoid error string - 'Not Settable' or 'Not Present'
Line 627: __hostUUID = out.strip()
Line 628: elif force:
:/ What can I say against that.. if there is no way to get better uuid for a 
server and we must use uuid package so let it be. for now.

Although I'm quite convinced that we can find better uuid to use based on the 
hardware. Don't know how urgent this workaround is, but I would prefer to see 
hardware based solution..
Line 629: hostid = str(uuid.uuid4())
Line 630: with open(constants.P_VDSM_NODE_ID, 'w') as f:
Line 631: f.write("%s\n", hostid)
Line 632: ovirtNodePersist([constants.P_VDSM_NODE_ID])


-- 
To view, visit http://gerrit.ovirt.org/20808
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I89f1e29c9cdad0cadb32545fa27c1702ad2e116a
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Alon Bar-Lev 
Gerrit-Reviewer: Alon Bar-Lev 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Yaniv Bronhaim 
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: Unified network persistence [4.1/4.*] - Upgrade mechanism

2013-11-03 Thread amuller
Assaf Muller has posted comments on this change.

Change subject: Unified network persistence [4.1/4.*] - Upgrade mechanism
..


Patch Set 10:

(1 comment)


File lib/vdsm/tool/upgrade.py
Line 36: self.log = logging.getLogger('vdsm-tool')
Line 37: except Exception:
Line 38: logging.basicConfig(filename='/dev/stdout', filemode='w+',
Line 39: level=logging.DEBUG)
Line 40: logging.error("Could not init proper logging", 
exc_info=True)
I don't care much if upgrades are ran as part of vdsm-tool or by some other 
mechanism. I don't think the comparison to vdsClient is valid. Either case, I'd 
still want logs of successful / failed upgrades, what did they do and when they 
were ran. If we just log to /var/log/messages the data might be lost by the 
time the user reports a bug or asks for help.
Line 41: 
Line 42: def isNeeded(self):
Line 43: return not os.path.exists(self.upgradeFilePath)
Line 44: 


-- 
To view, visit http://gerrit.ovirt.org/17726
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Iba3c9c34f03134c192db1c2add31084824e195d9
Gerrit-PatchSet: 10
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Assaf Muller 
Gerrit-Reviewer: Alon Bar-Lev 
Gerrit-Reviewer: Antoni Segura Puimedon 
Gerrit-Reviewer: Assaf Muller 
Gerrit-Reviewer: Barak Azulay 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: Giuseppe Vallarelli 
Gerrit-Reviewer: Mark Wu 
Gerrit-Reviewer: Saggi Mizrahi 
Gerrit-Reviewer: Yaniv Bronhaim 
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: vm: Pause vm in case volume allocation is greater than lv size

2013-11-03 Thread oVirt Jenkins CI Server
oVirt Jenkins CI Server has posted comments on this change.

Change subject: vm: Pause vm in case volume allocation is greater than lv size
..


Patch Set 9:

Build Successful 

http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/4390/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/5194/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/5270/ : SUCCESS

-- 
To view, visit http://gerrit.ovirt.org/19719
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Id95aa1ed7508218d8ae6a878606cd2d251ac2b05
Gerrit-PatchSet: 9
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yeela Kaplan 
Gerrit-Reviewer: Arik Hadas 
Gerrit-Reviewer: Ayal Baron 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Eduardo 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: Michal Skrivanek 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Sergey Gotliv 
Gerrit-Reviewer: Yeela Kaplan 
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: vdsm-tool: vdsm-id: add force option to force generate id

2013-11-03 Thread Alon Bar-Lev
Alon Bar-Lev has posted comments on this change.

Change subject: vdsm-tool: vdsm-id: add force option to force generate id
..


Patch Set 6:

(1 comment)


File lib/vdsm/utils.py
Line 624: 
Line 625: if p.returncode == 0 and 'Not' not in out:
Line 626: #Avoid error string - 'Not Settable' or 'Not Present'
Line 627: __hostUUID = out.strip()
Line 628: elif force:
there are bioses without uuid.
Line 629: hostid = str(uuid.uuid4())
Line 630: with open(constants.P_VDSM_NODE_ID, 'w') as f:
Line 631: f.write("%s\n", hostid)
Line 632: ovirtNodePersist([constants.P_VDSM_NODE_ID])


-- 
To view, visit http://gerrit.ovirt.org/20808
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I89f1e29c9cdad0cadb32545fa27c1702ad2e116a
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Alon Bar-Lev 
Gerrit-Reviewer: Alon Bar-Lev 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Yaniv Bronhaim 
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: vdsm-tool: vdsm-id: add force option to force generate id

2013-11-03 Thread ybronhei
Yaniv Bronhaim has posted comments on this change.

Change subject: vdsm-tool: vdsm-id: add force option to force generate id
..


Patch Set 6:

(1 comment)


File lib/vdsm/utils.py
Line 624: 
Line 625: if p.returncode == 0 and 'Not' not in out:
Line 626: #Avoid error string - 'Not Settable' or 'Not Present'
Line 627: __hostUUID = out.strip()
Line 628: elif force:
when did it fail to gather HostUUID anyway? did you bump in case where 
getHostUUID was failed? .. I haven't . the only case I can think of is ppc.. so 
I'm just trying to understand why will we ever reach to the force part. anyhow, 
I don't mind to have such ability to get the uuid, but still .. why\when
Line 629: hostid = str(uuid.uuid4())
Line 630: with open(constants.P_VDSM_NODE_ID, 'w') as f:
Line 631: f.write("%s\n", hostid)
Line 632: ovirtNodePersist([constants.P_VDSM_NODE_ID])


-- 
To view, visit http://gerrit.ovirt.org/20808
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I89f1e29c9cdad0cadb32545fa27c1702ad2e116a
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Alon Bar-Lev 
Gerrit-Reviewer: Alon Bar-Lev 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Yaniv Bronhaim 
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: fcp: Deactivate vdsm volume groups during boot

2013-11-03 Thread Alon Bar-Lev
Alon Bar-Lev has posted comments on this change.

Change subject: fcp: Deactivate vdsm volume groups during boot
..


Patch Set 1:

> Current plan is to have this patch only for zstream

so why do you submit it into master? strange...

-- 
To view, visit http://gerrit.ovirt.org/20720
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I8f72a68ad09566ba222aa45448c78d1577c40d21
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Alon Bar-Lev 
Gerrit-Reviewer: Ayal Baron 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Yaniv Bronhaim 
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: fcp: Deactivate vdsm volume groups during boot

2013-11-03 Thread nsoffer
Nir Soffer has posted comments on this change.

Change subject: fcp: Deactivate vdsm volume groups during boot
..


Patch Set 1:

Current plan is to have this patch only for zstream, and use lvm activation 
skipping for el 6.5 (depends on lvm-2.02.100).

To support upgrade on FCP storage, we must detect volumes used by qemu and 
deactivate only those.

If we have enough infrastructure in zstream to write this as a pre-start 
script, I'll do this. This will handle both boot and upgrade use cases.

-- 
To view, visit http://gerrit.ovirt.org/20720
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I8f72a68ad09566ba222aa45448c78d1577c40d21
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Alon Bar-Lev 
Gerrit-Reviewer: Ayal Baron 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Yaniv Bronhaim 
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: utils: Create AsyncProcessOperation

2013-11-03 Thread ybronhei
Yaniv Bronhaim has posted comments on this change.

Change subject: utils: Create AsyncProcessOperation
..


Patch Set 11: Code-Review+1

-- 
To view, visit http://gerrit.ovirt.org/19254
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I79d0eefc9d917a4a93916d52867fb4f1e793c60e
Gerrit-PatchSet: 11
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Saggi Mizrahi 
Gerrit-Reviewer: Ayal Baron 
Gerrit-Reviewer: Barak Azulay 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Eduardo 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: Saggi Mizrahi 
Gerrit-Reviewer: Yaniv Bronhaim 
Gerrit-Reviewer: Yeela Kaplan 
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: vdsm-tool: vdsm-id: add force option to force generate id

2013-11-03 Thread Alon Bar-Lev
Alon Bar-Lev has posted comments on this change.

Change subject: vdsm-tool: vdsm-id: add force option to force generate id
..


Patch Set 6:

(1 comment)


File lib/vdsm/utils.py
Line 624: 
Line 625: if p.returncode == 0 and 'Not' not in out:
Line 626: #Avoid error string - 'Not Settable' or 'Not Present'
Line 627: __hostUUID = out.strip()
Line 628: elif force:
I do not understand... I am not trying to fix the world now...

all I need is to ask vdsm what is its id without having to deal with handling 
the /etc/vdsm/vdsm.id

are the above valid to this task?

the new code of vdsm-reg will call vdsm-tool vdsm-id --force and get id no 
matter if it is uuid or generated from anywhere else, this id is required for 
registration.

if you then want to improve how it is 'generated' you are welcome to do so, it 
will not break the interface.
Line 629: hostid = str(uuid.uuid4())
Line 630: with open(constants.P_VDSM_NODE_ID, 'w') as f:
Line 631: f.write("%s\n", hostid)
Line 632: ovirtNodePersist([constants.P_VDSM_NODE_ID])


-- 
To view, visit http://gerrit.ovirt.org/20808
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I89f1e29c9cdad0cadb32545fa27c1702ad2e116a
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Alon Bar-Lev 
Gerrit-Reviewer: Alon Bar-Lev 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Yaniv Bronhaim 
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: vdsm-tool: vdsm-id: add force option to force generate id

2013-11-03 Thread ybronhei
Yaniv Bronhaim has posted comments on this change.

Change subject: vdsm-tool: vdsm-id: add force option to force generate id
..


Patch Set 6:

(1 comment)


File lib/vdsm/utils.py
Line 624: 
Line 625: if p.returncode == 0 and 'Not' not in out:
Line 626: #Avoid error string - 'Not Settable' or 'Not Present'
Line 627: __hostUUID = out.strip()
Line 628: elif force:
first, please explain this in the commit message

second, for other archs as ppc you should have specific code base (already done 
in http://gerrit.ovirt.org/#/c/19395/ so use that), the use of dmidecode should 
use the dmidecode_util.py and not explicit as now.

third, also appropriate solution for nested visualization instead of relying on 
uuid package 

fourth - you have to have something depends of the machine that should be much 
more unique than using uuid package

and last, when you have specific solution for archs and nested virt, why would 
you need force in any flow?
Line 629: hostid = str(uuid.uuid4())
Line 630: with open(constants.P_VDSM_NODE_ID, 'w') as f:
Line 631: f.write("%s\n", hostid)
Line 632: ovirtNodePersist([constants.P_VDSM_NODE_ID])


-- 
To view, visit http://gerrit.ovirt.org/20808
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I89f1e29c9cdad0cadb32545fa27c1702ad2e116a
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Alon Bar-Lev 
Gerrit-Reviewer: Alon Bar-Lev 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Yaniv Bronhaim 
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: fcp: Deactivate vdsm volume groups during boot

2013-11-03 Thread ybronhei
Yaniv Bronhaim has posted comments on this change.

Change subject: fcp: Deactivate vdsm volume groups during boot
..


Patch Set 1:

(1 comment)


File init/sysvinit/vdsm-deactivate-vgs.init
Line 22: ### END INIT INFO
Line 23: 
Line 24: . /etc/init.d/functions
Line 25: 
Line 26: run_file="var/run/vdsm/lvm/deactivate-vgs"
also.. and find your match in configure.ac, should be @VDSMRUNDIR@
Line 27: prog="vdsm-deactivate-vgs"
Line 28: retval=0
Line 29: 
Line 30: log_failure_msg()


-- 
To view, visit http://gerrit.ovirt.org/20720
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I8f72a68ad09566ba222aa45448c78d1577c40d21
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Alon Bar-Lev 
Gerrit-Reviewer: Ayal Baron 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Yaniv Bronhaim 
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: vdsm-tool: vdsm-id: add force option to force generate id

2013-11-03 Thread Alon Bar-Lev
Alon Bar-Lev has posted comments on this change.

Change subject: vdsm-tool: vdsm-id: add force option to force generate id
..


Patch Set 6:

(1 comment)


File lib/vdsm/utils.py
Line 624: 
Line 625: if p.returncode == 0 and 'Not' not in out:
Line 626: #Avoid error string - 'Not Settable' or 'Not Present'
Line 627: __hostUUID = out.strip()
Line 628: elif force:
this is a question for the entire patch...

1. there is no machine uuid at all.

2. it is nested virtualization and similar.

3. it is other arch

4. it is vdsm-reg that needs to get id (which is what I am doing now... remove 
vdsm-reg)
Line 629: hostid = str(uuid.uuid4())
Line 630: with open(constants.P_VDSM_NODE_ID, 'w') as f:
Line 631: f.write("%s\n", hostid)
Line 632: ovirtNodePersist([constants.P_VDSM_NODE_ID])


-- 
To view, visit http://gerrit.ovirt.org/20808
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I89f1e29c9cdad0cadb32545fa27c1702ad2e116a
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Alon Bar-Lev 
Gerrit-Reviewer: Alon Bar-Lev 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Yaniv Bronhaim 
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: fcp: Deactivate vdsm volume groups during boot

2013-11-03 Thread ybronhei
Yaniv Bronhaim has posted comments on this change.

Change subject: fcp: Deactivate vdsm volume groups during boot
..


Patch Set 1:

first: no "require reboot" ever

second: files under /var/run persist during upgrade, so I don't understand the 
problem

third: "volumes should never be activated" - that's how it works and you change 
it, anything you change should not be forced if you don't run, as i see it

-- 
To view, visit http://gerrit.ovirt.org/20720
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I8f72a68ad09566ba222aa45448c78d1577c40d21
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Alon Bar-Lev 
Gerrit-Reviewer: Ayal Baron 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Yaniv Bronhaim 
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: vdsm-tool: vdsm-id: add force option to force generate id

2013-11-03 Thread ybronhei
Yaniv Bronhaim has posted comments on this change.

Change subject: vdsm-tool: vdsm-id: add force option to force generate id
..


Patch Set 6:

(1 comment)


File lib/vdsm/utils.py
Line 624: 
Line 625: if p.returncode == 0 and 'Not' not in out:
Line 626: #Avoid error string - 'Not Settable' or 'Not Present'
Line 627: __hostUUID = out.strip()
Line 628: elif force:
why do you need the force option unless dmidecode doesn't work properly ?
Line 629: hostid = str(uuid.uuid4())
Line 630: with open(constants.P_VDSM_NODE_ID, 'w') as f:
Line 631: f.write("%s\n", hostid)
Line 632: ovirtNodePersist([constants.P_VDSM_NODE_ID])


-- 
To view, visit http://gerrit.ovirt.org/20808
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I89f1e29c9cdad0cadb32545fa27c1702ad2e116a
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Alon Bar-Lev 
Gerrit-Reviewer: Alon Bar-Lev 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Yaniv Bronhaim 
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: vdsm-tool: vdsm-id: add force option to force generate id

2013-11-03 Thread Alon Bar-Lev
Alon Bar-Lev has posted comments on this change.

Change subject: vdsm-tool: vdsm-id: add force option to force generate id
..


Patch Set 6:

what hack? maybe I do not follow.

-- 
To view, visit http://gerrit.ovirt.org/20808
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I89f1e29c9cdad0cadb32545fa27c1702ad2e116a
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Alon Bar-Lev 
Gerrit-Reviewer: Alon Bar-Lev 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Yaniv Bronhaim 
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: fcp: Deactivate vdsm volume groups during boot

2013-11-03 Thread nsoffer
Nir Soffer has posted comments on this change.

Change subject: fcp: Deactivate vdsm volume groups during boot
..


Patch Set 1:

(2 comments)

> also bare in mind that user can have vdsmd installed without running it at 
> all, so why forcing him this in each restart?

Force the deactivation?

> it should run only on first boot when vdsmd starts up.

vdsm volumes should never be activated, even if vdsm is not running.

> Upgrading or restarting won't harm when using that logic only on first boot.

When upgrading from old version that did not run on first boot, we should not 
deactivate volumes without checking they are used.

> this logic must be under pre-start tasks

To handle upgrade correctly, we depend on application logic and state. Or we 
can require reboot for this one time upgrade when using FCP.


File init/sysvinit/vdsm-deactivate-vgs.init
Line 22: ### END INIT INFO
Line 23: 
Line 24: . /etc/init.d/functions
Line 25: 
Line 26: run_file="var/run/vdsm/lvm/deactivate-vgs"
Do you mean UPPERCASE?
Line 27: prog="vdsm-deactivate-vgs"
Line 28: retval=0
Line 29: 
Line 30: log_failure_msg()


Line 80: ;;
Line 81: stop)
Line 82: ;;
Line 83: *)
Line 84: echo "Usage: $0 {start|stop}"
Script is being called with stop argument on shutdown, and doing nothing is 
currently the best implementation.
Line 85: retval=2
Line 86: esac
Line 87: 


-- 
To view, visit http://gerrit.ovirt.org/20720
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I8f72a68ad09566ba222aa45448c78d1577c40d21
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Alon Bar-Lev 
Gerrit-Reviewer: Ayal Baron 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Yaniv Bronhaim 
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: Quarantine ovirt-3.0.0 network upgrade

2013-11-03 Thread amuller
Assaf Muller has posted comments on this change.

Change subject: Quarantine ovirt-3.0.0 network upgrade
..


Patch Set 1: -Code-Review

Dan, if you do end up using vdsm-tool then you might want to use the upgrade 
mechanism :)

-- 
To view, visit http://gerrit.ovirt.org/20803
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Icb21715dc3b92fc6c198dbb4c49f0bbef0cb
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dan Kenigsberg 
Gerrit-Reviewer: Antoni Segura Puimedon 
Gerrit-Reviewer: Assaf Muller 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Yaniv Bronhaim 
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: vdsm-tool: vdsm-id: add force option to force generate id

2013-11-03 Thread ybronhei
Yaniv Bronhaim has posted comments on this change.

Change subject: vdsm-tool: vdsm-id: add force option to force generate id
..


Patch Set 6:

can't find your response to my question "why not having appropriate solution 
for cases where dmidecode is not the way to get unique uuid, instead of making 
such hack" ..? isn't that relevant only for new archs? if yes, lets have 
specific solution for each as we do for gathering hardware information

-- 
To view, visit http://gerrit.ovirt.org/20808
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I89f1e29c9cdad0cadb32545fa27c1702ad2e116a
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Alon Bar-Lev 
Gerrit-Reviewer: Alon Bar-Lev 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Yaniv Bronhaim 
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: Quarantine ovirt-3.0.0 network upgrade

2013-11-03 Thread ybronhei
Yaniv Bronhaim has posted comments on this change.

Change subject: Quarantine ovirt-3.0.0 network upgrade
..


Patch Set 1: Code-Review-1

until convincing me why not having it under the tool I must use my -1 force.. 
sorry :/

-- 
To view, visit http://gerrit.ovirt.org/20803
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Icb21715dc3b92fc6c198dbb4c49f0bbef0cb
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dan Kenigsberg 
Gerrit-Reviewer: Antoni Segura Puimedon 
Gerrit-Reviewer: Assaf Muller 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Yaniv Bronhaim 
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: Quarantine ovirt-3.0.0 network upgrade

2013-11-03 Thread ybronhei
Yaniv Bronhaim has posted comments on this change.

Change subject: Quarantine ovirt-3.0.0 network upgrade
..


Patch Set 1:

use whatever you need from /usr/share/vdsm and add the path for that if 
required.. although I don't understand why, we import from vdsm also in other 
parts of the tool. I really prefer to have it there, and I want to move all 
external scripts there to centralize all vdsm's external utils\scripts

-- 
To view, visit http://gerrit.ovirt.org/20803
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Icb21715dc3b92fc6c198dbb4c49f0bbef0cb
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dan Kenigsberg 
Gerrit-Reviewer: Antoni Segura Puimedon 
Gerrit-Reviewer: Assaf Muller 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Yaniv Bronhaim 
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: cleanup: Improve lib/vdsm imports (PEP328)

2013-11-03 Thread nsoffer
Nir Soffer has posted comments on this change.

Change subject: cleanup: Improve lib/vdsm imports (PEP328)
..


Patch Set 4:

(1 comment)


Commit Message
Line 29: from . import moduleX
Line 30: from ..subpackage2 import moduleZ
Line 31: 
Line 32: Which makes it much more evident where things are and avoids possible
Line 33: shadowing of packages in site-packages.
I agree that the syntax is ugly, but debugging import errors is even more ugly.

We can use the old way to handle this, using fully qualified modules names, for 
example:

from foo.bar import baz

This works everywhere, even when you move modules around. You have to change 
the import only when package names are modified.

The only trouble with this is where you want to take foo and include it in 
another project, where foo is at other/foo. But you can easily fix this by 
putting foo in other/lib/ and add other/lib to python path.

Of course this requires having code packaged properly. Not sure if it will work 
with current codebase.
Line 34: 
Line 35: Change-Id: I249cfa0ad734ea45ecbbecbade9daeed6c3adc12


-- 
To view, visit http://gerrit.ovirt.org/20555
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I249cfa0ad734ea45ecbbecbade9daeed6c3adc12
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Antoni Segura Puimedon 
Gerrit-Reviewer: Antoni Segura Puimedon 
Gerrit-Reviewer: Assaf Muller 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: Saggi Mizrahi 
Gerrit-Reviewer: Yaniv Bronhaim 
Gerrit-Reviewer: mooli tayer 
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: xmlrpc: Parsing error logging enhancement - vdsClient

2013-11-03 Thread danken
Dan Kenigsberg has posted comments on this change.

Change subject: xmlrpc: Parsing error logging enhancement - vdsClient
..


Patch Set 2:

(1 comment)


File lib/vdsm/vdscli.py.in
Line 37: class TransportWrapper:
Line 38: def __init__(self, transport):
Line 39: self._transport = transport
Line 40: 
Line 41: def __getattr__(self, name):
I suspect that in this case, it would be clearer what you want to wrap.
Line 42: if hasattr(self._transport, name):
Line 43: func = getattr(self._transport, name)
Line 44: return lambda *args, **kwargs: self._wrap(name, func, 
args, kwargs)
Line 45: raise AttributeError(name)


-- 
To view, visit http://gerrit.ovirt.org/20627
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ife29c4f7749b9cd8a4ad892f486d91509e505ae4
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Piotr Kliczewski 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: Saggi Mizrahi 
Gerrit-Reviewer: Yaniv Bronhaim 
Gerrit-Reviewer: mooli tayer 
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: vm: Pause vm in case volume allocation is greater than lv size

2013-11-03 Thread sgotliv
Sergey Gotliv has posted comments on this change.

Change subject: vm: Pause vm in case volume allocation is greater than lv size
..


Patch Set 8:

I am not sure why Jenkins decided that vm.py line 514 is not properly indented. 
I successfully passed the build on my local environment.

I'll check it...

-- 
To view, visit http://gerrit.ovirt.org/19719
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Id95aa1ed7508218d8ae6a878606cd2d251ac2b05
Gerrit-PatchSet: 8
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yeela Kaplan 
Gerrit-Reviewer: Arik Hadas 
Gerrit-Reviewer: Ayal Baron 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Eduardo 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: Michal Skrivanek 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Sergey Gotliv 
Gerrit-Reviewer: Yeela Kaplan 
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: fcp: Deactivate vdsm volume groups during boot

2013-11-03 Thread Alon Bar-Lev
Alon Bar-Lev has posted comments on this change.

Change subject: fcp: Deactivate vdsm volume groups during boot
..


Patch Set 1:

> For this kind of upgrade we may either require a reboot, or detect which lvs 
> are used and deactivate the rest.

and because of this the entire solution of doing that in the packaging seems to 
be wrong. you require application logic.

I do not understand why vdsm cannot start, determine if this fixup is required 
via looking at system, communicating with libvirt, and if the fixup is required 
call svdsm to perform this on its behalf.

-- 
To view, visit http://gerrit.ovirt.org/20720
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I8f72a68ad09566ba222aa45448c78d1577c40d21
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Alon Bar-Lev 
Gerrit-Reviewer: Ayal Baron 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Yaniv Bronhaim 
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: cleanup: Improve lib/vdsm imports (PEP328)

2013-11-03 Thread ybronhei
Yaniv Bronhaim has posted comments on this change.

Change subject: cleanup: Improve lib/vdsm imports (PEP328)
..


Patch Set 4:

(1 comment)


Commit Message
Line 29: from . import moduleX
Line 30: from ..subpackage2 import moduleZ
Line 31: 
Line 32: Which makes it much more evident where things are and avoids possible
Line 33: shadowing of packages in site-packages.
don't see why it much more evident for anything. I didn't like it in c and I 
won't like it here.. I prefer to leave the old way until or when we'll decide 
to move to python version that forces it
Line 34: 
Line 35: Change-Id: I249cfa0ad734ea45ecbbecbade9daeed6c3adc12


-- 
To view, visit http://gerrit.ovirt.org/20555
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I249cfa0ad734ea45ecbbecbade9daeed6c3adc12
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Antoni Segura Puimedon 
Gerrit-Reviewer: Antoni Segura Puimedon 
Gerrit-Reviewer: Assaf Muller 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Yaniv Bronhaim 
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: vm: Pause vm in case volume allocation is greater than lv size

2013-11-03 Thread sgotliv
Sergey Gotliv has posted comments on this change.

Change subject: vm: Pause vm in case volume allocation is greater than lv size
..


Patch Set 8:

1. Rebase.

2. Added new log printing due to Ayal's comment.

-- 
To view, visit http://gerrit.ovirt.org/19719
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Id95aa1ed7508218d8ae6a878606cd2d251ac2b05
Gerrit-PatchSet: 8
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yeela Kaplan 
Gerrit-Reviewer: Arik Hadas 
Gerrit-Reviewer: Ayal Baron 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Eduardo 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: Michal Skrivanek 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Sergey Gotliv 
Gerrit-Reviewer: Yeela Kaplan 
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: vm: Pause vm in case volume allocation is greater than lv size

2013-11-03 Thread oVirt Jenkins CI Server
oVirt Jenkins CI Server has posted comments on this change.

Change subject: vm: Pause vm in case volume allocation is greater than lv size
..


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

Build Failed 

http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/4389/ : FAILURE

http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/5193/ : UNSTABLE

http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/5269/ : FAILURE

-- 
To view, visit http://gerrit.ovirt.org/19719
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Id95aa1ed7508218d8ae6a878606cd2d251ac2b05
Gerrit-PatchSet: 8
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yeela Kaplan 
Gerrit-Reviewer: Arik Hadas 
Gerrit-Reviewer: Ayal Baron 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Eduardo 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: Michal Skrivanek 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Sergey Gotliv 
Gerrit-Reviewer: Yeela Kaplan 
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: vm: Pause vm in case volume allocation is greater than lv size

2013-11-03 Thread sgotliv
Sergey Gotliv has posted comments on this change.

Change subject: vm: Pause vm in case volume allocation is greater than lv size
..


Patch Set 7:

(1 comment)


File vdsm/vm.py
Line 515: 
Line 516: if physical - alloc < drive.watermarkLimit:
Line 517: extend.append((drive, capacity, alloc, physical))
Line 518: 
Line 519: for drive, capacity, alloc, physical in extend:
I am adding a log here, but here its probably interesting only in case you are 
coming from _onAbnormalStop.
Maybe it would be better to pint log there before calling "extend" and then 
just follow the other printings in order to understand whether extend is 
happened or not.

What do you think?
Line 520: self._log.info('%s/%s apparent: %s capacity: %s, alloc: 
%s, '
Line 521:'physical: %s', drive.domainID, 
drive.volumeID,
Line 522:drive.apparentsize, capacity, alloc, 
physical)
Line 523: self._vm.extendDriveVolume(drive)


-- 
To view, visit http://gerrit.ovirt.org/19719
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Id95aa1ed7508218d8ae6a878606cd2d251ac2b05
Gerrit-PatchSet: 7
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yeela Kaplan 
Gerrit-Reviewer: Arik Hadas 
Gerrit-Reviewer: Ayal Baron 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Eduardo 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: Michal Skrivanek 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Sergey Gotliv 
Gerrit-Reviewer: Yeela Kaplan 
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: fcp: Deactivate vdsm volume groups during boot

2013-11-03 Thread ybronhei
Yaniv Bronhaim has posted comments on this change.

Change subject: fcp: Deactivate vdsm volume groups during boot
..


Patch Set 1: Code-Review-1

(2 comments)

using a file under /var/run to sign first boot is enough. also bare in mind 
that user can have vdsmd installed without running it at all, so why forcing 
him this in each restart? it should run only on first boot when vdsmd starts 
up. Upgrading or restarting won't harm when using that logic only on first 
boot. this logic must be under pre-start tasks, using another external script 
just make vdsm more complex to understand imo, and harder to maintain for other 
distributions


File init/sysvinit/vdsm-deactivate-vgs.init
Line 22: ### END INIT INFO
Line 23: 
Line 24: . /etc/init.d/functions
Line 25: 
Line 26: run_file="var/run/vdsm/lvm/deactivate-vgs"
why not using constants?
Line 27: prog="vdsm-deactivate-vgs"
Line 28: retval=0
Line 29: 
Line 30: log_failure_msg()


Line 80: ;;
Line 81: stop)
Line 82: ;;
Line 83: *)
Line 84: echo "Usage: $0 {start|stop}"
stop does nothing.. so why having it?
Line 85: retval=2
Line 86: esac
Line 87: 


-- 
To view, visit http://gerrit.ovirt.org/20720
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I8f72a68ad09566ba222aa45448c78d1577c40d21
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Alon Bar-Lev 
Gerrit-Reviewer: Ayal Baron 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Yaniv Bronhaim 
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: fcp: Deactivate vdsm volume groups during boot

2013-11-03 Thread Alon Bar-Lev
Alon Bar-Lev has posted comments on this change.

Change subject: fcp: Deactivate vdsm volume groups during boot
..


Patch Set 1:

> you can touch var/run in the spec file to avoid this problem.

of course that everything that is not long living can be done at pre-start...

> Alon, I much prefer to keep this (temporary) hack out of the main vdsm logic.

why is it temporary?

anyway, from what I see /now/ within the pre-tasks... a cleaner design would 
have used this logic within vdsm->vdsmd at startup... I think that it is better 
to create that logic and start a move... which much more control over the 
process.

-- 
To view, visit http://gerrit.ovirt.org/20720
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I8f72a68ad09566ba222aa45448c78d1577c40d21
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Alon Bar-Lev 
Gerrit-Reviewer: Ayal Baron 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Yaniv Bronhaim 
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: fcp: Deactivate vdsm volume groups during boot

2013-11-03 Thread nsoffer
Nir Soffer has posted comments on this change.

Change subject: fcp: Deactivate vdsm volume groups during boot
..


Patch Set 1:

>> For this kind of upgrade we may either require a reboot, or detect which lvs 
>> are used and deactivate the rest.

> and because of this the entire solution of doing that in the packaging seems 
> to be wrong. you require application logic.

> I do not understand why vdsm cannot start, determine if this fixup is 
> required via looking at system, communicating with libvirt, and if the fixup 
> is required call svdsm to perform this on its behalf.

We can do that, but it is much more complicated, and really needed only when 
upgrading from unfixed version when using FCP.

-- 
To view, visit http://gerrit.ovirt.org/20720
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I8f72a68ad09566ba222aa45448c78d1577c40d21
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Alon Bar-Lev 
Gerrit-Reviewer: Ayal Baron 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Yaniv Bronhaim 
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: fcp: Deactivate vdsm volume groups during boot

2013-11-03 Thread danken
Dan Kenigsberg has posted comments on this change.

Change subject: fcp: Deactivate vdsm volume groups during boot
..


Patch Set 1:

Nir, right, i did not think about upgrade logic. Though you can touch var/run 
in the spec file to avoid this problem.

Alon, I much prefer to keep this (temporary) hack out of the main vdsm logic.

-- 
To view, visit http://gerrit.ovirt.org/20720
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I8f72a68ad09566ba222aa45448c78d1577c40d21
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Alon Bar-Lev 
Gerrit-Reviewer: Ayal Baron 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Yaniv Bronhaim 
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: fcp: Deactivate vdsm volume groups during boot

2013-11-03 Thread nsoffer
Nir Soffer has posted comments on this change.

Change subject: fcp: Deactivate vdsm volume groups during boot
..


Patch Set 1:

(3 comments)

> Ayal, the current implementation would not cut the storage under a running 
> VM, as it makes sure deactivate_vdsm_vgs runs only once since boot.

> Moving the code to a pre-task would save you the need to duplicate this 
> service for upstart and systemd.

Using pre-start will work for install or when upgrading from fixed version to 
newer one, but is not correct when upgrading unfixed version, because there may 
be running vms using activated lvs. For this kind of upgrade we may either 
require a reboot, or detect which lvs are used and deactivate the rest.

> Nir, please add the new file to the Makefile, spec and deb definitions.

I'm working an alternative fix. I'll continue in this direction if the 
alternative fix is rejected.


Commit Message
Line 17: This script must also be used during installation or ugprade. It is 
safe
Line 18: to invoke it multiple times; it will modify volume groups only on the
Line 19: first run.
Line 20: 
Line 21: On RHEL 6.5 we can use new activation skipping option instead of this
This will work for Fedora, with the proper systemd configuration file.

Will fix RHEL trademark issue.
Line 22: script. I'll address this in a separate patch.
Line 23: 
Line 24: Change-Id: I8f72a68ad09566ba222aa45448c78d1577c40d21
Line 25: Bug-Url: https://bugzilla.redhat.com/1009812



File init/sysvinit/vdsm-deactivate-vgs.init
Line 22: ### END INIT INFO
Line 23: 
Line 24: . /etc/init.d/functions
Line 25: 
Line 26: run_file="var/run/vdsm/lvm/deactivate-vgs"
Will fix.
Line 27: prog="vdsm-deactivate-vgs"
Line 28: retval=0
Line 29: 
Line 30: log_failure_msg()


Line 38: }
Line 39: 
Line 40: is_first_run()
Line 41: {
Line 42: test ! -f $run_file
Will fix.
Line 43: }
Line 44: 
Line 45: set_was_run()
Line 46: {


-- 
To view, visit http://gerrit.ovirt.org/20720
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I8f72a68ad09566ba222aa45448c78d1577c40d21
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Alon Bar-Lev 
Gerrit-Reviewer: Ayal Baron 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Yaniv Bronhaim 
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches