Enable passing pre-opened chardev socket FD
v5 cover Enable passing pre-opened chardev socket FD
An update of:
v1: https://lists.gnu.org/archive/html/qemu-devel/2017-12/msg04618.html
v2: https://lists.gnu.org/archive/html/qemu-devel/2017-12/msg04706.html
v3: https://lists.gnu.org/archive/html/qemu-devel/2017-12/msg04892.html
v4: https://lists.gnu.org/archive/html/qemu-devel/2018-02/msg00950.html
This enables fixing a long standing problem that libvirt has with starting up QEMU. It has to busy-wait retrying connect() on the QMP monitor socket until QEMU finally creates & listens on it, but at same time must be careful to not wait forever if QEMU exits.
With this patch series, libvirt can simply pass in a pre-opened UNIX domain socket file descriptor, which it can immediately connect to with no busy-wait.
NB, this will generate one expected failure with patchew / checkpatch.pl
ERROR: consider using qemu_strtol in preference to strtol #729: FILE: util/cutils.c:338:
+ lresult = strtol(nptr, &ep, base);
ERROR: consider using qemu_strtol in preference to strtol #779: FILE: util/cutils.c:388:
+ lresult = strtol(nptr, &ep, base);
This is ok to ignore, because the patch in question is introducing new qemu_strtoXXX wrappers
Changed in v5:
- Docs typo (Eric)
- Fix errno setting in int parsing code (Eric)
Changed in v4:
- Add test for fd_is_socket() API (Marc-Andre)
Changed in v3:
- Introduce qemu_strtoi & qemu_stroui functions.
- Split patchs up into more pieces to better separate each logical change
- Introduce a new test/test-sockets.c to directly test the SocketAddress FD handling, separately from chardev code.
- Add qapi docs for FD passing syntax
- Other misc fixes in tests
- Reduce code duplication when getting pre-opened FDs in socket_connect/listen.
Changed in v2:
- Drop 'fdset' property / address kind, and use 'fd' for both CLI and HMP
- Add unit tests
Daniel P. Berrangé (9):
char: don't silently skip tn3270 protocol init when TLS is enabled
cutils: add qemu_strtoi & qemu_strtoui parsers for int/unsigned int
types
sockets: pull code for testing IP availability out of specific test
sockets: strengthen test suite IP protocol availability checks
sockets: move fd_is_socket() into common sockets code
sockets: check that the named file descriptor is a socket
sockets: allow SocketAddress 'fd' to reference numeric file
descriptors
char: refactor parsing of socket address information
char: allow passing pre-opened socket file descriptor at startup
chardev/char-socket.c | 34 ++- chardev/char.c | 3 + include/qemu/cutils.h | 4 + include/qemu/sockets.h | 1 + io/channel-util.c | 13 - qapi/sockets.json | 7 + tests/.gitignore | 1 + tests/Makefile.include | 5 +- tests/socket-helpers.c | 153 ++++++++++ tests/socket-helpers.h | 42 +++ tests/test-char.c | 47 ++- tests/test-cutils.c | 657 +++++++++++++++++++++++++++++++++++++++++ tests/test-io-channel-socket.c | 72 +---- tests/test-util-sockets.c | 266 +++++++++++++++++ util/cutils.c | 108 +++++++ util/qemu-sockets.c | 36 ++- 16 files changed, 1351 insertions(+), 98 deletions(-) create mode 100644 tests/socket-helpers.c create mode 100644 tests/socket-helpers.h create mode 100644 tests/test-util-sockets.c
Signature · 1 line
2.14.3
PATCH v5 1/9 char: don't silently skip tn3270 protocol init when TLS is enabled
From: "Daniel P. Berrange" <[email protected]>
Even if common tn3270 implementations do not support TLS, it is trivial to have them proxied over a proxy like stunnel which adds TLS at the sockets layer. We should thus not silently skip tn3270 protocol initialization when TLS is enabled.
--- chardev/char-socket.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)
chardev/char-socket.c +1 -2
@@ -663,8 +663,7 @@ static void tcp_chr_tls_handshake(QIOTask *task,if (qio_task_propagate_error (task,NULL )) {tcp_chr_disconnect (chr); }else { -/* tn3270 does not support TLS yet */ -if (s->do_telnetopt && !s->is_tn3270) { +if (s->do_telnetopt) {tcp_chr_telnet_init (chr); }else {tcp_chr_connect (chr);
Signature · 1 line
2.14.3
PATCH v5 2/9 cutils: add qemu_strtoi & qemu_strtoui parsers for int/unsigned int types
In reply to Daniel P. BerrangéFrom: "Daniel P. Berrange" <[email protected]>
There are qemu_strtoNN functions for various sized integers. This adds two more for plain int & unsigned int types, with suitable range checking.
--- include/qemu/cutils.h | 4 + tests/test-cutils.c | 657 ++++++++++++++++++++++++++++++++++++++++++++++++++ util/cutils.c | 108 +++++++++ 3 files changed, 769 insertions(+)
include/qemu/cutils.h +4 -0
@@ -126,6 +126,10 @@ time_t mktimegm(struct tm *tm);int qemu_fdatasync (int fd);int fcntl_setfl (int fd,int flag);int qemu_parse_fd (const char *param); +int qemu_strtoi (const char *nptr,const char **endptr,int base, +int *result); +int qemu_strtoui (const char *nptr,const char **endptr,int base, +unsigned int *result);int qemu_strtol (const char *nptr,const char **endptr,int base,long *result); int qemu_strtoul(const char *nptr, const char **endptr, int base,
tests/test-cutils.c +657 -0
669 lines not on this page · raw message
util/cutils.c +108 -0
114 lines not on this page · raw message
Signature · 1 line
2.14.3
PATCH v5 3/9 sockets: pull code for testing IP availability out of specific test
In reply to Daniel P. BerrangéFrom: "Daniel P. Berrange" <[email protected]>
The test-io-channel-socket.c file has some useful helper functions for checking if a specific IP protocol is available. Other tests need to perform similar kinds of checks to avoid running tests that will fail due to missing IP protocols.
--- tests/Makefile.include | 2 +- tests/socket-helpers.c | 104 +++++++++++++++++++++++++++++++++++++++++ tests/socket-helpers.h | 42 +++++++++++++++++ tests/test-io-channel-socket.c | 72 +--------------------------- 4 files changed, 149 insertions(+), 71 deletions(-) create mode 100644 tests/socket-helpers.c create mode 100644 tests/socket-helpers.h
tests/Makefile.include +1 -1
@@ -715,7 +715,7 @@ tests/test-crypto-tlssession$(EXESUF): tests/test-crypto-tlssession.o \ tests/crypto-tls-x509-helpers.o tests/pkix_asn1_tab.o $(test-crypto-obj-y) tests/test-io-task$(EXESUF): tests/test-io-task.o $(test-io-obj-y) tests/test-io-channel-socket$(EXESUF): tests/test-io-channel-socket.o \ - tests/io-channel-helpers.o $(test-io-obj-y) + tests/io-channel-helpers.o tests/socket-helpers.o $(test-io-obj-y) tests/tpm-crb-test$(EXESUF): tests/tpm-crb-test.o tests/tpm-emu.o $(test-io-obj-y) tests/tpm-tis-test$(EXESUF): tests/tpm-tis-test.o tests/tpm-emu.o $(test-io-obj-y) tests/test-io-channel-file$(EXESUF): tests/test-io-channel-file.o \
tests/socket-helpers.c new file +104 -0
104 lines not on this page · raw message
tests/socket-helpers.h new file +42 -0
42 lines not on this page · raw message
tests/test-io-channel-socket.c +2 -70
86 lines not on this page · raw message
Signature · 1 line
2.14.3
Re: [Qemu-devel] [PATCH v5 3/9] sockets: pull code for testing IP availability out of specific test
Quoted 129 lines · Daniel P. Berrangé
From: "Daniel P. Berrange" <[email protected]>
The
test-io-channel-socket.cfile has some useful helper functions for checking if a specific IP protocol is available. Other tests need to perform similar kinds of checks to avoid running tests that will fail due to missing IP protocols.Reviewed-by Marc-André Lureau <[email protected]>Signed-off-by Daniel P. Berrange <[email protected]>--- tests/Makefile.include | 2 +- tests/socket-helpers.c | 104 +++++++++++++++++++++++++++++++++++++++++ tests/socket-helpers.h | 42 +++++++++++++++++ tests/test-io-channel-socket.c | 72 +--------------------------- 4 files changed, 149 insertions(+), 71 deletions(-) create mode 100644 tests/socket-helpers.c create mode 100644 tests/socket-helpers.htests/Makefile.include +1
-1@@ -715,7 +715,7 @@ tests/test-crypto-tlssession$(EXESUF): tests/test-crypto-tlssession.o \ tests/crypto-tls-x509-helpers.o tests/pkix_asn1_tab.o $(test-crypto-obj-y) tests/test-io-task$(EXESUF): tests/test-io-task.o $(test-io-obj-y) tests/test-io-channel-socket$(EXESUF): tests/test-io-channel-socket.o \ - tests/io-channel-helpers.o $(test-io-obj-y) + tests/io-channel-helpers.o tests/socket-helpers.o $(test-io-obj-y) tests/tpm-crb-test$(EXESUF): tests/tpm-crb-test.o tests/tpm-emu.o $(test-io-obj-y) tests/tpm-tis-test$(EXESUF): tests/tpm-tis-test.o tests/tpm-emu.o $(test-io-obj-y) tests/test-io-channel-file$(EXESUF): tests/test-io-channel-file.o \tests/socket-helpers.c new file +92
-0@@ -0,0 +1,104 @@ +/* +* Helper functions for tests using sockets +* +* Copyright 2015-2018 Red Hat, Inc. +* +* This program is free software; you can redistribute it and/or +* modify it under the terms of the GNU General Public License as +* published by the Free Software Foundation; either version 2 or +* (at your option) version 3 of the License. +* +* This program is distributed in the hope that it will be useful, +* but WITHOUT ANY WARRANTY; without even the implied warranty of +* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +* GNU General Public License for more details. +* +* You should have received a copy of the GNU General Public License +* along with this program; if not, see <http://www.gnu.org/licenses/>. +* +*/ + +#include "qemu/osdep.h" +#include "qemu-common.h" +#include "qemu/sockets.h" +#include "socket-helpers.h" + +#ifndef AI_ADDRCONFIG +# define AI_ADDRCONFIG 0 +#endif +#ifndef EAI_ADDRFAMILY +# define EAI_ADDRFAMILY 0 +#endif + +int socket_can_bind (const char *hostname) +{ +int fd =-1 ; +struct addrinfo ai, *res =NULL ; +int rc; +int ret =-1 ; + +memset (&ai,0 ,sizeof (ai)); + ai.ai_flags =AI_CANONNAME |AI_ADDRCONFIG ; + ai.ai_family =AF_UNSPEC ; + ai.ai_socktype =SOCK_STREAM ; + +/* lookup */ + rc =getaddrinfo (hostname,NULL , &ai, &res); +if (rc !=0 ) { +if (rc ==EAI_ADDRFAMILY || + rc ==EAI_FAMILY ) { + errno =EADDRNOTAVAIL ; + goto done; + } + errno =EINVAL ; + goto cleanup; + } + + fd =qemu_socket (res->ai_family, res->ai_socktype, res->ai_protocol); +if (fd <0 ) { + goto cleanup; + } + +if (bind (fd, res->ai_addr, res->ai_addrlen) <0 ) { +if (errno ==EADDRNOTAVAIL ) { + goto done; + } + goto cleanup; + } + + done: + ret =0 ; + + cleanup: +if (fd !=-1 ) { +close (fd); + } +if (res) { +freeaddrinfo (res); + } +return ret; +} + + +int socket_check_protocol_support (bool *has_ipv4,bool *has_ipv6) +{ + *has_ipv4 = *has_ipv6 = false; + +if (socket_can_bind ("127.0.0.1" ) <0 ) { +if (errno !=EADDRNOTAVAIL ) { +return -1 ; + } + }else { + *has_ipv4 = true;
tests/socket-helpers.c line 102
Quoted 10 lines
tests/socket-helpers.c new file +10
-0+ } + +if (socket_can_bind ("::1" ) <0 ) { +if (errno !=EADDRNOTAVAIL ) { +return -1 ; + } + }else { + *has_ipv6 = true; + } +
Sigh, I should have kept the new code identical to the old code, rather than trying to improve it, as this is in fact broken. The socket_can_bind() is mistakenly returning '0' when EADDRNOTAVAIL is set, so we always set the has_ipv4|6 vars to true.
It needs this squashed in:
@@ -48,9 +48,9 @@ int socket_can_bind(const char *hostname)
if (rc == EAI_ADDRFAMILY ||
rc == EAI_FAMILY) {
errno = EADDRNOTAVAIL;
- goto done;
+ } else {
+ errno = EINVAL;
}
- errno = EINVAL;
goto cleanup;
}@@ -60,13 +60,9 @@ int socket_can_bind(const char *hostname)
}
if (bind(fd, res->ai_addr, res->ai_addrlen) < 0) {
- if (errno == EADDRNOTAVAIL) {
- goto done;
- }
goto cleanup;
}- done:
ret = 0;
cleanup:
Regards,
Daniel
Signature · 3 lines
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
Re: [Qemu-devel] [PATCH v5 3/9] sockets: pull code for testing IP availability out of specific test
Quoted 21 lines · Daniel P. Berrangé
Quoted 11 lines · Daniel P. Berrangé
From: "Daniel P. Berrange" <[email protected]>
The
test-io-channel-socket.cfile has some useful helper functions for checking if a specific IP protocol is available. Other tests need to perform similar kinds of checks to avoid running tests that will fail due to missing IP protocols.Reviewed-by Marc-André Lureau <[email protected]>Signed-off-by Daniel P. Berrange <[email protected]>---
tests/socket-helpers.c line 102
Quoted 8 lines
tests/socket-helpers.c new file +8
-0+if (socket_can_bind ("::1" ) <0 ) { +if (errno !=EADDRNOTAVAIL ) { +return -1 ; + } + }else { + *has_ipv6 = true; + } +
Quoted 6 lines
Sigh, I should have kept the new code identical to the old code, rather than trying to improve it, as this is in fact broken. The
socket_can_bind()is mistakenly returning '0' when EADDRNOTAVAIL is set, so we always set the has_ipv4|6 vars to true.It needs this squashed in:
The squash makes sense; with that, you can keep the R-b I added on the series (I guess that shows I only read the code, not tried to run the testsuite with the code applied, or I might have found this too).
Signature · 3 lines
Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
Re: [Qemu-devel] [PATCH v5 3/9] sockets: pull code for testing IP availability out of specific test
Quoted 27 lines · Eric Blake
Quoted 26 lines · Daniel P. Berrangé
Quoted 11 lines · Daniel P. Berrangé
From: "Daniel P. Berrange" <[email protected]>
The
test-io-channel-socket.cfile has some useful helper functions for checking if a specific IP protocol is available. Other tests need to perform similar kinds of checks to avoid running tests that will fail due to missing IP protocols.Reviewed-by Marc-André Lureau <[email protected]>Signed-off-by Daniel P. Berrange <[email protected]>---
tests/socket-helpers.c line 102
Quoted 8 lines
tests/socket-helpers.c new file +8
-0+if (socket_can_bind ("::1" ) <0 ) { +if (errno !=EADDRNOTAVAIL ) { +return -1 ; + } + }else { + *has_ipv6 = true; + } +Sigh, I should have kept the new code identical to the old code, rather than trying to improve it, as this is in fact broken. The
socket_can_bind()is mistakenly returning '0' when EADDRNOTAVAIL is set, so we always set the has_ipv4|6 vars to true.
Quoted 5 lines
Quoted 1 line
It needs this squashed in:
The squash makes sense; with that, you can keep the R-b I added on the series (I guess that shows I only read the code, not tried to run the testsuite with the code applied, or I might have found this too).
Even if you had run the testsuite (like I did), you would not have seen the bug unless your host has IPv6 disabled *entirely*. Fortunately Travis hits that scenario which is how I noticed when I sent my branch through Travis prior to generating a pull request :-)
Regards,
Daniel
Signature · 3 lines
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
PATCH v5 4/9 sockets: strengthen test suite IP protocol availability checks
In reply to Daniel P. BerrangéFrom: "Daniel P. Berrange" <[email protected]>
Instead of just checking whether it is possible to bind() on a socket, also check that we can successfully connect() to the socket we bound to. This more closely replicates the level of functionality that tests will actually use.
--- tests/socket-helpers.c | 67 +++++++++++++++++++++++++++++++++++++++++++------- tests/socket-helpers.h | 4 +-- 2 files changed, 60 insertions(+), 11 deletions(-)
tests/socket-helpers.c +58 -9
@@ -30,10 +30,15 @@# define EAI_ADDRFAMILY 0#endif -int socket_can_bind (const char *hostname) +int socket_can_bind_connect (const char *hostname) { -int fd =-1 ; +int lfd =-1 , cfd =-1 , afd =-1 ;struct addrinfo ai, *res =NULL ; +struct sockaddr_storage ss; +socklen_t sslen =sizeof (ss); +int soerr; +socklen_t soerrlen =sizeof (soerr); +bool check_soerr = false;int rc;int ret =-1 ;
@@ -54,24 +59,68 @@ int socket_can_bind(const char *hostname) goto cleanup; } - fd =qemu_socket (res->ai_family, res->ai_socktype, res->ai_protocol); -if (fd <0 ) { + lfd =qemu_socket (res->ai_family, res->ai_socktype, res->ai_protocol); +if (lfd <0 ) { goto cleanup; } -if (bind (fd, res->ai_addr, res->ai_addrlen) <0 ) { + cfd =qemu_socket (res->ai_family, res->ai_socktype, res->ai_protocol); +if (cfd <0 ) { + goto cleanup; + } + +if (bind (lfd, res->ai_addr, res->ai_addrlen) <0 ) {if (errno ==EADDRNOTAVAIL ) { goto done; } goto cleanup; } +if (listen (lfd,1 ) <0 ) { + goto cleanup; + } + +if (getsockname (lfd, (struct sockaddr *)&ss, &sslen) <0 ) { + goto cleanup; + } + +qemu_set_nonblock (cfd); +if (connect (cfd, (struct sockaddr *)&ss, sslen) <0 ) { +if (errno ==EINPROGRESS ) { + check_soerr = true; + }else { + goto cleanup; + } + } + + sslen =sizeof (ss); + afd =accept (lfd, (struct sockaddr *)&ss, &sslen); +if (afd <0 ) { + goto cleanup; + } + +if (check_soerr) { +if (qemu_getsockopt (cfd,SOL_SOCKET ,SO_ERROR , &soerr, &soerrlen) <0 ) { + goto cleanup; + } +if (soerr) { + errno = soerr; + goto cleanup; + } + } + done: ret =0 ; cleanup: -if (fd !=-1 ) { -close (fd); +if (afd !=-1 ) { +close (afd); + } +if (cfd !=-1 ) { +close (cfd); + } +if (lfd !=-1 ) { +close (lfd); }if (res) {freeaddrinfo (res);
@@ -84,7 +133,7 @@ int socket_check_protocol_support(bool *has_ipv4, bool *has_ipv6) { *has_ipv4 = *has_ipv6 = false; -if (socket_can_bind ("127.0.0.1" ) <0 ) { +if (socket_can_bind_connect ("127.0.0.1" ) <0 ) {if (errno !=EADDRNOTAVAIL ) {return -1 ; }
@@ -92,7 +141,7 @@ int socket_check_protocol_support(bool *has_ipv4, bool *has_ipv6) *has_ipv4 = true; } -if (socket_can_bind ("::1" ) <0 ) { +if (socket_can_bind_connect ("::1" ) <0 ) {if (errno !=EADDRNOTAVAIL ) {return -1 ; }
tests/socket-helpers.h +2 -2
@@ -21,13 +21,13 @@/* * @hostname: a DNS name or numeric IP address * -* Check whether it is possible to bind to ports +* Check whether it is possible to bind & connect to ports * on the DNS name or IP address @hostname. If an IP address * is used, it must not be a wildcard address. * * Returns 0 on success, -1 on error with errno set */ -int socket_can_bind (const char *hostname); +int socket_can_bind_connect (const char *hostname);/* * @has_ipv4: set to true on return if IPv4 is available
Signature · 1 line
2.14.3
PATCH v5 5/9 sockets: move fd_is_socket() into common sockets code
In reply to Daniel P. BerrangéFrom: "Daniel P. Berrange" <[email protected]>
The fd_is_socket() helper method is useful in a few places, so put it in the common sockets code. Make the code more compact while moving it.
--- include/qemu/sockets.h | 1 + io/channel-util.c | 13 -------- tests/.gitignore | 1 + tests/Makefile.include | 3 ++ tests/test-util-sockets.c | 77 +++++++++++++++++++++++++++++++++++++++++++++++ util/qemu-sockets.c | 8 +++++ 6 files changed, 90 insertions(+), 13 deletions(-) create mode 100644 tests/test-util-sockets.c
include/qemu/sockets.h +1 -0
@@ -12,6 +12,7 @@ int inet_aton(const char *cp, struct in_addr *ia);#include "qapi/qapi-types-sockets.h" /* misc helpers */ +bool fd_is_socket (int fd);int qemu_socket (int domain,int type,int protocol);int qemu_accept (int s,struct sockaddr *addr,socklen_t *addrlen);int socket_set_cork (int fd,int v);
io/channel-util.c +0 -13
@@ -24,19 +24,6 @@#include "io/channel-socket.h" -static bool fd_is_socket (int fd) -{ -int optval; -socklen_t optlen; - optlen =sizeof (optval); -return qemu_getsockopt (fd, -SOL_SOCKET , -SO_TYPE , - (char *)&optval, - &optlen) ==0 ; -} - -QIOChannel *qio_channel_new_fd (int fd,Error **errp) {
tests/.gitignore +1 -0
@@ -86,6 +86,7 @@ test-thread-pool test-throttle test-timed-average test-uuid +test-util-sockets test-visitor-serialization test-vmstate test-write-threshold
tests/Makefile.include +3 -0
@@ -146,6 +146,7 @@ ifneq (,$(findstring qemu-ga,$(TOOLS))) check-unit-$(CONFIG_LINUX) += tests/test-qga$(EXESUF) endif check-unit-y += tests/test-timed-average$(EXESUF) +check-unit-y += tests/test-util-sockets$(EXESUF) check-unit-y += tests/test-io-task$(EXESUF) check-unit-y += tests/test-io-channel-socket$(EXESUF) check-unit-y += tests/test-io-channel-file$(EXESUF)
@@ -713,6 +714,8 @@ tests/test-crypto-tlscredsx509$(EXESUF): tests/test-crypto-tlscredsx509.o \ tests/test-crypto-tlssession.o-cflags := $(TASN1_CFLAGS) tests/test-crypto-tlssession$(EXESUF): tests/test-crypto-tlssession.o \ tests/crypto-tls-x509-helpers.o tests/pkix_asn1_tab.o $(test-crypto-obj-y) +tests/test-util-sockets$(EXESUF): tests/test-util-sockets.o \ + tests/socket-helpers.o $(test-util-obj-y) tests/test-io-task$(EXESUF): tests/test-io-task.o $(test-io-obj-y) tests/test-io-channel-socket$(EXESUF): tests/test-io-channel-socket.o \ tests/io-channel-helpers.o tests/socket-helpers.o $(test-io-obj-y)
tests/test-util-sockets.c new file +77 -0
77 lines not on this page · raw message
util/qemu-sockets.c +8 -0
@@ -91,6 +91,14 @@ NetworkAddressFamily inet_netfamily(int family)return NETWORK_ADDRESS_FAMILY_UNKNOWN ; } +bool fd_is_socket (int fd) +{ +int optval; +socklen_t optlen =sizeof (optval); +return !qemu_getsockopt (fd,SOL_SOCKET ,SO_TYPE , &optval, &optlen); +} + +/* * Matrix we're trying to apply *
Signature · 1 line
2.14.3
PATCH v5 6/9 sockets: check that the named file descriptor is a socket
In reply to Daniel P. BerrangéFrom: "Daniel P. Berrange" <[email protected]>
The SocketAddress struct has an "fd" type, which references the name of a file descriptor passed over the monitor using the "getfd" command. We currently blindly assume the FD is a socket, which can lead to hard to diagnose errors later. This adds an explicit check that the FD is actually a socket to improve the error diagnosis.
--- tests/test-util-sockets.c | 89 +++++++++++++++++++++++++++++++++++++++++++++++ util/qemu-sockets.c | 18 ++++++++-- 2 files changed, 105 insertions(+), 2 deletions(-)
tests/test-util-sockets.c +89 -0
@@ -23,6 +23,7 @@#include "qemu/sockets.h" #include "qapi/error.h" #include "socket-helpers.h" +#include "monitor/monitor.h" static void test_fd_is_socket_bad (void ) {
@@ -49,6 +50,90 @@ static void test_fd_is_socket_good(void)close (fd); } +static int mon_fd =-1 ; +static const char *mon_fdname; + +int monitor_get_fd (Monitor *mon,const char *fdname,Error **errp) +{ +g_assert (cur_mon); +g_assert (mon == cur_mon); +if (mon_fd ==-1 || !g_str_equal (mon_fdname, fdname)) { +error_setg (errp,"No fd named %s" , fdname); +return -1 ; + } +return dup (mon_fd); +} + +/* Syms in libqemustub.a are discarded at .o file granularity. +* To replace monitor_get_fd() we must ensure everything in +* stubs/monitor.c is defined, to make sure monitor.o is discarded +* otherwise we get duplicate syms at link time. +*/ +Monitor *cur_mon; +void monitor_init (Chardev *chr,int flags) {} + + +static void test_socket_fd_pass_good (void ) +{ +SocketAddress addr; +int fd; + + cur_mon =g_malloc (1 );/* Fake a monitor */ + mon_fdname ="myfd" ; + mon_fd =qemu_socket (AF_INET ,SOCK_STREAM ,0 ); + g_assert_cmpint(mon_fd, >, STDERR_FILENO); + + addr.type =SOCKET_ADDRESS_TYPE_FD ; + addr.u.fd.str =g_strdup (mon_fdname); + + fd =socket_connect (&addr, &error_abort); + g_assert_cmpint(fd, !=, -1); + g_assert_cmpint(fd, !=, mon_fd); +close (fd); + + fd =socket_listen (&addr, &error_abort); + g_assert_cmpint(fd, !=, -1); + g_assert_cmpint(fd, !=, mon_fd); +close (fd); + +g_free (addr.u.fd.str); + mon_fdname =NULL ; +close (mon_fd); + mon_fd =-1 ; +g_free (cur_mon); + cur_mon =NULL ; +} + +static void test_socket_fd_pass_bad (void ) +{ +SocketAddress addr; +Error *err =NULL ; +int fd; + + cur_mon =g_malloc (1 );/* Fake a monitor */ + mon_fdname ="myfd" ; + mon_fd =dup (STDOUT_FILENO ); + g_assert_cmpint(mon_fd, >, STDERR_FILENO); + + addr.type =SOCKET_ADDRESS_TYPE_FD ; + addr.u.fd.str =g_strdup (mon_fdname); + + fd =socket_connect (&addr, &err); + g_assert_cmpint(fd, ==, -1); +error_free_or_abort (&err); + + fd =socket_listen (&addr, &err); + g_assert_cmpint(fd, ==, -1); +error_free_or_abort (&err); + +g_free (addr.u.fd.str); + mon_fdname =NULL ; +close (mon_fd); + mon_fd =-1 ; +g_free (cur_mon); + cur_mon =NULL ; +} +int main (int argc,char **argv) {bool has_ipv4, has_ipv6;
@@ -71,6 +156,10 @@ int main(int argc, char **argv) test_fd_is_socket_bad);g_test_add_func ("/util/socket/is-socket/good" , test_fd_is_socket_good); +g_test_add_func ("/socket/fd-pass/good" , + test_socket_fd_pass_good); +g_test_add_func ("/socket/fd-pass/bad" , + test_socket_fd_pass_bad); }return g_test_run ();
util/qemu-sockets.c +16 -2
@@ -1042,6 +1042,20 @@ fail:return NULL ; } +static int socket_get_fd (const char *fdstr,Error **errp) +{ +int fd =monitor_get_fd (cur_mon, fdstr, errp); +if (fd <0 ) { +return -1 ; + } +if (!fd_is_socket (fd)) { +error_setg (errp,"File descriptor '%s' is not a socket" , fdstr); +close (fd); +return -1 ; + } +return fd; +} +int socket_connect (SocketAddress *addr,Error **errp) {int fd;
@@ -1056,7 +1070,7 @@ int socket_connect(SocketAddress *addr, Error **errp)break ;case SOCKET_ADDRESS_TYPE_FD : - fd =monitor_get_fd (cur_mon, addr->u.fd.str, errp); + fd =socket_get_fd (addr->u.fd.str, errp);break ;case SOCKET_ADDRESS_TYPE_VSOCK :
@@ -1083,7 +1097,7 @@ int socket_listen(SocketAddress *addr, Error **errp)break ;case SOCKET_ADDRESS_TYPE_FD : - fd =monitor_get_fd (cur_mon, addr->u.fd.str, errp); + fd =socket_get_fd (addr->u.fd.str, errp);break ;case SOCKET_ADDRESS_TYPE_VSOCK :
Signature · 1 line
2.14.3
PATCH v5 7/9 sockets: allow SocketAddress 'fd' to reference numeric file descriptors
In reply to Daniel P. BerrangéFrom: "Daniel P. Berrange" <[email protected]>
The SocketAddress 'fd' kind accepts the name of a file descriptor passed to the monitor with the 'getfd' command. This makes it impossible to use the 'fd' kind in cases where a monitor is not available. This can apply in handling command line argv at startup, or simply if internal code wants to use SocketAddress and pass a numeric FD it has acquired from elsewhere.
Fortunately the 'getfd' command mandated that the FD names must not start with a leading digit. We can thus safely extend semantics of the SocketAddress 'fd' kind, to allow a purely numeric name to reference an file descriptor that QEMU already has open. There will be restrictions on when each kind can be used.
In codepaths where we are handling a monitor command (ie cur_mon != NULL), we will only support use of named file descriptors as before. Use of FD numbers is still not permitted for monitor commands.
In codepaths where we are not handling a monitor command (ie cur_mon == NULL), we will not support named file descriptors. Instead we can reference FD numers explicitly. This allows the app spawning QEMU to intentionally "leak" a pre-opened socket to QEMU and reference that in a SocketAddress definition, or for code inside QEMU to pass pre-opened FDs around.
--- qapi/sockets.json | 7 +++ tests/test-util-sockets.c | 112 +++++++++++++++++++++++++++++++++++++++++++--- util/qemu-sockets.c | 16 +++++-- 3 files changed, 126 insertions(+), 9 deletions(-)
qapi/sockets.json +7 -0
@@ -123,6 +123,13 @@ # # @unix: Unix domain socket # +# @vsock: VMCI address +# +# @fd: decimal is for file descriptor number, otherwise a file descriptor name. +# Named file descriptors are permitted in monitor commands, in combination +# with the 'getfd' command. Decimal file descriptors are permitted at +# startup or other contexts where no monitor context is active. +# # Since: 2.9 ## { 'enum': 'SocketAddressType',
tests/test-util-sockets.c +106 -6
136 lines not on this page · raw message
util/qemu-sockets.c +13 -3
@@ -1044,9 +1044,19 @@ fail:static int socket_get_fd (const char *fdstr,Error **errp) { -int fd =monitor_get_fd (cur_mon, fdstr, errp); -if (fd <0 ) { -return -1 ; +int fd; +if (cur_mon) { + fd =monitor_get_fd (cur_mon, fdstr, errp); +if (fd <0 ) { +return -1 ; + } + }else { +if (qemu_strtoi (fdstr,NULL ,10 , &fd) <0 ) { +error_setg_errno (errp, errno, +"Unable to parse FD number %s" , + fdstr); +return -1 ; + } }if (!fd_is_socket (fd)) {error_setg (errp,"File descriptor '%s' is not a socket" , fdstr);
Signature · 1 line
2.14.3
PATCH v5 8/9 char: refactor parsing of socket address information
In reply to Daniel P. BerrangéFrom: "Daniel P. Berrange" <[email protected]>
To prepare for handling more address types, refactor the parsing of socket address information to make it more robust and extensible.
--- chardev/char-socket.c | 22 ++++++++++++++-------- 1 file changed, 14 insertions(+), 8 deletions(-)
chardev/char-socket.c +14 -8
@@ -1012,21 +1012,25 @@ static void qemu_chr_parse_socket(QemuOpts *opts, ChardevBackend *backend,SocketAddressLegacy *addr;ChardevSocket *sock; +if ((!!path + !!host) !=1 ) { +error_setg (errp, +"Exactly one of 'path' or 'host' required" ); +return ; + } + backend->type =CHARDEV_BACKEND_KIND_SOCKET ; -if (!path) { -if (!host) { -error_setg (errp,"chardev: socket: no host given" ); +if (path) { +if (tls_creds) { +error_setg (errp,"TLS can only be used over TCP socket" );return ; } + }else if (host) {if (!port) {error_setg (errp,"chardev: socket: no port given" );return ; } }else { -if (tls_creds) { -error_setg (errp,"TLS can only be used over TCP socket" ); -return ; - } +g_assert_not_reached (); } sock = backend->u.socket.data =g_new0 (ChardevSocket,1 );
@@ -1052,7 +1056,7 @@ static void qemu_chr_parse_socket(QemuOpts *opts, ChardevBackend *backend, addr->type =SOCKET_ADDRESS_LEGACY_KIND_UNIX ; q_unix = addr->u.q_unix.data =g_new0 (UnixSocketAddress,1 ); q_unix->path =g_strdup (path); - }else { + } else if (host) { addr->type =SOCKET_ADDRESS_LEGACY_KIND_INET ; addr->u.inet.data =g_new (InetSocketAddress,1 ); *addr->u.inet.data = (InetSocketAddress ) {
@@ -1065,6 +1069,8 @@ static void qemu_chr_parse_socket(QemuOpts *opts, ChardevBackend *backend, .has_ipv6 = qemu_opt_get(opts, "ipv6"), .ipv6 = qemu_opt_get_bool(opts, "ipv6", 0), }; + }else { +g_assert_not_reached (); } sock->addr = addr; }
Signature · 1 line
2.14.3
PATCH v5 9/9 char: allow passing pre-opened socket file descriptor at startup
In reply to Daniel P. BerrangéFrom: "Daniel P. Berrange" <[email protected]>
When starting QEMU management apps will usually setup a monitor socket, and then open it immediately after startup. If not using QEMU's own -daemonize arg, this process can be troublesome to handle correctly. The mgmt app will need to repeatedly call connect() until it succeeds, because it does not know when QEMU has created the listener socket. If can't retry connect() forever though, because an error might have caused QEMU to exit before it even creates the monitor.
The obvious way to fix this kind of problem is to just pass in a pre-opened socket file descriptor for the QEMU monitor to listen on. The management app can now immediately call connect() just once. If connect() fails it knows that QEMU has exited with an error.
The SocketAddress(Legacy) structs allow for FD passing via the monitor, and now via inherited file descriptors from the process that spawned QEMU. The final missing piece is adding a 'fd' parameter in the socket chardev options.
This allows both HMP usage, pass any FD number with SCM_RIGHTS, then running HMP commands:
getfd myfd
chardev-add socket,fd=myfd
Note that numeric FDs cannot be referenced directly in HMP, only named FDs.
And also CLI usage, by leak FD 3 from parent by clearing O_CLOEXEC, then spawning QEMU with
-chardev socket,fd=3,id=mon
-mon chardev=mon,mode=control
Note that named FDs cannot be referenced in CLI args, only numeric FDs.
We do not wire this up in the legacy chardev syntax, so you cannot use FD passing with '-qmp', you must use the modern '-mon' + '-chardev' pair.
When passing pre-opened FDs there is a restriction on use of TLS encryption. It can be used on a server socket chardev, but cannot be used for a client socket chardev. This is because when validating a server's certificate, the client needs to have a hostname available to match against the certificate identity.
An illustrative example of usage is:
#!/usr/bin/perl
use IO::Socket::UNIX;
use Fcntl;
unlink "/tmp/qmp";
my $srv = IO::Socket::UNIX->new(
Type => SOCK_STREAM(),
Local => "/tmp/qmp",
Listen => 1,);
my $flags = fcntl $srv, F_GETFD, 0;
fcntl $srv, F_SETFD, $flags & ~FD_CLOEXEC;
my $fd = $srv->fileno();
exec "qemu-system-x86_64", \
"-chardev", "socket,fd=$fd,server,nowait,id=mon", \
"-mon", "chardev=mon,mode=control";--- chardev/char-socket.c | 15 +++++++++++++-- chardev/char.c | 3 +++ tests/test-char.c | 47 ++++++++++++++++++++++++++++++++++++++++++++--- 3 files changed, 60 insertions(+), 5 deletions(-)
chardev/char-socket.c +13 -2
@@ -1008,13 +1008,14 @@ static void qemu_chr_parse_socket(QemuOpts *opts, ChardevBackend *backend,const char *path =qemu_opt_get (opts,"path" );const char *host =qemu_opt_get (opts,"host" );const char *port =qemu_opt_get (opts,"port" ); +const char *fd =qemu_opt_get (opts,"fd" );const char *tls_creds =qemu_opt_get (opts,"tls-creds" );SocketAddressLegacy *addr;ChardevSocket *sock; -if ((!!path + !!host) !=1 ) { +if ((!!path + !!fd + !!host) !=1 ) {error_setg (errp, -"Exactly one of 'path' or 'host' required" ); +"Exactly one of 'path', 'fd' or 'host' required" );return ; }
@@ -1029,6 +1030,12 @@ static void qemu_chr_parse_socket(QemuOpts *opts, ChardevBackend *backend,error_setg (errp,"chardev: socket: no port given" );return ; } + }else if (fd) { +/* We don't know what host to validate against when in client mode */ +if (tls_creds && !is_listen) { +error_setg (errp,"TLS can not be used with pre-opened client FD" ); +return ; + } }else {g_assert_not_reached (); }
@@ -1069,6 +1076,10 @@ static void qemu_chr_parse_socket(QemuOpts *opts, ChardevBackend *backend, .has_ipv6 = qemu_opt_get(opts, "ipv6"), .ipv6 = qemu_opt_get_bool(opts, "ipv6", 0), }; + }else if (fd) { + addr->type =SOCKET_ADDRESS_LEGACY_KIND_FD ; + addr->u.fd.data =g_new (String,1 ); + addr->u.fd.data->str =g_strdup (fd); }else {g_assert_not_reached (); }
chardev/char.c +3 -0
@@ -807,6 +807,9 @@ QemuOptsList qemu_chardev_opts = { },{ .name = "port", .type = QEMU_OPT_STRING, + },{ + .name = "fd", + .type = QEMU_OPT_STRING, },{ .name = "localaddr", .type = QEMU_OPT_STRING,
tests/test-char.c +44 -3
66 lines not on this page · raw message
Signature · 1 line
2.14.3
Quoted 24 lines · Daniel P. Berrangé
An update of:
v1: https://lists.gnu.org/archive/html/qemu-devel/2017-12/msg04618.html v2: https://lists.gnu.org/archive/html/qemu-devel/2017-12/msg04706.html v3: https://lists.gnu.org/archive/html/qemu-devel/2017-12/msg04892.html v4: https://lists.gnu.org/archive/html/qemu-devel/2018-02/msg00950.htmlThis enables fixing a long standing problem that libvirt has with starting up QEMU. It has to busy-wait retrying
connect()on the QMP monitor socket until QEMU finally creates & listens on it, but at same time must be careful to not wait forever if QEMU exits.With this patch series, libvirt can simply pass in a pre-opened UNIX domain socket file descriptor, which it can immediately connect to with no busy-wait.
NB, this will generate one expected failure with patchew / checkpatch.pl
Changed in v5:
- Docs typo (Eric)
- Fix errno setting in int parsing code (Eric)
Quoted 12 lines
Daniel P. Berrangé (9):
char: don't silently skip tn3270 protocol init when TLS is enabled
cutils: addqemu_strtoi&qemu_strtouiparsers for int/unsigned int
types
sockets: pull code for testing IP availability out of specific test
sockets: strengthen test suite IP protocol availability checks
sockets: movefd_is_socket()into common sockets code
sockets: check that the named file descriptor is a socket
sockets: allow SocketAddress 'fd' to reference numeric file
descriptors
char: refactor parsing of socket address information
char: allow passing pre-opened socket file descriptor at startup
Whose tree should this go through? It's got some QAPI impact, so I'm okay including it in my QAPI pull request later today, if there is no better tree...
Signature · 3 lines
Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
Quoted 38 lines · Eric Blake
Quoted 24 lines · Daniel P. Berrangé
An update of:
v1: https://lists.gnu.org/archive/html/qemu-devel/2017-12/msg04618.html v2: https://lists.gnu.org/archive/html/qemu-devel/2017-12/msg04706.html v3: https://lists.gnu.org/archive/html/qemu-devel/2017-12/msg04892.html v4: https://lists.gnu.org/archive/html/qemu-devel/2018-02/msg00950.htmlThis enables fixing a long standing problem that libvirt has with starting up QEMU. It has to busy-wait retrying
connect()on the QMP monitor socket until QEMU finally creates & listens on it, but at same time must be careful to not wait forever if QEMU exits.With this patch series, libvirt can simply pass in a pre-opened UNIX domain socket file descriptor, which it can immediately connect to with no busy-wait.
NB, this will generate one expected failure with patchew / checkpatch.pl
Changed in v5:
- Docs typo (Eric)
- Fix errno setting in int parsing code (Eric)Quoted 12 lines
Daniel P. Berrangé (9):
char: don't silently skip tn3270 protocol init when TLS is enabled
cutils: addqemu_strtoi&qemu_strtouiparsers for int/unsigned int
types
sockets: pull code for testing IP availability out of specific test
sockets: strengthen test suite IP protocol availability checks
sockets: movefd_is_socket()into common sockets code
sockets: check that the named file descriptor is a socket
sockets: allow SocketAddress 'fd' to reference numeric file
descriptors
char: refactor parsing of socket address information
char: allow passing pre-opened socket file descriptor at startup
Quoted 3 lines
Whose tree should this go through? It's got some QAPI impact, so I'm okay including it in my QAPI pull request later today, if there is no better tree...
These days I usually send PR for stuff touching sockets code myself. So unless someone else strongly prefers to take it via their tree for sake of any easy of conflict / merge resolution, I can do a PR once acked.
Regards,
Daniel
Signature · 3 lines
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
Quoted 18 lines · Daniel P. Berrangé
Quoted 16 lines · Eric Blake
Quoted 15 lines · Daniel P. Berrangé
An update of:
v1: https://lists.gnu.org/archive/html/qemu-devel/2017-12/msg04618.html v2: https://lists.gnu.org/archive/html/qemu-devel/2017-12/msg04706.html v3: https://lists.gnu.org/archive/html/qemu-devel/2017-12/msg04892.html v4: https://lists.gnu.org/archive/html/qemu-devel/2018-02/msg00950.htmlThis enables fixing a long standing problem that libvirt has with starting up QEMU. It has to busy-wait retrying
connect()on the QMP monitor socket until QEMU finally creates & listens on it, but at same time must be careful to not wait forever if QEMU exits.With this patch series, libvirt can simply pass in a pre-opened UNIX domain socket file descriptor, which it can immediately connect to with no busy-wait.
Quoted 7 lines
Quoted 3 lines
Whose tree should this go through? It's got some QAPI impact, so I'm okay including it in my QAPI pull request later today, if there is no better tree...
These days I usually send PR for stuff touching sockets code myself. So unless someone else strongly prefers to take it via their tree for sake of any easy of conflict / merge resolution, I can do a PR once acked.
Okay, I'll go through one more round of reviews, then let you send the PR; we still have time to make softfreeze tomorrow.
Signature · 3 lines
Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
Quoted 25 lines · Daniel P. Berrangé
An update of:
v1: https://lists.gnu.org/archive/html/qemu-devel/2017-12/msg04618.html v2: https://lists.gnu.org/archive/html/qemu-devel/2017-12/msg04706.html v3: https://lists.gnu.org/archive/html/qemu-devel/2017-12/msg04892.html v4: https://lists.gnu.org/archive/html/qemu-devel/2018-02/msg00950.htmlThis enables fixing a long standing problem that libvirt has with starting up QEMU. It has to busy-wait retrying
connect()on the QMP monitor socket until QEMU finally creates & listens on it, but at same time must be careful to not wait forever if QEMU exits.With this patch series, libvirt can simply pass in a pre-opened UNIX domain socket file descriptor, which it can immediately connect to with no busy-wait.
NB, this will generate one expected failure with patchew / checkpatch.pl
ERROR: consider using
qemu_strtolin preference to strtol #729: FILE:util/cutils.c:338:
+ lresult =strtol(nptr, &ep, base);ERROR: consider using
qemu_strtolin preference to strtol #779: FILE:util/cutils.c:388:
+ lresult =strtol(nptr, &ep, base);
Quoted 7 lines
This is ok to ignore, because the patch in question is introducing new
qemu_strtoXXXwrappersChanged in v5:
- Docs typo (Eric)
- Fix errno setting in int parsing code (Eric)
Series:
Signature · 3 lines
Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
This series seems to have some coding style problems. See output below for more information:
Hi,
This series seems to have some coding style problems. See output below for more information:
Type: series
Message-id: [email protected]
Subject: [Qemu-devel] [PATCH v5 0/9] Enable passing pre-opened chardev socket FD
=== TEST SCRIPT BEGIN ===
#!/bin/bash
BASE=base
n=1
total=$(git log --oneline $BASE.. | wc -l)
failed=0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
commits="$(git log --format=%H --reverse $BASE..)" for c in $commits; do
echo "Checking PATCH $n/$total: $(git log -n 1 --format=%s $c)..."
if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; then
failed=1
echo
fi
n=$((n+1))done
exit $failed
=== TEST SCRIPT END ===
Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
Switched to a new branch 'test'11a2d24854 char: allow passing pre-opened socket file descriptor at startup 38d7ed855a char: refactor parsing of socket address information dacc6f8ebe sockets: allow SocketAddress 'fd' to reference numeric file descriptors 8d91d776c7 sockets: check that the named file descriptor is a socket 4ac9d04d7b sockets: move fd_is_socket() into common sockets code 179118052a sockets: strengthen test suite IP protocol availability checks 357edea8d3 sockets: pull code for testing IP availability out of specific test 7fca025ea8 cutils: add qemu_strtoi & qemu_strtoui parsers for int/unsigned int types 68d51518fb char: don't silently skip tn3270 protocol init when TLS is enabled
=== OUTPUT BEGIN ===
Checking PATCH 1/9: char: don't silently skip tn3270 protocol init when TLS is enabled... Checking PATCH 2/9: cutils: add qemu_strtoi & qemu_strtoui parsers for int/unsigned int types...
ERROR: consider using qemu_strtol in preference to strtol
#753: FILE: util/cutils.c:338:
+ lresult = strtol(nptr, &ep, base);
ERROR: consider using qemu_strtoul in preference to strtoul #805: FILE: util/cutils.c:390:
+ lresult = strtoul(nptr, &ep, base);
total: 2 errors, 0 warnings, 793 lines checked
Your patch has style problems, please review. If any of these errors are false positives report them to the maintainer, see CHECKPATCH in MAINTAINERS.
Checking PATCH 3/9: sockets: pull code for testing IP availability out of specific test... Checking PATCH 4/9: sockets: strengthen test suite IP protocol availability checks... Checking PATCH 5/9: sockets: move fd_is_socket() into common sockets code... Checking PATCH 6/9: sockets: check that the named file descriptor is a socket... Checking PATCH 7/9: sockets: allow SocketAddress 'fd' to reference numeric file descriptors... Checking PATCH 8/9: char: refactor parsing of socket address information... Checking PATCH 9/9: char: allow passing pre-opened socket file descriptor at startup... === OUTPUT END ===
Test command exited with code: 1
---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to [email protected]