Re: [Libvir] PATCH: Autodetect QEMU version
On Wed, Feb 21, 2007 at 05:26:52PM +0000, Mark McLoughlin wrote:
On Wed, 2007-02-21 at 15:45 +0000, Daniel P. Berrange wrote:
> if (vm->def->graphicsType == QEMUD_GRAPHICS_VNC) {
> char port[10];
> - snprintf(port, 10, "%d", vm->def->vncActivePort - 5900);
> + int ret;
> + ret = snprintf(port, sizeof(port),
> + (server->qemuVersion >= 9000 ?
> + ":%d" : "%d"),
> + vm->def->vncActivePort - 5900);
How about something like:
server->qemuCmdFlags & QEMUD_CMD_VNC_PORT_NEEDS_COLON ? ":%d" :
"%d"
That way we could keep the actual logic around what version does what
in qemudExtractVersion()
(Ditto for QEMUD_CMD_HAS_KQEMU ...)
Seems reasonable.
> +int qemudExtractVersion(const char *qemu, int *version, int
*hasKQEMU) {
> + int child;
> + int newstdout[2];
> +
> + if (pipe(newstdout) < 0) {
> + return -1;
> + }
> +
> + if ((child = fork()) < 0) {
Close the pipes here
Fixed.
> + if ((got = read(newstdout[0], help, sizeof(help)-1))
< 0)
> + goto cleanup2;
Handle EINTR here ... it's quite a likely place to hit it, I guess
(e.g. SIGCHLD?)
Yes, I also check & try in the waitpid() call too now.
> + cleanup2:
> + if (close(newstdout[0]) < 0)
> + ret = -1;
> +
> + if (waitpid(child, &got, 0) != child ||
> + WEXITSTATUS(got) != 1) {
> + qemudLog(QEMUD_ERR, "Unexpected exit status from qemu %d pid
%d", got, child);
> + ret = -1;
> + }
Don't know that we actually want to fail under both these
circumstances.
Switched it to only fail on the waitpid return value case. For unexpected
exit status we simply log a warning message.
> --- qemud/driver.h 14 Feb 2007 16:20:38 -0000 1.3
> +++ qemud/driver.h 21 Feb 2007 15:40:41 -0000
> @@ -30,6 +30,7 @@
> void qemudReportError(struct qemud_server *server,
> int code, const char *fmt, ...);
>
> +int qemudExtractVersion(const char *qemu, int *version, int *hasKQEMU);
The driver seems to be a strange place to put this function.
Not quite sure what I was thinking. Have moved it to conf.c instead
so its a static function now.
> diff -u -p -r1.23 qemud.c
> --- qemud/qemud.c 20 Feb 2007 17:51:41 -0000 1.23
> +++ qemud/qemud.c 21 Feb 2007 15:40:42 -0000
> @@ -411,20 +411,26 @@ static struct qemud_server *qemudInitial
> + if (!(binary = qemudLocateBinaryForArch(server, QEMUD_VIRT_QEMU,
"i686")))
> + goto cleanup;
> + if (qemudExtractVersion(binary, &server->qemuVersion,
&server->qemuHasKQEMU) < 0)
> + goto cleanup;
> + free(binary);
> + binary = NULL;
Note, this means we don't have support for networks if something's
wrong with qemu ... think we should just try and locate the binary when
starting domains and return an error if it fails.
The info is also needed in the getVersionCall for virConnectGetVersion, so
I've made a thin wrapper to lazy-init the version number when needed.
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-detect-version-2.patch (text/plain — 8.1 KB)