Re: [Libvir] PATCH: Process QEMU monitor at startup
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 -=|
Attachments:
- libvirt-qemu-startup-2.patch (text/plain — 17.0 KB)