All of lore.kernel.org
 help / color / mirror / Atom feed
From: Max Reitz <mreitz@redhat.com>
To: qemu-block@nongnu.org
Cc: qemu-devel@nongnu.org, Max Reitz <mreitz@redhat.com>,
	Eduardo Habkost <ehabkost@redhat.com>,
	Cleber Rosa <crosa@redhat.com>, Kevin Wolf <kwolf@redhat.com>
Subject: [Qemu-devel] [PATCH v3 6/9] iotests: Explicitly bequeath FDs in Python
Date: Mon, 22 Oct 2018 14:53:04 +0100	[thread overview]
Message-ID: <20181022135307.14398-7-mreitz@redhat.com> (raw)
In-Reply-To: <20181022135307.14398-1-mreitz@redhat.com>

Python 3.4 introduced the inheritable attribute for FDs.  At the same
time, it changed the default so that all FDs are not inheritable by
default, that only inheritable FDs are inherited to subprocesses, and
only if close_fds is explicitly set to False.

Adhere to this by setting close_fds to False when working with
subprocesses that may want to inherit FDs, and by trying to
set_inheritable() on FDs that we do want to bequeath to them.

Signed-off-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>
Reviewed-by: Cleber Rosa <crosa@redhat.com>
---
 scripts/qemu.py        | 34 +++++++++++++++++++++++++++++-----
 tests/qemu-iotests/045 |  2 +-
 tests/qemu-iotests/147 |  2 +-
 3 files changed, 31 insertions(+), 7 deletions(-)

diff --git a/scripts/qemu.py b/scripts/qemu.py
index f099ce7278..fb29b73c30 100644
--- a/scripts/qemu.py
+++ b/scripts/qemu.py
@@ -142,11 +142,19 @@ class QEMUMachine(object):
         if opts:
             options.append(opts)
 
+        # This did not exist before 3.4, but since then it is
+        # mandatory for our purpose
+        if hasattr(os, 'set_inheritable'):
+            os.set_inheritable(fd, True)
+
         self._args.append('-add-fd')
         self._args.append(','.join(options))
         return self
 
-    def send_fd_scm(self, fd_file_path):
+    # Exactly one of fd and file_path must be given.
+    # (If it is file_path, the helper will open that file and pass its
+    # own fd)
+    def send_fd_scm(self, fd=None, file_path=None):
         # In iotest.py, the qmp should always use unix socket.
         assert self._qmp.is_scm_available()
         if self._socket_scm_helper is None:
@@ -154,12 +162,27 @@ class QEMUMachine(object):
         if not os.path.exists(self._socket_scm_helper):
             raise QEMUMachineError("%s does not exist" %
                                    self._socket_scm_helper)
+
+        # This did not exist before 3.4, but since then it is
+        # mandatory for our purpose
+        if hasattr(os, 'set_inheritable'):
+            os.set_inheritable(self._qmp.get_sock_fd(), True)
+            if fd is not None:
+                os.set_inheritable(fd, True)
+
         fd_param = ["%s" % self._socket_scm_helper,
-                    "%d" % self._qmp.get_sock_fd(),
-                    "%s" % fd_file_path]
+                    "%d" % self._qmp.get_sock_fd()]
+
+        if file_path is not None:
+            assert fd is None
+            fd_param.append(file_path)
+        else:
+            assert fd is not None
+            fd_param.append(str(fd))
+
         devnull = open(os.path.devnull, 'rb')
         proc = subprocess.Popen(fd_param, stdin=devnull, stdout=subprocess.PIPE,
-                                stderr=subprocess.STDOUT)
+                                stderr=subprocess.STDOUT, close_fds=False)
         output = proc.communicate()[0]
         if output:
             LOG.debug(output)
@@ -280,7 +303,8 @@ class QEMUMachine(object):
                                        stdin=devnull,
                                        stdout=self._qemu_log_file,
                                        stderr=subprocess.STDOUT,
-                                       shell=False)
+                                       shell=False,
+                                       close_fds=False)
         self._post_launch()
 
     def wait(self):
diff --git a/tests/qemu-iotests/045 b/tests/qemu-iotests/045
index 6be8fc4912..55a5d31ca8 100755
--- a/tests/qemu-iotests/045
+++ b/tests/qemu-iotests/045
@@ -140,7 +140,7 @@ class TestSCMFd(iotests.QMPTestCase):
         os.remove(image0)
 
     def _send_fd_by_SCM(self):
-        ret = self.vm.send_fd_scm(image0)
+        ret = self.vm.send_fd_scm(file_path=image0)
         self.assertEqual(ret, 0, 'Failed to send fd with UNIX SCM')
 
     def test_add_fd(self):
diff --git a/tests/qemu-iotests/147 b/tests/qemu-iotests/147
index d2081df84b..05b374b7d3 100755
--- a/tests/qemu-iotests/147
+++ b/tests/qemu-iotests/147
@@ -229,7 +229,7 @@ class BuiltinNBD(NBDBlockdevAddBase):
         sockfd = socket.socket(socket.AF_UNIX, socket.SOCK_STREAM)
         sockfd.connect(unix_socket)
 
-        result = self.vm.send_fd_scm(str(sockfd.fileno()))
+        result = self.vm.send_fd_scm(fd=sockfd.fileno())
         self.assertEqual(result, 0, 'Failed to send socket FD')
 
         result = self.vm.qmp('getfd', fdname='nbd-fifo')
-- 
2.17.1

  parent reply	other threads:[~2018-10-22 13:53 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-10-22 13:52 [Qemu-devel] [PATCH v3 0/9] iotests: Make them work for both Python 2 and 3 Max Reitz
2018-10-22 13:52 ` [Qemu-devel] [PATCH v3 1/9] iotests: Make nbd-fault-injector flush Max Reitz
2018-10-22 13:53 ` [Qemu-devel] [PATCH v3 2/9] iotests: Flush in iotests.py's QemuIoInteractive Max Reitz
2018-10-22 13:53 ` [Qemu-devel] [PATCH v3 3/9] iotests: Use Python byte strings where appropriate Max Reitz
2018-10-22 13:53 ` [Qemu-devel] [PATCH v3 4/9] iotests: Use // for Python integer division Max Reitz
2018-10-22 13:53 ` [Qemu-devel] [PATCH v3 5/9] iotests: Different iterator behavior in Python 3 Max Reitz
2018-10-22 13:53 ` Max Reitz [this message]
2018-10-22 13:53 ` [Qemu-devel] [PATCH v3 7/9] iotests: 'new' module replacement in 169 Max Reitz
2018-10-22 13:53 ` [Qemu-devel] [PATCH v3 8/9] iotests: Modify imports for Python 3 Max Reitz
2018-10-22 13:53 ` [Qemu-devel] [PATCH v3 9/9] iotests: Unify log outputs between Python 2 and 3 Max Reitz
2018-10-26  8:54 ` [Qemu-devel] [PATCH v3 0/9] iotests: Make them work for both " Eduardo Habkost

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20181022135307.14398-7-mreitz@redhat.com \
    --to=mreitz@redhat.com \
    --cc=crosa@redhat.com \
    --cc=ehabkost@redhat.com \
    --cc=kwolf@redhat.com \
    --cc=qemu-block@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.