Am 29. August 2011 20:06 schrieb Andrea Spadaccini <[email protected]>:
> --- a/lib/hypervisor/hv_kvm.py
> +++ b/lib/hypervisor/hv_kvm.py
> +class QmpConnection:
> + def __init__(self, monitor_filename):
> + # We want to fail if the server doesn't send a complete message
> + # in a reasonable amount of time
> + self.sock.settimeout(5)
Please put the timeout into a constant.
> + def _Recv(self):
> + self._check_connection()
> + recv_buffer = StringIO.StringIO()
> + try:
> + while True:
> + data = self.sock.recv(4096)
> + if not data:
> + break
> + recv_buffer.write(data)
> +
> + # Check if we got the message end token (CRLF, as per the QEMU
> Protocol
> + # Specification 0.1 - Section 2.1.1)
> + if self._MESSAGE_END_TOKEN in data:
> + break
Already much better, but there are two issues:
- What if more than one message is sent? You buffer it locally, but
the next call to _Recv will be missing that part of the message. You
need to keep the receive buffer in the instance.
- If one byte of the end token happens to be at the end of the read
("…\r", second read would yield "\n") this doesn't work. You need to
check if the token is in the buffer (since this is more expensive you
can also check the temporary variable).
> + received_data = recv_buffer.getvalue()
Not sure: assert received_data.endswith(_MESSAGE_END_TOKEN)?
> + recv_buffer.close()
No need for this on StringIO.
> --- a/test/ganeti.hypervisor.hv_kvm_unittest.py
> +++ b/test/ganeti.hypervisor.hv_kvm_unittest.py
> + def encode_string(self, message):
> + return serializer.DumpJson(message, indent = False) + \
Don't use backslash in this case, but rather parentheses. No spaces
around keyword parameter's equal sign.
> + hv_kvm.QmpConnection._MESSAGE_END_TOKEN
> […]
> + def _executeTestScript(self, script):
> + # Set up the stub
> + socket_filename = self._CreateTempFile()
> + os.remove(socket_filename)
Why don't you just use the tempfile module? _CreateTempFile & Co. are painful.
> + qmp_stub = QmpStub(socket_filename, script)
> + qmp_stub.start()
Michael
