Change in vdsm[master]: tests: prevent hook validation decorator from leaving script...
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
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
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.
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.
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.
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.
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.
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
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
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.
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
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)
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)
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
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
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
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
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
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
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
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
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
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.
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.
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)
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
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
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
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
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
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
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
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
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
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
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
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)
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)
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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)
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
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
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
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)
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
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
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
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
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
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
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
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
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