Errors in qemuAgentIOProcessLine stop agent IO processing just like any regular IO error, however some of current errors that this functions spawns are false positives. Consider next case for example: 1. send sync (unsynced state) 2. receive sync reply (sync established) 3. command send, but timeout occured (unsynced state) 4. receive command reply Last IO triggers error because current code ignores only delayed syncs when unsynced We should not treat any delayed reply as error in unsynced state. Until client and qga are not in sync delayed reply to any command is possible. msg == NULL is the exact criterion that we are not in sync. --- src/qemu/qemu_agent.c | 21 +++------------------ 1 file changed, 3 insertions(+), 18 deletions(-) diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c index fdc4fed..88bcb4c 100644 --- a/src/qemu/qemu_agent.c +++ b/src/qemu/qemu_agent.c @@ -308,7 +308,6 @@ qemuAgentIOProcessLine(qemuAgentPtr mon, { virJSONValuePtr obj = NULL; int ret = -1; - unsigned long long id; VIR_DEBUG("Line [%s]", line); @@ -331,25 +330,11 @@ qemuAgentIOProcessLine(qemuAgentPtr mon, msg->rxObject = obj; msg->finished = 1; obj = NULL; - ret = 0; } else { - /* If we've received something like: - * {"return": 1234} - * it is likely that somebody started GA - * which is now processing our previous - * guest-sync commands. Check if this is - * the case and don't report an error but - * return silently. - */ - if (virJSONValueObjectGetNumberUlong(obj, "return", &id) == 0) { - VIR_DEBUG("Ignoring delayed reply to guest-sync: %llu", id); - ret = 0; - goto cleanup; - } - - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Unexpected JSON reply '%s'"), line); + /* we are out of sync */ + VIR_DEBUG("Ignoring delayed reply"); } + ret = 0; } else { virReportError(VIR_ERR_INTERNAL_ERROR, _("Unknown JSON reply '%s'"), line); -- 1.8.3.1

When we wait for sync reply we can receive delayed reply to syncs or commands that were sent erlier. We can safely skip them until we receive sync reply with correct id. There is no much sense report this situation to client. Actually with a bit of "luck" if we involve client into this the play can go on forever: send sync 0, receive sync reply -1, send sync 1, receive reply 0 ... --- src/qemu/qemu_agent.c | 37 ++++++++++++++++++++++--------------- 1 file changed, 22 insertions(+), 15 deletions(-) diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c index 8e447d6..30a1c77 100644 --- a/src/qemu/qemu_agent.c +++ b/src/qemu/qemu_agent.c @@ -83,6 +83,8 @@ struct _qemuAgentMessage { bool finished; /* true for sync command */ bool sync; + /* id of the issued sync comand */ + unsigned long long id; }; @@ -337,6 +339,24 @@ qemuAgentIOProcessLine(qemuAgentPtr mon, } else if (virJSONValueObjectHasKey(obj, "error") == 1 || virJSONValueObjectHasKey(obj, "return") == 1) { if (msg) { + if (msg->sync) { + unsigned long long id; + + if (virJSONValueObjectGetNumberUlong(obj, "return", &id) < 0) { + VIR_DEBUG("Ignoring delayed reply on sync"); + ret = 0; + goto cleanup; + } + + VIR_DEBUG("Guest returned ID: %llu", id); + + if (msg->id != id) { + VIR_DEBUG("Guest agent returned ID: %llu instead of %llu", + id, msg->id); + ret = 0; + goto cleanup; + } + } msg->rxObject = obj; msg->finished = 1; obj = NULL; @@ -935,7 +955,7 @@ qemuAgentGuestSync(qemuAgentPtr mon) { int ret = -1; int send_ret; - unsigned long long id, id_ret; + unsigned long long id; qemuAgentMessage sync_msg; memset(&sync_msg, 0, sizeof(sync_msg)); @@ -950,6 +970,7 @@ qemuAgentGuestSync(qemuAgentPtr mon) sync_msg.txLength = strlen(sync_msg.txBuffer); sync_msg.sync = true; + sync_msg.id = id; VIR_DEBUG("Sending guest-sync command with ID: %llu", id); @@ -967,20 +988,6 @@ qemuAgentGuestSync(qemuAgentPtr mon) goto cleanup; } - if (virJSONValueObjectGetNumberUlong(sync_msg.rxObject, - "return", &id_ret) < 0) { - virReportError(VIR_ERR_AGENT_UNSYNCED, "%s", - _("Malformed return value")); - goto cleanup; - } - - VIR_DEBUG("Guest returned ID: %llu", id_ret); - if (id_ret != id) { - virReportError(VIR_ERR_AGENT_UNSYNCED, - _("Guest agent returned ID: %llu instead of %llu"), - id_ret, id); - goto cleanup; - } ret = 0; cleanup: -- 1.8.3.1

We can receive NULL as sync reply in two situations. First is garbage sync reply and this situation is handled by resending sync message. Second is different cases of rebooting guest, destroing domain etc and we can give more meaningful error message. Actually we have this error message in qemuAgentCommand already which checks for the same sitatuion. AFAIK case with mon->running is just to be safe on adding some future(?) cases of returning NULL reply. --- src/qemu/qemu_agent.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c index 8f47618..8aea3c4 100644 --- a/src/qemu/qemu_agent.c +++ b/src/qemu/qemu_agent.c @@ -992,8 +992,12 @@ qemuAgentGuestSync(qemuAgentPtr mon) memset(&sync_msg, 0, sizeof(sync_msg)); goto retry; } else { - virReportError(VIR_ERR_AGENT_UNSYNCED, "%s", - _("Missing monitor reply object")); + if (mon->running) + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Missing monitor reply object")); + else + virReportError(VIR_ERR_AGENT_UNRESPONSIVE, "%s", + _("Guest agent disappeared while executing command")); goto cleanup; } } -- 1.8.3.1