This introduces a new domain VIR_DOMAIN_EVENT_ID_CONTROL_ERROR Which uses the existing generic callback typedef void (*virConnectDomainEventGenericCallback)(virConnectPtr conn, virDomainPtr dom, void *opaque); This event is intended to be emitted when there is a failure in some part of the domain virtualization system. Whether the domain continues to run/exist after the failure is an implementation detail specific to the hypervisor. The idea is that with some types of failure, hypervisors may prefer to leave the domain running in a "degraded" mode of operation. For example, if something goes wrong with the QEMU monitor, it is possible to leave the guest OS running quite happily. The mgmt app will simply loose the ability todo various tasks. The mgmt app can then choose how/when to deal with the failure that occured. * daemon/remote.c: Dispatch of new event * examples/domain-events/events-c/event-test.c: Demo catch of event * include/libvirt/libvirt.h.in: Define event ID and callback * src/conf/domain_event.c, src/conf/domain_event.h: Internal event handling * src/remote/remote_driver.c: Receipt of new event from daemon * src/remote/remote_protocol.x: Wire protocol for new event --- daemon/remote.c | 29 ++++++++++++++++++++++++ examples/domain-events/events-c/event-test.c | 19 ++++++++++++++++ include/libvirt/libvirt.h.in | 1 + python/libvirt-override.c | 3 ++ src/conf/domain_event.c | 23 +++++++++++++++++++ src/conf/domain_event.h | 2 + src/libvirt_private.syms | 2 + src/remote/remote_driver.c | 31 ++++++++++++++++++++++++++ src/remote/remote_protocol.x | 7 +++++- 9 files changed, 116 insertions(+), 1 deletions(-) diff --git a/daemon/remote.c b/daemon/remote.c index 80783b3..59a7377 100644 --- a/daemon/remote.c +++ b/daemon/remote.c @@ -380,6 +380,34 @@ static int remoteRelayDomainEventGraphics(virConnectPtr conn ATTRIBUTE_UNUSED, } +static int remoteRelayDomainEventControlError(virConnectPtr conn ATTRIBUTE_UNUSED, + virDomainPtr dom, + void *opaque) +{ + struct qemud_client *client = opaque; + remote_domain_event_control_error_msg data; + + if (!client) + return -1; + + VIR_DEBUG("Relaying domain control error %s %d", dom->name, dom->id); + + virMutexLock(&client->lock); + + /* build return data */ + memset(&data, 0, sizeof data); + make_nonnull_domain(&data.dom, dom); + + remoteDispatchDomainEventSend(client, + REMOTE_PROC_DOMAIN_EVENT_CONTROL_ERROR, + (xdrproc_t)xdr_remote_domain_event_control_error_msg, &data); + + virMutexUnlock(&client->lock); + + return 0; +} + + static virConnectDomainEventGenericCallback domainEventCallbacks[] = { VIR_DOMAIN_EVENT_CALLBACK(remoteRelayDomainEventLifecycle), VIR_DOMAIN_EVENT_CALLBACK(remoteRelayDomainEventReboot), @@ -388,6 +416,7 @@ static virConnectDomainEventGenericCallback domainEventCallbacks[] = { VIR_DOMAIN_EVENT_CALLBACK(remoteRelayDomainEventIOError), VIR_DOMAIN_EVENT_CALLBACK(remoteRelayDomainEventGraphics), VIR_DOMAIN_EVENT_CALLBACK(remoteRelayDomainEventIOErrorReason), + VIR_DOMAIN_EVENT_CALLBACK(remoteRelayDomainEventControlError), }; verify(ARRAY_CARDINALITY(domainEventCallbacks) == VIR_DOMAIN_EVENT_ID_LAST); diff --git a/examples/domain-events/events-c/event-test.c b/examples/domain-events/events-c/event-test.c index 2da58b8..4766a0d 100644 --- a/examples/domain-events/events-c/event-test.c +++ b/examples/domain-events/events-c/event-test.c @@ -245,6 +245,17 @@ static int myDomainEventGraphicsCallback(virConnectPtr conn ATTRIBUTE_UNUSED, return 0; } +static int myDomainEventControlErrorCallback(virConnectPtr conn ATTRIBUTE_UNUSED, + virDomainPtr dom, + void *opaque ATTRIBUTE_UNUSED) +{ + printf("%s EVENT: Domain %s(%d) control error\n", __func__, virDomainGetName(dom), + virDomainGetID(dom)); + + return 0; +} + + static void myFreeFunc(void *opaque) { char *str = opaque; @@ -278,6 +289,7 @@ int main(int argc, char **argv) int callback5ret = -1; int callback6ret = -1; int callback7ret = -1; + int callback8ret = -1; struct sigaction action_stop; memset(&action_stop, 0, sizeof action_stop); @@ -336,6 +348,11 @@ int main(int argc, char **argv) VIR_DOMAIN_EVENT_ID_GRAPHICS, VIR_DOMAIN_EVENT_CALLBACK(myDomainEventGraphicsCallback), strdup("callback graphics"), myFreeFunc); + callback8ret = virConnectDomainEventRegisterAny(dconn, + NULL, + VIR_DOMAIN_EVENT_ID_CONTROL_ERROR, + VIR_DOMAIN_EVENT_CALLBACK(myDomainEventControlErrorCallback), + strdup("callback control error"), myFreeFunc); if ((callback1ret != -1) && (callback2ret != -1) && @@ -360,6 +377,8 @@ int main(int argc, char **argv) virConnectDomainEventDeregisterAny(dconn, callback5ret); virConnectDomainEventDeregisterAny(dconn, callback6ret); virConnectDomainEventDeregisterAny(dconn, callback7ret); + if (callback8ret != -1) + virConnectDomainEventDeregisterAny(dconn, callback8ret); } VIR_DEBUG("Closing connection"); diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index 7cd6e13..448951f 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -2525,6 +2525,7 @@ typedef enum { VIR_DOMAIN_EVENT_ID_IO_ERROR = 4, /* virConnectDomainEventIOErrorCallback */ VIR_DOMAIN_EVENT_ID_GRAPHICS = 5, /* virConnectDomainEventGraphicsCallback */ VIR_DOMAIN_EVENT_ID_IO_ERROR_REASON = 6, /* virConnectDomainEventIOErrorReasonCallback */ + VIR_DOMAIN_EVENT_ID_CONTROL_ERROR = 7, /* virConnectDomainEventGenericCallback */ /* * NB: this enum value will increase over time as new events are diff --git a/python/libvirt-override.c b/python/libvirt-override.c index a151e78..d15a865 100644 --- a/python/libvirt-override.c +++ b/python/libvirt-override.c @@ -3485,6 +3485,9 @@ libvirt_virConnectDomainEventRegisterAny(ATTRIBUTE_UNUSED PyObject * self, case VIR_DOMAIN_EVENT_ID_GRAPHICS: cb = VIR_DOMAIN_EVENT_CALLBACK(libvirt_virConnectDomainEventGraphicsCallback); break; + case VIR_DOMAIN_EVENT_ID_CONTROL_ERROR: + cb = VIR_DOMAIN_EVENT_CALLBACK(libvirt_virConnectDomainEventGenericCallback); + break; } if (!cb) { diff --git a/src/conf/domain_event.c b/src/conf/domain_event.c index b85765e..34a9d91 100644 --- a/src/conf/domain_event.c +++ b/src/conf/domain_event.c @@ -875,6 +875,24 @@ virDomainEventPtr virDomainEventGraphicsNewFromObj(virDomainObjPtr obj, } +virDomainEventPtr virDomainEventControlErrorNewFromDom(virDomainPtr dom) +{ + virDomainEventPtr ev = + virDomainEventNewInternal(VIR_DOMAIN_EVENT_ID_CONTROL_ERROR, + dom->id, dom->name, dom->uuid); + return ev; +} + + +virDomainEventPtr virDomainEventControlErrorNewFromObj(virDomainObjPtr obj) +{ + virDomainEventPtr ev = + virDomainEventNewInternal(VIR_DOMAIN_EVENT_ID_CONTROL_ERROR, + obj->def->id, obj->def->name, obj->def->uuid); + return ev; +} + + /** * virDomainEventQueuePop: * @evtQueue: the queue of events @@ -1004,6 +1022,11 @@ void virDomainEventDispatchDefaultFunc(virConnectPtr conn, cbopaque); break; + case VIR_DOMAIN_EVENT_ID_CONTROL_ERROR: + (cb)(conn, dom, + cbopaque); + break; + default: VIR_WARN("Unexpected event ID %d", event->eventID); break; diff --git a/src/conf/domain_event.h b/src/conf/domain_event.h index efc05f9..68dc8a8 100644 --- a/src/conf/domain_event.h +++ b/src/conf/domain_event.h @@ -167,6 +167,8 @@ virDomainEventPtr virDomainEventGraphicsNewFromObj(virDomainObjPtr obj, virDomainEventGraphicsAddressPtr remote, const char *authScheme, virDomainEventGraphicsSubjectPtr subject); +virDomainEventPtr virDomainEventControlErrorNewFromDom(virDomainPtr dom); +virDomainEventPtr virDomainEventControlErrorNewFromObj(virDomainObjPtr obj); diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 7b6151c..6036afe 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -370,6 +370,8 @@ virDomainEventCallbackListPurgeMarked; virDomainEventCallbackListRemove; virDomainEventCallbackListRemoveConn; virDomainEventCallbackListRemoveID; +virDomainEventControlErrorNewFromDom; +virDomainEventControlErrorNewFromObj; virDomainEventDispatch; virDomainEventDispatchDefaultFunc; virDomainEventFree; diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index 8c69743..97e9e2f 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -4093,6 +4093,33 @@ no_memory: } +static virDomainEventPtr +remoteDomainReadEventControlError(virConnectPtr conn, XDR *xdr) +{ + remote_domain_event_control_error_msg msg; + virDomainPtr dom; + virDomainEventPtr event = NULL; + memset (&msg, 0, sizeof msg); + + /* unmarshall parameters, and process it*/ + if (! xdr_remote_domain_event_control_error_msg(xdr, &msg) ) { + remoteError(VIR_ERR_RPC, "%s", + _("unable to demarshall reboot event")); + return NULL; + } + + dom = get_nonnull_domain(conn,msg.dom); + if (!dom) + return NULL; + + event = virDomainEventControlErrorNewFromDom(dom); + xdr_free ((xdrproc_t) &xdr_remote_domain_event_control_error_msg, (char *) &msg); + + virDomainFree(dom); + return event; +} + + static virDrvOpenStatus ATTRIBUTE_NONNULL (1) remoteSecretOpen(virConnectPtr conn, virConnectAuthPtr auth, int flags) { @@ -5903,6 +5930,10 @@ processCallDispatchMessage(virConnectPtr conn, struct private_data *priv, event = remoteDomainReadEventGraphics(conn, xdr); break; + case REMOTE_PROC_DOMAIN_EVENT_CONTROL_ERROR: + event = remoteDomainReadEventControlError(conn, xdr); + break; + default: VIR_DEBUG("Unexpected event proc %d", hdr->proc); break; diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x index f0da95d..61f994f 100644 --- a/src/remote/remote_protocol.x +++ b/src/remote/remote_protocol.x @@ -2042,6 +2042,10 @@ struct remote_domain_migrate_confirm3_args { int cancelled; }; +struct remote_domain_event_control_error_msg { + remote_nonnull_domain dom; +}; + /*----- Protocol. -----*/ /* Define the program number, protocol version and procedure numbers here. */ @@ -2291,7 +2295,8 @@ enum remote_procedure { REMOTE_PROC_DOMAIN_MIGRATE_PERFORM3 = 216, /* skipgen skipgen */ REMOTE_PROC_DOMAIN_MIGRATE_FINISH3 = 217, /* skipgen skipgen */ REMOTE_PROC_DOMAIN_MIGRATE_CONFIRM3 = 218, /* skipgen skipgen */ - REMOTE_PROC_DOMAIN_SET_SCHEDULER_PARAMETERS_FLAGS = 219 /* skipgen skipgen */ + REMOTE_PROC_DOMAIN_SET_SCHEDULER_PARAMETERS_FLAGS = 219, /* skipgen skipgen */ + REMOTE_PROC_DOMAIN_EVENT_CONTROL_ERROR = 220 /* skipgen skipgen */ /* * Notice how the entries are grouped in sets of 10 ? -- 1.7.4.4

On Tue, May 24, 2011 at 18:06:42 +0100, Daniel P. Berrange wrote: ... I don't think this change is correct. Wee need to correct reason to be passed to qemuProcessStop so that virDomainGetState can report it. This way we would always report normal shutdown. Jirka

Currently the QEMU monitor I/O handler code uses errno values to report errors. This results in a sub-optimal error messages on certain conditions, in particular when parsing JSON strings malformed data simply results in 'EINVAL'. This changes the code to use the standard libvirt error reporting APIs. The virError is stored against the qemuMonitorPtr struct, and when a monitor API is run, any existing stored error is copied into that thread's error local * src/qemu/qemu_monitor.c, src/qemu/qemu_monitor.h, src/qemu/qemu_monitor_json.c, src/qemu/qemu_monitor_text.c: Use virError APIs for all monitor I/O handling code --- src/qemu/qemu_monitor.c | 161 ++++++++++++++++++++++++++++-------------- src/qemu/qemu_monitor.h | 11 ++- src/qemu/qemu_monitor_json.c | 85 +++++++++++----------- src/qemu/qemu_monitor_text.c | 51 +++++++------- 4 files changed, 183 insertions(+), 125 deletions(-) diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index 28e5252..09a1b8e 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -69,7 +69,9 @@ struct _qemuMonitor { /* If anything went wrong, this will be fed back * the next monitor msg */ - int lastErrno; + virError lastError; + + int nextSerial; unsigned json: 1; unsigned json_hmp: 1; @@ -312,6 +314,10 @@ qemuMonitorOpenPty(const char *monitor) } +/* This method processes data that has been received + * from the monitor. Looking for async events and + * replies/errors. + */ static int qemuMonitorIOProcess(qemuMonitorPtr mon) { @@ -344,10 +350,8 @@ qemuMonitorIOProcess(qemuMonitorPtr mon) mon->buffer, mon->bufferOffset, msg); - if (len < 0) { - mon->lastErrno = errno; + if (len < 0) return -1; - } if (len < mon->bufferOffset) { memmove(mon->buffer, mon->buffer + len, mon->bufferOffset - len); @@ -378,11 +382,6 @@ qemuMonitorIOWriteWithFD(qemuMonitorPtr mon, char control[CMSG_SPACE(sizeof(int))]; struct cmsghdr *cmsg; - if (!mon->hasSendFD) { - errno = EINVAL; - return -1; - } - memset(&msg, 0, sizeof(msg)); iov[0].iov_base = (void *)data; @@ -423,6 +422,12 @@ qemuMonitorIOWrite(qemuMonitorPtr mon) if (!mon->msg || mon->msg->txOffset == mon->msg->txLength) return 0; + if (mon->msg->txFD != -1 && !mon->hasSendFD) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Monitor does not support sending of file descriptors")); + return -1; + } + if (mon->msg->txFD == -1) done = write(mon->fd, mon->msg->txBuffer + mon->msg->txOffset, @@ -437,7 +442,8 @@ qemuMonitorIOWrite(qemuMonitorPtr mon) if (errno == EAGAIN) return 0; - mon->lastErrno = errno; + virReportSystemError(errno, "%s", + _("Unable to write to monitor")); return -1; } mon->msg->txOffset += done; @@ -459,7 +465,7 @@ qemuMonitorIORead(qemuMonitorPtr mon) if (avail < 1024) { if (VIR_REALLOC_N(mon->buffer, mon->bufferLength + 1024) < 0) { - errno = ENOMEM; + virReportOOMError(); return -1; } mon->bufferLength += 1024; @@ -476,7 +482,8 @@ qemuMonitorIORead(qemuMonitorPtr mon) if (got < 0) { if (errno == EAGAIN) break; - mon->lastErrno = errno; + virReportSystemError(errno, "%s", + _("Unable to read from monitor")); ret = -1; break; } @@ -503,7 +510,7 @@ static void qemuMonitorUpdateWatch(qemuMonitorPtr mon) VIR_EVENT_HANDLE_HANGUP | VIR_EVENT_HANDLE_ERROR; - if (!mon->lastErrno) { + if (mon->lastError.code == VIR_ERR_OK) { events |= VIR_EVENT_HANDLE_READABLE; if (mon->msg && mon->msg->txOffset < mon->msg->txLength) @@ -528,61 +535,84 @@ qemuMonitorIO(int watch, int fd, int events, void *opaque) { #endif if (mon->fd != fd || mon->watch != watch) { - VIR_ERROR(_("event from unexpected fd %d!=%d / watch %d!=%d"), mon->fd, fd, mon->watch, watch); + if (events & VIR_EVENT_HANDLE_HANGUP) + eof = true; + qemuReportError(VIR_ERR_INTERNAL_ERROR, + _("event from unexpected fd %d!=%d / watch %d!=%d"), + mon->fd, fd, mon->watch, watch); + error = true; + } else if (mon->lastError.code != VIR_ERR_OK) { + if (events & VIR_EVENT_HANDLE_HANGUP) + eof = true; error = true; } else { - if (!mon->lastErrno && - events & VIR_EVENT_HANDLE_WRITABLE) { - int done = qemuMonitorIOWrite(mon); - if (done < 0) - error = 1; + if (events & VIR_EVENT_HANDLE_WRITABLE) { + if (qemuMonitorIOWrite(mon) < 0) + error = true; events &= ~VIR_EVENT_HANDLE_WRITABLE; } - if (!mon->lastErrno && + + if (!error && events & VIR_EVENT_HANDLE_READABLE) { int got = qemuMonitorIORead(mon); - if (got < 0) + events &= ~VIR_EVENT_HANDLE_READABLE; + if (got < 0) { error = true; - /* Ignore hangup/error events if we read some data, to - * give time for that data to be consumed */ - if (got > 0) { + } else if (got == 0) { + eof = true; + } else { + /* Ignore hangup/error events if we read some data, to + * give time for that data to be consumed */ events = 0; if (qemuMonitorIOProcess(mon) < 0) error = true; - } else - events &= ~VIR_EVENT_HANDLE_READABLE; + } } - /* If IO process resulted in an error & we have a message, - * then wakeup that waiter */ - if (mon->lastErrno && mon->msg && !mon->msg->finished) { - mon->msg->lastErrno = mon->lastErrno; - mon->msg->finished = 1; - virCondSignal(&mon->notify); + if (!error && + events & VIR_EVENT_HANDLE_HANGUP) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + _("End of file from monitor")); + eof = 1; + events &= ~VIR_EVENT_HANDLE_HANGUP; } - qemuMonitorUpdateWatch(mon); - - if (events & (VIR_EVENT_HANDLE_HANGUP | - VIR_EVENT_HANDLE_ERROR)) { - /* If IO process resulted in EOF & we have a message, - * then wakeup that waiter */ - if (mon->msg && !mon->msg->finished) { - mon->msg->finished = 1; - mon->msg->lastErrno = EIO; - virCondSignal(&mon->notify); - } - eof = 1; - } else if (events) { - VIR_ERROR(_("unhandled fd event %d for monitor fd %d"), - events, mon->fd); + if (!error && !eof && + events & VIR_EVENT_HANDLE_ERROR) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + _("Error while waiting for monitor")); + error = 1; + } + if (!error && events) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + _("Unhandled event %d for monitor fd %d"), + events, mon->fd); error = 1; } } - if (eof || error) - mon->lastErrno = EIO; + if (error || eof) { + if (mon->lastError.code != VIR_ERR_OK) { + /* Already have an error, so clear any new error */ + virResetLastError(); + } else { + virErrorPtr err = virGetLastError(); + if (!err) + qemuReportError(VIR_ERR_INTERNAL_ERROR, + _("Error while processing monitor IO")); + virCopyLastError(&mon->lastError); + virResetLastError(); + } + + VIR_DEBUG("Error on monitor %s", NULLSTR(mon->lastError.message)); + /* If IO process resulted in an error & we have a message, + * then wakeup that waiter */ + if (mon->msg && !mon->msg->finished) { + mon->msg->finished = 1; + virCondSignal(&mon->notify); + } + } qemuMonitorUpdateWatch(mon); @@ -738,14 +768,28 @@ void qemuMonitorClose(qemuMonitorPtr mon) } +char *qemuMonitorNextCommandID(qemuMonitorPtr mon) +{ + char *id; + + if (virAsprintf(&id, "libvirt-%d", ++mon->nextSerial) < 0) { + virReportOOMError(); + return NULL; + } + return id; +} + + int qemuMonitorSend(qemuMonitorPtr mon, qemuMonitorMessagePtr msg) { int ret = -1; /* Check whether qemu quited unexpectedly */ - if (mon->lastErrno) { - msg->lastErrno = mon->lastErrno; + if (mon->lastError.code != VIR_ERR_OK) { + VIR_DEBUG("Attempt to send command while error is set %s", + NULLSTR(mon->lastError.message)); + virSetError(&mon->lastError); return -1; } @@ -753,12 +797,21 @@ int qemuMonitorSend(qemuMonitorPtr mon, qemuMonitorUpdateWatch(mon); while (!mon->msg->finished) { - if (virCondWait(&mon->notify, &mon->lock) < 0) + if (virCondWait(&mon->notify, &mon->lock) < 0) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Unable to wait on monitor condition")); goto cleanup; + } } - if (mon->lastErrno == 0) - ret = 0; + if (mon->lastError.code != VIR_ERR_OK) { + VIR_DEBUG("Send command resulted in error %s", + NULLSTR(mon->lastError.message)); + virSetError(&mon->lastError); + goto cleanup; + } + + ret = 0; cleanup: mon->msg = NULL; diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index 0adef81..910865b 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -49,12 +49,16 @@ struct _qemuMonitorMessage { int txOffset; int txLength; + /* Used by the text monitor reply / error */ char *rxBuffer; int rxLength; + /* Used by the JSON monitor to hold reply / error */ + void *rxObject; - int finished; - - int lastErrno; + /* True if rxBuffer / rxObject are ready, or a + * fatal error occurred on the monitor channel + */ + bool finished; qemuMonitorPasswordHandler passwordHandler; void *passwordOpaque; @@ -137,6 +141,7 @@ int qemuMonitorRef(qemuMonitorPtr mon); int qemuMonitorUnref(qemuMonitorPtr mon) ATTRIBUTE_RETURN_CHECK; /* These APIs are for use by the internal Text/JSON monitor impl code only */ +char *qemuMonitorNextCommandID(qemuMonitorPtr mon); int qemuMonitorSend(qemuMonitorPtr mon, qemuMonitorMessagePtr msg); int qemuMonitorHMPCommandWithFd(qemuMonitorPtr mon, diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 2d8a390..87cd45d 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -111,43 +111,38 @@ qemuMonitorJSONIOProcessLine(qemuMonitorPtr mon, VIR_DEBUG("Line [%s]", line); - if (!(obj = virJSONValueFromString(line))) { - VIR_DEBUG("Parsing JSON string failed"); - errno = EINVAL; + if (!(obj = virJSONValueFromString(line))) goto cleanup; - } if (obj->type != VIR_JSON_TYPE_OBJECT) { - VIR_DEBUG("Parsed JSON string isn't an object"); - errno = EINVAL; + qemuReportError(VIR_ERR_INTERNAL_ERROR, + _("Parsed JSON reply '%s' isn't an object"), line); + goto cleanup; } if (virJSONValueObjectHasKey(obj, "QMP") == 1) { - VIR_DEBUG("Got QMP capabilities data"); ret = 0; - goto cleanup; - } - - if (virJSONValueObjectHasKey(obj, "event") == 1) { + virJSONValueFree(obj); + } else if (virJSONValueObjectHasKey(obj, "event") == 1) { ret = qemuMonitorJSONIOProcessEvent(mon, obj); - goto cleanup; - } - - if (msg) { - if (!(msg->rxBuffer = strdup(line))) { - errno = ENOMEM; - goto cleanup; + } else if (virJSONValueObjectHasKey(obj, "error") == 1 || + virJSONValueObjectHasKey(obj, "return") == 1) { + if (msg) { + msg->rxObject = obj; + msg->finished = 1; + ret = 0; + } else { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + _("Unexpected JSON reply '%s'"), line); } - msg->rxLength = strlen(line); - msg->finished = 1; } else { - VIR_DEBUG("Ignoring unexpected JSON message [%s]", line); + qemuReportError(VIR_ERR_INTERNAL_ERROR, + _("Unknown JSON reply '%s'"), line); } - ret = 0; - cleanup: - virJSONValueFree(obj); + if (ret < 0) + virJSONValueFree(obj); return ret; } @@ -166,7 +161,7 @@ int qemuMonitorJSONIOProcess(qemuMonitorPtr mon, int got = nl - (data + used); char *line = strndup(data + used, got); if (!line) { - errno = ENOMEM; + virReportOOMError(); return -1; } used += got + strlen(LINE_ENDING); @@ -195,11 +190,24 @@ qemuMonitorJSONCommandWithFd(qemuMonitorPtr mon, int ret = -1; qemuMonitorMessage msg; char *cmdstr = NULL; + char *id = NULL; + virJSONValuePtr exe; *reply = NULL; memset(&msg, 0, sizeof msg); + exe = virJSONValueObjectGet(cmd, "execute"); + if (exe) { + if (!(id = qemuMonitorNextCommandID(mon))) + goto cleanup; + if (virJSONValueObjectAppendString(cmd, "id", id) < 0) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Unable to append command 'id' string")); + goto cleanup; + } + } + if (!(cmdstr = virJSONValueToString(cmd))) { virReportOOMError(); goto cleanup; @@ -215,33 +223,24 @@ qemuMonitorJSONCommandWithFd(qemuMonitorPtr mon, ret = qemuMonitorSend(mon, &msg); - VIR_DEBUG("Receive command reply ret=%d errno=%d %d bytes '%s'", - ret, msg.lastErrno, msg.rxLength, msg.rxBuffer); + VIR_DEBUG("Receive command reply ret=%d rxObject=%p", + ret, msg.rxObject); - /* If we got ret==0, but not reply data something rather bad - * went wrong, so lets fake an EIO error */ - if (!msg.rxBuffer && ret == 0) { - msg.lastErrno = EIO; - ret = -1; - } - if (ret == 0) { - if (!((*reply) = virJSONValueFromString(msg.rxBuffer))) { - qemuReportError(VIR_ERR_INTERNAL_ERROR, - _("cannot parse JSON doc '%s'"), msg.rxBuffer); - goto cleanup; + if (!msg.rxObject) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Missing monitor reply object")); + ret = -1; + } else { + *reply = msg.rxObject; } } - if (ret < 0) - virReportSystemError(msg.lastErrno, - _("cannot send monitor command '%s'"), cmdstr); - cleanup: + VIR_FREE(id); VIR_FREE(cmdstr); VIR_FREE(msg.txBuffer); - VIR_FREE(msg.rxBuffer); return ret; } diff --git a/src/qemu/qemu_monitor_text.c b/src/qemu/qemu_monitor_text.c index 106f2d3..3b42e7a 100644 --- a/src/qemu/qemu_monitor_text.c +++ b/src/qemu/qemu_monitor_text.c @@ -153,7 +153,8 @@ int qemuMonitorTextIOProcess(qemuMonitorPtr mon ATTRIBUTE_UNUSED, #endif if (msg->passwordHandler) { int i; - /* Try and handle the prompt */ + /* Try and handle the prompt. The handler is required + * to report a normal libvirt error */ if (msg->passwordHandler(mon, msg, start, passwd - start + strlen(PASSWORD_PROMPT), @@ -168,7 +169,8 @@ int qemuMonitorTextIOProcess(qemuMonitorPtr mon ATTRIBUTE_UNUSED, /* Handled, so skip forward over password prompt */ start = passwd; } else { - errno = EACCES; + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Password request seen, but no handler available")); return -1; } } @@ -183,8 +185,10 @@ int qemuMonitorTextIOProcess(qemuMonitorPtr mon ATTRIBUTE_UNUSED, * BASIC_PROMPT we can reasonably reliably cope */ if (want) { if (VIR_REALLOC_N(msg->rxBuffer, - msg->rxLength + want + 1) < 0) + msg->rxLength + want + 1) < 0) { + virReportOOMError(); return -1; + } memcpy(msg->rxBuffer + msg->rxLength, start, want); msg->rxLength += want; msg->rxBuffer[msg->rxLength] = '\0'; @@ -234,30 +238,26 @@ qemuMonitorTextCommandWithHandler(qemuMonitorPtr mon, ret = qemuMonitorSend(mon, &msg); - VIR_DEBUG("Receive command reply ret=%d errno=%d %d bytes '%s'", - ret, msg.lastErrno, msg.rxLength, msg.rxBuffer); + VIR_DEBUG("Receive command reply ret=%d rxLength=%d rxBuffer='%s'", + ret, msg.rxLength, msg.rxBuffer); /* Just in case buffer had some passwords in */ memset(msg.txBuffer, 0, msg.txLength); VIR_FREE(msg.txBuffer); - /* To make life safer for callers, already ensure there's at least an empty string */ - if (msg.rxBuffer) { - *reply = msg.rxBuffer; - } else { - *reply = strdup(""); - if (!*reply) { - virReportOOMError(); - return -1; + if (ret >= 0) { + /* To make life safer for callers, already ensure there's at least an empty string */ + if (msg.rxBuffer) { + *reply = msg.rxBuffer; + } else { + *reply = strdup(""); + if (!*reply) { + virReportOOMError(); + return -1; + } } } - if (ret < 0) { - virReportSystemError(msg.lastErrno, - _("cannot send monitor command '%s'"), cmd); - VIR_FREE(*reply); - } - return ret; } @@ -297,15 +297,16 @@ qemuMonitorSendDiskPassphrase(qemuMonitorPtr mon, pathStart = strstr(data, DISK_ENCRYPTION_PREFIX); pathEnd = strstr(data, DISK_ENCRYPTION_POSTFIX); if (!pathStart || !pathEnd || pathStart >= pathEnd) { - errno = -EINVAL; + qemuReportError(VIR_ERR_INTERNAL_ERROR, + _("Unable to extract disk path from %s"), + data); return -1; } /* Extra the path */ pathStart += strlen(DISK_ENCRYPTION_PREFIX); - path = strndup(pathStart, pathEnd - pathStart); - if (!path) { - errno = ENOMEM; + if (!(path = strndup(pathStart, pathEnd - pathStart))) { + virReportOOMError(); return -1; } @@ -325,7 +326,7 @@ qemuMonitorSendDiskPassphrase(qemuMonitorPtr mon, msg->txLength + passphrase_len + 1 + 1) < 0) { memset(passphrase, 0, passphrase_len); VIR_FREE(passphrase); - errno = ENOMEM; + virReportOOMError(); return -1; } @@ -763,7 +764,7 @@ qemuMonitorSendVNCPassphrase(qemuMonitorPtr mon ATTRIBUTE_UNUSED, * to be sent back */ if (VIR_REALLOC_N(msg->txBuffer, msg->txLength + passphrase_len + 1 + 1) < 0) { - errno = ENOMEM; + virReportOOMError(); return -1; } -- 1.7.4.4