3:22 a.m.
Dan Kenigsberg has uploaded a new change for review.
Change subject: PARTIAL: explicitly close libvirt connection after use
......................................................................
PARTIAL: explicitly close libvirt connection after use
Otherwise, libvirt cries about
: error : virNetSocketReadWire:1184 : End of file while reading data: Input/output
error
TODO: fix other usages of libvirtconnection.get().
Change-Id: I2b15f3ab017828a63960ba9311fa2bf6bfab4729
Signed-off-by: Dan Kenigsberg <danken(a)redhat.com>
---
M lib/vdsm/tool/dummybr.py
1 file changed, 6 insertions(+), 5 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/92/17192/1
diff --git a/lib/vdsm/tool/dummybr.py b/lib/vdsm/tool/dummybr.py
index 762e726..68a9568 100644
--- a/lib/vdsm/tool/dummybr.py
+++ b/lib/vdsm/tool/dummybr.py
@@ -20,6 +20,7 @@
import os
+import contextlib
from vdsm.netinfo import DUMMY_BRIDGE
from vdsm import libvirtconnection, utils, constants
@@ -34,11 +35,11 @@
def addBridgeToLibvirt(bridgeName):
- conn = libvirtconnection.get(None, False)
- if bridgeName not in conn.listNetworks():
- conn.networkCreateXML(
- '''<network><name>%s</name><forward
mode='bridge'/><bridge '''
- '''name='%s'/></network>''' %
(bridgeName, bridgeName))
+ with contextlib.closing(libvirtconnection.get(None, False)) as conn:
+ if bridgeName not in conn.listNetworks():
+ conn.networkCreateXML(
+ '''<network><name>%s</name><forward
mode='bridge'/><bridge '''
+ '''name='%s'/></network>''' %
(bridgeName, bridgeName))
@expose('dummybr')
def main():
--
To view, visit http://gerrit.ovirt.org/17192
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I2b15f3ab017828a63960ba9311fa2bf6bfab4729
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dan Kenigsberg <danken(a)redhat.com>
6:40 a.m.
Dan Kenigsberg has posted comments on this change.
Change subject: PARTIAL: explicitly close libvirt connection after use
......................................................................
Patch Set 1:
Why using `new` is better than returning contextlib.closing(conn) in
libvirtconnection.get()?
I thought of doing something like this, but it requires more thought, as at the moment,
libvirtconnection.get() may return an existing instance, that is used in another context.
In that case, we should not close the connection when it falls out of scope - we should
have some kind of refcounting.
--
To view, visit http://gerrit.ovirt.org/17192
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I2b15f3ab017828a63960ba9311fa2bf6bfab4729
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Antoni Segura Puimedon <asegurap(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
6:24 a.m.
Antoni Segura Puimedon has posted comments on this change.
Change subject: PARTIAL: explicitly close libvirt connection after use
......................................................................
Patch Set 1: I would prefer that you didn't submit this
I'd rather modify the connection object in libvirtconnection.py in the following way:
Top of the module:
import new
and before return connection do:
connection.__enter__ = new.instancemethod(lambda self: self, connection,
connection.__class__)
def conn_close(self, *args):
self.close()
connection.__exit__ = new.instancemethod(conn_close, connection,
connection.__class__)
This will allow the current usage as well as:
with libvirtconnection.get() as conn:
conn.xxxx
and automatically close.
--
To view, visit http://gerrit.ovirt.org/17192
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I2b15f3ab017828a63960ba9311fa2bf6bfab4729
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Antoni Segura Puimedon <asegurap(a)redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
3:25 a.m.
oVirt Jenkins CI Server has posted comments on this change.
Change subject: PARTIAL: explicitly close libvirt connection after use
......................................................................
Patch Set 1:
Build Successful
http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/2601/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/3408/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/3492/ : SUCCESS
--
To view, visit http://gerrit.ovirt.org/17192
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I2b15f3ab017828a63960ba9311fa2bf6bfab4729
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
4:59 p.m.
New subject: Change in vdsm[master]: Explicitly close libvirt connections at exist
Dan Kenigsberg has submitted this change and it was merged.
Change subject: Explicitly close libvirt connections at exist
......................................................................
Explicitly close libvirt connections at exist
When a process (such as Vdsm, or `vdsm-tool dummybr`) exists while
holding an open libvirt connection, libvirt logs an error
: error : virNetSocketReadWire:1184 : End of file while reading data: Input/output
error
This patch close all libvirt connections at process exit.
Change-Id: I2b15f3ab017828a63960ba9311fa2bf6bfab4729
Signed-off-by: Dan Kenigsberg <danken(a)redhat.com>
Reviewed-on: http://gerrit.ovirt.org/17192
Reviewed-by: Yeela Kaplan <ykaplan(a)redhat.com>
Reviewed-by: mooli tayer <mtayer(a)redhat.com>
Reviewed-by: Antoni Segura Puimedon <asegurap(a)redhat.com>
Reviewed-by: Yaniv Bronhaim <ybronhei(a)redhat.com>
Reviewed-by: Federico Simoncelli <fsimonce(a)redhat.com>
---
M lib/vdsm/libvirtconnection.py
1 file changed, 8 insertions(+), 0 deletions(-)
Approvals:
Yeela Kaplan: Looks good to me, but someone else must approve
Yaniv Bronhaim: Looks good to me, but someone else must approve
Antoni Segura Puimedon: Looks good to me, but someone else must approve
Federico Simoncelli: Looks good to me, approved
mooli tayer: Looks good to me, but someone else must approve
Dan Kenigsberg: Verified
--
To view, visit http://gerrit.ovirt.org/17192
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I2b15f3ab017828a63960ba9311fa2bf6bfab4729
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Antoni Segura Puimedon <asegurap(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com>
Gerrit-Reviewer: Itamar Heim <iheim(a)redhat.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com>
Gerrit-Reviewer: Yeela Kaplan <ykaplan(a)redhat.com>
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: mooli tayer <mtayer(a)redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
4:57 p.m.
New subject: Change in vdsm[master]: Explicitly close libvirt connections at exist
Federico Simoncelli has posted comments on this change.
Change subject: Explicitly close libvirt connections at exist
......................................................................
Patch Set 3: Code-Review+2
--
To view, visit http://gerrit.ovirt.org/17192
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I2b15f3ab017828a63960ba9311fa2bf6bfab4729
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Antoni Segura Puimedon <asegurap(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com>
Gerrit-Reviewer: Itamar Heim <iheim(a)redhat.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com>
Gerrit-Reviewer: Yeela Kaplan <ykaplan(a)redhat.com>
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: mooli tayer <mtayer(a)redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
4:51 p.m.
New subject: Change in vdsm[master]: Explicitly close libvirt connections at exist
Dan Kenigsberg has posted comments on this change.
Change subject: Explicitly close libvirt connections at exist
......................................................................
Patch Set 3: Verified+1
Verified that the patch has no functional effect on VM lifecycle or vdsm-tool commands,
while avoiding libvirt log errors.
--
To view, visit http://gerrit.ovirt.org/17192
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I2b15f3ab017828a63960ba9311fa2bf6bfab4729
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Antoni Segura Puimedon <asegurap(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com>
Gerrit-Reviewer: Itamar Heim <iheim(a)redhat.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com>
Gerrit-Reviewer: Yeela Kaplan <ykaplan(a)redhat.com>
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: mooli tayer <mtayer(a)redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
5:34 p.m.
New subject: Change in vdsm[master]: Explicitly close libvirt connections at exist
Yaniv Bronhaim has posted comments on this change.
Change subject: Explicitly close libvirt connections at exist
......................................................................
Patch Set 3: Code-Review+1
--
To view, visit http://gerrit.ovirt.org/17192
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I2b15f3ab017828a63960ba9311fa2bf6bfab4729
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Antoni Segura Puimedon <asegurap(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com>
Gerrit-Reviewer: Itamar Heim <iheim(a)redhat.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com>
Gerrit-Reviewer: Yeela Kaplan <ykaplan(a)redhat.com>
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: mooli tayer <mtayer(a)redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
4:13 a.m.
New subject: Change in vdsm[master]: Explicitly close libvirt connections at exist
Dan Kenigsberg has posted comments on this change.
Change subject: Explicitly close libvirt connections at exist
......................................................................
Patch Set 3:
(1 comment)
http://gerrit.ovirt.org/#/c/17192/3/lib/vdsm/libvirtconnection.py
File lib/vdsm/libvirtconnection.py:
Line 176: def __close_connections():
Line 177: for conn in __connections.values():
Line 178: conn.close()
Line 179:
Line 180: atexit.register(__close_connections)
+1 for prepareForShutdown (that in theory should already call
stop_event_lo
I much prefer having this module standalone and usable outside of vdsm
proper. I actually noticed the the stray connections causing libvirt errors after running
`vdsm-tool dummybr` several times, where there's no prepareForShutdown.
Ack for calling stop_event_loop() in http://gerrit.ovirt.org/26532
--
To view, visit http://gerrit.ovirt.org/17192
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I2b15f3ab017828a63960ba9311fa2bf6bfab4729
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Antoni Segura Puimedon <asegurap(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com>
Gerrit-Reviewer: Itamar Heim <iheim(a)redhat.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com>
Gerrit-Reviewer: Yeela Kaplan <ykaplan(a)redhat.com>
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: mooli tayer <mtayer(a)redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
3:57 a.m.
New subject: Change in vdsm[master]: Explicitly close libvirt connections at exist
Antoni Segura Puimedon has posted comments on this change.
Change subject: Explicitly close libvirt connections at exist
......................................................................
Patch Set 3: Code-Review+1
--
To view, visit http://gerrit.ovirt.org/17192
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I2b15f3ab017828a63960ba9311fa2bf6bfab4729
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Antoni Segura Puimedon <asegurap(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com>
Gerrit-Reviewer: Itamar Heim <iheim(a)redhat.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com>
Gerrit-Reviewer: Yeela Kaplan <ykaplan(a)redhat.com>
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: mooli tayer <mtayer(a)redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
8:16 a.m.
New subject: Change in vdsm[master]: Explicitly close libvirt connections at exist
Federico Simoncelli has posted comments on this change.
Change subject: Explicitly close libvirt connections at exist
......................................................................
Patch Set 3:
(1 comment)
http://gerrit.ovirt.org/#/c/17192/3/lib/vdsm/libvirtconnection.py
File lib/vdsm/libvirtconnection.py:
Line 176: def __close_connections():
Line 177: for conn in __connections.values():
Line 178: conn.close()
Line 179:
Line 180: atexit.register(__close_connections)
could be confusing.. don't you prefer something more tight to
vdsm, and not
+1 for prepareForShutdown (that in theory should already call
stop_event_loop for cleanness).
--
To view, visit http://gerrit.ovirt.org/17192
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I2b15f3ab017828a63960ba9311fa2bf6bfab4729
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Antoni Segura Puimedon <asegurap(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com>
Gerrit-Reviewer: Itamar Heim <iheim(a)redhat.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com>
Gerrit-Reviewer: Yeela Kaplan <ykaplan(a)redhat.com>
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: mooli tayer <mtayer(a)redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
2:35 a.m.
New subject: Change in vdsm[master]: Explicitly close libvirt connections at exist
mooli tayer has posted comments on this change.
Change subject: Explicitly close libvirt connections at exist
......................................................................
Patch Set 3: Code-Review+1
--
To view, visit http://gerrit.ovirt.org/17192
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I2b15f3ab017828a63960ba9311fa2bf6bfab4729
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Antoni Segura Puimedon <asegurap(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Itamar Heim <iheim(a)redhat.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com>
Gerrit-Reviewer: Yeela Kaplan <ykaplan(a)redhat.com>
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: mooli tayer <mtayer(a)redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
2:28 a.m.
New subject: Change in vdsm[master]: Explicitly close libvirt connections at exist
Yaniv Bronhaim has posted comments on this change.
Change subject: Explicitly close libvirt connections at exist
......................................................................
Patch Set 3:
(1 comment)
http://gerrit.ovirt.org/#/c/17192/3/lib/vdsm/libvirtconnection.py
File lib/vdsm/libvirtconnection.py:
Line 176: def __close_connections():
Line 177: for conn in __connections.values():
Line 178: conn.close()
Line 179:
Line 180: atexit.register(__close_connections)
could be confusing.. don't you prefer something more tight to vdsm, and not being
depended on the interpreter ? like maybe calling this part only on prepareForShutdown or
on sigterm handling ?
--
To view, visit http://gerrit.ovirt.org/17192
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I2b15f3ab017828a63960ba9311fa2bf6bfab4729
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Antoni Segura Puimedon <asegurap(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Itamar Heim <iheim(a)redhat.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com>
Gerrit-Reviewer: Yeela Kaplan <ykaplan(a)redhat.com>
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: mooli tayer <mtayer(a)redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
11:32 a.m.
New subject: Change in vdsm[master]: Explicitly close libvirt connections at exist
Yeela Kaplan has posted comments on this change.
Change subject: Explicitly close libvirt connections at exist
......................................................................
Patch Set 3: Code-Review+1
--
To view, visit http://gerrit.ovirt.org/17192
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I2b15f3ab017828a63960ba9311fa2bf6bfab4729
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Antoni Segura Puimedon <asegurap(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Itamar Heim <iheim(a)redhat.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com>
Gerrit-Reviewer: Yeela Kaplan <ykaplan(a)redhat.com>
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: mooli tayer <mtayer(a)redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
11:04 a.m.
New subject: Change in vdsm[master]: Explicitly close libvirt connections at exist
oVirt Jenkins CI Server has posted comments on this change.
Change subject: Explicitly close libvirt connections at exist
......................................................................
Patch Set 3:
Build Failed
http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/7074/ : FAILURE
http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/7976/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/7865/ : SUCCESS
--
To view, visit http://gerrit.ovirt.org/17192
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I2b15f3ab017828a63960ba9311fa2bf6bfab4729
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Antoni Segura Puimedon <asegurap(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Itamar Heim <iheim(a)redhat.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com>
Gerrit-Reviewer: Yeela Kaplan <ykaplan(a)redhat.com>
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: mooli tayer <mtayer(a)redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
10:49 a.m.
New subject: Change in vdsm[master]: Explicitly close libvirt connections at exist
oVirt Jenkins CI Server has posted comments on this change.
Change subject: Explicitly close libvirt connections at exist
......................................................................
Patch Set 2: Code-Review-1 Verified-1
Build Failed
http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/7073/ : FAILURE
http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/7975/ : FAILURE
http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/7864/ : UNSTABLE
--
To view, visit http://gerrit.ovirt.org/17192
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I2b15f3ab017828a63960ba9311fa2bf6bfab4729
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Antoni Segura Puimedon <asegurap(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Itamar Heim <iheim(a)redhat.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com>
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
12:39 p.m.
Antoni Segura Puimedon has posted comments on this change.
Change subject: PARTIAL: explicitly close libvirt connection after use
......................................................................
Patch Set 1:
The difference is that using "new" allows you not to change the old usage, i.e.,
you still return a conn object on which you can execute queries. However, if you use it
with "with" it will behave like a context manager.
As for the refcounting, I agree, we should probably do better connection management.
--
To view, visit http://gerrit.ovirt.org/17192
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I2b15f3ab017828a63960ba9311fa2bf6bfab4729
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Antoni Segura Puimedon <asegurap(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
1:03 p.m.
Dan Kenigsberg has posted comments on this change.
Change subject: PARTIAL: explicitly close libvirt connection after use
......................................................................
Patch Set 1:
Toni, I still do not get why explicitly setting conn.__enter__ is any better than
def get():
bla
conn = foo
return closing(conn)
(not that we can actually use either of these due to the refcount issue)
--
To view, visit http://gerrit.ovirt.org/17192
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I2b15f3ab017828a63960ba9311fa2bf6bfab4729
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Antoni Segura Puimedon <asegurap(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
12:54 a.m.
Itamar Heim has posted comments on this change.
Change subject: PARTIAL: explicitly close libvirt connection after use
......................................................................
Patch Set 1:
ping - still relevant?
--
To view, visit http://gerrit.ovirt.org/17192
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I2b15f3ab017828a63960ba9311fa2bf6bfab4729
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Antoni Segura Puimedon <asegurap(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Itamar Heim <iheim(a)redhat.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
3699
days inactive
3981
days old
vdsm-patches@lists.fedorahosted.org
18 comments
8 participants
participants (8)
-
asegurap＠redhat.com -
Dan Kenigsberg -
Federico Simoncelli -
iheim＠redhat.com -
Jenkins CI RO -
mtayer＠redhat.com -
ybronhei＠redhat.com -
ykaplan＠redhat.com