mbox series

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

Message ID 20191107142613.2379-1-robert.foley@linaro.org (mailing list archive)
Headers show
Series Make the qemu_logfile handle thread safe. | expand

Message

Robert Foley Nov. 7, 2019, 2:26 p.m. UTC
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(-)

Comments

no-reply@patchew.org Nov. 7, 2019, 2:46 p.m. UTC | #1
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
Alex Bennée Nov. 7, 2019, 4:35 p.m. UTC | #2
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