Dan Kenigsberg has posted comments on this change.
Change subject: vdsm: Do not fail releasing a VM due to agent stopping errors
......................................................................
Patch Set 3: I would prefer that you didn't submit this
(2 inline comments)
....................................................
File vdsm/guestIF.py
Line 241: # socket was already closed
Line 242: pass
Line 243: else:
Line 244: raise
Line 245: except AttributeError:
adding the self._sock attribute outside __init__ leads to all kinds of
complexities, such as catching all AttributeError.
How about initializing
self._sock = socket.socket(socket.AF_UNIX, socket.SOCK_STREAM)
in __init__()? This would also let us drop the
if hasattr(self, '_sock'):
conditional.
Line 246: # This is not supposed to happen, but might if we fail to
connect
Line 247: # to the VirtIO channel because it does not exist.
Line 248: self.log.exception("Failure on unregistering guest VirtIO
channel")
Line 249: else:
....................................................
File vdsm/vm.py
Line 4209: if self.guestAgent:
Line 4210: try:
Line 4211: self.guestAgent.stop()
Line 4212: except Exception:
Line 4213: # This is on purpose that generic because
there's no
I am not at all sure about this. You can use the same logic regarding
self._vmStats.stop() above, and frankly any other method that can raise an
exception.
Our code is riddled with too much of these - I would prefer to solve only the
bug
Line 4214: # reason for letting the destroy now fail.
Line 4215: # We'll just logging it
Line 4216: self.log.exception("Failure occurred during
stopping "
Line 4217: "of the guest agent
communication "
--
To view, visit http://gerrit.ovirt.org/17072
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I10ac23b11294dc90e4980944d7ba6fad6abc22dc
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Vinzenz Feenstra <[email protected]>
Gerrit-Reviewer: Dan Kenigsberg <[email protected]>
Gerrit-Reviewer: Federico Simoncelli <[email protected]>
Gerrit-Reviewer: Martin Sivák <[email protected]>
Gerrit-Reviewer: Michal Skrivanek <[email protected]>
Gerrit-Reviewer: Peter V. Saveliev <[email protected]>
Gerrit-Reviewer: oVirt Jenkins CI Server
_______________________________________________
vdsm-patches mailing list
[email protected]
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
