[libvirt] [PATCH] qemu_agent: report VIR_ERR_AGENT_UNSYNCED if guest_sync failed.

From: Chen Hanxiao <chenhanxiao@gmail.com> Commit 5e5019bf removed this kind of error. But we've already protected race condition by qemuDomainObjEnterAgent, it was very unlikely to revieve a sync from other calls. For call from virDomain*, it's better to show this error if we really got a mismatch sync. Signed-off-by: Chen Hanxiao <chenhanxiao@gmail.com> --- src/qemu/qemu_agent.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c index 5d125c413..cdcacf5c8 100644 --- a/src/qemu/qemu_agent.c +++ b/src/qemu/qemu_agent.c @@ -354,7 +354,9 @@ qemuAgentIOProcessLine(qemuAgentPtr mon, if (msg->id != id) { VIR_DEBUG("Guest agent returned ID: %llu instead of %llu", id, msg->id); - ret = 0; + virReportError(VIR_ERR_AGENT_UNSYNCED, + _("Guest agent returned ID: %llu " + "instead of %llu"), id, msg->id); goto cleanup; } } -- 2.14.3

On 30.11.2017 06:08, Chen Hanxiao wrote:
From: Chen Hanxiao <chenhanxiao@gmail.com>
Commit 5e5019bf removed this kind of error.
But we've already protected race condition by qemuDomainObjEnterAgent, it was very unlikely to revieve a sync from other calls. For call from virDomain*, it's better to show this error if we really got a mismatch sync.
Hi, Chen. We've observed such condition on practice. So if you report error in this place then agent monitor will enter error state and will become unusable and there are no means to leave this state other then restarting libvirtd AFAIK. Even if there is a option to reset monitor then client will reset it by itself on receiving VIR_ERR_AGENT_UNSYNCED error. But then why enter error state in this case? Nikolay
Signed-off-by: Chen Hanxiao <chenhanxiao@gmail.com> --- src/qemu/qemu_agent.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c index 5d125c413..cdcacf5c8 100644 --- a/src/qemu/qemu_agent.c +++ b/src/qemu/qemu_agent.c @@ -354,7 +354,9 @@ qemuAgentIOProcessLine(qemuAgentPtr mon, if (msg->id != id) { VIR_DEBUG("Guest agent returned ID: %llu instead of %llu", id, msg->id); - ret = 0; + virReportError(VIR_ERR_AGENT_UNSYNCED, + _("Guest agent returned ID: %llu " + "instead of %llu"), id, msg->id); goto cleanup; } }

At 2017-11-30 14:11:36, "Nikolay Shirokovskiy" <nshirokovskiy@virtuozzo.com> wrote:
On 30.11.2017 06:08, Chen Hanxiao wrote:
From: Chen Hanxiao <chenhanxiao@gmail.com>
Commit 5e5019bf removed this kind of error.
But we've already protected race condition by qemuDomainObjEnterAgent, it was very unlikely to revieve a sync from other calls. For call from virDomain*, it's better to show this error if we really got a mismatch sync.
Hi, Chen.
We've observed such condition on practice. So if you report error in this place then agent monitor will enter error state and will become unusable and there are no means to leave this state other then restarting libvirtd AFAIK. Even if there is a option to reset monitor then client will reset it by itself on receiving VIR_ERR_AGENT_UNSYNCED error. But then why enter error state in this case?
Hi, Nikolay My concern is that the mismatch of sync id informs qga is malfunctioning. If in that state, we SHOULD set agent monitor to error state, we'll get VIR_ERR_AGENT_UNRESPONSIVE. The sysadmin should notice this, and login to VMs to do something. Restarting qga inside guest will reset the monitor's agentError, maybe malfunction of qga will be reset at the same time. Regards, - Chen

On 30.11.2017 13:37, Chen Hanxiao wrote:
At 2017-11-30 14:11:36, "Nikolay Shirokovskiy" <nshirokovskiy@virtuozzo.com> wrote:
On 30.11.2017 06:08, Chen Hanxiao wrote:
From: Chen Hanxiao <chenhanxiao@gmail.com>
Commit 5e5019bf removed this kind of error.
But we've already protected race condition by qemuDomainObjEnterAgent, it was very unlikely to revieve a sync from other calls. For call from virDomain*, it's better to show this error if we really got a mismatch sync.
Hi, Chen.
We've observed such condition on practice. So if you report error in this place then agent monitor will enter error state and will become unusable and there are no means to leave this state other then restarting libvirtd AFAIK. Even if there is a option to reset monitor then client will reset it by itself on receiving VIR_ERR_AGENT_UNSYNCED error. But then why enter error state in this case?
Hi, Nikolay
My concern is that the mismatch of sync id informs qga is malfunctioning.
Not neccessarily. This can happen due to the way guest channel works.
If in that state, we SHOULD set agent monitor to error state, we'll get VIR_ERR_AGENT_UNRESPONSIVE> The sysadmin should notice this, and login to VMs to do something.
Restarting qga inside guest will reset the monitor's agentError, maybe malfunction of qga will be reset at the same time.
Regards, - Chen
participants (2)
-
Chen Hanxiao
-
Nikolay Shirokovskiy