Change in vdsm[master]: fcoe hook: enable service

2016-07-08 Thread pzhukov
Pavel Zhukov has posted comments on this change.

Change subject: fcoe hook: enable service
..


Patch Set 1:

(1 comment)

https://gerrit.ovirt.org/#/c/60311/1/vdsm_hooks/fcoe/fcoe_before_network_setup.py
File vdsm_hooks/fcoe/fcoe_before_network_setup.py:

PS1, Line 216: ret, _, err = hooking.execCmd(['/bin/systemctl', 
'enable', 'fcoe'])
As I wrote in email I don't like this approach. 
1) node users (not node-next but "legacy" one). You will have  ret = 0 here but 
service will not be enabled because of systemd 
https://bugzilla.redhat.com/show_bug.cgi?id=1238659. That's why we have lldpad 
and fcoe services enabled by default 
https://bugzilla.redhat.com/show_bug.cgi?id=1237212 so this code absolutely 
useless for ovirt-node/RHEVH
2) "normal" Fedora/CentOS users will not install the hook if it's not needed so 
it's better to enable services using post installation section of the hook (or 
even fcoe-utils package like hundreds of services do).


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Icb3e7c45620ebb4ee80dfe333119a3d0766491ea
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dan Kenigsberg 
Gerrit-Reviewer: Elad Ben Aharon 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Pavel Zhukov 
Gerrit-Reviewer: Petr Horáček 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org


Change in vdsm[master]: hooks: Fix import of netconfpersistence

2016-06-22 Thread pzhukov
Pavel Zhukov has uploaded a new change for review.

Change subject: hooks: Fix import of netconfpersistence
..

hooks: Fix import of netconfpersistence

Change-Id: Ia1060367ff4edca560b41f8b9c82c69172d85971
Signed-off-by: Pavel Zhukov 
---
M vdsm_hooks/fcoe/fcoe_before_network_setup.py
1 file changed, 1 insertion(+), 1 deletion(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/52/59652/1

diff --git a/vdsm_hooks/fcoe/fcoe_before_network_setup.py 
b/vdsm_hooks/fcoe/fcoe_before_network_setup.py
index b2de9b9..c507c07 100755
--- a/vdsm_hooks/fcoe/fcoe_before_network_setup.py
+++ b/vdsm_hooks/fcoe/fcoe_before_network_setup.py
@@ -15,7 +15,7 @@
 
 import hooking
 from vdsm import utils
-from vdsm.netconfpersistence import RunningConfig
+from vdsm.network.netconfpersistence import RunningConfig
 
 
 FCOE_CONFIG_DIR = '/etc/fcoe/'


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: Ia1060367ff4edca560b41f8b9c82c69172d85971
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Pavel Zhukov 
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org


Change in vdsm[ovirt-3.6]: net: Ignore noqueue queuing discipline

2016-05-18 Thread pzhukov
Pavel Zhukov has posted comments on this change.

Change subject: net: Ignore noqueue queuing discipline
..


Patch Set 1: Code-Review+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I80fba89a4fe9ddf705f2e4be8e56cab1af909f01
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-3.6
Gerrit-Owner: Dan Kenigsberg 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Edward Haas 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Pavel Zhukov 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[ovirt-3.6]: hooks: Add fcoe hook

2016-05-17 Thread pzhukov
Pavel Zhukov has posted comments on this change.

Change subject: hooks: Add fcoe hook
..


Patch Set 2:

DEBUG util.py:393:  
https://repos.fedorapeople.org/repos/openstack/openstack-kilo/el7/repodata/repomd.xml:
 [Errno 14] HTTPS Error 404 - Not Found

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Iad2faed7205ca08801132df072b469dbe781318c
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-3.6
Gerrit-Owner: Pavel Zhukov 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Pavel Zhukov 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[ovirt-3.6]: hooks: Add fcoe hook

2016-05-17 Thread pzhukov
Hello Dan Kenigsberg,

I'd like you to do a code review.  Please visit

https://gerrit.ovirt.org/57553

to review the following change.

Change subject: hooks: Add fcoe hook
..

hooks: Add fcoe hook

This hook is used to enable fcoe on one or several network interfaces

Change-Id: Iad2faed7205ca08801132df072b469dbe781318c
Bug-Url: https://bugzilla.redhat.com/1334745
Signed-off-by: Pavel Zhukov 
Reviewed-on: https://gerrit.ovirt.org/55029
Reviewed-by: Dan Kenigsberg 
Continuous-Integration: Jenkins CI
---
M configure.ac
M vdsm.spec.in
M vdsm_hooks/Makefile.am
A vdsm_hooks/fcoe/Makefile.am
A vdsm_hooks/fcoe/README
A vdsm_hooks/fcoe/fcoe_before_network_setup.py
6 files changed, 216 insertions(+), 0 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/53/57553/1

diff --git a/configure.ac b/configure.ac
index 7c19fd9..343b474 100644
--- a/configure.ac
+++ b/configure.ac
@@ -369,6 +369,7 @@
vdsm_hooks/extnet/Makefile
vdsm_hooks/fakevmstats/Makefile
vdsm_hooks/faqemu/Makefile
+   vdsm_hooks/fcoe/Makefile
vdsm_hooks/fileinject/Makefile
vdsm_hooks/floppy/Makefile
vdsm_hooks/hostusb/Makefile
diff --git a/vdsm.spec.in b/vdsm.spec.in
index 9b29ee9..9207d83 100644
--- a/vdsm.spec.in
+++ b/vdsm.spec.in
@@ -614,6 +614,16 @@
 VDSM hook used for applying IPv6 configuration through custom network
 properties
 
+%package hook-fcoe
+Summary:Hook to enable FCoE support
+BuildArch:  noarch
+Requires:   %{name} = %{version}-%{release}
+Requires:   fcoe-utils
+
+%description hook-fcoe
+VDSM hook used for configure specified NICs as FCoE interface through custom
+network properties
+
 %if 0%{?with_gluster}
 %package gluster
 Summary:Gluster Plugin for VDSM
@@ -1299,6 +1309,10 @@
 %defattr(-, root, root, -)
 %{_libexecdir}/%{vdsm_name}/hooks/before_vm_start/60_checkimages
 
+%files hook-fcoe
+%defattr(-, root, root, -)
+%{_libexecdir}/%{vdsm_name}/hooks/before_network_setup/50_fcoe
+
 %files hook-diskunmap
 %defattr(-, root, root, -)
 %{_libexecdir}/%{vdsm_name}/hooks/before_vm_start/50_diskunmap
diff --git a/vdsm_hooks/Makefile.am b/vdsm_hooks/Makefile.am
index 44856f1..6d0b2e0 100644
--- a/vdsm_hooks/Makefile.am
+++ b/vdsm_hooks/Makefile.am
@@ -40,6 +40,7 @@
extnet \
fileinject \
fakevmstats \
+   fcoe \
floppy \
hostusb \
httpsisoboot \
diff --git a/vdsm_hooks/fcoe/Makefile.am b/vdsm_hooks/fcoe/Makefile.am
new file mode 100644
index 000..f23c7c7
--- /dev/null
+++ b/vdsm_hooks/fcoe/Makefile.am
@@ -0,0 +1,30 @@
+#
+# Copyright 2016 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
+#
+
+EXTRA_DIST = \
+   fcoe_before_network_setup.py
+
+install-data-local:
+   $(MKDIR_P) $(DESTDIR)$(vdsmhooksdir)/before_network_setup
+   $(INSTALL_SCRIPT) $(srcdir)/fcoe_before_network_setup.py \
+   $(DESTDIR)$(vdsmhooksdir)/before_network_setup/50_fcoe
+
+uninstall-local:
+   $(RM) $(DESTDIR)$(vdsmhooksdir)/before_network_setup/50_fcoe
diff --git a/vdsm_hooks/fcoe/README b/vdsm_hooks/fcoe/README
new file mode 100644
index 000..49ac99d
--- /dev/null
+++ b/vdsm_hooks/fcoe/README
@@ -0,0 +1,15 @@
+fcoe vdsm hook
+=
+This hook allow to configure one or more NICs as FCoE interface(s)
+
+Intallation:
+* Use engine-config to append the appropriate custom property:
+  engine-config -s UserDefinedNetworkCustomProperties='fcoe=^(true|false)$'
+
+* Verify that custom property was added:
+  engine-config -g UserDefinedNetworkCustomProperties
+
+Usage:
+* Define virtual network for FCoE
+* Under "setup host networks" attach (drag) it to interface.
+* Click small "Edit" button and add fcoe property with value "true"
diff --git a/vdsm_hooks/fcoe/fcoe_before_network_setup.py 
b/vdsm_hooks/fcoe/fcoe_before_network_setup.py
new file mode 100755
index 000..fddf166
--- /dev/null
+++ b/vdsm_hooks/fcoe/fcoe_before_network_setup.py
@@ -0,0 +1,155 @@
+#!/bin/env python
+"""
+FCoE hook:
+   if fcoe = true custom networks was specified enable FCoE for specified NIC
+syntax:
+   fcoe = true|false
+"""
+
+
+import os
+import traceback

Change in vdsm[master]: hooks: Add fcoe hook

2016-05-15 Thread pzhukov
Pavel Zhukov has posted comments on this change.

Change subject: hooks: Add fcoe hook
..


Patch Set 33: Verified+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Iad2faed7205ca08801132df072b469dbe781318c
Gerrit-PatchSet: 33
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Pavel Zhukov 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Douglas Schilling Landgraf 
Gerrit-Reviewer: Edward Haas 
Gerrit-Reviewer: Eyal Edri 
Gerrit-Reviewer: Fabian Deutsch 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Pavel Zhukov 
Gerrit-Reviewer: Petr Horáček 
Gerrit-Reviewer: Yaniv Kaul 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: hooks: Add fcoe hook

2016-05-15 Thread pzhukov
Pavel Zhukov has posted comments on this change.

Change subject: hooks: Add fcoe hook
..


Patch Set 27: Verified+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Iad2faed7205ca08801132df072b469dbe781318c
Gerrit-PatchSet: 27
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Pavel Zhukov 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Douglas Schilling Landgraf 
Gerrit-Reviewer: Edward Haas 
Gerrit-Reviewer: Eyal Edri 
Gerrit-Reviewer: Fabian Deutsch 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Pavel Zhukov 
Gerrit-Reviewer: Petr Horáček 
Gerrit-Reviewer: Yaniv Kaul 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: hooks: Add fcoe hook

2016-05-11 Thread pzhukov
Pavel Zhukov has posted comments on this change.

Change subject: hooks: Add fcoe hook
..


Patch Set 26:

Seems irrelevant to this change:
08:16:48 InvalidCall: Attempt to call function: > with arguments: 
({'vmID': '773adfc7-10d4-4e60-b700-3272ee1871f9'},) error: migrationCreate() 
takes exactly 3 arguments (2 given)

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Iad2faed7205ca08801132df072b469dbe781318c
Gerrit-PatchSet: 26
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Pavel Zhukov 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Douglas Schilling Landgraf 
Gerrit-Reviewer: Edward Haas 
Gerrit-Reviewer: Eyal Edri 
Gerrit-Reviewer: Fabian Deutsch 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Pavel Zhukov 
Gerrit-Reviewer: Petr Horáček 
Gerrit-Reviewer: Yaniv Kaul 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: hooks: Add fcoe hook

2016-05-11 Thread pzhukov
Pavel Zhukov has posted comments on this change.

Change subject: hooks: Add fcoe hook
..


Patch Set 25: Verified+1

Verified as per #23.
Fixed pep8/pylint stuff

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Iad2faed7205ca08801132df072b469dbe781318c
Gerrit-PatchSet: 25
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Pavel Zhukov 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Douglas Schilling Landgraf 
Gerrit-Reviewer: Edward Haas 
Gerrit-Reviewer: Eyal Edri 
Gerrit-Reviewer: Fabian Deutsch 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Pavel Zhukov 
Gerrit-Reviewer: Petr Horáček 
Gerrit-Reviewer: Yaniv Kaul 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: hooks: Add fcoe hook

2016-05-11 Thread pzhukov
Pavel Zhukov has posted comments on this change.

Change subject: hooks: Add fcoe hook
..


Patch Set 24: Verified+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Iad2faed7205ca08801132df072b469dbe781318c
Gerrit-PatchSet: 24
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Pavel Zhukov 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Douglas Schilling Landgraf 
Gerrit-Reviewer: Edward Haas 
Gerrit-Reviewer: Eyal Edri 
Gerrit-Reviewer: Fabian Deutsch 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Pavel Zhukov 
Gerrit-Reviewer: Petr Horáček 
Gerrit-Reviewer: Yaniv Kaul 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: hooks: Add fcoe hook

2016-05-11 Thread pzhukov
Pavel Zhukov has posted comments on this change.

Change subject: hooks: Add fcoe hook
..


Patch Set 23: Verified+1

Verified using:
1) Added fcoe network to NIC: check configuration exists
2) Removed fcoe attribute: check file gone
3) added fcoe attribute: check file exists
4) removed network: check file gone

Did NOT checked "moved" scenario once I don't have enough NICs at that machine

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Iad2faed7205ca08801132df072b469dbe781318c
Gerrit-PatchSet: 23
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Pavel Zhukov 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Douglas Schilling Landgraf 
Gerrit-Reviewer: Edward Haas 
Gerrit-Reviewer: Eyal Edri 
Gerrit-Reviewer: Fabian Deutsch 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Pavel Zhukov 
Gerrit-Reviewer: Petr Horáček 
Gerrit-Reviewer: Yaniv Kaul 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: hooks: Add fcoe hook

2016-05-10 Thread pzhukov
Pavel Zhukov has posted comments on this change.

Change subject: hooks: Add fcoe hook
..


Patch Set 22: Verified-1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Iad2faed7205ca08801132df072b469dbe781318c
Gerrit-PatchSet: 22
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Pavel Zhukov 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Douglas Schilling Landgraf 
Gerrit-Reviewer: Edward Haas 
Gerrit-Reviewer: Eyal Edri 
Gerrit-Reviewer: Fabian Deutsch 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Pavel Zhukov 
Gerrit-Reviewer: Petr Horáček 
Gerrit-Reviewer: Yaniv Kaul 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: hooks: Add fcoe hook

2016-05-10 Thread pzhukov
Pavel Zhukov has posted comments on this change.

Change subject: hooks: Add fcoe hook
..


Patch Set 22:

Seems like jenkins is broken.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Iad2faed7205ca08801132df072b469dbe781318c
Gerrit-PatchSet: 22
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Pavel Zhukov 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Douglas Schilling Landgraf 
Gerrit-Reviewer: Edward Haas 
Gerrit-Reviewer: Eyal Edri 
Gerrit-Reviewer: Fabian Deutsch 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Pavel Zhukov 
Gerrit-Reviewer: Petr Horáček 
Gerrit-Reviewer: Yaniv Kaul 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: hooks: Add fcoe hook

2016-05-10 Thread pzhukov
Pavel Zhukov has posted comments on this change.

Change subject: hooks: Add fcoe hook
..


Patch Set 21:

(3 comments)

https://gerrit.ovirt.org/#/c/55029/21/vdsm_hooks/fcoe/fcoe_before_network_setup.py
File vdsm_hooks/fcoe/fcoe_before_network_setup.py:

PS21, Line 46: )
> if utils.isOvirtNode():
Do we really need this? 
Utils.py contains:
try:
from ovirt.node.utils.fs import Config
persist = Config().persist
unpersist = Config().unpersist
except ImportError:
persist = lambda name: None
unpersist = lambda name: None


PS21, Line 55: ):
> please check if it's ovirt-node and unpersist the file before trying to rem
ok


PS21, Line 56: )
> if utils.isOvirtNode():
same as before


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Iad2faed7205ca08801132df072b469dbe781318c
Gerrit-PatchSet: 21
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Pavel Zhukov 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Douglas Schilling Landgraf 
Gerrit-Reviewer: Edward Haas 
Gerrit-Reviewer: Eyal Edri 
Gerrit-Reviewer: Fabian Deutsch 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Pavel Zhukov 
Gerrit-Reviewer: Petr Horáček 
Gerrit-Reviewer: Yaniv Kaul 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: hooks: Add fcoe hook

2016-05-10 Thread pzhukov
Pavel Zhukov has posted comments on this change.

Change subject: hooks: Add fcoe hook
..


Patch Set 20:

(9 comments)

https://gerrit.ovirt.org/#/c/55029/20/vdsm_hooks/fcoe/fcoe_before_network_setup.py
File vdsm_hooks/fcoe/fcoe_before_network_setup.py:

Line 8: 
Line 9: 
Line 10: import os
Line 11: import traceback
Line 12: from shutil import copyfile
> Dont import functions from module, it make it hard to understand what is th
ok
Line 13: 
Line 14: import six
Line 15: 
Line 16: import hooking


Line 20: 
Line 21: FCOE_CONFIG_DIR = '/etc/fcoe/'
Line 22: # TODO this is default for EL7, Fedora.
Line 23: # Debian, EL6 etc should be checked
Line 24: FCOE_DEFAULT_CONFIG = 'cfg-ethx'
> +1
ack
Line 25: 
Line 26: 
Line 27: def _has_fcoe_param(net_attr):
Line 28: """ Check if fcoe parameter was specified as custom network 
property """


Line 26: 
Line 27: def _has_fcoe_param(net_attr):
Line 28: """ Check if fcoe parameter was specified as custom network 
property """
Line 29: return (net_attr.get('custom')
Line 30: and hooking.tobool(net_attr['custom'].get('fcoe')))
> You can simplify this to:
Ack. thanks
Line 31: 
Line 32: 
Line 33: def _configure(interface):
Line 34: """ Enable FCoE on specified interface by coping default 
configuration """


Line 32: 
Line 33: def _configure(interface):
Line 34: """ Enable FCoE on specified interface by coping default 
configuration """
Line 35: if interface is None:
Line 36: return
> if bond network is removed (for example).
moved checks level up
Line 37: filename = os.path.join(FCOE_CONFIG_DIR, 'cfg-%s' % interface)
Line 38: if not os.path.exists(filename):
Line 39: copyfile(os.path.join(FCOE_CONFIG_DIR, FCOE_DEFAULT_CONFIG),
Line 40:  filename)


Line 33: def _configure(interface):
Line 34: """ Enable FCoE on specified interface by coping default 
configuration """
Line 35: if interface is None:
Line 36: return
Line 37: filename = os.path.join(FCOE_CONFIG_DIR, 'cfg-%s' % interface)
> We repeat this code everywhere, lets make a helper to return the path:
Two times only but ok
Line 38: if not os.path.exists(filename):
Line 39: copyfile(os.path.join(FCOE_CONFIG_DIR, FCOE_DEFAULT_CONFIG),
Line 40:  filename)
Line 41: utils.persist(filename)


Line 35: if interface is None:
Line 36: return
Line 37: filename = os.path.join(FCOE_CONFIG_DIR, 'cfg-%s' % interface)
Line 38: if not os.path.exists(filename):
Line 39: copyfile(os.path.join(FCOE_CONFIG_DIR, FCOE_DEFAULT_CONFIG),
> If you use my suggestion about  FCOE_DEFAULT_CONFIG, we can do now:
ack
Line 40:  filename)
Line 41: utils.persist(filename)
Line 42: 
Line 43: 


Line 37: filename = os.path.join(FCOE_CONFIG_DIR, 'cfg-%s' % interface)
Line 38: if not os.path.exists(filename):
Line 39: copyfile(os.path.join(FCOE_CONFIG_DIR, FCOE_DEFAULT_CONFIG),
Line 40:  filename)
Line 41: utils.persist(filename)
> What if the file exists but the contents are incorrect? for example, the co
Nope. We have pending task to change content of the file. it will be needed for 
some FCoE adapters. We cannot protect file from being overwritten by user 
anyway.
Line 42: 
Line 43: 
Line 44: def _unconfigure(interface):
Line 45: """ Remove config file for specified interface """


Line 63: 
Line 64: config = RunningConfig()
Line 65: for net, net_attr in six.iteritems(config.networks):
Line 66: if _has_fcoe_param(net_attr):
Line 67: nic = configured[net].get('nic')
> +1
ack. copy=paste mistake
Line 68: if nic is None:
Line 69: hooking.exit_hook("Failed to configure fcoe \
Line 70: on %s with no physical nic" % (net))
Line 71: else:


Line 83: changed_non_fcoe[net] = net_attr.get('nic')
Line 84: 
Line 85: for net, net_nic in six.iteritems(removed_networks):
Line 86: if net in configured:
Line 87: _unconfigure(config.networks[net].get('nic'))
> You man it can be None, and we still need to unconfigure it?
We have to unconfigure "existing" network which has name listed in "removed" 
while nic is none in "removed" json. I don't know why it works like that
Line 88: 
Line 89: for net, net_nic in six.iteritems(changed_non_fcoe):
Line 90: if net in configured:
Line 91: _unconfigure(net_nic)


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Iad2faed7205ca08801132df072b469dbe781318c
Gerrit-PatchSet: 20
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Pavel Zhukov 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Edward Haas 
Gerrit-Reviewer: Fabian Deutsch 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerri

Change in vdsm[master]: hooks: Add fcoe hook

2016-05-09 Thread pzhukov
Pavel Zhukov has posted comments on this change.

Change subject: hooks: Add fcoe hook
..


Patch Set 20: Verified-1

(1 comment)

https://gerrit.ovirt.org/#/c/55029/20/vdsm_hooks/fcoe/fcoe_before_network_setup.py
File vdsm_hooks/fcoe/fcoe_before_network_setup.py:

Line 50: utils.rmFile(filename)
Line 51: utils.unpersist(filename)
Line 52: 
Line 53: 
Line 54: def main():
> There is too may variable here with unclear names, and too much logic.
For existed networks it's ok but that about removed and changed_ dicts? I don't 
think three iterations is better than one
Line 55: """
Line 56: Create lists of running networks
Line 57: and networks to be (un)configured as FCoE or removed.
Line 58: """


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Iad2faed7205ca08801132df072b469dbe781318c
Gerrit-PatchSet: 20
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Pavel Zhukov 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Edward Haas 
Gerrit-Reviewer: Fabian Deutsch 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Pavel Zhukov 
Gerrit-Reviewer: Petr Horáček 
Gerrit-Reviewer: Yaniv Kaul 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: hooks: Add fcoe hook

2016-04-11 Thread pzhukov
Pavel Zhukov has posted comments on this change.

Change subject: hooks: Add fcoe hook
..


Patch Set 20:

(11 comments)

https://gerrit.ovirt.org/#/c/55029/18/vdsm_hooks/fcoe/fcoe_before_network_setup.py
File vdsm_hooks/fcoe/fcoe_before_network_setup.py:

Line 38: if not os.path.exists(filename):
Line 39: copyfile(os.path.join(FCOE_CONFIG_DIR, FCOE_DEFAULT_CONFIG),
Line 40:  filename)
Line 41: utils.persist(filename)
Line 42: 
> We like to see nicely wrapped text in comments/docstrings.
Nir,
Thank you for explanation.
I know about gq. 
I prefer to have lines warped using "logic" rules not "counters". 

For example: 
# IOError can be raised here is FCOE_DEFAULT_CONFIG doesn't exist or we're not
# able to copy file

Doesn't look good, do it? But if it's so important I can close my eyes and use 
automatic tool.
Line 43: 
Line 44: def _unconfigure(interface):
Line 45: """ Remove config file for specified interface """
Line 46: if interface is None:


PS18, Line 90: if net in configured:
> Since master. On 3.6, we need to use tobool.
I'd prefer to keep it just because it's more safe. Design mistake (not used 
value) doesn't guarantee it will not used in the future. In case if behavior 
will be changed you will silently break the hook with ("remove" : false) and 
fcoe will be removed for all interfaces with a lot of fun for users.


https://gerrit.ovirt.org/#/c/55029/20/vdsm_hooks/fcoe/fcoe_before_network_setup.py
File vdsm_hooks/fcoe/fcoe_before_network_setup.py:

Line 14: import six
Line 15: 
Line 16: import hooking
Line 17: from vdsm import utils
Line 18: from vdsm.netconfpersistence import RunningConfig
> Same, import modules instead of names from the modules.
Well. I'm not good programmer but I can use grep and I checked how more 
experience colleagues write the code:
14 modules use "from vdsm.netconfpersistence import RunningConfig" and only 2 
use "import netconfpersistence" one of them has name "legacy_switch.py". 

The reason of importing entire module in these modules is very simple:
return netconfpersistence
Line 19: 
Line 20: 
Line 21: FCOE_CONFIG_DIR = '/etc/fcoe/'
Line 22: # TODO this is default for EL7, Fedora.


Line 32: 
Line 33: def _configure(interface):
Line 34: """ Enable FCoE on specified interface by coping default 
configuration """
Line 35: if interface is None:
Line 36: return
> Why interface can be None? This kind of code hides error in the calling cod
if bond network is removed (for example).
Line 37: filename = os.path.join(FCOE_CONFIG_DIR, 'cfg-%s' % interface)
Line 38: if not os.path.exists(filename):
Line 39: copyfile(os.path.join(FCOE_CONFIG_DIR, FCOE_DEFAULT_CONFIG),
Line 40:  filename)


Line 43: 
Line 44: def _unconfigure(interface):
Line 45: """ Remove config file for specified interface """
Line 46: if interface is None:
Line 47: return
> Same
See above
Line 48: filename = os.path.join(FCOE_CONFIG_DIR, 'cfg-%s' % (interface))
Line 49: if os.path.exists(filename):
Line 50: utils.rmFile(filename)
Line 51: utils.unpersist(filename)


Line 65: for net, net_attr in six.iteritems(config.networks):
Line 66: if _has_fcoe_param(net_attr):
Line 67: nic = configured[net].get('nic')
Line 68: if nic is None:
Line 69: hooking.exit_hook("Failed to configure fcoe \
> your string still has redundant spaces.
Ack. I spent too much time with this long string :(
Line 70: on %s with no physical nic" % (net))
Line 71: else:
Line 72: configured[net] = nic
Line 73: 


Line 75: changed_networks = setup_nets_config['request']['networks']
Line 76: 
Line 77: for net, net_attr in changed_networks:
Line 78: if _has_fcoe_param(config.networks[net]):
Line 79: changed_fcoe[net] = net_attr.get('nic')
> This is the root cause of having to handle None in _configure and _unconfig
We have to handle None anyway.
For example removed network can have None
Line 80: elif hooking.tobool(net_attr.get('remove')):
Line 81: removed_networks[net] = config.networks[net].get('nic')
Line 82: else:
Line 83: changed_non_fcoe[net] = net_attr.get('nic')


Line 77: for net, net_attr in changed_networks:
Line 78: if _has_fcoe_param(config.networks[net]):
Line 79: changed_fcoe[net] = net_attr.get('nic')
Line 80: elif hooking.tobool(net_attr.get('remove')):
Line 81: removed_networks[net] = config.networks[net].get('nic')
> Is it possible to have config.networks[net] without a 'nic'?
It's possible. It's even possible to have two different 'nic' for same network 
in config.networks[net] and changed_networks[net]. As well as it's possible to 
have one of them None and second one not None.
Line 82: else:
Line 

Change in vdsm[master]: hooks: Add fcoe hook

2016-04-07 Thread pzhukov
Pavel Zhukov has posted comments on this change.

Change subject: hooks: Add fcoe hook
..


Patch Set 18:

(4 comments)

https://gerrit.ovirt.org/#/c/55029/17/vdsm_hooks/fcoe/fcoe_before_network_setup.py
File vdsm_hooks/fcoe/fcoe_before_network_setup.py:

Line 73: 
Line 74: config = RunningConfig()
Line 75: for net, net_attr in six.iteritems(config.networks):
Line 76: if _has_fcoe_param(net_attr):
Line 77: if configured[net] is None:
> you seem to have missed my two comments around here, plase see
Yes. I did. Sorry
Line 78: hooking.exit_hook("""Failed to configure fcoe
Line 79:on %s with no physical nic""" %
Line 80:   (net))
Line 81: else:


https://gerrit.ovirt.org/#/c/55029/18/vdsm_hooks/fcoe/fcoe_before_network_setup.py
File vdsm_hooks/fcoe/fcoe_before_network_setup.py:

PS18, Line 36: """
 : Enable FCoE on specified interface by coping default 
configuration
 : """
> I think you should fit in on one line:
Why should I do it? Any code convention? I can see hundreds of docstrings in 
vdsm code like
"""
One line here
"""


PS18, Line 39: # IOError can be raised here is FCOE_DEFAULT_CONFIG doesn't 
exist
 : # or we're not able to copy file
 : # Don't catch exceptions here. Propagate them to main
 : # and show traceback in supervdsm log
> i feel bad for nagging for code typography but i cannot not comment this.
I don't think so.
Do we have code convention to break the line at random place just because of 
stupid 80 characters limitation?


PS18, Line 90: elif hooking.tobool(net_attr.get('remove')):
> you don't have to use tobool, as we canonicalize values before we expose th
Since which version? I can see type 'unicode' here


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Iad2faed7205ca08801132df072b469dbe781318c
Gerrit-PatchSet: 18
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Pavel Zhukov 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Edward Haas 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Pavel Zhukov 
Gerrit-Reviewer: Petr Horáček 
Gerrit-Reviewer: Yaniv Kaul 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: hooks: Add fcoe hook

2016-04-04 Thread pzhukov
Pavel Zhukov has posted comments on this change.

Change subject: hooks: Add fcoe hook
..


Patch Set 14:

(1 comment)

https://gerrit.ovirt.org/#/c/55029/14/vdsm_hooks/fcoe/fcoe_before_network_setup.py
File vdsm_hooks/fcoe/fcoe_before_network_setup.py:

Line 67:and 
hooking.tobool(config.networks[net]['custom'].get('fcoe')):
Line 68: configured[net] = config.networks[net].get('nic')
Line 69: 
Line 70: setup_nets_config = hooking.read_json()
Line 71: changed_networks = setup_nets_config['request']['networks']
> I prefer to accept fcoe on top of a bond, with a proper warning in document
Dan,
I've checked kernel code and it clearly says:
/* Do not support for bonding device */
if (netdev->priv_flags & IFF_BONDING && netdev->flags & IFF_MASTER) {
FCOE_NETDEV_DBG(netdev, "Bonded interfaces not supported\n");
return -EOPNOTSUPP;
}
Line 72: 
Line 73: for net in changed_networks:
Line 74: if ('custom' in changed_networks[net].keys()) \
Line 75:and 
hooking.tobool(changed_networks[net]['custom'].get('fcoe')):


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Iad2faed7205ca08801132df072b469dbe781318c
Gerrit-PatchSet: 14
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Pavel Zhukov 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Edward Haas 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Pavel Zhukov 
Gerrit-Reviewer: Yaniv Kaul 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: hooks: Add fcoe hook

2016-04-02 Thread pzhukov
Pavel Zhukov has posted comments on this change.

Change subject: hooks: Add fcoe hook
..


Patch Set 14:

(4 comments)

https://gerrit.ovirt.org/#/c/55029/14/vdsm_hooks/fcoe/fcoe_before_network_setup.py
File vdsm_hooks/fcoe/fcoe_before_network_setup.py:

Line 67:and 
hooking.tobool(config.networks[net]['custom'].get('fcoe')):
Line 68: configured[net] = config.networks[net].get('nic')
Line 69: 
Line 70: setup_nets_config = hooking.read_json()
Line 71: changed_networks = setup_nets_config['request']['networks']
> what about network with bonding? we should either handle them, or fail loud
It's a little bit complicated.
First of all FCoE driver itself takes care about some bonding related 
verifications  (it checks if it runs on top of active-backup bonding device)
FCoO can be running on top of LACP bonding only if target is running on top of 
LACP as well. 
I don't think we can check both options (first one can be checked only at 
service restart step).
So I'm in doubts how to handle it properly or notify user that it's unsupported 
configuration. Raise an exception (as far as we're in before_net_setup it will 
trigger popup window in UI "Exception") and write logs like "ERROR: Running 
FCoE on top of bonded device is unsupported"?
Line 72: 
Line 73: for net in changed_networks:
Line 74: if ('custom' in changed_networks[net].keys()) \
Line 75:and 
hooking.tobool(changed_networks[net]['custom'].get('fcoe')):


PS14, Line 74: changed_networks[net].keys()
> with proper use of iteritems, this can be replaced with a much clearer net_
ack


PS14, Line 82: (net, net_nic)
> the parenthesis are not required
ack. fixed


PS14, Line 95: Remove file in _fail hook if services are not able to start
> I suppose you mean that we need to rollback recent changes?
Yes, ideally recent changes should be rolled back. I hope saving list of files 
under "/etc/fcoe/" and restoring them in case of failure should be enough. But 
in that case UI will be messed up (i.e. network has "fcoe" attribute in the UI 
but not on host).  Or raise an exception here to stop the configuration and 
allow user to deal with it?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Iad2faed7205ca08801132df072b469dbe781318c
Gerrit-PatchSet: 14
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Pavel Zhukov 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Edward Haas 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Pavel Zhukov 
Gerrit-Reviewer: Yaniv Kaul 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: hooks: Add fcoe hook

2016-04-02 Thread pzhukov
Pavel Zhukov has posted comments on this change.

Change subject: hooks: Add fcoe hook
..


Patch Set 14:

F23 Build failed but seems not related:
18:50:43 ==
18:50:43 ERROR: test_verify_schema (schemaValidationTest.SchemaValidation)
18:50:43 --
18:50:43 Traceback (most recent call last):
18:50:43   File 
"/home/jenkins/workspace/vdsm_master_check-patch-fc23-x86_64/vdsm/tests/schemaValidationTest.py",
 line 43, in test_verify_schema
18:50:43 self._validate(apiobj)
18:50:43   File 
"/home/jenkins/workspace/vdsm_master_check-patch-fc23-x86_64/vdsm/tests/schemaValidationTest.py",
 line 110, in _validate
18:50:43 method_args = bridge._getArgList(spec_class, method_name)
18:50:43 AttributeError: 'DynamicBridge' object has no attribute '_getArgList'

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Iad2faed7205ca08801132df072b469dbe781318c
Gerrit-PatchSet: 14
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Pavel Zhukov 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Edward Haas 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Pavel Zhukov 
Gerrit-Reviewer: Yaniv Kaul 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: hooks: Add fcoe hook

2016-04-02 Thread pzhukov
Pavel Zhukov has posted comments on this change.

Change subject: hooks: Add fcoe hook
..


Patch Set 13: Verified+1

(5 comments)

https://gerrit.ovirt.org/#/c/55029/3/vdsm_hooks/fcoe/fcoe_before_network_setup.py
File vdsm_hooks/fcoe/fcoe_before_network_setup.py:

Line 13: 
Line 14: from shutil import copyfile
Line 15: from vdsm import utils
Line 16: from vdsm.netconfpersistence import RunningConfig
Line 17: 
> we do not need this section, as we would backport this only to 3.6.
done
Line 18: 
Line 19: FCOE_CONFIG_DIR = '/etc/fcoe/'
Line 20: # TODO this is default for EL7, Fedora.
Line 21: # Debian, EL6 etc should be checked


Line 21: # Debian, EL6 etc should be checked
Line 22: FCOE_DEFAULT_CONFIG = 'cfg-ethx'
Line 23: 
Line 24: 
Line 25: def _configure(interface):
> file is a python keyword. please use another name. such as filename.
Switched to use RunningConfig().
Done
Line 26: """
Line 27: Enable FCoE on specified interface by coping default configuration
Line 28: """
Line 29: # IOError can be raised here is FCOE_DEFAULT_CONFIG doesn't exist


https://gerrit.ovirt.org/#/c/55029/12/vdsm_hooks/fcoe/fcoe_before_network_setup.py
File vdsm_hooks/fcoe/fcoe_before_network_setup.py:

PS12, Line 24: 
> please use triple doublequotes, as pep8 prefers.
Fixed


PS12, Line 81: 
 : for (net, net_nic) in six.iteritems(removed_networks):
 : if net in configured:
 : _unconfigure(config.networks[net].get('nic'))
 : 
 : for (net, net_nic) in six.iteritems(changed_non_fcoe):
 :  
> please use hooking.log if you must, and avoid trailing spaces.
Sorry. I forgot to remove this again. Not sure if we need this logged.


PS12, Line 88: 
> using
Fixed


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Iad2faed7205ca08801132df072b469dbe781318c
Gerrit-PatchSet: 13
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Pavel Zhukov 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Edward Haas 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Pavel Zhukov 
Gerrit-Reviewer: Yaniv Kaul 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: hooks: Add fcoe hook

2016-03-22 Thread pzhukov
Pavel Zhukov has posted comments on this change.

Change subject: hooks: Add fcoe hook
..


Patch Set 11: Verified+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Iad2faed7205ca08801132df072b469dbe781318c
Gerrit-PatchSet: 11
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Pavel Zhukov 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Edward Haas 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Pavel Zhukov 
Gerrit-Reviewer: Yaniv Kaul 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: hooks: Add fcoe hook

2016-03-22 Thread pzhukov
Pavel Zhukov has posted comments on this change.

Change subject: hooks: Add fcoe hook
..


Patch Set 10:

(3 comments)

https://gerrit.ovirt.org/#/c/55029/10/vdsm.spec.in
File vdsm.spec.in:

Line 622: VDSM hook used for applying IPv6 configuration through custom network
Line 623: properties
Line 624: 
Line 625: %package hook-fcoe
Line 626: Summary:Hook for enable fcoe support
> for -> to
fixed
Line 627: BuildArch:  noarch
Line 628: Requires:   %{name} = %{version}-%{release}
Line 629: Requires:   fcoe-utils
Line 630: 


Line 625: %package hook-fcoe
Line 626: Summary:Hook for enable fcoe support
Line 627: BuildArch:  noarch
Line 628: Requires:   %{name} = %{version}-%{release}
Line 629: Requires:   fcoe-utils
> What about lldpad?
fcoe-utils depends on it
Line 630: 
Line 631: %description hook-fcoe
Line 632: %{summary}
Line 633: 


Line 627: BuildArch:  noarch
Line 628: Requires:   %{name} = %{version}-%{release}
Line 629: Requires:   fcoe-utils
Line 630: 
Line 631: %description hook-fcoe
> thin description...
fixed
Line 632: %{summary}
Line 633: 
Line 634: %if 0%{?with_gluster_mgmt}
Line 635: %package gluster


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Iad2faed7205ca08801132df072b469dbe781318c
Gerrit-PatchSet: 10
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Pavel Zhukov 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Edward Haas 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Pavel Zhukov 
Gerrit-Reviewer: Yaniv Kaul 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: hooks: Add fcoe hook

2016-03-22 Thread pzhukov
Pavel Zhukov has posted comments on this change.

Change subject: hooks: Add fcoe hook
..


Patch Set 10: Verified+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Iad2faed7205ca08801132df072b469dbe781318c
Gerrit-PatchSet: 10
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Pavel Zhukov 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Edward Haas 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Pavel Zhukov 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: hooks: Add fcoe hook

2016-03-22 Thread pzhukov
Pavel Zhukov has posted comments on this change.

Change subject: hooks: Add fcoe hook
..


Patch Set 6: Verified+1

Tested with:
1) Add two fcoe networks
2) Move one of them to another interface
3) Changed fcoe=true to false
4) Detach fcoe networks

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Iad2faed7205ca08801132df072b469dbe781318c
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Pavel Zhukov 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Edward Haas 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Pavel Zhukov 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: hooks: Add fcoe hook

2016-03-22 Thread pzhukov
Pavel Zhukov has uploaded a new change for review.

Change subject: hooks: Add fcoe hook
..

hooks: Add fcoe hook

This hook is used to enable fcoe on one or several network interfaces

Change-Id: Iad2faed7205ca08801132df072b469dbe781318c
Bug-Url: https://bugzilla.redhat.com/1239122
Signed-off-by: Pavel Zhukov 
---
M vdsm.spec.in
A vdsm_hooks/fcoe/Makefile.am
A vdsm_hooks/fcoe/README
A vdsm_hooks/fcoe/fcoe_before_network_setup.py
4 files changed, 150 insertions(+), 0 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/29/55029/4

diff --git a/vdsm.spec.in b/vdsm.spec.in
index f348028..14bc74b 100644
--- a/vdsm.spec.in
+++ b/vdsm.spec.in
@@ -622,6 +622,15 @@
 VDSM hook used for applying IPv6 configuration through custom network
 properties
 
+%package hook-fcoe
+Summary:Hook for enable fcoe support
+BuildArch:  noarch
+Requires:   %{name} = %{version}-%{release}
+Requires:   fcoe-utils
+
+%description hook-fcoe
+%{summary}
+
 %if 0%{?with_gluster_mgmt}
 %package gluster
 Summary:Gluster Plugin for VDSM
@@ -1474,6 +1483,10 @@
 %{_libexecdir}/%{vdsm_name}/hooks/before_vm_start/50_vmfex
 %endif
 
+%files hook-ovs
+%defattr(-, root, root, -)
+%{_libexecdir}/%{vdsm_name}/hooks/before_network_setup/50_fcoe
+
 %files cli
 %defattr(-, root, root, -)
 %license COPYING
diff --git a/vdsm_hooks/fcoe/Makefile.am b/vdsm_hooks/fcoe/Makefile.am
new file mode 100644
index 000..12de02b
--- /dev/null
+++ b/vdsm_hooks/fcoe/Makefile.am
@@ -0,0 +1,30 @@
+#
+# Copyright 2011 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
+#
+
+EXTRA_DIST = \
+   fcoe_before_network_setup.py
+
+install-data-local:
+   $(MKDIR_P) $(DESTDIR)$(vdsmhooksdir)/before_network_setup
+   $(INSTALL_SCRIPT) $(srcdir)/fcoe_before_network_setup.py \
+   $(DESTDIR)$(vdsmhooksdir)/before_network_setup/50_fcoe
+
+uninstall-local:
+   $(RM) $(DESTDIR)$(vdsmhooksdir)/before_network_setup/50_fcoe
diff --git a/vdsm_hooks/fcoe/README b/vdsm_hooks/fcoe/README
new file mode 100644
index 000..d8d2bbd
--- /dev/null
+++ b/vdsm_hooks/fcoe/README
@@ -0,0 +1,18 @@
+fcoe vdsm hook
+=
+This hook allow to configure one or more NIC as FCoE interface(s)
+
+Intallation:
+* Use engine-config to append the appropriate custom property:
+  sudo engine-config -s 
UserDefinedNetworkCustomProperties='fcoe=^(true|false)$' --cver=3.6
+
+* Verify that custom property was added:
+  engine-config  -g UserDefinedNetworkCustomProperties 
   
+
+Usage:
+* Define virtual network for FCoE
+* Under "setup host networks" attach (drag) it to interface.
+* Click small "Edit" button and add fcoe property with value "true"
+
+
+
diff --git a/vdsm_hooks/fcoe/fcoe_before_network_setup.py 
b/vdsm_hooks/fcoe/fcoe_before_network_setup.py
new file mode 100755
index 000..82e5b02
--- /dev/null
+++ b/vdsm_hooks/fcoe/fcoe_before_network_setup.py
@@ -0,0 +1,89 @@
+#!/bin/env python
+
+import hooking
+import os
+import traceback
+import subprocess
+from shutil import copyfile
+from vdsm import utils
+from vdsm.netconfpersistence import RunningConfig
+
+FCOE_CONFIG_DIR = "/etc/fcoe/"
+FCOE_DEFAULT_CONFIG = "cfg-ethx"
+
+try:
+# 3.0 compat
+import libvirtconnection
+libvirtconnection
+except ImportError:
+# 3.1 compat
+from vdsm import libvirtconnection
+
+
+def _configured_fcoe_interfaces():
+files = os.listdir(FCOE_CONFIG_DIR)
+return [file[4:] for file in files
+if file != FCOE_DEFAULT_CONFIG and file.startswith("cfg-")]
+
+
+def _configure(interface):
+# IOError can be raised here is FCOE_DEFAULT_CONFIG doesn't exist
+# or we're not able to copy file
+# Don't catch exceptions here. Propagate them to main
+# and show traceback in supervdsm log
+filename = os.path.join(FCOE_CONFIG_DIR, "cfg-%s" % interface)
+if not os.path.exists(filename):
+copyfile(os.path.join(FCOE_CONFIG_DIR, FCOE_DEFAULT_CONFIG),
+ filename)
+utils.persist(filename)
+
+
+def _unconfigure(interface):
+filename = os.path.join(FCOE_CONFIG_DIR, "cfg-%s" % (interface))
+if os.path.exi

Change in vdsm[master]: hooks: Add fcoe hook

2016-03-22 Thread pzhukov
Pavel Zhukov has posted comments on this change.

Change subject: hooks: Add fcoe hook
..


Patch Set 4:

Tested with flow:
1) Add 2 fcoe networks - both configs are in place
2) Changed one network set fcoe = false - corresponding config removed
3) Removed both fcoe networks - all configs gone
4) Moved fcoe networks from one interface to another - FAIL (old interface is 
still configured as fcoe) WIP

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Iad2faed7205ca08801132df072b469dbe781318c
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Pavel Zhukov 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Edward Haas 
Gerrit-Reviewer: Pavel Zhukov 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: hooks: Add fcoe hook

2016-03-21 Thread pzhukov
Pavel Zhukov has posted comments on this change.

Change subject: hooks: Add fcoe hook
..


Patch Set 2:

I removed persist code because I was thinking it's redundant (supervdsm will 
restore config every boot (setupNetworks call) based on persistent network 
configuration anyway). But I'm not sure...

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Iad2faed7205ca08801132df072b469dbe781318c
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Pavel Zhukov 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Edward Haas 
Gerrit-Reviewer: Pavel Zhukov 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: monitor: Refresh domains only if needed

2016-02-15 Thread pzhukov
Pavel Zhukov has posted comments on this change.

Change subject: monitor: Refresh domains only if needed
..


Patch Set 1: Verified+1

Nir, Thank you for explanation.
Switched verified flag back as the original issue seems to be fixed. I hope QA 
team will test it more carefully.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I974c26fb1af3594684e00a3f9e5d64d5bfaedecd
Gerrit-PatchSet: 1
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: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Pavel Zhukov 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: monitor: Refresh domains only if needed

2016-02-15 Thread pzhukov
Pavel Zhukov has posted comments on this change.

Change subject: monitor: Refresh domains only if needed
..


Patch Set 1:

Nir, 
I've put Verified -1 because the fix makes the situation even worse.
Example:
User had X domains and everything worked fine
User added 3 more SD and everything works fine. Versions of existing storage 
domains were read and refreshing is not needed
after some time (months?) SPM was restarted and not able to contend anymore 
because vdsm has to read all vgs metadata at once (to get versions) and it's 
failed to do it because of lvm locking. Logs are rotated and nobody knows what 
happened.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I974c26fb1af3594684e00a3f9e5d64d5bfaedecd
Gerrit-PatchSet: 1
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: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Pavel Zhukov 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: monitor: Refresh domains only if needed

2016-02-15 Thread pzhukov
Pavel Zhukov has posted comments on this change.

Change subject: monitor: Refresh domains only if needed
..


Patch Set 1: Verified-1

Once SPM restarted the issue came back. vgs should be run for all SDs as the 
result host is flipped again

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I974c26fb1af3594684e00a3f9e5d64d5bfaedecd
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Pavel Zhukov 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: monitor: Refresh domains only if needed

2016-02-15 Thread pzhukov
Pavel Zhukov has posted comments on this change.

Change subject: monitor: Refresh domains only if needed
..


Patch Set 1:

It works fine until SPM re-contending. 
LVM locking storm starts again...

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I974c26fb1af3594684e00a3f9e5d64d5bfaedecd
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Pavel Zhukov 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: monitor: Refresh domains only if needed

2016-02-15 Thread pzhukov
Pavel Zhukov has posted comments on this change.

Change subject: monitor: Refresh domains only if needed
..


Patch Set 1:

Tested with reproducers from the BZ:
Added 50 SD. everything went smooth
Copy disk images between SD. OK

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I974c26fb1af3594684e00a3f9e5d64d5bfaedecd
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Pavel Zhukov 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: monitor: Refresh domains only if needed

2016-02-15 Thread pzhukov
Pavel Zhukov has posted comments on this change.

Change subject: monitor: Refresh domains only if needed
..


Patch Set 1: Verified+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I974c26fb1af3594684e00a3f9e5d64d5bfaedecd
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Pavel Zhukov 
Gerrit-Reviewer: gerrit-hooks 
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: Add support for vgamem attribute

2015-12-09 Thread pzhukov
Pavel Zhukov has posted comments on this change.

Change subject: virt: Add support for vgamem attribute
..


Patch Set 2:

(1 comment)

https://gerrit.ovirt.org/#/c/50091/2/vdsm/virt/vmdevices/core.py
File vdsm/virt/vmdevices/core.py:

Line 293: ('
Isn't something like:
 sourceAttrs[attr] = [self.specParams[attr] for attr in  ('ram', 'vgamem',) if 
attr in self.specParams]

more "Pythonic"?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ic595761ef7195ec12830dd7f057471512b5c6355
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Milan Zamazal 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Polednik 
Gerrit-Reviewer: Pavel Zhukov 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: Improve readability of _ownedIfcfg procedure

2015-06-23 Thread pzhukov
Pavel Zhukov has posted comments on this change.

Change subject: Improve readability of _ownedIfcfg procedure
..


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

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I5f6e4aaf919a5bec26d297cf01d83b9adf6a9461
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Pavel Zhukov 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Pavel Zhukov 
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]: Improve readability of _ownedIfcfg procedure

2015-06-23 Thread pzhukov
Pavel Zhukov has posted comments on this change.

Change subject: Improve readability of _ownedIfcfg procedure
..


Patch Set 1:

(1 comment)

https://gerrit.ovirt.org/#/c/42287/1/vdsm/network/configurators/ifcfg.py
File vdsm/network/configurators/ifcfg.py:

Line 499: return content.startswith(self.CONFFILE_HEADER_BASE)
Line 500: except IOError as e:
Line 501: if e.errno != os.errno.ENOENT:
Line 502: raise
Line 503: return False
> Aha, sure! I like the original 'if err: return False else raise' because it
ENOENT means "No such file or directory" and this will never happens because of 
first check os.isfile(). Please correct me if I'm wrong but this check is 
ridicolous. I've uploaded new revision
Line 504: 
Line 505: def _ownedFiles(self):
Line 506: for fpath in glob.iglob(netinfo.NET_CONF_DIR + '/*'):
Line 507: if self._ownedIfcfg(fpath):


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I5f6e4aaf919a5bec26d297cf01d83b9adf6a9461
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Pavel Zhukov 
Gerrit-Reviewer: Pavel Zhukov 
Gerrit-Reviewer: Petr Horáček 
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]: Improve readability of _ownedIfcfg procedure

2015-06-17 Thread pzhukov
Pavel Zhukov has posted comments on this change.

Change subject: Improve readability of _ownedIfcfg procedure
..


Patch Set 1:

(1 comment)

Petr, I don't understand you question. See inline comment (and original code). 
Thanks

https://gerrit.ovirt.org/#/c/42287/1/vdsm/network/configurators/ifcfg.py
File vdsm/network/configurators/ifcfg.py:

Line 499: return content.startswith(self.CONFFILE_HEADER_BASE)
Line 500: except IOError as e:
Line 501: if e.errno != os.errno.ENOENT:
Line 502: raise
Line 503: return False
> can this line ever happen?
e.error == os.errno.ENOENT ?
Line 504: 
Line 505: def _ownedFiles(self):
Line 506: for fpath in glob.iglob(netinfo.NET_CONF_DIR + '/*'):
Line 507: if self._ownedIfcfg(fpath):


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I5f6e4aaf919a5bec26d297cf01d83b9adf6a9461
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Pavel Zhukov 
Gerrit-Reviewer: Pavel Zhukov 
Gerrit-Reviewer: Petr Horáček 
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]: Improve readability of _ownedIfcfg procedure

2015-06-14 Thread pzhukov
Pavel Zhukov has posted comments on this change.

Change subject: Improve readability of _ownedIfcfg procedure
..


Patch Set 1: Verified+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I5f6e4aaf919a5bec26d297cf01d83b9adf6a9461
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Pavel Zhukov 
Gerrit-Reviewer: Pavel Zhukov 
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]: Improve readability of _ownedIfcfg procedure

2015-06-12 Thread pzhukov
Pavel Zhukov has uploaded a new change for review.

Change subject: Improve readability of _ownedIfcfg procedure
..

Improve readability of _ownedIfcfg procedure

Before _ownedIfcfg returned free possible values (True, False or None). It 
should be more clear to return
False or True explicitly

Change-Id: I5f6e4aaf919a5bec26d297cf01d83b9adf6a9461
Signed-off-by: Pavel Zhukov 
---
M vdsm/network/configurators/ifcfg.py
1 file changed, 3 insertions(+), 5 deletions(-)


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

diff --git a/vdsm/network/configurators/ifcfg.py 
b/vdsm/network/configurators/ifcfg.py
index e008994..95598dc 100644
--- a/vdsm/network/configurators/ifcfg.py
+++ b/vdsm/network/configurators/ifcfg.py
@@ -496,13 +496,11 @@
 try:
 with open(fpath) as confFile:
 content = confFile.readline()
+return content.startswith(self.CONFFILE_HEADER_BASE)
 except IOError as e:
-if e.errno == os.errno.ENOENT:
-return False
-else:
+if e.errno != os.errno.ENOENT:
 raise
-if content.startswith(self.CONFFILE_HEADER_BASE):
-return True
+return False
 
 def _ownedFiles(self):
 for fpath in glob.iglob(netinfo.NET_CONF_DIR + '/*'):


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I5f6e4aaf919a5bec26d297cf01d83b9adf6a9461
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Pavel Zhukov 
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]: Invalidate filters on HSMs before rescanning extended VG

2013-11-08 Thread pzhukov
Pavel Zhukov has posted comments on this change.

Change subject: Invalidate filters  on HSMs before rescanning extended VG
..


Patch Set 18:

Dan, I though about it (that's why I've submitted wrong patch first time 
actually). But vgck is not able to manipulate (update) LVMCache (well it can 
only if rc != 0, but it invalidates filter before second attempt anyway). The 
vgck logic works properly because It invalidates filters if vg is partial 
(retcode = 5).  

You can check the logs attached to the bugzilla. There is ~200 secs between 
getDevVisibil. and vgs calls so vgs used updated filters but (actually) 
outdated LVMCache this time.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: If1eeed1c203f2c8c73370987048565d665932299
Gerrit-PatchSet: 18
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Pavel Zhukov 
Gerrit-Reviewer: Ayal Baron 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Eduardo 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: Lee Yarwood 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Pavel Zhukov 
Gerrit-Reviewer: Saggi Mizrahi 
Gerrit-Reviewer: Sergey Gotliv 
Gerrit-Reviewer: Tomáš Došek 
Gerrit-Reviewer: Yeela Kaplan 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: Invalidate filters on HSMs before rescanning extended VG

2013-11-08 Thread pzhukov
Pavel Zhukov has posted comments on this change.

Change subject: Invalidate filters  on HSMs before rescanning extended VG
..


Patch Set 18:

Dan, 
>> On other threads, getDevVis may now report the device as visible, a vgextend 
>> command takes place, and the old domainMonitor would return a dreaded 
>> "partial" result.

It's not correct. domainMonitor uses LVMCache instance. The instance is being 
updated in _reloadvgs flow (not in vgck unfortunately, the fix might be easy). 
So it'll never marked as partial. The only thing here is that vg is extended 
but domainMonitor uses "old" cache until next invalidateSD is called.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: If1eeed1c203f2c8c73370987048565d665932299
Gerrit-PatchSet: 18
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Pavel Zhukov 
Gerrit-Reviewer: Ayal Baron 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Eduardo 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: Lee Yarwood 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Pavel Zhukov 
Gerrit-Reviewer: Saggi Mizrahi 
Gerrit-Reviewer: Sergey Gotliv 
Gerrit-Reviewer: Tomáš Došek 
Gerrit-Reviewer: Yeela Kaplan 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: Invalidate filters on HSMs before rescanning extended VG

2013-11-05 Thread pzhukov
Pavel Zhukov has posted comments on this change.

Change subject: Invalidate filters  on HSMs before rescanning extended VG
..


Patch Set 18: Verified+1

verification steps:
Extend VG with new LUN.
wait for next vgs call (domainMonitor selftest) up to 300 sec
new device should be in filter set and vgs output should have empty stderr 
output.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: If1eeed1c203f2c8c73370987048565d665932299
Gerrit-PatchSet: 18
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Pavel Zhukov 
Gerrit-Reviewer: Ayal Baron 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Eduardo 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: Lee Yarwood 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Pavel Zhukov 
Gerrit-Reviewer: Saggi Mizrahi 
Gerrit-Reviewer: Sergey Gotliv 
Gerrit-Reviewer: Tomáš Došek 
Gerrit-Reviewer: Yeela Kaplan 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: Invalidate filters on HSMs before rescanning extended VG

2013-10-30 Thread pzhukov
Pavel Zhukov has posted comments on this change.

Change subject: Invalidate filters  on HSMs before rescanning extended VG
..


Patch Set 16: Verified+1

Step to verification:
- Extend VG with new LUN.
- wait for next vgs call  (domainMonitor selftest) up to 300 sec
- new device should be in filter set and vgs output should  contain nothing in 
the stderr. (without patch applied new device is not in filter set).

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

Gerrit-MessageType: comment
Gerrit-Change-Id: If1eeed1c203f2c8c73370987048565d665932299
Gerrit-PatchSet: 16
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Pavel Zhukov 
Gerrit-Reviewer: Ayal Baron 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Eduardo 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: Lee Yarwood 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Pavel Zhukov 
Gerrit-Reviewer: Saggi Mizrahi 
Gerrit-Reviewer: Sergey Gotliv 
Gerrit-Reviewer: Tomáš Došek 
Gerrit-Reviewer: Yeela Kaplan 
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: Invalidate filters before reloading vgs

2013-10-29 Thread pzhukov
Pavel Zhukov has restored this change.

Change subject: Invalidate filters before reloading vgs
..


Restored

Checked http://gerrit.ovirt.org/#/c/17968/ It doesn't solve the problem

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

Gerrit-MessageType: restore
Gerrit-Change-Id: If1eeed1c203f2c8c73370987048565d665932299
Gerrit-PatchSet: 11
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Pavel Zhukov 
Gerrit-Reviewer: Ayal Baron 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Eduardo 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: Lee Yarwood 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Pavel Zhukov 
Gerrit-Reviewer: Sergey Gotliv 
Gerrit-Reviewer: Tomáš Došek 
Gerrit-Reviewer: Yeela Kaplan 
Gerrit-Reviewer: oVirt Jenkins CI Server
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: Invalidate filters before reloading vgs

2013-10-28 Thread pzhukov
Pavel Zhukov has abandoned this change.

Change subject: Invalidate filters before reloading vgs
..


Abandoned

Fixed in http://gerrit.ovirt.org/#/c/17968/

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

Gerrit-MessageType: abandon
Gerrit-Change-Id: If1eeed1c203f2c8c73370987048565d665932299
Gerrit-PatchSet: 11
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Pavel Zhukov 
Gerrit-Reviewer: Ayal Baron 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Eduardo 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: Lee Yarwood 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Pavel Zhukov 
Gerrit-Reviewer: Sergey Gotliv 
Gerrit-Reviewer: Tomáš Došek 
Gerrit-Reviewer: Yeela Kaplan 
Gerrit-Reviewer: oVirt Jenkins CI Server
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: Add additional vgcheck to fix vgextend workflow on HSMs.

2013-10-27 Thread pzhukov
Pavel Zhukov has posted comments on this change.

Change subject: Add additional vgcheck to fix vgextend workflow on HSMs.
..


Patch Set 9:

>> I don't understand how this fixes the related bug. If the issue was a 
>> missing pv, how running vgchk again fixes the missing pv?
The issue is stale filters
>> If the issue is stale filters, why not invalidate the filters in _reloadvgs, 
>> when we find that a vg has missing pv? Why wait until selftest()?

Because _reloadvgs uses vgs (not vgck). VGs returns zero even if PV is missed 
(see commit message) and We know nothing about missed PVs on this step. I 
thought about rebuilding filters before vgs command, but do we really want to 
do it each time for each vg?

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

Gerrit-MessageType: comment
Gerrit-Change-Id: If1eeed1c203f2c8c73370987048565d665932299
Gerrit-PatchSet: 9
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Pavel Zhukov 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Eduardo 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: Lee Yarwood 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Pavel Zhukov 
Gerrit-Reviewer: Sergey Gotliv 
Gerrit-Reviewer: Tomáš Došek 
Gerrit-Reviewer: Yeela Kaplan 
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: Add additional vgcheck to fix vgextend workflow on HSMs.

2013-10-25 Thread pzhukov
Pavel Zhukov has posted comments on this change.

Change subject: Add additional vgcheck to fix vgextend workflow on HSMs.
..


Patch Set 8:

I'm sorry for the push flooding. Added the main reason of the patch (vgextend 
workflow) to the subject.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: If1eeed1c203f2c8c73370987048565d665932299
Gerrit-PatchSet: 8
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Pavel Zhukov 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: Lee Yarwood 
Gerrit-Reviewer: Pavel Zhukov 
Gerrit-Reviewer: Tomáš Došek 
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: vgscan doesn't return nonzero return code if one or more phi...

2013-10-25 Thread pzhukov
Pavel Zhukov has uploaded a new change for review.

Change subject: vgscan doesn't return nonzero return code if one or more 
phisycal volumes are filtered (missed). We should check vg before raising of 
the exception. chkVG itself raises storageAccessError if failed, so selftest 
doesn't have to it second time.
..

vgscan doesn't return nonzero return code if one or more phisycal
volumes are filtered (missed). We should check vg before raising of
the exception. chkVG itself raises storageAccessError if failed, so
selftest doesn't have to it second time.

Change-Id: If1eeed1c203f2c8c73370987048565d665932299
Bugzilla-Url: https://bugzilla.redhat.com/1022976
Signed-off-by: Pavel Zhukov 
---
M vdsm/storage/blockSD.py
1 file changed, 1 insertion(+), 1 deletion(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/52/20552/1

diff --git a/vdsm/storage/blockSD.py b/vdsm/storage/blockSD.py
index b95be21..4b21c35 100644
--- a/vdsm/storage/blockSD.py
+++ b/vdsm/storage/blockSD.py
@@ -846,7 +846,7 @@
 self._lastUncachedSelftest = now
 lvm.chkVG(self.sdUUID)
 elif lvm.getVG(self.sdUUID).partial != lvm.VG_OK:
-raise se.StorageDomainAccessError(self.sdUUID)
+lvm.chkVG(self.sdUUID)
 
 def validate(self):
 """


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

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


Change in vdsm[master]: Add additional vgck.

2013-10-25 Thread pzhukov
Pavel Zhukov has abandoned this change.

Change subject: Add additional vgck.
..


Abandoned

need to resubmit the patch

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

Gerrit-MessageType: abandon
Gerrit-Change-Id: Ia9b54bc0563735300a688c297f0f7605dc815018
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Pavel Zhukov 
Gerrit-Reviewer: oVirt Jenkins CI Server
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: Add additional vgck.

2013-10-25 Thread pzhukov
Pavel Zhukov has uploaded a new change for review.

Change subject: Add additional vgck.
..

Add additional vgck.

vgscan doesn't return nonzero return code if one or more phisycal
volumes are filtered (missed). As a result domainMonitorThread has
outdated metadata (with filtered PVs).
Bugzilla-Url: https://bugzilla.redhat.com/1022976

Change-Id: Ia9b54bc0563735300a688c297f0f7605dc815018
Signed-off-by: Pavel Zhukov 
---
M vdsm/storage/lvm.py
1 file changed, 2 insertions(+), 0 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/46/20546/1

diff --git a/vdsm/storage/lvm.py b/vdsm/storage/lvm.py
index 0c2964e..c604e4e 100644
--- a/vdsm/storage/lvm.py
+++ b/vdsm/storage/lvm.py
@@ -410,6 +410,8 @@
 vgsFields[uuid][pvNameIdx].append(pv_name)
 for fields in vgsFields.itervalues():
 vg = makeVG(*fields)
+if not chkVG(vg.name):
+log.warning("vgck failed: %s check filters", vg.name)
 if vg.pv_count != len(vg.pv_name):
 log.error("vg %s has pv_count %d but pv_names %s",
   vg.name, vg.pv_count, vg.pv_name)


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

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


Change in vdsm[master]: dumpStorageTable: Avoid an unnecessary traceback if vdsmd is...

2013-07-24 Thread pzhukov
Pavel Zhukov has posted comments on this change.

Change subject: dumpStorageTable: Avoid an unnecessary traceback if vdsmd is 
down.
..


Patch Set 3: Looks good to me, but someone else must approve

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I6d3c0758d9580bfaf164bb3f706d91d8b0ab7f33
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Lee Yarwood 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Lee Yarwood 
Gerrit-Reviewer: Pavel Zhukov 
Gerrit-Reviewer: Tomáš Došek 
Gerrit-Reviewer: humble devassy 
Gerrit-Reviewer: oVirt Jenkins CI Server
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: dumpStorageTable: Avoid an unnecessary traceback if vdsmd is...

2013-07-24 Thread pzhukov
Pavel Zhukov has posted comments on this change.

Change subject: dumpStorageTable: Avoid an unnecessary traceback if vdsmd is 
down.
..


Patch Set 2: I would prefer that you didn't submit this

(1 inline comment)


File vdsm/dumpStorageTable.py.in
Line 317: except socket.error, e:
Line 318: print 'Unable to connect to vdsmd', str(e)
Line 319: sys.exit(1)
Line 320: 
Line 321: rc, msg = StorageTable(conn).show()
Can we wrap the entire StorageTable(vdscli.connect()).show()) call with 
try-except socket.error and remove first connect call as Dan said in the last 
comment? In that case we cann't use rc and msg anyway so they can be 
overwritten.
Line 322: if rc:
Line 323: print >>sys.stderr, msg


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I6d3c0758d9580bfaf164bb3f706d91d8b0ab7f33
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Lee Yarwood 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Lee Yarwood 
Gerrit-Reviewer: Pavel Zhukov 
Gerrit-Reviewer: Tomáš Došek 
Gerrit-Reviewer: humble devassy 
Gerrit-Reviewer: oVirt Jenkins CI Server
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: vdsmd.init: replace libvirt logs filter

2013-04-24 Thread pzhukov
Pavel Zhukov has posted comments on this change.

Change subject: vdsmd.init: replace libvirt logs filter
..


Patch Set 3: (1 inline comment)


File vdsm/vdsmd.init.in
Line 309: set_if_default $qconf save_image_format \"lzop\"
Line 310: # FIXME until we are confident with libvirt integration, let us 
have a verbose log
Line 311: set_if_default $lconf log_outputs \"1:file:/var/log/libvirtd.log\"
Line 312: set_if_default $lconf log_filters "\"3:virobject 3:virfile 
2:virnetlink \
Line 313: 3:cgroup 3:event 3:json 1:libvirt 1:util 1:qemu\""
3:virobject disables all OBJECT_(UN)REF messages from the logs. 
3:virnetlink disables all (useless)  virnetlink messages
3:cgroups does the same for cgroups messages
Line 314: 
Line 315: # If the ssl flag is set, update the libvirt and qemu 
configuration files
Line 316: # with the location for certificates and permissions.
Line 317: if [ -f $ts/certs/cacert.pem -a \


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I31218b3a6ac2f0866df03c6f84c9303d1a53f745
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Douglas Schilling Landgraf 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Douglas Schilling Landgraf 
Gerrit-Reviewer: Pavel Zhukov 
Gerrit-Reviewer: Yaniv Bronhaim 
Gerrit-Reviewer: oVirt Jenkins CI Server
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: vdsmd.init: replace libvirt logs filter

2013-04-17 Thread pzhukov
Pavel Zhukov has posted comments on this change.

Change subject: vdsmd.init: replace libvirt logs filter
..


Patch Set 3: Verified; Looks good to me, but someone else must approve

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I31218b3a6ac2f0866df03c6f84c9303d1a53f745
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Douglas Schilling Landgraf 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Douglas Schilling Landgraf 
Gerrit-Reviewer: Pavel Zhukov 
Gerrit-Reviewer: Yaniv Bronhaim 
Gerrit-Reviewer: oVirt Jenkins CI Server
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: vdsm: fix Vm cleanup routine

2013-04-07 Thread pzhukov
Pavel Zhukov has posted comments on this change.

Change subject: vdsm: fix Vm cleanup routine
..


Patch Set 2: Looks good to me, but someone else must approve

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie355022866d008a83257b1b6e3a4226a07d34c2f
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Peter V. Saveliev 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Douglas Schilling Landgraf 
Gerrit-Reviewer: Michal Skrivanek 
Gerrit-Reviewer: Pavel Zhukov 
Gerrit-Reviewer: Peter V. Saveliev 
Gerrit-Reviewer: Vered Volansky 
Gerrit-Reviewer: Vinzenz Feenstra 
Gerrit-Reviewer: oVirt Jenkins CI Server
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: vdsmd.init: replace libvirt logs filter

2013-04-05 Thread pzhukov
Pavel Zhukov has posted comments on this change.

Change subject: vdsmd.init: replace libvirt logs filter
..


Patch Set 2: Verified; Looks good to me, but someone else must approve

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I31218b3a6ac2f0866df03c6f84c9303d1a53f745
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Douglas Schilling Landgraf 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Douglas Schilling Landgraf 
Gerrit-Reviewer: Pavel Zhukov 
Gerrit-Reviewer: Yaniv Bronhaim 
Gerrit-Reviewer: oVirt Jenkins CI Server
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: Use absolute path of multipath.conf file, patch fix error 's...

2013-02-20 Thread pzhukov
Pavel Zhukov has uploaded a new change for review.

Change subject: Use absolute path of multipath.conf file, patch fix error 
'sudo: sorry, a password is required to run sudo' while rotating multipath.conf 
file.
..

Use absolute path of multipath.conf file, patch fix error
'sudo: sorry, a password is required to run sudo' while
rotating multipath.conf file.

Change-Id: Ie732400423f12b0a783bd251b4e2cc2e2409ffec
Signed-off-by: Pavel Zhukov 
---
M vdsm/sudoers.vdsm.in
1 file changed, 5 insertions(+), 4 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/26/12226/1

diff --git a/vdsm/sudoers.vdsm.in b/vdsm/sudoers.vdsm.in
index 7e53ab9..b8450c6 100644
--- a/vdsm/sudoers.vdsm.in
+++ b/vdsm/sudoers.vdsm.in
@@ -31,13 +31,14 @@
 @CAT_PATH@ /etc/multipath.conf, \
 @DD_PATH@ of=/sys/class/scsi_host/host*/scan, \
 @DD_PATH@, \
-@PERSIST_PATH@ *multipath.conf, \
+@PERSIST_PATH@ /etc/multipath.conf, \
 @PERSIST_PATH@ /var/log/vdsm/backup/*, \
-@UNPERSIST_PATH@ *multipath.conf, \
+@UNPERSIST_PATH@ /etc/multipath.conf, \
 @UNPERSIST_PATH@ /var/log/vdsm/backup/*, \
-@CP_PATH@ *multipath.conf *, \
+@CP_PATH@ /etc/multipath.conf *, \
 @CP_PATH@ * /var/log/vdsm/backup/* *, \
-@MULTIPATH_PATH@ *, \
+@MULTIPATH_PATH@, \
+@MULTIPATH_PATH@ -F, \
 @BLOCKDEV_PATH@ --getsize64 *, \
 @SETSID_PATH@ @IONICE_PATH@ -c ? -n ? @SU_PATH@ vdsm -s /bin/sh -c 
/usr/libexec/vdsm/spmprotect.sh*, \
 @SERVICE_PATH@ vdsmd *, \


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

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


Change in vdsm[master]: Change sudo wildcards to work with absolute paths

2013-02-04 Thread pzhukov
Pavel Zhukov has uploaded a new change for review.

Change subject: Change sudo wildcards to work with absolute paths
..

Change sudo wildcards to work with absolute paths

VDSM works with absolute paths of multipath,conf file but wildcards
in /etc/sudoers.d/50_vdsm don't allow it. The patch just modify wildcards
to allow absolute paths like  "/usr/bin/sudo -n /usr/sbin/persist 
/etc/multipath.conf'"
instead of "/usr/bin/sudo -n /usr/sbin/persist multipath.conf'" and so on

Change-Id: I8ac8f99bf54f28b1207fd0236f78c4d7d5169fdb
Signed-off-by: Pavel Zhukov 
---
M vdsm/sudoers.vdsm.in
1 file changed, 4 insertions(+), 4 deletions(-)


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

diff --git a/vdsm/sudoers.vdsm.in b/vdsm/sudoers.vdsm.in
index 2ebec96..7e53ab9 100644
--- a/vdsm/sudoers.vdsm.in
+++ b/vdsm/sudoers.vdsm.in
@@ -31,13 +31,13 @@
 @CAT_PATH@ /etc/multipath.conf, \
 @DD_PATH@ of=/sys/class/scsi_host/host*/scan, \
 @DD_PATH@, \
-@PERSIST_PATH@ multipath.conf, \
+@PERSIST_PATH@ *multipath.conf, \
 @PERSIST_PATH@ /var/log/vdsm/backup/*, \
-@UNPERSIST_PATH@ multipath.conf, \
+@UNPERSIST_PATH@ *multipath.conf, \
 @UNPERSIST_PATH@ /var/log/vdsm/backup/*, \
-@CP_PATH@ * multipath.conf *, \
+@CP_PATH@ *multipath.conf *, \
 @CP_PATH@ * /var/log/vdsm/backup/* *, \
-@MULTIPATH_PATH@, \
+@MULTIPATH_PATH@ *, \
 @BLOCKDEV_PATH@ --getsize64 *, \
 @SETSID_PATH@ @IONICE_PATH@ -c ? -n ? @SU_PATH@ vdsm -s /bin/sh -c 
/usr/libexec/vdsm/spmprotect.sh*, \
 @SERVICE_PATH@ vdsmd *, \


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

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