diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index 28e525294e..09a1b8e38a 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); - } - - 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); } + } + + if (!error && + events & VIR_EVENT_HANDLE_HANGUP) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + _("End of file from monitor")); eof = 1; - } else if (events) { - VIR_ERROR(_("unhandled fd event %d for monitor fd %d"), - events, mon->fd); + events &= ~VIR_EVENT_HANDLE_HANGUP; + } + + 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 0adef813a0..910865b7d8 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 bdd0dcb3ba..75adf661cc 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 106f2d3770..3b42e7ab43 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; }