From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:43413) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VFEfw-0002ch-Rs for qemu-devel@nongnu.org; Thu, 29 Aug 2013 22:43:13 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1VFEfo-0006FI-Uk for qemu-devel@nongnu.org; Thu, 29 Aug 2013 22:43:04 -0400 Received: from e23smtp07.au.ibm.com ([202.81.31.140]:50249) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VFEfo-0006FC-A0 for qemu-devel@nongnu.org; Thu, 29 Aug 2013 22:42:56 -0400 Received: from /spool/local by e23smtp07.au.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Fri, 30 Aug 2013 12:28:51 +1000 Received: from d23relay05.au.ibm.com (d23relay05.au.ibm.com [9.190.235.152]) by d23dlp01.au.ibm.com (Postfix) with ESMTP id AD8272CE804D for ; Fri, 30 Aug 2013 12:42:35 +1000 (EST) Received: from d23av03.au.ibm.com (d23av03.au.ibm.com [9.190.234.97]) by d23relay05.au.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id r7U2QIB88520128 for ; Fri, 30 Aug 2013 12:26:18 +1000 Received: from d23av03.au.ibm.com (localhost [127.0.0.1]) by d23av03.au.ibm.com (8.14.4/8.14.4/NCO v10.0 AVout) with ESMTP id r7U2gYDs008388 for ; Fri, 30 Aug 2013 12:42:35 +1000 Message-ID: <52200693.6070602@linux.vnet.ibm.com> Date: Fri, 30 Aug 2013 10:42:27 +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> In-Reply-To: <20130829105007.37454b3d@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-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. >> + >> + 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