Re: [PATCH v3 5/5] qemu: Connect to guest agent iff needed

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, Jan 11, 2016 at 11:31:12AM +0100, Michal Privoznik wrote:
https://bugzilla.redhat.com/show_bug.cgi?id=1293351

Since 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 <mprivozn@xxxxxxxxxx>
---
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

Attachment: signature.asc
Description: PGP signature

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]