On 04/18/2018 01:30 PM, Daniel P. Berrangé wrote:
There is a race condition when spawning QEMU where libvirt has
spawned
QEMU but the monitor socket is not yet open. Libvirt has to repeatedly
try to connect() to QEMU's monitor until eventually it succeeds, or
times out. We use kill() to check if QEMU is still alive so we avoid
waiting a long time if QEMU exited, but having a timeout at all is still
unpleasant.
With QEMU 2.12 we can pass in a pre-opened FD for UNIX domain or TCP
sockets. If libvirt has called bind() and listen() on this FD, then we
have a guarantee that libvirt can immediately call connect() and
succeed without any race.
Although we only really care about this for the monitor socket and agent
socket, this patch does FD passing for all UNIX socket based character
devices since there appears to be no downside to it.
We don't do FD passing for TCP sockets, however, because it is only
possible to pass a single FD, while some hostnames may require listening
on multiple FDs to cover IPv4 and IPv6 concurrently.
Reviewed-by: John Ferlan <jferlan(a)redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com>
---
src/qemu/qemu_command.c | 54 +++++++++++++++++++++++++++++++++++++++++++++++--
1 file changed, 52 insertions(+), 2 deletions(-)
This one is now affected by Peter's recent xml2argv adjustment for:
tests/qemuxml2argvdata/disk-drive-write-cache.x86_64-latest.args
which now fails w/ :
253) QEMU XML-2-ARGV disk-drive-write-cache.x86_64-latest
... libvirt: QEMU Driver error : Unable to bind to UNIX socket path
'/tmp/lib/domain--1-QEMUGuest1/monitor.sock': No such file or directory
FAILED
because the qemuProcessCreatePretendCmd doesn't allow this code to
distinguish that it shouldn't attempt a bind/listen <sigh>
I "think" this may require moving the socket setup code into
qemuProcessLaunch or somehow passing a flag into BuildCommandLine to
tell it not to do anything "real".
John
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 0c109c63e7..9fc48eb829 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -5034,8 +5034,58 @@ qemuBuildChrChardevStr(virLogManagerPtr logManager,
break;
case VIR_DOMAIN_CHR_TYPE_UNIX:
- virBufferAsprintf(&buf, "socket,id=%s,path=", charAlias);
- virQEMUBuildBufferEscapeComma(&buf, dev->data.nix.path);
+ if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_CHARDEV_FD_PASS)) {
+ struct sockaddr_un addr;
+ socklen_t addrlen = sizeof(addr);
+ int fd;
+
+ if ((fd = socket(AF_UNIX, SOCK_STREAM, 0)) < 0) {
+ virReportSystemError(errno, "%s",
+ _("Unable to create UNIX socket"));
+ goto cleanup;
+ }
+
+ memset(&addr, 0, sizeof(addr));
+ addr.sun_family = AF_UNIX;
+ if (virStrcpyStatic(addr.sun_path, dev->data.nix.path) == NULL) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("UNIX socket path '%s' too long"),
+ dev->data.nix.path);
+ VIR_FORCE_CLOSE(fd);
+ goto cleanup;
+ }
+
+ if (unlink(dev->data.nix.path) < 0 && errno != ENOENT) {
+ virReportSystemError(errno,
+ _("Unable to unlink %s"),
+ dev->data.nix.path);
+ VIR_FORCE_CLOSE(fd);
+ goto cleanup;
+ }
+
+ if (bind(fd, (struct sockaddr *)&addr, addrlen) < 0) {
+ virReportSystemError(errno,
+ _("Unable to bind to UNIX socket path
'%s'"),
+ dev->data.nix.path);
+ VIR_FORCE_CLOSE(fd);
+ goto cleanup;
+ }
+
+ if (listen(fd, 1) < 0) {
+ virReportSystemError(errno,
+ _("Unable to listen to UNIX socket path
'%s'"),
+ dev->data.nix.path);
+ VIR_FORCE_CLOSE(fd);
+ goto cleanup;
+ }
+
+ virBufferAsprintf(&buf, "socket,id=%s,fd=%d", charAlias, fd);
+
+ virCommandPassFD(cmd, fd, VIR_COMMAND_PASS_FD_CLOSE_PARENT);
+ } else {
+ virBufferAsprintf(&buf, "socket,id=%s,path=", charAlias);
+ virQEMUBuildBufferEscapeComma(&buf, dev->data.nix.path);
+ }
if (dev->data.nix.listen)
virBufferAdd(&buf, nowait ? ",server,nowait" :
",server", -1);