[v3] block: local qiov helper

[Qemu-devel] [PATCH v3 00/17] block: local qiov helper

Posted by Vladimir Sementsov-Ogievskiy 6 years, 9 months ago

Hi all!

Here is a new simple helper for a very often patter
around qemu_iovec_init_external, when we need simple qiov with only
one iov, initialized from external buffer.

v3:
  01-02: tiny improvements, described in patch-emails
  03-17: new patches

  Note: only hw/scsi/scsi-disk.c not updated, as it has too tricky
        logic around @iov fields of structures. So, it is simpler to
        keep it as is.

Previous series version was "[PATCH v2 0/2] block: local qiov helper: part I"
https://lists.gnu.org/archive/html/qemu-devel/2019-02/msg01610.html

Vladimir Sementsov-Ogievskiy (17):
  block: enhance QEMUIOVector structure
  block/io: use qemu_iovec_init_buf
  block/block-backend: use QEMU_IOVEC_INIT_BUF
  block/backup: use qemu_iovec_init_buf
  block/commit: use QEMU_IOVEC_INIT_BUF
  block/stream: use QEMU_IOVEC_INIT_BUF
  block/parallels: use QEMU_IOVEC_INIT_BUF
  block/qcow: use qemu_iovec_init_buf
  block/qcow2: use qemu_iovec_init_buf
  block/qed: use qemu_iovec_init_buf
  block/vmdk: use qemu_iovec_init_buf
  qemu-img: use qemu_iovec_init_buf
  migration/block: use qemu_iovec_init_buf
  tests/test-bdrv-drain: use QEMU_IOVEC_INIT_BUF
  hw/ide: drop iov field from IDEState
  hw/ide: drop iov field from IDEBufferedRequest
  hw/ide: drop iov field from IDEDMA

 include/hw/ide/internal.h |  3 --
 include/qemu/iov.h        | 64 +++++++++++++++++++++++++++-
 block/backup.c            |  5 +--
 block/block-backend.c     | 13 +-----
 block/commit.c            |  7 +--
 block/io.c                | 89 +++++++++------------------------------
 block/parallels.c         | 13 +++---
 block/qcow.c              | 21 ++-------
 block/qcow2.c             | 12 +-----
 block/qed-table.c         | 16 ++-----
 block/qed.c               | 31 ++++----------
 block/stream.c            |  7 +--
 block/vmdk.c              |  7 +--
 hw/ide/atapi.c            | 14 +++---
 hw/ide/core.c             | 19 ++++-----
 migration/block.c         | 10 ++---
 qemu-img.c                | 10 +----
 tests/test-bdrv-drain.c   | 29 ++-----------
 18 files changed, 134 insertions(+), 236 deletions(-)

-- 
2.18.0

Re: [Qemu-devel] [PATCH v3 00/17] block: local qiov helper

Posted by Stefan Hajnoczi 6 years, 8 months ago

On Thu, Feb 07, 2019 at 01:24:28PM +0300, Vladimir Sementsov-Ogievskiy wrote:
> v3:

Will you send a v4 based on Eric's comments or do you want to keep the
series as it is?

Stefan

Re: [Qemu-devel] [PATCH v3 00/17] block: local qiov helper

Posted by Vladimir Sementsov-Ogievskiy 6 years, 8 months ago

On 11.02.2019 6:04, Stefan Hajnoczi wrote:
> On Thu, Feb 07, 2019 at 01:24:28PM +0300, Vladimir Sementsov-Ogievskiy wrote:
>> v3:
> 
> Will you send a v4 based on Eric's comments or do you want to keep the
> series as it is?
> 

I don't really want to resend, and I don't think that open-coding in 16 
is a good practice. With my helper we at least have and assertion. I'd be
grateful if tiny improvements of 01 are applied in-flight. Anyway, I don't
strongly against other Eric's suggestions to be applied, and can resend if
it needed.

Re: [Qemu-devel] [PATCH v3 00/17] block: local qiov helper

Posted by Stefan Hajnoczi 6 years, 8 months ago

On Thu, Feb 07, 2019 at 01:24:28PM +0300, Vladimir Sementsov-Ogievskiy wrote:
> Hi all!
> 
> Here is a new simple helper for a very often patter
> around qemu_iovec_init_external, when we need simple qiov with only
> one iov, initialized from external buffer.
> 
> v3:

Hi Vladimir,
"make check" is failing:

  TEST    check-qtest-x86_64: tests/ide-test
qemu-system-x86_64: /home/stefanha/qemu/include/qemu/iov.h:195: qemu_iovec_get_buf: Assertion `qiov->niov == -1 && qiov->iov == &qiov->local_iov' failed.

Please take a look and send v4.

Stefan

Re: [Qemu-devel] [PATCH v3 00/17] block: local qiov helper

Posted by Vladimir Sementsov-Ogievskiy 6 years, 8 months ago

13.02.2019 11:19, Stefan Hajnoczi wrote:
> On Thu, Feb 07, 2019 at 01:24:28PM +0300, Vladimir Sementsov-Ogievskiy wrote:
>> Hi all!
>>
>> Here is a new simple helper for a very often patter
>> around qemu_iovec_init_external, when we need simple qiov with only
>> one iov, initialized from external buffer.
>>
>> v3:
> 
> Hi Vladimir,
> "make check" is failing:
> 
>    TEST    check-qtest-x86_64: tests/ide-test
> qemu-system-x86_64: /home/stefanha/qemu/include/qemu/iov.h:195: qemu_iovec_get_buf: Assertion `qiov->niov == -1 && qiov->iov == &qiov->local_iov' failed.
> 
> Please take a look and send v4.
> 
> Stefan
> 

Oops, sorry for this, it obviously should be qiov->nalloc == -1 in assertion. I'll resend and rename s/qemu_iovec_get_buf/qemu_iovec_embedded_buf/ to make it more obvious.

-- 
Best regards,
Vladimir

Re: [Qemu-devel] [PATCH v3 00/17] block: local qiov helper

Posted by Stefan Hajnoczi 6 years, 8 months ago

On Thu, Feb 07, 2019 at 01:24:28PM +0300, Vladimir Sementsov-Ogievskiy wrote:
> Hi all!
> 
> Here is a new simple helper for a very often patter
> around qemu_iovec_init_external, when we need simple qiov with only
> one iov, initialized from external buffer.
> 
> v3:
>   01-02: tiny improvements, described in patch-emails
>   03-17: new patches
> 
>   Note: only hw/scsi/scsi-disk.c not updated, as it has too tricky
>         logic around @iov fields of structures. So, it is simpler to
>         keep it as is.
> 
> Previous series version was "[PATCH v2 0/2] block: local qiov helper: part I"
> https://lists.gnu.org/archive/html/qemu-devel/2019-02/msg01610.html
> 
> Vladimir Sementsov-Ogievskiy (17):
>   block: enhance QEMUIOVector structure
>   block/io: use qemu_iovec_init_buf
>   block/block-backend: use QEMU_IOVEC_INIT_BUF
>   block/backup: use qemu_iovec_init_buf
>   block/commit: use QEMU_IOVEC_INIT_BUF
>   block/stream: use QEMU_IOVEC_INIT_BUF
>   block/parallels: use QEMU_IOVEC_INIT_BUF
>   block/qcow: use qemu_iovec_init_buf
>   block/qcow2: use qemu_iovec_init_buf
>   block/qed: use qemu_iovec_init_buf
>   block/vmdk: use qemu_iovec_init_buf
>   qemu-img: use qemu_iovec_init_buf
>   migration/block: use qemu_iovec_init_buf
>   tests/test-bdrv-drain: use QEMU_IOVEC_INIT_BUF
>   hw/ide: drop iov field from IDEState
>   hw/ide: drop iov field from IDEBufferedRequest
>   hw/ide: drop iov field from IDEDMA
> 
>  include/hw/ide/internal.h |  3 --
>  include/qemu/iov.h        | 64 +++++++++++++++++++++++++++-
>  block/backup.c            |  5 +--
>  block/block-backend.c     | 13 +-----
>  block/commit.c            |  7 +--
>  block/io.c                | 89 +++++++++------------------------------
>  block/parallels.c         | 13 +++---
>  block/qcow.c              | 21 ++-------
>  block/qcow2.c             | 12 +-----
>  block/qed-table.c         | 16 ++-----
>  block/qed.c               | 31 ++++----------
>  block/stream.c            |  7 +--
>  block/vmdk.c              |  7 +--
>  hw/ide/atapi.c            | 14 +++---
>  hw/ide/core.c             | 19 ++++-----
>  migration/block.c         | 10 ++---
>  qemu-img.c                | 10 +----
>  tests/test-bdrv-drain.c   | 29 ++-----------
>  18 files changed, 134 insertions(+), 236 deletions(-)
> 
> -- 
> 2.18.0

I made the changes suggested by Eric in Patch 1.

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

Stefan

Re: [Qemu-devel] [PATCH v3 00/17] block: local qiov helper

Posted by Stefan Hajnoczi 6 years, 8 months ago

On Thu, Feb 07, 2019 at 01:24:28PM +0300, Vladimir Sementsov-Ogievskiy wrote:
> Hi all!
> 
> Here is a new simple helper for a very often patter
> around qemu_iovec_init_external, when we need simple qiov with only
> one iov, initialized from external buffer.
> 
> v3:
>   01-02: tiny improvements, described in patch-emails
>   03-17: new patches
> 
>   Note: only hw/scsi/scsi-disk.c not updated, as it has too tricky
>         logic around @iov fields of structures. So, it is simpler to
>         keep it as is.
> 
> Previous series version was "[PATCH v2 0/2] block: local qiov helper: part I"
> https://lists.gnu.org/archive/html/qemu-devel/2019-02/msg01610.html
> 
> Vladimir Sementsov-Ogievskiy (17):
>   block: enhance QEMUIOVector structure
>   block/io: use qemu_iovec_init_buf
>   block/block-backend: use QEMU_IOVEC_INIT_BUF
>   block/backup: use qemu_iovec_init_buf
>   block/commit: use QEMU_IOVEC_INIT_BUF
>   block/stream: use QEMU_IOVEC_INIT_BUF
>   block/parallels: use QEMU_IOVEC_INIT_BUF
>   block/qcow: use qemu_iovec_init_buf
>   block/qcow2: use qemu_iovec_init_buf
>   block/qed: use qemu_iovec_init_buf
>   block/vmdk: use qemu_iovec_init_buf
>   qemu-img: use qemu_iovec_init_buf
>   migration/block: use qemu_iovec_init_buf
>   tests/test-bdrv-drain: use QEMU_IOVEC_INIT_BUF
>   hw/ide: drop iov field from IDEState
>   hw/ide: drop iov field from IDEBufferedRequest
>   hw/ide: drop iov field from IDEDMA
> 
>  include/hw/ide/internal.h |  3 --
>  include/qemu/iov.h        | 64 +++++++++++++++++++++++++++-
>  block/backup.c            |  5 +--
>  block/block-backend.c     | 13 +-----
>  block/commit.c            |  7 +--
>  block/io.c                | 89 +++++++++------------------------------
>  block/parallels.c         | 13 +++---
>  block/qcow.c              | 21 ++-------
>  block/qcow2.c             | 12 +-----
>  block/qed-table.c         | 16 ++-----
>  block/qed.c               | 31 ++++----------
>  block/stream.c            |  7 +--
>  block/vmdk.c              |  7 +--
>  hw/ide/atapi.c            | 14 +++---
>  hw/ide/core.c             | 19 ++++-----
>  migration/block.c         | 10 ++---
>  qemu-img.c                | 10 +----
>  tests/test-bdrv-drain.c   | 29 ++-----------
>  18 files changed, 134 insertions(+), 236 deletions(-)
> 
> -- 
> 2.18.0
> 

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>