Dan Kenigsberg has posted comments on this change.
Change subject: A new method to read from VMs' channels.
......................................................................
Patch Set 1: I would prefer that you didn't submit this
(13 inline comments)
I need to read it more closer later
....................................................
Commit Message
Line 11: VMs' channels.
describe our motivation - the huge memory overhead of each thread.
....................................................
File vdsm/clientIF.py
Line 189: self.channelListener.join()
why wait here for the listener?
stop it here and (only if needed) wait when the process tries to exit
....................................................
File vdsm/guestIF.py
Line 50: def __init__(self, socketName, channelListener, log, user='Unknown',
ips='', connect=True):
longline
Line 77: # Timeouts are handled in the channels' listener.
so why do you bother touching it here?
Line 85: logging.debug("Connection attempt failed: %s", e)
exc_info=True gives more interesting info than 'e'
Line 228: def _onChannelRead(self):
I think _onChannel may be called while it is already running, causing havoc to
self._buffer.
we may need to use StringIO
....................................................
File vdsm/libvirtvm.py
Line 1019: self.guestAgent = guestIF.GuestAgent(self._guestSocektFile,
self.cif.channelListener,
please keep lines shorter than 80 chars. I have a small laptop.
....................................................
File vdsm/Makefile.am
Line 38:
red white space
....................................................
File vdsm/vmChannels.py
Line 21: import threading, time, select
for some reason, current style mandates separate import statement per module.
Line 31:
gerrit does not like your trailing spaces.
Line 57: now = int(time.time())
why not work with floats?
Line 103: self.log.info("Setting channels' timeout to %d seconds.",
seconds)
again, why limit yourself to integral seconds?
Line 110: read_callback, timeout_callback, param)
passing args and kwargs is more pythonic and general than "param".
--
To view, visit
http://gerrit.ovirt.org/869
To unsubscribe, visit
http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I687a22a3fba6c1d2b42e837c5f2024aee0a98bdc
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Gal Hammer <ghammer(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>