On Fri, Mar 02, 2007 at 03:34:33PM +0000, Daniel P. Berrange wrote:
On Fri, Mar 02, 2007 at 03:06:56PM +0000, Mark McLoughlin wrote:
+static int qemudWaitForMonitor(struct qemud_vm *vm) {
We don't set an error in here, so how about returning the appropriate errno from the function?
I'll add in some error reporting.
We now get errors back like: $ ./virsh --connect qemu:///session start Fedora libvir: QEMU error : internal error Domain shutdown before sending monitor PTY path error: Failed to start domain Fedora
+ if (sscanf(tmp, "char device redirected to %19s", monitor) == 1) {
Path length of 100, maximum field with of 19? That's all rather voodooish ... hope about strstr() to find it, buffer length of PATH_MAX and then just strncpy()? Also, it'd be nice to split that out into e.g. qemudMonitorPathFromStr(buffer, monitorPath, PATH_MAX)
I'll have a go at splitting it out.
This is now done.
index() is a bit odd, why not strstr() ?
Well we're only looking for a single character match - index() is for single characters, strstr() is for strings.
Changed to use strchr() - although there's several other bits in existing code which already using 'index()' which also need updating.
+ if (connect(fd, (struct sockaddr*)&addr, sizeof(addr)) == 0)
Um, why not bind() (with SO_REUSEADDR)?
No particular reason - either will work just fine.
Actually one case where bind() is better is if the server is not accepting new connections - connect() will block indefinitely unless the server actually promptly accepts them. So I've changed to bind() + REUSEADDR. Dan. -- |=- Red Hat, Engineering, Emerging Technologies, Boston. +1 978 392 2496 -=| |=- Perl modules: http://search.cpan.org/~danberr/ -=| |=- Projects: http://freshmeat.net/~danielpb/ -=| |=- GnuPG: 7D3B9505 F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 -=|