1. Patchew
  2. QEMU
  3. View series

[Qemu-devel] [PATCH v3 0/3] add bdrv_co_drain_begin/end BlockDriver callbacks

Manos Pitsidianakis posted 3 patches 6 years, 7 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20170923111411.18626-1-el13635@mail.ntua.gr
Test checkpatch passed
Test docker passed
Test s390x passed
include/block/block_int.h | 13 ++++++++++---
block/io.c                | 48 +++++++++++++++++++++++++++++++++--------------
block/qed.c               |  6 +++---
block/throttle.c          | 18 ++++++++++++++++++
4 files changed, 65 insertions(+), 20 deletions(-)
[Qemu-devel] [PATCH v3 0/3] add bdrv_co_drain_begin/end BlockDriver callbacks
Posted by Manos Pitsidianakis 6 years, 7 months ago 
This patch series renames bdrv_co_drain to bdrv_co_drain_begin and adds a new 
bdrv_co_drain_end callback to match bdrv_drained_begin/end and 
drained_begin/end of BdrvChild. This is needed because the throttle driver 
(block/throttle.c) needs a way to mark the end of the drain in order to toggle 
io_limits_disabled correctly.

Based-on: <20170918202529.28379-1-el13635@mail.ntua.gr>
    "block/throttle-groups.c: allocate RestartData on the heap"
    Which fixes a coroutine crash in block/throttle-groups.c

v3:
  fixed commit message typo in first patch [Fam]
  rephrased doc comment based on mailing discussion
v2: 
  add doc for callbacks and change order of request polling for completion 
  [Stefan]

Manos Pitsidianakis (3):
  block: add bdrv_co_drain_end callback
  block: rename bdrv_co_drain to bdrv_co_drain_begin
  block/throttle.c: add bdrv_co_drain_begin/end callbacks

 include/block/block_int.h | 13 ++++++++++---
 block/io.c                | 48 +++++++++++++++++++++++++++++++++--------------
 block/qed.c               |  6 +++---
 block/throttle.c          | 18 ++++++++++++++++++
 4 files changed, 65 insertions(+), 20 deletions(-)

-- 
2.11.0
Re: [Qemu-devel] [PATCH v3 0/3] add bdrv_co_drain_begin/end BlockDriver callbacks
Posted by Stefan Hajnoczi 6 years, 7 months ago 
On Sat, Sep 23, 2017 at 02:14:08PM +0300, Manos Pitsidianakis wrote:
> This patch series renames bdrv_co_drain to bdrv_co_drain_begin and adds a new 
> bdrv_co_drain_end callback to match bdrv_drained_begin/end and 
> drained_begin/end of BdrvChild. This is needed because the throttle driver 
> (block/throttle.c) needs a way to mark the end of the drain in order to toggle 
> io_limits_disabled correctly.
> 
> Based-on: <20170918202529.28379-1-el13635@mail.ntua.gr>
>     "block/throttle-groups.c: allocate RestartData on the heap"
>     Which fixes a coroutine crash in block/throttle-groups.c
> 
> v3:
>   fixed commit message typo in first patch [Fam]
>   rephrased doc comment based on mailing discussion
> v2: 
>   add doc for callbacks and change order of request polling for completion 
>   [Stefan]
> 
> Manos Pitsidianakis (3):
>   block: add bdrv_co_drain_end callback
>   block: rename bdrv_co_drain to bdrv_co_drain_begin
>   block/throttle.c: add bdrv_co_drain_begin/end callbacks
> 
>  include/block/block_int.h | 13 ++++++++++---
>  block/io.c                | 48 +++++++++++++++++++++++++++++++++--------------
>  block/qed.c               |  6 +++---
>  block/throttle.c          | 18 ++++++++++++++++++
>  4 files changed, 65 insertions(+), 20 deletions(-)
> 
> -- 
> 2.11.0
> 

Thanks, applied to my block tree:
https://github.com/stefanha/qemu/commits/block

Stefan
Re: [Qemu-devel] [PATCH v3 0/3] add bdrv_co_drain_begin/end BlockDriver callbacks
Posted by Stefan Hajnoczi 6 years, 7 months ago 
On Sat, Sep 23, 2017 at 02:14:08PM +0300, Manos Pitsidianakis wrote:
> This patch series renames bdrv_co_drain to bdrv_co_drain_begin and adds a new 
> bdrv_co_drain_end callback to match bdrv_drained_begin/end and 
> drained_begin/end of BdrvChild. This is needed because the throttle driver 
> (block/throttle.c) needs a way to mark the end of the drain in order to toggle 
> io_limits_disabled correctly.
> 
> Based-on: <20170918202529.28379-1-el13635@mail.ntua.gr>
>     "block/throttle-groups.c: allocate RestartData on the heap"
>     Which fixes a coroutine crash in block/throttle-groups.c
> 
> v3:
>   fixed commit message typo in first patch [Fam]
>   rephrased doc comment based on mailing discussion
> v2: 
>   add doc for callbacks and change order of request polling for completion 
>   [Stefan]
> 
> Manos Pitsidianakis (3):
>   block: add bdrv_co_drain_end callback
>   block: rename bdrv_co_drain to bdrv_co_drain_begin
>   block/throttle.c: add bdrv_co_drain_begin/end callbacks
> 
>  include/block/block_int.h | 13 ++++++++++---
>  block/io.c                | 48 +++++++++++++++++++++++++++++++++--------------
>  block/qed.c               |  6 +++---
>  block/throttle.c          | 18 ++++++++++++++++++
>  4 files changed, 65 insertions(+), 20 deletions(-)

Oops, this seems to cause a qemu-iotests failure.  Please take a look:

$ ./check -qcow2 184
184 0s ... - output mismatch (see 184.out.bad)
--- /home/stefanha/qemu/tests/qemu-iotests/184.out	2017-09-19 14:51:46.673854437 +0100
+++ 184.out.bad	2017-09-26 11:13:06.946610239 +0100
@@ -142,6 +142,9 @@
         "guest": false
     }
 }
+./common.config: line 118:  9196 Segmentation fault      (core dumped) ( if [ -n "${QEMU_NEED_PID}" ]; then
+    echo $BASHPID > "${QEMU_TEST_DIR}/qemu-${_QEMU_HANDLE}.pid";
+fi; exec "$QEMU_PROG" $QEMU_OPTIONS "$@" )
Re: [Qemu-devel] [PATCH v3 0/3] add bdrv_co_drain_begin/end BlockDriver callbacks
Posted by Manos Pitsidianakis 6 years, 7 months ago 
On Tue, Sep 26, 2017 at 12:00:24PM +0100, Stefan Hajnoczi wrote:
>On Sat, Sep 23, 2017 at 02:14:08PM +0300, Manos Pitsidianakis wrote:
>> This patch series renames bdrv_co_drain to bdrv_co_drain_begin and adds a new
>> bdrv_co_drain_end callback to match bdrv_drained_begin/end and
>> drained_begin/end of BdrvChild. This is needed because the throttle driver
>> (block/throttle.c) needs a way to mark the end of the drain in order to toggle
>> io_limits_disabled correctly.
>>
>> Based-on: <20170918202529.28379-1-el13635@mail.ntua.gr>
>>     "block/throttle-groups.c: allocate RestartData on the heap"
>>     Which fixes a coroutine crash in block/throttle-groups.c
>>
>> v3:
>>   fixed commit message typo in first patch [Fam]
>>   rephrased doc comment based on mailing discussion
>> v2:
>>   add doc for callbacks and change order of request polling for completion
>>   [Stefan]
>>
>> Manos Pitsidianakis (3):
>>   block: add bdrv_co_drain_end callback
>>   block: rename bdrv_co_drain to bdrv_co_drain_begin
>>   block/throttle.c: add bdrv_co_drain_begin/end callbacks
>>
>>  include/block/block_int.h | 13 ++++++++++---
>>  block/io.c                | 48 +++++++++++++++++++++++++++++++++--------------
>>  block/qed.c               |  6 +++---
>>  block/throttle.c          | 18 ++++++++++++++++++
>>  4 files changed, 65 insertions(+), 20 deletions(-)
>
>Oops, this seems to cause a qemu-iotests failure.  Please take a look:
>
>$ ./check -qcow2 184
>184 0s ... - output mismatch (see 184.out.bad)
>--- /home/stefanha/qemu/tests/qemu-iotests/184.out	2017-09-19 14:51:46.673854437 +0100
>+++ 184.out.bad	2017-09-26 11:13:06.946610239 +0100
>@@ -142,6 +142,9 @@
>         "guest": false
>     }
> }
>+./common.config: line 118:  9196 Segmentation fault      (core dumped) ( if [ -n "${QEMU_NEED_PID}" ]; then
>+    echo $BASHPID > "${QEMU_TEST_DIR}/qemu-${_QEMU_HANDLE}.pid";
>+fi; exec "$QEMU_PROG" $QEMU_OPTIONS "$@" )
>

Hey Stefan,

This is fixed in 
>> Based-on: <20170918202529.28379-1-el13635@mail.ntua.gr>
>>     "block/throttle-groups.c: allocate RestartData on the heap"
>>     Which fixes a coroutine crash in block/throttle-groups.c

Which I sent before this series and is in Kevin's branch.
Re: [Qemu-devel] [PATCH v3 0/3] add bdrv_co_drain_begin/end BlockDriver callbacks
Posted by Kevin Wolf 6 years, 6 months ago 
Am 26.09.2017 um 14:44 hat Manos Pitsidianakis geschrieben:
> On Tue, Sep 26, 2017 at 12:00:24PM +0100, Stefan Hajnoczi wrote:
> > On Sat, Sep 23, 2017 at 02:14:08PM +0300, Manos Pitsidianakis wrote:
> > > This patch series renames bdrv_co_drain to bdrv_co_drain_begin and adds a new
> > > bdrv_co_drain_end callback to match bdrv_drained_begin/end and
> > > drained_begin/end of BdrvChild. This is needed because the throttle driver
> > > (block/throttle.c) needs a way to mark the end of the drain in order to toggle
> > > io_limits_disabled correctly.
> > > 
> > > Based-on: <20170918202529.28379-1-el13635@mail.ntua.gr>
> > >     "block/throttle-groups.c: allocate RestartData on the heap"
> > >     Which fixes a coroutine crash in block/throttle-groups.c
> > > 
> > > v3:
> > >   fixed commit message typo in first patch [Fam]
> > >   rephrased doc comment based on mailing discussion
> > > v2:
> > >   add doc for callbacks and change order of request polling for completion
> > >   [Stefan]
> > > 
> > > Manos Pitsidianakis (3):
> > >   block: add bdrv_co_drain_end callback
> > >   block: rename bdrv_co_drain to bdrv_co_drain_begin
> > >   block/throttle.c: add bdrv_co_drain_begin/end callbacks
> > > 
> > >  include/block/block_int.h | 13 ++++++++++---
> > >  block/io.c                | 48 +++++++++++++++++++++++++++++++++--------------
> > >  block/qed.c               |  6 +++---
> > >  block/throttle.c          | 18 ++++++++++++++++++
> > >  4 files changed, 65 insertions(+), 20 deletions(-)
> > 
> > Oops, this seems to cause a qemu-iotests failure.  Please take a look:
> > 
> > $ ./check -qcow2 184
> > 184 0s ... - output mismatch (see 184.out.bad)
> > --- /home/stefanha/qemu/tests/qemu-iotests/184.out	2017-09-19 14:51:46.673854437 +0100
> > +++ 184.out.bad	2017-09-26 11:13:06.946610239 +0100
> > @@ -142,6 +142,9 @@
> >         "guest": false
> >     }
> > }
> > +./common.config: line 118:  9196 Segmentation fault      (core dumped) ( if [ -n "${QEMU_NEED_PID}" ]; then
> > +    echo $BASHPID > "${QEMU_TEST_DIR}/qemu-${_QEMU_HANDLE}.pid";
> > +fi; exec "$QEMU_PROG" $QEMU_OPTIONS "$@" )
> > 
> 
> Hey Stefan,
> 
> This is fixed in
> > > Based-on: <20170918202529.28379-1-el13635@mail.ntua.gr>
> > >     "block/throttle-groups.c: allocate RestartData on the heap"
> > >     Which fixes a coroutine crash in block/throttle-groups.c
> 
> Which I sent before this series and is in Kevin's branch.

Stefan, the fix is in master by now, so you can stage this series again.
(Or I can take it, if you prefer.)

Kevin
Re: [Qemu-devel] [Qemu-block] [PATCH v3 0/3] add bdrv_co_drain_begin/end BlockDriver callbacks
Posted by Stefan Hajnoczi 6 years, 6 months ago 
On Sat, Sep 23, 2017 at 02:14:08PM +0300, Manos Pitsidianakis wrote:
> This patch series renames bdrv_co_drain to bdrv_co_drain_begin and adds a new 
> bdrv_co_drain_end callback to match bdrv_drained_begin/end and 
> drained_begin/end of BdrvChild. This is needed because the throttle driver 
> (block/throttle.c) needs a way to mark the end of the drain in order to toggle 
> io_limits_disabled correctly.
> 
> Based-on: <20170918202529.28379-1-el13635@mail.ntua.gr>
>     "block/throttle-groups.c: allocate RestartData on the heap"
>     Which fixes a coroutine crash in block/throttle-groups.c
> 
> v3:
>   fixed commit message typo in first patch [Fam]
>   rephrased doc comment based on mailing discussion
> v2: 
>   add doc for callbacks and change order of request polling for completion 
>   [Stefan]
> 
> Manos Pitsidianakis (3):
>   block: add bdrv_co_drain_end callback
>   block: rename bdrv_co_drain to bdrv_co_drain_begin
>   block/throttle.c: add bdrv_co_drain_begin/end callbacks
> 
>  include/block/block_int.h | 13 ++++++++++---
>  block/io.c                | 48 +++++++++++++++++++++++++++++++++--------------
>  block/qed.c               |  6 +++---
>  block/throttle.c          | 18 ++++++++++++++++++
>  4 files changed, 65 insertions(+), 20 deletions(-)
> 
> -- 
> 2.11.0
> 
> 

Thanks, applied to my block tree:
https://github.com/stefanha/qemu/commits/block

Stefan

Patchew 2.1-devel-0870f84

© 2016 - 2024 Red Hat, Inc.