Change in vdsm[master]: vdsm-tool/load_needed_modules: fix creation of legacy bonds

[Dan Kenigsberg]

Dan Kenigsberg has uploaded a new change for review.

Change subject: vdsm-tool/load_needed_modules: fix creation of legacy bonds
......................................................................

vdsm-tool/load_needed_modules: fix creation of legacy bonds

Fix commit 816238891be which has failed to open MASTER_FILE for writing
before wrting to it.

Change-Id: I82dd48d52541b6d35606fd8358cd98cd9f46fed5
Signed-off-by: Dan Kenigsberg <danken(a)redhat.com&gt;
---
M vdsm-tool/load_needed_modules.py.in
1 file changed, 1 insertion(+), 1 deletion(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/79/7879/1

diff --git a/vdsm-tool/load_needed_modules.py.in b/vdsm-tool/load_needed_modules.py.in
index ba80c72..675a172 100644
--- a/vdsm-tool/load_needed_modules.py.in
+++ b/vdsm-tool/load_needed_modules.py.in
@@ -45,7 +45,7 @@
 
     # @ENGINENAME@ currently assumes that all bonding devices pre-exist
     existing = set(file(MASTER_FILE).read().split())
-    with open(MASTER_FILE) as f:
+    with open(MASTER_FILE, 'w') as f:
         for bond in REQUIRED - existing:
             f.write('+%s\n' % bond)
 


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I82dd48d52541b6d35606fd8358cd98cd9f46fed5
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dan Kenigsberg <danken(a)redhat.com&gt;

[Dan Kenigsberg]

Dan Kenigsberg has submitted this change and it was merged.

Change subject: vdsm-tool/load_needed_modules: fix creation of legacy bonds
......................................................................


vdsm-tool/load_needed_modules: fix creation of legacy bonds

Fix commit 816238891be which has failed to open MASTER_FILE for writing
before wrting to it.

Change-Id: I82dd48d52541b6d35606fd8358cd98cd9f46fed5
Signed-off-by: Dan Kenigsberg <danken(a)redhat.com&gt;
---
M vdsm-tool/load_needed_modules.py.in
1 file changed, 1 insertion(+), 1 deletion(-)

Approvals:
  Yaniv Bronhaim: Verified; Looks good to me, but someone else must approve
  Dan Kenigsberg: Looks good to me, approved


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

Gerrit-MessageType: merged
Gerrit-Change-Id: I82dd48d52541b6d35606fd8358cd98cd9f46fed5
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Wenyi Gao <wenyi(a)linux.vnet.ibm.com&gt;
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com&gt;

[Dan Kenigsberg]

Dan Kenigsberg has posted comments on this change.

Change subject: vdsm-tool/load_needed_modules: fix creation of legacy bonds
......................................................................


Patch Set 1: Looks good to me, approved

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I82dd48d52541b6d35606fd8358cd98cd9f46fed5
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Wenyi Gao <wenyi(a)linux.vnet.ibm.com&gt;
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com&gt;

[Dan Kenigsberg]

Dan Kenigsberg has posted comments on this change.

Change subject: vdsm-tool/load_needed_modules: fix creation of legacy bonds
......................................................................


Patch Set 1: (1 inline comment)

....................................................
File vdsm-tool/load_needed_modules.py.in
Line 47:     existing = set(file(MASTER_FILE).read().split())
Line 48:     with open(MASTER_FILE, 'w') as f:
Line 49:         for bond in REQUIRED - existing:
Line 50:             f.write('+%s\n' % bond)
Line 51: 
what would you do with it when you caught it? is it really better than Python's
defaults?
Line 52: 
Line 53: @expose('load-needed-modules')
Line 54: def load_needed_modules():
Line 55:     """


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I82dd48d52541b6d35606fd8358cd98cd9f46fed5
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Wenyi Gao <wenyi(a)linux.vnet.ibm.com&gt;
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com&gt;

Yaniv Bronhaim has posted comments on this change.

Change subject: vdsm-tool/load_needed_modules: fix creation of legacy bonds
......................................................................


Patch Set 1: (1 inline comment)

....................................................
File vdsm-tool/load_needed_modules.py.in
Line 47:     existing = set(file(MASTER_FILE).read().split())
Line 48:     with open(MASTER_FILE, 'w') as f:
Line 49:         for bond in REQUIRED - existing:
Line 50:             f.write('+%s\n' % bond)
Line 51: 
I would catch exception here.
Line 52: 
Line 53: @expose('load-needed-modules')
Line 54: def load_needed_modules():
Line 55:     """


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I82dd48d52541b6d35606fd8358cd98cd9f46fed5
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Wenyi Gao <wenyi(a)linux.vnet.ibm.com&gt;
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com&gt;

Yaniv Bronhaim has posted comments on this change.

Change subject: vdsm-tool/load_needed_modules: fix creation of legacy bonds
......................................................................


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

(1 inline comment)

....................................................
File vdsm-tool/load_needed_modules.py.in
Line 46:     # @ENGINENAME@ currently assumes that all bonding devices pre-exist
Line 47:     existing = set(file(MASTER_FILE).read().split())
Line 48:     with open(MASTER_FILE, 'w') as f:
Line 49:         for bond in REQUIRED - existing:
Line 50:             f.write('+%s\n' % bond)
seems like this is what need to be here. verified.
Line 51: 
Line 52: 
Line 53: @expose('load-needed-modules')
Line 54: def load_needed_modules():


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I82dd48d52541b6d35606fd8358cd98cd9f46fed5
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Wenyi Gao <wenyi(a)linux.vnet.ibm.com&gt;
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com&gt;

Yaniv Bronhaim has posted comments on this change.

Change subject: vdsm-tool/load_needed_modules: fix creation of legacy bonds
......................................................................


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

(1 inline comment)

....................................................
File vdsm-tool/load_needed_modules.py.in
Line 44:     MASTER_FILE = '/sys/class/net/bonding_masters'
Line 45: 
Line 46:     # @ENGINENAME@ currently assumes that all bonding devices pre-exist
Line 47:     existing = set(file(MASTER_FILE).read().split())
Line 48:     with open(MASTER_FILE, 'w') as f:
.. If mode is omitted, it defaults to 'r'...
(http://docs.python.org/library/functions.html#open)

are you sure about that change??
Line 49:         for bond in REQUIRED - existing:
Line 50:             f.write('+%s\n' % bond)
Line 51: 
Line 52: 


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I82dd48d52541b6d35606fd8358cd98cd9f46fed5
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Wenyi Gao <wenyi(a)linux.vnet.ibm.com&gt;
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com&gt;