[libvirt] [PATCH 3/3] Get QEMU pty paths from the monitor

[Daniel Veillard]

On Wed, Nov 25, 2009 at 11:31:37AM +0000, Daniel P. Berrange wrote:
> On Mon, Nov 23, 2009 at 12:30:29PM +0000, Matthew Booth wrote:
> > @@ -1291,7 +1330,7 @@ qemudWaitForMonitor(virConnectPtr conn,
> >  {
> >      char buf[4096]; /* Plenty of space to get startup greeting */
> >      int logfd;
> > -    int ret;
> > +    int ret = -1;
> >  
> >      if ((logfd = qemudLogReadFD(conn, driver->logDir, vm->def->name, pos))
> >          < 0)
> > @@ -1317,7 +1356,32 @@ qemudWaitForMonitor(virConnectPtr conn,
> >      if (qemuConnectMonitor(vm) < 0)
> >          return -1;
> >  
> > -    return 0;
> > +    /* Try to get the pty path mappings again via the monitor. This is much more
> > +     * reliable if it's available.
> > +     * Note that the monitor itself can be on a pty, so we still need to try the
> > +     * log output method. */
> > +    virHashTablePtr paths = virHashCreate(0);
> > +    if (paths == NULL) {
> > +        virReportOOMError(NULL);
> > +        goto cleanup;
> > +    }
> > +
> > +    qemuDomainObjEnterMonitor(vm);
> 
> This needs to be EnterMonitorWithDriver(driver, vm), since the 'driver'
> is locked in this context
> 
> > +    qemuDomainObjPrivatePtr priv = vm->privateData;
> > +    ret = qemuMonitorGetPtyPaths(priv->mon, paths);
> > +    qemuDomainObjExitMonitor(vm);
> 
> And ExitMonitorWithDriver


  Loooks serious and was missing fro last patch so I made the change


> > +        /* Path is everything after needle to the end of the line */
> > +        *eol = '\0';
> > +        char *path = needle + strlen(NEEDLE);
> > +
> > +        virHashAddEntry(paths, id, strdup(path));
> 
> Not checking OOM on strdup() here, or for failure of virHashAddEntry()

  I fixed that too, I pushed the attached patch to clean those 2 issues,
please check :-)

Daniel

-- 
Daniel Veillard      | libxml Gnome XML XSLT toolkit  http://xmlsoft.org/
daniel at veillard.com  | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library  http://libvirt.org/
-------------- next part --------------
commit a7d1eb3c446fdbbd4183ba398c7f21a9fc0c1696
Author: Daniel Veillard <veillard at redhat.com>
Date:   Mon Dec 14 11:05:55 2009 +0100

    Fix a couple of problems in last patch
    
    Those were pointed by DanB in his review but not yet fixed
    
    * src/qemu/qemu_driver.c: qemudWaitForMonitor() use EnterMonitorWithDriver()
      and ExitMonitorWithDriver() there
    * src/qemu/qemu_monitor_text.c: checking fro strdu failure and hash
      table add error in qemuMonitorTextGetPtyPaths()

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 7b8c447..5a61b8c 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -1534,10 +1534,10 @@ qemudWaitForMonitor(virConnectPtr conn,
         goto cleanup;
     }
 
-    qemuDomainObjEnterMonitor(vm);
+    qemuDomainObjEnterMonitorWithDriver(driver, vm);
     qemuDomainObjPrivatePtr priv = vm->privateData;
     ret = qemuMonitorGetPtyPaths(priv->mon, paths);
-    qemuDomainObjExitMonitor(vm);
+    qemuDomainObjExitMonitorWithDriver(driver, vm);
 
     VIR_DEBUG("qemuMonitorGetPtyPaths returned %i", ret);
     if (ret == 0) {
diff --git a/src/qemu/qemu_monitor_text.c b/src/qemu/qemu_monitor_text.c
index 3ed45ba..0cb9ea6 100644
--- a/src/qemu/qemu_monitor_text.c
+++ b/src/qemu/qemu_monitor_text.c
@@ -1650,9 +1650,19 @@ int qemuMonitorTextGetPtyPaths(qemuMonitorPtr mon,
 
         /* Path is everything after needle to the end of the line */
         *eol = '\0';
-        char *path = needle + strlen(NEEDLE);
+        char *path = strdup(needle + strlen(NEEDLE));
+        if (path == NULL) {
+            virReportOOMError(NULL);
+            goto cleanup;
+        }
 
-        virHashAddEntry(paths, id, strdup(path));
+        if (virHashAddEntry(paths, id, path) < 0) {
+            qemudReportError(NULL, NULL, NULL, VIR_ERR_OPERATION_FAILED,
+                             _("failed to save chardev path '%s'"),
+                             path);
+            VIR_FREE(path);
+            goto cleanup;
+        }
 #undef NEEDLE
 
     next: