On Sat, 2006-08-26 at 10:18 -0400, Daniel Veillard wrote:
> On Sat, Aug 26, 2006 at 09:49:05AM -0400, Jeremy Katz wrote:
> > 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.
>
> Honnestly I would prefer to add a new sexpr_fmt_node() routine with
> the snprintf call which would then call the old sexpr_node() and replace
> the few places where you need that functionality with call to sexpr_fmt_node.
> I'm just a bit afraid that people may not realize it's expanded that way
> and pass uncheck strings.
>
> Otherwise it just look fine, I can do the change or you can do it, either
> ways.
Simple enough, attached
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 15:21:05 -0000
@@ -486,3 +486,27 @@
return (n && n->car->kind == SEXPR_VALUE) ? n->car->value : NULL;
}
+
+/**
+ * sexpr_fmt_node:
+ * @sexpr: a pointer to a parsed S-Expression
+ * @fmt: a path for the node to lookup in the S-Expression
+ * @... extra data to build the path
+ *
+ * Search a node value in the S-Expression based on its path
+ * NOTE: path are limited to 4096 bytes.
+ *
+ * Returns the value of the node or NULL if not found.
+ */
+const char *
+sexpr_fmt_node(struct sexpr *sexpr, const char *fmt, ...)
+{
+ va_list ap;
+ char node[4096];
+
+ va_start(ap, fmt);
+ vsnprintf(node, sizeof(node), fmt, ap);
+ va_end(ap);
+
+ return sexpr_node(sexpr, node);
+}
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 15:21:05 -0000
@@ -45,5 +45,6 @@
/* lookup in S-Expressions */
const char *sexpr_node(struct sexpr *sexpr, const char *node);
+const char *sexpr_fmt_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 15:21:07 -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_fmt_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_fmt_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 15:21:08 -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);