From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:47853) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VGJR6-0005w8-K3 for qemu-devel@nongnu.org; Sun, 01 Sep 2013 22:00:20 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1VGJQy-0005M5-H3 for qemu-devel@nongnu.org; Sun, 01 Sep 2013 22:00:12 -0400 Received: from e23smtp01.au.ibm.com ([202.81.31.143]:58028) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VGJQx-00058R-N5 for qemu-devel@nongnu.org; Sun, 01 Sep 2013 22:00:04 -0400 Received: from /spool/local by e23smtp01.au.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Mon, 2 Sep 2013 11:49:16 +1000 Received: from d23relay04.au.ibm.com (d23relay04.au.ibm.com [9.190.234.120]) by d23dlp01.au.ibm.com (Postfix) with ESMTP id 5F3E12CE804D for ; Mon, 2 Sep 2013 11:59:55 +1000 (EST) Received: from d23av01.au.ibm.com (d23av01.au.ibm.com [9.190.234.96]) by d23relay04.au.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id r821hbVd50200622 for ; Mon, 2 Sep 2013 11:43:43 +1000 Received: from d23av01.au.ibm.com (localhost [127.0.0.1]) by d23av01.au.ibm.com (8.14.4/8.14.4/NCO v10.0 AVout) with ESMTP id r821xmVM012082 for ; Mon, 2 Sep 2013 11:59:48 +1000 Message-ID: <5223F109.8010801@linux.vnet.ibm.com> Date: Mon, 02 Sep 2013 09:59:37 +0800 From: Wenchao Xia MIME-Version: 1.0 References: <1377571931-9144-1-git-send-email-xiawenc@linux.vnet.ibm.com> <1377571931-9144-2-git-send-email-xiawenc@linux.vnet.ibm.com> <20130829105007.37454b3d@redhat.com> <52200693.6070602@linux.vnet.ibm.com> <20130830073310.4a4f20c6@redhat.com> In-Reply-To: <20130830073310.4a4f20c6@redhat.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit Subject: Re: [Qemu-devel] [PATCH V2 1/3] qemu-iotests: add unix socket help program List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Luiz Capitulino Cc: pbonzini@redhat.com, qemu-devel@nongnu.org, anthony@codemonkey.ws, armbru@redhat.com 于 2013-8-30 19:33, Luiz Capitulino 写道: > On Fri, 30 Aug 2013 10:42:27 +0800 > Wenchao Xia wrote: > >> 于 2013-8-29 22:50, Luiz Capitulino 写道: >>> On Tue, 27 Aug 2013 10:52:09 +0800 >>> 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. >>> >>> Review comments in addition to Eric's. >>> >>>> >>>> Signed-off-by: Wenchao Xia >>>> --- >>>> 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 >>>> + * >>>> + * 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 >>>> +#include >>>> +#include >>>> +#include >>>> +#include >>>> + >>>> +/* #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