On Mon, Sep 14, 2009 at 01:56:35PM +0100, Matthew Booth wrote:
On 14/09/09 12:10, Richard W.M. Jones wrote:
This is the only patch I currently have outstanding. No changes from the previous posting, except I rebased it against the head of git.
I'm running out of time, so I'm going to dump what I've got:
* I already moaned about gotos.
* Why does the patch change the set_trace test in generator.ml?
Should have explained - this was broken. If we enable set_trace in the test, then it means all tests that follow it have tracing enabled. This is not a show-stopper or anything like that, but it is undesirable. It should be a separate patch, but I noticed it and slipped the small change in.
* Don't just comment out xread: remove it. These pile up over time and make the code unreadable.
Done.
* Don't put a usleep() in read_log_message_or_eog. I understand your argument, but in this case the cure is worse than the disease. If you think about it, the patch back into the main loop and back here is actually very short, and very cheap. As well as being ugly as sin, this usleep is only going to slow it down.
strace reveals this makes a difference. Without it we end up with: read (of 1-16 bytes, usually 1-2 bytes) select read (of 1-16 bytes, usually 1-2 bytes) select [...] With it we get much larger reads and we don't reenter the select. _On the other hand_ the size of the sleep is difficult to judge. If it's any smaller than the current value then qemu doesn't have enough time to coalesce the serial port messages. This probably also means that it is sensitive to the particular details of my test machine.
* guestfs__send and send_file_chunk both leak msg_out in the non-error case.
Ugh, will fix that one.
I'll continue until I have to go.
Thanks, good luck. Rich. -- Richard Jones, Emerging Technologies, Red Hat http://et.redhat.com/~rjones virt-df lists disk usage of guests without needing to install any software inside the virtual machine. Supports Linux and Windows. http://et.redhat.com/~rjones/virt-df/