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@xxxxxxxxxxxx | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library http://libvirt.org/
commit a7d1eb3c446fdbbd4183ba398c7f21a9fc0c1696
Author: Daniel Veillard <veillard@xxxxxxxxxx>
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:
--
Libvir-list mailing list
Libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list