On Wed, Jun 03, 2009 at 05:42:20PM +0100, Daniel P. Berrange wrote:
On Tue, Jun 02, 2009 at 03:55:27PM +0200, Daniel Veillard wrote:
On Wed, May 27, 2009 at 04:58:44PM +0100, Daniel P. Berrange wrote: [...]
+ xmlXPathContextPtr ctxt) +{ + char *tmp = NULL; + long val; + xmlNodePtr config; + xmlNodePtr oldctxt;
I would s/oldctxt/oldnode/ as what is saved is really only the old XPath current node not the context itself.
Good idea, changed this.
[...]
+char *virDomainObjFormat(virConnectPtr conn, + virDomainObjPtr obj, + int flags) +{ + char *config_xml = NULL, *xml = NULL; + virBuffer buf = VIR_BUFFER_INITIALIZER; + + virBufferVSprintf(&buf, "<domstatus state='%s' pid='%d'>\n", + virDomainStateTypeToString(obj->state), + obj->pid); + virBufferEscapeString(&buf, " <monitor path='%s'/>\n", obj->monitorpath); + + if (!(config_xml = virDomainDefFormat(conn, + obj->def, + flags)))
Hum we are leaking the buffer content here.
+ goto cleanup; + + virBufferAdd(&buf, config_xml, strlen(config_xml)); + virBufferAddLit(&buf, "</domstatus>\n"); + + xml = virBufferContentAndReset(&buf); +cleanup: + VIR_FREE(config_xml); + return xml; + +}
Yes, and also forgetting to check virBufferError() to report OOM. Fixed the cleanup in this function now.
+ virDomainObjUnlock(obj); + virDomainReportError(conn, VIR_ERR_INTERNAL_ERROR, + _("unexpected domain %s already exists"), obj->def->name);
let's wrap to 80 columns
[...]
+/* + * Open an existing VM's monitor, and re-detect VCPUs + * threads
maybe update the comment about the security labels too, especially as this is a bit arcane.
Updated these two.
@@ -1519,10 +1519,8 @@ cleanup: vm->def->graphics[0]->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC && vm->def->graphics[0]->data.vnc.autoport) vm->def->graphics[0]->data.vnc.port = -1; - if (vm->logfile != -1) { - close(vm->logfile); - vm->logfile = -1; - } + if (logfile != -1) + close(logfile); vm->def->id = -1; return -1; }
Hum, do we still use vm->logfile field then ? Maybe I didn't see the place where it was removed from the structure.
Yep, this field in the struct has been killed off - see domain_conf.h
Here's the updated patch in full
ACK, in case you didn't commit it yet ! Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/