qemu: remove pointless connect retry logic in agent

When the agent code was first introduced back in

  commit c160ce3316
  Author: Daniel P. Berrange <berrange@redhat.com>
  Date:   Wed Oct 5 18:31:54 2011 +0100

    QEMU guest agent support

there was code that would loop and retry the connection when opening
the agent socket. At this time, the only thing done in between the
opening of the monitor socket & opening of the agent socket was a
call to set the monitor capabilities. This was a no-op on non-QMP
versions, so in theory there could be a race which let us connect
to the monitor while the agent socket was still not created by QEMU.

In the modern world, however, we long ago mandated the use of QMP
for managing QEMU, so we're guaranteed to have a set capabilities
QMP call. Once we've seen a reply to this, we're guaranteed that
QEMU has fully initialized all backends and is in its event loop.

We can thus be sure the QEMU agent socket is present and don't need
to retry connections to it, even without having the chardev FD passing
feature.

Reviewed-by: John Ferlan <jferlan@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
This commit is contained in:
Daniel P. Berrangé 2018-04-16 15:44:16 +01:00
parent 7ef0471bf7
commit fc06debd0c

View File

@ -106,7 +106,6 @@ struct _qemuAgent {
int fd; int fd;
int watch; int watch;
bool connectPending;
bool running; bool running;
virDomainObjPtr vm; virDomainObjPtr vm;
@ -180,15 +179,12 @@ static void qemuAgentDispose(void *obj)
} }
static int static int
qemuAgentOpenUnix(const char *monitor, pid_t cpid, bool *inProgress) qemuAgentOpenUnix(const char *monitor)
{ {
struct sockaddr_un addr; struct sockaddr_un addr;
int monfd; int monfd;
virTimeBackOffVar timeout;
int ret = -1; int ret = -1;
*inProgress = false;
if ((monfd = socket(AF_UNIX, SOCK_STREAM, 0)) < 0) { if ((monfd = socket(AF_UNIX, SOCK_STREAM, 0)) < 0) {
virReportSystemError(errno, virReportSystemError(errno,
"%s", _("failed to create socket")); "%s", _("failed to create socket"));
@ -217,39 +213,11 @@ qemuAgentOpenUnix(const char *monitor, pid_t cpid, bool *inProgress)
goto error; goto error;
} }
if (virTimeBackOffStart(&timeout, 1, 3*1000 /* ms */) < 0)
goto error;
while (virTimeBackOffWait(&timeout)) {
ret = connect(monfd, (struct sockaddr *)&addr, sizeof(addr)); ret = connect(monfd, (struct sockaddr *)&addr, sizeof(addr));
if (ret < 0) {
if (ret == 0)
break;
if ((errno == ENOENT || errno == ECONNREFUSED) &&
virProcessKill(cpid, 0) == 0) {
/* ENOENT : Socket may not have shown up yet
* ECONNREFUSED : Leftover socket hasn't been removed yet */
continue;
}
if ((errno == EINPROGRESS) ||
(errno == EAGAIN)) {
VIR_DEBUG("Connection attempt continuing in background");
*inProgress = true;
ret = 0;
break;
}
virReportSystemError(errno, "%s", virReportSystemError(errno, "%s",
_("failed to connect to monitor socket")); _("failed to connect to monitor socket"));
goto error; goto error;
}
if (ret != 0) {
virReportSystemError(errno, "%s",
_("monitor socket did not show up"));
goto error;
} }
return monfd; return monfd;
@ -470,35 +438,6 @@ qemuAgentIOProcess(qemuAgentPtr mon)
} }
static int
qemuAgentIOConnect(qemuAgentPtr mon)
{
int optval;
socklen_t optlen;
VIR_DEBUG("Checking on background connection status");
mon->connectPending = false;
optlen = sizeof(optval);
if (getsockopt(mon->fd, SOL_SOCKET, SO_ERROR,
&optval, &optlen) < 0) {
virReportSystemError(errno, "%s",
_("Cannot check socket connection status"));
return -1;
}
if (optval != 0) {
virReportSystemError(optval, "%s",
_("Cannot connect to agent socket"));
return -1;
}
VIR_DEBUG("Agent is now connected");
return 0;
}
/* /*
* Called when the monitor is able to write data * Called when the monitor is able to write data
* Call this function while holding the monitor lock. * Call this function while holding the monitor lock.
@ -630,13 +569,8 @@ qemuAgentIO(int watch, int fd, int events, void *opaque)
error = true; error = true;
} else { } else {
if (events & VIR_EVENT_HANDLE_WRITABLE) { if (events & VIR_EVENT_HANDLE_WRITABLE) {
if (mon->connectPending) {
if (qemuAgentIOConnect(mon) < 0)
error = true;
} else {
if (qemuAgentIOWrite(mon) < 0) if (qemuAgentIOWrite(mon) < 0)
error = true; error = true;
}
events &= ~VIR_EVENT_HANDLE_WRITABLE; events &= ~VIR_EVENT_HANDLE_WRITABLE;
} }
@ -768,8 +702,7 @@ qemuAgentOpen(virDomainObjPtr vm,
switch (config->type) { switch (config->type) {
case VIR_DOMAIN_CHR_TYPE_UNIX: case VIR_DOMAIN_CHR_TYPE_UNIX:
mon->fd = qemuAgentOpenUnix(config->data.nix.path, vm->pid, mon->fd = qemuAgentOpenUnix(config->data.nix.path);
&mon->connectPending);
break; break;
case VIR_DOMAIN_CHR_TYPE_PTY: case VIR_DOMAIN_CHR_TYPE_PTY:
@ -790,10 +723,7 @@ qemuAgentOpen(virDomainObjPtr vm,
if ((mon->watch = virEventAddHandle(mon->fd, if ((mon->watch = virEventAddHandle(mon->fd,
VIR_EVENT_HANDLE_HANGUP | VIR_EVENT_HANDLE_HANGUP |
VIR_EVENT_HANDLE_ERROR | VIR_EVENT_HANDLE_ERROR |
VIR_EVENT_HANDLE_READABLE | VIR_EVENT_HANDLE_READABLE,
(mon->connectPending ?
VIR_EVENT_HANDLE_WRITABLE :
0),
qemuAgentIO, qemuAgentIO,
mon, mon,
virObjectFreeCallback)) < 0) { virObjectFreeCallback)) < 0) {