[Qemu-devel] [PULL 00/22] Docker and block patches

Fam Zheng posted 22 patches 6 years, 10 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20170526075246.20265-1-famz@redhat.com
Test checkpatch failed
Test docker passed
Test s390x passed
There is a newer version of this series
block.c                                 |   9 +-
block/accounting.c                      |  65 ++++++-----
block/block-backend.c                   |   5 +-
block/dirty-bitmap.c                    | 114 +++++++++++++++++--
block/io.c                              |  51 +++++----
block/mirror.c                          |  14 ++-
block/nfs.c                             |   4 +-
block/qapi.c                            |   2 +-
block/sheepdog.c                        |   3 +-
block/throttle-groups.c                 |  91 +++++++++++----
blockdev.c                              |  46 ++------
include/block/accounting.h              |   8 +-
include/block/block.h                   |   5 +-
include/block/block_int.h               |  65 +++++++----
include/block/dirty-bitmap.h            |  25 +++--
include/qemu/stats64.h                  | 193 ++++++++++++++++++++++++++++++++
include/sysemu/block-backend.h          |  10 +-
migration/block.c                       |  17 ++-
tests/docker/Makefile.include           |   2 +-
tests/docker/dockerfiles/centos6.docker |   2 +-
tests/docker/dockerfiles/fedora.docker  |   4 +-
util/Makefile.objs                      |   1 +
util/stats64.c                          | 136 ++++++++++++++++++++++
23 files changed, 687 insertions(+), 185 deletions(-)
create mode 100644 include/qemu/stats64.h
create mode 100644 util/stats64.c
[Qemu-devel] [PULL 00/22] Docker and block patches
Posted by Fam Zheng 6 years, 10 months ago
The following changes since commit 9964e96dc9999cf7f7c936ee854a795415d19b60:

  Merge remote-tracking branch 'jasowang/tags/net-pull-request' into staging (2017-05-23 15:01:31 +0100)

are available in the git repository at:

  git://github.com/famz/qemu.git tags/docker-and-block-pull-request

for you to fetch changes up to 77269bba94ef97de99ae61fdc98629a8704ae2ed:

  block: make accounting thread-safe (2017-05-26 09:25:30 +0800)

----------------------------------------------------------------

For Paolo's block layer thread safety part I and my docker testing
enhancements.

----------------------------------------------------------------

Fam Zheng (4):
  docker: Run tests with current user
  docker: Add bzip2 and hostname to fedora image
  docker: Add libaio to fedora image
  docker: Add flex and bison to centos6 image

Paolo Bonzini (18):
  block: access copy_on_read with atomic ops
  block: access quiesce_counter with atomic ops
  block: access io_limits_disabled with atomic ops
  block: access serialising_in_flight with atomic ops
  block: access wakeup with atomic ops
  block: access io_plugged with atomic ops
  throttle-groups: only start one coroutine from drained_begin
  throttle-groups: do not use qemu_co_enter_next
  throttle-groups: protect throttled requests with a CoMutex
  util: add stats64 module
  block: use Stat64 for wr_highest_offset
  block: access write_gen with atomics
  block: protect tracked_requests and flush_queue with reqs_lock
  block: introduce dirty_bitmap_mutex
  migration/block: reset dirty bitmap before reading
  block: protect modification of dirty bitmaps with a mutex
  block: introduce block_account_one_io
  block: make accounting thread-safe

 block.c                                 |   9 +-
 block/accounting.c                      |  65 ++++++-----
 block/block-backend.c                   |   5 +-
 block/dirty-bitmap.c                    | 114 +++++++++++++++++--
 block/io.c                              |  51 +++++----
 block/mirror.c                          |  14 ++-
 block/nfs.c                             |   4 +-
 block/qapi.c                            |   2 +-
 block/sheepdog.c                        |   3 +-
 block/throttle-groups.c                 |  91 +++++++++++----
 blockdev.c                              |  46 ++------
 include/block/accounting.h              |   8 +-
 include/block/block.h                   |   5 +-
 include/block/block_int.h               |  65 +++++++----
 include/block/dirty-bitmap.h            |  25 +++--
 include/qemu/stats64.h                  | 193 ++++++++++++++++++++++++++++++++
 include/sysemu/block-backend.h          |  10 +-
 migration/block.c                       |  17 ++-
 tests/docker/Makefile.include           |   2 +-
 tests/docker/dockerfiles/centos6.docker |   2 +-
 tests/docker/dockerfiles/fedora.docker  |   4 +-
 util/Makefile.objs                      |   1 +
 util/stats64.c                          | 136 ++++++++++++++++++++++
 23 files changed, 687 insertions(+), 185 deletions(-)
 create mode 100644 include/qemu/stats64.h
 create mode 100644 util/stats64.c

-- 
2.9.4


Re: [Qemu-devel] [PULL 00/22] Docker and block patches
Posted by no-reply@patchew.org 6 years, 10 months ago
Hi,

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

Type: series
Message-id: 20170526075246.20265-1-famz@redhat.com
Subject: [Qemu-devel] [PULL 00/22] Docker and block patches

=== 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
 * [new tag]         patchew/20170526082925.2888-1-maozy.fnst@cn.fujitsu.com -> patchew/20170526082925.2888-1-maozy.fnst@cn.fujitsu.com
Switched to a new branch 'test'
16cb74a block: make accounting thread-safe
8239635 block: introduce block_account_one_io
63bcf13 block: protect modification of dirty bitmaps with a mutex
f58097c migration/block: reset dirty bitmap before reading
ed83d5e block: introduce dirty_bitmap_mutex
f046d70 block: protect tracked_requests and flush_queue with reqs_lock
a7745ae block: access write_gen with atomics
1541938 block: use Stat64 for wr_highest_offset
6ebccfc util: add stats64 module
d0f217d throttle-groups: protect throttled requests with a CoMutex
0e06e5a throttle-groups: do not use qemu_co_enter_next
31d6860 throttle-groups: only start one coroutine from drained_begin
68c0e16 block: access io_plugged with atomic ops
621c393 block: access wakeup with atomic ops
c1c29f6 block: access serialising_in_flight with atomic ops
87f6654 block: access io_limits_disabled with atomic ops
16e5db1 block: access quiesce_counter with atomic ops
018cb8b block: access copy_on_read with atomic ops
f57b4b1 docker: Add flex and bison to centos6 image
6ac5045 docker: Add libaio to fedora image
7a2103e docker: Add bzip2 and hostname to fedora image
7e83232 docker: Run tests with current user

=== OUTPUT BEGIN ===
Checking PATCH 1/22: docker: Run tests with current user...
Checking PATCH 2/22: docker: Add bzip2 and hostname to fedora image...
Checking PATCH 3/22: docker: Add libaio to fedora image...
Checking PATCH 4/22: docker: Add flex and bison to centos6 image...
Checking PATCH 5/22: block: access copy_on_read with atomic ops...
Checking PATCH 6/22: block: access quiesce_counter with atomic ops...
Checking PATCH 7/22: block: access io_limits_disabled with atomic ops...
Checking PATCH 8/22: block: access serialising_in_flight with atomic ops...
Checking PATCH 9/22: block: access wakeup with atomic ops...
Checking PATCH 10/22: block: access io_plugged with atomic ops...
Checking PATCH 11/22: throttle-groups: only start one coroutine from drained_begin...
Checking PATCH 12/22: throttle-groups: do not use qemu_co_enter_next...
Checking PATCH 13/22: throttle-groups: protect throttled requests with a CoMutex...
Checking PATCH 14/22: util: add stats64 module...
ERROR: memory barrier without comment
#330: FILE: util/stats64.c:101:
+        smp_wmb();

ERROR: memory barrier without comment
#359: FILE: util/stats64.c:130:
+        smp_wmb();

total: 2 errors, 0 warnings, 334 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 15/22: block: use Stat64 for wr_highest_offset...
Checking PATCH 16/22: block: access write_gen with atomics...
Checking PATCH 17/22: block: protect tracked_requests and flush_queue with reqs_lock...
Checking PATCH 18/22: block: introduce dirty_bitmap_mutex...
Checking PATCH 19/22: migration/block: reset dirty bitmap before reading...
Checking PATCH 20/22: block: protect modification of dirty bitmaps with a mutex...
WARNING: line over 80 characters
#306: FILE: migration/block.c:539:
+            bdrv_reset_dirty_bitmap_locked(bmds->dirty_bitmap, sector, nr_sectors);

total: 0 errors, 1 warnings, 272 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 21/22: block: introduce block_account_one_io...
Checking PATCH 22/22: block: make accounting thread-safe...
=== 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] [PULL 00/22] Docker and block patches
Posted by Stefan Hajnoczi 6 years, 10 months ago
On Fri, May 26, 2017 at 03:52:24PM +0800, Fam Zheng wrote:
> The following changes since commit 9964e96dc9999cf7f7c936ee854a795415d19b60:
> 
>   Merge remote-tracking branch 'jasowang/tags/net-pull-request' into staging (2017-05-23 15:01:31 +0100)
> 
> are available in the git repository at:
> 
>   git://github.com/famz/qemu.git tags/docker-and-block-pull-request
> 
> for you to fetch changes up to 77269bba94ef97de99ae61fdc98629a8704ae2ed:
> 
>   block: make accounting thread-safe (2017-05-26 09:25:30 +0800)
> 
> ----------------------------------------------------------------
> 
> For Paolo's block layer thread safety part I and my docker testing
> enhancements.

Please fix the checkpatch issues.
Re: [Qemu-devel] [PULL 00/22] Docker and block patches
Posted by Fam Zheng 6 years, 10 months ago
On Tue, 05/30 10:19, Stefan Hajnoczi wrote:
> On Fri, May 26, 2017 at 03:52:24PM +0800, Fam Zheng wrote:
> > The following changes since commit 9964e96dc9999cf7f7c936ee854a795415d19b60:
> > 
> >   Merge remote-tracking branch 'jasowang/tags/net-pull-request' into staging (2017-05-23 15:01:31 +0100)
> > 
> > are available in the git repository at:
> > 
> >   git://github.com/famz/qemu.git tags/docker-and-block-pull-request
> > 
> > for you to fetch changes up to 77269bba94ef97de99ae61fdc98629a8704ae2ed:
> > 
> >   block: make accounting thread-safe (2017-05-26 09:25:30 +0800)
> > 
> > ----------------------------------------------------------------
> > 
> > For Paolo's block layer thread safety part I and my docker testing
> > enhancements.
> 
> Please fix the checkpatch issues.

Paolo, could you provide the comments that can be added to the memory barriers?

Fam

Re: [Qemu-devel] [PULL 00/22] Docker and block patches
Posted by Paolo Bonzini 6 years, 10 months ago

On 30/05/2017 11:33, Fam Zheng wrote:
> On Tue, 05/30 10:19, Stefan Hajnoczi wrote:
>> On Fri, May 26, 2017 at 03:52:24PM +0800, Fam Zheng wrote:
>>> The following changes since commit 9964e96dc9999cf7f7c936ee854a795415d19b60:
>>>
>>>   Merge remote-tracking branch 'jasowang/tags/net-pull-request' into staging (2017-05-23 15:01:31 +0100)
>>>
>>> are available in the git repository at:
>>>
>>>   git://github.com/famz/qemu.git tags/docker-and-block-pull-request
>>>
>>> for you to fetch changes up to 77269bba94ef97de99ae61fdc98629a8704ae2ed:
>>>
>>>   block: make accounting thread-safe (2017-05-26 09:25:30 +0800)
>>>
>>> ----------------------------------------------------------------
>>>
>>> For Paolo's block layer thread safety part I and my docker testing
>>> enhancements.
>>
>> Please fix the checkpatch issues.
> 
> Paolo, could you provide the comments that can be added to the memory barriers?

It's a false positive.  The comments are just a couple lines above:

        /* We have to set low before high, just like stat64_max reads
         * high before low.  The value may become lower temporarily, but
         * stat64_get does not notice (it takes the lock) and the only ill
         * effect on stat64_max is that the slow path may be triggered
         * unnecessarily.
         */
        atomic_set(&s->low, (uint32_t)value);
        smp_wmb();
        atomic_set(&s->high, value >> 32);

Paolo

Re: [Qemu-devel] [PULL 00/22] Docker and block patches
Posted by Fam Zheng 6 years, 10 months ago
On Tue, 05/30 11:36, Paolo Bonzini wrote:
> 
> 
> On 30/05/2017 11:33, Fam Zheng wrote:
> > On Tue, 05/30 10:19, Stefan Hajnoczi wrote:
> >> On Fri, May 26, 2017 at 03:52:24PM +0800, Fam Zheng wrote:
> >>> The following changes since commit 9964e96dc9999cf7f7c936ee854a795415d19b60:
> >>>
> >>>   Merge remote-tracking branch 'jasowang/tags/net-pull-request' into staging (2017-05-23 15:01:31 +0100)
> >>>
> >>> are available in the git repository at:
> >>>
> >>>   git://github.com/famz/qemu.git tags/docker-and-block-pull-request
> >>>
> >>> for you to fetch changes up to 77269bba94ef97de99ae61fdc98629a8704ae2ed:
> >>>
> >>>   block: make accounting thread-safe (2017-05-26 09:25:30 +0800)
> >>>
> >>> ----------------------------------------------------------------
> >>>
> >>> For Paolo's block layer thread safety part I and my docker testing
> >>> enhancements.
> >>
> >> Please fix the checkpatch issues.
> > 
> > Paolo, could you provide the comments that can be added to the memory barriers?
> 
> It's a false positive.  The comments are just a couple lines above:
> 
>         /* We have to set low before high, just like stat64_max reads
>          * high before low.  The value may become lower temporarily, but
>          * stat64_get does not notice (it takes the lock) and the only ill
>          * effect on stat64_max is that the slow path may be triggered
>          * unnecessarily.
>          */
>         atomic_set(&s->low, (uint32_t)value);
>         smp_wmb();
>         atomic_set(&s->high, value >> 32);

Ah okay, thanks!

Fam

Re: [Qemu-devel] [PULL 00/22] Docker and block patches
Posted by Fam Zheng 6 years, 10 months ago
On Fri, 05/26 15:52, Fam Zheng wrote:
> The following changes since commit 9964e96dc9999cf7f7c936ee854a795415d19b60:
> 
>   Merge remote-tracking branch 'jasowang/tags/net-pull-request' into staging (2017-05-23 15:01:31 +0100)
> 
> are available in the git repository at:
> 
>   git://github.com/famz/qemu.git tags/docker-and-block-pull-request
> 
> for you to fetch changes up to 77269bba94ef97de99ae61fdc98629a8704ae2ed:
> 
>   block: make accounting thread-safe (2017-05-26 09:25:30 +0800)
> 
> ----------------------------------------------------------------
> 
> For Paolo's block layer thread safety part I and my docker testing
> enhancements.

Just in case this falls through the cracks - the checkpatch.pl error is false
positive and this pull request is ready to go.

Fam

Re: [Qemu-devel] [PULL 00/22] Docker and block patches
Posted by Peter Maydell 6 years, 10 months ago
On 26 May 2017 at 08:52, Fam Zheng <famz@redhat.com> wrote:
> The following changes since commit 9964e96dc9999cf7f7c936ee854a795415d19b60:
>
>   Merge remote-tracking branch 'jasowang/tags/net-pull-request' into staging (2017-05-23 15:01:31 +0100)
>
> are available in the git repository at:
>
>   git://github.com/famz/qemu.git tags/docker-and-block-pull-request
>
> for you to fetch changes up to 77269bba94ef97de99ae61fdc98629a8704ae2ed:
>
>   block: make accounting thread-safe (2017-05-26 09:25:30 +0800)
>
> ----------------------------------------------------------------
>
> For Paolo's block layer thread safety part I and my docker testing
> enhancements.
>
> ----------------------------------------------------------------

Hi. I'm afraid this doesn't build on BSD or OSX:

libqemuutil.a(stats64.o): In function `stat64_rdlock':
/root/qemu/util/stats64.c:24: undefined reference to `cpu_relax'
libqemuutil.a(stats64.o): In funclibqemuutil.a(stats64.o): In function
`stat64_rdlock':
/root/qemu/util/stats64.c:24: undefined reference to `cpu_relax'
libqemuutil.a(stats64.o): In function `stat64_add32_carry':
/root/qemu/util/stats64.c:64: undefined reference to `cpu_relax'
libqemuutil.a(stats64.o): In function `stat64_min_slow':
/root/qemu/util/stats64.c:85: undefined reference to `cpu_relax'
libqemuutil.a(stats64.o): In function `stat64_max_slow':
/root/qemu/util/stats64.c:114: undefined reference to `cpu_relax'
tilibqemuutil.a(stats64.o): In function `stat64_rdlock':
/root/qemu/util/stats64.c:24: undefined reference to `cpu_relax'
libqemuutil.a(stats64.o): In function `stat64_add32_carry':
/root/qemu/util/stats64.c:64: undefined reference to `cpu_relax'
libqemuutil.a(stats64.o): In function `stat64_min_slow':
/root/qemu/util/stats64.c:85: undefined reference to `cpu_relax'
libqemuutil.a(stats64.o): In function `stat64_max_slow':
/root/qemu/util/stats64.c:114: undefined reference to `cpu_relax'
on `stat64_add32_carry':
/root/qemu/util/stats64.c:64: undefined reference to `cpu_relax'
libqemuutil.a(stats64.o): In function `stat64_min_slow':
/root/qemu/util/stats64.c:85: undefined reference to `cpu_relax'
libqemuutil.a(stats64.o): In function `stat64_max_slow':
/root/qemu/util/stats64.c:114: undefined reference to `cpu_relax'


and
/Users/pm215/src/qemu-for-merges/util/stats64.c:24:9: error: implicit
declaration of function 'cpu_relax' is invalid in C99
[-Werror,-Wimplicit-function-declaration]
        cpu_relax();
        ^


Looks like an omitted include of qemu/processor.h ?

I'm not sure why this compiles on Linux...

thanks
-- PMM

Re: [Qemu-devel] [PULL 00/22] Docker and block patches
Posted by Fam Zheng 6 years, 10 months ago
On Thu, 06/01 18:18, Peter Maydell wrote:
> On 26 May 2017 at 08:52, Fam Zheng <famz@redhat.com> wrote:
> > The following changes since commit 9964e96dc9999cf7f7c936ee854a795415d19b60:
> >
> >   Merge remote-tracking branch 'jasowang/tags/net-pull-request' into staging (2017-05-23 15:01:31 +0100)
> >
> > are available in the git repository at:
> >
> >   git://github.com/famz/qemu.git tags/docker-and-block-pull-request
> >
> > for you to fetch changes up to 77269bba94ef97de99ae61fdc98629a8704ae2ed:
> >
> >   block: make accounting thread-safe (2017-05-26 09:25:30 +0800)
> >
> > ----------------------------------------------------------------
> >
> > For Paolo's block layer thread safety part I and my docker testing
> > enhancements.
> >
> > ----------------------------------------------------------------
> 
> Hi. I'm afraid this doesn't build on BSD or OSX:
> 
> libqemuutil.a(stats64.o): In function `stat64_rdlock':
> /root/qemu/util/stats64.c:24: undefined reference to `cpu_relax'
> libqemuutil.a(stats64.o): In funclibqemuutil.a(stats64.o): In function
> `stat64_rdlock':
> /root/qemu/util/stats64.c:24: undefined reference to `cpu_relax'
> libqemuutil.a(stats64.o): In function `stat64_add32_carry':
> /root/qemu/util/stats64.c:64: undefined reference to `cpu_relax'
> libqemuutil.a(stats64.o): In function `stat64_min_slow':
> /root/qemu/util/stats64.c:85: undefined reference to `cpu_relax'
> libqemuutil.a(stats64.o): In function `stat64_max_slow':
> /root/qemu/util/stats64.c:114: undefined reference to `cpu_relax'
> tilibqemuutil.a(stats64.o): In function `stat64_rdlock':
> /root/qemu/util/stats64.c:24: undefined reference to `cpu_relax'
> libqemuutil.a(stats64.o): In function `stat64_add32_carry':
> /root/qemu/util/stats64.c:64: undefined reference to `cpu_relax'
> libqemuutil.a(stats64.o): In function `stat64_min_slow':
> /root/qemu/util/stats64.c:85: undefined reference to `cpu_relax'
> libqemuutil.a(stats64.o): In function `stat64_max_slow':
> /root/qemu/util/stats64.c:114: undefined reference to `cpu_relax'
> on `stat64_add32_carry':
> /root/qemu/util/stats64.c:64: undefined reference to `cpu_relax'
> libqemuutil.a(stats64.o): In function `stat64_min_slow':
> /root/qemu/util/stats64.c:85: undefined reference to `cpu_relax'
> libqemuutil.a(stats64.o): In function `stat64_max_slow':
> /root/qemu/util/stats64.c:114: undefined reference to `cpu_relax'
> 
> 
> and
> /Users/pm215/src/qemu-for-merges/util/stats64.c:24:9: error: implicit
> declaration of function 'cpu_relax' is invalid in C99
> [-Werror,-Wimplicit-function-declaration]
>         cpu_relax();
>         ^
> 
> 
> Looks like an omitted include of qemu/processor.h ?

Yes, including this one works for me, I'll add it and send v2. Thanks,

Fam