[libvirt] can't install fedora guest
hi, while i try to install fedora-10 guest on centos-5 host i've got the error bellow virt-manager. our setup: virt-manager-0.6.1-1 libvirt-0.6.0-1 kvm-83-1 ----------------------------------------------- Unable to complete install 'libvirt.libvirtError internal error unable to start guest: Traceback (most recent call last): File "/usr/share/virt-manager/virtManager/create.py", line 717, in do_install dom = guest.start_install(False, meter = meter) File "/usr/lib/python2.4/site-packages/virtinst/Guest.py", line 926, in start_install return self._do_install(consolecb, meter, removeOld, wait) File "/usr/lib/python2.4/site-packages/virtinst/Guest.py", line 966, in _do_install self.domain = self.conn.createLinux(install_xml, 0) File "/usr/lib64/python2.4/site-packages/libvirt.py", line 973, in createLinux if ret is None:raise libvirtError('virDomainCreateLinux() failed', conn=self) libvirtError: internal error unable to start guest: ----------------------------------------------- while this is in virt-manager's log: ----------------------------------------------- [Fri, 06 Feb 2009 10:27:15 virt-manager 31523] DEBUG (Storage:908) Storage volume 'fedora-i386.img' install complete. [Fri, 06 Feb 2009 10:27:15 virt-manager 31523] DEBUG (Guest:964) Creating guest from: <domain type='kvm'> <name>fedora-i386</name> <currentMemory>524288</currentMemory> <memory>524288</memory> <uuid>75330b36-f8e3-8459-2d71-fb47245d7203</uuid> <os> <type arch='x86_64'>hvm</type> <boot dev='cdrom'/> </os> <features> <acpi/><apic/> </features> <clock offset="utc"/> <on_poweroff>destroy</on_poweroff> <on_reboot>destroy</on_reboot> <on_crash>destroy</on_crash> <vcpu>1</vcpu> <devices> <emulator>/usr/bin/qemu-kvm</emulator> <console type='pty'/> <disk type='file' device='disk'> <source file='/var/lib/libvirt/images/fedora-i386.img'/> <target dev='vda' bus='virtio'/> </disk> <disk type='block' device='cdrom'> <source dev='/dev/hda'/> <target dev='hdc' bus='ide'/> <readonly/> </disk> <interface type='bridge'> <source bridge='eth0'/> <mac address='54:52:00:5d:99:c0'/> <model type='virtio'/> </interface> <input type='mouse' bus='ps2'/> <graphics type='vnc' port='-1' keymap='en-us'/> <sound model='es1370'/> </devices> </domain> [Fri, 06 Feb 2009 10:27:16 virt-manager 31523] ERROR (create:735) Unable to complete install 'libvirt.libvirtError internal error unable to start guest: Traceback (most recent call last): File "/usr/share/virt-manager/virtManager/create.py", line 717, in do_install dom = guest.start_install(False, meter = meter) File "/usr/lib/python2.4/site-packages/virtinst/Guest.py", line 926, in start_install return self._do_install(consolecb, meter, removeOld, wait) File "/usr/lib/python2.4/site-packages/virtinst/Guest.py", line 966, in _do_install self.domain = self.conn.createLinux(install_xml, 0) File "/usr/lib64/python2.4/site-packages/libvirt.py", line 973, in createLinux if ret is None:raise libvirtError('virDomainCreateLinux() failed', conn=self) libvirtError: internal error unable to start guest: ' [Fri, 06 Feb 2009 10:27:16 virt-manager 31523] DEBUG (error:76) Uncaught Error: Unable to complete install: 'internal error unable to start guest: ' : Unable to complete install 'libvirt.libvirtError interna l error unable to start guest: Traceback (most recent call last): File "/usr/share/virt-manager/virtManager/create.py", line 717, in do_install dom = guest.start_install(False, meter = meter) File "/usr/lib/python2.4/site-packages/virtinst/Guest.py", line 926, in start_install return self._do_install(consolecb, meter, removeOld, wait) File "/usr/lib/python2.4/site-packages/virtinst/Guest.py", line 966, in _do_install self.domain = self.conn.createLinux(install_xml, 0) File "/usr/lib64/python2.4/site-packages/libvirt.py", line 973, in createLinux if ret is None:raise libvirtError('virDomainCreateLinux() failed', conn=self) libvirtError: internal error unable to start guest: ' ----------------------------------------------- -- Levente "Si vis pacem para bellum!"
On Fri, Feb 06, 2009 at 10:33:51AM +0100, Farkas Levente wrote:
if ret is None:raise libvirtError('virDomainCreateLinux() failed', conn=self) libvirtError: internal error unable to start guest:
I'm currently working around this with: diff --git a/src/qemu_driver.c b/src/qemu_driver.c index 09f69bf..b2f2b47 100644 --- a/src/qemu_driver.c +++ b/src/qemu_driver.c @@ -674,8 +674,6 @@ qemudReadMonitorOutput(virConnectPtr conn, _("Failure while reading %s startup output"), what); return -1; } - } else if (ret == 0) { - return 0; } else { got += ret; buf[got] = '\0'; Didn't find the time to debug this properly yet. Cheers, -- Guido
On Fri, Feb 06, 2009 at 05:27:47PM +0100, Guido G?nther wrote:
On Fri, Feb 06, 2009 at 10:33:51AM +0100, Farkas Levente wrote:
if ret is None:raise libvirtError('virDomainCreateLinux() failed', conn=self) libvirtError: internal error unable to start guest:
I'm currently working around this with:
diff --git a/src/qemu_driver.c b/src/qemu_driver.c index 09f69bf..b2f2b47 100644 --- a/src/qemu_driver.c +++ b/src/qemu_driver.c @@ -674,8 +674,6 @@ qemudReadMonitorOutput(virConnectPtr conn, _("Failure while reading %s startup output"), what); return -1; } - } else if (ret == 0) { - return 0; } else { got += ret; buf[got] = '\0';
Didn't find the time to debug this properly yet.
That will cause libvirtd to spin 100% cpu forever, if a guest fails to start up. eg disk to mis-configured disk The core problem here, is that ret == 0 has 2 possible implications - QEMU has exited, and no more data will be written - QEMU is still starting up, and we have read all the data written so far, but more may arrive soon. The current code there is correct for the first scenario, but even removing it, is not entirely correct for the 2nd scenario. If we hit ret == 0, and QEMU is still running, we shouldn't spin 100% CPU in read - we should poll() to wait for more data. As a quick fix though, we could probably detect whether QEMU has exited by doing 'kill(vm->pid, 0)' and check for errno == ESRCH - indicates that the process no longer exists. eg, } else if (ret == 0) { if (kill(vm->pid, 0) == -1) { if (errno == ERSCH) return 0; else return -1; } else { continue; /* should really go into poll() at this point */ } } else { Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
On Fri, Feb 06, 2009 at 04:48:09PM +0000, Daniel P. Berrange wrote:
On Fri, Feb 06, 2009 at 05:27:47PM +0100, Guido G?nther wrote:
On Fri, Feb 06, 2009 at 10:33:51AM +0100, Farkas Levente wrote:
if ret is None:raise libvirtError('virDomainCreateLinux() failed', conn=self) libvirtError: internal error unable to start guest:
I'm currently working around this with:
diff --git a/src/qemu_driver.c b/src/qemu_driver.c index 09f69bf..b2f2b47 100644 --- a/src/qemu_driver.c +++ b/src/qemu_driver.c @@ -674,8 +674,6 @@ qemudReadMonitorOutput(virConnectPtr conn, _("Failure while reading %s startup output"), what); return -1; } - } else if (ret == 0) { - return 0; } else { got += ret; buf[got] = '\0';
Didn't find the time to debug this properly yet.
That will cause libvirtd to spin 100% cpu forever, if a guest fails to start up. eg disk to mis-configured disk Yes, I know. This was really just meant as a quick workaround to be able to start up vms at all until I (or somebody else) gets around to have another look. -- Guido
On Fri, Feb 06, 2009 at 04:48:09PM +0000, Daniel P. Berrange wrote:
On Fri, Feb 06, 2009 at 05:27:47PM +0100, Guido G?nther wrote:
On Fri, Feb 06, 2009 at 10:33:51AM +0100, Farkas Levente wrote:
if ret is None:raise libvirtError('virDomainCreateLinux() failed', conn=self) libvirtError: internal error unable to start guest:
I'm currently working around this with:
diff --git a/src/qemu_driver.c b/src/qemu_driver.c index 09f69bf..b2f2b47 100644 --- a/src/qemu_driver.c +++ b/src/qemu_driver.c @@ -674,8 +674,6 @@ qemudReadMonitorOutput(virConnectPtr conn, _("Failure while reading %s startup output"), what); return -1; } - } else if (ret == 0) { - return 0; } else { got += ret; buf[got] = '\0';
Didn't find the time to debug this properly yet.
That will cause libvirtd to spin 100% cpu forever, if a guest fails to start up. eg disk to mis-configured disk
The core problem here, is that ret == 0 has 2 possible implications
- QEMU has exited, and no more data will be written - QEMU is still starting up, and we have read all the data written so far, but more may arrive soon.
The current code there is correct for the first scenario, but even removing it, is not entirely correct for the 2nd scenario. If we hit ret == 0, and QEMU is still running, we shouldn't spin 100% CPU in read - we should poll() to wait for more data.
As a quick fix though, we could probably detect whether QEMU has exited by doing 'kill(vm->pid, 0)' and check for errno == ESRCH - indicates that the process no longer exists.
eg,
} else if (ret == 0) { if (kill(vm->pid, 0) == -1) { if (errno == ERSCH) return 0; else return -1; } else { continue; /* should really go into poll() at this point */ } } else { The problem is that we're using this function for two purposes: To read from a logfile where poll()'ing on POLLIN always returns "data readable" which leaves us spinning (and EOF might get in our way) and also using it on the monitor PTY itself where it works as expected. Why not simply introduce qemudReadLogOutput? This at least allows us to usleep while waiting for the log file to fill up with data. I couldn't make libvirtd spin with this patch anymore. O.k. to apply? Cheers, -- Guido
P.S.: I also changed the timeouts to be in seconds.
On Sat, Feb 07, 2009 at 06:05:51PM +0100, Guido G?nther wrote:
On Fri, Feb 06, 2009 at 04:48:09PM +0000, Daniel P. Berrange wrote:
That will cause libvirtd to spin 100% cpu forever, if a guest fails to start up. eg disk to mis-configured disk
The core problem here, is that ret == 0 has 2 possible implications
- QEMU has exited, and no more data will be written - QEMU is still starting up, and we have read all the data written so far, but more may arrive soon.
The current code there is correct for the first scenario, but even removing it, is not entirely correct for the 2nd scenario. If we hit ret == 0, and QEMU is still running, we shouldn't spin 100% CPU in read - we should poll() to wait for more data.
As a quick fix though, we could probably detect whether QEMU has exited by doing 'kill(vm->pid, 0)' and check for errno == ESRCH - indicates that the process no longer exists.
eg,
} else if (ret == 0) { if (kill(vm->pid, 0) == -1) { if (errno == ERSCH) return 0; else return -1; } else { continue; /* should really go into poll() at this point */ } } else { The problem is that we're using this function for two purposes: To read from a logfile where poll()'ing on POLLIN always returns "data readable" which leaves us spinning (and EOF might get in our way) and also using it on the monitor PTY itself where it works as expected. Why not simply introduce qemudReadLogOutput? This at least allows us to usleep while waiting for the log file to fill up with data. I couldn't make libvirtd spin with this patch anymore. O.k. to apply?
Ah of course, I forgot poll() isn't particularly useful on regular files. For a non-spinning version we'd need to integrate with inotify to monitor changes. ACK to your proposed patch for now. Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
On Mon, Feb 09, 2009 at 11:02:58AM +0000, Daniel P. Berrange wrote:
On Sat, Feb 07, 2009 at 06:05:51PM +0100, Guido G?nther wrote:
The problem is that we're using this function for two purposes: To read from a logfile where poll()'ing on POLLIN always returns "data readable" which leaves us spinning (and EOF might get in our way) and also using it on the monitor PTY itself where it works as expected. Why not simply introduce qemudReadLogOutput? This at least allows us to usleep while waiting for the log file to fill up with data. I couldn't make libvirtd spin with this patch anymore. O.k. to apply? Committed now.
Ah of course, I forgot poll() isn't particularly useful on regular files. For a non-spinning version we'd need to integrate with inotify to monitor changes. ACK to your proposed patch for now. I didn't use inotify since it's not portable. It might make sense to add an alternative linux only implementation though. -- Guido
Guido Günther <agx@sigxcpu.org> wrote: ...
From 6655474aed6e601a6c2e16e7020589f51f58893b Mon Sep 17 00:00:00 2001 From: =?utf-8?q?Guido=20G=C3=BCnther?= <agx@sigxcpu.org> Date: Sat, 7 Feb 2009 17:16:39 +0100 Subject: [PATCH] usleep to wait for domain logfile to fill up
--- src/qemu_driver.c | 65 ++++++++++++++++++++++++++++++++++++++++++++++------ 1 files changed, 57 insertions(+), 8 deletions(-)
diff --git a/src/qemu_driver.c b/src/qemu_driver.c index 47ca6c7..09be3fb 100644 --- a/src/qemu_driver.c +++ b/src/qemu_driver.c @@ -601,6 +601,7 @@ qemudReadMonitorOutput(virConnectPtr conn, { int got = 0; buf[0] = '\0'; + timeout *= 1000; /* poll wants milli seconds */
/* Consume & discard the initial greeting */ while (got < (buflen-1)) { @@ -662,6 +663,56 @@ qemudReadMonitorOutput(virConnectPtr conn,
}
+ +/* + * Returns -1 for error, 0 on success + */ +static int +qemudReadLogOutput(virConnectPtr conn, + virDomainObjPtr vm, + int fd, + char *buf, + int buflen, + qemudHandlerMonitorOutput func, + const char *what, + int timeout) +{ + int got = 0; + int ret; + int retries = timeout*10; + buf[0] = '\0'; + + while (retries) { + while((ret = read(fd, buf+got, buflen-got-1)) > 0) { + got += ret; + buf[got] = '\0'; + if ((buflen-got-1) == 0) { + qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, + _("Out of space while reading %s log output"), what); + return -1; + } + }
Hi Guido, Sorry I didn't review this sooner, but I looked just today after you committed it. Here's a proposed patch to make it use better types (always suspect that using "int" is wrong ;-). Also, shouldn't it handle read failing with EAGAIN? diff --git a/src/qemu_driver.c b/src/qemu_driver.c index 09be3fb..ae393be 100644 --- a/src/qemu_driver.c +++ b/src/qemu_driver.c @@ -672,17 +672,17 @@ qemudReadLogOutput(virConnectPtr conn, virDomainObjPtr vm, int fd, char *buf, - int buflen, + size_t buflen, qemudHandlerMonitorOutput func, const char *what, int timeout) { - int got = 0; - int ret; int retries = timeout*10; buf[0] = '\0'; while (retries) { + ssize_t ret; + size_t got = 0; while((ret = read(fd, buf+got, buflen-got-1)) > 0) { got += ret; buf[got] = '\0';
Hi Jim, On Fri, Feb 13, 2009 at 07:07:49PM +0100, Jim Meyering wrote:
Sorry I didn't review this sooner, but I looked just today after you committed it.
Here's a proposed patch to make it use better types (always suspect that using "int" is wrong ;-). Agreed. This is basically a straigt copy from qemudReadMonitorOutput. I'll fix things up there too.
Also, shouldn't it handle read failing with EAGAIN? We didn't set O_NONBLOCK. Do we have to when reading from plain file? Cheers, -- Guido
Guido Günther <agx@sigxcpu.org> wrote:
On Fri, Feb 13, 2009 at 07:07:49PM +0100, Jim Meyering wrote:
Here's a proposed patch to make it use better types (always suspect that using "int" is wrong ;-). What about the attached version? -- Guido
From 5b2c2328195f0cf4eb32d2da1d5a5ef57b2fede4 Mon Sep 17 00:00:00 2001 From: =?utf-8?q?Guido=20G=C3=BCnther?= <agx@sigxcpu.org> Date: Sat, 14 Feb 2009 14:18:45 +0100 Subject: [PATCH] (s)size_t type cleanup
--- src/qemu_driver.c | 12 ++++++------ 1 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/src/qemu_driver.c b/src/qemu_driver.c index 09be3fb..8f8b44d 100644 --- a/src/qemu_driver.c +++ b/src/qemu_driver.c @@ -594,18 +594,18 @@ qemudReadMonitorOutput(virConnectPtr conn, virDomainObjPtr vm, int fd, char *buf, - int buflen, + size_t buflen, qemudHandlerMonitorOutput func, const char *what, int timeout) { - int got = 0; + size_t got = 0; buf[0] = '\0'; timeout *= 1000; /* poll wants milli seconds */
/* Consume & discard the initial greeting */ while (got < (buflen-1)) { - int ret; + ssize_t ret;
ret = read(fd, buf+got, buflen-got-1);
@@ -672,13 +672,13 @@ qemudReadLogOutput(virConnectPtr conn, virDomainObjPtr vm, int fd, char *buf, - int buflen, + size_t buflen, qemudHandlerMonitorOutput func, const char *what, int timeout) { - int got = 0; - int ret; + size_t got = 0; + ssize_t ret; int retries = timeout*10; buf[0] = '\0';
That looks ok, but since the latter two variables (in qemudReadLogOutput) are used only from within the while-loop, their declarations belong in that inner scope.
On Sat, Feb 14, 2009 at 03:42:00PM +0100, Jim Meyering wrote:
Guido Günther <agx@sigxcpu.org> wrote:
On Fri, Feb 13, 2009 at 07:07:49PM +0100, Jim Meyering wrote:
Here's a proposed patch to make it use better types (always suspect that using "int" is wrong ;-). What about the attached version? -- Guido
From 5b2c2328195f0cf4eb32d2da1d5a5ef57b2fede4 Mon Sep 17 00:00:00 2001 From: =?utf-8?q?Guido=20G=C3=BCnther?= <agx@sigxcpu.org> Date: Sat, 14 Feb 2009 14:18:45 +0100 Subject: [PATCH] (s)size_t type cleanup
--- src/qemu_driver.c | 12 ++++++------ 1 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/src/qemu_driver.c b/src/qemu_driver.c index 09be3fb..8f8b44d 100644 --- a/src/qemu_driver.c +++ b/src/qemu_driver.c @@ -594,18 +594,18 @@ qemudReadMonitorOutput(virConnectPtr conn, virDomainObjPtr vm, int fd, char *buf, - int buflen, + size_t buflen, qemudHandlerMonitorOutput func, const char *what, int timeout) { - int got = 0; + size_t got = 0; buf[0] = '\0'; timeout *= 1000; /* poll wants milli seconds */
/* Consume & discard the initial greeting */ while (got < (buflen-1)) { - int ret; + ssize_t ret;
ret = read(fd, buf+got, buflen-got-1);
@@ -672,13 +672,13 @@ qemudReadLogOutput(virConnectPtr conn, virDomainObjPtr vm, int fd, char *buf, - int buflen, + size_t buflen, qemudHandlerMonitorOutput func, const char *what, int timeout) { - int got = 0; - int ret; + size_t got = 0; + ssize_t ret; int retries = timeout*10; buf[0] = '\0';
That looks ok, but since the latter two variables (in qemudReadLogOutput) are used only from within the while-loop, their declarations belong in that inner scope. O.k. Applied now with that minor change. -- Guido
participants (4)
-
Daniel P. Berrange -
Farkas Levente -
Guido Günther -
Jim Meyering