qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/3] python: remove socket_scm_helper
@ 2021-09-28 13:53 Paolo Bonzini
  2021-09-28 13:53 ` [PATCH 1/4] python: stop using socket_scm_helper Paolo Bonzini
                   ` (4 more replies)
  0 siblings, 5 replies; 6+ messages in thread
From: Paolo Bonzini @ 2021-09-28 13:53 UTC (permalink / raw)
  To: qemu-devel; +Cc: jsnow

I suspect no one has ever looked at socket_scm_helper.c, because when I
did my reaction was just "why".  The functionality of this 136-line program
can be reproduced in Python with fewer lines of code than it takes to
invoke it.  Do it, and let it rest in peace.

The only reason not to do that could be because of the upcoming switch
to aqmp.  For that, the new send_fd method has to be wrapped to use
transport.get_extra_info('socket') and loop.run_in_executor.  Let me
know if you prefer to hold on this until later.

Paolo

Paolo Bonzini (4):
  python: stop using socket_scm_helper
  socket_scm_helper: remove
  python: raise OSError from send_fd_scm
  python: split the two sides of send_fd_scm

 python/qemu/machine/machine.py         |  58 +++--------
 python/qemu/machine/qtest.py           |   2 -
 python/qemu/qmp/__init__.py            |  15 +++
 tests/Makefile.include                 |   5 +-
 tests/meson.build                      |   4 -
 tests/qemu-iotests/045                 |   3 +-
 tests/qemu-iotests/147                 |   3 +-
 tests/qemu-iotests/iotests.py          |   3 -
 tests/qemu-iotests/meson.build         |   5 -
 tests/qemu-iotests/socket_scm_helper.c | 136 -------------------------
 tests/qemu-iotests/testenv.py          |   8 +-
 11 files changed, 35 insertions(+), 207 deletions(-)
 delete mode 100644 tests/qemu-iotests/meson.build
 delete mode 100644 tests/qemu-iotests/socket_scm_helper.c

-- 
2.31.1



^ permalink raw reply	[flat|nested] 6+ messages in thread
* [PATCH 1/4] python: stop using socket_scm_helper
  2021-09-28 13:53 [PATCH 0/3] python: remove socket_scm_helper Paolo Bonzini
@ 2021-09-28 13:53 ` Paolo Bonzini
  2021-09-28 13:53 ` [PATCH 2/4] socket_scm_helper: remove Paolo Bonzini
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 6+ messages in thread
From: Paolo Bonzini @ 2021-09-28 13:53 UTC (permalink / raw)
  To: qemu-devel; +Cc: jsnow

Python is able to call sendmsg, it does not need a helper.  Write the
code directly; for now, keep the weird calling convention.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 python/qemu/machine/machine.py | 48 ++++++++++++----------------------
 python/qemu/qmp/__init__.py    | 15 +++++++++++
 2 files changed, 32 insertions(+), 31 deletions(-)

diff --git a/python/qemu/machine/machine.py b/python/qemu/machine/machine.py
index 34131884a5..e4356ea99e 100644
--- a/python/qemu/machine/machine.py
+++ b/python/qemu/machine/machine.py
@@ -213,48 +213,34 @@ def add_fd(self: _T, fd: int, fdset: int,
     def send_fd_scm(self, fd: Optional[int] = None,
                     file_path: Optional[str] = None) -> int:
         """
-        Send an fd or file_path to socket_scm_helper.
+        Send an fd or file_path via QMP.
 
         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.
+        If it is file_path, the function will open that file and pass
+        its own fd.
         """
         # In iotest.py, the qmp should always use unix socket.
         assert self._qmp.is_scm_available()
-        if self._socket_scm_helper is None:
-            raise QEMUMachineError("No path to socket_scm_helper set")
-        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()]
 
         if file_path is not None:
             assert fd is None
-            fd_param.append(file_path)
+            fd = -1
+            try:
+                fd = os.open(file_path, os.O_RDONLY)
+                self._qmp.send_fd(fd)
+            except OSError:
+                return 1
+            finally:
+                if fd != -1:
+                    os.close(fd)
         else:
             assert fd is not None
-            fd_param.append(str(fd))
+            try:
+                self._qmp.send_fd(fd)
+            except OSError:
+                return 1
 
-        proc = subprocess.run(
-            fd_param,
-            stdin=subprocess.DEVNULL,
-            stdout=subprocess.PIPE,
-            stderr=subprocess.STDOUT,
-            check=False,
-            close_fds=False,
-        )
-        if proc.stdout:
-            LOG.debug(proc.stdout)
-
-        return proc.returncode
+        return 0
 
     @staticmethod
     def _remove_if_exists(path: str) -> None:
diff --git a/python/qemu/qmp/__init__.py b/python/qemu/qmp/__init__.py
index 269516a79b..27a3e8f7af 100644
--- a/python/qemu/qmp/__init__.py
+++ b/python/qemu/qmp/__init__.py
@@ -17,6 +17,7 @@
 # This work is licensed under the terms of the GNU GPL, version 2.  See
 # the COPYING file in the top-level directory.
 
+import array
 import errno
 import json
 import logging
@@ -421,3 +422,17 @@ def is_scm_available(self) -> bool:
         @return True if SCM_RIGHTS is available, otherwise False.
         """
         return self.__sock.family == socket.AF_UNIX
+
+    def send_fd(self, fd: int) -> None:
+        """
+        Send a file descriptor to QEMU via SCM_RIGHTS.
+
+        @param fd (int): file descriptor do be sent
+
+        @raise OSError: if the sendmsg system call fails.
+        """
+        # Send a single space so that QEMU looks at the ancillary data
+        self.__sock.sendmsg((b" ", ),
+                            [(socket.SOL_SOCKET,
+                              socket.SCM_RIGHTS,
+                              array.array("i", [fd]).tobytes())])
-- 
2.31.1




^ permalink raw reply related	[flat|nested] 6+ messages in thread
* [PATCH 2/4] socket_scm_helper: remove
  2021-09-28 13:53 [PATCH 0/3] python: remove socket_scm_helper Paolo Bonzini
  2021-09-28 13:53 ` [PATCH 1/4] python: stop using socket_scm_helper Paolo Bonzini
@ 2021-09-28 13:53 ` Paolo Bonzini
  2021-09-28 13:53 ` [PATCH 3/4] python: raise OSError from send_fd_scm Paolo Bonzini
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 6+ messages in thread
From: Paolo Bonzini @ 2021-09-28 13:53 UTC (permalink / raw)
  To: qemu-devel; +Cc: jsnow

It is not used anymore, remove the source and all remaning traces of it.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 python/qemu/machine/machine.py         |   3 -
 python/qemu/machine/qtest.py           |   2 -
 tests/Makefile.include                 |   5 +-
 tests/meson.build                      |   4 -
 tests/qemu-iotests/iotests.py          |   3 -
 tests/qemu-iotests/meson.build         |   5 -
 tests/qemu-iotests/socket_scm_helper.c | 136 -------------------------
 tests/qemu-iotests/testenv.py          |   8 +-
 8 files changed, 2 insertions(+), 164 deletions(-)
 delete mode 100644 tests/qemu-iotests/meson.build
 delete mode 100644 tests/qemu-iotests/socket_scm_helper.c

diff --git a/python/qemu/machine/machine.py b/python/qemu/machine/machine.py
index e4356ea99e..d230915fbf 100644
--- a/python/qemu/machine/machine.py
+++ b/python/qemu/machine/machine.py
@@ -98,7 +98,6 @@ def __init__(self,
                  name: Optional[str] = None,
                  base_temp_dir: str = "/var/tmp",
                  monitor_address: Optional[SocketAddrT] = None,
-                 socket_scm_helper: Optional[str] = None,
                  sock_dir: Optional[str] = None,
                  drain_console: bool = False,
                  console_log: Optional[str] = None,
@@ -113,7 +112,6 @@ def __init__(self,
         @param name: prefix for socket and log file names (default: qemu-PID)
         @param base_temp_dir: default location where temp files are created
         @param monitor_address: address for QMP monitor
-        @param socket_scm_helper: helper program, required for send_fd_scm()
         @param sock_dir: where to create socket (defaults to base_temp_dir)
         @param drain_console: (optional) True to drain console socket to buffer
         @param console_log: (optional) path to console log file
@@ -134,7 +132,6 @@ def __init__(self,
         self._base_temp_dir = base_temp_dir
         self._sock_dir = sock_dir or self._base_temp_dir
         self._log_dir = log_dir
-        self._socket_scm_helper = socket_scm_helper
 
         if monitor_address is not None:
             self._monitor_address = monitor_address
diff --git a/python/qemu/machine/qtest.py b/python/qemu/machine/qtest.py
index 395cc8fbfe..f2f9aaa5e5 100644
--- a/python/qemu/machine/qtest.py
+++ b/python/qemu/machine/qtest.py
@@ -115,7 +115,6 @@ def __init__(self,
                  wrapper: Sequence[str] = (),
                  name: Optional[str] = None,
                  base_temp_dir: str = "/var/tmp",
-                 socket_scm_helper: Optional[str] = None,
                  sock_dir: Optional[str] = None,
                  qmp_timer: Optional[float] = None):
         # pylint: disable=too-many-arguments
@@ -126,7 +125,6 @@ def __init__(self,
             sock_dir = base_temp_dir
         super().__init__(binary, args, wrapper=wrapper, name=name,
                          base_temp_dir=base_temp_dir,
-                         socket_scm_helper=socket_scm_helper,
                          sock_dir=sock_dir, qmp_timer=qmp_timer)
         self._qtest: Optional[QEMUQtestProtocol] = None
         self._qtest_path = os.path.join(sock_dir, name + "-qtest.sock")
diff --git a/tests/Makefile.include b/tests/Makefile.include
index 7426522bbe..86dc8286b4 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -148,17 +148,14 @@ check-acceptance: check-venv $(TESTS_RESULTS_DIR) get-vm-images
 check:
 
 ifeq ($(CONFIG_TOOLS)$(CONFIG_POSIX),yy)
-QEMU_IOTESTS_HELPERS-$(CONFIG_LINUX) = tests/qemu-iotests/socket_scm_helper$(EXESUF)
 check: check-block
 export PYTHON
 check-block: $(SRC_PATH)/tests/check-block.sh qemu-img$(EXESUF) \
-		qemu-io$(EXESUF) qemu-nbd$(EXESUF) $(QEMU_IOTESTS_HELPERS-y) \
+		qemu-io$(EXESUF) qemu-nbd$(EXESUF) \
 		$(filter qemu-system-%, $(ninja-targets))
 	@$<
 endif
 
-check-build: $(QEMU_IOTESTS_HELPERS-y)
-
 check-clean:
 	rm -rf $(TESTS_VENV_DIR) $(TESTS_RESULTS_DIR)
 
diff --git a/tests/meson.build b/tests/meson.build
index 55a7b08275..3f3882748a 100644
--- a/tests/meson.build
+++ b/tests/meson.build
@@ -67,10 +67,6 @@ if have_tools and 'CONFIG_VHOST_USER' in config_host and 'CONFIG_LINUX' in confi
              dependencies: [qemuutil, vhost_user])
 endif
 
-if have_system and 'CONFIG_POSIX' in config_host
-  subdir('qemu-iotests')
-endif
-
 test('decodetree', sh,
      args: [ files('decode/check.sh'), config_host['PYTHON'], files('../scripts/decodetree.py') ],
      workdir: meson.current_source_dir() / 'decode',
diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index ce06cf5630..9afa258a40 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -109,8 +109,6 @@
 
     qemu_valgrind = ['valgrind', valgrind_logfile, '--error-exitcode=99']
 
-socket_scm_helper = os.environ.get('SOCKET_SCM_HELPER', 'socket_scm_helper')
-
 luks_default_secret_object = 'secret,id=keysec0,data=' + \
                              os.environ.get('IMGKEYSECRET', '')
 luks_default_key_secret_opt = 'key-secret=keysec0'
@@ -600,7 +598,6 @@ def __init__(self, path_suffix=''):
         super().__init__(qemu_prog, qemu_opts, wrapper=wrapper,
                          name=name,
                          base_temp_dir=test_dir,
-                         socket_scm_helper=socket_scm_helper,
                          sock_dir=sock_dir, qmp_timer=timer)
         self._num_drives = 0
 
diff --git a/tests/qemu-iotests/meson.build b/tests/qemu-iotests/meson.build
deleted file mode 100644
index 67aed1e492..0000000000
--- a/tests/qemu-iotests/meson.build
+++ /dev/null
@@ -1,5 +0,0 @@
-if 'CONFIG_LINUX' in config_host
-    socket_scm_helper = executable('socket_scm_helper', 'socket_scm_helper.c')
-else
-    socket_scm_helper = []
-endif
diff --git a/tests/qemu-iotests/socket_scm_helper.c b/tests/qemu-iotests/socket_scm_helper.c
deleted file mode 100644
index eb76d31aa9..0000000000
--- a/tests/qemu-iotests/socket_scm_helper.c
+++ /dev/null
@@ -1,136 +0,0 @@
-/*
- * SCM_RIGHTS with unix socket help program for test
- *
- * Copyright IBM, Inc. 2013
- *
- * Authors:
- *  Wenchao Xia    <xiawenc@linux.vnet.ibm.com>
- *
- * This work is licensed under the terms of the GNU LGPL, version 2 or later.
- * See the COPYING.LIB file in the top-level directory.
- */
-
-#include "qemu/osdep.h"
-#include <sys/socket.h>
-#include <sys/un.h>
-
-/* #define SOCKET_SCM_DEBUG */
-
-/*
- * @fd and @fd_to_send will not be checked for validation in this function,
- * a blank will be sent as iov data to notify qemu.
- */
-static int send_fd(int fd, int fd_to_send)
-{
-    struct msghdr msg;
-    struct iovec iov[1];
-    int ret;
-    char control[CMSG_SPACE(sizeof(int))];
-    struct cmsghdr *cmsg;
-
-    memset(&msg, 0, sizeof(msg));
-    memset(control, 0, sizeof(control));
-
-    /* Send a blank to notify qemu */
-    iov[0].iov_base = (void *)" ";
-    iov[0].iov_len = 1;
-
-    msg.msg_iov = iov;
-    msg.msg_iovlen = 1;
-
-    msg.msg_control = control;
-    msg.msg_controllen = sizeof(control);
-
-    cmsg = CMSG_FIRSTHDR(&msg);
-
-    cmsg->cmsg_len = CMSG_LEN(sizeof(int));
-    cmsg->cmsg_level = SOL_SOCKET;
-    cmsg->cmsg_type = SCM_RIGHTS;
-    memcpy(CMSG_DATA(cmsg), &fd_to_send, sizeof(int));
-
-    do {
-        ret = sendmsg(fd, &msg, 0);
-    } while (ret < 0 && errno == EINTR);
-
-    if (ret < 0) {
-        fprintf(stderr, "Failed to send msg, reason: %s\n", strerror(errno));
-    }
-
-    return ret;
-}
-
-/* Convert string to fd number. */
-static int get_fd_num(const char *fd_str, bool silent)
-{
-    int sock;
-    char *err;
-
-    errno = 0;
-    sock = strtol(fd_str, &err, 10);
-    if (errno) {
-        if (!silent) {
-            fprintf(stderr, "Failed in strtol for socket fd, reason: %s\n",
-                    strerror(errno));
-        }
-        return -1;
-    }
-    if (!*fd_str || *err || sock < 0) {
-        if (!silent) {
-            fprintf(stderr, "bad numerical value for socket fd '%s'\n", fd_str);
-        }
-        return -1;
-    }
-
-    return sock;
-}
-
-/*
- * To make things simple, the caller needs to specify:
- * 1. socket fd.
- * 2. path of the file to be sent.
- */
-int main(int argc, char **argv, char **envp)
-{
-    int sock, fd, ret;
-
-#ifdef SOCKET_SCM_DEBUG
-    int i;
-    for (i = 0; i < argc; i++) {
-        fprintf(stderr, "Parameter %d: %s\n", i, argv[i]);
-    }
-#endif
-
-    if (argc != 3) {
-        fprintf(stderr,
-                "Usage: %s < socket-fd > < file-path >\n",
-                argv[0]);
-        return EXIT_FAILURE;
-    }
-
-
-    sock = get_fd_num(argv[1], false);
-    if (sock < 0) {
-        return EXIT_FAILURE;
-    }
-
-    fd = get_fd_num(argv[2], true);
-    if (fd < 0) {
-        /* Now only open a file in readonly mode for test purpose. If more
-           precise control is needed, use python script in file operation, which
-           is supposed to fork and exec this program. */
-        fd = open(argv[2], O_RDONLY);
-        if (fd < 0) {
-            fprintf(stderr, "Failed to open file '%s'\n", argv[2]);
-            return EXIT_FAILURE;
-        }
-    }
-
-    ret = send_fd(sock, fd);
-    if (ret < 0) {
-        close(fd);
-        return EXIT_FAILURE;
-    }
-
-    close(fd);
-    return EXIT_SUCCESS;
-}
diff --git a/tests/qemu-iotests/testenv.py b/tests/qemu-iotests/testenv.py
index 70da0d60c8..207bafb649 100644
--- a/tests/qemu-iotests/testenv.py
+++ b/tests/qemu-iotests/testenv.py
@@ -68,7 +68,7 @@ class TestEnv(ContextManager['TestEnv']):
     env_variables = ['PYTHONPATH', 'TEST_DIR', 'SOCK_DIR', 'SAMPLE_IMG_DIR',
                      'OUTPUT_DIR', 'PYTHON', 'QEMU_PROG', 'QEMU_IMG_PROG',
                      'QEMU_IO_PROG', 'QEMU_NBD_PROG', 'QSD_PROG',
-                     'SOCKET_SCM_HELPER', 'QEMU_OPTIONS', 'QEMU_IMG_OPTIONS',
+                     'QEMU_OPTIONS', 'QEMU_IMG_OPTIONS',
                      'QEMU_IO_OPTIONS', 'QEMU_IO_OPTIONS_NO_FMT',
                      'QEMU_NBD_OPTIONS', 'IMGOPTS', 'IMGFMT', 'IMGPROTO',
                      'AIOMODE', 'CACHEMODE', 'VALGRIND_QEMU',
@@ -137,7 +137,6 @@ def init_binaries(self) -> None:
         """Init binary path variables:
              PYTHON (for bash tests)
              QEMU_PROG, QEMU_IMG_PROG, QEMU_IO_PROG, QEMU_NBD_PROG, QSD_PROG
-             SOCKET_SCM_HELPER
         """
         self.python = sys.executable
 
@@ -171,10 +170,6 @@ def root(*names: str) -> str:
             if not isxfile(b):
                 sys.exit('Not executable: ' + b)
 
-        helper_path = os.path.join(self.build_iotests, 'socket_scm_helper')
-        if isxfile(helper_path):
-            self.socket_scm_helper = helper_path  # SOCKET_SCM_HELPER
-
     def __init__(self, imgfmt: str, imgproto: str, aiomode: str,
                  cachemode: Optional[str] = None,
                  imgopts: Optional[str] = None,
@@ -300,7 +295,6 @@ def print_env(self) -> None:
 PLATFORM      -- {platform}
 TEST_DIR      -- {TEST_DIR}
 SOCK_DIR      -- {SOCK_DIR}
-SOCKET_SCM_HELPER -- {SOCKET_SCM_HELPER}
 GDB_OPTIONS   -- {GDB_OPTIONS}
 VALGRIND_QEMU -- {VALGRIND_QEMU}
 PRINT_QEMU_OUTPUT -- {PRINT_QEMU}
-- 
2.31.1




^ permalink raw reply related	[flat|nested] 6+ messages in thread
* [PATCH 3/4] python: raise OSError from send_fd_scm
  2021-09-28 13:53 [PATCH 0/3] python: remove socket_scm_helper Paolo Bonzini
  2021-09-28 13:53 ` [PATCH 1/4] python: stop using socket_scm_helper Paolo Bonzini
  2021-09-28 13:53 ` [PATCH 2/4] socket_scm_helper: remove Paolo Bonzini
@ 2021-09-28 13:53 ` Paolo Bonzini
  2021-09-28 13:53 ` [PATCH 4/4] python: split the two sides of send_fd_scm Paolo Bonzini
  2021-09-28 17:46 ` [PATCH 0/3] python: remove socket_scm_helper John Snow
  4 siblings, 0 replies; 6+ messages in thread
From: Paolo Bonzini @ 2021-09-28 13:53 UTC (permalink / raw)
  To: qemu-devel; +Cc: jsnow

The previous weird calling convention was a consequence of using
socket_scm_helper.  Just use exceptions now that it is gone.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 python/qemu/machine/machine.py | 11 ++---------
 tests/qemu-iotests/045         |  3 +--
 tests/qemu-iotests/147         |  3 +--
 3 files changed, 4 insertions(+), 13 deletions(-)

diff --git a/python/qemu/machine/machine.py b/python/qemu/machine/machine.py
index d230915fbf..813ccb17c2 100644
--- a/python/qemu/machine/machine.py
+++ b/python/qemu/machine/machine.py
@@ -208,7 +208,7 @@ def add_fd(self: _T, fd: int, fdset: int,
         return self
 
     def send_fd_scm(self, fd: Optional[int] = None,
-                    file_path: Optional[str] = None) -> int:
+                    file_path: Optional[str] = None) -> None:
         """
         Send an fd or file_path via QMP.
 
@@ -225,19 +225,12 @@ def send_fd_scm(self, fd: Optional[int] = None,
             try:
                 fd = os.open(file_path, os.O_RDONLY)
                 self._qmp.send_fd(fd)
-            except OSError:
-                return 1
             finally:
                 if fd != -1:
                     os.close(fd)
         else:
             assert fd is not None
-            try:
-                self._qmp.send_fd(fd)
-            except OSError:
-                return 1
-
-        return 0
+            self._qmp.send_fd(fd)
 
     @staticmethod
     def _remove_if_exists(path: str) -> None:
diff --git a/tests/qemu-iotests/045 b/tests/qemu-iotests/045
index 45eb239baa..3e6d42010e 100755
--- a/tests/qemu-iotests/045
+++ b/tests/qemu-iotests/045
@@ -141,8 +141,7 @@ class TestSCMFd(iotests.QMPTestCase):
         os.remove(image0)
 
     def _send_fd_by_SCM(self):
-        ret = self.vm.send_fd_scm(file_path=image0)
-        self.assertEqual(ret, 0, 'Failed to send fd with UNIX SCM')
+        self.vm.send_fd_scm(file_path=image0)
 
     def test_add_fd(self):
         self._send_fd_by_SCM()
diff --git a/tests/qemu-iotests/147 b/tests/qemu-iotests/147
index 47dfa62e6b..58de6db52e 100755
--- a/tests/qemu-iotests/147
+++ b/tests/qemu-iotests/147
@@ -269,8 +269,7 @@ class BuiltinNBD(NBDBlockdevAddBase):
         sockfd = socket.socket(socket.AF_UNIX, socket.SOCK_STREAM)
         sockfd.connect(unix_socket)
 
-        result = self.vm.send_fd_scm(fd=sockfd.fileno())
-        self.assertEqual(result, 0, 'Failed to send socket FD')
+        self.vm.send_fd_scm(fd=sockfd.fileno())
 
         result = self.vm.qmp('getfd', fdname='nbd-fifo')
         self.assert_qmp(result, 'return', {})
-- 
2.31.1




^ permalink raw reply related	[flat|nested] 6+ messages in thread
* [PATCH 4/4] python: split the two sides of send_fd_scm
  2021-09-28 13:53 [PATCH 0/3] python: remove socket_scm_helper Paolo Bonzini
                   ` (2 preceding siblings ...)
  2021-09-28 13:53 ` [PATCH 3/4] python: raise OSError from send_fd_scm Paolo Bonzini
@ 2021-09-28 13:53 ` Paolo Bonzini
  2021-09-28 17:46 ` [PATCH 0/3] python: remove socket_scm_helper John Snow
  4 siblings, 0 replies; 6+ messages in thread
From: Paolo Bonzini @ 2021-09-28 13:53 UTC (permalink / raw)
  To: qemu-devel; +Cc: jsnow

send_fd_scm can be used as a simple wrapper for self._qmp.send_fd, or it
can be given a file that will be opened for the duration of the sendmsg
system call.  Split the two cases to separate functions.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 python/qemu/machine/machine.py | 34 ++++++++++++++++------------------
 tests/qemu-iotests/045         |  2 +-
 tests/qemu-iotests/147         |  2 +-
 3 files changed, 18 insertions(+), 20 deletions(-)

diff --git a/python/qemu/machine/machine.py b/python/qemu/machine/machine.py
index 813ccb17c2..8ad3604049 100644
--- a/python/qemu/machine/machine.py
+++ b/python/qemu/machine/machine.py
@@ -207,30 +207,28 @@ def add_fd(self: _T, fd: int, fdset: int,
         self._args.append(','.join(options))
         return self
 
-    def send_fd_scm(self, fd: Optional[int] = None,
-                    file_path: Optional[str] = None) -> None:
+    def send_file_scm(self, file_path: str) -> None:
         """
-        Send an fd or file_path via QMP.
-
-        Exactly one of fd and file_path must be given.
-        If it is file_path, the function will open that file and pass
-        its own fd.
+        Open a file and pass it to QEMU as a file descriptor.
         """
         # In iotest.py, the qmp should always use unix socket.
         assert self._qmp.is_scm_available()
 
-        if file_path is not None:
-            assert fd is None
-            fd = -1
-            try:
-                fd = os.open(file_path, os.O_RDONLY)
-                self._qmp.send_fd(fd)
-            finally:
-                if fd != -1:
-                    os.close(fd)
-        else:
-            assert fd is not None
+        fd = -1
+        try:
+            fd = os.open(file_path, os.O_RDONLY)
             self._qmp.send_fd(fd)
+        finally:
+            if fd != -1:
+                os.close(fd)
+
+    def send_fd_scm(self, fd: int) -> None:
+        """
+        Send a file descriptor via QMP.
+        """
+        # In iotest.py, the qmp should always use unix socket.
+        assert self._qmp.is_scm_available()
+        self._qmp.send_fd(fd)
 
     @staticmethod
     def _remove_if_exists(path: str) -> None:
diff --git a/tests/qemu-iotests/045 b/tests/qemu-iotests/045
index 3e6d42010e..1d1fe4a19a 100755
--- a/tests/qemu-iotests/045
+++ b/tests/qemu-iotests/045
@@ -141,7 +141,7 @@ class TestSCMFd(iotests.QMPTestCase):
         os.remove(image0)
 
     def _send_fd_by_SCM(self):
-        self.vm.send_fd_scm(file_path=image0)
+        self.vm.send_file_scm(image0)
 
     def test_add_fd(self):
         self._send_fd_by_SCM()
diff --git a/tests/qemu-iotests/147 b/tests/qemu-iotests/147
index 58de6db52e..e493ff4d0d 100755
--- a/tests/qemu-iotests/147
+++ b/tests/qemu-iotests/147
@@ -269,7 +269,7 @@ class BuiltinNBD(NBDBlockdevAddBase):
         sockfd = socket.socket(socket.AF_UNIX, socket.SOCK_STREAM)
         sockfd.connect(unix_socket)
 
-        self.vm.send_fd_scm(fd=sockfd.fileno())
+        self.vm.send_fd_scm(sockfd.fileno())
 
         result = self.vm.qmp('getfd', fdname='nbd-fifo')
         self.assert_qmp(result, 'return', {})
-- 
2.31.1



^ permalink raw reply related	[flat|nested] 6+ messages in thread
* Re: [PATCH 0/3] python: remove socket_scm_helper
  2021-09-28 13:53 [PATCH 0/3] python: remove socket_scm_helper Paolo Bonzini
                   ` (3 preceding siblings ...)
  2021-09-28 13:53 ` [PATCH 4/4] python: split the two sides of send_fd_scm Paolo Bonzini
@ 2021-09-28 17:46 ` John Snow
  4 siblings, 0 replies; 6+ messages in thread
From: John Snow @ 2021-09-28 17:46 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: qemu-devel

[-- Attachment #1: Type: text/plain, Size: 2043 bytes --]

On Tue, Sep 28, 2021 at 9:53 AM Paolo Bonzini <pbonzini@redhat.com> wrote:

> I suspect no one has ever looked at socket_scm_helper.c, because when I
> did my reaction was just "why".  The functionality of this 136-line program
> can be reproduced in Python with fewer lines of code than it takes to
> invoke it.  Do it, and let it rest in peace.
>
> The only reason not to do that could be because of the upcoming switch
> to aqmp.  For that, the new send_fd method has to be wrapped to use
> transport.get_extra_info('socket') and loop.run_in_executor.  Let me
> know if you prefer to hold on this until later.
>
> Paolo
>
> Paolo Bonzini (4):
>   python: stop using socket_scm_helper
>   socket_scm_helper: remove
>   python: raise OSError from send_fd_scm
>   python: split the two sides of send_fd_scm
>
>  python/qemu/machine/machine.py         |  58 +++--------
>  python/qemu/machine/qtest.py           |   2 -
>  python/qemu/qmp/__init__.py            |  15 +++
>  tests/Makefile.include                 |   5 +-
>  tests/meson.build                      |   4 -
>  tests/qemu-iotests/045                 |   3 +-
>  tests/qemu-iotests/147                 |   3 +-
>  tests/qemu-iotests/iotests.py          |   3 -
>  tests/qemu-iotests/meson.build         |   5 -
>  tests/qemu-iotests/socket_scm_helper.c | 136 -------------------------
>  tests/qemu-iotests/testenv.py          |   8 +-
>  11 files changed, 35 insertions(+), 207 deletions(-)
>  delete mode 100644 tests/qemu-iotests/meson.build
>  delete mode 100644 tests/qemu-iotests/socket_scm_helper.c
>
> --
> 2.31.1
>

 We discussed this extremely briefly on IRC, but I have some similar
patches already floating around on the list, so I won't be taking these.
Nevertheless, we're in agreement about socket_scm_helper setting sail for
the great beyond!
Any additional cleanups made in these patches I'll just pull into my
ongoing series of Python cleanups and add additional authorship credits.

Thanks!
--js

(For patchew? I don't know if this works. Let's find out:)
NACK

[-- Attachment #2: Type: text/html, Size: 2730 bytes --]

^ permalink raw reply	[flat|nested] 6+ messages in thread
end of thread, other threads:[~2021-09-28 17:50 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-28 13:53 [PATCH 0/3] python: remove socket_scm_helper Paolo Bonzini
2021-09-28 13:53 ` [PATCH 1/4] python: stop using socket_scm_helper Paolo Bonzini
2021-09-28 13:53 ` [PATCH 2/4] socket_scm_helper: remove Paolo Bonzini
2021-09-28 13:53 ` [PATCH 3/4] python: raise OSError from send_fd_scm Paolo Bonzini
2021-09-28 13:53 ` [PATCH 4/4] python: split the two sides of send_fd_scm Paolo Bonzini
2021-09-28 17:46 ` [PATCH 0/3] python: remove socket_scm_helper John Snow

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).