Change in vdsm[master]: iscsicred: Support per-host iscsi credentials database
Nir Soffer has uploaded a new change for review. Change subject: iscsicred: Support per-host iscsi credentials database .. iscsicred: Support per-host iscsi credentials database Some users need unique host iSCSI credentials for each target, but Engine supports only same credentials for all hosts. This series adds support for simple per-host iSCSI credentials database. Credentials are stored in /etc/vdsm/iscsi-cred/ Credentials file format: username = foo:bar password = 12345678 This patch adds the iscsicred module, providing readonly access to the iSCSI credentials database. Change-Id: I8f6a838f6b8e132d6b0c1a8135f2c28ef1e7f847 Signed-off-by: Nir Soffer --- M debian/vdsm.install M tests/Makefile.am A tests/iscsicred_test.py M vdsm.spec.in M vdsm/storage/Makefile.am A vdsm/storage/iscsicred.py M vdsm/supervdsmServer 7 files changed, 207 insertions(+), 0 deletions(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/79/43179/1 diff --git a/debian/vdsm.install b/debian/vdsm.install index 35fcba7..c7cc90e 100644 --- a/debian/vdsm.install +++ b/debian/vdsm.install @@ -110,6 +110,7 @@ ./usr/share/vdsm/storage/imageSharing.py ./usr/share/vdsm/storage/iscsi.py ./usr/share/vdsm/storage/iscsiadm.py +./usr/share/vdsm/storage/iscsicred.py ./usr/share/vdsm/storage/localFsSD.py ./usr/share/vdsm/storage/lvm.env ./usr/share/vdsm/storage/lvm.py diff --git a/tests/Makefile.am b/tests/Makefile.am index b9b8e72..30ee221 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -56,6 +56,7 @@ iproute2Tests.py \ ipwrapperTests.py \ iscsiTests.py \ + iscsicred_test.py \ jsonRpcHelper.py \ jsonRpcTests.py \ libvirtconnectionTests.py \ diff --git a/tests/iscsicred_test.py b/tests/iscsicred_test.py new file mode 100644 index 000..715dded --- /dev/null +++ b/tests/iscsicred_test.py @@ -0,0 +1,121 @@ +# +# Copyright 2015 Red Hat, Inc. +# +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 2 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write to the Free Software +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA +# +# Refer to the README and COPYING files for full details of the license +# + +from contextlib import contextmanager +import os + +from testlib import VdsmTestCase +from testlib import namedTemporaryDir +from monkeypatch import MonkeyPatchScope, MonkeyClass + +from vdsm.password import ProtectedPassword +from storage import iscsicred + + +class FakeSupervdsm(object): + +def getProxy(self): +return self + +def readTargetCredfile(self, targetName): +return iscsicred._read_credfile(targetName) + + +@MonkeyClass(iscsicred, "supervdsm", FakeSupervdsm()) +class IscsicredTests(VdsmTestCase): + +TARGET = "iqn.1994-05.com.redhat:target7" + +def test_not_found(self): +self.assertRaises(iscsicred.NotFound, iscsicred.get_credentials, + self.TARGET) + +def test_simple(self): +data = "".join(["username=foo:bar\n", +"password=12345678\n"]) +with credfile(self.TARGET, data): +d = iscsicred.get_credentials(self.TARGET) +self.assertEqual(d, {"username": "foo:bar", + "password": ProtectedPassword("12345678")}) + +def test_whitespace(self): +data = "".join(["username = foo:bar \n", +"password = 12345678 \n"]) +with credfile(self.TARGET, data): +d = iscsicred.get_credentials(self.TARGET) +self.assertEqual(d, {"username": "foo:bar", + "password": ProtectedPassword("12345678")}) + +def test_any_whitespace(self): +data = "".join(["\t\tusername\t\t=\t\tfoo:bar\t\t\n", +"\t\tpassword\t\t=\t\t12345678\t\t\n"]) +with credfile(self.TARGET, data): +d = iscsicred.get_credentials(self.TARGET) +self.assertEqual(d, {"username": "foo:bar", + "password": ProtectedPassword("12345678")}) + +def test_empty_lines(self): +data = "".join(["\n", +"username=foo:bar\n", +"\n", +"password=12345678\n", +"\n"]) +with credfile(self.TARGET, data): +d = iscsicred.get_credentials(self.TARGET) +self.assertEqu
Change in vdsm[master]: iscsicred: Support per-host iscsi credentials database
automat...@ovirt.org has posted comments on this change. Change subject: iscsicred: Support per-host iscsi credentials database .. Patch Set 1: * Update tracker::IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3']) -- To view, visit https://gerrit.ovirt.org/43179 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I8f6a838f6b8e132d6b0c1a8135f2c28ef1e7f847 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Nir Soffer Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: iscsicred: Protect sensitive return value in supervdsm log
automat...@ovirt.org has posted comments on this change. Change subject: iscsicred: Protect sensitive return value in supervdsm log .. Patch Set 1: * Update tracker::IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3']) -- To view, visit https://gerrit.ovirt.org/43178 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Idcfa4ee17466f75270909587c45ee9f703e5e9f3 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Nir Soffer Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: iscsicred: Protect sensitive return value in supervdsm log
Nir Soffer has uploaded a new change for review. Change subject: iscsicred: Protect sensitive return value in supervdsm log .. iscsicred: Protect sensitive return value in supervdsm log SuperVdsm logs all calls and return values, possibly exposive sensitive data in its log file. This patch allows Supervdsm methods to wrap the return value with ProtectedPassword object, so the results are logged as . Change-Id: Idcfa4ee17466f75270909587c45ee9f703e5e9f3 Signed-off-by: Nir Soffer --- M vdsm/supervdsmServer 1 file changed, 3 insertions(+), 0 deletions(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/78/43178/1 diff --git a/vdsm/supervdsmServer b/vdsm/supervdsmServer index 2a1a8f0..36c67b1 100755 --- a/vdsm/supervdsmServer +++ b/vdsm/supervdsmServer @@ -53,6 +53,7 @@ except ImportError: _glusterEnabled = False +from vdsm import password from vdsm import utils from vdsm import sysctl from vdsm.tool import restore_nets @@ -112,6 +113,8 @@ raise callbackLogger.debug('return %s with %s', func.__name__, res) +if isinstance(res, password.ProtectedPassword): +res = res.value return res return wrapper -- To view, visit https://gerrit.ovirt.org/43178 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Idcfa4ee17466f75270909587c45ee9f703e5e9f3 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]: tests: Allow setting custom libvirt error message
automat...@ovirt.org has posted comments on this change. Change subject: tests: Allow setting custom libvirt error message .. Patch Set 5: * Update tracker::IGNORE, no Bug-Url found * Set MODIFIED::IGNORE, no Bug-Url found. -- To view, visit https://gerrit.ovirt.org/43020 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I61e26d50ef7babd8bf9a9aadc38451297631a65b Gerrit-PatchSet: 5 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Nir Soffer Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Martin Polednik Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Vinzenz Feenstra Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: tests: Allow setting custom libvirt error message
Dan Kenigsberg has submitted this change and it was merged. Change subject: tests: Allow setting custom libvirt error message .. tests: Allow setting custom libvirt error message Fake Domain object was repeating the code in Error factory, and was using a default empty error message. Now we use Error and allow faking also the error message. Change-Id: I61e26d50ef7babd8bf9a9aadc38451297631a65b Signed-off-by: Nir Soffer Reviewed-on: https://gerrit.ovirt.org/43020 Reviewed-by: Martin Polednik Reviewed-by: Francesco Romani Continuous-Integration: Jenkins CI Reviewed-by: Dan Kenigsberg --- M tests/vmfakelib.py 1 file changed, 3 insertions(+), 3 deletions(-) Approvals: Nir Soffer: Verified Jenkins CI: Passed CI tests Dan Kenigsberg: Looks good to me, approved Francesco Romani: Looks good to me, but someone else must approve Martin Polednik: Looks good to me, but someone else must approve -- To view, visit https://gerrit.ovirt.org/43020 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: merged Gerrit-Change-Id: I61e26d50ef7babd8bf9a9aadc38451297631a65b Gerrit-PatchSet: 5 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Nir Soffer Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Martin Polednik Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Vinzenz Feenstra Gerrit-Reviewer: automat...@ovirt.org ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: tests: Allow setting custom libvirt error message
Dan Kenigsberg has posted comments on this change. Change subject: tests: Allow setting custom libvirt error message .. Patch Set 4: Code-Review+2 -- To view, visit https://gerrit.ovirt.org/43020 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I61e26d50ef7babd8bf9a9aadc38451297631a65b Gerrit-PatchSet: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Nir Soffer Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Martin Polednik Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Vinzenz Feenstra Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: tests: Add @recorded method decorator
Dan Kenigsberg has submitted this change and it was merged. Change subject: tests: Add @recorded method decorator .. tests: Add @recorded method decorator Manually recording function arguments and keywords again and again is not fun and error prone. This patch introduces a new decorator to make this task easier. To make a method recordable, mark the method you want to record: class Foo: @recorded def foo(self, a, b, c=3): pass def bar(self): return 42 Methods decorated with @recorded will record all calls to the instance's __recording__ list: f = Foo() f.foo(1, 2, 4) f.bar() f.foo(2, 3) print(f.__recording__) [("foo", (1, 2), {"c": 4}), ("foo", (2, 3), {"c": 3})] The new decorator is used now by vmfakelib.Domain. It may be useful for other fake objects as well. Change-Id: I43c7750ae1deb5fa9bbee40d9f6f624ce1105c4e Signed-off-by: Nir Soffer Reviewed-on: https://gerrit.ovirt.org/43015 Reviewed-by: Francesco Romani Continuous-Integration: Jenkins CI Reviewed-by: Dan Kenigsberg --- M tests/testlib.py M tests/testlibTests.py M tests/vmTests.py M tests/vmfakelib.py 4 files changed, 85 insertions(+), 3 deletions(-) Approvals: Nir Soffer: Verified Jenkins CI: Passed CI tests Dan Kenigsberg: Looks good to me, approved Francesco Romani: Looks good to me, but someone else must approve -- To view, visit https://gerrit.ovirt.org/43015 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: merged Gerrit-Change-Id: I43c7750ae1deb5fa9bbee40d9f6f624ce1105c4e Gerrit-PatchSet: 7 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Nir Soffer Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Allon Mureinik Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Federico Simoncelli Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Martin Polednik Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: automat...@ovirt.org ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: tests: Add @recorded method decorator
automat...@ovirt.org has posted comments on this change. Change subject: tests: Add @recorded method decorator .. Patch Set 7: * Update tracker::IGNORE, no Bug-Url found * Set MODIFIED::IGNORE, no Bug-Url found. -- To view, visit https://gerrit.ovirt.org/43015 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I43c7750ae1deb5fa9bbee40d9f6f624ce1105c4e Gerrit-PatchSet: 7 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Nir Soffer Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Allon Mureinik Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Federico Simoncelli Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Martin Polednik Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: tests: Record fake domain calls in a list
Dan Kenigsberg has submitted this change and it was merged. Change subject: tests: Record fake domain calls in a list .. tests: Record fake domain calls in a list Keeping calls in a list allows testing of multiple calls. We can check how many time a methods was called, what args and kwargs were used on each call, and we can verify the correct order of the calls. Change-Id: I279a1fd7a2e07b7adef4f446c9487a51d9df21f4 Signed-off-by: Nir Soffer Reviewed-on: https://gerrit.ovirt.org/42998 Reviewed-by: Francesco Romani Reviewed-by: Martin Polednik Continuous-Integration: Jenkins CI Reviewed-by: Dan Kenigsberg --- M tests/vmTests.py M tests/vmfakelib.py 2 files changed, 3 insertions(+), 3 deletions(-) Approvals: Nir Soffer: Verified Jenkins CI: Passed CI tests Dan Kenigsberg: Looks good to me, approved Francesco Romani: Looks good to me, but someone else must approve Martin Polednik: Looks good to me, but someone else must approve -- To view, visit https://gerrit.ovirt.org/42998 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: merged Gerrit-Change-Id: I279a1fd7a2e07b7adef4f446c9487a51d9df21f4 Gerrit-PatchSet: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Nir Soffer Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Martin Polednik Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: automat...@ovirt.org ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: tests: Record fake domain calls in a list
automat...@ovirt.org has posted comments on this change. Change subject: tests: Record fake domain calls in a list .. Patch Set 4: * Update tracker::IGNORE, no Bug-Url found * Set MODIFIED::IGNORE, no Bug-Url found. -- To view, visit https://gerrit.ovirt.org/42998 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I279a1fd7a2e07b7adef4f446c9487a51d9df21f4 Gerrit-PatchSet: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Nir Soffer Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Martin Polednik Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: tests: Add @recorded method decorator
Dan Kenigsberg has posted comments on this change. Change subject: tests: Add @recorded method decorator .. Patch Set 6: Code-Review+2 -- To view, visit https://gerrit.ovirt.org/43015 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I43c7750ae1deb5fa9bbee40d9f6f624ce1105c4e Gerrit-PatchSet: 6 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Nir Soffer Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Allon Mureinik Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Federico Simoncelli Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Martin Polednik Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: tests: Record fake domain calls in a list
Dan Kenigsberg has posted comments on this change. Change subject: tests: Record fake domain calls in a list .. Patch Set 3: Code-Review+2 -- To view, visit https://gerrit.ovirt.org/42998 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I279a1fd7a2e07b7adef4f446c9487a51d9df21f4 Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Nir Soffer Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Martin Polednik Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: vdsm hooks: report hook stderr to Engine if it fails an action
automat...@ovirt.org has posted comments on this change. Change subject: vdsm hooks: report hook stderr to Engine if it fails an action .. Patch Set 13: * Update tracker::#1219630::OK * Check Bug-Url::OK * Check Public Bug::#1219630::OK, public bug * Check Product::#1219630::OK, Correct product Red Hat Enterprise Virtualization Manager * Check TR::SKIP, not in a monitored branch (ovirt-3.5 ovirt-3.4 ovirt-3.3 ovirt-3.2) * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3']) -- To view, visit https://gerrit.ovirt.org/42592 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ibe2d5eb52cf2c8855d9d7d5e0ff1628a6cf1dc51 Gerrit-PatchSet: 13 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Marcin Mirecki Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Feng Yang Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Marcin Mirecki Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: build: Change dependency on glusterfs-cli and glusterfs-fuse
automat...@ovirt.org has posted comments on this change. Change subject: build: Change dependency on glusterfs-cli and glusterfs-fuse .. Patch Set 5: * Update tracker::IGNORE, no Bug-Url found * Set MODIFIED::IGNORE, no Bug-Url found. -- To view, visit https://gerrit.ovirt.org/43097 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I4cf424e7055ad071561d037b65972dde3b1b0b8b Gerrit-PatchSet: 5 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Ala Hino Gerrit-Reviewer: Allon Mureinik Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Eyal Edri Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Michal Skrivanek Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Yaniv Bronhaim Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: build: Change dependency on glusterfs-cli and glusterfs-fuse
Dan Kenigsberg has posted comments on this change. Change subject: build: Change dependency on glusterfs-cli and glusterfs-fuse .. Patch Set 4: Code-Review+2 -- To view, visit https://gerrit.ovirt.org/43097 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I4cf424e7055ad071561d037b65972dde3b1b0b8b Gerrit-PatchSet: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Ala Hino Gerrit-Reviewer: Allon Mureinik Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Eyal Edri Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Michal Skrivanek Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Yaniv Bronhaim Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: build: Change dependency on glusterfs-cli and glusterfs-fuse
Dan Kenigsberg has submitted this change and it was merged. Change subject: build: Change dependency on glusterfs-cli and glusterfs-fuse .. build: Change dependency on glusterfs-cli and glusterfs-fuse This is a revert of 766a54776555e6303b4ceae593c41bbfb0c764ae. Originally, glusterfs-cli and glusterfs-fuse were required only when not building vdsm on RHEV. We changed the spec file to always require gluster-cli and glusterfs-fuse in order to use gluster get volume info api to implement mounting gluster backup servers (https://gerrit.ovirt.org/41858). However, requiring these dependencies fails vdsm build on ppc64le. This patch revert requiring glusterfs-cli and glusterfs-fuse dependencies as were originally defined. Change-Id: I4cf424e7055ad071561d037b65972dde3b1b0b8b Signed-off-by: Ala Hino Reviewed-on: https://gerrit.ovirt.org/43097 Continuous-Integration: Jenkins CI Reviewed-by: Allon Mureinik Tested-by: Allon Mureinik Reviewed-by: Dan Kenigsberg --- M vdsm.spec.in 1 file changed, 2 insertions(+), 0 deletions(-) Approvals: Jenkins CI: Passed CI tests Dan Kenigsberg: Looks good to me, approved Allon Mureinik: Verified; Looks good to me, but someone else must approve -- To view, visit https://gerrit.ovirt.org/43097 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: merged Gerrit-Change-Id: I4cf424e7055ad071561d037b65972dde3b1b0b8b Gerrit-PatchSet: 5 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Ala Hino Gerrit-Reviewer: Allon Mureinik Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Eyal Edri Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Michal Skrivanek Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Yaniv Bronhaim Gerrit-Reviewer: automat...@ovirt.org ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: build: Change dependency on glusterfs-cli and glusterfs-fuse
automat...@ovirt.org has posted comments on this change. Change subject: build: Change dependency on glusterfs-cli and glusterfs-fuse .. Patch Set 4: * Update tracker::IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3']) -- To view, visit https://gerrit.ovirt.org/43097 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I4cf424e7055ad071561d037b65972dde3b1b0b8b Gerrit-PatchSet: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Ala Hino Gerrit-Reviewer: Allon Mureinik Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Eyal Edri Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Michal Skrivanek Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Yaniv Bronhaim Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: migration: use the response module
Dan Kenigsberg has submitted this change and it was merged. Change subject: migration: use the response module .. migration: use the response module Switch to response module for the sake of code clarity. Change-Id: I6d56b93d02467b4e95a29a9a98d4a09b22e64573 Signed-off-by: Francesco Romani Reviewed-on: https://gerrit.ovirt.org/40492 Reviewed-by: Dan Kenigsberg Continuous-Integration: Jenkins CI --- M vdsm/virt/migration.py 1 file changed, 5 insertions(+), 5 deletions(-) Approvals: Jenkins CI: Passed CI tests Dan Kenigsberg: Looks good to me, approved Francesco Romani: Verified -- To view, visit https://gerrit.ovirt.org/40492 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: merged Gerrit-Change-Id: I6d56b93d02467b4e95a29a9a98d4a09b22e64573 Gerrit-PatchSet: 9 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: automat...@ovirt.org ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: migration: use the response module
automat...@ovirt.org has posted comments on this change. Change subject: migration: use the response module .. Patch Set 9: * Update tracker::IGNORE, no Bug-Url found * Set MODIFIED::IGNORE, no Bug-Url found. -- To view, visit https://gerrit.ovirt.org/40492 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I6d56b93d02467b4e95a29a9a98d4a09b22e64573 Gerrit-PatchSet: 9 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: tcTest: skip tests if somehow same bridge name is reused
Nir Soffer has posted comments on this change. Change subject: tcTest: skip tests if somehow same bridge name is reused .. Patch Set 3: (1 comment) https://gerrit.ovirt.org/#/c/41898/3/tests/tcTests.py File tests/tcTests.py: Line 47: EXT_IP = "/sbin/ip" Line 48: Line 49: Line 50: class ExecError(RuntimeError): Line 51: pass This works, but it would be more clear to do: def __init__(self, msg, out, err): RuntimeError.__init__(self, msg) self.out = out self.err = err And later when we test this error: except ExecError as e: if needle in e.out: raise SkipTest(e.out) Line 52: Line 53: Line 54: def check_call(cmd): Line 55: rc, out, err = execCmd(cmd, raw=True) -- To view, visit https://gerrit.ovirt.org/41898 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Id678882d68976618360bcce76f16837d67e274ea Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Dan Kenigsberg Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Federico Simoncelli Gerrit-Reviewer: Ido Barkan Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: snapshot: Add Vm.freeze() and Vm.thaw() methods
Nir Soffer has posted comments on this change. Change subject: snapshot: Add Vm.freeze() and Vm.thaw() methods .. Patch Set 8: Verified+1 No code change, copying verify flag. -- To view, visit https://gerrit.ovirt.org/43018 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I8e4c54f461039de99823c3b80a10be0a960c4273 Gerrit-PatchSet: 8 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Nir Soffer Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Allon Mureinik Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Daniel Erez Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Maor Lipchuk Gerrit-Reviewer: Martin Polednik Gerrit-Reviewer: Michal Skrivanek Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Vinzenz Feenstra Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: snapshot: Add Vm.freeze() and Vm.thaw() methods
Nir Soffer has posted comments on this change. Change subject: snapshot: Add Vm.freeze() and Vm.thaw() methods .. Patch Set 7: This version address Francesco comments in vmfakelib. -- To view, visit https://gerrit.ovirt.org/43018 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I8e4c54f461039de99823c3b80a10be0a960c4273 Gerrit-PatchSet: 7 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Nir Soffer Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Allon Mureinik Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Daniel Erez Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Maor Lipchuk Gerrit-Reviewer: Martin Polednik Gerrit-Reviewer: Michal Skrivanek Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Vinzenz Feenstra Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: tests: Record fake domain calls in a list
automat...@ovirt.org has posted comments on this change. Change subject: tests: Record fake domain calls in a list .. Patch Set 3: * Update tracker::IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3']) -- To view, visit https://gerrit.ovirt.org/42998 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I279a1fd7a2e07b7adef4f446c9487a51d9df21f4 Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Nir Soffer Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Martin Polednik Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: snapshot: Add VM.freeze() and VM.thaw() verbs
automat...@ovirt.org has posted comments on this change. Change subject: snapshot: Add VM.freeze() and VM.thaw() verbs .. Patch Set 6: * Update tracker::IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3']) -- To view, visit https://gerrit.ovirt.org/43058 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I44c4237841e44548f48f626f4241d3f2e484930e Gerrit-PatchSet: 6 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Nir Soffer Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Allon Mureinik Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Daniel Erez Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Maor Lipchuk Gerrit-Reviewer: Martin Polednik Gerrit-Reviewer: Michal Skrivanek Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: snapshot: Add vdsClient freeze() and thaw() APIs
automat...@ovirt.org has posted comments on this change. Change subject: snapshot: Add vdsClient freeze() and thaw() APIs .. Patch Set 6: * Update tracker::IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3']) -- To view, visit https://gerrit.ovirt.org/43098 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I98872132767f3edd6d028cd9545e282e89f12777 Gerrit-PatchSet: 6 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Nir Soffer Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Allon Mureinik Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Daniel Erez Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Maor Lipchuk Gerrit-Reviewer: Martin Polednik Gerrit-Reviewer: Michal Skrivanek Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: tests: Allow setting custom libvirt error message
automat...@ovirt.org has posted comments on this change. Change subject: tests: Allow setting custom libvirt error message .. Patch Set 4: * Update tracker::IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3']) -- To view, visit https://gerrit.ovirt.org/43020 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I61e26d50ef7babd8bf9a9aadc38451297631a65b Gerrit-PatchSet: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Nir Soffer Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Martin Polednik Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Vinzenz Feenstra Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: tests: Add @recorded method decorator
automat...@ovirt.org has posted comments on this change. Change subject: tests: Add @recorded method decorator .. Patch Set 6: * Update tracker::IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3']) -- To view, visit https://gerrit.ovirt.org/43015 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I43c7750ae1deb5fa9bbee40d9f6f624ce1105c4e Gerrit-PatchSet: 6 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Nir Soffer Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Allon Mureinik Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Federico Simoncelli Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Martin Polednik Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: snapshot: Add Vm.freeze() and Vm.thaw() methods
automat...@ovirt.org has posted comments on this change. Change subject: snapshot: Add Vm.freeze() and Vm.thaw() methods .. Patch Set 8: * Update tracker::IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3']) -- To view, visit https://gerrit.ovirt.org/43018 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I8e4c54f461039de99823c3b80a10be0a960c4273 Gerrit-PatchSet: 8 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Nir Soffer Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Allon Mureinik Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Daniel Erez Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Maor Lipchuk Gerrit-Reviewer: Martin Polednik Gerrit-Reviewer: Michal Skrivanek Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Vinzenz Feenstra Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: snapshot: Add Vm.freeze() and Vm.thaw() methods
Nir Soffer has posted comments on this change. Change subject: snapshot: Add Vm.freeze() and Vm.thaw() methods .. Patch Set 7: (4 comments) https://gerrit.ovirt.org/#/c/43018/7/tests/vmfakelib.py File tests/vmfakelib.py: Line 245: Line 246: @recorded Line 247: def fsFreeze(self, mountpoints=None, flags=0): Line 248: self._failIfRequested() Line 249: return 3 > please add a comment (or perhaps better a constant) reminding us that this ok Line 250: Line 251: @recorded Line 252: def fsThaw(self, mountpoints=None, flags=0): Line 253: self._failIfRequested() Line 250: Line 251: @recorded Line 252: def fsThaw(self, mountpoints=None, flags=0): Line 253: self._failIfRequested() Line 254: return 3 > same ok Line 255: Line 256: Line 257: class GuestAgent(object): Line 258: def __init__(self): https://gerrit.ovirt.org/#/c/43018/7/vdsm/virt/vm.py File vdsm/virt/vm.py: Line 2890: else: Line 2891: name = "freezeErr" Line 2892: return response.error(name, message=e.get_error_message()) Line 2893: Line 2894: self.log.info("%d guest filesystems frozen", frozen) > Usually I'd have skipped, but we have recently begun a journey to reduce ou Freezing may take lot of time, since the guest agent is running multiple scripts one after another, so we would like to see one log message when this process start, and one log message when the process is finished. We need these info logs for any vm operation that may take lot of time. So I think that log.info is the correct way to log this. We have debug logs around xmlrpc and jsonrpc verbs, but we need info log for operations. If you turn of debug log, you will see this output during snapshot: [00] vmid=xxx Freezing guest filesystems [30] vmid=xxx 3 guest filesystems frozen [35] vmid=xxx Taking a snapshot drives=... memory=True [37] vmid=xxx Snapshot done [37] vmid=xxx Thawing guest filesystems [38] vmid=xxx 3 guest filesystems thawed We are using same logic for exeCmd - you get automatic log when a command start, and when command finished. These logs are very helpful when running commands that may get stuck or take log of time. The number of file systems is not very important, but I don't think it makes sense to add another debug log for this. Line 2895: return response.success() Line 2896: Line 2897: def thaw(self): Line 2898: """ Line 2913: else: Line 2914: name = "thawErr" Line 2915: return response.error(name, message=e.get_error_message()) Line 2916: Line 2917: self.log.info("%d guest filesystems thawed", thawed) > same comment as above See reply above. Line 2918: return response.success() Line 2919: Line 2920: def snapshot(self, snapDrives, memoryParams): Line 2921: """Live snapshot command""" -- To view, visit https://gerrit.ovirt.org/43018 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I8e4c54f461039de99823c3b80a10be0a960c4273 Gerrit-PatchSet: 7 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Nir Soffer Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Allon Mureinik Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Daniel Erez Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Maor Lipchuk Gerrit-Reviewer: Martin Polednik Gerrit-Reviewer: Michal Skrivanek Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Vinzenz Feenstra Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: snapshot: Add VM.freeze() and VM.thaw() verbs
Francesco Romani has posted comments on this change. Change subject: snapshot: Add VM.freeze() and VM.thaw() verbs .. Patch Set 5: Code-Review+1 -- To view, visit https://gerrit.ovirt.org/43058 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I44c4237841e44548f48f626f4241d3f2e484930e Gerrit-PatchSet: 5 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Nir Soffer Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Allon Mureinik Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Daniel Erez Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Maor Lipchuk Gerrit-Reviewer: Martin Polednik Gerrit-Reviewer: Michal Skrivanek Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: snapshot: Add Vm.freeze() and Vm.thaw() methods
Francesco Romani has posted comments on this change. Change subject: snapshot: Add Vm.freeze() and Vm.thaw() methods .. Patch Set 7: Code-Review+1 (4 comments) looks OK. Few minor comments inside for further improvements. I don't mind if you want to address them i separate patches. https://gerrit.ovirt.org/#/c/43018/7/tests/vmfakelib.py File tests/vmfakelib.py: Line 245: Line 246: @recorded Line 247: def fsFreeze(self, mountpoints=None, flags=0): Line 248: self._failIfRequested() Line 249: return 3 please add a comment (or perhaps better a constant) reminding us that this is the number of FS freezed Line 250: Line 251: @recorded Line 252: def fsThaw(self, mountpoints=None, flags=0): Line 253: self._failIfRequested() Line 250: Line 251: @recorded Line 252: def fsThaw(self, mountpoints=None, flags=0): Line 253: self._failIfRequested() Line 254: return 3 same Line 255: Line 256: Line 257: class GuestAgent(object): Line 258: def __init__(self): https://gerrit.ovirt.org/#/c/43018/7/vdsm/virt/vm.py File vdsm/virt/vm.py: Line 2890: else: Line 2891: name = "freezeErr" Line 2892: return response.error(name, message=e.get_error_message()) Line 2893: Line 2894: self.log.info("%d guest filesystems frozen", frozen) Usually I'd have skipped, but we have recently begun a journey to reduce our logging, so: why is this interesting? if it is important, we should probably report back it to Engine as well. If it is just debug, maybe better to use debug() Line 2895: return response.success() Line 2896: Line 2897: def thaw(self): Line 2898: """ Line 2913: else: Line 2914: name = "thawErr" Line 2915: return response.error(name, message=e.get_error_message()) Line 2916: Line 2917: self.log.info("%d guest filesystems thawed", thawed) same comment as above Line 2918: return response.success() Line 2919: Line 2920: def snapshot(self, snapDrives, memoryParams): Line 2921: """Live snapshot command""" -- To view, visit https://gerrit.ovirt.org/43018 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I8e4c54f461039de99823c3b80a10be0a960c4273 Gerrit-PatchSet: 7 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Nir Soffer Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Allon Mureinik Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Daniel Erez Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Maor Lipchuk Gerrit-Reviewer: Martin Polednik Gerrit-Reviewer: Michal Skrivanek Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Vinzenz Feenstra Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: snapshot: Add Vm.freeze() and Vm.thaw() methods
Nir Soffer has posted comments on this change. Change subject: snapshot: Add Vm.freeze() and Vm.thaw() methods .. Patch Set 7: Michal, the logic is very similar, but not the same. The error handling is almost the same, so we can refactor it out using a helper, but changing the api because the implementation is similar is not a good idea. -- To view, visit https://gerrit.ovirt.org/43018 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I8e4c54f461039de99823c3b80a10be0a960c4273 Gerrit-PatchSet: 7 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Nir Soffer Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Allon Mureinik Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Daniel Erez Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Maor Lipchuk Gerrit-Reviewer: Martin Polednik Gerrit-Reviewer: Michal Skrivanek Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Vinzenz Feenstra Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: snapshot: Add Vm.freeze() and Vm.thaw() methods
Michal Skrivanek has posted comments on this change. Change subject: snapshot: Add Vm.freeze() and Vm.thaw() methods .. Patch Set 7: I can live with current solution. But when whole logic is the same I prefer to have an argument. Yours fit this pattern quite nicely, unless it's going to diverge -- To view, visit https://gerrit.ovirt.org/43018 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I8e4c54f461039de99823c3b80a10be0a960c4273 Gerrit-PatchSet: 7 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Nir Soffer Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Allon Mureinik Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Daniel Erez Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Maor Lipchuk Gerrit-Reviewer: Martin Polednik Gerrit-Reviewer: Michal Skrivanek Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Vinzenz Feenstra Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: snapshot: Add Vm.freeze() and Vm.thaw() methods
Nir Soffer has posted comments on this change. Change subject: snapshot: Add Vm.freeze() and Vm.thaw() methods .. Patch Set 7: Michal, I will check using freeze() and thaw() in the next patch, handling the case where vm was already frozen when snapshot is called. Regarding using one verb, I don't think this is a good idea. Using this logic we can put all verbs into one verb, accepting the name of the verb as the first argument :-) -- To view, visit https://gerrit.ovirt.org/43018 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I8e4c54f461039de99823c3b80a10be0a960c4273 Gerrit-PatchSet: 7 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Nir Soffer Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Allon Mureinik Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Daniel Erez Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Maor Lipchuk Gerrit-Reviewer: Martin Polednik Gerrit-Reviewer: Michal Skrivanek Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Vinzenz Feenstra Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: snapshot: Add VM.freeze() and VM.thaw() verbs
Michal Skrivanek has posted comments on this change. Change subject: snapshot: Add VM.freeze() and VM.thaw() verbs .. Patch Set 5: Code-Review+1 -- To view, visit https://gerrit.ovirt.org/43058 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I44c4237841e44548f48f626f4241d3f2e484930e Gerrit-PatchSet: 5 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Nir Soffer Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Allon Mureinik Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Daniel Erez Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Maor Lipchuk Gerrit-Reviewer: Martin Polednik Gerrit-Reviewer: Michal Skrivanek Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: snapshot: Add Vm.freeze() and Vm.thaw() methods
Michal Skrivanek has posted comments on this change. Change subject: snapshot: Add Vm.freeze() and Vm.thaw() methods .. Patch Set 7: Code-Review+1 Thanks. Just 2 things for consideration - a single function taking freeze/thaw as argument? They are currently both almost the same, if it is expected to remain like that it would be "nicer" to consolidate -use freeze and thaw for normal snapshots too, just call the functions before and after libvirt snapshot call, removing the ugly "try snapshot with freeze flag, if it fails do it without a flag" -- To view, visit https://gerrit.ovirt.org/43018 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I8e4c54f461039de99823c3b80a10be0a960c4273 Gerrit-PatchSet: 7 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Nir Soffer Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Allon Mureinik Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Daniel Erez Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Maor Lipchuk Gerrit-Reviewer: Martin Polednik Gerrit-Reviewer: Michal Skrivanek Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Vinzenz Feenstra Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: tcTest: skip tests if somehow same bridge name is reused
automat...@ovirt.org has posted comments on this change. Change subject: tcTest: skip tests if somehow same bridge name is reused .. Patch Set 3: * Update tracker::IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3']) -- To view, visit https://gerrit.ovirt.org/41898 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Id678882d68976618360bcce76f16837d67e274ea Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Dan Kenigsberg Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Federico Simoncelli Gerrit-Reviewer: Ido Barkan Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: tcTest: skip tests if somehow same bridge name is reused
Dan Kenigsberg has posted comments on this change. Change subject: tcTest: skip tests if somehow same bridge name is reused .. Patch Set 3: Verified+1 >>> b=tcTests._Bridge() >>> b.addDevice() >>> b.addDevice() Traceback (most recent call last): File "", line 1, in File "tcTests.py", line 92, in addDevice raise SkipTest(e.args[2]) unittest.case.SkipTest: device vdsmtest-DpiB5 already exists; can't create bridge with the same name -- To view, visit https://gerrit.ovirt.org/41898 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Id678882d68976618360bcce76f16837d67e274ea Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Dan Kenigsberg Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Federico Simoncelli Gerrit-Reviewer: Ido Barkan Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: snapshot: Add VM.freeze() and VM.thaw() verbs
Nir Soffer has posted comments on this change. Change subject: snapshot: Add VM.freeze() and VM.thaw() verbs .. Patch Set 5: Manually rebase over parent patch. -- To view, visit https://gerrit.ovirt.org/43058 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I44c4237841e44548f48f626f4241d3f2e484930e Gerrit-PatchSet: 5 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Nir Soffer Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Allon Mureinik Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Daniel Erez Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Maor Lipchuk Gerrit-Reviewer: Martin Polednik Gerrit-Reviewer: Michal Skrivanek Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: snapshot: Add VM.freeze() and VM.thaw() verbs
Nir Soffer has posted comments on this change. Change subject: snapshot: Add VM.freeze() and VM.thaw() verbs .. Patch Set 5: Verified+1 -- To view, visit https://gerrit.ovirt.org/43058 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I44c4237841e44548f48f626f4241d3f2e484930e Gerrit-PatchSet: 5 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Nir Soffer Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Allon Mureinik Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Daniel Erez Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Maor Lipchuk Gerrit-Reviewer: Martin Polednik Gerrit-Reviewer: Michal Skrivanek Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: snapshot: Add Vm.freeze() and Vm.thaw() methods
Nir Soffer has posted comments on this change. Change subject: snapshot: Add Vm.freeze() and Vm.thaw() methods .. Patch Set 7: Verified+1 This version move freeze() and thaw() into Vm, since we did not have a consensus about the new snapshot module. We will discuss redesign of the Vm god class later. I managed to move the tests into vmTests.py, so this version is actually better tests, as Vm.freeze() and Vm.thaw() were not tested in the previous version. -- To view, visit https://gerrit.ovirt.org/43018 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I8e4c54f461039de99823c3b80a10be0a960c4273 Gerrit-PatchSet: 7 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Nir Soffer Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Allon Mureinik Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Daniel Erez Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Maor Lipchuk Gerrit-Reviewer: Martin Polednik Gerrit-Reviewer: Michal Skrivanek Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Vinzenz Feenstra Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: virt: fix VM creation logic condition
automat...@ovirt.org has posted comments on this change. Change subject: virt: fix VM creation logic condition .. Patch Set 1: * Update tracker::IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3']) -- To view, visit https://gerrit.ovirt.org/43170 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie1eb15f9865dadab791be97b7d009d5bae025265 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Martin Polednik Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: virt: fix VM creation logic condition
Martin Polednik has uploaded a new change for review. Change subject: virt: fix VM creation logic condition .. virt: fix VM creation logic condition We want to look up the CPU in tuple of CPUs, but we currently supply string instead of tuple. Change-Id: Ie1eb15f9865dadab791be97b7d009d5bae025265 Signed-off-by: Martin Polednik --- M vdsm/virt/vmxml.py 1 file changed, 1 insertion(+), 1 deletion(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/70/43170/1 diff --git a/vdsm/virt/vmxml.py b/vdsm/virt/vmxml.py index 7b9ab9c..b9ff3be 100644 --- a/vdsm/virt/vmxml.py +++ b/vdsm/virt/vmxml.py @@ -337,7 +337,7 @@ cpu = Element('cpu') -if self.arch in (caps.Architecture.X86_64): +if self.arch in (caps.Architecture.X86_64,): cpu.setAttrs(match='exact') features = self.conf.get('cpuType', 'qemu64').split(',') -- To view, visit https://gerrit.ovirt.org/43170 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Ie1eb15f9865dadab791be97b7d009d5bae025265 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Martin Polednik ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: virt: add logic for POWER cpu xml element
automat...@ovirt.org has posted comments on this change. Change subject: virt: add logic for POWER cpu xml element .. Patch Set 3: * Update tracker::IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3']) -- To view, visit https://gerrit.ovirt.org/42736 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I9303b76904ef1344508136343104b37c09d2a370 Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Martin Polednik Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Martin Polednik Gerrit-Reviewer: Michal Skrivanek Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: snapshot: Add Vm.freeze() and Vm.thaw() methods
automat...@ovirt.org has posted comments on this change. Change subject: snapshot: Add Vm.freeze() and Vm.thaw() methods .. Patch Set 7: * Update tracker::IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3']) -- To view, visit https://gerrit.ovirt.org/43018 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I8e4c54f461039de99823c3b80a10be0a960c4273 Gerrit-PatchSet: 7 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Nir Soffer Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Allon Mureinik Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Daniel Erez Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Maor Lipchuk Gerrit-Reviewer: Martin Polednik Gerrit-Reviewer: Michal Skrivanek Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Vinzenz Feenstra Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: snapshot: Add VM.freeze() and VM.thaw() verbs
automat...@ovirt.org has posted comments on this change. Change subject: snapshot: Add VM.freeze() and VM.thaw() verbs .. Patch Set 5: * Update tracker::IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3']) -- To view, visit https://gerrit.ovirt.org/43058 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I44c4237841e44548f48f626f4241d3f2e484930e Gerrit-PatchSet: 5 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Nir Soffer Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Allon Mureinik Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Daniel Erez Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Maor Lipchuk Gerrit-Reviewer: Martin Polednik Gerrit-Reviewer: Michal Skrivanek Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: snapshot: Add vdsClient freeze() and thaw() APIs
automat...@ovirt.org has posted comments on this change. Change subject: snapshot: Add vdsClient freeze() and thaw() APIs .. Patch Set 5: * Update tracker::IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3']) -- To view, visit https://gerrit.ovirt.org/43098 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I98872132767f3edd6d028cd9545e282e89f12777 Gerrit-PatchSet: 5 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Nir Soffer Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Allon Mureinik Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Daniel Erez Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Maor Lipchuk Gerrit-Reviewer: Martin Polednik Gerrit-Reviewer: Michal Skrivanek Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: hostdev: fix hostdev addressing
automat...@ovirt.org has posted comments on this change. Change subject: hostdev: fix hostdev addressing .. Patch Set 6: * Update tracker::IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3']) -- To view, visit https://gerrit.ovirt.org/43042 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I6d21555ec18d0e8f9766064755443785be3ef697 Gerrit-PatchSet: 6 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Martin Polednik Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Ido Barkan Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Martin Polednik Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: lib: migration: add response helpers
Francesco Romani has posted comments on this change. Change subject: lib: migration: add response helpers .. Patch Set 4: Verified+1 changes in response module covered by tests. Changes in migration.py are trivial, so little room for breakage. Run the patched VDSM and did a few migrations, no obvious breakage. -- To view, visit https://gerrit.ovirt.org/42795 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I6ccefec4f1bebcb2ca64f0bc9f6b9e9954dbf04c Gerrit-PatchSet: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Marcin Mirecki Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: migration: use the response module
Francesco Romani has posted comments on this change. Change subject: migration: use the response module .. Patch Set 8: Verified+1 The patch is simple, so little risk, verified running migrations on patched VDSM. -- To view, visit https://gerrit.ovirt.org/40492 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I6d56b93d02467b4e95a29a9a98d4a09b22e64573 Gerrit-PatchSet: 8 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: virt: do not use status after getStat()
automat...@ovirt.org has posted comments on this change. Change subject: virt: do not use status after getStat() .. Patch Set 6: * Update tracker::IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3']) -- To view, visit https://gerrit.ovirt.org/40522 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I2f1d330376590d0c4060baa9b13e5496c8b7f9ee Gerrit-PatchSet: 6 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: lib: migration: add response helpers
automat...@ovirt.org has posted comments on this change. Change subject: lib: migration: add response helpers .. Patch Set 4: * Update tracker::IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3']) -- To view, visit https://gerrit.ovirt.org/42795 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I6ccefec4f1bebcb2ca64f0bc9f6b9e9954dbf04c Gerrit-PatchSet: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Marcin Mirecki Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: migration: move progress update into an helper
automat...@ovirt.org has posted comments on this change. Change subject: migration: move progress update into an helper .. Patch Set 3: * Update tracker::IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3']) -- To view, visit https://gerrit.ovirt.org/42797 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie9707fcc492a394f8d7aebd57482c44b7b5a703d Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: migration: de-entangle migration stat reporting
automat...@ovirt.org has posted comments on this change. Change subject: migration: de-entangle migration stat reporting .. Patch Set 3: * Update tracker::IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3']) -- To view, visit https://gerrit.ovirt.org/42796 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I6d745bacddf80c54354a2f7ec2d290dfd3b12d03 Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: migration: use the response module
automat...@ovirt.org has posted comments on this change. Change subject: migration: use the response module .. Patch Set 8: * Update tracker::IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3']) -- To view, visit https://gerrit.ovirt.org/40492 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I6d56b93d02467b4e95a29a9a98d4a09b22e64573 Gerrit-PatchSet: 8 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: migration: build new reason on success
automat...@ovirt.org has posted comments on this change. Change subject: migration: build new reason on success .. Patch Set 3: * Update tracker::IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3']) -- To view, visit https://gerrit.ovirt.org/42800 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ic42b70be6f5a5406e23916e16a678dd8caf0f500 Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: virt: enhance/fix migration.SourceThread.stop()
automat...@ovirt.org has posted comments on this change. Change subject: virt: enhance/fix migration.SourceThread.stop() .. Patch Set 6: * Update tracker::IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3']) -- To view, visit https://gerrit.ovirt.org/40520 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I0ab50fc789dde969b2fb9ab969241ed4ad12545c Gerrit-PatchSet: 6 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: migration: replace boolean with proper Events
automat...@ovirt.org has posted comments on this change. Change subject: migration: replace boolean with proper Events .. Patch Set 3: * Update tracker::IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3']) -- To view, visit https://gerrit.ovirt.org/42887 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I02639749ab884c3542c4d29715756044d777f738 Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: migration: make status attribute private
automat...@ovirt.org has posted comments on this change. Change subject: migration: make status attribute private .. Patch Set 3: * Update tracker::IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3']) -- To view, visit https://gerrit.ovirt.org/42794 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I33884c0e4942f9e2d7ef93b939f33b2e6147ca62 Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: migration: add helper to add status field
automat...@ovirt.org has posted comments on this change. Change subject: migration: add helper to add status field .. Patch Set 3: * Update tracker::IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3']) -- To view, visit https://gerrit.ovirt.org/42798 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I047705e2f45221ad3a7527ca6b018dc6d0f3368e Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: migration: return plain response on error
automat...@ovirt.org has posted comments on this change. Change subject: migration: return plain response on error .. Patch Set 3: * Update tracker::IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3']) -- To view, visit https://gerrit.ovirt.org/42799 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ifafa5de4db317ed0e2f735eca6d944042e19c4e1 Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: test: customize destination for an event
Piotr Kliczewski has posted comments on this change. Change subject: test: customize destination for an event .. Patch Set 1: Verified+1 Verified by running local build. -- To view, visit https://gerrit.ovirt.org/43168 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ia9acf87891b3a45b5af5361d6fb0bc6882962eae Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Piotr Kliczewski Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Piotr Kliczewski Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: jsonrpc: handle timeout in callMethod
Piotr Kliczewski has posted comments on this change. Change subject: jsonrpc: handle timeout in callMethod .. Patch Set 1: Verified+1 Verified by running local build. -- To view, visit https://gerrit.ovirt.org/43167 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I28dd33c9ff64962747b5592f9e373d3f81112186 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Piotr Kliczewski Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Piotr Kliczewski Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: jsonrpc: handle timeout in callMethod
automat...@ovirt.org has posted comments on this change. Change subject: jsonrpc: handle timeout in callMethod .. Patch Set 1: * Update tracker::IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3']) -- To view, visit https://gerrit.ovirt.org/43167 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I28dd33c9ff64962747b5592f9e373d3f81112186 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Piotr Kliczewski Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: stomp: standalone client
Piotr Kliczewski has posted comments on this change. Change subject: stomp: standalone client .. Patch Set 3: Verified+1 Verified by running local build. -- To view, visit https://gerrit.ovirt.org/42850 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I8af6272679e115cf8eb80a14227476b59812581c Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Piotr Kliczewski Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Piotr Kliczewski Gerrit-Reviewer: Yaniv Bronhaim Gerrit-Reviewer: Yeela Kaplan Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: test: customize destination for an event
automat...@ovirt.org has posted comments on this change. Change subject: test: customize destination for an event .. Patch Set 1: * Update tracker::IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3']) -- To view, visit https://gerrit.ovirt.org/43168 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ia9acf87891b3a45b5af5361d6fb0bc6882962eae Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Piotr Kliczewski Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: jsonrpc: handle timeout in callMethod
Piotr Kliczewski has uploaded a new change for review. Change subject: jsonrpc: handle timeout in callMethod .. jsonrpc: handle timeout in callMethod When we do not receive any response from the server JsonRpcClient fails when calling callMethod. Change-Id: I28dd33c9ff64962747b5592f9e373d3f81112186 Signed-off-by: pkliczewski --- M lib/yajsonrpc/__init__.py 1 file changed, 8 insertions(+), 4 deletions(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/67/43167/1 diff --git a/lib/yajsonrpc/__init__.py b/lib/yajsonrpc/__init__.py index 68af242..bcb33e0 100644 --- a/lib/yajsonrpc/__init__.py +++ b/lib/yajsonrpc/__init__.py @@ -328,11 +328,15 @@ self._eventcbs = [] def callMethod(self, methodName, params=[], rid=None): -resp = self.call(JsonRpcRequest(methodName, params, rid))[0] -if resp.error: -raise JsonRpcError(resp.error['code'], resp.error['message']) +responses = self.call(JsonRpcRequest(methodName, params, rid)) +if not responses: +raise JsonRpcNoResponseError(methodName) +response = responses[0] +if response.error: +raise JsonRpcError(response.error['code'], + response.error['message']) -return resp.result +return response.result def call(self, *reqs, **kwargs): call = self.call_async(*reqs) -- To view, visit https://gerrit.ovirt.org/43167 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I28dd33c9ff64962747b5592f9e373d3f81112186 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Piotr Kliczewski ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: stomp: standalone client
automat...@ovirt.org has posted comments on this change. Change subject: stomp: standalone client .. Patch Set 3: * Update tracker::IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3']) -- To view, visit https://gerrit.ovirt.org/42850 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I8af6272679e115cf8eb80a14227476b59812581c Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Piotr Kliczewski Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Piotr Kliczewski Gerrit-Reviewer: Yaniv Bronhaim Gerrit-Reviewer: Yeela Kaplan Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: test: customize destination for an event
Piotr Kliczewski has uploaded a new change for review. Change subject: test: customize destination for an event .. test: customize destination for an event During test run we need to customize destinations where events are sent. Change-Id: Ia9acf87891b3a45b5af5361d6fb0bc6882962eae Signed-off-by: pkliczewski --- M tests/jsonRpcHelper.py 1 file changed, 6 insertions(+), 4 deletions(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/68/43168/1 diff --git a/tests/jsonRpcHelper.py b/tests/jsonRpcHelper.py index 4705a42..b456bff 100644 --- a/tests/jsonRpcHelper.py +++ b/tests/jsonRpcHelper.py @@ -47,8 +47,9 @@ class FakeClientIf(object): log = logging.getLogger("FakeClientIf") -def __init__(self, binding): +def __init__(self, binding, dest): self.threadLocal = threading.local() +self.dest = dest self.json_binding = binding self.irs = True self.gluster = None @@ -72,11 +73,12 @@ def _send_notification(self, message): server = self.json_binding.reactor.server -server.send(message, LEGACY_SUBSCRIPTION_ID_RESPONSE) +server.send(message, self.dest) @contextmanager -def constructAcceptor(log, ssl, jsonBridge): +def constructAcceptor(log, ssl, jsonBridge, + dest=LEGACY_SUBSCRIPTION_ID_RESPONSE): sslctx = DEAFAULT_SSL_CONTEXT if ssl else None reactor = Reactor() acceptor = MultiProtocolAcceptor( @@ -88,7 +90,7 @@ json_binding = BindingJsonRpc(jsonBridge, defaultdict(list), 60) json_binding.start() -cif = FakeClientIf(json_binding) +cif = FakeClientIf(json_binding, dest) xml_binding = BindingXMLRPC(cif, cif.log) xml_binding.start() -- To view, visit https://gerrit.ovirt.org/43168 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Ia9acf87891b3a45b5af5361d6fb0bc6882962eae Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Piotr Kliczewski ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: virt: Safe device type check for console device
Shmuel Leib Melamud has abandoned this change. Change subject: virt: Safe device type check for console device .. Abandoned Needs to be fixed in Engine and API spec corrected. -- To view, visit https://gerrit.ovirt.org/42881 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: abandon Gerrit-Change-Id: I6a0c1adc6c3b2ab8d50b6e3ac712ce2dcf04cc6c Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Shmuel Leib Melamud Gerrit-Reviewer: Arik Hadas Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Martin Polednik Gerrit-Reviewer: Michal Skrivanek Gerrit-Reviewer: Omer Frenkel Gerrit-Reviewer: Shmuel Leib Melamud Gerrit-Reviewer: Shmuel Melamud Gerrit-Reviewer: automat...@ovirt.org ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: virt: Safe device type check for console device
automat...@ovirt.org has posted comments on this change. Change subject: virt: Safe device type check for console device .. Patch Set 1: * Update tracker::#1233825::OK -- To view, visit https://gerrit.ovirt.org/42881 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I6a0c1adc6c3b2ab8d50b6e3ac712ce2dcf04cc6c Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Shmuel Leib Melamud Gerrit-Reviewer: Arik Hadas Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Martin Polednik Gerrit-Reviewer: Michal Skrivanek Gerrit-Reviewer: Omer Frenkel Gerrit-Reviewer: Shmuel Leib Melamud Gerrit-Reviewer: Shmuel Melamud Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: api: 'device' property for RNG device
Shmuel Leib Melamud has posted comments on this change. Change subject: api: 'device' property for RNG device .. Patch Set 1: Verified+1 -- To view, visit https://gerrit.ovirt.org/43166 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ic1b5b8a9c908b1d3588a32fbd3e2197fe4926bfa Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Shmuel Leib Melamud Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Martin Polednik Gerrit-Reviewer: Michal Skrivanek Gerrit-Reviewer: Shmuel Leib Melamud Gerrit-Reviewer: Shmuel Melamud Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: api: 'device' property for RNG device
automat...@ovirt.org has posted comments on this change. Change subject: api: 'device' property for RNG device .. Patch Set 1: * Update tracker::#1233825::OK * Check Bug-Url::OK * Check Public Bug::#1233825::OK, public bug * Check Product::#1233825::OK, Correct product oVirt * Check TR::SKIP, not in a monitored branch (ovirt-3.5 ovirt-3.4 ovirt-3.3 ovirt-3.2) * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3']) -- To view, visit https://gerrit.ovirt.org/43166 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ic1b5b8a9c908b1d3588a32fbd3e2197fe4926bfa Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Shmuel Leib Melamud Gerrit-Reviewer: Shmuel Melamud Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: api: 'device' property for RNG device
Hello Shmuel Melamud, I'd like you to do a code review. Please visit https://gerrit.ovirt.org/43166 to review the following change. Change subject: api: 'device' property for RNG device .. api: 'device' property for RNG device Added 'device' property for RNG devices as VDSM relies on presence of this property. Change-Id: Ic1b5b8a9c908b1d3588a32fbd3e2197fe4926bfa Bug-Url: https://bugzilla.redhat.com/show_bug.cgi?id=1233825 Signed-off-by: Shmuel Melamud --- M vdsm/rpc/vdsmapi-schema.json 1 file changed, 16 insertions(+), 2 deletions(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/66/43166/1 diff --git a/vdsm/rpc/vdsmapi-schema.json b/vdsm/rpc/vdsmapi-schema.json index b16657d..6f2cf0a 100644 --- a/vdsm/rpc/vdsmapi-schema.json +++ b/vdsm/rpc/vdsmapi-schema.json @@ -3406,6 +3406,18 @@ 'specParams': 'VmSmartcardDeviceSpecParams'}} ## +# @VmRngDeviceType: +# +# An enumeration of VM rng device types +# +# @virtio: supported by qemu and virtio-rng kernel module +# +# Since: 4.18.0 +## +{'enum': 'VmRngDeviceType', + 'data': ['virtio']} + +## # @VmRngDeviceModel: # # An enumeration of VM rng device models @@ -3457,6 +3469,8 @@ # # @deviceType: The device type (always @rng) # +# @device: The the type of rng device +# # @model: The model of rng device # # @specParams: Additional device parameters @@ -3464,8 +3478,8 @@ # Since: 4.14.0 ## {'type': 'VmRngDevice', - 'data': {'deviceType': 'VmDeviceType', 'model': 'VmRngDeviceModel', - 'specParams': 'VmRngDeviceSpecParams'}} + 'data': {'deviceType': 'VmDeviceType', 'device': 'VmRngDeviceType', + 'model': 'VmRngDeviceModel', 'specParams': 'VmRngDeviceSpecParams'}} ## -- To view, visit https://gerrit.ovirt.org/43166 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Ic1b5b8a9c908b1d3588a32fbd3e2197fe4926bfa Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Shmuel Leib Melamud Gerrit-Reviewer: Shmuel Melamud ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Removing support in sysv and upstart
Dima Kuznetsov has posted comments on this change. Change subject: Removing support in sysv and upstart .. Patch Set 13: Code-Review+1 -- To view, visit https://gerrit.ovirt.org/40726 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ib9af14b3d78badc5250042508d25f294dc514a2d Gerrit-PatchSet: 13 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Yaniv Bronhaim Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Dima Kuznetsov Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Yaniv Bronhaim Gerrit-Reviewer: Yeela Kaplan Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: vdsm hooks: report hook stderr to Engine if it fails an action
Francesco Romani has posted comments on this change. Change subject: vdsm hooks: report hook stderr to Engine if it fails an action .. Patch Set 12: Code-Review-1 (9 comments) This version is a bit worse, and maybe I've contributed to it with cryptic suggestions. See inside for more verbose comments. https://gerrit.ovirt.org/#/c/42592/12/lib/vdsm/response.py File lib/vdsm/response.py: Line 40: "message": message or status["message"] Line 41: } Line 42: } Line 43: Line 44: spurious change, please remove Line 45: def error_raw(code, message): Line 46: return { Line 47: "status": { Line 48: "code": code, Line 49: "message": message Line 50: } Line 51: } Line 52: Line 53: same Line 54: def is_failure(res): https://gerrit.ovirt.org/#/c/42592/12/vdsm/API.py File vdsm/API.py: Line 355: v = self._cif.vmContainer[self._UUID] Line 356: except KeyError: Line 357: return errCode['noVM'] Line 358: Line 359: status = v.get_last_migration_status() why is not v.migrateStatus() not good enough? Line 360: # check for errors Line 361: if response.is_failure(status): Line 362: return status Line 363: # if no error, return progress Line 585: params['vmId'] = self._UUID Line 586: result = self.create(params) Line 587: if result['status']['code']: Line 588: self.log.debug('Migration create - Failed') Line 589: return result please split these three in a (trivial) separate patch, which will prepare the soil to use response module inside API.py, and which will get almost instanteously my +1 Line 590: Line 591: v = self._cif.vmContainer.get(self._UUID) Line 592: Line 593: try: Line 591: v = self._cif.vmContainer.get(self._UUID) Line 592: Line 593: try: Line 594: if not v.waitForMigrationDestinationPrepare(): Line 595: return response.errCode['createErr'] perhaps you mean response.error('createErr') ? Line 596: except hooks.HookError as e: Line 597: self.log.debug('Destination VM creation failed due to hook' + Line 598:' error:' + str(e)) Line 599: return response.error('migrateErr', 'Destination hook failed: ' + https://gerrit.ovirt.org/#/c/42592/12/vdsm/virt/migration.py File vdsm/virt/migration.py: Line 323 Line 324 Line 325 Line 326 Line 327 spurious change, please remove Line 103: def hibernating(self): Line 104: return self._mode == MODE_FILE Line 105: Line 106: def get_last_status(self): Line 107: return self.lastStatus Well, I feel that I've very likely derailed your work. Let me fix that. It is true that we do have plans to de-entangle the mess of status reporting in the migration.SourceThread ( https://gerrit.ovirt.org/#/c/42794/2 , https://gerrit.ovirt.org/#/c/42796/2 , ) but this code isn't close enough to get merged - days away at least. So, for the time being, there is no point in being forward looking: just access self.status wherever you need to. Please note that SourceThread.status and SourceThread._vm.lastStatus are two wildly different beasts. SourceThread.status it is a response value, the same value you get from the functions in the response module. SourceThread._vm.lastStatus is an enumerated value (aka one integer), see vdsm/virt/vmstatus.py I *think* you need SourceThread.status in your patch. Line 108: Line 109: def getStat(self): Line 110: """ Line 111: Get the status of the migration. Line 301: del self._vm.conf['_migrationParams'] Line 302: SourceThread._ongoingMigrations.release() Line 303: except hooks.HookError as e: Line 304: self._last_status = response.error('migrateErr', 'Hook error ' + Line 305:'during migration: ' + str(e)) you probably want to do self.status = response.error(...) (which looks OK) Line 306: self._recover(str(e)) Line 307: self.log.error('Hook error during migration: ' + str(e)) Line 308: Line 309: except Exception as e: https://gerrit.ovirt.org/#/c/42592/12/vdsm/virt/vm.py File vdsm/virt/vm.py: Line 1542: finally: Line 1543: self._guestCpuLock.release() Line 1544: Line 1545: def get_last_migration_status(self): Line 1546: return self._migrationSourceThread.get_last_status() I think we don't need this (or please explain why we do) Line 1547: Line 1548: def migrateStatus(self): Line 1549: return self._migrationSourceThread.getStat() Line 1550: -- To view, visit https://gerrit.ovirt.org/42592 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ibe2d5eb52cf2c8855d9d7d5e
Change in vdsm[master]: xmlrpc: do not block forever when waiting on incoming messages
Piotr Kliczewski has posted comments on this change. Change subject: xmlrpc: do not block forever when waiting on incoming messages .. Patch Set 3: Yes, we need to solve it for 3.5. Will try to provide as simple as possible fix. -- To view, visit https://gerrit.ovirt.org/42915 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I295e3099ea06d786741164e1f240f4662631bf8a Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Piotr Kliczewski Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Piotr Kliczewski Gerrit-Reviewer: Yaniv Bronhaim Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[ovirt-3.5]: network: allow custom bondOption
Petr Horáček has posted comments on this change. Change subject: network: allow custom bondOption .. Patch Set 2: Verified+1 Passes functional network tests and net* unit tests without a regression. -- To view, visit https://gerrit.ovirt.org/43149 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I9ace532d959bc3195a8a92b4536bdc0062bc7d1d Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: ovirt-3.5 Gerrit-Owner: Petr Horáček Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Petr Horáček Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[ovirt-3.5]: network: allow custom bondOption
automat...@ovirt.org has posted comments on this change. Change subject: network: allow custom bondOption .. Patch Set 2: * Update tracker::#1234867::OK * Check Bug-Url::OK * Check Public Bug::#1234867::OK, public bug * Check Product::#1234867::OK, Correct product oVirt * Check TR::#1234867::OK, correct target release 3.5.5 * Check merged to previous::OK, change not open on any previous branch -- To view, visit https://gerrit.ovirt.org/43149 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I9ace532d959bc3195a8a92b4536bdc0062bc7d1d Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: ovirt-3.5 Gerrit-Owner: Petr Horáček Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: tests: network: when using bonding, attach at least 2 slaves
automat...@ovirt.org has posted comments on this change. Change subject: tests: network: when using bonding, attach at least 2 slaves .. Patch Set 7: * Update tracker::#1234867::OK * Set MODIFIED::bug 1234867#1234867IGNORE, not all related patches are closed, check 43149 -- To view, visit https://gerrit.ovirt.org/41840 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I1763bc3409aa7da91dcd2e3b21dd8540870138ef Gerrit-PatchSet: 7 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Petr Horáček Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Petr Horáček Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: tests: network: when using bonding, attach at least 2 slaves
Dan Kenigsberg has submitted this change and it was merged. Change subject: tests: network: when using bonding, attach at least 2 slaves .. tests: network: when using bonding, attach at least 2 slaves In some tests we attach just one slave to a bonding, it works with native linux bonding, but does not allow us to use existing for OVS bonding. Change-Id: I1763bc3409aa7da91dcd2e3b21dd8540870138ef Signed-off-by: Petr Horacek Bug-Url: https://bugzilla.redhat.com/show_bug.cgi?id=1234867 Reviewed-on: https://gerrit.ovirt.org/41840 Reviewed-by: Dan Kenigsberg Continuous-Integration: Jenkins CI --- M tests/functional/networkTests.py 1 file changed, 34 insertions(+), 26 deletions(-) Approvals: Jenkins CI: Passed CI tests Petr Horáček: Verified Dan Kenigsberg: Looks good to me, approved -- To view, visit https://gerrit.ovirt.org/41840 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: merged Gerrit-Change-Id: I1763bc3409aa7da91dcd2e3b21dd8540870138ef Gerrit-PatchSet: 7 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Petr Horáček Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Petr Horáček Gerrit-Reviewer: automat...@ovirt.org ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: xmlrpc: do not block forever when waiting on incoming messages
Nir Soffer has posted comments on this change. Change subject: xmlrpc: do not block forever when waiting on incoming messages .. Patch Set 3: This is issue must be solve also in 3.5 - blocking shutdown is breaking our shutdown code. For a minimal fix: - replace the queue in the connected server with a deque and a lock (see lib/vdsm/executor.py) - override shutdown, clear the request queue, and add a sentinel - in get_request, shutdown when getting the sentinel. Later we can drop the while XMLRPC server and replace it with something much simpler. -- To view, visit https://gerrit.ovirt.org/42915 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I295e3099ea06d786741164e1f240f4662631bf8a Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Piotr Kliczewski Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Piotr Kliczewski Gerrit-Reviewer: Yaniv Bronhaim Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: build: Change dependency on glusterfs-cli and glusterfs-fuse
Nir Soffer has posted comments on this change. Change subject: build: Change dependency on glusterfs-cli and glusterfs-fuse .. Patch Set 3: (1 comment) https://gerrit.ovirt.org/#/c/43097/3//COMMIT_MSG Commit Message: Line 13: info api to implement mounting gluster backup servers Line 14: (https://gerrit.ovirt.org/41858). Line 15: However, requiring these dependencies fails vdsm build on ppc64le. Line 16: This patch revert requiring glusterfs-cli and glusterfs-fuse Line 17: dependencies as were originally defined. > Please describe the result of this patch - what will happen if someone is t This is a revert of 766a54776555e6303b4ceae593c41bbfb0c764ae. Please recreated this patch using git revert, which will add the necessary metadata in the commit message, or add this metadata to this patch. Line 18: Line 19: Change-Id: I4cf424e7055ad071561d037b65972dde3b1b0b8b -- To view, visit https://gerrit.ovirt.org/43097 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I4cf424e7055ad071561d037b65972dde3b1b0b8b Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Ala Hino Gerrit-Reviewer: Allon Mureinik Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Eyal Edri Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Michal Skrivanek Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Yaniv Bronhaim Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[ovirt-3.5]: net: bonding: validate options based on pre-dumped ones
Petr Horáček has posted comments on this change. Change subject: net: bonding: validate options based on pre-dumped ones .. Patch Set 2: Verified+1 Passes functional network tests and net* unit tests without a regression. -- To view, visit https://gerrit.ovirt.org/43148 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I03fce903280d5eda817e713a51c8079dab9d7649 Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: ovirt-3.5 Gerrit-Owner: Petr Horáček Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Ondřej Svoboda Gerrit-Reviewer: Petr Horáček Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[ovirt-3.5]: net: bonding: validate options based on pre-dumped ones
automat...@ovirt.org has posted comments on this change. Change subject: net: bonding: validate options based on pre-dumped ones .. Patch Set 2: * Update tracker::#1234867::OK * Check Bug-Url::OK * Check Public Bug::#1234867::ERROR, private bug * Check Public Bug::WARN, no public bug url found * Check merged to previous::OK, change not open on any previous branch -- To view, visit https://gerrit.ovirt.org/43148 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I03fce903280d5eda817e713a51c8079dab9d7649 Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: ovirt-3.5 Gerrit-Owner: Petr Horáček Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Ondřej Svoboda Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Removing support in sysv and upstart
automat...@ovirt.org has posted comments on this change. Change subject: Removing support in sysv and upstart .. Patch Set 13: * Update tracker::IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3']) -- To view, visit https://gerrit.ovirt.org/40726 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ib9af14b3d78badc5250042508d25f294dc514a2d Gerrit-PatchSet: 13 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Yaniv Bronhaim Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Dima Kuznetsov Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Yaniv Bronhaim Gerrit-Reviewer: Yeela Kaplan Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: build: Change dependency on glusterfs-cli and glusterfs-fuse
Allon Mureinik has posted comments on this change. Change subject: build: Change dependency on glusterfs-cli and glusterfs-fuse .. Patch Set 3: Verified+1 Code-Review+1 -- To view, visit https://gerrit.ovirt.org/43097 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I4cf424e7055ad071561d037b65972dde3b1b0b8b Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Ala Hino Gerrit-Reviewer: Allon Mureinik Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Eyal Edri Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Michal Skrivanek Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Yaniv Bronhaim Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: virt: Handle disconnects only if endpoints match
Vinzenz Feenstra has posted comments on this change. Change subject: virt: Handle disconnects only if endpoints match .. Patch Set 1: (2 comments) https://gerrit.ovirt.org/#/c/43141/1//COMMIT_MSG Commit Message: Line 26: This patch addresses the above mentioned issue by performing a comparison Line 27: of the remote endpoint that disconnected and remembering only initiated Line 28: endpoints. In case that multiple connections are established, like in case Line 29: of spice, the last initiated channel would be remembered and when that one Line 30: disconnects, it will cause the disconnect handling to proceed. > Question: what happens if disconnections happens in random order? Those aren't clients which are disconnecting, those are the different channels for spice. Only one disconnect should never happen alone. They all should disconnect since the spice sessions would be over. Line 31: Line 32: Due to some bug in the handling of graphic events in libvirt, SPICE Line 33: connections currently do not include the port. A fix for this has been Line 34: posted upstream and should eventually become available. Therefore, the https://gerrit.ovirt.org/#/c/43141/1/vdsm/virt/vm.py File vdsm/virt/vm.py: Line 351: self._vcpuLimit = None Line 352: self._vcpuTuneInfo = {} Line 353: self._numaInfo = {} Line 354: self._vmJobs = None Line 355: self._clientPort = '' > do we want to record all the connections in the future? We only need to know about one, I wouldn't know why we want to record all. The only reason is to allow multiple connections on the same vm, which is currently not supported. Line 356: Line 357: def _get_lastStatus(self): Line 358: # note that we don't use _statusLock here. One of the reasons is the Line 359: # non-obvious recursive locking in the following flow: -- To view, visit https://gerrit.ovirt.org/43141 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Idfb3441407aa6e7bc31d37304bce0076e12bb1a3 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Vinzenz Feenstra Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Vinzenz Feenstra Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: adding the StorageDomain.upgradeVersion verb
automat...@ovirt.org has posted comments on this change. Change subject: adding the StorageDomain.upgradeVersion verb .. Patch Set 6: * Update tracker::IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3']) -- To view, visit https://gerrit.ovirt.org/42478 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Id8fb87c6e04e48072ec90d07a0f4c7a61411a808 Gerrit-PatchSet: 6 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Liron Aravot Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: adding StorageDomain.activateHsm()
automat...@ovirt.org has posted comments on this change. Change subject: adding StorageDomain.activateHsm() .. Patch Set 1: * Update tracker::IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3']) -- To view, visit https://gerrit.ovirt.org/43153 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I68eccd300b8ba0cc45b2485f978eb3ae367c16e2 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Liron Aravot Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: adding StorageDomain.getStoredVmsInfo
automat...@ovirt.org has posted comments on this change. Change subject: adding StorageDomain.getStoredVmsInfo .. Patch Set 8: * Update tracker::IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3']) -- To view, visit https://gerrit.ovirt.org/42129 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie2896897bccc493457695181436a55f688bb596f Gerrit-PatchSet: 8 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Liron Aravot Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: adding StorageDomain.removeVmData()
automat...@ovirt.org has posted comments on this change. Change subject: adding StorageDomain.removeVmData() .. Patch Set 1: * Update tracker::IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3']) -- To view, visit https://gerrit.ovirt.org/43154 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ic53b19a43850341ee488b50fbb339fdf68c1a048 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Liron Aravot Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: adding StorageDomain.updateVmData()
automat...@ovirt.org has posted comments on this change. Change subject: adding StorageDomain.updateVmData() .. Patch Set 4: * Update tracker::IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3']) -- To view, visit https://gerrit.ovirt.org/42930 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Id79e7d6375e60216d1eb9a58e5a5c087db98625f Gerrit-PatchSet: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Liron Aravot Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: adding StorageDomain.getBackedUpVmsInfo
Liron Aravot has restored this change. Change subject: adding StorageDomain.getBackedUpVmsInfo .. Restored -- To view, visit https://gerrit.ovirt.org/42129 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: restore Gerrit-Change-Id: Ie2896897bccc493457695181436a55f688bb596f Gerrit-PatchSet: 7 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Liron Aravot Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: automat...@ovirt.org ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[ovirt-3.5]: network: allow custom bondOption
automat...@ovirt.org has posted comments on this change. Change subject: network: allow custom bondOption .. Patch Set 1: * Update tracker::#1234867::OK * Check Bug-Url::OK * Check Public Bug::#1234867::OK, public bug * Check Product::#1234867::OK, Correct product oVirt * Check TR::#1234867::ERROR, wrong target release for stable branch, should match ^3.[54321].* * Check merged to previous::OK, change not open on any previous branch -- To view, visit https://gerrit.ovirt.org/43149 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I9ace532d959bc3195a8a92b4536bdc0062bc7d1d Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: ovirt-3.5 Gerrit-Owner: Petr Horáček Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[ovirt-3.5]: network: allow custom bondOption
Hello Dan Kenigsberg, I'd like you to do a code review. Please visit https://gerrit.ovirt.org/43149 to review the following change. Change subject: network: allow custom bondOption .. network: allow custom bondOption Allow 'custom' in bond options. This value will be accepted by Bond.validateOptions method but it will not be passed to configurator. Thanks to this patch, users will be allowed to pass custom properties to setupNetworks hooks. This path was reverted in past because of malfunction, now it should work, there is a new test for sure. Change-Id: I9ace532d959bc3195a8a92b4536bdc0062bc7d1d Signed-off-by: Petr Horáček Bug-Url: https://bugzilla.redhat.com/show_bug.cgi?id=1234867 Reviewed-on: https://gerrit.ovirt.org/40882 Reviewed-by: Dan Kenigsberg Continuous-Integration: Dan Kenigsberg --- M lib/vdsm/netinfo.py M tests/functional/networkTests.py M vdsm/network/models.py 3 files changed, 37 insertions(+), 3 deletions(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/49/43149/1 diff --git a/lib/vdsm/netinfo.py b/lib/vdsm/netinfo.py index 01f25c7..987a40c 100644 --- a/lib/vdsm/netinfo.py +++ b/lib/vdsm/netinfo.py @@ -73,8 +73,9 @@ _BONDING_FAILOVER_MODES = frozenset(('1', '3')) _BONDING_LOADBALANCE_MODES = frozenset(('0', '2', '4', '5', '6')) _EXCLUDED_BONDING_ENTRIES = frozenset(( -'slaves', 'active_slave', 'mii_status', 'queue_id', 'ad_aggregator', -'ad_num_ports', 'ad_actor_key', 'ad_partner_key', 'ad_partner_mac' +'slaves', 'active_slave', 'mii_status', 'queue_id', 'ad_aggregator', +'ad_num_ports', 'ad_actor_key', 'ad_partner_key', 'ad_partner_mac', +'custom' )) _IFCFG_ZERO_SUFFIXED = frozenset( ('IPADDR0', 'GATEWAY0', 'PREFIX0', 'NETMASK0')) diff --git a/tests/functional/networkTests.py b/tests/functional/networkTests.py index 37979ba..3a3d6f9 100644 --- a/tests/functional/networkTests.py +++ b/tests/functional/networkTests.py @@ -2337,3 +2337,22 @@ self.assertNetworkDoesntExist(NETWORK_NAME) self.assertBondDoesntExist(BONDING_NAME, [nic]) self.vdsm_net.save_config() + +@cleanupNet +@ValidateRunningAsRoot +def test_setupNetworks_bond_with_custom_option(self): +with dummyIf(2) as nics: +status, msg = self.setupNetworks( +{}, +{BONDING_NAME: {'nics': nics, +'options': 'custom=foo mode=balance-rr'}}, +NOCHK, test_kernel_config=False) +self.assertEqual(status, SUCCESS, msg) +self.assertBondExists(BONDING_NAME, nics) +opts = self.vdsm_net.config.bonds.get(BONDING_NAME).get('options') +self.assertEquals(opts, 'mode=balance-rr') + +status, msg = self.setupNetworks( +{}, {BONDING_NAME: {'remove': True}}, NOCHK) +self.assertEqual(status, SUCCESS, msg) +self.assertBondDoesntExist(BONDING_NAME, nics) diff --git a/vdsm/network/models.py b/vdsm/network/models.py index 9608937..aacf462 100644 --- a/vdsm/network/models.py +++ b/vdsm/network/models.py @@ -235,9 +235,23 @@ _netinfo=_netinfo)) return slaves +@staticmethod +def remove_custom_option(options): +""" 'custom' property should not be exposed to configurator, it is +only for purpose of hooks """ +d_opts = dict(opt.split('=', 1) for opt in options.split()) +if d_opts.pop('custom', None) is not None: +options = ' '.join(['%s=%s' % (key, value) +for key, value in d_opts.items()]) +return options + @classmethod def objectivize(cls, name, configurator, options, nics, mtu, _netinfo, destroyOnMasterRemoval=None): + +if options: +options = cls.remove_custom_option(options) + if nics: # New bonding or edit bonding. slaves = cls._objectivizeSlaves(name, configurator, _nicSort(nics), mtu, _netinfo) @@ -299,7 +313,7 @@ for option in bondingOptions.split(): key, _ = option.split('=', 1) -if key not in defaults: +if key not in defaults and key != 'custom': raise ConfigNetworkError(ne.ERR_BAD_BONDING, '%r is not a ' 'valid bonding option' % key) -- To view, visit https://gerrit.ovirt.org/43149 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I9ace532d959bc3195a8a92b4536bdc0062bc7d1d Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: ovirt-3.5 Gerrit-Owner: Petr Horáček Gerrit-Reviewer: Dan Kenigsberg ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: snapshot: Start the snapshot module
Michal Skrivanek has posted comments on this change. Change subject: snapshot: Start the snapshot module .. Patch Set 6: (3 comments) >> Michal, Martin: Vm class is way too big, and we cannot test it (in a sane >> way). We should not add new code to this class. If I embed the code in snapshot.py into vm.py, I cannot test it. The idea is to move also the snapshot code from vm py in a similar way, so code can be easily tested. testability is an issue of current code too. Again, I'd prefer to have a single flow (and test) for a single action, regardless underlying storage technology. regarding separation, I think snapshot is too small thing to factor out. We moved out significant pieces already, but I think our goal is not to go all the way to 0. I'm not opposed to refactoring, but I would ideally envision a function within vm.py, which would handle both "regular" and ceph snapshot. If it looks ugly then we can delegate the guest agent call to the current guest agent class (and that way encapsulate the fact whether we want to use our agent or qemu-ga or whatever, at least it's clear from high level that it is a guest agent "thing") https://gerrit.ovirt.org/#/c/43018/6//COMMIT_MSG Commit Message: Line 10: include only the GuestAgent class, responsible for freezing and thawing Line 11: guest file systems during a snapshot. Line 12: Line 13: Freezing or thawing filesystems can fail because of many reasons; Line 14: libvirt driver does not support freezing, guest agent is not installed > Libvirt drivers (e.g. qemu) may implement domainFSFreese/Thaw or not. If fu I think this assumption is so built in that we should not even mention another possibility:) Line 15: or not running or guest agent failure. We detect the first two cases, and Line 16: treat any other libvirt error as guest agent failure. Line 17: Line 18: The errors are reported back to engine so it can handled them correctly. https://gerrit.ovirt.org/#/c/43018/6/tests/vmfakelib.py File tests/vmfakelib.py: Line 244: return self._downtimes Line 245: Line 246: @recorded Line 247: def fsFreeze(self, mountpoints=None, flags=0): Line 248: self._failIfRequested() > Because we want to simulate error so we can test how code handle them. ok, I guess it's harmless as the global snapshot action succeeds once there is a need it can be extended... Line 249: return 3 Line 250: Line 251: @recorded Line 252: def fsThaw(self, mountpoints=None, flags=0): https://gerrit.ovirt.org/#/c/43018/6/vdsm/virt/snapshot.py File vdsm/virt/snapshot.py: Line 51: """ Line 52: self._log.info("Freezing guest filesystems") Line 53: Line 54: try: Line 55: frozen = self._dom.fsFreeze() > Ok, silly me just missed that ovirt-guest-agent has an hard-dep on qemu-ga, it does. But to date we intentionally never communicated with qemu-ga, but went through ovirt-ga. There are few semi-exceptions, like current snapshot, where we pass a flag which in turn calls qemu-ga, but that's encapsulated fully. Line 56: except libvirt.libvirtError as e: Line 57: self._log.warning("Unable to freeze guest filesystems: %s", e) Line 58: code = e.get_error_code() Line 59: if code == libvirt.VIR_ERR_AGENT_UNRESPONSIVE: -- To view, visit https://gerrit.ovirt.org/43018 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I8e4c54f461039de99823c3b80a10be0a960c4273 Gerrit-PatchSet: 6 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Nir Soffer Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Allon Mureinik Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Daniel Erez Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Maor Lipchuk Gerrit-Reviewer: Martin Polednik Gerrit-Reviewer: Michal Skrivanek Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Vinzenz Feenstra Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[ovirt-3.5]: net: bonding: validate options based on pre-dumped ones
automat...@ovirt.org has posted comments on this change. Change subject: net: bonding: validate options based on pre-dumped ones .. Patch Set 1: * update_tracker: OK * Check Bug-Url::OK * Check Public Bug::#1234867::OK, public bug * Check Product::#1234867::OK, Correct product oVirt * Check TR::#1234867::OK, correct target release 3.5.5 * Check merged to previous::OK, change not open on any previous branch -- To view, visit https://gerrit.ovirt.org/43148 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I03fce903280d5eda817e713a51c8079dab9d7649 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: ovirt-3.5 Gerrit-Owner: Petr Horáček Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Ondřej Svoboda Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[ovirt-3.5]: net: bonding: validate options based on pre-dumped ones
Hello Ondřej Svoboda, Dan Kenigsberg, I'd like you to do a code review. Please visit https://gerrit.ovirt.org/43148 to review the following change. Change subject: net: bonding: validate options based on pre-dumped ones .. net: bonding: validate options based on pre-dumped ones The removed code created a bond device only to check if the provided options are recongnized by the kernel. This was buggy, as the options depends on the bonding mode. It was also redundant, as we already depend on pre-dumped maps of bond options per bond mode, so we can (and should) use it here, too. Change-Id: I03fce903280d5eda817e713a51c8079dab9d7649 Signed-off-by: Dan Kenigsberg Reviewed-on: https://gerrit.ovirt.org/41616 Continuous-Integration: Jenkins CI Reviewed-by: Ondřej Svoboda Reviewed-by: Petr Horáček Bug-Url: https://bugzilla.redhat.com/show_bug.cgi?id=1234867 --- M tests/netmodelsTests.py M vdsm/network/api.py M vdsm/network/models.py 3 files changed, 58 insertions(+), 41 deletions(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/48/43148/1 diff --git a/tests/netmodelsTests.py b/tests/netmodelsTests.py index a2a3ee3..548f01b 100644 --- a/tests/netmodelsTests.py +++ b/tests/netmodelsTests.py @@ -28,8 +28,6 @@ from network.models import _nicSort from testrunner import VdsmTestCase as TestCaseBase -from testValidation import ValidateRunningAsRoot -from nose.plugins.skip import SkipTest from monkeypatch import MonkeyPatch @@ -80,18 +78,17 @@ self.assertEqual(Bond.validateName('bond11'), None) self.assertEqual(Bond.validateName('bond11128421982'), None) -@ValidateRunningAsRoot +@MonkeyPatch(netinfo, 'BONDING_DEFAULTS', netinfo.BONDING_DEFAULTS + if os.path.exists(netinfo.BONDING_DEFAULTS) + else '../vdsm/bonding-defaults.json') def testValidateBondingOptions(self): -if not os.path.exists(netinfo.BONDING_MASTERS): -raise SkipTest("bonding kernel module could not be found.") - opts = 'mode=802.3ad miimon=150' badOpts = 'foo=bar badopt=one' with self.assertRaises(errors.ConfigNetworkError) as cne: -Bond.validateOptions('bond0', badOpts) +Bond.validateOptions(badOpts) self.assertEqual(cne.exception.errCode, errors.ERR_BAD_BONDING) -self.assertEqual(Bond.validateOptions('bond0', opts), None) +self.assertEqual(Bond.validateOptions(opts), None) def testIsIpValid(self): addresses = ('10.18.1.254', '10.50.25.177', '250.0.0.1', @@ -122,7 +119,6 @@ self.assertEqual(IPv4.validateNetmask(mask), None) @MonkeyPatch(netinfo, 'getMtu', lambda *x: 1500) -@MonkeyPatch(Bond, 'validateOptions', lambda *x: 0) def testTextualRepr(self): _netinfo = {'networks': {}, 'vlans': {}, 'nics': ['testnic1', 'testnic2'], diff --git a/vdsm/network/api.py b/vdsm/network/api.py index f829732..52bbf45 100755 --- a/vdsm/network/api.py +++ b/vdsm/network/api.py @@ -470,7 +470,7 @@ for bonding, bondingAttrs in bondings.iteritems(): Bond.validateName(bonding) if 'options' in bondingAttrs: -Bond.validateOptions(bonding, bondingAttrs['options']) +Bond.validateOptions(bondingAttrs['options']) if bondingAttrs.get('remove', False): continue diff --git a/vdsm/network/models.py b/vdsm/network/models.py index 8e33950..9608937 100644 --- a/vdsm/network/models.py +++ b/vdsm/network/models.py @@ -17,9 +17,7 @@ # Refer to the README and COPYING files for full details of the license # from collections import namedtuple -from contextlib import contextmanager import logging -import os import re import socket import struct @@ -179,8 +177,11 @@ for slave in slaves: slave.master = self self.slaves = slaves -self.options = options or '' -self.validateOptions(name, self.options) +if options is None: +self.options = 'mode=802.3ad miimon=150' +else: +self.validateOptions(options) +self.options = self._reorderOptions(options) self.destroyOnMasterRemoval = destroyOnMasterRemoval super(Bond, self).__init__(name, configurator, ipconfig, mtu) @@ -270,36 +271,56 @@ name) @classmethod -def validateOptions(cls, bonding, bondingOptions): +def validateOptions(cls, bondingOptions): 'Example: BONDING_OPTS="mode=802.3ad miimon=150"' -with cls._validationBond(bonding) as bond: -try: -for option in bondingOptions.split(): -key, _ = option.split('=') -if not os.path.exists('/sys/class/net/%s/bonding/%s' % - (bond, key)): -raise ConfigNetworkError(ne.ERR_BAD_BONDING, '%r i