1. Patchew
  2. QEMU
  3. View series

[Qemu-devel] [PATCH v5 0/9] Enable passing pre-opened chardev socket FD

Daniel P. Berrangé posted 9 patches 6 years, 1 month ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20180312124939.20562-1-berrange@redhat.com
Test checkpatch failed
Test docker-build@min-glib passed
Test docker-mingw@fedora passed
Test docker-quick@centos6 passed
Test s390x passed
chardev/char-socket.c          |  34 ++-
chardev/char.c                 |   3 +
include/qemu/cutils.h          |   4 +
include/qemu/sockets.h         |   1 +
io/channel-util.c              |  13 -
qapi/sockets.json              |   7 +
tests/.gitignore               |   1 +
tests/Makefile.include         |   5 +-
tests/socket-helpers.c         | 153 ++++++++++
tests/socket-helpers.h         |  42 +++
tests/test-char.c              |  47 ++-
tests/test-cutils.c            | 657 +++++++++++++++++++++++++++++++++++++++++
tests/test-io-channel-socket.c |  72 +----
tests/test-util-sockets.c      | 266 +++++++++++++++++
util/cutils.c                  | 108 +++++++
util/qemu-sockets.c            |  36 ++-
16 files changed, 1351 insertions(+), 98 deletions(-)
create mode 100644 tests/socket-helpers.c
create mode 100644 tests/socket-helpers.h
create mode 100644 tests/test-util-sockets.c
[Qemu-devel] [PATCH v5 0/9] Enable passing pre-opened chardev socket FD
Posted by Daniel P. Berrangé 6 years, 1 month ago 
An update of:

  v1: https://lists.gnu.org/archive/html/qemu-devel/2017-12/msg04618.html
  v2: https://lists.gnu.org/archive/html/qemu-devel/2017-12/msg04706.html
  v3: https://lists.gnu.org/archive/html/qemu-devel/2017-12/msg04892.html
  v4: https://lists.gnu.org/archive/html/qemu-devel/2018-02/msg00950.html

This enables fixing a long standing problem that libvirt has with
starting up QEMU. It has to busy-wait retrying connect() on the QMP
monitor socket until QEMU finally creates & listens on it, but at same
time must be careful to not wait forever if QEMU exits.

With this patch series, libvirt can simply pass in a pre-opened UNIX domain
socket file descriptor, which it can immediately connect to with no busy-wait.

NB, this will generate one expected failure with patchew / checkpatch.pl

  ERROR: consider using qemu_strtol in preference to strtol
  #729: FILE: util/cutils.c:338:
  +    lresult = strtol(nptr, &ep, base);

  ERROR: consider using qemu_strtol in preference to strtol
  #779: FILE: util/cutils.c:388:
  +    lresult = strtol(nptr, &ep, base);

This is ok to ignore, because the patch in question is introducing new
qemu_strtoXXX wrappers

Changed in v5:

  - Docs typo (Eric)
  - Fix errno setting in int parsing code (Eric)

Changed in v4:

  - Add test for fd_is_socket() API (Marc-Andre)

Changed in v3:

  - Introduce  qemu_strtoi & qemu_stroui functions.

  - Split patchs up into more pieces to better separate each logical
    change

  - Introduce a new test/test-sockets.c to directly test the
    SocketAddress FD handling, separately from chardev code.

  - Add qapi docs for FD passing syntax

  - Other misc fixes in tests

  - Reduce code duplication when getting pre-opened FDs in
    socket_connect/listen.

Changed in v2:

  - Drop 'fdset' property / address kind, and use 'fd' for both CLI and HMP
  - Add unit tests

Daniel P. Berrangé (9):
  char: don't silently skip tn3270 protocol init when TLS is enabled
  cutils: add qemu_strtoi & qemu_strtoui parsers for int/unsigned int
    types
  sockets: pull code for testing IP availability out of specific test
  sockets: strengthen test suite IP protocol availability checks
  sockets: move fd_is_socket() into common sockets code
  sockets: check that the named file descriptor is a socket
  sockets: allow SocketAddress 'fd' to reference numeric file
    descriptors
  char: refactor parsing of socket address information
  char: allow passing pre-opened socket file descriptor at startup

 chardev/char-socket.c          |  34 ++-
 chardev/char.c                 |   3 +
 include/qemu/cutils.h          |   4 +
 include/qemu/sockets.h         |   1 +
 io/channel-util.c              |  13 -
 qapi/sockets.json              |   7 +
 tests/.gitignore               |   1 +
 tests/Makefile.include         |   5 +-
 tests/socket-helpers.c         | 153 ++++++++++
 tests/socket-helpers.h         |  42 +++
 tests/test-char.c              |  47 ++-
 tests/test-cutils.c            | 657 +++++++++++++++++++++++++++++++++++++++++
 tests/test-io-channel-socket.c |  72 +----
 tests/test-util-sockets.c      | 266 +++++++++++++++++
 util/cutils.c                  | 108 +++++++
 util/qemu-sockets.c            |  36 ++-
 16 files changed, 1351 insertions(+), 98 deletions(-)
 create mode 100644 tests/socket-helpers.c
 create mode 100644 tests/socket-helpers.h
 create mode 100644 tests/test-util-sockets.c

-- 
2.14.3
Re: [Qemu-devel] [PATCH v5 0/9] Enable passing pre-opened chardev socket FD
Posted by Eric Blake 6 years, 1 month ago 
On 03/12/2018 07:49 AM, Daniel P. Berrangé wrote:
> An update of:
> 
>    v1: https://lists.gnu.org/archive/html/qemu-devel/2017-12/msg04618.html
>    v2: https://lists.gnu.org/archive/html/qemu-devel/2017-12/msg04706.html
>    v3: https://lists.gnu.org/archive/html/qemu-devel/2017-12/msg04892.html
>    v4: https://lists.gnu.org/archive/html/qemu-devel/2018-02/msg00950.html
> 
> This enables fixing a long standing problem that libvirt has with
> starting up QEMU. It has to busy-wait retrying connect() on the QMP
> monitor socket until QEMU finally creates & listens on it, but at same
> time must be careful to not wait forever if QEMU exits.
> 
> With this patch series, libvirt can simply pass in a pre-opened UNIX domain
> socket file descriptor, which it can immediately connect to with no busy-wait.
> 
> NB, this will generate one expected failure with patchew / checkpatch.pl

> 
> Changed in v5:
> 
>    - Docs typo (Eric)
>    - Fix errno setting in int parsing code (Eric)
> 

> Daniel P. Berrangé (9):
>    char: don't silently skip tn3270 protocol init when TLS is enabled
>    cutils: add qemu_strtoi & qemu_strtoui parsers for int/unsigned int
>      types
>    sockets: pull code for testing IP availability out of specific test
>    sockets: strengthen test suite IP protocol availability checks
>    sockets: move fd_is_socket() into common sockets code
>    sockets: check that the named file descriptor is a socket
>    sockets: allow SocketAddress 'fd' to reference numeric file
>      descriptors
>    char: refactor parsing of socket address information
>    char: allow passing pre-opened socket file descriptor at startup

Whose tree should this go through?  It's got some QAPI impact, so I'm 
okay including it in my QAPI pull request later today, if there is no 
better tree...

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org
Re: [Qemu-devel] [PATCH v5 0/9] Enable passing pre-opened chardev socket FD
Posted by Daniel P. Berrangé 6 years, 1 month ago 
On Mon, Mar 12, 2018 at 08:12:44AM -0500, Eric Blake wrote:
> On 03/12/2018 07:49 AM, Daniel P. Berrangé wrote:
> > An update of:
> > 
> >    v1: https://lists.gnu.org/archive/html/qemu-devel/2017-12/msg04618.html
> >    v2: https://lists.gnu.org/archive/html/qemu-devel/2017-12/msg04706.html
> >    v3: https://lists.gnu.org/archive/html/qemu-devel/2017-12/msg04892.html
> >    v4: https://lists.gnu.org/archive/html/qemu-devel/2018-02/msg00950.html
> > 
> > This enables fixing a long standing problem that libvirt has with
> > starting up QEMU. It has to busy-wait retrying connect() on the QMP
> > monitor socket until QEMU finally creates & listens on it, but at same
> > time must be careful to not wait forever if QEMU exits.
> > 
> > With this patch series, libvirt can simply pass in a pre-opened UNIX domain
> > socket file descriptor, which it can immediately connect to with no busy-wait.
> > 
> > NB, this will generate one expected failure with patchew / checkpatch.pl
> 
> > 
> > Changed in v5:
> > 
> >    - Docs typo (Eric)
> >    - Fix errno setting in int parsing code (Eric)
> > 
> 
> > Daniel P. Berrangé (9):
> >    char: don't silently skip tn3270 protocol init when TLS is enabled
> >    cutils: add qemu_strtoi & qemu_strtoui parsers for int/unsigned int
> >      types
> >    sockets: pull code for testing IP availability out of specific test
> >    sockets: strengthen test suite IP protocol availability checks
> >    sockets: move fd_is_socket() into common sockets code
> >    sockets: check that the named file descriptor is a socket
> >    sockets: allow SocketAddress 'fd' to reference numeric file
> >      descriptors
> >    char: refactor parsing of socket address information
> >    char: allow passing pre-opened socket file descriptor at startup
> 
> Whose tree should this go through?  It's got some QAPI impact, so I'm okay
> including it in my QAPI pull request later today, if there is no better
> tree...

These days I usually send PR for stuff touching sockets code myself. So
unless someone else strongly prefers to take it via their tree for sake
of any easy of conflict / merge resolution, I can do a PR once acked.

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|
Re: [Qemu-devel] [PATCH v5 0/9] Enable passing pre-opened chardev socket FD
Posted by Eric Blake 6 years, 1 month ago 
On 03/12/2018 08:14 AM, Daniel P. Berrangé wrote:
> On Mon, Mar 12, 2018 at 08:12:44AM -0500, Eric Blake wrote:
>> On 03/12/2018 07:49 AM, Daniel P. Berrangé wrote:
>>> An update of:
>>>
>>>     v1: https://lists.gnu.org/archive/html/qemu-devel/2017-12/msg04618.html
>>>     v2: https://lists.gnu.org/archive/html/qemu-devel/2017-12/msg04706.html
>>>     v3: https://lists.gnu.org/archive/html/qemu-devel/2017-12/msg04892.html
>>>     v4: https://lists.gnu.org/archive/html/qemu-devel/2018-02/msg00950.html
>>>
>>> This enables fixing a long standing problem that libvirt has with
>>> starting up QEMU. It has to busy-wait retrying connect() on the QMP
>>> monitor socket until QEMU finally creates & listens on it, but at same
>>> time must be careful to not wait forever if QEMU exits.
>>>
>>> With this patch series, libvirt can simply pass in a pre-opened UNIX domain
>>> socket file descriptor, which it can immediately connect to with no busy-wait.
>>>

>> Whose tree should this go through?  It's got some QAPI impact, so I'm okay
>> including it in my QAPI pull request later today, if there is no better
>> tree...
> 
> These days I usually send PR for stuff touching sockets code myself. So
> unless someone else strongly prefers to take it via their tree for sake
> of any easy of conflict / merge resolution, I can do a PR once acked.

Okay, I'll go through one more round of reviews, then let you send the 
PR; we still have time to make softfreeze tomorrow.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org
Re: [Qemu-devel] [PATCH v5 0/9] Enable passing pre-opened chardev socket FD
Posted by no-reply@patchew.org 6 years, 1 month ago 
Hi,

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

Type: series
Message-id: 20180312124939.20562-1-berrange@redhat.com
Subject: [Qemu-devel] [PATCH v5 0/9] Enable passing pre-opened chardev socket FD

=== TEST SCRIPT BEGIN ===
#!/bin/bash

BASE=base
n=1
total=$(git log --oneline $BASE.. | wc -l)
failed=0

git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram

commits="$(git log --format=%H --reverse $BASE..)"
for c in $commits; do
    echo "Checking PATCH $n/$total: $(git log -n 1 --format=%s $c)..."
    if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; then
        failed=1
        echo
    fi
    n=$((n+1))
done

exit $failed
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
Switched to a new branch 'test'
11a2d24854 char: allow passing pre-opened socket file descriptor at startup
38d7ed855a char: refactor parsing of socket address information
dacc6f8ebe sockets: allow SocketAddress 'fd' to reference numeric file descriptors
8d91d776c7 sockets: check that the named file descriptor is a socket
4ac9d04d7b sockets: move fd_is_socket() into common sockets code
179118052a sockets: strengthen test suite IP protocol availability checks
357edea8d3 sockets: pull code for testing IP availability out of specific test
7fca025ea8 cutils: add qemu_strtoi & qemu_strtoui parsers for int/unsigned int types
68d51518fb char: don't silently skip tn3270 protocol init when TLS is enabled

=== OUTPUT BEGIN ===
Checking PATCH 1/9: char: don't silently skip tn3270 protocol init when TLS is enabled...
Checking PATCH 2/9: cutils: add qemu_strtoi & qemu_strtoui parsers for int/unsigned int types...
ERROR: consider using qemu_strtol in preference to strtol
#753: FILE: util/cutils.c:338:
+    lresult = strtol(nptr, &ep, base);

ERROR: consider using qemu_strtoul in preference to strtoul
#805: FILE: util/cutils.c:390:
+    lresult = strtoul(nptr, &ep, base);

total: 2 errors, 0 warnings, 793 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

Checking PATCH 3/9: sockets: pull code for testing IP availability out of specific test...
Checking PATCH 4/9: sockets: strengthen test suite IP protocol availability checks...
Checking PATCH 5/9: sockets: move fd_is_socket() into common sockets code...
Checking PATCH 6/9: sockets: check that the named file descriptor is a socket...
Checking PATCH 7/9: sockets: allow SocketAddress 'fd' to reference numeric file descriptors...
Checking PATCH 8/9: char: refactor parsing of socket address information...
Checking PATCH 9/9: char: allow passing pre-opened socket file descriptor at startup...
=== OUTPUT END ===

Test command exited with code: 1


---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-devel@freelists.org
Re: [Qemu-devel] [PATCH v5 0/9] Enable passing pre-opened chardev socket FD
Posted by Eric Blake 6 years, 1 month ago 
On 03/12/2018 07:49 AM, Daniel P. Berrangé wrote:
> An update of:
> 
>    v1: https://lists.gnu.org/archive/html/qemu-devel/2017-12/msg04618.html
>    v2: https://lists.gnu.org/archive/html/qemu-devel/2017-12/msg04706.html
>    v3: https://lists.gnu.org/archive/html/qemu-devel/2017-12/msg04892.html
>    v4: https://lists.gnu.org/archive/html/qemu-devel/2018-02/msg00950.html
> 
> This enables fixing a long standing problem that libvirt has with
> starting up QEMU. It has to busy-wait retrying connect() on the QMP
> monitor socket until QEMU finally creates & listens on it, but at same
> time must be careful to not wait forever if QEMU exits.
> 
> With this patch series, libvirt can simply pass in a pre-opened UNIX domain
> socket file descriptor, which it can immediately connect to with no busy-wait.
> 
> NB, this will generate one expected failure with patchew / checkpatch.pl
> 
>    ERROR: consider using qemu_strtol in preference to strtol
>    #729: FILE: util/cutils.c:338:
>    +    lresult = strtol(nptr, &ep, base);
> 
>    ERROR: consider using qemu_strtol in preference to strtol
>    #779: FILE: util/cutils.c:388:
>    +    lresult = strtol(nptr, &ep, base);
> 
> This is ok to ignore, because the patch in question is introducing new
> qemu_strtoXXX wrappers
> 
> Changed in v5:
> 
>    - Docs typo (Eric)
>    - Fix errno setting in int parsing code (Eric)

Series:
Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

Patchew 2.1-devel-0870f84

© 2016 - 2024 Red Hat, Inc.