[Qemu-devel] [PATCH v3 0/8] Enable passing pre-opened chardev socket FDs

[Qemu-devel] [PATCH v3 0/8] Enable passing pre-opened chardev socket FDs

[Daniel P. Berrange]

This fixes a long standing problem that libvirt has with starting up QEMU.

We have 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.

This 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.

Changed in v3:

  - Introduce  qemu_strtoi & qemu_stroui functions. NB, patchew and
    checkpatch.pl complain about this patch because it calls strtol,
    but that is valid in this case.

  - 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. Berrange (8):
  cutils: add qemu_strtoi & qemu_strtoui parsers for int/unsigned int
    types
  sockets: move fd_is_socket() into common sockets code
  sockets: pull code for testing IP availability out of specific test
  sockets: strengthen test suite IP protocol availability checks
  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          |  31 +-
 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            | 653 +++++++++++++++++++++++++++++++++++++++++
 tests/test-io-channel-socket.c |  72 +----
 tests/test-sockets.c           | 239 +++++++++++++++
 util/cutils.c                  | 104 +++++++
 util/qemu-sockets.c            |  36 ++-
 16 files changed, 1315 insertions(+), 96 deletions(-)
 create mode 100644 tests/socket-helpers.c
 create mode 100644 tests/socket-helpers.h
 create mode 100644 tests/test-sockets.c

-- 
2.14.3

[Qemu-devel] [PATCH v3 1/8] cutils: add qemu_strtoi & qemu_strtoui parsers for int/unsigned int types

[Daniel P. Berrange]

There are qemu_strtoNN functions for various sized integers. This adds two
more for plain int & unsigned int types, with suitable range checking.

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
---
 include/qemu/cutils.h |   4 +
 tests/test-cutils.c   | 653 ++++++++++++++++++++++++++++++++++++++++++++++++++
 util/cutils.c         | 104 ++++++++
 3 files changed, 761 insertions(+)

diff --git a/include/qemu/cutils.h b/include/qemu/cutils.h
index f0878eaafa..a663340b23 100644
--- a/include/qemu/cutils.h
+++ b/include/qemu/cutils.h
@@ -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,
diff --git a/tests/test-cutils.c b/tests/test-cutils.c
index f64a49b7fb..0b2b5d417b 100644
--- a/tests/test-cutils.c
+++ b/tests/test-cutils.c
@@ -223,6 +223,579 @@ static void test_parse_uint_full_correct(void)
     g_assert_cmpint(i, ==, 123);
 }
 
+static void test_qemu_strtoi_correct(void)
+{
+    const char *str = "12345 foo";
+    char f = 'X';
+    const char *endptr = &f;
+    int res = 999;
+    int err;
+
+    err = qemu_strtoi(str, &endptr, 0, &res);
+
+    g_assert_cmpint(err, ==, 0);
+    g_assert_cmpint(res, ==, 12345);
+    g_assert(endptr == str + 5);
+}
+
+static void test_qemu_strtoi_null(void)
+{
+    char f = 'X';
+    const char *endptr = &f;
+    int res = 999;
+    int err;
+
+    err = qemu_strtoi(NULL, &endptr, 0, &res);
+
+    g_assert_cmpint(err, ==, -EINVAL);
+    g_assert(endptr == NULL);
+}
+
+static void test_qemu_strtoi_empty(void)
+{
+    const char *str = "";
+    char f = 'X';
+    const char *endptr = &f;
+    int res = 999;
+    int err;
+
+    err = qemu_strtoi(str, &endptr, 0, &res);
+
+    g_assert_cmpint(err, ==, -EINVAL);
+    g_assert(endptr == str);
+}
+
+static void test_qemu_strtoi_whitespace(void)
+{
+    const char *str = "  \t  ";
+    char f = 'X';
+    const char *endptr = &f;
+    int res = 999;
+    int err;
+
+    err = qemu_strtoi(str, &endptr, 0, &res);
+
+    g_assert_cmpint(err, ==, -EINVAL);
+    g_assert(endptr == str);
+}
+
+static void test_qemu_strtoi_invalid(void)
+{
+    const char *str = "   xxxx  \t abc";
+    char f = 'X';
+    const char *endptr = &f;
+    int res = 999;
+    int err;
+
+    err = qemu_strtoi(str, &endptr, 0, &res);
+
+    g_assert_cmpint(err, ==, -EINVAL);
+    g_assert(endptr == str);
+}
+
+static void test_qemu_strtoi_trailing(void)
+{
+    const char *str = "123xxx";
+    char f = 'X';
+    const char *endptr = &f;
+    int res = 999;
+    int err;
+
+    err = qemu_strtoi(str, &endptr, 0, &res);
+
+    g_assert_cmpint(err, ==, 0);
+    g_assert_cmpint(res, ==, 123);
+    g_assert(endptr == str + 3);
+}
+
+static void test_qemu_strtoi_octal(void)
+{
+    const char *str = "0123";
+    char f = 'X';
+    const char *endptr = &f;
+    int res = 999;
+    int err;
+
+    err = qemu_strtoi(str, &endptr, 8, &res);
+
+    g_assert_cmpint(err, ==, 0);
+    g_assert_cmpint(res, ==, 0123);
+    g_assert(endptr == str + strlen(str));
+
+    res = 999;
+    endptr = &f;
+    err = qemu_strtoi(str, &endptr, 0, &res);
+
+    g_assert_cmpint(err, ==, 0);
+    g_assert_cmpint(res, ==, 0123);
+    g_assert(endptr == str + strlen(str));
+}
+
+static void test_qemu_strtoi_decimal(void)
+{
+    const char *str = "0123";
+    char f = 'X';
+    const char *endptr = &f;
+    int res = 999;
+    int err;
+
+    err = qemu_strtoi(str, &endptr, 10, &res);
+
+    g_assert_cmpint(err, ==, 0);
+    g_assert_cmpint(res, ==, 123);
+    g_assert(endptr == str + strlen(str));
+
+    str = "123";
+    res = 999;
+    endptr = &f;
+    err = qemu_strtoi(str, &endptr, 0, &res);
+
+    g_assert_cmpint(err, ==, 0);
+    g_assert_cmpint(res, ==, 123);
+    g_assert(endptr == str + strlen(str));
+}
+
+static void test_qemu_strtoi_hex(void)
+{
+    const char *str = "0123";
+    char f = 'X';
+    const char *endptr = &f;
+    int res = 999;
+    int err;
+
+    err = qemu_strtoi(str, &endptr, 16, &res);
+
+    g_assert_cmpint(err, ==, 0);
+    g_assert_cmpint(res, ==, 0x123);
+    g_assert(endptr == str + strlen(str));
+
+    str = "0x123";
+    res = 999;
+    endptr = &f;
+    err = qemu_strtoi(str, &endptr, 0, &res);
+
+    g_assert_cmpint(err, ==, 0);
+    g_assert_cmpint(res, ==, 0x123);
+    g_assert(endptr == str + strlen(str));
+}
+
+static void test_qemu_strtoi_max(void)
+{
+    char *str = g_strdup_printf("%d", INT_MAX);
+    char f = 'X';
+    const char *endptr = &f;
+    int res = 999;
+    int err;
+
+    err = qemu_strtoi(str, &endptr, 0, &res);
+
+    g_assert_cmpint(err, ==, 0);
+    g_assert_cmpint(res, ==, INT_MAX);
+    g_assert(endptr == str + strlen(str));
+    g_free(str);
+}
+
+static void test_qemu_strtoi_overflow(void)
+{
+    const char *str = "99999999999999999999999999999999999999999999";
+    char f = 'X';
+    const char *endptr = &f;
+    int res = 999;
+    int err;
+
+    err = qemu_strtoi(str, &endptr, 0, &res);
+
+    g_assert_cmpint(err, ==, -ERANGE);
+    g_assert_cmpint(res, ==, INT_MAX);
+    g_assert(endptr == str + strlen(str));
+}
+
+static void test_qemu_strtoi_underflow(void)
+{
+    const char *str = "-99999999999999999999999999999999999999999999";
+    char f = 'X';
+    const char *endptr = &f;
+    int res = 999;
+    int err;
+
+    err  = qemu_strtoi(str, &endptr, 0, &res);
+
+    g_assert_cmpint(err, ==, -ERANGE);
+    g_assert_cmpint(res, ==, INT_MIN);
+    g_assert(endptr == str + strlen(str));
+}
+
+static void test_qemu_strtoi_negative(void)
+{
+    const char *str = "  \t -321";
+    char f = 'X';
+    const char *endptr = &f;
+    int res = 999;
+    int err;
+
+    err = qemu_strtoi(str, &endptr, 0, &res);
+
+    g_assert_cmpint(err, ==, 0);
+    g_assert_cmpint(res, ==, -321);
+    g_assert(endptr == str + strlen(str));
+}
+
+static void test_qemu_strtoi_full_correct(void)
+{
+    const char *str = "123";
+    int res = 999;
+    int err;
+
+    err = qemu_strtoi(str, NULL, 0, &res);
+
+    g_assert_cmpint(err, ==, 0);
+    g_assert_cmpint(res, ==, 123);
+}
+
+static void test_qemu_strtoi_full_null(void)
+{
+    char f = 'X';
+    const char *endptr = &f;
+    int res = 999;
+    int err;
+
+    err = qemu_strtoi(NULL, &endptr, 0, &res);
+
+    g_assert_cmpint(err, ==, -EINVAL);
+    g_assert(endptr == NULL);
+}
+
+static void test_qemu_strtoi_full_empty(void)
+{
+    const char *str = "";
+    int res = 999L;
+    int err;
+
+    err =  qemu_strtoi(str, NULL, 0, &res);
+
+    g_assert_cmpint(err, ==, -EINVAL);
+}
+
+static void test_qemu_strtoi_full_negative(void)
+{
+    const char *str = " \t -321";
+    int res = 999;
+    int err;
+
+    err = qemu_strtoi(str, NULL, 0, &res);
+
+    g_assert_cmpint(err, ==, 0);
+    g_assert_cmpint(res, ==, -321);
+}
+
+static void test_qemu_strtoi_full_trailing(void)
+{
+    const char *str = "123xxx";
+    int res;
+    int err;
+
+    err = qemu_strtoi(str, NULL, 0, &res);
+
+    g_assert_cmpint(err, ==, -EINVAL);
+}
+
+static void test_qemu_strtoi_full_max(void)
+{
+    char *str = g_strdup_printf("%d", INT_MAX);
+    int res;
+    int err;
+
+    err = qemu_strtoi(str, NULL, 0, &res);
+
+    g_assert_cmpint(err, ==, 0);
+    g_assert_cmpint(res, ==, INT_MAX);
+    g_free(str);
+}
+
+static void test_qemu_strtoui_correct(void)
+{
+    const char *str = "12345 foo";
+    char f = 'X';
+    const char *endptr = &f;
+    unsigned int res = 999;
+    int err;
+
+    err = qemu_strtoui(str, &endptr, 0, &res);
+
+    g_assert_cmpint(err, ==, 0);
+    g_assert_cmpuint(res, ==, 12345);
+    g_assert(endptr == str + 5);
+}
+
+static void test_qemu_strtoui_null(void)
+{
+    char f = 'X';
+    const char *endptr = &f;
+    unsigned int res = 999;
+    int err;
+
+    err = qemu_strtoui(NULL, &endptr, 0, &res);
+
+    g_assert_cmpint(err, ==, -EINVAL);
+    g_assert(endptr == NULL);
+}
+
+static void test_qemu_strtoui_empty(void)
+{
+    const char *str = "";
+    char f = 'X';
+    const char *endptr = &f;
+    unsigned int res = 999;
+    int err;
+
+    err = qemu_strtoui(str, &endptr, 0, &res);
+
+    g_assert_cmpint(err, ==, -EINVAL);
+    g_assert(endptr == str);
+}
+
+static void test_qemu_strtoui_whitespace(void)
+{
+    const char *str = "  \t  ";
+    char f = 'X';
+    const char *endptr = &f;
+    unsigned int res = 999;
+    int err;
+
+    err = qemu_strtoui(str, &endptr, 0, &res);
+
+    g_assert_cmpint(err, ==, -EINVAL);
+    g_assert(endptr == str);
+}
+
+static void test_qemu_strtoui_invalid(void)
+{
+    const char *str = "   xxxx  \t abc";
+    char f = 'X';
+    const char *endptr = &f;
+    unsigned int res = 999;
+    int err;
+
+    err = qemu_strtoui(str, &endptr, 0, &res);
+
+    g_assert_cmpint(err, ==, -EINVAL);
+    g_assert(endptr == str);
+}
+
+static void test_qemu_strtoui_trailing(void)
+{
+    const char *str = "123xxx";
+    char f = 'X';
+    const char *endptr = &f;
+    unsigned int res = 999;
+    int err;
+
+    err = qemu_strtoui(str, &endptr, 0, &res);
+
+    g_assert_cmpint(err, ==, 0);
+    g_assert_cmpuint(res, ==, 123);
+    g_assert(endptr == str + 3);
+}
+
+static void test_qemu_strtoui_octal(void)
+{
+    const char *str = "0123";
+    char f = 'X';
+    const char *endptr = &f;
+    unsigned int res = 999;
+    int err;
+
+    err = qemu_strtoui(str, &endptr, 8, &res);
+
+    g_assert_cmpint(err, ==, 0);
+    g_assert_cmpuint(res, ==, 0123);
+    g_assert(endptr == str + strlen(str));
+
+    res = 999;
+    endptr = &f;
+    err = qemu_strtoui(str, &endptr, 0, &res);
+
+    g_assert_cmpint(err, ==, 0);
+    g_assert_cmpuint(res, ==, 0123);
+    g_assert(endptr == str + strlen(str));
+}
+
+static void test_qemu_strtoui_decimal(void)
+{
+    const char *str = "0123";
+    char f = 'X';
+    const char *endptr = &f;
+    unsigned int res = 999;
+    int err;
+
+    err = qemu_strtoui(str, &endptr, 10, &res);
+
+    g_assert_cmpint(err, ==, 0);
+    g_assert_cmpuint(res, ==, 123);
+    g_assert(endptr == str + strlen(str));
+
+    str = "123";
+    res = 999;
+    endptr = &f;
+    err = qemu_strtoui(str, &endptr, 0, &res);
+
+    g_assert_cmpint(err, ==, 0);
+    g_assert_cmpuint(res, ==, 123);
+    g_assert(endptr == str + strlen(str));
+}
+
+static void test_qemu_strtoui_hex(void)
+{
+    const char *str = "0123";
+    char f = 'X';
+    const char *endptr = &f;
+    unsigned int res = 999;
+    int err;
+
+    err = qemu_strtoui(str, &endptr, 16, &res);
+
+    g_assert_cmpint(err, ==, 0);
+    g_assert_cmphex(res, ==, 0x123);
+    g_assert(endptr == str + strlen(str));
+
+    str = "0x123";
+    res = 999;
+    endptr = &f;
+    err = qemu_strtoui(str, &endptr, 0, &res);
+
+    g_assert_cmpint(err, ==, 0);
+    g_assert_cmphex(res, ==, 0x123);
+    g_assert(endptr == str + strlen(str));
+}
+
+static void test_qemu_strtoui_max(void)
+{
+    char *str = g_strdup_printf("%u", UINT_MAX);
+    char f = 'X';
+    const char *endptr = &f;
+    unsigned int res = 999;
+    int err;
+
+    err = qemu_strtoui(str, &endptr, 0, &res);
+
+    g_assert_cmpint(err, ==, 0);
+    g_assert_cmphex(res, ==, UINT_MAX);
+    g_assert(endptr == str + strlen(str));
+    g_free(str);
+}
+
+static void test_qemu_strtoui_overflow(void)
+{
+    const char *str = "99999999999999999999999999999999999999999999";
+    char f = 'X';
+    const char *endptr = &f;
+    unsigned int res = 999;
+    int err;
+
+    err = qemu_strtoui(str, &endptr, 0, &res);
+
+    g_assert_cmpint(err, ==, -ERANGE);
+    g_assert_cmphex(res, ==, UINT_MAX);
+    g_assert(endptr == str + strlen(str));
+}
+
+static void test_qemu_strtoui_underflow(void)
+{
+    const char *str = "-99999999999999999999999999999999999999999999";
+    char f = 'X';
+    const char *endptr = &f;
+    unsigned int res = 999;
+    int err;
+
+    err  = qemu_strtoui(str, &endptr, 0, &res);
+
+    g_assert_cmpint(err, ==, -ERANGE);
+    g_assert_cmpuint(res, ==, (unsigned int)-1);
+    g_assert(endptr == str + strlen(str));
+}
+
+static void test_qemu_strtoui_negative(void)
+{
+    const char *str = "  \t -321";
+    char f = 'X';
+    const char *endptr = &f;
+    unsigned int res = 999;
+    int err;
+
+    err = qemu_strtoui(str, &endptr, 0, &res);
+
+    g_assert_cmpint(err, ==, 0);
+    g_assert_cmpuint(res, ==, (unsigned int)-321);
+    g_assert(endptr == str + strlen(str));
+}
+
+static void test_qemu_strtoui_full_correct(void)
+{
+    const char *str = "123";
+    unsigned int res = 999;
+    int err;
+
+    err = qemu_strtoui(str, NULL, 0, &res);
+
+    g_assert_cmpint(err, ==, 0);
+    g_assert_cmpuint(res, ==, 123);
+}
+
+static void test_qemu_strtoui_full_null(void)
+{
+    unsigned int res = 999;
+    int err;
+
+    err = qemu_strtoui(NULL, NULL, 0, &res);
+
+    g_assert_cmpint(err, ==, -EINVAL);
+}
+
+static void test_qemu_strtoui_full_empty(void)
+{
+    const char *str = "";
+    unsigned int res = 999;
+    int err;
+
+    err = qemu_strtoui(str, NULL, 0, &res);
+
+    g_assert_cmpint(err, ==, -EINVAL);
+}
+static void test_qemu_strtoui_full_negative(void)
+{
+    const char *str = " \t -321";
+    unsigned int res = 999;
+    int err;
+
+    err = qemu_strtoui(str, NULL, 0, &res);
+    g_assert_cmpint(err, ==, 0);
+    g_assert_cmpuint(res, ==, (unsigned int)-321);
+}
+
+static void test_qemu_strtoui_full_trailing(void)
+{
+    const char *str = "123xxx";
+    unsigned int res;
+    int err;
+
+    err = qemu_strtoui(str, NULL, 0, &res);
+
+    g_assert_cmpint(err, ==, -EINVAL);
+}
+
+static void test_qemu_strtoui_full_max(void)
+{
+    char *str = g_strdup_printf("%u", UINT_MAX);
+    unsigned int res = 999;
+    int err;
+
+    err = qemu_strtoui(str, NULL, 0, &res);
+
+    g_assert_cmpint(err, ==, 0);
+    g_assert_cmphex(res, ==, UINT_MAX);
+    g_free(str);
+}
+
 static void test_qemu_strtol_correct(void)
 {
     const char *str = "12345 foo";
@@ -1612,6 +2185,86 @@ int main(int argc, char **argv)
     g_test_add_func("/cutils/parse_uint_full/correct",
                     test_parse_uint_full_correct);
 
+    /* qemu_strtoi() tests */
+    g_test_add_func("/cutils/qemu_strtoi/correct",
+                    test_qemu_strtoi_correct);
+    g_test_add_func("/cutils/qemu_strtoi/null",
+                    test_qemu_strtoi_null);
+    g_test_add_func("/cutils/qemu_strtoi/empty",
+                    test_qemu_strtoi_empty);
+    g_test_add_func("/cutils/qemu_strtoi/whitespace",
+                    test_qemu_strtoi_whitespace);
+    g_test_add_func("/cutils/qemu_strtoi/invalid",
+                    test_qemu_strtoi_invalid);
+    g_test_add_func("/cutils/qemu_strtoi/trailing",
+                    test_qemu_strtoi_trailing);
+    g_test_add_func("/cutils/qemu_strtoi/octal",
+                    test_qemu_strtoi_octal);
+    g_test_add_func("/cutils/qemu_strtoi/decimal",
+                    test_qemu_strtoi_decimal);
+    g_test_add_func("/cutils/qemu_strtoi/hex",
+                    test_qemu_strtoi_hex);
+    g_test_add_func("/cutils/qemu_strtoi/max",
+                    test_qemu_strtoi_max);
+    g_test_add_func("/cutils/qemu_strtoi/overflow",
+                    test_qemu_strtoi_overflow);
+    g_test_add_func("/cutils/qemu_strtoi/underflow",
+                    test_qemu_strtoi_underflow);
+    g_test_add_func("/cutils/qemu_strtoi/negative",
+                    test_qemu_strtoi_negative);
+    g_test_add_func("/cutils/qemu_strtoi_full/correct",
+                    test_qemu_strtoi_full_correct);
+    g_test_add_func("/cutils/qemu_strtoi_full/null",
+                    test_qemu_strtoi_full_null);
+    g_test_add_func("/cutils/qemu_strtoi_full/empty",
+                    test_qemu_strtoi_full_empty);
+    g_test_add_func("/cutils/qemu_strtoi_full/negative",
+                    test_qemu_strtoi_full_negative);
+    g_test_add_func("/cutils/qemu_strtoi_full/trailing",
+                    test_qemu_strtoi_full_trailing);
+    g_test_add_func("/cutils/qemu_strtoi_full/max",
+                    test_qemu_strtoi_full_max);
+
+    /* qemu_strtoui() tests */
+    g_test_add_func("/cutils/qemu_strtoui/correct",
+                    test_qemu_strtoui_correct);
+    g_test_add_func("/cutils/qemu_strtoui/null",
+                    test_qemu_strtoui_null);
+    g_test_add_func("/cutils/qemu_strtoui/empty",
+                    test_qemu_strtoui_empty);
+    g_test_add_func("/cutils/qemu_strtoui/whitespace",
+                    test_qemu_strtoui_whitespace);
+    g_test_add_func("/cutils/qemu_strtoui/invalid",
+                    test_qemu_strtoui_invalid);
+    g_test_add_func("/cutils/qemu_strtoui/trailing",
+                    test_qemu_strtoui_trailing);
+    g_test_add_func("/cutils/qemu_strtoui/octal",
+                    test_qemu_strtoui_octal);
+    g_test_add_func("/cutils/qemu_strtoui/decimal",
+                    test_qemu_strtoui_decimal);
+    g_test_add_func("/cutils/qemu_strtoui/hex",
+                    test_qemu_strtoui_hex);
+    g_test_add_func("/cutils/qemu_strtoui/max",
+                    test_qemu_strtoui_max);
+    g_test_add_func("/cutils/qemu_strtoui/overflow",
+                    test_qemu_strtoui_overflow);
+    g_test_add_func("/cutils/qemu_strtoui/underflow",
+                    test_qemu_strtoui_underflow);
+    g_test_add_func("/cutils/qemu_strtoui/negative",
+                    test_qemu_strtoui_negative);
+    g_test_add_func("/cutils/qemu_strtoui_full/correct",
+                    test_qemu_strtoui_full_correct);
+    g_test_add_func("/cutils/qemu_strtoui_full/null",
+                    test_qemu_strtoui_full_null);
+    g_test_add_func("/cutils/qemu_strtoui_full/empty",
+                    test_qemu_strtoui_full_empty);
+    g_test_add_func("/cutils/qemu_strtoui_full/negative",
+                    test_qemu_strtoui_full_negative);
+    g_test_add_func("/cutils/qemu_strtoui_full/trailing",
+                    test_qemu_strtoui_full_trailing);
+    g_test_add_func("/cutils/qemu_strtoui_full/max",
+                    test_qemu_strtoui_full_max);
+
     /* qemu_strtol() tests */
     g_test_add_func("/cutils/qemu_strtol/correct",
                     test_qemu_strtol_correct);
diff --git a/util/cutils.c b/util/cutils.c
index b33ede83d1..9f05b331d9 100644
--- a/util/cutils.c
+++ b/util/cutils.c
@@ -297,6 +297,110 @@ static int check_strtox_error(const char *nptr, char *ep,
     return -libc_errno;
 }
 
+/**
+ * Convert string @nptr to a long integer, and store it in @result.
+ *
+ * This is a wrapper around strtol() that is harder to misuse.
+ * Semantics of @nptr, @endptr, @base match strtol() with differences
+ * noted below.
+ *
+ * @nptr may be null, and no conversion is performed then.
+ *
+ * If no conversion is performed, store @nptr in *@endptr and return
+ * -EINVAL.
+ *
+ * If @endptr is null, and the string isn't fully converted, return
+ * -EINVAL.  This is the case when the pointer that would be stored in
+ * a non-null @endptr points to a character other than '\0'.
+ *
+ * If the conversion overflows @result, store INT_MAX in @result,
+ * and return -ERANGE.
+ *
+ * If the conversion underflows @result, store INT_MIN in @result,
+ * and return -ERANGE.
+ *
+ * Else store the converted value in @result, and return zero.
+ */
+int qemu_strtoi(const char *nptr, const char **endptr, int base,
+                int *result)
+{
+    char *ep;
+    long lresult;
+
+    if (!nptr) {
+        if (endptr) {
+            *endptr = nptr;
+        }
+        return -EINVAL;
+    }
+
+    errno = 0;
+    lresult = strtol(nptr, &ep, base);
+    if (lresult < INT_MIN) {
+        *result = INT_MIN;
+    } else if (lresult > INT_MAX) {
+        *result = INT_MAX;
+    } else {
+        *result = lresult;
+    }
+    return check_strtox_error(nptr, ep, endptr, errno);
+}
+
+/**
+ * Convert string @nptr to an unsigned int, and store it in @result.
+ *
+ * This is a wrapper around strtoul() that is harder to misuse.
+ * Semantics of @nptr, @endptr, @base match strtoul() with differences
+ * noted below.
+ *
+ * @nptr may be null, and no conversion is performed then.
+ *
+ * If no conversion is performed, store @nptr in *@endptr and return
+ * -EINVAL.
+ *
+ * If @endptr is null, and the string isn't fully converted, return
+ * -EINVAL.  This is the case when the pointer that would be stored in
+ * a non-null @endptr points to a character other than '\0'.
+ *
+ * If the conversion overflows @result, store UINT_MAX in @result,
+ * and return -ERANGE.
+ *
+ * Else store the converted value in @result, and return zero.
+ *
+ * Note that a number with a leading minus sign gets converted without
+ * the minus sign, checked for overflow (see above), then negated (in
+ * @result's type).  This is exactly how strtoul() works.
+ */
+int qemu_strtoui(const char *nptr, const char **endptr, int base,
+                 unsigned int *result)
+{
+    char *ep;
+    long lresult;
+
+    if (!nptr) {
+        if (endptr) {
+            *endptr = nptr;
+        }
+        return -EINVAL;
+    }
+
+    errno = 0;
+    lresult = strtol(nptr, &ep, base);
+    /* Windows returns 1 for negative out-of-range values.  */
+    if (errno == ERANGE) {
+        *result = -1;
+    } else {
+        if (lresult > UINT_MAX) {
+            *result = UINT_MAX;
+        } else if (lresult < INT_MIN) {
+            *result = UINT_MAX;
+        } else {
+            *result = lresult;
+        }
+    }
+    return check_strtox_error(nptr, ep, endptr, errno);
+}
+
 /**
  * Convert string @nptr to a long integer, and store it in @result.
  *
-- 
2.14.3

[Qemu-devel] [PATCH v3 2/8] sockets: move fd_is_socket() into common sockets code

[Daniel P. Berrange]

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.

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
---
 include/qemu/sockets.h |  1 +
 io/channel-util.c      | 13 -------------
 util/qemu-sockets.c    |  8 ++++++++
 3 files changed, 9 insertions(+), 13 deletions(-)

diff --git a/include/qemu/sockets.h b/include/qemu/sockets.h
index 4f7311b52a..5680880f5a 100644
--- a/include/qemu/sockets.h
+++ b/include/qemu/sockets.h
@@ -12,6 +12,7 @@ int inet_aton(const char *cp, struct in_addr *ia);
 #include "qapi-types.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);
diff --git a/io/channel-util.c b/io/channel-util.c
index 0fb4bd0837..423d79845a 100644
--- a/io/channel-util.c
+++ b/io/channel-util.c
@@ -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)
 {
diff --git a/util/qemu-sockets.c b/util/qemu-sockets.c
index af4f01211a..5e42f6d88d 100644
--- a/util/qemu-sockets.c
+++ b/util/qemu-sockets.c
@@ -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
  *
-- 
2.14.3

[Qemu-devel] [PATCH v3 3/8] sockets: pull code for testing IP availability out of specific test

[Daniel P. Berrange]

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.

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
---
 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

diff --git a/tests/Makefile.include b/tests/Makefile.include
index f8e20d9f5d..bede832dc1 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -696,7 +696,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/test-io-channel-file$(EXESUF): tests/test-io-channel-file.o \
         tests/io-channel-helpers.o $(test-io-obj-y)
 tests/test-io-channel-tls$(EXESUF): tests/test-io-channel-tls.o \
diff --git a/tests/socket-helpers.c b/tests/socket-helpers.c
new file mode 100644
index 0000000000..13b6bb38eb
--- /dev/null
+++ b/tests/socket-helpers.c
@@ -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;
+    }
+
+    if (socket_can_bind("::1") < 0) {
+        if (errno != EADDRNOTAVAIL) {
+            return -1;
+        }
+    } else {
+        *has_ipv6 = true;
+    }
+
+    return 0;
+}
diff --git a/tests/socket-helpers.h b/tests/socket-helpers.h
new file mode 100644
index 0000000000..efa96eddc2
--- /dev/null
+++ b/tests/socket-helpers.h
@@ -0,0 +1,42 @@
+/*
+ * 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/>.
+ *
+ */
+
+/*
+ * @hostname: a DNS name or numeric IP address
+ *
+ * Check whether it is possible to bind 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);
+
+/*
+ * @has_ipv4: set to true on return if IPv4 is available
+ * @has_ipv6: set to true on return if IPv6 is available
+ *
+ * Check whether IPv4 and/or IPv6 are available for use.
+ * On success, @has_ipv4 and @has_ipv6 will be set to
+ * indicate whether the respective protocols are available.
+ *
+ * Returns 0 on success, -1 on fatal error
+ */
+int socket_check_protocol_support(bool *has_ipv4, bool *has_ipv6);
diff --git a/tests/test-io-channel-socket.c b/tests/test-io-channel-socket.c
index d357cd2a8e..b0b69e8ad0 100644
--- a/tests/test-io-channel-socket.c
+++ b/tests/test-io-channel-socket.c
@@ -22,77 +22,9 @@
 #include "io/channel-socket.h"
 #include "io/channel-util.h"
 #include "io-channel-helpers.h"
+#include "socket-helpers.h"
 #include "qapi/error.h"
 
-#ifndef AI_ADDRCONFIG
-# define AI_ADDRCONFIG 0
-#endif
-#ifndef EAI_ADDRFAMILY
-# define EAI_ADDRFAMILY 0
-#endif
-
-static int check_bind(const char *hostname, bool *has_proto)
-{
-    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) {
-            *has_proto = false;
-            goto done;
-        }
-        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) {
-            *has_proto = false;
-            goto done;
-        }
-        goto cleanup;
-    }
-
-    *has_proto = true;
- done:
-    ret = 0;
-
- cleanup:
-    if (fd != -1) {
-        close(fd);
-    }
-    if (res) {
-        freeaddrinfo(res);
-    }
-    return ret;
-}
-
-static int check_protocol_support(bool *has_ipv4, bool *has_ipv6)
-{
-    if (check_bind("127.0.0.1", has_ipv4) < 0) {
-        return -1;
-    }
-    if (check_bind("::1", has_ipv6) < 0) {
-        return -1;
-    }
-
-    return 0;
-}
-
 
 static void test_io_channel_set_socket_bufs(QIOChannel *src,
                                             QIOChannel *dst)
@@ -566,7 +498,7 @@ int main(int argc, char **argv)
      * each protocol to avoid breaking tests on machines
      * with either IPv4 or IPv6 disabled.
      */
-    if (check_protocol_support(&has_ipv4, &has_ipv6) < 0) {
+    if (socket_check_protocol_support(&has_ipv4, &has_ipv6) < 0) {
         return 1;
     }
 
-- 
2.14.3

[Qemu-devel] [PATCH v3 4/8] sockets: strengthen test suite IP protocol availability checks

[Daniel P. Berrange]

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.

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
---
 tests/socket-helpers.c | 67 +++++++++++++++++++++++++++++++++++++++++++-------
 tests/socket-helpers.h |  4 +--
 2 files changed, 60 insertions(+), 11 deletions(-)

diff --git a/tests/socket-helpers.c b/tests/socket-helpers.c
index 13b6bb38eb..161aa5da09 100644
--- a/tests/socket-helpers.c
+++ b/tests/socket-helpers.c
@@ -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;
         }
diff --git a/tests/socket-helpers.h b/tests/socket-helpers.h
index efa96eddc2..1c07d6d656 100644
--- a/tests/socket-helpers.h
+++ b/tests/socket-helpers.h
@@ -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
-- 
2.14.3

[Qemu-devel] [PATCH v3 5/8] sockets: check that the named file descriptor is a socket

[Daniel P. Berrange]

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.

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
---
 tests/.gitignore       |   1 +
 tests/Makefile.include |   3 ++
 tests/test-sockets.c   | 139 +++++++++++++++++++++++++++++++++++++++++++++++++
 util/qemu-sockets.c    |  18 ++++++-
 4 files changed, 159 insertions(+), 2 deletions(-)
 create mode 100644 tests/test-sockets.c

diff --git a/tests/.gitignore b/tests/.gitignore
index 74e55d7264..44be8c2c79 100644
--- a/tests/.gitignore
+++ b/tests/.gitignore
@@ -80,6 +80,7 @@ test-qobject-output-visitor
 test-rcu-list
 test-replication
 test-shift128
+test-sockets
 test-string-input-visitor
 test-string-output-visitor
 test-thread-pool
diff --git a/tests/Makefile.include b/tests/Makefile.include
index bede832dc1..2730f7562b 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -135,6 +135,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-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)
@@ -694,6 +695,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-sockets$(EXESUF): tests/test-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)
diff --git a/tests/test-sockets.c b/tests/test-sockets.c
new file mode 100644
index 0000000000..8a2962fe7b
--- /dev/null
+++ b/tests/test-sockets.c
@@ -0,0 +1,139 @@
+/*
+ * Test core sockets APIs
+ *
+ * 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/sockets.h"
+#include "socket-helpers.h"
+#include "qapi/error.h"
+#include "monitor/monitor.h"
+
+
+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;
+
+    module_call_init(MODULE_INIT_QOM);
+    socket_init();
+
+    g_test_init(&argc, &argv, NULL);
+
+    /* We're creating actual IPv4/6 sockets, so we should
+     * check if the host running tests actually supports
+     * each protocol to avoid breaking tests on machines
+     * with either IPv4 or IPv6 disabled.
+     */
+    if (socket_check_protocol_support(&has_ipv4, &has_ipv6) < 0) {
+        return 1;
+    }
+
+    if (has_ipv4) {
+        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();
+}
diff --git a/util/qemu-sockets.c b/util/qemu-sockets.c
index 5e42f6d88d..6a7b194428 100644
--- a/util/qemu-sockets.c
+++ b/util/qemu-sockets.c
@@ -1027,6 +1027,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;
@@ -1041,7 +1055,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:
@@ -1068,7 +1082,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:
-- 
2.14.3

[Qemu-devel] [PATCH v3 6/8] sockets: allow SocketAddress 'fd' to reference numeric file descriptors

[Daniel P. Berrange]

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.

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
---
 qapi/sockets.json    |   7 ++++
 tests/test-sockets.c | 112 ++++++++++++++++++++++++++++++++++++++++++++++++---
 util/qemu-sockets.c  |  16 ++++++--
 3 files changed, 126 insertions(+), 9 deletions(-)

diff --git a/qapi/sockets.json b/qapi/sockets.json
index ac022c6ad0..fc81d8d5e8 100644
--- a/qapi/sockets.json
+++ b/qapi/sockets.json
@@ -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',
diff --git a/tests/test-sockets.c b/tests/test-sockets.c
index 8a2962fe7b..8307e4774f 100644
--- a/tests/test-sockets.c
+++ b/tests/test-sockets.c
@@ -49,7 +49,7 @@ Monitor *cur_mon;
 void monitor_init(Chardev *chr, int flags) {}
 
 
-static void test_socket_fd_pass_good(void)
+static void test_socket_fd_pass_name_good(void)
 {
     SocketAddress addr;
     int fd;
@@ -80,7 +80,7 @@ static void test_socket_fd_pass_good(void)
     cur_mon = NULL;
 }
 
-static void test_socket_fd_pass_bad(void)
+static void test_socket_fd_pass_name_bad(void)
 {
     SocketAddress addr;
     Error *err = NULL;
@@ -110,6 +110,98 @@ static void test_socket_fd_pass_bad(void)
     cur_mon = NULL;
 }
 
+static void test_socket_fd_pass_name_nomon(void)
+{
+    SocketAddress addr;
+    Error *err = NULL;
+    int fd;
+
+    g_assert(cur_mon == NULL);
+
+    addr.type = SOCKET_ADDRESS_TYPE_FD;
+    addr.u.fd.str = g_strdup("myfd");
+
+    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);
+}
+
+
+static void test_socket_fd_pass_num_good(void)
+{
+    SocketAddress addr;
+    int fd, sfd;
+
+    g_assert(cur_mon == NULL);
+    sfd = qemu_socket(AF_INET, SOCK_STREAM, 0);
+    g_assert_cmpint(sfd, >, STDERR_FILENO);
+
+    addr.type = SOCKET_ADDRESS_TYPE_FD;
+    addr.u.fd.str = g_strdup_printf("%d", sfd);
+
+    fd = socket_connect(&addr, &error_abort);
+    g_assert_cmpint(fd, ==, sfd);
+
+    fd = socket_listen(&addr, &error_abort);
+    g_assert_cmpint(fd, ==, sfd);
+
+    g_free(addr.u.fd.str);
+    close(sfd);
+}
+
+static void test_socket_fd_pass_num_bad(void)
+{
+    SocketAddress addr;
+    Error *err = NULL;
+    int fd, sfd;
+
+    g_assert(cur_mon == NULL);
+    sfd = dup(STDOUT_FILENO);
+
+    addr.type = SOCKET_ADDRESS_TYPE_FD;
+    addr.u.fd.str = g_strdup_printf("%d", sfd);
+
+    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);
+    close(sfd);
+}
+
+static void test_socket_fd_pass_num_nocli(void)
+{
+    SocketAddress addr;
+    Error *err = NULL;
+    int fd;
+
+    cur_mon = g_malloc(1); /* Fake a monitor */
+
+    addr.type = SOCKET_ADDRESS_TYPE_FD;
+    addr.u.fd.str = g_strdup_printf("%d", STDOUT_FILENO);
+
+    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);
+}
+
+
 int main(int argc, char **argv)
 {
     bool has_ipv4, has_ipv6;
@@ -129,10 +221,18 @@ int main(int argc, char **argv)
     }
 
     if (has_ipv4) {
-        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);
+        g_test_add_func("/socket/fd-pass/name/good",
+                        test_socket_fd_pass_name_good);
+        g_test_add_func("/socket/fd-pass/name/bad",
+                        test_socket_fd_pass_name_bad);
+        g_test_add_func("/socket/fd-pass/name/nomon",
+                        test_socket_fd_pass_name_nomon);
+        g_test_add_func("/socket/fd-pass/num/good",
+                        test_socket_fd_pass_num_good);
+        g_test_add_func("/socket/fd-pass/num/bad",
+                        test_socket_fd_pass_num_bad);
+        g_test_add_func("/socket/fd-pass/num/nocli",
+                        test_socket_fd_pass_num_nocli);
     }
 
     return g_test_run();
diff --git a/util/qemu-sockets.c b/util/qemu-sockets.c
index 6a7b194428..8ac4eed122 100644
--- a/util/qemu-sockets.c
+++ b/util/qemu-sockets.c
@@ -1029,9 +1029,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);
-- 
2.14.3

[Qemu-devel] [PATCH v3 7/8] char: refactor parsing of socket address information

[Daniel P. Berrange]

To prepare for handling more address types, refactor the parsing of
socket address information to make it more robust and extensible.

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
---
 chardev/char-socket.c | 22 ++++++++++++++--------
 1 file changed, 14 insertions(+), 8 deletions(-)

diff --git a/chardev/char-socket.c b/chardev/char-socket.c
index 6013972f72..9113ea0169 100644
--- a/chardev/char-socket.c
+++ b/chardev/char-socket.c
@@ -987,21 +987,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);
@@ -1027,7 +1031,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) {
@@ -1040,6 +1044,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;
 }
-- 
2.14.3

[Qemu-devel] [PATCH v3 8/8] char: allow passing pre-opened socket file descriptor at startup

[Daniel P. Berrange]

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";

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
---
 chardev/char-socket.c | 15 +++++++++++++--
 chardev/char.c        |  3 +++
 tests/test-char.c     | 47 ++++++++++++++++++++++++++++++++++++++++++++---
 3 files changed, 60 insertions(+), 5 deletions(-)

diff --git a/chardev/char-socket.c b/chardev/char-socket.c
index 9113ea0169..a1e6da4fc8 100644
--- a/chardev/char-socket.c
+++ b/chardev/char-socket.c
@@ -983,13 +983,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;
     }
 
@@ -1004,6 +1005,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();
     }
@@ -1044,6 +1051,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();
     }
diff --git a/chardev/char.c b/chardev/char.c
index 2ae4f465ec..9d674855ae 100644
--- a/chardev/char.c
+++ b/chardev/char.c
@@ -798,6 +798,9 @@ QemuOptsList qemu_chardev_opts = {
         },{
             .name = "port",
             .type = QEMU_OPT_STRING,
+        },{
+            .name = "fd",
+            .type = QEMU_OPT_STRING,
         },{
             .name = "localaddr",
             .type = QEMU_OPT_STRING,
diff --git a/tests/test-char.c b/tests/test-char.c
index 7ac25ff73f..f0c5544f24 100644
--- a/tests/test-char.c
+++ b/tests/test-char.c
@@ -284,9 +284,8 @@ static int socket_can_read_hello(void *opaque)
     return 10;
 }
 
-static void char_socket_test(void)
+static void char_socket_test_common(Chardev *chr)
 {
-    Chardev *chr = qemu_chr_new("server", "tcp:127.0.0.1:0,server,nowait");
     Chardev *chr_client;
     QObject *addr;
     QDict *qdict;
@@ -341,6 +340,47 @@ static void char_socket_test(void)
     object_unparent(OBJECT(chr));
 }
 
+
+static void char_socket_basic_test(void)
+{
+    Chardev *chr = qemu_chr_new("server", "tcp:127.0.0.1:0,server,nowait");
+
+    char_socket_test_common(chr);
+}
+
+
+static void char_socket_fdpass_test(void)
+{
+    Chardev *chr;
+    char *optstr;
+    QemuOpts *opts;
+    int fd;
+    SocketAddress *addr = g_new0(SocketAddress, 1);
+
+    addr->type = SOCKET_ADDRESS_TYPE_INET;
+    addr->u.inet.host = g_strdup("127.0.0.1");
+    addr->u.inet.port = g_strdup("0");
+
+    fd = socket_listen(addr, &error_abort);
+    g_assert(fd >= 0);
+
+    qapi_free_SocketAddress(addr);
+
+    optstr = g_strdup_printf("socket,id=cdev,fd=%d,server,nowait", fd);
+
+    opts = qemu_opts_parse_noisily(qemu_find_opts("chardev"),
+                                   optstr, true);
+    g_free(optstr);
+    g_assert_nonnull(opts);
+
+    chr = qemu_chr_new_from_opts(opts, &error_abort);
+
+    qemu_opts_del(opts);
+
+    char_socket_test_common(chr);
+}
+
+
 #ifndef _WIN32
 static void char_pipe_test(void)
 {
@@ -757,7 +797,8 @@ int main(int argc, char **argv)
 #ifndef _WIN32
     g_test_add_func("/char/file-fifo", char_file_fifo_test);
 #endif
-    g_test_add_func("/char/socket", char_socket_test);
+    g_test_add_func("/char/socket/basic", char_socket_basic_test);
+    g_test_add_func("/char/socket/fdpass", char_socket_fdpass_test);
     g_test_add_func("/char/udp", char_udp_test);
 #ifdef HAVE_CHARDEV_SERIAL
     g_test_add_func("/char/serial", char_serial_test);
-- 
2.14.3

Re: [Qemu-devel] [PATCH v3 2/8] sockets: move fd_is_socket() into common sockets code

[Marc-André Lureau]

Hi

On Fri, Dec 22, 2017 at 2:45 PM, Daniel P. Berrange <berrange@redhat.com> wrote:
> 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.
>
> Signed-off-by: Daniel P. Berrange <berrange@redhat.com>

Now that you do some code change, it would be welcome with a small unit test ;)

anyway,

Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>


> ---
>  include/qemu/sockets.h |  1 +
>  io/channel-util.c      | 13 -------------
>  util/qemu-sockets.c    |  8 ++++++++
>  3 files changed, 9 insertions(+), 13 deletions(-)
>
> diff --git a/include/qemu/sockets.h b/include/qemu/sockets.h
> index 4f7311b52a..5680880f5a 100644
> --- a/include/qemu/sockets.h
> +++ b/include/qemu/sockets.h
> @@ -12,6 +12,7 @@ int inet_aton(const char *cp, struct in_addr *ia);
>  #include "qapi-types.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);
> diff --git a/io/channel-util.c b/io/channel-util.c
> index 0fb4bd0837..423d79845a 100644
> --- a/io/channel-util.c
> +++ b/io/channel-util.c
> @@ -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)
>  {
> diff --git a/util/qemu-sockets.c b/util/qemu-sockets.c
> index af4f01211a..5e42f6d88d 100644
> --- a/util/qemu-sockets.c
> +++ b/util/qemu-sockets.c
> @@ -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
>   *
> --
> 2.14.3
>
>



-- 
Marc-André Lureau

Re: [Qemu-devel] [PATCH v3 3/8] sockets: pull code for testing IP availability out of specific test

[Marc-André Lureau]

On Fri, Dec 22, 2017 at 2:45 PM, Daniel P. Berrange <berrange@redhat.com> wrote:
> 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.
>
> Signed-off-by: Daniel P. Berrange <berrange@redhat.com>

Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>


> ---
>  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
>
> diff --git a/tests/Makefile.include b/tests/Makefile.include
> index f8e20d9f5d..bede832dc1 100644
> --- a/tests/Makefile.include
> +++ b/tests/Makefile.include
> @@ -696,7 +696,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/test-io-channel-file$(EXESUF): tests/test-io-channel-file.o \
>          tests/io-channel-helpers.o $(test-io-obj-y)
>  tests/test-io-channel-tls$(EXESUF): tests/test-io-channel-tls.o \
> diff --git a/tests/socket-helpers.c b/tests/socket-helpers.c
> new file mode 100644
> index 0000000000..13b6bb38eb
> --- /dev/null
> +++ b/tests/socket-helpers.c
> @@ -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;
> +    }
> +
> +    if (socket_can_bind("::1") < 0) {
> +        if (errno != EADDRNOTAVAIL) {
> +            return -1;
> +        }
> +    } else {
> +        *has_ipv6 = true;
> +    }
> +
> +    return 0;
> +}
> diff --git a/tests/socket-helpers.h b/tests/socket-helpers.h
> new file mode 100644
> index 0000000000..efa96eddc2
> --- /dev/null
> +++ b/tests/socket-helpers.h
> @@ -0,0 +1,42 @@
> +/*
> + * 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/>.
> + *
> + */
> +
> +/*
> + * @hostname: a DNS name or numeric IP address
> + *
> + * Check whether it is possible to bind 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);
> +
> +/*
> + * @has_ipv4: set to true on return if IPv4 is available
> + * @has_ipv6: set to true on return if IPv6 is available
> + *
> + * Check whether IPv4 and/or IPv6 are available for use.
> + * On success, @has_ipv4 and @has_ipv6 will be set to
> + * indicate whether the respective protocols are available.
> + *
> + * Returns 0 on success, -1 on fatal error
> + */
> +int socket_check_protocol_support(bool *has_ipv4, bool *has_ipv6);
> diff --git a/tests/test-io-channel-socket.c b/tests/test-io-channel-socket.c
> index d357cd2a8e..b0b69e8ad0 100644
> --- a/tests/test-io-channel-socket.c
> +++ b/tests/test-io-channel-socket.c
> @@ -22,77 +22,9 @@
>  #include "io/channel-socket.h"
>  #include "io/channel-util.h"
>  #include "io-channel-helpers.h"
> +#include "socket-helpers.h"
>  #include "qapi/error.h"
>
> -#ifndef AI_ADDRCONFIG
> -# define AI_ADDRCONFIG 0
> -#endif
> -#ifndef EAI_ADDRFAMILY
> -# define EAI_ADDRFAMILY 0
> -#endif
> -
> -static int check_bind(const char *hostname, bool *has_proto)
> -{
> -    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) {
> -            *has_proto = false;
> -            goto done;
> -        }
> -        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) {
> -            *has_proto = false;
> -            goto done;
> -        }
> -        goto cleanup;
> -    }
> -
> -    *has_proto = true;
> - done:
> -    ret = 0;
> -
> - cleanup:
> -    if (fd != -1) {
> -        close(fd);
> -    }
> -    if (res) {
> -        freeaddrinfo(res);
> -    }
> -    return ret;
> -}
> -
> -static int check_protocol_support(bool *has_ipv4, bool *has_ipv6)
> -{
> -    if (check_bind("127.0.0.1", has_ipv4) < 0) {
> -        return -1;
> -    }
> -    if (check_bind("::1", has_ipv6) < 0) {
> -        return -1;
> -    }
> -
> -    return 0;
> -}
> -
>
>  static void test_io_channel_set_socket_bufs(QIOChannel *src,
>                                              QIOChannel *dst)
> @@ -566,7 +498,7 @@ int main(int argc, char **argv)
>       * each protocol to avoid breaking tests on machines
>       * with either IPv4 or IPv6 disabled.
>       */
> -    if (check_protocol_support(&has_ipv4, &has_ipv6) < 0) {
> +    if (socket_check_protocol_support(&has_ipv4, &has_ipv6) < 0) {
>          return 1;
>      }
>
> --
> 2.14.3
>
>



-- 
Marc-André Lureau

Re: [Qemu-devel] [PATCH v3 0/8] Enable passing pre-opened chardev socket FDs

[no-reply]

Hi,

This series seems to have some coding style problems. See output below for
more information:

Type: series
Message-id: 20171222134527.14467-1-berrange@redhat.com
Subject: [Qemu-devel] [PATCH v3 0/8] Enable passing pre-opened chardev socket FDs

=== 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

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
From https://github.com/patchew-project/qemu
 t [tag update]            patchew/20171221212125.19075-1-maxime.coquelin@redhat.com -> patchew/20171221212125.19075-1-maxime.coquelin@redhat.com
 t [tag update]            patchew/20171222134527.14467-1-berrange@redhat.com -> patchew/20171222134527.14467-1-berrange@redhat.com
Switched to a new branch 'test'
67c8205ee6 char: allow passing pre-opened socket file descriptor at startup
4d76d8236d char: refactor parsing of socket address information
c994a8be65 sockets: allow SocketAddress 'fd' to reference numeric file descriptors
0b119aed04 sockets: check that the named file descriptor is a socket
46492af33f sockets: strengthen test suite IP protocol availability checks
6b8bfebc73 sockets: pull code for testing IP availability out of specific test
e8740ef3aa sockets: move fd_is_socket() into common sockets code
be50c154af cutils: add qemu_strtoi & qemu_strtoui parsers for int/unsigned int types

=== OUTPUT BEGIN ===
Checking PATCH 1/8: cutils: add qemu_strtoi & qemu_strtoui parsers for int/unsigned int types...
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);

total: 2 errors, 0 warnings, 785 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 2/8: sockets: move fd_is_socket() into common sockets code...
Checking PATCH 3/8: sockets: pull code for testing IP availability out of specific test...
Checking PATCH 4/8: sockets: strengthen test suite IP protocol availability checks...
Checking PATCH 5/8: sockets: check that the named file descriptor is a socket...
Checking PATCH 6/8: sockets: allow SocketAddress 'fd' to reference numeric file descriptors...
Checking PATCH 7/8: char: refactor parsing of socket address information...
Checking PATCH 8/8: 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 patchew-devel@freelists.org

Re: [Qemu-devel] [PATCH v3 1/8] cutils: add qemu_strtoi & qemu_strtoui parsers for int/unsigned int types

[Marc-André Lureau]

On Fri, Dec 22, 2017 at 2:45 PM, Daniel P. Berrange <berrange@redhat.com> wrote:
> There are qemu_strtoNN functions for various sized integers. This adds two
> more for plain int & unsigned int types, with suitable range checking.
>
> Signed-off-by: Daniel P. Berrange <berrange@redhat.com>

Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>


> ---
>  include/qemu/cutils.h |   4 +
>  tests/test-cutils.c   | 653 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  util/cutils.c         | 104 ++++++++
>  3 files changed, 761 insertions(+)
>
> diff --git a/include/qemu/cutils.h b/include/qemu/cutils.h
> index f0878eaafa..a663340b23 100644
> --- a/include/qemu/cutils.h
> +++ b/include/qemu/cutils.h
> @@ -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,
> diff --git a/tests/test-cutils.c b/tests/test-cutils.c
> index f64a49b7fb..0b2b5d417b 100644
> --- a/tests/test-cutils.c
> +++ b/tests/test-cutils.c
> @@ -223,6 +223,579 @@ static void test_parse_uint_full_correct(void)
>      g_assert_cmpint(i, ==, 123);
>  }
>
> +static void test_qemu_strtoi_correct(void)
> +{
> +    const char *str = "12345 foo";
> +    char f = 'X';
> +    const char *endptr = &f;
> +    int res = 999;
> +    int err;
> +
> +    err = qemu_strtoi(str, &endptr, 0, &res);
> +
> +    g_assert_cmpint(err, ==, 0);
> +    g_assert_cmpint(res, ==, 12345);
> +    g_assert(endptr == str + 5);
> +}
> +
> +static void test_qemu_strtoi_null(void)
> +{
> +    char f = 'X';
> +    const char *endptr = &f;
> +    int res = 999;
> +    int err;
> +
> +    err = qemu_strtoi(NULL, &endptr, 0, &res);
> +
> +    g_assert_cmpint(err, ==, -EINVAL);
> +    g_assert(endptr == NULL);
> +}
> +
> +static void test_qemu_strtoi_empty(void)
> +{
> +    const char *str = "";
> +    char f = 'X';
> +    const char *endptr = &f;
> +    int res = 999;
> +    int err;
> +
> +    err = qemu_strtoi(str, &endptr, 0, &res);
> +
> +    g_assert_cmpint(err, ==, -EINVAL);
> +    g_assert(endptr == str);
> +}
> +
> +static void test_qemu_strtoi_whitespace(void)
> +{
> +    const char *str = "  \t  ";
> +    char f = 'X';
> +    const char *endptr = &f;
> +    int res = 999;
> +    int err;
> +
> +    err = qemu_strtoi(str, &endptr, 0, &res);
> +
> +    g_assert_cmpint(err, ==, -EINVAL);
> +    g_assert(endptr == str);
> +}
> +
> +static void test_qemu_strtoi_invalid(void)
> +{
> +    const char *str = "   xxxx  \t abc";
> +    char f = 'X';
> +    const char *endptr = &f;
> +    int res = 999;
> +    int err;
> +
> +    err = qemu_strtoi(str, &endptr, 0, &res);
> +
> +    g_assert_cmpint(err, ==, -EINVAL);
> +    g_assert(endptr == str);
> +}
> +
> +static void test_qemu_strtoi_trailing(void)
> +{
> +    const char *str = "123xxx";
> +    char f = 'X';
> +    const char *endptr = &f;
> +    int res = 999;
> +    int err;
> +
> +    err = qemu_strtoi(str, &endptr, 0, &res);
> +
> +    g_assert_cmpint(err, ==, 0);
> +    g_assert_cmpint(res, ==, 123);
> +    g_assert(endptr == str + 3);
> +}
> +
> +static void test_qemu_strtoi_octal(void)
> +{
> +    const char *str = "0123";
> +    char f = 'X';
> +    const char *endptr = &f;
> +    int res = 999;
> +    int err;
> +
> +    err = qemu_strtoi(str, &endptr, 8, &res);
> +
> +    g_assert_cmpint(err, ==, 0);
> +    g_assert_cmpint(res, ==, 0123);
> +    g_assert(endptr == str + strlen(str));
> +
> +    res = 999;
> +    endptr = &f;
> +    err = qemu_strtoi(str, &endptr, 0, &res);
> +
> +    g_assert_cmpint(err, ==, 0);
> +    g_assert_cmpint(res, ==, 0123);
> +    g_assert(endptr == str + strlen(str));
> +}
> +
> +static void test_qemu_strtoi_decimal(void)
> +{
> +    const char *str = "0123";
> +    char f = 'X';
> +    const char *endptr = &f;
> +    int res = 999;
> +    int err;
> +
> +    err = qemu_strtoi(str, &endptr, 10, &res);
> +
> +    g_assert_cmpint(err, ==, 0);
> +    g_assert_cmpint(res, ==, 123);
> +    g_assert(endptr == str + strlen(str));
> +
> +    str = "123";
> +    res = 999;
> +    endptr = &f;
> +    err = qemu_strtoi(str, &endptr, 0, &res);
> +
> +    g_assert_cmpint(err, ==, 0);
> +    g_assert_cmpint(res, ==, 123);
> +    g_assert(endptr == str + strlen(str));
> +}
> +
> +static void test_qemu_strtoi_hex(void)
> +{
> +    const char *str = "0123";
> +    char f = 'X';
> +    const char *endptr = &f;
> +    int res = 999;
> +    int err;
> +
> +    err = qemu_strtoi(str, &endptr, 16, &res);
> +
> +    g_assert_cmpint(err, ==, 0);
> +    g_assert_cmpint(res, ==, 0x123);
> +    g_assert(endptr == str + strlen(str));
> +
> +    str = "0x123";
> +    res = 999;
> +    endptr = &f;
> +    err = qemu_strtoi(str, &endptr, 0, &res);
> +
> +    g_assert_cmpint(err, ==, 0);
> +    g_assert_cmpint(res, ==, 0x123);
> +    g_assert(endptr == str + strlen(str));
> +}
> +
> +static void test_qemu_strtoi_max(void)
> +{
> +    char *str = g_strdup_printf("%d", INT_MAX);
> +    char f = 'X';
> +    const char *endptr = &f;
> +    int res = 999;
> +    int err;
> +
> +    err = qemu_strtoi(str, &endptr, 0, &res);
> +
> +    g_assert_cmpint(err, ==, 0);
> +    g_assert_cmpint(res, ==, INT_MAX);
> +    g_assert(endptr == str + strlen(str));
> +    g_free(str);
> +}
> +
> +static void test_qemu_strtoi_overflow(void)
> +{
> +    const char *str = "99999999999999999999999999999999999999999999";
> +    char f = 'X';
> +    const char *endptr = &f;
> +    int res = 999;
> +    int err;
> +
> +    err = qemu_strtoi(str, &endptr, 0, &res);
> +
> +    g_assert_cmpint(err, ==, -ERANGE);
> +    g_assert_cmpint(res, ==, INT_MAX);
> +    g_assert(endptr == str + strlen(str));
> +}
> +
> +static void test_qemu_strtoi_underflow(void)
> +{
> +    const char *str = "-99999999999999999999999999999999999999999999";
> +    char f = 'X';
> +    const char *endptr = &f;
> +    int res = 999;
> +    int err;
> +
> +    err  = qemu_strtoi(str, &endptr, 0, &res);
> +
> +    g_assert_cmpint(err, ==, -ERANGE);
> +    g_assert_cmpint(res, ==, INT_MIN);
> +    g_assert(endptr == str + strlen(str));
> +}
> +
> +static void test_qemu_strtoi_negative(void)
> +{
> +    const char *str = "  \t -321";
> +    char f = 'X';
> +    const char *endptr = &f;
> +    int res = 999;
> +    int err;
> +
> +    err = qemu_strtoi(str, &endptr, 0, &res);
> +
> +    g_assert_cmpint(err, ==, 0);
> +    g_assert_cmpint(res, ==, -321);
> +    g_assert(endptr == str + strlen(str));
> +}
> +
> +static void test_qemu_strtoi_full_correct(void)
> +{
> +    const char *str = "123";
> +    int res = 999;
> +    int err;
> +
> +    err = qemu_strtoi(str, NULL, 0, &res);
> +
> +    g_assert_cmpint(err, ==, 0);
> +    g_assert_cmpint(res, ==, 123);
> +}
> +
> +static void test_qemu_strtoi_full_null(void)
> +{
> +    char f = 'X';
> +    const char *endptr = &f;
> +    int res = 999;
> +    int err;
> +
> +    err = qemu_strtoi(NULL, &endptr, 0, &res);
> +
> +    g_assert_cmpint(err, ==, -EINVAL);
> +    g_assert(endptr == NULL);
> +}
> +
> +static void test_qemu_strtoi_full_empty(void)
> +{
> +    const char *str = "";
> +    int res = 999L;
> +    int err;
> +
> +    err =  qemu_strtoi(str, NULL, 0, &res);
> +
> +    g_assert_cmpint(err, ==, -EINVAL);
> +}
> +
> +static void test_qemu_strtoi_full_negative(void)
> +{
> +    const char *str = " \t -321";
> +    int res = 999;
> +    int err;
> +
> +    err = qemu_strtoi(str, NULL, 0, &res);
> +
> +    g_assert_cmpint(err, ==, 0);
> +    g_assert_cmpint(res, ==, -321);
> +}
> +
> +static void test_qemu_strtoi_full_trailing(void)
> +{
> +    const char *str = "123xxx";
> +    int res;
> +    int err;
> +
> +    err = qemu_strtoi(str, NULL, 0, &res);
> +
> +    g_assert_cmpint(err, ==, -EINVAL);
> +}
> +
> +static void test_qemu_strtoi_full_max(void)
> +{
> +    char *str = g_strdup_printf("%d", INT_MAX);
> +    int res;
> +    int err;
> +
> +    err = qemu_strtoi(str, NULL, 0, &res);
> +
> +    g_assert_cmpint(err, ==, 0);
> +    g_assert_cmpint(res, ==, INT_MAX);
> +    g_free(str);
> +}
> +
> +static void test_qemu_strtoui_correct(void)
> +{
> +    const char *str = "12345 foo";
> +    char f = 'X';
> +    const char *endptr = &f;
> +    unsigned int res = 999;
> +    int err;
> +
> +    err = qemu_strtoui(str, &endptr, 0, &res);
> +
> +    g_assert_cmpint(err, ==, 0);
> +    g_assert_cmpuint(res, ==, 12345);
> +    g_assert(endptr == str + 5);
> +}
> +
> +static void test_qemu_strtoui_null(void)
> +{
> +    char f = 'X';
> +    const char *endptr = &f;
> +    unsigned int res = 999;
> +    int err;
> +
> +    err = qemu_strtoui(NULL, &endptr, 0, &res);
> +
> +    g_assert_cmpint(err, ==, -EINVAL);
> +    g_assert(endptr == NULL);
> +}
> +
> +static void test_qemu_strtoui_empty(void)
> +{
> +    const char *str = "";
> +    char f = 'X';
> +    const char *endptr = &f;
> +    unsigned int res = 999;
> +    int err;
> +
> +    err = qemu_strtoui(str, &endptr, 0, &res);
> +
> +    g_assert_cmpint(err, ==, -EINVAL);
> +    g_assert(endptr == str);
> +}
> +
> +static void test_qemu_strtoui_whitespace(void)
> +{
> +    const char *str = "  \t  ";
> +    char f = 'X';
> +    const char *endptr = &f;
> +    unsigned int res = 999;
> +    int err;
> +
> +    err = qemu_strtoui(str, &endptr, 0, &res);
> +
> +    g_assert_cmpint(err, ==, -EINVAL);
> +    g_assert(endptr == str);
> +}
> +
> +static void test_qemu_strtoui_invalid(void)
> +{
> +    const char *str = "   xxxx  \t abc";
> +    char f = 'X';
> +    const char *endptr = &f;
> +    unsigned int res = 999;
> +    int err;
> +
> +    err = qemu_strtoui(str, &endptr, 0, &res);
> +
> +    g_assert_cmpint(err, ==, -EINVAL);
> +    g_assert(endptr == str);
> +}
> +
> +static void test_qemu_strtoui_trailing(void)
> +{
> +    const char *str = "123xxx";
> +    char f = 'X';
> +    const char *endptr = &f;
> +    unsigned int res = 999;
> +    int err;
> +
> +    err = qemu_strtoui(str, &endptr, 0, &res);
> +
> +    g_assert_cmpint(err, ==, 0);
> +    g_assert_cmpuint(res, ==, 123);
> +    g_assert(endptr == str + 3);
> +}
> +
> +static void test_qemu_strtoui_octal(void)
> +{
> +    const char *str = "0123";
> +    char f = 'X';
> +    const char *endptr = &f;
> +    unsigned int res = 999;
> +    int err;
> +
> +    err = qemu_strtoui(str, &endptr, 8, &res);
> +
> +    g_assert_cmpint(err, ==, 0);
> +    g_assert_cmpuint(res, ==, 0123);
> +    g_assert(endptr == str + strlen(str));
> +
> +    res = 999;
> +    endptr = &f;
> +    err = qemu_strtoui(str, &endptr, 0, &res);
> +
> +    g_assert_cmpint(err, ==, 0);
> +    g_assert_cmpuint(res, ==, 0123);
> +    g_assert(endptr == str + strlen(str));
> +}
> +
> +static void test_qemu_strtoui_decimal(void)
> +{
> +    const char *str = "0123";
> +    char f = 'X';
> +    const char *endptr = &f;
> +    unsigned int res = 999;
> +    int err;
> +
> +    err = qemu_strtoui(str, &endptr, 10, &res);
> +
> +    g_assert_cmpint(err, ==, 0);
> +    g_assert_cmpuint(res, ==, 123);
> +    g_assert(endptr == str + strlen(str));
> +
> +    str = "123";
> +    res = 999;
> +    endptr = &f;
> +    err = qemu_strtoui(str, &endptr, 0, &res);
> +
> +    g_assert_cmpint(err, ==, 0);
> +    g_assert_cmpuint(res, ==, 123);
> +    g_assert(endptr == str + strlen(str));
> +}
> +
> +static void test_qemu_strtoui_hex(void)
> +{
> +    const char *str = "0123";
> +    char f = 'X';
> +    const char *endptr = &f;
> +    unsigned int res = 999;
> +    int err;
> +
> +    err = qemu_strtoui(str, &endptr, 16, &res);
> +
> +    g_assert_cmpint(err, ==, 0);
> +    g_assert_cmphex(res, ==, 0x123);
> +    g_assert(endptr == str + strlen(str));
> +
> +    str = "0x123";
> +    res = 999;
> +    endptr = &f;
> +    err = qemu_strtoui(str, &endptr, 0, &res);
> +
> +    g_assert_cmpint(err, ==, 0);
> +    g_assert_cmphex(res, ==, 0x123);
> +    g_assert(endptr == str + strlen(str));
> +}
> +
> +static void test_qemu_strtoui_max(void)
> +{
> +    char *str = g_strdup_printf("%u", UINT_MAX);
> +    char f = 'X';
> +    const char *endptr = &f;
> +    unsigned int res = 999;
> +    int err;
> +
> +    err = qemu_strtoui(str, &endptr, 0, &res);
> +
> +    g_assert_cmpint(err, ==, 0);
> +    g_assert_cmphex(res, ==, UINT_MAX);
> +    g_assert(endptr == str + strlen(str));
> +    g_free(str);
> +}
> +
> +static void test_qemu_strtoui_overflow(void)
> +{
> +    const char *str = "99999999999999999999999999999999999999999999";
> +    char f = 'X';
> +    const char *endptr = &f;
> +    unsigned int res = 999;
> +    int err;
> +
> +    err = qemu_strtoui(str, &endptr, 0, &res);
> +
> +    g_assert_cmpint(err, ==, -ERANGE);
> +    g_assert_cmphex(res, ==, UINT_MAX);
> +    g_assert(endptr == str + strlen(str));
> +}
> +
> +static void test_qemu_strtoui_underflow(void)
> +{
> +    const char *str = "-99999999999999999999999999999999999999999999";
> +    char f = 'X';
> +    const char *endptr = &f;
> +    unsigned int res = 999;
> +    int err;
> +
> +    err  = qemu_strtoui(str, &endptr, 0, &res);
> +
> +    g_assert_cmpint(err, ==, -ERANGE);
> +    g_assert_cmpuint(res, ==, (unsigned int)-1);
> +    g_assert(endptr == str + strlen(str));
> +}
> +
> +static void test_qemu_strtoui_negative(void)
> +{
> +    const char *str = "  \t -321";
> +    char f = 'X';
> +    const char *endptr = &f;
> +    unsigned int res = 999;
> +    int err;
> +
> +    err = qemu_strtoui(str, &endptr, 0, &res);
> +
> +    g_assert_cmpint(err, ==, 0);
> +    g_assert_cmpuint(res, ==, (unsigned int)-321);
> +    g_assert(endptr == str + strlen(str));
> +}
> +
> +static void test_qemu_strtoui_full_correct(void)
> +{
> +    const char *str = "123";
> +    unsigned int res = 999;
> +    int err;
> +
> +    err = qemu_strtoui(str, NULL, 0, &res);
> +
> +    g_assert_cmpint(err, ==, 0);
> +    g_assert_cmpuint(res, ==, 123);
> +}
> +
> +static void test_qemu_strtoui_full_null(void)
> +{
> +    unsigned int res = 999;
> +    int err;
> +
> +    err = qemu_strtoui(NULL, NULL, 0, &res);
> +
> +    g_assert_cmpint(err, ==, -EINVAL);
> +}
> +
> +static void test_qemu_strtoui_full_empty(void)
> +{
> +    const char *str = "";
> +    unsigned int res = 999;
> +    int err;
> +
> +    err = qemu_strtoui(str, NULL, 0, &res);
> +
> +    g_assert_cmpint(err, ==, -EINVAL);
> +}
> +static void test_qemu_strtoui_full_negative(void)
> +{
> +    const char *str = " \t -321";
> +    unsigned int res = 999;
> +    int err;
> +
> +    err = qemu_strtoui(str, NULL, 0, &res);
> +    g_assert_cmpint(err, ==, 0);
> +    g_assert_cmpuint(res, ==, (unsigned int)-321);
> +}
> +
> +static void test_qemu_strtoui_full_trailing(void)
> +{
> +    const char *str = "123xxx";
> +    unsigned int res;
> +    int err;
> +
> +    err = qemu_strtoui(str, NULL, 0, &res);
> +
> +    g_assert_cmpint(err, ==, -EINVAL);
> +}
> +
> +static void test_qemu_strtoui_full_max(void)
> +{
> +    char *str = g_strdup_printf("%u", UINT_MAX);
> +    unsigned int res = 999;
> +    int err;
> +
> +    err = qemu_strtoui(str, NULL, 0, &res);
> +
> +    g_assert_cmpint(err, ==, 0);
> +    g_assert_cmphex(res, ==, UINT_MAX);
> +    g_free(str);
> +}
> +
>  static void test_qemu_strtol_correct(void)
>  {
>      const char *str = "12345 foo";
> @@ -1612,6 +2185,86 @@ int main(int argc, char **argv)
>      g_test_add_func("/cutils/parse_uint_full/correct",
>                      test_parse_uint_full_correct);
>
> +    /* qemu_strtoi() tests */
> +    g_test_add_func("/cutils/qemu_strtoi/correct",
> +                    test_qemu_strtoi_correct);
> +    g_test_add_func("/cutils/qemu_strtoi/null",
> +                    test_qemu_strtoi_null);
> +    g_test_add_func("/cutils/qemu_strtoi/empty",
> +                    test_qemu_strtoi_empty);
> +    g_test_add_func("/cutils/qemu_strtoi/whitespace",
> +                    test_qemu_strtoi_whitespace);
> +    g_test_add_func("/cutils/qemu_strtoi/invalid",
> +                    test_qemu_strtoi_invalid);
> +    g_test_add_func("/cutils/qemu_strtoi/trailing",
> +                    test_qemu_strtoi_trailing);
> +    g_test_add_func("/cutils/qemu_strtoi/octal",
> +                    test_qemu_strtoi_octal);
> +    g_test_add_func("/cutils/qemu_strtoi/decimal",
> +                    test_qemu_strtoi_decimal);
> +    g_test_add_func("/cutils/qemu_strtoi/hex",
> +                    test_qemu_strtoi_hex);
> +    g_test_add_func("/cutils/qemu_strtoi/max",
> +                    test_qemu_strtoi_max);
> +    g_test_add_func("/cutils/qemu_strtoi/overflow",
> +                    test_qemu_strtoi_overflow);
> +    g_test_add_func("/cutils/qemu_strtoi/underflow",
> +                    test_qemu_strtoi_underflow);
> +    g_test_add_func("/cutils/qemu_strtoi/negative",
> +                    test_qemu_strtoi_negative);
> +    g_test_add_func("/cutils/qemu_strtoi_full/correct",
> +                    test_qemu_strtoi_full_correct);
> +    g_test_add_func("/cutils/qemu_strtoi_full/null",
> +                    test_qemu_strtoi_full_null);
> +    g_test_add_func("/cutils/qemu_strtoi_full/empty",
> +                    test_qemu_strtoi_full_empty);
> +    g_test_add_func("/cutils/qemu_strtoi_full/negative",
> +                    test_qemu_strtoi_full_negative);
> +    g_test_add_func("/cutils/qemu_strtoi_full/trailing",
> +                    test_qemu_strtoi_full_trailing);
> +    g_test_add_func("/cutils/qemu_strtoi_full/max",
> +                    test_qemu_strtoi_full_max);
> +
> +    /* qemu_strtoui() tests */
> +    g_test_add_func("/cutils/qemu_strtoui/correct",
> +                    test_qemu_strtoui_correct);
> +    g_test_add_func("/cutils/qemu_strtoui/null",
> +                    test_qemu_strtoui_null);
> +    g_test_add_func("/cutils/qemu_strtoui/empty",
> +                    test_qemu_strtoui_empty);
> +    g_test_add_func("/cutils/qemu_strtoui/whitespace",
> +                    test_qemu_strtoui_whitespace);
> +    g_test_add_func("/cutils/qemu_strtoui/invalid",
> +                    test_qemu_strtoui_invalid);
> +    g_test_add_func("/cutils/qemu_strtoui/trailing",
> +                    test_qemu_strtoui_trailing);
> +    g_test_add_func("/cutils/qemu_strtoui/octal",
> +                    test_qemu_strtoui_octal);
> +    g_test_add_func("/cutils/qemu_strtoui/decimal",
> +                    test_qemu_strtoui_decimal);
> +    g_test_add_func("/cutils/qemu_strtoui/hex",
> +                    test_qemu_strtoui_hex);
> +    g_test_add_func("/cutils/qemu_strtoui/max",
> +                    test_qemu_strtoui_max);
> +    g_test_add_func("/cutils/qemu_strtoui/overflow",
> +                    test_qemu_strtoui_overflow);
> +    g_test_add_func("/cutils/qemu_strtoui/underflow",
> +                    test_qemu_strtoui_underflow);
> +    g_test_add_func("/cutils/qemu_strtoui/negative",
> +                    test_qemu_strtoui_negative);
> +    g_test_add_func("/cutils/qemu_strtoui_full/correct",
> +                    test_qemu_strtoui_full_correct);
> +    g_test_add_func("/cutils/qemu_strtoui_full/null",
> +                    test_qemu_strtoui_full_null);
> +    g_test_add_func("/cutils/qemu_strtoui_full/empty",
> +                    test_qemu_strtoui_full_empty);
> +    g_test_add_func("/cutils/qemu_strtoui_full/negative",
> +                    test_qemu_strtoui_full_negative);
> +    g_test_add_func("/cutils/qemu_strtoui_full/trailing",
> +                    test_qemu_strtoui_full_trailing);
> +    g_test_add_func("/cutils/qemu_strtoui_full/max",
> +                    test_qemu_strtoui_full_max);
> +
>      /* qemu_strtol() tests */
>      g_test_add_func("/cutils/qemu_strtol/correct",
>                      test_qemu_strtol_correct);
> diff --git a/util/cutils.c b/util/cutils.c
> index b33ede83d1..9f05b331d9 100644
> --- a/util/cutils.c
> +++ b/util/cutils.c
> @@ -297,6 +297,110 @@ static int check_strtox_error(const char *nptr, char *ep,
>      return -libc_errno;
>  }
>
> +/**
> + * Convert string @nptr to a long integer, and store it in @result.
> + *
> + * This is a wrapper around strtol() that is harder to misuse.
> + * Semantics of @nptr, @endptr, @base match strtol() with differences
> + * noted below.
> + *
> + * @nptr may be null, and no conversion is performed then.
> + *
> + * If no conversion is performed, store @nptr in *@endptr and return
> + * -EINVAL.
> + *
> + * If @endptr is null, and the string isn't fully converted, return
> + * -EINVAL.  This is the case when the pointer that would be stored in
> + * a non-null @endptr points to a character other than '\0'.
> + *
> + * If the conversion overflows @result, store INT_MAX in @result,
> + * and return -ERANGE.
> + *
> + * If the conversion underflows @result, store INT_MIN in @result,
> + * and return -ERANGE.
> + *
> + * Else store the converted value in @result, and return zero.
> + */
> +int qemu_strtoi(const char *nptr, const char **endptr, int base,
> +                int *result)
> +{
> +    char *ep;
> +    long lresult;
> +
> +    if (!nptr) {
> +        if (endptr) {
> +            *endptr = nptr;
> +        }
> +        return -EINVAL;
> +    }
> +
> +    errno = 0;
> +    lresult = strtol(nptr, &ep, base);
> +    if (lresult < INT_MIN) {
> +        *result = INT_MIN;
> +    } else if (lresult > INT_MAX) {
> +        *result = INT_MAX;
> +    } else {
> +        *result = lresult;
> +    }
> +    return check_strtox_error(nptr, ep, endptr, errno);
> +}
> +
> +/**
> + * Convert string @nptr to an unsigned int, and store it in @result.
> + *
> + * This is a wrapper around strtoul() that is harder to misuse.
> + * Semantics of @nptr, @endptr, @base match strtoul() with differences
> + * noted below.
> + *
> + * @nptr may be null, and no conversion is performed then.
> + *
> + * If no conversion is performed, store @nptr in *@endptr and return
> + * -EINVAL.
> + *
> + * If @endptr is null, and the string isn't fully converted, return
> + * -EINVAL.  This is the case when the pointer that would be stored in
> + * a non-null @endptr points to a character other than '\0'.
> + *
> + * If the conversion overflows @result, store UINT_MAX in @result,
> + * and return -ERANGE.
> + *
> + * Else store the converted value in @result, and return zero.
> + *
> + * Note that a number with a leading minus sign gets converted without
> + * the minus sign, checked for overflow (see above), then negated (in
> + * @result's type).  This is exactly how strtoul() works.
> + */
> +int qemu_strtoui(const char *nptr, const char **endptr, int base,
> +                 unsigned int *result)
> +{
> +    char *ep;
> +    long lresult;
> +
> +    if (!nptr) {
> +        if (endptr) {
> +            *endptr = nptr;
> +        }
> +        return -EINVAL;
> +    }
> +
> +    errno = 0;
> +    lresult = strtol(nptr, &ep, base);
> +    /* Windows returns 1 for negative out-of-range values.  */
> +    if (errno == ERANGE) {
> +        *result = -1;
> +    } else {
> +        if (lresult > UINT_MAX) {
> +            *result = UINT_MAX;
> +        } else if (lresult < INT_MIN) {
> +            *result = UINT_MAX;
> +        } else {
> +            *result = lresult;
> +        }
> +    }
> +    return check_strtox_error(nptr, ep, endptr, errno);
> +}
> +
>  /**
>   * Convert string @nptr to a long integer, and store it in @result.
>   *
> --
> 2.14.3
>
>



-- 
Marc-André Lureau

Re: [Qemu-devel] [PATCH v3 5/8] sockets: check that the named file descriptor is a socket

[Marc-André Lureau]

On Fri, Dec 22, 2017 at 2:45 PM, Daniel P. Berrange <berrange@redhat.com> wrote:
> 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.
>
> Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
> ---
>  tests/.gitignore       |   1 +
>  tests/Makefile.include |   3 ++
>  tests/test-sockets.c   | 139 +++++++++++++++++++++++++++++++++++++++++++++++++
>  util/qemu-sockets.c    |  18 ++++++-
>  4 files changed, 159 insertions(+), 2 deletions(-)
>  create mode 100644 tests/test-sockets.c
>
> diff --git a/tests/.gitignore b/tests/.gitignore
> index 74e55d7264..44be8c2c79 100644
> --- a/tests/.gitignore
> +++ b/tests/.gitignore
> @@ -80,6 +80,7 @@ test-qobject-output-visitor
>  test-rcu-list
>  test-replication
>  test-shift128
> +test-sockets
>  test-string-input-visitor
>  test-string-output-visitor
>  test-thread-pool
> diff --git a/tests/Makefile.include b/tests/Makefile.include
> index bede832dc1..2730f7562b 100644
> --- a/tests/Makefile.include
> +++ b/tests/Makefile.include
> @@ -135,6 +135,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-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)
> @@ -694,6 +695,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-sockets$(EXESUF): tests/test-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)
> diff --git a/tests/test-sockets.c b/tests/test-sockets.c
> new file mode 100644
> index 0000000000..8a2962fe7b
> --- /dev/null
> +++ b/tests/test-sockets.c
> @@ -0,0 +1,139 @@
> +/*
> + * Test core sockets APIs
> + *
> + * 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/sockets.h"
> +#include "socket-helpers.h"
> +#include "qapi/error.h"
> +#include "monitor/monitor.h"
> +
> +
> +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) {}

The above is a bit of trick, but it works so:

Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>


> +
> +
> +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;
> +
> +    module_call_init(MODULE_INIT_QOM);
> +    socket_init();
> +
> +    g_test_init(&argc, &argv, NULL);
> +
> +    /* We're creating actual IPv4/6 sockets, so we should
> +     * check if the host running tests actually supports
> +     * each protocol to avoid breaking tests on machines
> +     * with either IPv4 or IPv6 disabled.
> +     */
> +    if (socket_check_protocol_support(&has_ipv4, &has_ipv6) < 0) {
> +        return 1;
> +    }
> +
> +    if (has_ipv4) {
> +        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();
> +}
> diff --git a/util/qemu-sockets.c b/util/qemu-sockets.c
> index 5e42f6d88d..6a7b194428 100644
> --- a/util/qemu-sockets.c
> +++ b/util/qemu-sockets.c
> @@ -1027,6 +1027,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;
> @@ -1041,7 +1055,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:
> @@ -1068,7 +1082,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:
> --
> 2.14.3
>
>



-- 
Marc-André Lureau

Re: [Qemu-devel] [PATCH v3 4/8] sockets: strengthen test suite IP protocol availability checks

[Marc-André Lureau]

On Fri, Dec 22, 2017 at 2:45 PM, Daniel P. Berrange <berrange@redhat.com> wrote:
> 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.
>
> Signed-off-by: Daniel P. Berrange <berrange@redhat.com>

Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>


> ---
>  tests/socket-helpers.c | 67 +++++++++++++++++++++++++++++++++++++++++++-------
>  tests/socket-helpers.h |  4 +--
>  2 files changed, 60 insertions(+), 11 deletions(-)
>
> diff --git a/tests/socket-helpers.c b/tests/socket-helpers.c
> index 13b6bb38eb..161aa5da09 100644
> --- a/tests/socket-helpers.c
> +++ b/tests/socket-helpers.c
> @@ -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;
>          }
> diff --git a/tests/socket-helpers.h b/tests/socket-helpers.h
> index efa96eddc2..1c07d6d656 100644
> --- a/tests/socket-helpers.h
> +++ b/tests/socket-helpers.h
> @@ -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
> --
> 2.14.3
>
>



-- 
Marc-André Lureau

Re: [Qemu-devel] [PATCH v3 6/8] sockets: allow SocketAddress 'fd' to reference numeric file descriptors

[Marc-André Lureau]

Hi

On Fri, Dec 22, 2017 at 2:45 PM, Daniel P. Berrange <berrange@redhat.com> wrote:
> 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.

That looks reasonable to me. If in the future we get rid of cur_mon
somehow, the code should be fairly simple to adapt, but I don't think
we need to worry about it now,

Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>


>
> Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
> ---
>  qapi/sockets.json    |   7 ++++
>  tests/test-sockets.c | 112 ++++++++++++++++++++++++++++++++++++++++++++++++---
>  util/qemu-sockets.c  |  16 ++++++--
>  3 files changed, 126 insertions(+), 9 deletions(-)
>
> diff --git a/qapi/sockets.json b/qapi/sockets.json
> index ac022c6ad0..fc81d8d5e8 100644
> --- a/qapi/sockets.json
> +++ b/qapi/sockets.json
> @@ -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',
> diff --git a/tests/test-sockets.c b/tests/test-sockets.c
> index 8a2962fe7b..8307e4774f 100644
> --- a/tests/test-sockets.c
> +++ b/tests/test-sockets.c
> @@ -49,7 +49,7 @@ Monitor *cur_mon;
>  void monitor_init(Chardev *chr, int flags) {}
>
>
> -static void test_socket_fd_pass_good(void)
> +static void test_socket_fd_pass_name_good(void)
>  {
>      SocketAddress addr;
>      int fd;
> @@ -80,7 +80,7 @@ static void test_socket_fd_pass_good(void)
>      cur_mon = NULL;
>  }
>
> -static void test_socket_fd_pass_bad(void)
> +static void test_socket_fd_pass_name_bad(void)
>  {
>      SocketAddress addr;
>      Error *err = NULL;
> @@ -110,6 +110,98 @@ static void test_socket_fd_pass_bad(void)
>      cur_mon = NULL;
>  }
>
> +static void test_socket_fd_pass_name_nomon(void)
> +{
> +    SocketAddress addr;
> +    Error *err = NULL;
> +    int fd;
> +
> +    g_assert(cur_mon == NULL);
> +
> +    addr.type = SOCKET_ADDRESS_TYPE_FD;
> +    addr.u.fd.str = g_strdup("myfd");
> +
> +    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);
> +}
> +
> +
> +static void test_socket_fd_pass_num_good(void)
> +{
> +    SocketAddress addr;
> +    int fd, sfd;
> +
> +    g_assert(cur_mon == NULL);
> +    sfd = qemu_socket(AF_INET, SOCK_STREAM, 0);
> +    g_assert_cmpint(sfd, >, STDERR_FILENO);
> +
> +    addr.type = SOCKET_ADDRESS_TYPE_FD;
> +    addr.u.fd.str = g_strdup_printf("%d", sfd);
> +
> +    fd = socket_connect(&addr, &error_abort);
> +    g_assert_cmpint(fd, ==, sfd);
> +
> +    fd = socket_listen(&addr, &error_abort);
> +    g_assert_cmpint(fd, ==, sfd);
> +
> +    g_free(addr.u.fd.str);
> +    close(sfd);
> +}
> +
> +static void test_socket_fd_pass_num_bad(void)
> +{
> +    SocketAddress addr;
> +    Error *err = NULL;
> +    int fd, sfd;
> +
> +    g_assert(cur_mon == NULL);
> +    sfd = dup(STDOUT_FILENO);
> +
> +    addr.type = SOCKET_ADDRESS_TYPE_FD;
> +    addr.u.fd.str = g_strdup_printf("%d", sfd);
> +
> +    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);
> +    close(sfd);
> +}
> +
> +static void test_socket_fd_pass_num_nocli(void)
> +{
> +    SocketAddress addr;
> +    Error *err = NULL;
> +    int fd;
> +
> +    cur_mon = g_malloc(1); /* Fake a monitor */
> +
> +    addr.type = SOCKET_ADDRESS_TYPE_FD;
> +    addr.u.fd.str = g_strdup_printf("%d", STDOUT_FILENO);
> +
> +    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);
> +}
> +
> +
>  int main(int argc, char **argv)
>  {
>      bool has_ipv4, has_ipv6;
> @@ -129,10 +221,18 @@ int main(int argc, char **argv)
>      }
>
>      if (has_ipv4) {
> -        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);
> +        g_test_add_func("/socket/fd-pass/name/good",
> +                        test_socket_fd_pass_name_good);
> +        g_test_add_func("/socket/fd-pass/name/bad",
> +                        test_socket_fd_pass_name_bad);
> +        g_test_add_func("/socket/fd-pass/name/nomon",
> +                        test_socket_fd_pass_name_nomon);
> +        g_test_add_func("/socket/fd-pass/num/good",
> +                        test_socket_fd_pass_num_good);
> +        g_test_add_func("/socket/fd-pass/num/bad",
> +                        test_socket_fd_pass_num_bad);
> +        g_test_add_func("/socket/fd-pass/num/nocli",
> +                        test_socket_fd_pass_num_nocli);
>      }
>
>      return g_test_run();
> diff --git a/util/qemu-sockets.c b/util/qemu-sockets.c
> index 6a7b194428..8ac4eed122 100644
> --- a/util/qemu-sockets.c
> +++ b/util/qemu-sockets.c
> @@ -1029,9 +1029,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);
> --
> 2.14.3
>
>



-- 
Marc-André Lureau

Re: [Qemu-devel] [PATCH v3 7/8] char: refactor parsing of socket address information

[Marc-André Lureau]

Hi

On Fri, Dec 22, 2017 at 2:45 PM, Daniel P. Berrange <berrange@redhat.com> wrote:
> To prepare for handling more address types, refactor the parsing of
> socket address information to make it more robust and extensible.
>
> Signed-off-by: Daniel P. Berrange <berrange@redhat.com>

Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>


>  chardev/char-socket.c | 22 ++++++++++++++--------
>  1 file changed, 14 insertions(+), 8 deletions(-)
>
> diff --git a/chardev/char-socket.c b/chardev/char-socket.c
> index 6013972f72..9113ea0169 100644
> --- a/chardev/char-socket.c
> +++ b/chardev/char-socket.c
> @@ -987,21 +987,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);
> @@ -1027,7 +1031,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) {
> @@ -1040,6 +1044,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;
>  }
> --
> 2.14.3
>
>



-- 
Marc-André Lureau