3:42 p.m.
"Daniel P. Berrange" <berrange(a)redhat.com> wrote:
On Fri, Apr 18, 2008 at 09:27:05PM +0100, Daniel P. Berrange wrote:
> This patch updates Xen HVM to allow use of serial ¶llel ports, though
> XenD limits you to single one of each even though QEMU supports many.
> It also updates the <console> tag to support new syntax extensions.
This version fixes a few memory leaks in the previous code...
Hi Dan,
Index: src/xend_internal.c
===================================================================
RCS file: /data/cvs/libvirt/src/xend_internal.c,v
retrieving revision 1.180
diff -u -p -r1.180 xend_internal.c
--- src/xend_internal.c 10 Apr 2008 16:54:54 -0000 1.180
+++ src/xend_internal.c 24 Apr 2008 15:10:00 -0000
@@ -1376,6 +1376,243 @@ xend_parse_sexp_desc_os(virConnectPtr xe
return(0);
}
+
+int
+xend_parse_sexp_desc_char(virConnectPtr conn,
+ virBufferPtr buf,
+ const char *devtype,
+ int portNum,
+ const char *value,
+ const char *tty)
+{
+ const char *type;
+ int telnet = 0;
+ char *bindPort = NULL;
+ char *bindHost = NULL;
+ char *connectPort = NULL;
+ char *connectHost = NULL;
+ char *path = NULL;
+ int ret = -1;
+
+ if (value[0] == '/') {
+ type = "dev";
+ } else if (STREQLEN(value, "null", 4)) {
+ type = "null";
+ value = NULL;
+ } else if (STREQLEN(value, "vc", 2)) {
+ type = "vc";
+ value = NULL;
+ } else if (STREQLEN(value, "pty", 3)) {
+ type = "pty";
+ value = NULL;
+ } else if (STREQLEN(value, "stdio", 5)) {
+ type = "stdio";
+ value = NULL;
+ } else if (STREQLEN(value, "file:", 5)) {
+ type = "file";
+ value += 5;
+ } else if (STREQLEN(value, "pipe:", 5)) {
+ type = "pipe";
+ value += 5;
+ } else if (STREQLEN(value, "tcp:", 4)) {
+ type = "tcp";
+ value += 4;
+ } else if (STREQLEN(value, "telnet:", 4)) {
That "4" should be 7.
It'd be nice to avoid the riskily redundant length,
by using a STR* macro that requires a literal string S
as argument #2, and uses sizeof(S)-1 as the length argument.
This code could be factored even more with a loop.
The only special cases would be value[0]=='/' and
"telnet:" having type of "tcp" and setting telnet=1.
+ type = "tcp";
+ value += 7;
+ telnet = 1;
+ } else if (STREQLEN(value, "udp:", 4)) {
+ type = "udp";
+ value += 4;
+ } else if (STREQLEN(value, "unix:", 5)) {
+ type = "unix";
+ value += 5;
+ } else {
+ virXendError(conn, VIR_ERR_INTERNAL_ERROR,
+ _("Unknown char device type"));
+ return -1;
+ }
+
...
+ } else if (STREQ(type, "unix")) {
+ const char *offset = strchr(value, ',');
+ int dolisten = 0;
+ if (offset)
+ path = strndup(value, (offset - value));
+ else
+ path = strdup(value);
+ if (path == NULL)
+ goto no_memory;
+
+ if (strstr(offset, ",listen") != NULL)
If the strchr call above finds no comma, then
"offset" will be NULL and strstr call will segfault.
...
+ tmp = sexpr_node(root,
"domain/image/hvm/parallel");
+ if (tmp && STRNEQ(tmp, "none")) {
+ /* XXX does XenD stuff parallel port tty info into xenstore somewhere ? */
+ if (xend_parse_sexp_desc_char(conn, &buf, "parallel", 0, tmp,
NULL) < 0)
+ goto error;
+ }
+ } else {
+ /* Paravirt always has a console */
+ if (tty) {
+ virBufferVSprintf(&buf, " <console type='pty'
tty='%s'>\n", tty);
+ virBufferVSprintf(&buf, " <source
path='%s'/>\n", tty);
+ } else {
+ virBufferAddLit(&buf, " <console
type='pty'>\n");
+ }
+ virBufferAddLit(&buf, " <target
port='0'/>\n");
+ virBufferAddLit(&buf, " </console>\n");
}
+ free(tty);
virBufferAddLit(&buf, " </devices>\n");
virBufferAddLit(&buf, "</domain>\n");
All of the virBufferAddLit and virBufferVSprintf calls
above can fail with ENOMEM. I see that there are *many* more
virBufferAddLit and virBufferVSprintf calls that ignore their
return values (over 300), so maybe I'm missing something.
Index: src/xend_internal.h
===================================================================
RCS file: /data/cvs/libvirt/src/xend_internal.h,v
retrieving revision 1.40
diff -u -p -r1.40 xend_internal.h
--- src/xend_internal.h 10 Apr 2008 16:54:54 -0000 1.40
+++ src/xend_internal.h 24 Apr 2008 15:10:00 -0000
@@ -20,12 +20,12 @@
#include "libvirt/libvirt.h"
#include "capabilities.h"
+#include "buf.h"
#ifdef __cplusplus
extern "C" {
#endif
-
/**
* \brief Setup the connection to a xend instance via TCP
* \param host The host name to connect to
@@ -180,6 +180,13 @@ char *xenDaemonDomainDumpXMLByName(virCo
*/
int xend_log(virConnectPtr xend, char *buffer, size_t n_buffer);
+ int xend_parse_sexp_desc_char(virConnectPtr conn,
+ virBufferPtr buf,
+ const char *devtype,
+ int portNum,
+ const char *value,
+ const char *tty);
+
char *xend_parse_domain_sexp(virConnectPtr conn, char *root, int xendConfigVersion);
/* refactored ones */
Index: src/xm_internal.c
===================================================================
RCS file: /data/cvs/libvirt/src/xm_internal.c,v
retrieving revision 1.70
diff -u -p -r1.70 xm_internal.c
--- src/xm_internal.c 10 Apr 2008 16:54:54 -0000 1.70
+++ src/xm_internal.c 24 Apr 2008 15:10:01 -0000
@@ -1025,11 +1025,22 @@ char *xenXMDomainFormatXML(virConnectPtr
}
if (hvm) {
- if (xenXMConfigGetString(conf, "serial", &str) == 0 &&
!strcmp(str, "pty")) {
- virBufferAddLit(buf, " <console/>\n");
+ if (xenXMConfigGetString(conf, "parallel", &str) == 0) {
+ if (STRNEQ(str, "none"))
+ xend_parse_sexp_desc_char(conn, buf, "parallel", 0, str,
NULL);
+ }
+ if (xenXMConfigGetString(conf, "serial", &str) == 0) {
+ if (STRNEQ(str, "none")) {
+ xend_parse_sexp_desc_char(conn, buf, "serial", 0, str, NULL);
+ /* Add back-compat console tag for primary console */
+ xend_parse_sexp_desc_char(conn, buf, "console", 0, str,
NULL);
+ }
}
- } else { /* Paravirt implicitly always has a console */
- virBufferAddLit(buf, " <console/>\n");
+ } else {
+ /* Paravirt implicitly always has a single console */
+ virBufferAddLit(buf, " <console type='pty'>\n");
+ virBufferAddLit(buf, " <target port='0'/>\n");
+ virBufferAddLit(buf, " </console>\n");
}
virBufferAddLit(buf, " </devices>\n");
@@ -2267,14 +2278,38 @@ virConfPtr xenXMParseXMLToConfig(virConn
obj = NULL;
if (hvm) {
- obj = xmlXPathEval(BAD_CAST "count(/domain/devices/console) > 0",
ctxt);
- if ((obj != NULL) && (obj->type == XPATH_BOOLEAN) &&
- (obj->boolval)) {
- if (xenXMConfigSetString(conf, "serial", "pty") < 0)
+ xmlNodePtr cur;
+ cur = virXPathNode("/domain/devices/parallel[1]", ctxt);
+ if (cur != NULL) {
+ char scratch[PATH_MAX];
+
+ if (virDomainParseXMLOSDescHVMChar(conn, scratch, sizeof(scratch), cur) <
0) {
+ goto error;
+ }
+
+ if (xenXMConfigSetString(conf, "parallel", scratch) < 0)
+ goto error;
+ } else {
+ if (xenXMConfigSetString(conf, "parallel", "none") <
0)
goto error;
}
- xmlXPathFreeObject(obj);
- obj = NULL;
+
+ cur = virXPathNode("/domain/devices/serial[1]", ctxt);
+ if (cur != NULL) {
+ char scratch[PATH_MAX];
+ if (virDomainParseXMLOSDescHVMChar(conn, scratch, sizeof(scratch), cur) <
0)
+ goto error;
+ if (xenXMConfigSetString(conf, "serial", scratch) < 0)
+ goto error;
+ } else {
+ if (virXPathBoolean("count(/domain/devices/console) > 0",
ctxt)) {
+ if (xenXMConfigSetString(conf, "serial", "pty") <
0)
+ goto error;
+ } else {
+ if (xenXMConfigSetString(conf, "serial", "none")
< 0)
+ goto error;
+ }
+ }
}
xmlFreeDoc(doc);
Index: src/xml.c
===================================================================
RCS file: /data/cvs/libvirt/src/xml.c,v
retrieving revision 1.117
diff -u -p -r1.117 xml.c
--- src/xml.c 10 Apr 2008 16:54:54 -0000 1.117
+++ src/xml.c 24 Apr 2008 15:10:01 -0000
@@ -673,6 +673,178 @@ virDomainParseXMLGraphicsDescVFB(virConn
}
+int
+virDomainParseXMLOSDescHVMChar(virConnectPtr conn,
+ char *buf,
+ size_t buflen,
+ xmlNodePtr node)
+{
+ xmlChar *type = NULL;
Can you use "const char *" above, instead of "xmlChar *"?
If so, that'd let you remove most of those risky and
ugly (const char*) casts below.
Ditto for all of these others.
+ xmlChar *path = NULL;
+ xmlChar *bindHost = NULL;
+ xmlChar *bindService = NULL;
+ xmlChar *connectHost = NULL;
+ xmlChar *connectService = NULL;
+ xmlChar *mode = NULL;
+ xmlChar *protocol = NULL;
+ xmlNodePtr cur;
+
+ type = xmlGetProp(node, BAD_CAST "type");
+
+ if (type != NULL) {
+ cur = node->children;
+ while (cur != NULL) {
+ if (cur->type == XML_ELEMENT_NODE) {
+ if (xmlStrEqual(cur->name, BAD_CAST "source")) {
+ if (mode == NULL)
+ mode = xmlGetProp(cur, BAD_CAST "mode");
+
+ if (STREQ((const char *)type, "dev") ||
+ STREQ((const char *)type, "file") ||
+ STREQ((const char *)type, "pipe") ||
+ STREQ((const char *)type, "unix")) {
+ if (path == NULL)
+ path = xmlGetProp(cur, BAD_CAST "path");
+
+ } else if (STREQ((const char *)type, "udp") ||
+ STREQ((const char *)type, "tcp")) {
+ if (mode == NULL ||
+ STREQ((const char *)mode, "connect")) {
+
+ if (connectHost == NULL)
+ connectHost = xmlGetProp(cur, BAD_CAST
"host");
+ if (connectService == NULL)
+ connectService = xmlGetProp(cur, BAD_CAST
"service");
+ } else {
+ if (bindHost == NULL)
+ bindHost = xmlGetProp(cur, BAD_CAST "host");
+ if (bindService == NULL)
+ bindService = xmlGetProp(cur, BAD_CAST
"service");
+ }
+
+ if (STREQ((const char*)type, "udp")) {
+ xmlFree(mode);
+ mode = NULL;
+ }
+ }
+ } else if (xmlStrEqual(cur->name, BAD_CAST "protocol")) {
+ if (protocol == NULL)
+ protocol = xmlGetProp(cur, BAD_CAST "type");
+ }
+ }
+ cur = cur->next;
+ }
+ }
+
+ if (type == NULL ||
+ STREQ((const char *)type, "pty")) {
+ strncpy(buf, "pty", buflen);
+ } else if (STREQ((const char *)type, "null") ||
+ STREQ((const char *)type, "stdio") ||
+ STREQ((const char *)type, "vc")) {
+ snprintf(buf, buflen, "%s", type);
+ } else if (STREQ((const char *)type, "file") ||
+ STREQ((const char *)type, "dev") ||
+ STREQ((const char *)type, "pipe")) {
...