[PATCH 0/4] Make the qemu_logfile handle thread safe.

Robert Foley posted 4 patches 4 years, 5 months ago
Test asan failed
Test checkpatch passed
Test FreeBSD passed
Test docker-mingw@fedora passed
Test docker-clang@ubuntu failed
Test docker-quick@centos7 failed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20191107142613.2379-1-robert.foley@linaro.org
Maintainers: Guan Xuetao <gxt@mprc.pku.edu.cn>, Richard Henderson <rth@twiddle.net>, Marek Vasut <marex@denx.de>, Chris Wulff <crwulff@gmail.com>, Michael Walle <michael@walle.cc>, "Edgar E. Iglesias" <edgar.iglesias@gmail.com>, Paolo Bonzini <pbonzini@redhat.com>, Eduardo Habkost <ehabkost@redhat.com>, Jason Wang <jasowang@redhat.com>
There is a newer version of this series
accel/tcg/cpu-exec.c          |  4 +-
accel/tcg/translate-all.c     |  4 +-
accel/tcg/translator.c        |  4 +-
exec.c                        |  4 +-
hw/net/can/can_sja1000.c      |  4 +-
include/exec/log.h            | 33 ++++++++++--
include/qemu/log.h            | 51 +++++++++++++++---
net/can/can_socketcan.c       |  5 +-
target/cris/translate.c       |  4 +-
target/i386/translate.c       |  5 +-
target/lm32/translate.c       |  4 +-
target/microblaze/translate.c |  4 +-
target/nios2/translate.c      |  4 +-
target/tilegx/translate.c     |  7 +--
target/unicore32/translate.c  |  4 +-
tcg/tcg.c                     | 28 ++++++----
tests/test-logging.c          | 74 ++++++++++++++++++++++++++
util/log.c                    | 99 ++++++++++++++++++++++++++++-------
18 files changed, 273 insertions(+), 69 deletions(-)
[PATCH 0/4] Make the qemu_logfile handle thread safe.
Posted by Robert Foley 4 years, 5 months ago
This patch adds thread safety to the qemu_logfile handle.  This now
allows changing the logfile while logging is active, and also solves 
the issue of a seg fault while changing the logfile.

This patch adds use of RCU for handling the swap out of the 
old qemu_logfile file descriptor.

Robert Foley (4):
  Add a mutex to guarantee single writer to qemu_logfile handle.
  Add use of RCU for qemu_logfile.
  qemu_log_lock/unlock now preserves the qemu_logfile handle.
  Added tests for close and change of logfile.

 accel/tcg/cpu-exec.c          |  4 +-
 accel/tcg/translate-all.c     |  4 +-
 accel/tcg/translator.c        |  4 +-
 exec.c                        |  4 +-
 hw/net/can/can_sja1000.c      |  4 +-
 include/exec/log.h            | 33 ++++++++++--
 include/qemu/log.h            | 51 +++++++++++++++---
 net/can/can_socketcan.c       |  5 +-
 target/cris/translate.c       |  4 +-
 target/i386/translate.c       |  5 +-
 target/lm32/translate.c       |  4 +-
 target/microblaze/translate.c |  4 +-
 target/nios2/translate.c      |  4 +-
 target/tilegx/translate.c     |  7 +--
 target/unicore32/translate.c  |  4 +-
 tcg/tcg.c                     | 28 ++++++----
 tests/test-logging.c          | 74 ++++++++++++++++++++++++++
 util/log.c                    | 99 ++++++++++++++++++++++++++++-------
 18 files changed, 273 insertions(+), 69 deletions(-)

-- 
2.17.1


Re: [PATCH 0/4] Make the qemu_logfile handle thread safe.
Posted by no-reply@patchew.org 4 years, 5 months ago
Patchew URL: https://patchew.org/QEMU/20191107142613.2379-1-robert.foley@linaro.org/



Hi,

This series failed the docker-quick@centos7 build test. Please find the testing commands and
their output below. If you have Docker installed, you can probably reproduce it
locally.

=== TEST SCRIPT BEGIN ===
#!/bin/bash
make docker-image-centos7 V=1 NETWORK=1
time make docker-test-quick@centos7 SHOW_ENV=1 J=14 NETWORK=1
=== TEST SCRIPT END ===

  TEST    check-unit: tests/test-hbitmap
  TEST    check-unit: tests/test-bdrv-drain
test-bdrv-drain: /tmp/qemu-test/src/util/async.c:283: aio_ctx_finalize: Assertion `!qemu_lockcnt_count(&ctx->list_lock)' failed.
ERROR - too few tests run (expected 42, got 17)
make: *** [check-unit] Error 1
make: *** Waiting for unfinished jobs....
  TEST    iotest-qcow2: 013
  TEST    iotest-qcow2: 017
---
    raise CalledProcessError(retcode, cmd)
subprocess.CalledProcessError: Command '['sudo', '-n', 'docker', 'run', '--label', 'com.qemu.instance.uuid=eaa57e3449fb433ebde0eceb9d05b6c2', '-u', '1003', '--security-opt', 'seccomp=unconfined', '--rm', '-e', 'TARGET_LIST=', '-e', 'EXTRA_CONFIGURE_OPTS=', '-e', 'V=', '-e', 'J=14', '-e', 'DEBUG=', '-e', 'SHOW_ENV=1', '-e', 'CCACHE_DIR=/var/tmp/ccache', '-v', '/home/patchew2/.cache/qemu-docker-ccache:/var/tmp/ccache:z', '-v', '/var/tmp/patchew-tester-tmp-xw43l1zq/src/docker-src.2019-11-07-09.34.09.25341:/var/tmp/qemu:z,ro', 'qemu:centos7', '/var/tmp/qemu/run', 'test-quick']' returned non-zero exit status 2.
filter=--filter=label=com.qemu.instance.uuid=eaa57e3449fb433ebde0eceb9d05b6c2
make[1]: *** [docker-run] Error 1
make[1]: Leaving directory `/var/tmp/patchew-tester-tmp-xw43l1zq/src'
make: *** [docker-run-test-quick@centos7] Error 2

real    12m39.833s
user    0m8.137s


The full log is available at
http://patchew.org/logs/20191107142613.2379-1-robert.foley@linaro.org/testing.docker-quick@centos7/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com
Re: [PATCH 0/4] Make the qemu_logfile handle thread safe.
Posted by Alex Bennée 4 years, 5 months ago
Robert Foley <robert.foley@linaro.org> writes:

> This patch adds thread safety to the qemu_logfile handle.  This now
> allows changing the logfile while logging is active, and also solves
> the issue of a seg fault while changing the logfile.
>
> This patch adds use of RCU for handling the swap out of the
> old qemu_logfile file descriptor.

I've finished my pass. Looks pretty good - a few minor comments around
the persistence of the read lock and some minor stylistic nits.

>
> Robert Foley (4):
>   Add a mutex to guarantee single writer to qemu_logfile handle.
>   Add use of RCU for qemu_logfile.
>   qemu_log_lock/unlock now preserves the qemu_logfile handle.
>   Added tests for close and change of logfile.
>
>  accel/tcg/cpu-exec.c          |  4 +-
>  accel/tcg/translate-all.c     |  4 +-
>  accel/tcg/translator.c        |  4 +-
>  exec.c                        |  4 +-
>  hw/net/can/can_sja1000.c      |  4 +-
>  include/exec/log.h            | 33 ++++++++++--
>  include/qemu/log.h            | 51 +++++++++++++++---
>  net/can/can_socketcan.c       |  5 +-
>  target/cris/translate.c       |  4 +-
>  target/i386/translate.c       |  5 +-
>  target/lm32/translate.c       |  4 +-
>  target/microblaze/translate.c |  4 +-
>  target/nios2/translate.c      |  4 +-
>  target/tilegx/translate.c     |  7 +--
>  target/unicore32/translate.c  |  4 +-
>  tcg/tcg.c                     | 28 ++++++----
>  tests/test-logging.c          | 74 ++++++++++++++++++++++++++
>  util/log.c                    | 99 ++++++++++++++++++++++++++++-------
>  18 files changed, 273 insertions(+), 69 deletions(-)


--
Alex Bennée