On Mon, Jan 11, 2016 at 11:31:12AM +0100, Michal Privoznik wrote:
https://bugzilla.redhat.com/show_bug.cgi?id=1293351Since we already have virtio channel events, we know when guest agent within guest has (dis-)connected. Instead of us blindly connecting to a socket that no one is listening to, we can just follow what qemu-ga does. This has a nice benefit that we don't need to 'guest-ping' the agent just to timeout and find out nobody is listening. The way that this commit is implemented: - don't connect in qemuProcessStart directly, defer that to event callback (which already follows the agent). - after migration is settled, before we resume vCPUs, ask qemu whether somebody is listening on the socket and if so, connect to it. Signed-off-by: Michal Privoznik <mpriv...@redhat.com> --- src/qemu/qemu_driver.c | 21 ++++++++++++++++++++- src/qemu/qemu_migration.c | 20 ++++++++++++++++++++ src/qemu/qemu_process.c | 9 +++++++++ 3 files changed, 49 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 8ccf68b..43ef18a 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -6650,12 +6650,13 @@ qemuDomainSaveImageStartVM(virConnectPtr conn, bool start_paused, qemuDomainAsyncJob asyncJob) { - int ret = -1; + int rc, ret = -1; virObjectEventPtr event; int intermediatefd = -1; virCommandPtr cmd = NULL; char *errbuf = NULL; virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); + qemuDomainObjPrivatePtr priv = vm->privateData; if ((header->version == 2) && (header->compressed != QEMU_SAVE_FORMAT_RAW)) { @@ -6710,6 +6711,24 @@ qemuDomainSaveImageStartVM(virConnectPtr conn, goto cleanup; } + if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_VSERPORT_CHANGE)) { + /* If QEMU is capable of vserport_change events, we ought to + * refresh channel states here and possibly connect to the guest + * agent too. */ + if (qemuProcessRefreshChannelState(driver, vm) < 0) + goto cleanup; + + if ((rc = qemuConnectAgent(driver, vm)) < 0) { + if (rc == -2) + goto cleanup; + + VIR_WARN("Cannot connect to QEMU guest agent for %s", + vm->def->name); + virResetLastError(); + priv->agentError = true; + } + } +
This pattern is repeating. If we moved the channel state refresh into qemuConnectAgent() (it could even be called only once per domain lifetime, all other times we would depend on the state being correct), mainly if there is a check for the state anyway, that would help in more cases than just this one. And more generally, we could always skip connecting to the agent if qemu supports VSERPORT_CHANGE, so that condition could be moved inside qemuConnectAgent() as well (or creating a wrapper around it). That way your problem would be also solved for domains we connected to after libvirt's restart and all other cases that none of us thought about. If there is a reason against that, which I missed, then at least putting this into it's own function, and considering calling it from other parts of the code where qemuConnectAgent() is being called currently, would be nice. ACK to 1-4, by the way. Martin
signature.asc
Description: PGP signature
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
