On Fri, 30 Aug 2013 10:42:27 +0800
Wenchao Xia <xiaw...@linux.vnet.ibm.com> wrote:
> δΊ 2013-8-29 22:50, Luiz Capitulino ει:
> > On Tue, 27 Aug 2013 10:52:09 +0800
> > Wenchao Xia <xiaw...@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 <xiaw...@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 <xiaw...@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;
> >> +}
> >
>
>
