[Qemu-devel] [PATCH V2 0/3] qemu-iotests: add test for fd passing via SCM rights

[Qemu-devel] [PATCH V2 0/3] qemu-iotests: add test for fd passing via SCM rights

[Wenchao Xia]

This series add test case for fd passing with unix socket at runtime. Since
getfd and closefd interface will interact with monitor's data, so it will
help to do regression test for monitor patches. Since python2 do not support
sendmsg(), so a C helper program is added to do the job.

v2:
  1: add missing $ in the makefile rule.

Wenchao Xia (3):
  1 qemu-iotests: add unix socket help program
  2 qemu-iotests: add infrastructure of fd passing via SCM
  3 qemu-iotests: add tests for runtime fd passing via SCM rights

 QMP/qmp.py                             |    6 ++
 configure                              |    2 +-
 tests/Makefile                         |    4 +-
 tests/qemu-iotests/045                 |   37 ++++++++++-
 tests/qemu-iotests/045.out             |    4 +-
 tests/qemu-iotests/check               |    1 +
 tests/qemu-iotests/iotests.py          |   26 +++++++
 tests/qemu-iotests/socket_scm_helper.c |  119 ++++++++++++++++++++++++++++++++
 8 files changed, 194 insertions(+), 5 deletions(-)
 create mode 100644 tests/qemu-iotests/socket_scm_helper.c

[Qemu-devel] [PATCH V2 1/3] qemu-iotests: add unix socket help program

[Wenchao Xia]

This program can do a sendmsg call to transfer fd with unix
socket, which is not supported in python2.

The built binary will not be deleted in clean, but it is a
existing issue in ./tests, which should be solved in another
patch.

Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
---
 configure                              |    2 +-
 tests/Makefile                         |    4 +-
 tests/qemu-iotests/socket_scm_helper.c |  119 ++++++++++++++++++++++++++++++++
 3 files changed, 123 insertions(+), 2 deletions(-)
 create mode 100644 tests/qemu-iotests/socket_scm_helper.c

diff --git a/configure b/configure
index 0a55c20..5080c38 100755
--- a/configure
+++ b/configure
@@ -4544,7 +4544,7 @@ if [ "$dtc_internal" = "yes" ]; then
 fi
 
 # build tree in object directory in case the source is not in the current directory
-DIRS="tests tests/tcg tests/tcg/cris tests/tcg/lm32 tests/libqos tests/qapi-schema tests/tcg/xtensa"
+DIRS="tests tests/tcg tests/tcg/cris tests/tcg/lm32 tests/libqos tests/qapi-schema tests/tcg/xtensa tests/qemu-iotests"
 DIRS="$DIRS pc-bios/optionrom pc-bios/spapr-rtas pc-bios/s390-ccw"
 DIRS="$DIRS roms/seabios roms/vgabios"
 DIRS="$DIRS qapi-generated"
diff --git a/tests/Makefile b/tests/Makefile
index baba9e9..d2f3fcb 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -172,6 +172,8 @@ tests/boot-order-test$(EXESUF): tests/boot-order-test.o $(libqos-obj-y)
 tests/tmp105-test$(EXESUF): tests/tmp105-test.o $(libqos-omap-obj-y)
 tests/i440fx-test$(EXESUF): tests/i440fx-test.o $(libqos-pc-obj-y)
 tests/fw_cfg-test$(EXESUF): tests/fw_cfg-test.o $(libqos-pc-obj-y)
+tests/qemu-iotests/socket_scm_helper$(EXESUF): tests/qemu-iotests/socket_scm_helper.o
+	$(call LINK, $^)
 
 # QTest rules
 
@@ -250,7 +252,7 @@ check-report.html: check-report.xml
 # Other tests
 
 .PHONY: check-tests/qemu-iotests-quick.sh
-check-tests/qemu-iotests-quick.sh: tests/qemu-iotests-quick.sh qemu-img$(EXESUF) qemu-io$(EXESUF)
+check-tests/qemu-iotests-quick.sh: tests/qemu-iotests-quick.sh qemu-img$(EXESUF) qemu-io$(EXESUF) tests/qemu-iotests/socket_scm_helper$(EXESUF)
 	$<
 
 .PHONY: check-tests/test-qapi.py
diff --git a/tests/qemu-iotests/socket_scm_helper.c b/tests/qemu-iotests/socket_scm_helper.c
new file mode 100644
index 0000000..1955a3e
--- /dev/null
+++ b/tests/qemu-iotests/socket_scm_helper.c
@@ -0,0 +1,119 @@
+/*
+ * SCM_RIGHT 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 <stdio.h>
+#include <errno.h>
+#include <sys/socket.h>
+#include <sys/un.h>
+#include <stdlib.h>
+
+/* #define SOCKET_SCM_DEBUG */
+
+static int send_fd(int fd, int fd_to_send, const char *data, size_t len)
+{
+    struct msghdr msg;
+    struct iovec iov[1];
+    int ret;
+    char control[CMSG_SPACE(sizeof(int))];
+    struct cmsghdr *cmsg;
+
+    if (fd < 0) {
+        fprintf(stderr, "Socket fd is invalid.\n");
+        return -1;
+    }
+    if (fd_to_send < 0) {
+        fprintf(stderr, "Fd to send is invalid.\n");
+        return -1;
+    }
+    if (data == NULL || len <= 0) {
+        fprintf(stderr, "Data buffer must be valid.\n");
+        return -1;
+    }
+
+    memset(&msg, 0, sizeof(msg));
+    memset(control, 0, sizeof(control));
+
+    iov[0].iov_base = (void *)data;
+    iov[0].iov_len = len;
+
+    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, 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;
+}
+
+/*
+ * To make things simple, the caller need to specify:
+ * 1. socket fd.
+ * 2. fd to send.
+ * 3. msg to send.
+ */
+int main(int argc, char **argv, char **envp)
+{
+    int sock, fd, ret, buflen;
+    const char *buf;
+#ifdef SOCKET_SCM_DEBUG
+    int i;
+    for (i = 0; i < argc; i++) {
+        fprintf(stderr, "Parameter %d: %s.\n", i, argv[i]);
+    }
+#endif
+
+    if (argc < 4) {
+        fprintf(stderr,
+                "Invalid parameter, use it as:\n"
+                "%s SOCKET_FD FD_TO_SEND MSG.\n",
+                argv[0]);
+        return 1;
+    }
+
+    errno = 0;
+    sock = strtol(argv[1], NULL, 10);
+    if (errno) {
+        fprintf(stderr, "Failed in strtol for socket fd, reason: %s.\n",
+                strerror(errno));
+        return 1;
+    }
+    fd = strtol(argv[2], NULL, 10);
+    if (errno) {
+        fprintf(stderr, "Failed in strtol for fd to send, reason: %s.\n",
+                strerror(errno));
+        return 1;
+    }
+
+    buf = argv[3];
+    buflen = strlen(buf);
+
+    ret = send_fd(sock, fd, buf, buflen);
+    if (ret < 0) {
+        return 1;
+    }
+    return 0;
+}
-- 
1.7.1

[Qemu-devel] [PATCH V2 2/3] qemu-iotests: add infrastructure of fd passing via SCM

[Wenchao Xia]

This patch make use of the compiled scm helper program to transfer
fd via unix socket at runtime.

Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
---
 QMP/qmp.py                    |    6 ++++++
 tests/qemu-iotests/check      |    1 +
 tests/qemu-iotests/iotests.py |   26 ++++++++++++++++++++++++++
 3 files changed, 33 insertions(+), 0 deletions(-)

diff --git a/QMP/qmp.py b/QMP/qmp.py
index c551df1..074f09a 100644
--- a/QMP/qmp.py
+++ b/QMP/qmp.py
@@ -188,3 +188,9 @@ class QEMUMonitorProtocol:
 
     def settimeout(self, timeout):
         self.__sock.settimeout(timeout)
+
+    def get_sock_fd(self):
+        return self.__sock.fileno()
+
+    def is_scm_available(self):
+        return self.__sock.family == socket.AF_UNIX
diff --git a/tests/qemu-iotests/check b/tests/qemu-iotests/check
index 74628ae..2f7f78e 100755
--- a/tests/qemu-iotests/check
+++ b/tests/qemu-iotests/check
@@ -164,6 +164,7 @@ QEMU_IO       -- $QEMU_IO
 IMGFMT        -- $FULL_IMGFMT_DETAILS
 IMGPROTO      -- $FULL_IMGPROTO_DETAILS
 PLATFORM      -- $FULL_HOST_DETAILS
+SOCKET_SCM_HELPER -- $SOCKET_SCM_HELPER
 
 EOF
 #MKFS_OPTIONS  -- $FULL_MKFS_OPTIONS
diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index 33ad0ec..d40b2de 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -38,6 +38,8 @@ imgfmt = os.environ.get('IMGFMT', 'raw')
 imgproto = os.environ.get('IMGPROTO', 'file')
 test_dir = os.environ.get('TEST_DIR', '/var/tmp')
 
+socket_scm_helper = os.environ.get('SOCKET_SCM_HELPER', 'socket_scm_helper')
+
 def qemu_img(*args):
     '''Run qemu-img and return the exit code'''
     devnull = open('/dev/null', 'r+')
@@ -80,6 +82,12 @@ class VM(object):
                      '-display', 'none', '-vga', 'none']
         self._num_drives = 0
 
+    #This can be used to add unused monitor
+    def add_monitor_telnet(self, ip, port):
+        args = 'tcp:%s:%d,server,nowait,telnet' % (ip, port)
+        self._args.append('-monitor')
+        self._args.append(args)
+
     def add_drive(self, path, opts=''):
         '''Add a virtio-blk drive to the VM'''
         options = ['if=virtio',
@@ -112,6 +120,24 @@ class VM(object):
         self._args.append(','.join(options))
         return self
 
+    #Sendmsg must carry out some data to get qemu's notice, so send a blank
+    #by default.
+    def send_fd_scm(self, fd, msg=' '):
+        #In iotest.py, the qmp should always use unix socket.
+        assert self._qmp.is_scm_available()
+        fd_bin = socket_scm_helper
+        if os.path.exists(fd_bin) == False:
+            print "Scm help program does not present, path %s." % fd_bin
+            return -1
+        fd_param = ["%s" % fd_bin,
+                    "%d" % self._qmp.get_sock_fd(),
+                    "%d" % fd,
+                    "%s" % msg]
+        devnull = open('/dev/null', 'rb')
+        p = subprocess.Popen(fd_param, stdin=devnull, stdout=sys.stdout,
+                             stderr=sys.stderr)
+        return p.wait()
+
     def launch(self):
         '''Launch the VM and establish a QMP connection'''
         devnull = open('/dev/null', 'rb')
-- 
1.7.1

[Qemu-devel] [PATCH V2 3/3] qemu-iotests: add tests for runtime fd passing via SCM rights

[Wenchao Xia]

This case will test whether the monitor can receive fd at runtime.
To verify better, additional monitor is created to see if qemu
can handler two monitor instance correctly.

Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
---
 tests/qemu-iotests/045     |   37 ++++++++++++++++++++++++++++++++++++-
 tests/qemu-iotests/045.out |    4 ++--
 2 files changed, 38 insertions(+), 3 deletions(-)

diff --git a/tests/qemu-iotests/045 b/tests/qemu-iotests/045
index 2b6f1af..f95e5fa 100755
--- a/tests/qemu-iotests/045
+++ b/tests/qemu-iotests/045
@@ -1,6 +1,6 @@
 #!/usr/bin/env python
 #
-# Tests for fdsets.
+# Tests for fdsets and getfd.
 #
 # Copyright (C) 2012 IBM Corp.
 #
@@ -125,5 +125,40 @@ class TestFdSets(iotests.QMPTestCase):
                 'No file descriptor supplied via SCM_RIGHTS')
         self.vm.shutdown()
 
+#Add fd at runtime, there are two ways: monitor related or fdset related
+class TestSCMFd(iotests.QMPTestCase):
+    def setUp(self):
+        self.vm = iotests.VM()
+        qemu_img('create', '-f', iotests.imgfmt, image0, '128K')
+        self.file0 = open(image0, 'r')
+        self.vm.add_monitor_telnet("0",4445)
+        self.vm.launch()
+
+    def tearDown(self):
+        self.vm.shutdown()
+        self.file0.close()
+        os.remove(image0)
+
+    def _send_fd_by_SCM(self):
+        ret = self.vm.send_fd_scm(self.file0.fileno())
+        self.assertEqual(ret, 0, 'Failed to send fd with UNIX SCM')
+
+    def test_add_fd(self):
+        self._send_fd_by_SCM()
+        result = self.vm.qmp('add-fd', fdset_id=2, opaque='image0:r')
+        self.assert_qmp(result, 'return/fdset-id', 2)
+
+    def test_getfd(self):
+        self._send_fd_by_SCM()
+        result = self.vm.qmp('getfd', fdname='image0:r')
+        self.assert_qmp(result, 'return', {})
+
+    def test_closefd(self):
+        self._send_fd_by_SCM()
+        result = self.vm.qmp('getfd', fdname='image0:r')
+        self.assert_qmp(result, 'return', {})
+        result = self.vm.qmp('closefd', fdname='image0:r')
+        self.assert_qmp(result, 'return', {})
+
 if __name__ == '__main__':
     iotests.main(supported_fmts=['raw'])
diff --git a/tests/qemu-iotests/045.out b/tests/qemu-iotests/045.out
index 3f8a935..dae404e 100644
--- a/tests/qemu-iotests/045.out
+++ b/tests/qemu-iotests/045.out
@@ -1,5 +1,5 @@
-......
+.........
 ----------------------------------------------------------------------
-Ran 6 tests
+Ran 9 tests
 
 OK
-- 
1.7.1

Re: [Qemu-devel] [PATCH V2 1/3] qemu-iotests: add unix socket help program

[Eric Blake]

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

On 08/26/2013 08:52 PM, Wenchao Xia wrote:
> This program can do a sendmsg call to transfer fd with unix
> socket, which is not supported in python2.
> 
> The built binary will not be deleted in clean, but it is a
> existing issue in ./tests, which should be solved in another
> patch.
> 
> Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
> ---

> +++ b/tests/qemu-iotests/socket_scm_helper.c
> @@ -0,0 +1,119 @@
> +/*
> + * SCM_RIGHT with unix socket help program for test

s/SCM_RIGHT/&S/

> +
> +static int send_fd(int fd, int fd_to_send, const char *data, size_t len)
> +{
> +    struct msghdr msg;
> +    struct iovec iov[1];
> +    int ret;
> +    char control[CMSG_SPACE(sizeof(int))];
> +    struct cmsghdr *cmsg;
> +
> +    if (fd < 0) {
> +        fprintf(stderr, "Socket fd is invalid.\n");
> +        return -1;
> +    }
> +    if (fd_to_send < 0) {
> +        fprintf(stderr, "Fd to send is invalid.\n");
> +        return -1;
> +    }
> +    if (data == NULL || len <= 0) {

len cannot be < 0, since it is size_t (or did you mean for it to be
ssize_t?)


> +    if (ret < 0) {
> +        fprintf(stderr, "Failed to send msg, reason: %s.\n", strerror(errno));

Messages typically shouldn't end with '.'; especially when ending the
message with strerror.

> +    }
> +
> +    return ret;
> +}
> +
> +/*
> + * To make things simple, the caller need to specify:

s/need/needs/

> + * 1. socket fd.
> + * 2. fd to send.
> + * 3. msg to send.
> + */
> +int main(int argc, char **argv, char **envp)
> +{
> +    int sock, fd, ret, buflen;
> +    const char *buf;
> +#ifdef SOCKET_SCM_DEBUG
> +    int i;
> +    for (i = 0; i < argc; i++) {
> +        fprintf(stderr, "Parameter %d: %s.\n", i, argv[i]);

Another useless ending '.' (and elsewhere throughout the patch)

> +    }
> +#endif
> +
> +    if (argc < 4) {
> +        fprintf(stderr,
> +                "Invalid parameter, use it as:\n"
> +                "%s SOCKET_FD FD_TO_SEND MSG.\n",
> +                argv[0]);

This rejects too few, but not too many, arguments.  Should the condition
be: if (argc != 4)

> +        return 1;

I prefer EXIT_FAILURE over the magic number 1 (multiple instances).

> +    }
> +
> +    errno = 0;
> +    sock = strtol(argv[1], NULL, 10);

Failure to pass in a second argument means you cannot distinguish ""
from "0" from "0a" - all three input strings for argv[1] result in
sock==0 without you knowing any better.  If you're going to use
strtol(), use it correctly; if you don't care about garbage, then atoi()
is just as (in)correct and with less typing.

> +    if (errno) {
> +        fprintf(stderr, "Failed in strtol for socket fd, reason: %s.\n",
> +                strerror(errno));
> +        return 1;
> +    }
> +    fd = strtol(argv[2], NULL, 10);
> +    if (errno) {
> +        fprintf(stderr, "Failed in strtol for fd to send, reason: %s.\n",
> +                strerror(errno));
> +        return 1;
> +    }
> +
> +    buf = argv[3];
> +    buflen = strlen(buf);
> +
> +    ret = send_fd(sock, fd, buf, buflen);
> +    if (ret < 0) {
> +        return 1;
> +    }
> +    return 0;

I prefer EXIT_SUCCESS over the magic number 0.

> +}
> 

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 621 bytes --]

Re: [Qemu-devel] [PATCH V2 1/3] qemu-iotests: add unix socket help program

[Wenchao Xia]

于 2013-8-28 9:11, Eric Blake 写道:
> On 08/26/2013 08:52 PM, Wenchao Xia wrote:
>> This program can do a sendmsg call to transfer fd with unix
>> socket, which is not supported in python2.
>>
>> The built binary will not be deleted in clean, but it is a
>> existing issue in ./tests, which should be solved in another
>> patch.
>>
>> Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
>> ---
>
>> +++ b/tests/qemu-iotests/socket_scm_helper.c
>> @@ -0,0 +1,119 @@
>> +/*
>> + * SCM_RIGHT with unix socket help program for test
>
> s/SCM_RIGHT/&S/
>
>> +
>> +static int send_fd(int fd, int fd_to_send, const char *data, size_t len)
>> +{
>> +    struct msghdr msg;
>> +    struct iovec iov[1];
>> +    int ret;
>> +    char control[CMSG_SPACE(sizeof(int))];
>> +    struct cmsghdr *cmsg;
>> +
>> +    if (fd < 0) {
>> +        fprintf(stderr, "Socket fd is invalid.\n");
>> +        return -1;
>> +    }
>> +    if (fd_to_send < 0) {
>> +        fprintf(stderr, "Fd to send is invalid.\n");
>> +        return -1;
>> +    }
>> +    if (data == NULL || len <= 0) {
>
> len cannot be < 0, since it is size_t (or did you mean for it to be
> ssize_t?)
>
>
>> +    if (ret < 0) {
>> +        fprintf(stderr, "Failed to send msg, reason: %s.\n", strerror(errno));
>
> Messages typically shouldn't end with '.'; especially when ending the
> message with strerror.
>
>> +    }
>> +
>> +    return ret;
>> +}
>> +
>> +/*
>> + * To make things simple, the caller need to specify:
>
> s/need/needs/
>
>> + * 1. socket fd.
>> + * 2. fd to send.
>> + * 3. msg to send.
>> + */
>> +int main(int argc, char **argv, char **envp)
>> +{
>> +    int sock, fd, ret, buflen;
>> +    const char *buf;
>> +#ifdef SOCKET_SCM_DEBUG
>> +    int i;
>> +    for (i = 0; i < argc; i++) {
>> +        fprintf(stderr, "Parameter %d: %s.\n", i, argv[i]);
>
> Another useless ending '.' (and elsewhere throughout the patch)
>
>> +    }
>> +#endif
>> +
>> +    if (argc < 4) {
>> +        fprintf(stderr,
>> +                "Invalid parameter, use it as:\n"
>> +                "%s SOCKET_FD FD_TO_SEND MSG.\n",
>> +                argv[0]);
>
> This rejects too few, but not too many, arguments.  Should the condition
> be: if (argc != 4)
>
>> +        return 1;
>
> I prefer EXIT_FAILURE over the magic number 1 (multiple instances).
>
>> +    }
>> +
>> +    errno = 0;
>> +    sock = strtol(argv[1], NULL, 10);
>
> Failure to pass in a second argument means you cannot distinguish ""
> from "0" from "0a" - all three input strings for argv[1] result in
> sock==0 without you knowing any better.  If you're going to use
> strtol(), use it correctly; if you don't care about garbage, then atoi()
> is just as (in)correct and with less typing.
>
     I will check error more carefully with other issues addressed in
next version. Since the build system is modified, I'd like to wait a
few days to see if more comment comes.

>> +    if (errno) {
>> +        fprintf(stderr, "Failed in strtol for socket fd, reason: %s.\n",
>> +                strerror(errno));
>> +        return 1;
>> +    }
>> +    fd = strtol(argv[2], NULL, 10);
>> +    if (errno) {
>> +        fprintf(stderr, "Failed in strtol for fd to send, reason: %s.\n",
>> +                strerror(errno));
>> +        return 1;
>> +    }
>> +
>> +    buf = argv[3];
>> +    buflen = strlen(buf);
>> +
>> +    ret = send_fd(sock, fd, buf, buflen);
>> +    if (ret < 0) {
>> +        return 1;
>> +    }
>> +    return 0;
>
> I prefer EXIT_SUCCESS over the magic number 0.
>
>> +}
>>
>


-- 
Best Regards

Wenchao Xia

Re: [Qemu-devel] [PATCH V2 1/3] qemu-iotests: add unix socket help program

[Luiz Capitulino]

On Tue, 27 Aug 2013 10:52:09 +0800
Wenchao Xia <xiawenc@linux.vnet.ibm.com> wrote:

> This program can do a sendmsg call to transfer fd with unix
> socket, which is not supported in python2.
> 
> The built binary will not be deleted in clean, but it is a
> existing issue in ./tests, which should be solved in another
> patch.

Review comments in addition to Eric's.

> 
> Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
> ---
>  configure                              |    2 +-
>  tests/Makefile                         |    4 +-
>  tests/qemu-iotests/socket_scm_helper.c |  119 ++++++++++++++++++++++++++++++++
>  3 files changed, 123 insertions(+), 2 deletions(-)
>  create mode 100644 tests/qemu-iotests/socket_scm_helper.c
> 
> diff --git a/configure b/configure
> index 0a55c20..5080c38 100755
> --- a/configure
> +++ b/configure
> @@ -4544,7 +4544,7 @@ if [ "$dtc_internal" = "yes" ]; then
>  fi
>  
>  # build tree in object directory in case the source is not in the current directory
> -DIRS="tests tests/tcg tests/tcg/cris tests/tcg/lm32 tests/libqos tests/qapi-schema tests/tcg/xtensa"
> +DIRS="tests tests/tcg tests/tcg/cris tests/tcg/lm32 tests/libqos tests/qapi-schema tests/tcg/xtensa tests/qemu-iotests"
>  DIRS="$DIRS pc-bios/optionrom pc-bios/spapr-rtas pc-bios/s390-ccw"
>  DIRS="$DIRS roms/seabios roms/vgabios"
>  DIRS="$DIRS qapi-generated"
> diff --git a/tests/Makefile b/tests/Makefile
> index baba9e9..d2f3fcb 100644
> --- a/tests/Makefile
> +++ b/tests/Makefile
> @@ -172,6 +172,8 @@ tests/boot-order-test$(EXESUF): tests/boot-order-test.o $(libqos-obj-y)
>  tests/tmp105-test$(EXESUF): tests/tmp105-test.o $(libqos-omap-obj-y)
>  tests/i440fx-test$(EXESUF): tests/i440fx-test.o $(libqos-pc-obj-y)
>  tests/fw_cfg-test$(EXESUF): tests/fw_cfg-test.o $(libqos-pc-obj-y)
> +tests/qemu-iotests/socket_scm_helper$(EXESUF): tests/qemu-iotests/socket_scm_helper.o
> +	$(call LINK, $^)
>  
>  # QTest rules
>  
> @@ -250,7 +252,7 @@ check-report.html: check-report.xml
>  # Other tests
>  
>  .PHONY: check-tests/qemu-iotests-quick.sh
> -check-tests/qemu-iotests-quick.sh: tests/qemu-iotests-quick.sh qemu-img$(EXESUF) qemu-io$(EXESUF)
> +check-tests/qemu-iotests-quick.sh: tests/qemu-iotests-quick.sh qemu-img$(EXESUF) qemu-io$(EXESUF) tests/qemu-iotests/socket_scm_helper$(EXESUF)
>  	$<
>  
>  .PHONY: check-tests/test-qapi.py
> diff --git a/tests/qemu-iotests/socket_scm_helper.c b/tests/qemu-iotests/socket_scm_helper.c
> new file mode 100644
> index 0000000..1955a3e
> --- /dev/null
> +++ b/tests/qemu-iotests/socket_scm_helper.c
> @@ -0,0 +1,119 @@
> +/*
> + * SCM_RIGHT 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 <stdio.h>
> +#include <errno.h>
> +#include <sys/socket.h>
> +#include <sys/un.h>
> +#include <stdlib.h>
> +
> +/* #define SOCKET_SCM_DEBUG */
> +
> +static int send_fd(int fd, int fd_to_send, const char *data, size_t len)
> +{
> +    struct msghdr msg;
> +    struct iovec iov[1];
> +    int ret;
> +    char control[CMSG_SPACE(sizeof(int))];
> +    struct cmsghdr *cmsg;
> +
> +    if (fd < 0) {
> +        fprintf(stderr, "Socket fd is invalid.\n");
> +        return -1;
> +    }
> +    if (fd_to_send < 0) {
> +        fprintf(stderr, "Fd to send is invalid.\n");
> +        return -1;
> +    }
> +    if (data == NULL || len <= 0) {
> +        fprintf(stderr, "Data buffer must be valid.\n");
> +        return -1;
> +    }

I haven't looked at the test itself yet, but my idea for the helper was
to have something like "pass-fd < file > < qmp-socket >". So, what the
helper does is to open 'file' and pass its fd to 'qmp-socket'. Seems
simpler to me.

> +
> +    memset(&msg, 0, sizeof(msg));
> +    memset(control, 0, sizeof(control));
> +
> +    iov[0].iov_base = (void *)data;
> +    iov[0].iov_len = len;
> +
> +    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, 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;
> +}
> +
> +/*
> + * To make things simple, the caller need to specify:
> + * 1. socket fd.
> + * 2. fd to send.
> + * 3. msg to send.
> + */
> +int main(int argc, char **argv, char **envp)
> +{
> +    int sock, fd, ret, buflen;
> +    const char *buf;
> +#ifdef SOCKET_SCM_DEBUG
> +    int i;
> +    for (i = 0; i < argc; i++) {
> +        fprintf(stderr, "Parameter %d: %s.\n", i, argv[i]);
> +    }
> +#endif
> +
> +    if (argc < 4) {
> +        fprintf(stderr,
> +                "Invalid parameter, use it as:\n"
> +                "%s SOCKET_FD FD_TO_SEND MSG.\n",
> +                argv[0]);
> +        return 1;
> +    }

Some like "usage: %s < socket-fd > < fd-to-send > < msg >\n" is more
readable, IMO.

> +
> +    errno = 0;
> +    sock = strtol(argv[1], NULL, 10);
> +    if (errno) {
> +        fprintf(stderr, "Failed in strtol for socket fd, reason: %s.\n",
> +                strerror(errno));
> +        return 1;
> +    }

Apart from what Eric suggested, you could move this to a function.

> +    fd = strtol(argv[2], NULL, 10);
> +    if (errno) {
> +        fprintf(stderr, "Failed in strtol for fd to send, reason: %s.\n",
> +                strerror(errno));
> +        return 1;
> +    }
> +
> +    buf = argv[3];
> +    buflen = strlen(buf);
> +
> +    ret = send_fd(sock, fd, buf, buflen);
> +    if (ret < 0) {
> +        return 1;
> +    }
> +    return 0;
> +}

Re: [Qemu-devel] [PATCH V2 2/3] qemu-iotests: add infrastructure of fd passing via SCM

[Luiz Capitulino]

On Tue, 27 Aug 2013 10:52:10 +0800
Wenchao Xia <xiawenc@linux.vnet.ibm.com> wrote:

> This patch make use of the compiled scm helper program to transfer
> fd via unix socket at runtime.

I'm not familiar with the qemu-iotests and I can't tell if this
makes sense.

Kevin, Stefan, Markus, can you help reviewing this?

> 
> Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
> ---
>  QMP/qmp.py                    |    6 ++++++
>  tests/qemu-iotests/check      |    1 +
>  tests/qemu-iotests/iotests.py |   26 ++++++++++++++++++++++++++
>  3 files changed, 33 insertions(+), 0 deletions(-)
> 
> diff --git a/QMP/qmp.py b/QMP/qmp.py
> index c551df1..074f09a 100644
> --- a/QMP/qmp.py
> +++ b/QMP/qmp.py
> @@ -188,3 +188,9 @@ class QEMUMonitorProtocol:
>  
>      def settimeout(self, timeout):
>          self.__sock.settimeout(timeout)
> +
> +    def get_sock_fd(self):
> +        return self.__sock.fileno()
> +
> +    def is_scm_available(self):
> +        return self.__sock.family == socket.AF_UNIX
> diff --git a/tests/qemu-iotests/check b/tests/qemu-iotests/check
> index 74628ae..2f7f78e 100755
> --- a/tests/qemu-iotests/check
> +++ b/tests/qemu-iotests/check
> @@ -164,6 +164,7 @@ QEMU_IO       -- $QEMU_IO
>  IMGFMT        -- $FULL_IMGFMT_DETAILS
>  IMGPROTO      -- $FULL_IMGPROTO_DETAILS
>  PLATFORM      -- $FULL_HOST_DETAILS
> +SOCKET_SCM_HELPER -- $SOCKET_SCM_HELPER
>  
>  EOF
>  #MKFS_OPTIONS  -- $FULL_MKFS_OPTIONS
> diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
> index 33ad0ec..d40b2de 100644
> --- a/tests/qemu-iotests/iotests.py
> +++ b/tests/qemu-iotests/iotests.py
> @@ -38,6 +38,8 @@ imgfmt = os.environ.get('IMGFMT', 'raw')
>  imgproto = os.environ.get('IMGPROTO', 'file')
>  test_dir = os.environ.get('TEST_DIR', '/var/tmp')
>  
> +socket_scm_helper = os.environ.get('SOCKET_SCM_HELPER', 'socket_scm_helper')
> +
>  def qemu_img(*args):
>      '''Run qemu-img and return the exit code'''
>      devnull = open('/dev/null', 'r+')
> @@ -80,6 +82,12 @@ class VM(object):
>                       '-display', 'none', '-vga', 'none']
>          self._num_drives = 0
>  
> +    #This can be used to add unused monitor
> +    def add_monitor_telnet(self, ip, port):
> +        args = 'tcp:%s:%d,server,nowait,telnet' % (ip, port)
> +        self._args.append('-monitor')
> +        self._args.append(args)
> +
>      def add_drive(self, path, opts=''):
>          '''Add a virtio-blk drive to the VM'''
>          options = ['if=virtio',
> @@ -112,6 +120,24 @@ class VM(object):
>          self._args.append(','.join(options))
>          return self
>  
> +    #Sendmsg must carry out some data to get qemu's notice, so send a blank
> +    #by default.
> +    def send_fd_scm(self, fd, msg=' '):
> +        #In iotest.py, the qmp should always use unix socket.
> +        assert self._qmp.is_scm_available()
> +        fd_bin = socket_scm_helper
> +        if os.path.exists(fd_bin) == False:
> +            print "Scm help program does not present, path %s." % fd_bin
> +            return -1
> +        fd_param = ["%s" % fd_bin,
> +                    "%d" % self._qmp.get_sock_fd(),
> +                    "%d" % fd,
> +                    "%s" % msg]
> +        devnull = open('/dev/null', 'rb')
> +        p = subprocess.Popen(fd_param, stdin=devnull, stdout=sys.stdout,
> +                             stderr=sys.stderr)
> +        return p.wait()
> +
>      def launch(self):
>          '''Launch the VM and establish a QMP connection'''
>          devnull = open('/dev/null', 'rb')

Re: [Qemu-devel] [PATCH V2 1/3] qemu-iotests: add unix socket help program

[Wenchao Xia]

于 2013-8-29 22:50, Luiz Capitulino 写道:
> On Tue, 27 Aug 2013 10:52:09 +0800
> Wenchao Xia <xiawenc@linux.vnet.ibm.com> wrote:
>
>> This program can do a sendmsg call to transfer fd with unix
>> socket, which is not supported in python2.
>>
>> The built binary will not be deleted in clean, but it is a
>> existing issue in ./tests, which should be solved in another
>> patch.
>
> Review comments in addition to Eric's.
>
>>
>> Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
>> ---
>>   configure                              |    2 +-
>>   tests/Makefile                         |    4 +-
>>   tests/qemu-iotests/socket_scm_helper.c |  119 ++++++++++++++++++++++++++++++++
>>   3 files changed, 123 insertions(+), 2 deletions(-)
>>   create mode 100644 tests/qemu-iotests/socket_scm_helper.c
>>
>> diff --git a/configure b/configure
>> index 0a55c20..5080c38 100755
>> --- a/configure
>> +++ b/configure
>> @@ -4544,7 +4544,7 @@ if [ "$dtc_internal" = "yes" ]; then
>>   fi
>>
>>   # build tree in object directory in case the source is not in the current directory
>> -DIRS="tests tests/tcg tests/tcg/cris tests/tcg/lm32 tests/libqos tests/qapi-schema tests/tcg/xtensa"
>> +DIRS="tests tests/tcg tests/tcg/cris tests/tcg/lm32 tests/libqos tests/qapi-schema tests/tcg/xtensa tests/qemu-iotests"
>>   DIRS="$DIRS pc-bios/optionrom pc-bios/spapr-rtas pc-bios/s390-ccw"
>>   DIRS="$DIRS roms/seabios roms/vgabios"
>>   DIRS="$DIRS qapi-generated"
>> diff --git a/tests/Makefile b/tests/Makefile
>> index baba9e9..d2f3fcb 100644
>> --- a/tests/Makefile
>> +++ b/tests/Makefile
>> @@ -172,6 +172,8 @@ tests/boot-order-test$(EXESUF): tests/boot-order-test.o $(libqos-obj-y)
>>   tests/tmp105-test$(EXESUF): tests/tmp105-test.o $(libqos-omap-obj-y)
>>   tests/i440fx-test$(EXESUF): tests/i440fx-test.o $(libqos-pc-obj-y)
>>   tests/fw_cfg-test$(EXESUF): tests/fw_cfg-test.o $(libqos-pc-obj-y)
>> +tests/qemu-iotests/socket_scm_helper$(EXESUF): tests/qemu-iotests/socket_scm_helper.o
>> +	$(call LINK, $^)
>>
>>   # QTest rules
>>
>> @@ -250,7 +252,7 @@ check-report.html: check-report.xml
>>   # Other tests
>>
>>   .PHONY: check-tests/qemu-iotests-quick.sh
>> -check-tests/qemu-iotests-quick.sh: tests/qemu-iotests-quick.sh qemu-img$(EXESUF) qemu-io$(EXESUF)
>> +check-tests/qemu-iotests-quick.sh: tests/qemu-iotests-quick.sh qemu-img$(EXESUF) qemu-io$(EXESUF) tests/qemu-iotests/socket_scm_helper$(EXESUF)
>>   	$<
>>
>>   .PHONY: check-tests/test-qapi.py
>> diff --git a/tests/qemu-iotests/socket_scm_helper.c b/tests/qemu-iotests/socket_scm_helper.c
>> new file mode 100644
>> index 0000000..1955a3e
>> --- /dev/null
>> +++ b/tests/qemu-iotests/socket_scm_helper.c
>> @@ -0,0 +1,119 @@
>> +/*
>> + * SCM_RIGHT 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 <stdio.h>
>> +#include <errno.h>
>> +#include <sys/socket.h>
>> +#include <sys/un.h>
>> +#include <stdlib.h>
>> +
>> +/* #define SOCKET_SCM_DEBUG */
>> +
>> +static int send_fd(int fd, int fd_to_send, const char *data, size_t len)
>> +{
>> +    struct msghdr msg;
>> +    struct iovec iov[1];
>> +    int ret;
>> +    char control[CMSG_SPACE(sizeof(int))];
>> +    struct cmsghdr *cmsg;
>> +
>> +    if (fd < 0) {
>> +        fprintf(stderr, "Socket fd is invalid.\n");
>> +        return -1;
>> +    }
>> +    if (fd_to_send < 0) {
>> +        fprintf(stderr, "Fd to send is invalid.\n");
>> +        return -1;
>> +    }
>> +    if (data == NULL || len <= 0) {
>> +        fprintf(stderr, "Data buffer must be valid.\n");
>> +        return -1;
>> +    }
>
> I haven't looked at the test itself yet, but my idea for the helper was
> to have something like "pass-fd < file > < qmp-socket >". So, what the
> helper does is to open 'file' and pass its fd to 'qmp-socket'. Seems
> simpler to me.
>
   OK, will use that style. Do you think the data parameter should be
kept? I added it to let test framework choose one char, which can avoid
break the qmp protocol.

>> +
>> +    memset(&msg, 0, sizeof(msg));
>> +    memset(control, 0, sizeof(control));
>> +
>> +    iov[0].iov_base = (void *)data;
>> +    iov[0].iov_len = len;
>> +
>> +    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, 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;
>> +}
>> +
>> +/*
>> + * To make things simple, the caller need to specify:
>> + * 1. socket fd.
>> + * 2. fd to send.
>> + * 3. msg to send.
>> + */
>> +int main(int argc, char **argv, char **envp)
>> +{
>> +    int sock, fd, ret, buflen;
>> +    const char *buf;
>> +#ifdef SOCKET_SCM_DEBUG
>> +    int i;
>> +    for (i = 0; i < argc; i++) {
>> +        fprintf(stderr, "Parameter %d: %s.\n", i, argv[i]);
>> +    }
>> +#endif
>> +
>> +    if (argc < 4) {
>> +        fprintf(stderr,
>> +                "Invalid parameter, use it as:\n"
>> +                "%s SOCKET_FD FD_TO_SEND MSG.\n",
>> +                argv[0]);
>> +        return 1;
>> +    }
>
> Some like "usage: %s < socket-fd > < fd-to-send > < msg >\n" is more
> readable, IMO.
>
   It is better, will use that.

>> +
>> +    errno = 0;
>> +    sock = strtol(argv[1], NULL, 10);
>> +    if (errno) {
>> +        fprintf(stderr, "Failed in strtol for socket fd, reason: %s.\n",
>> +                strerror(errno));
>> +        return 1;
>> +    }
>
> Apart from what Eric suggested, you could move this to a function.
>
   OK.

>> +    fd = strtol(argv[2], NULL, 10);
>> +    if (errno) {
>> +        fprintf(stderr, "Failed in strtol for fd to send, reason: %s.\n",
>> +                strerror(errno));
>> +        return 1;
>> +    }
>> +
>> +    buf = argv[3];
>> +    buflen = strlen(buf);
>> +
>> +    ret = send_fd(sock, fd, buf, buflen);
>> +    if (ret < 0) {
>> +        return 1;
>> +    }
>> +    return 0;
>> +}
>


-- 
Best Regards

Wenchao Xia

Re: [Qemu-devel] [PATCH V2 1/3] qemu-iotests: add unix socket help program

[Luiz Capitulino]

On Fri, 30 Aug 2013 10:42:27 +0800
Wenchao Xia <xiawenc@linux.vnet.ibm.com> wrote:

> 于 2013-8-29 22:50, Luiz Capitulino 写道:
> > On Tue, 27 Aug 2013 10:52:09 +0800
> > Wenchao Xia <xiawenc@linux.vnet.ibm.com> wrote:
> >
> >> This program can do a sendmsg call to transfer fd with unix
> >> socket, which is not supported in python2.
> >>
> >> The built binary will not be deleted in clean, but it is a
> >> existing issue in ./tests, which should be solved in another
> >> patch.
> >
> > Review comments in addition to Eric's.
> >
> >>
> >> Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
> >> ---
> >>   configure                              |    2 +-
> >>   tests/Makefile                         |    4 +-
> >>   tests/qemu-iotests/socket_scm_helper.c |  119 ++++++++++++++++++++++++++++++++
> >>   3 files changed, 123 insertions(+), 2 deletions(-)
> >>   create mode 100644 tests/qemu-iotests/socket_scm_helper.c
> >>
> >> diff --git a/configure b/configure
> >> index 0a55c20..5080c38 100755
> >> --- a/configure
> >> +++ b/configure
> >> @@ -4544,7 +4544,7 @@ if [ "$dtc_internal" = "yes" ]; then
> >>   fi
> >>
> >>   # build tree in object directory in case the source is not in the current directory
> >> -DIRS="tests tests/tcg tests/tcg/cris tests/tcg/lm32 tests/libqos tests/qapi-schema tests/tcg/xtensa"
> >> +DIRS="tests tests/tcg tests/tcg/cris tests/tcg/lm32 tests/libqos tests/qapi-schema tests/tcg/xtensa tests/qemu-iotests"
> >>   DIRS="$DIRS pc-bios/optionrom pc-bios/spapr-rtas pc-bios/s390-ccw"
> >>   DIRS="$DIRS roms/seabios roms/vgabios"
> >>   DIRS="$DIRS qapi-generated"
> >> diff --git a/tests/Makefile b/tests/Makefile
> >> index baba9e9..d2f3fcb 100644
> >> --- a/tests/Makefile
> >> +++ b/tests/Makefile
> >> @@ -172,6 +172,8 @@ tests/boot-order-test$(EXESUF): tests/boot-order-test.o $(libqos-obj-y)
> >>   tests/tmp105-test$(EXESUF): tests/tmp105-test.o $(libqos-omap-obj-y)
> >>   tests/i440fx-test$(EXESUF): tests/i440fx-test.o $(libqos-pc-obj-y)
> >>   tests/fw_cfg-test$(EXESUF): tests/fw_cfg-test.o $(libqos-pc-obj-y)
> >> +tests/qemu-iotests/socket_scm_helper$(EXESUF): tests/qemu-iotests/socket_scm_helper.o
> >> +	$(call LINK, $^)
> >>
> >>   # QTest rules
> >>
> >> @@ -250,7 +252,7 @@ check-report.html: check-report.xml
> >>   # Other tests
> >>
> >>   .PHONY: check-tests/qemu-iotests-quick.sh
> >> -check-tests/qemu-iotests-quick.sh: tests/qemu-iotests-quick.sh qemu-img$(EXESUF) qemu-io$(EXESUF)
> >> +check-tests/qemu-iotests-quick.sh: tests/qemu-iotests-quick.sh qemu-img$(EXESUF) qemu-io$(EXESUF) tests/qemu-iotests/socket_scm_helper$(EXESUF)
> >>   	$<
> >>
> >>   .PHONY: check-tests/test-qapi.py
> >> diff --git a/tests/qemu-iotests/socket_scm_helper.c b/tests/qemu-iotests/socket_scm_helper.c
> >> new file mode 100644
> >> index 0000000..1955a3e
> >> --- /dev/null
> >> +++ b/tests/qemu-iotests/socket_scm_helper.c
> >> @@ -0,0 +1,119 @@
> >> +/*
> >> + * SCM_RIGHT 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 <stdio.h>
> >> +#include <errno.h>
> >> +#include <sys/socket.h>
> >> +#include <sys/un.h>
> >> +#include <stdlib.h>
> >> +
> >> +/* #define SOCKET_SCM_DEBUG */
> >> +
> >> +static int send_fd(int fd, int fd_to_send, const char *data, size_t len)
> >> +{
> >> +    struct msghdr msg;
> >> +    struct iovec iov[1];
> >> +    int ret;
> >> +    char control[CMSG_SPACE(sizeof(int))];
> >> +    struct cmsghdr *cmsg;
> >> +
> >> +    if (fd < 0) {
> >> +        fprintf(stderr, "Socket fd is invalid.\n");
> >> +        return -1;
> >> +    }
> >> +    if (fd_to_send < 0) {
> >> +        fprintf(stderr, "Fd to send is invalid.\n");
> >> +        return -1;
> >> +    }
> >> +    if (data == NULL || len <= 0) {
> >> +        fprintf(stderr, "Data buffer must be valid.\n");
> >> +        return -1;
> >> +    }
> >
> > I haven't looked at the test itself yet, but my idea for the helper was
> > to have something like "pass-fd < file > < qmp-socket >". So, what the
> > helper does is to open 'file' and pass its fd to 'qmp-socket'. Seems
> > simpler to me.
> >
>    OK, will use that style. Do you think the data parameter should be
> kept? I added it to let test framework choose one char, which can avoid
> break the qmp protocol.

If you're passing a QMP command there then it can stay.

> 
> >> +
> >> +    memset(&msg, 0, sizeof(msg));
> >> +    memset(control, 0, sizeof(control));
> >> +
> >> +    iov[0].iov_base = (void *)data;
> >> +    iov[0].iov_len = len;
> >> +
> >> +    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, 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;
> >> +}
> >> +
> >> +/*
> >> + * To make things simple, the caller need to specify:
> >> + * 1. socket fd.
> >> + * 2. fd to send.
> >> + * 3. msg to send.
> >> + */
> >> +int main(int argc, char **argv, char **envp)
> >> +{
> >> +    int sock, fd, ret, buflen;
> >> +    const char *buf;
> >> +#ifdef SOCKET_SCM_DEBUG
> >> +    int i;
> >> +    for (i = 0; i < argc; i++) {
> >> +        fprintf(stderr, "Parameter %d: %s.\n", i, argv[i]);
> >> +    }
> >> +#endif
> >> +
> >> +    if (argc < 4) {
> >> +        fprintf(stderr,
> >> +                "Invalid parameter, use it as:\n"
> >> +                "%s SOCKET_FD FD_TO_SEND MSG.\n",
> >> +                argv[0]);
> >> +        return 1;
> >> +    }
> >
> > Some like "usage: %s < socket-fd > < fd-to-send > < msg >\n" is more
> > readable, IMO.
> >
>    It is better, will use that.
> 
> >> +
> >> +    errno = 0;
> >> +    sock = strtol(argv[1], NULL, 10);
> >> +    if (errno) {
> >> +        fprintf(stderr, "Failed in strtol for socket fd, reason: %s.\n",
> >> +                strerror(errno));
> >> +        return 1;
> >> +    }
> >
> > Apart from what Eric suggested, you could move this to a function.
> >
>    OK.
> 
> >> +    fd = strtol(argv[2], NULL, 10);
> >> +    if (errno) {
> >> +        fprintf(stderr, "Failed in strtol for fd to send, reason: %s.\n",
> >> +                strerror(errno));
> >> +        return 1;
> >> +    }
> >> +
> >> +    buf = argv[3];
> >> +    buflen = strlen(buf);
> >> +
> >> +    ret = send_fd(sock, fd, buf, buflen);
> >> +    if (ret < 0) {
> >> +        return 1;
> >> +    }
> >> +    return 0;
> >> +}
> >
> 
> 

Re: [Qemu-devel] [PATCH V2 1/3] qemu-iotests: add unix socket help program

[Wenchao Xia]

于 2013-8-30 19:33, Luiz Capitulino 写道:
> On Fri, 30 Aug 2013 10:42:27 +0800
> Wenchao Xia <xiawenc@linux.vnet.ibm.com> wrote:
>
>> 于 2013-8-29 22:50, Luiz Capitulino 写道:
>>> On Tue, 27 Aug 2013 10:52:09 +0800
>>> Wenchao Xia <xiawenc@linux.vnet.ibm.com> wrote:
>>>
>>>> This program can do a sendmsg call to transfer fd with unix
>>>> socket, which is not supported in python2.
>>>>
>>>> The built binary will not be deleted in clean, but it is a
>>>> existing issue in ./tests, which should be solved in another
>>>> patch.
>>>
>>> Review comments in addition to Eric's.
>>>
>>>>
>>>> Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
>>>> ---
>>>>    configure                              |    2 +-
>>>>    tests/Makefile                         |    4 +-
>>>>    tests/qemu-iotests/socket_scm_helper.c |  119 ++++++++++++++++++++++++++++++++
>>>>    3 files changed, 123 insertions(+), 2 deletions(-)
>>>>    create mode 100644 tests/qemu-iotests/socket_scm_helper.c
>>>>
>>>> diff --git a/configure b/configure
>>>> index 0a55c20..5080c38 100755
>>>> --- a/configure
>>>> +++ b/configure
>>>> @@ -4544,7 +4544,7 @@ if [ "$dtc_internal" = "yes" ]; then
>>>>    fi
>>>>
>>>>    # build tree in object directory in case the source is not in the current directory
>>>> -DIRS="tests tests/tcg tests/tcg/cris tests/tcg/lm32 tests/libqos tests/qapi-schema tests/tcg/xtensa"
>>>> +DIRS="tests tests/tcg tests/tcg/cris tests/tcg/lm32 tests/libqos tests/qapi-schema tests/tcg/xtensa tests/qemu-iotests"
>>>>    DIRS="$DIRS pc-bios/optionrom pc-bios/spapr-rtas pc-bios/s390-ccw"
>>>>    DIRS="$DIRS roms/seabios roms/vgabios"
>>>>    DIRS="$DIRS qapi-generated"
>>>> diff --git a/tests/Makefile b/tests/Makefile
>>>> index baba9e9..d2f3fcb 100644
>>>> --- a/tests/Makefile
>>>> +++ b/tests/Makefile
>>>> @@ -172,6 +172,8 @@ tests/boot-order-test$(EXESUF): tests/boot-order-test.o $(libqos-obj-y)
>>>>    tests/tmp105-test$(EXESUF): tests/tmp105-test.o $(libqos-omap-obj-y)
>>>>    tests/i440fx-test$(EXESUF): tests/i440fx-test.o $(libqos-pc-obj-y)
>>>>    tests/fw_cfg-test$(EXESUF): tests/fw_cfg-test.o $(libqos-pc-obj-y)
>>>> +tests/qemu-iotests/socket_scm_helper$(EXESUF): tests/qemu-iotests/socket_scm_helper.o
>>>> +	$(call LINK, $^)
>>>>
>>>>    # QTest rules
>>>>
>>>> @@ -250,7 +252,7 @@ check-report.html: check-report.xml
>>>>    # Other tests
>>>>
>>>>    .PHONY: check-tests/qemu-iotests-quick.sh
>>>> -check-tests/qemu-iotests-quick.sh: tests/qemu-iotests-quick.sh qemu-img$(EXESUF) qemu-io$(EXESUF)
>>>> +check-tests/qemu-iotests-quick.sh: tests/qemu-iotests-quick.sh qemu-img$(EXESUF) qemu-io$(EXESUF) tests/qemu-iotests/socket_scm_helper$(EXESUF)
>>>>    	$<
>>>>
>>>>    .PHONY: check-tests/test-qapi.py
>>>> diff --git a/tests/qemu-iotests/socket_scm_helper.c b/tests/qemu-iotests/socket_scm_helper.c
>>>> new file mode 100644
>>>> index 0000000..1955a3e
>>>> --- /dev/null
>>>> +++ b/tests/qemu-iotests/socket_scm_helper.c
>>>> @@ -0,0 +1,119 @@
>>>> +/*
>>>> + * SCM_RIGHT 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 <stdio.h>
>>>> +#include <errno.h>
>>>> +#include <sys/socket.h>
>>>> +#include <sys/un.h>
>>>> +#include <stdlib.h>
>>>> +
>>>> +/* #define SOCKET_SCM_DEBUG */
>>>> +
>>>> +static int send_fd(int fd, int fd_to_send, const char *data, size_t len)
>>>> +{
>>>> +    struct msghdr msg;
>>>> +    struct iovec iov[1];
>>>> +    int ret;
>>>> +    char control[CMSG_SPACE(sizeof(int))];
>>>> +    struct cmsghdr *cmsg;
>>>> +
>>>> +    if (fd < 0) {
>>>> +        fprintf(stderr, "Socket fd is invalid.\n");
>>>> +        return -1;
>>>> +    }
>>>> +    if (fd_to_send < 0) {
>>>> +        fprintf(stderr, "Fd to send is invalid.\n");
>>>> +        return -1;
>>>> +    }
>>>> +    if (data == NULL || len <= 0) {
>>>> +        fprintf(stderr, "Data buffer must be valid.\n");
>>>> +        return -1;
>>>> +    }
>>>
>>> I haven't looked at the test itself yet, but my idea for the helper was
>>> to have something like "pass-fd < file > < qmp-socket >". So, what the
>>> helper does is to open 'file' and pass its fd to 'qmp-socket'. Seems
>>> simpler to me.
>>>
>>     OK, will use that style. Do you think the data parameter should be
>> kept? I added it to let test framework choose one char, which can avoid
>> break the qmp protocol.
>
> If you're passing a QMP command there then it can stay.
>
   OK, I'll remove it, since the python script didn't passing a QMP
command now, but a char which was chosen by script and didn't break the
QMP protocol in the wire.

>>
>>>> +
>>>> +    memset(&msg, 0, sizeof(msg));
>>>> +    memset(control, 0, sizeof(control));
>>>> +
>>>> +    iov[0].iov_base = (void *)data;
>>>> +    iov[0].iov_len = len;
>>>> +
>>>> +    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, 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;
>>>> +}
>>>> +
>>>> +/*
>>>> + * To make things simple, the caller need to specify:
>>>> + * 1. socket fd.
>>>> + * 2. fd to send.
>>>> + * 3. msg to send.
>>>> + */
>>>> +int main(int argc, char **argv, char **envp)
>>>> +{
>>>> +    int sock, fd, ret, buflen;
>>>> +    const char *buf;
>>>> +#ifdef SOCKET_SCM_DEBUG
>>>> +    int i;
>>>> +    for (i = 0; i < argc; i++) {
>>>> +        fprintf(stderr, "Parameter %d: %s.\n", i, argv[i]);
>>>> +    }
>>>> +#endif
>>>> +
>>>> +    if (argc < 4) {
>>>> +        fprintf(stderr,
>>>> +                "Invalid parameter, use it as:\n"
>>>> +                "%s SOCKET_FD FD_TO_SEND MSG.\n",
>>>> +                argv[0]);
>>>> +        return 1;
>>>> +    }
>>>
>>> Some like "usage: %s < socket-fd > < fd-to-send > < msg >\n" is more
>>> readable, IMO.
>>>
>>     It is better, will use that.
>>
>>>> +
>>>> +    errno = 0;
>>>> +    sock = strtol(argv[1], NULL, 10);
>>>> +    if (errno) {
>>>> +        fprintf(stderr, "Failed in strtol for socket fd, reason: %s.\n",
>>>> +                strerror(errno));
>>>> +        return 1;
>>>> +    }
>>>
>>> Apart from what Eric suggested, you could move this to a function.
>>>
>>     OK.
>>
>>>> +    fd = strtol(argv[2], NULL, 10);
>>>> +    if (errno) {
>>>> +        fprintf(stderr, "Failed in strtol for fd to send, reason: %s.\n",
>>>> +                strerror(errno));
>>>> +        return 1;
>>>> +    }
>>>> +
>>>> +    buf = argv[3];
>>>> +    buflen = strlen(buf);
>>>> +
>>>> +    ret = send_fd(sock, fd, buf, buflen);
>>>> +    if (ret < 0) {
>>>> +        return 1;
>>>> +    }
>>>> +    return 0;
>>>> +}
>>>
>>
>>
>


-- 
Best Regards

Wenchao Xia