[libvirt] PATCH: Misc user mode linux startup bugs

2009-06-02 Thread Daniel P. Berrange
There's a number of annoying bugs in the user mode linux driver startup
code. This patch fixes lots of them. We failed to check umlMonitorCommand
for error, leading to a NULL de-reference due to another bug in that
function. This patch also checks if 'res' is NULL before de-referenceing
it, just in case.  In the code for handling inotify delete events, we
duplicated alot of the umlShutdownVMDaemon code, but forgot to close
the monitor. This patches makes it just call umlShutdownVMDaemon directly.
In one place where we spin in a usleep loop, we forgot to increment our
retries counter, making it spin forever. Finally, switch to virKillProcess
and make the latter also avoids pid==1, since I can't  imagine a time 
where we'd ever want to kill 'init' :-)

Daniel

diff --git a/src/uml_driver.c b/src/uml_driver.c
--- a/src/uml_driver.c
+++ b/src/uml_driver.c
@@ -166,9 +166,10 @@ umlIdentifyOneChrPTY(virConnectPtr conn,
 return -1;
 }
 requery:
-umlMonitorCommand(NULL, driver, dom, cmd, res);
+if (umlMonitorCommand(NULL, driver, dom, cmd, res)  0)
+return -1;
 
-if (STRPREFIX(res, pts:)) {
+if (res  STRPREFIX(res, pts:)) {
 VIR_FREE(def-data.file.path);
 if ((def-data.file.path = strdup(res + 4)) == NULL) {
 virReportOOMError(conn);
@@ -176,7 +177,7 @@ requery:
 VIR_FREE(cmd);
 return -1;
 }
-} else if (STRPREFIX(res, pts)) {
+} else if (!res || STRPREFIX(res, pts)) {
 /* It can take a while to startup, so retry for
upto 5 seconds */
 /* XXX should do this in a better non-blocking
@@ -263,19 +264,15 @@ reread:
 }
 
 if (e-mask  IN_DELETE) {
+VIR_DEBUG(Got inotify domain shutdown '%s', name);
 if (!virDomainIsActive(dom)) {
 virDomainObjUnlock(dom);
 continue;
 }
 
-dom-def-id = -1;
-dom-pid = -1;
-if (dom-newDef) {
-virDomainDefFree(dom-def);
-dom-def = dom-newDef;
-}
-dom-state = VIR_DOMAIN_SHUTOFF;
+umlShutdownVMDaemon(NULL, driver, dom);
 } else if (e-mask  (IN_CREATE | IN_MODIFY)) {
+VIR_DEBUG(Got inotify domain startup '%s', name);
 if (virDomainIsActive(dom)) {
 virDomainObjUnlock(dom);
 continue;
@@ -289,11 +286,13 @@ reread:
 dom-def-id = driver-nextvmid++;
 dom-state = VIR_DOMAIN_RUNNING;
 
-if (umlOpenMonitor(NULL, driver, dom)  0)
+if (umlOpenMonitor(NULL, driver, dom)  0) {
+VIR_WARN0(Could not open monitor for new domain);
 umlShutdownVMDaemon(NULL, driver, dom);
-
-if (umlIdentifyChrPTY(NULL, driver, dom)  0)
+} else if (umlIdentifyChrPTY(NULL, driver, dom)  0) {
+VIR_WARN0(Could not identify charater devices for new 
domain);
 umlShutdownVMDaemon(NULL, driver, dom);
+}
 }
 virDomainObjUnlock(dom);
 }
@@ -383,6 +382,7 @@ umlStartup(void) {
 goto error;
 }
 
+VIR_INFO(Adding inotify watch on %s, uml_driver-monitorDir);
 if (inotify_add_watch(uml_driver-inotifyFD,
   uml_driver-monitorDir,
   IN_CREATE | IN_MODIFY | IN_DELETE)  0) {
@@ -595,10 +595,11 @@ static int umlOpenMonitor(virConnectPtr 
 if (umlMonitorAddress(conn, driver, vm, addr)  0)
 return -1;
 
+VIR_DEBUG(Dest address for monitor is '%s', addr.sun_path);
 restat:
 if (stat(addr.sun_path, sb)  0) {
 if (errno == ENOENT 
-retries  50) {
+retries++  50) {
 usleep(1000 * 100);
 goto restat;
 }
@@ -612,7 +613,8 @@ restat:
 }
 
 memset(addr.sun_path, 0, sizeof addr.sun_path);
-sprintf(addr.sun_path + 1, %u, getpid());
+sprintf(addr.sun_path + 1, libvirt-uml-%u, vm-pid);
+VIR_DEBUG(Reply address for monitor is '%s', addr.sun_path+1);
 if (bind(vm-monitor, (struct sockaddr *)addr, sizeof addr)  0) {
 virReportSystemError(conn, errno,
  %s, _(cannot bind socket));
@@ -656,6 +658,8 @@ static int umlMonitorCommand(virConnectP
 int retlen = 0, ret = 0;
 struct sockaddr_un addr;
 unsigned int addrlen;
+
+VIR_DEBUG(Run command '%s', cmd);
 
 *reply = NULL;
 
@@ -706,6 +710,8 @@ static int umlMonitorCommand(virConnectP
 
 } while (res.extra);
 
+VIR_DEBUG(Command reply is '%s', NULLSTR(retdata));
+
 *reply = retdata;
 
 return ret;
@@ -852,12 +881,10 @@ static void umlShutdownVMDaemon(virConne
 virDomainObjPtr vm)
 {
 int ret;
-if (!virDomainIsActive(vm) ||
-vm-pid = 1)
+if (!virDomainIsActive(vm))
 return;
 
-
-kill(vm-pid, SIGTERM);
+virKillProcess(vm-pid, SIGTERM);
 
 if (vm-monitor != -1)
 

Re: [libvirt] PATCH: Misc user mode linux startup bugs

2009-06-02 Thread Daniel Veillard
On Tue, Jun 02, 2009 at 10:50:56AM +0100, Daniel P. Berrange wrote:
 There's a number of annoying bugs in the user mode linux driver startup
 code. This patch fixes lots of them. We failed to check umlMonitorCommand
 for error, leading to a NULL de-reference due to another bug in that
 function. This patch also checks if 'res' is NULL before de-referenceing
 it, just in case.  In the code for handling inotify delete events, we
 duplicated alot of the umlShutdownVMDaemon code, but forgot to close
 the monitor. This patches makes it just call umlShutdownVMDaemon directly.
 In one place where we spin in a usleep loop, we forgot to increment our
 retries counter, making it spin forever. Finally, switch to virKillProcess

  nice one :-)

 and make the latter also avoids pid==1, since I can't  imagine a time 
 where we'd ever want to kill 'init' :-)

  Even better


  looks fine, ACK :-)

Daniel

-- 
Daniel Veillard  | libxml Gnome XML XSLT toolkit  http://xmlsoft.org/
dan...@veillard.com  | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library  http://libvirt.org/

--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list