On Thu, Dec 03, 2009 at 03:28:02PM +0000, Daniel P. Berrange wrote:
On Thu, Dec 03, 2009 at 04:05:54PM +0100, Daniel Veillard wrote:
On Thu, Nov 26, 2009 at 06:27:29PM +0000, Daniel P. Berrange wrote:
Initial support for the new QEMU monitor protocol using JSON as the data encoding format instead of plain text [...] --- a/src/Makefile.am +++ b/src/Makefile.am @@ -188,6 +188,8 @@ QEMU_DRIVER_SOURCES = \ qemu/qemu_monitor.c qemu/qemu_monitor.h \ qemu/qemu_monitor_text.c \ qemu/qemu_monitor_text.h \ + qemu/qemu_monitor_json.c \ + qemu/qemu_monitor_json.h \ qemu/qemu_driver.c qemu/qemu_driver.h \ qemu/qemu_bridge_filter.c \
Hum could you take the opportunity to cleanup the tab/space mix in the 2 following lines, above ?
[...]
@@ -283,9 +285,14 @@ qemuMonitorIOProcess(qemuMonitorPtr mon) msg = mon->msg;
VIR_DEBUG("Process %d", (int)mon->bufferOffset); - len = qemuMonitorTextIOProcess(mon, - mon->buffer, mon->bufferOffset, - msg); + if (mon->json) + len = qemuMonitorJSONIOProcess(mon, + mon->buffer, mon->bufferOffset, + msg); + else + len = qemuMonitorTextIOProcess(mon, + mon->buffer, mon->bufferOffset, + msg);
I have just one doubt here, assuming we have a json handle, is the text monitor still available. In which case should we try to fallback assuming the json interface get into troubles ? I assume if one fail then the other should fail too but ...
No, QEMU works in an either/or mode - you can't have both active.
okay
+#define LINE_ENDING "\r\n" + +static int +qemuMonitorJSONIOProcessLine(qemuMonitorPtr mon ATTRIBUTE_UNUSED, + const char *line, + qemuMonitorMessagePtr msg) +{ + virJSONValuePtr obj = NULL; + int ret = -1; + + VIR_DEBUG("Line [%s]", line); + + if (!(obj = virJSONValueFromString(line))) { + VIR_DEBUG0("Parsing JSON string failed"); + errno = EINVAL; + goto cleanup; + } + + if (obj->type != VIR_JSON_TYPE_OBJECT) { + VIR_DEBUG0("Parsed JSON string isn't an object"); + errno = EINVAL; + } + + if (virJSONValueObjectHasKey(obj, "QMP") == 1) { + VIR_DEBUG0("Got QMP capabilities data"); + ret = 0; + goto cleanup; + } + + if (virJSONValueObjectHasKey(obj, "event") == 1) { + VIR_DEBUG0("Got an event"); + ret = 0; + goto cleanup; + } + + if (msg) { + msg->rxBuffer = strdup(line); + msg->rxLength = strlen(line); + msg->finished = 1; + } else { + VIR_DEBUG("Ignoring unexpected JSON message [%s]", line); + } + + ret = 0; + +cleanup: + virJSONValueFree(obj); + return ret; +}
I must be missing something in that routine, we parse the json blob, get an object, check some of the object content and discard it, saving the raw text .... seems to me we dropped the actual parsed content instead of handling it, no ?
Yes this is a little bit of a wierd scenario, mostly an artifact of the way we have code sharing between the text & json modes. The shared I/O handling code simply works on char * buffers, and so we can't easily pass back the parsed JSON object at this point. So the object here is only used for detecting events - it is reparsed later on in the place which handles method replies/errors. A little inefficient perhaps, but these messages are very small so its not really too bad
Okay, double parsing but fine, I just wanted to make sure :-)
+ while ((key = va_arg(args, char *)) != NULL) { + int ret; + char type; + + if (strlen(key) < 3) { + qemudReportError(NULL, NULL, NULL, VIR_ERR_INTERNAL_ERROR, + _("argument key '%s' is too short, missing type prefix"), + key); + goto error; + } + + /* Keys look like s:name the first letter is a type code */ + type = key[0]; + key += 2;
Hum, we add a type info on top using prefixing ... weird but why not ...
I originally had 3 values per param in the callers, eg
qemuMonitorJSONMakeCommand("eject", "device, VIR_JSON_VALUE_STRING, "hda", NULL)
but I thought it was getting far too verbose, so I switched to this simple type format character
qemuMonitorJSONMakeCommand("eject", "s:device, "hda", NULL)
Yup that's nicer :-) 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/