mbox series

[v2,0/7] Call check and invalidate_cache from coroutine context

Message ID 1516188312-29612-1-git-send-email-pbonzini@redhat.com
Headers show
Series Call check and invalidate_cache from coroutine context | expand

Message

Paolo Bonzini Jan. 17, 2018, 11:25 a.m. UTC 
Check and invalidate_cache share some parts of the implementation
with the regular I/O path.  This is sometimes complicated because the
I/O path wants to use a CoMutex but that is not possible outside coroutine
context.  By moving things to coroutine context, we can remove special
cases.  In fact, invalidate_cache is already called from coroutine context
because incoming migration is placed in a coroutine.

While at it, I'm including two patches from Stefan to rename the
bdrv_create callback to bdrv_co_create, because it is already called
from coroutine context.

qcow2 still has cache flushing in non-coroutine context, coming from
qcow2_reopen_prepare->qcow2_update_options_prepare and
qcow2_close->qcow2_inactivate.

Paolo

v1->v2: fix file-win32

Paolo Bonzini (5):
  qcow2: make qcow2_do_open a coroutine_fn
  qed: make bdrv_qed_do_open a coroutine_fn
  block: convert bdrv_invalidate_cache callback to coroutine_fn
  qcow2: introduce qcow2_write_caches and qcow2_flush_caches
  block: convert bdrv_check callback to coroutine_fn

Stefan Hajnoczi (2):
  block: rename .bdrv_create() to .bdrv_co_create()
  qcow2: make qcow2_co_create2() a coroutine_fn

 block.c                   |  88 ++++++++++++++++++++++++++++++----
 block/crypto.c            |   8 ++--
 block/file-posix.c        |  15 +++---
 block/file-win32.c        |   5 +-
 block/gluster.c           |  12 ++---
 block/iscsi.c             |  13 ++---
 block/nfs.c               |  11 +++--
 block/parallels.c         |  23 +++++----
 block/qcow.c              |   5 +-
 block/qcow2-refcount.c    |  28 +++++++++++
 block/qcow2.c             | 118 +++++++++++++++++++++++++++++++---------------
 block/qcow2.h             |   2 +
 block/qed-check.c         |   1 +
 block/qed-table.c         |  26 ++++------
 block/qed.c               |  72 +++++++++++++++++++++-------
 block/raw-format.c        |   5 +-
 block/rbd.c               |  12 +++--
 block/sheepdog.c          |  10 ++--
 block/ssh.c               |   5 +-
 block/vdi.c               |  11 +++--
 block/vhdx.c              |  12 +++--
 block/vmdk.c              |  12 +++--
 block/vpc.c               |   5 +-
 include/block/block_int.h |  11 +++--
 24 files changed, 355 insertions(+), 155 deletions(-)

Comments

Kevin Wolf Jan. 17, 2018, 12:59 p.m. UTC | #1 
Am 17.01.2018 um 12:25 hat Paolo Bonzini geschrieben:
> Check and invalidate_cache share some parts of the implementation
> with the regular I/O path.  This is sometimes complicated because the
> I/O path wants to use a CoMutex but that is not possible outside coroutine
> context.  By moving things to coroutine context, we can remove special
> cases.  In fact, invalidate_cache is already called from coroutine context
> because incoming migration is placed in a coroutine.
> 
> While at it, I'm including two patches from Stefan to rename the
> bdrv_create callback to bdrv_co_create, because it is already called
> from coroutine context.

I'm using .bdrv_co_create for the QAPI type based function that will be
used from QMP blockdev-create, so it would be good if we leave the
legacy function with its old name or at least choose a different new
name for it.

Kevin
Paolo Bonzini Jan. 17, 2018, 1:15 p.m. UTC | #2 
On 17/01/2018 13:59, Kevin Wolf wrote:
> Am 17.01.2018 um 12:25 hat Paolo Bonzini geschrieben:
>> Check and invalidate_cache share some parts of the implementation
>> with the regular I/O path.  This is sometimes complicated because the
>> I/O path wants to use a CoMutex but that is not possible outside coroutine
>> context.  By moving things to coroutine context, we can remove special
>> cases.  In fact, invalidate_cache is already called from coroutine context
>> because incoming migration is placed in a coroutine.
>>
>> While at it, I'm including two patches from Stefan to rename the
>> bdrv_create callback to bdrv_co_create, because it is already called
>> from coroutine context.
> I'm using .bdrv_co_create for the QAPI type based function that will be
> used from QMP blockdev-create, so it would be good if we leave the
> legacy function with its old name or at least choose a different new
> name for it.

.bdrv_co_create_opts since it takes QemuOpts*?

Thanks,

Paolo
Kevin Wolf Jan. 17, 2018, 1:33 p.m. UTC | #3 
Am 17.01.2018 um 14:15 hat Paolo Bonzini geschrieben:
> On 17/01/2018 13:59, Kevin Wolf wrote:
> > Am 17.01.2018 um 12:25 hat Paolo Bonzini geschrieben:
> >> Check and invalidate_cache share some parts of the implementation
> >> with the regular I/O path.  This is sometimes complicated because the
> >> I/O path wants to use a CoMutex but that is not possible outside coroutine
> >> context.  By moving things to coroutine context, we can remove special
> >> cases.  In fact, invalidate_cache is already called from coroutine context
> >> because incoming migration is placed in a coroutine.
> >>
> >> While at it, I'm including two patches from Stefan to rename the
> >> bdrv_create callback to bdrv_co_create, because it is already called
> >> from coroutine context.
> > I'm using .bdrv_co_create for the QAPI type based function that will be
> > used from QMP blockdev-create, so it would be good if we leave the
> > legacy function with its old name or at least choose a different new
> > name for it.
> 
> .bdrv_co_create_opts since it takes QemuOpts*?

Sounds good enough to me.

Kevin