With the paravirt framebuffer patches for Xen, we can support having a graphical console for PV as well as HVM guests. The attached adds support to libvirt by breaking out the <graphics> parsing into its own function that can be used by both PV and HVM guests. It also adds support for looking at the appropriate sexpr for the vnc/sdl console for PV guests. One thing that is potentially not okay is that I've changed sexpr_node to take a format string instead of just a const char *, but since it's not public API and doesn't actually change anything for callers I think it should be fine. If not, let me know and I can change it. Jeremy
Index: src/sexpr.c =================================================================== RCS file: /data/cvs/libvirt/src/sexpr.c,v retrieving revision 1.3 diff -u -u -r1.3 sexpr.c --- src/sexpr.c 15 Mar 2006 12:13:25 -0000 1.3 +++ src/sexpr.c 26 Aug 2006 14:42:39 -0000 @@ -472,7 +472,8 @@ /** * sexpr_node: * @sexpr: a pointer to a parsed S-Expression - * @node: a path for the node to lookup in the S-Expression + * @fmt: a path for the node to lookup in the S-Expression + * @...: extra data to build the path for the node * * Search a node value in the S-Expression based on its path * NOTE: path are limited to 4096 bytes. @@ -480,9 +481,17 @@ * Returns the value of the node or NULL if not found. */ const char * -sexpr_node(struct sexpr *sexpr, const char *node) +sexpr_node(struct sexpr *sexpr, const char *fmt, ...) { - struct sexpr *n = sexpr_lookup(sexpr, node); + struct sexpr *n; + va_list ap; + char node[4096]; + + va_start(ap, fmt); + vsnprintf(node, sizeof(node), fmt, ap); + va_end(ap); + + n = sexpr_lookup(sexpr, node); return (n && n->car->kind == SEXPR_VALUE) ? n->car->value : NULL; } Index: src/sexpr.h =================================================================== RCS file: /data/cvs/libvirt/src/sexpr.h,v retrieving revision 1.2 diff -u -u -r1.2 sexpr.h --- src/sexpr.h 15 Mar 2006 12:13:25 -0000 1.2 +++ src/sexpr.h 26 Aug 2006 14:42:39 -0000 @@ -44,6 +44,6 @@ void sexpr_free(struct sexpr *sexpr); /* lookup in S-Expressions */ -const char *sexpr_node(struct sexpr *sexpr, const char *node); +const char *sexpr_node(struct sexpr *sexpr, const char *fmt, ...); struct sexpr *sexpr_lookup(struct sexpr *sexpr, const char *node); #endif Index: src/xend_internal.c =================================================================== RCS file: /data/cvs/libvirt/src/xend_internal.c,v retrieving revision 1.57 diff -u -u -r1.57 xend_internal.c --- src/xend_internal.c 24 Aug 2006 15:05:19 -0000 1.57 +++ src/xend_internal.c 26 Aug 2006 14:42:41 -0000 @@ -1618,23 +1618,23 @@ virBufferAdd(&buf, " <readonly/>\n", 18); virBufferAdd(&buf, " </disk>\n", 12); } + } - /* Graphics device */ - tmp = sexpr_node(root, "domain/image/hvm/vnc"); - if (tmp != NULL) { - if (tmp[0] == '1') { - int port = xenStoreDomainGetVNCPort(conn, domid); - if (port == -1) - port = 5900 + domid; - virBufferVSprintf(&buf, " <graphics type='vnc' port='%d'/>\n", port); - } + /* Graphics device */ + tmp = sexpr_node(root, "domain/image/%s/vnc", hvm ? "hvm" : "linux"); + if (tmp != NULL) { + if (tmp[0] == '1') { + int port = xenStoreDomainGetVNCPort(conn, domid); + if (port == -1) + port = 5900 + domid; + virBufferVSprintf(&buf, " <graphics type='vnc' port='%d'/>\n", port); } + } - tmp = sexpr_node(root, "domain/image/hvm/sdl"); - if (tmp != NULL) { - if (tmp[0] == '1') - virBufferAdd(&buf, " <graphics type='sdl'/>\n", 27 ); - } + tmp = sexpr_node(root, "domain/image/%s/sdl", hvm ? "hvm" : "linux"); + if (tmp != NULL) { + if (tmp[0] == '1') + virBufferAdd(&buf, " <graphics type='sdl'/>\n", 27 ); } tty = xenStoreDomainGetConsolePath(conn, domid); Index: src/xml.c =================================================================== RCS file: /data/cvs/libvirt/src/xml.c,v retrieving revision 1.36 diff -u -u -r1.36 xml.c --- src/xml.c 18 Aug 2006 20:20:50 -0000 1.36 +++ src/xml.c 26 Aug 2006 14:42:41 -0000 @@ -571,6 +571,40 @@ #ifndef PROXY /** + * virtDomainParseXMLGraphicsDesc: + * @node: node containing graphics description + * @buf: a buffer for the result S-Expr + * + * Parse the graphics part of the XML description and add it to the S-Expr + * in buf. This is a temporary interface as the S-Expr interface will be + * replaced by XML-RPC in the future. However the XML format should stay + * valid over time. + * + * Returns 0 in case of success, -1 in case of error + */ +static int virDomainParseXMLGraphicsDesc(xmlNodePtr node, virBufferPtr buf) +{ + xmlChar *graphics_type = NULL; + + graphics_type = xmlGetProp(node, BAD_CAST "type"); + if (graphics_type != NULL) { + if (xmlStrEqual(graphics_type, BAD_CAST "sdl")) { + virBufferAdd(buf, "(sdl 1)", 7); + // TODO: + // Need to understand sdl options + // + //virBufferAdd(buf, "(display localhost:10.0)", 24); + //virBufferAdd(buf, "(xauthority /root/.Xauthority)", 30); + } + else if (xmlStrEqual(graphics_type, BAD_CAST "vnc")) + virBufferAdd(buf, "(vnc 1)", 7); + xmlFree(graphics_type); + } + return 0; +} + + +/** * virDomainParseXMLOSDescHVM: * @node: node containing HVM OS description * @buf: a buffer for the result S-Expr @@ -591,7 +625,7 @@ const xmlChar *type = NULL; const xmlChar *loader = NULL; const xmlChar *boot_dev = NULL; - xmlChar *graphics_type = NULL; + int res; cur = node->children; while (cur != NULL) { @@ -733,24 +767,11 @@ /* Is a graphics device specified? */ obj = xmlXPathEval(BAD_CAST "/domain/devices/graphics[1]", ctxt); if ((obj != NULL) && (obj->type == XPATH_NODESET) && - (obj->nodesetval != NULL) && (obj->nodesetval->nodeNr = 0)) { - virXMLError(VIR_ERR_NO_OS, "", 0); /* TODO: error */ - goto error; - } - - graphics_type = xmlGetProp(obj->nodesetval->nodeTab[0], BAD_CAST "type"); - if (graphics_type != NULL) { - if (xmlStrEqual(graphics_type, BAD_CAST "sdl")) { - virBufferAdd(buf, "(sdl 1)", 7); - // TODO: - // Need to understand sdl options - // - //virBufferAdd(buf, "(display localhost:10.0)", 24); - //virBufferAdd(buf, "(xauthority /root/.Xauthority)", 30); + (obj->nodesetval != NULL) && (obj->nodesetval->nodeNr > 0)) { + res = virDomainParseXMLGraphicsDesc(obj->nodesetval->nodeTab[0], buf); + if (res != 0) { + goto error; } - else if (xmlStrEqual(graphics_type, BAD_CAST "vnc")) - virBufferAdd(buf, "(vnc 1)", 7); - xmlFree(graphics_type); } xmlXPathFreeObject(obj); @@ -767,6 +788,7 @@ * virDomainParseXMLOSDescPV: * @node: node containing PV OS description * @buf: a buffer for the result S-Expr + * @ctxt: a path context representing the XML description * * Parse the OS part of the XML description for a paravirtualized domain * and add it to the S-Expr in buf. This is a temporary interface as the @@ -776,14 +798,16 @@ * Returns 0 in case of success, -1 in case of error. */ static int -virDomainParseXMLOSDescPV(xmlNodePtr node, virBufferPtr buf) +virDomainParseXMLOSDescPV(xmlNodePtr node, virBufferPtr buf, xmlXPathContextPtr ctxt) { xmlNodePtr cur, txt; + xmlXPathObjectPtr obj = NULL; const xmlChar *type = NULL; const xmlChar *root = NULL; const xmlChar *kernel = NULL; const xmlChar *initrd = NULL; const xmlChar *cmdline = NULL; + int res; cur = node->children; while (cur != NULL) { @@ -840,6 +864,19 @@ virBufferVSprintf(buf, "(root '%s')", (const char *) root); if (cmdline != NULL) virBufferVSprintf(buf, "(args '%s')", (const char *) cmdline); + + /* Is a graphics device specified? */ + obj = xmlXPathEval(BAD_CAST "/domain/devices/graphics[1]", ctxt); + if ((obj != NULL) && (obj->type == XPATH_NODESET) && + (obj->nodesetval != NULL) && (obj->nodesetval->nodeNr > 0)) { + res = virDomainParseXMLGraphicsDesc(obj->nodesetval->nodeTab[0], buf); + if (res != 0) { + goto error; + } + } + xmlXPathFreeObject(obj); + + error: virBufferAdd(buf, "))", 2); return (0); } @@ -1177,7 +1214,7 @@ } if ((tmpobj == NULL) || !xmlStrEqual(tmpobj->stringval, BAD_CAST "hvm")) { - res = virDomainParseXMLOSDescPV(obj->nodesetval->nodeTab[0], &buf); + res = virDomainParseXMLOSDescPV(obj->nodesetval->nodeTab[0], &buf, ctxt); } else { hvm = 1; res = virDomainParseXMLOSDescHVM(obj->nodesetval->nodeTab[0], &buf, ctxt);