| Message ID | 1377571931-9144-2-git-send-email-xiawenc@linux.vnet.ibm.com |
|---|---|
| State | New |
| Headers | show |
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. > +} >
于 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. > >> +} >> >
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; > +}
于 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; >> +} >
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; > >> +} > > > >
于 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; >>>> +} >>> >> >> >
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; +}
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