[Qemu-devel] [PULL] iov* function rework (one more time)

[Qemu-devel] [PULL] iov* function rework (one more time)

[Michael Tokarev]

The following changes since commit 083dbf489d1b0592e910ecfb90b3858c23e49ab7:

  target-microblaze: fix swx build breakage (2012-06-07 10:12:00 +0200)

are available in the git repository at:

  git://git.corpit.ru/qemu.git mjt-iov

for you to fetch changes up to 85c5ece9bc0bd30f0573e43d1ddd756154ac6849:

  rewrite iov_send_recv() and move it to iov.c (2012-06-07 21:22:54 +0400)

----------------------------------------------------------------
Michael Tokarev (11):
      virtio-serial-bus: use correct lengths in control_out() message
      change iov_* function prototypes to be more appropriate
      rewrite iov_* functions
      consolidate qemu_iovec_memset{,_skip}() into single function and use existing iov_memset()
      allow qemu_iovec_from_buffer() to specify offset from which to start copying
      consolidate qemu_iovec_copy() and qemu_iovec_concat() and make them consistent
      change qemu_iovec_to_buf() to match other to,from_buf functions
      rename qemu_sendv to iov_send, change proto and move declarations to iov.h
      export iov_send_recv() and use it in iov_send() and iov_recv()
      cleanup qemu_co_sendv(), qemu_co_recvv() and friends
      rewrite iov_send_recv() and move it to iov.c

This is the same patchset which has been submitted, resubmitted,
rewritten, resubmitted, split, resubmitted, merged, rewritten again,
resubmitted and finally received a promise from Anthony to be looked
at.  Today, months later, I'm still awaiiting for the answer, and,
while I said before that the last resubmission was, well, last,
since I bothered so many people so many times, I decided to break
my promise, rebase against current qemy/master and resend it,
now as a git pull request, to not mailbomb people again.  The rebase
took quite some work again, since the code changed (obviously) since
the patchset was ready before.  During rebase I reviewed all the
commit messages too, and removed all the Cc lines they had.

As far as I remember, all comments about the patchset has been
addressed or answered, the last event was the above mentioned
promise.

I ensured that each commit in the series compiles and works (this
time I didn't perform extensive tests as before, just very basic
tests), so it is bisectable as before.

The description for the patchset is the same as before, can be found
at http://thread.gmane.org/gmane.comp.emulators.qemu/142079 .
New diffstat is below.

The patchest can be browsed online at
http://git.corpit.ru/?p=qemu.git;a=shortlog;h=refs/heads/mjt-iov

Please pull.

Thanks,

/mjt

 Makefile.objs          |    2 +-
 block.c                |   12 ++---
 block/curl.c           |    6 +--
 block/iscsi.c          |    3 +-
 block/nbd.c            |   18 ++++---
 block/qcow.c           |    4 +-
 block/qcow2.c          |   21 ++++----
 block/qed.c            |   10 ++--
 block/rbd.c            |    4 +-
 block/sheepdog.c       |    6 +--
 cutils.c               |  234 ++++++++++++---------------------------------------------------------------------------
 hw/9pfs/virtio-9p.c    |    8 +--
 hw/rtl8139.c           |    2 +-
 hw/usb/core.c          |    6 +--
 hw/virtio-balloon.c    |    4 +-
 hw/virtio-net.c        |    4 +-
 hw/virtio-serial-bus.c |   10 ++--
 iov.c                  |  192 ++++++++++++++++++++++++++++++++++++++++++++++++++++-------------------
 iov.h                  |   77 ++++++++++++++++++++++++++---
 linux-aio.c            |    4 +-
 net.c                  |    2 +-
 posix-aio-compat.c     |    8 ++-
 qemu-common.h          |   56 ++++++++++-----------
 qemu-coroutine-io.c    |   83 ++++++++++---------------------
 tests/Makefile         |    2 +
 tests/test-iov.c       |  260 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 26 files changed, 620 insertions(+), 418 deletions(-)
 create mode 100644 tests/test-iov.c

Re: [Qemu-devel] [PULL] iov* function rework (one more time)

[Paolo Bonzini]

Il 07/06/2012 19:53, Michael Tokarev ha scritto:
> 
> This is the same patchset which has been submitted, resubmitted,
> rewritten, resubmitted, split, resubmitted, merged, rewritten again,
> resubmitted and finally received a promise from Anthony to be looked
> at.  Today, months later, I'm still awaiiting for the answer, and,
> while I said before that the last resubmission was, well, last,
> since I bothered so many people so many times, I decided to break
> my promise, rebase against current qemy/master and resend it,
> now as a git pull request, to not mailbomb people again.  The rebase
> took quite some work again, since the code changed (obviously) since
> the patchset was ready before.  During rebase I reviewed all the
> commit messages too, and removed all the Cc lines they had.

I'll review the patches and hopefully send my Reviewed-by, since I
actually liked the idea.  But I'm travelling so I will only be able to
send the email on Monday.

Paolo

Re: [Qemu-devel] [PULL] iov* function rework (one more time)

[Paolo Bonzini]

Il 08/06/2012 01:20, Paolo Bonzini ha scritto:
>> > This is the same patchset which has been submitted, resubmitted,
>> > rewritten, resubmitted, split, resubmitted, merged, rewritten again,
>> > resubmitted and finally received a promise from Anthony to be looked
>> > at.  Today, months later, I'm still awaiiting for the answer, and,
>> > while I said before that the last resubmission was, well, last,
>> > since I bothered so many people so many times, I decided to break
>> > my promise, rebase against current qemy/master and resend it,
>> > now as a git pull request, to not mailbomb people again.  The rebase
>> > took quite some work again, since the code changed (obviously) since
>> > the patchset was ready before.  During rebase I reviewed all the
>> > commit messages too, and removed all the Cc lines they had.
> I'll review the patches and hopefully send my Reviewed-by, since I
> actually liked the idea.  But I'm travelling so I will only be able to
> send the email on Monday.

Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>

One day in advance ;)

Paolo

Re: [Qemu-devel] [PULL] iov* function rework (one more time)

[Kevin Wolf]

Am 07.06.2012 19:53, schrieb Michael Tokarev:
> The following changes since commit 083dbf489d1b0592e910ecfb90b3858c23e49ab7:
> 
>   target-microblaze: fix swx build breakage (2012-06-07 10:12:00 +0200)
> 
> are available in the git repository at:
> 
>   git://git.corpit.ru/qemu.git mjt-iov
> 
> for you to fetch changes up to 85c5ece9bc0bd30f0573e43d1ddd756154ac6849:
> 
>   rewrite iov_send_recv() and move it to iov.c (2012-06-07 21:22:54 +0400)
> 
> ----------------------------------------------------------------
> Michael Tokarev (11):
>       virtio-serial-bus: use correct lengths in control_out() message
>       change iov_* function prototypes to be more appropriate
>       rewrite iov_* functions
>       consolidate qemu_iovec_memset{,_skip}() into single function and use existing iov_memset()
>       allow qemu_iovec_from_buffer() to specify offset from which to start copying
>       consolidate qemu_iovec_copy() and qemu_iovec_concat() and make them consistent
>       change qemu_iovec_to_buf() to match other to,from_buf functions
>       rename qemu_sendv to iov_send, change proto and move declarations to iov.h
>       export iov_send_recv() and use it in iov_send() and iov_recv()
>       cleanup qemu_co_sendv(), qemu_co_recvv() and friends
>       rewrite iov_send_recv() and move it to iov.c


I haven't reviewed the series in detail, and mostly ignored the cutils.c
implementation and usage in other subsystems, but as far as the block
layer is concerned:

Acked-by: Kevin Wolf <kwolf@redhat.com>

(Even though I don't agree that the new argument order of iov_from_buf()
is better, but that's probably a matter of taste and the series has
enough other good stuff)

Kevin

Re: [Qemu-devel] [PULL] iov* function rework (one more time)

[Anthony Liguori]

On 06/07/2012 12:53 PM, Michael Tokarev wrote:
> The following changes since commit 083dbf489d1b0592e910ecfb90b3858c23e49ab7:
>
>    target-microblaze: fix swx build breakage (2012-06-07 10:12:00 +0200)
>
> are available in the git repository at:
>
>    git://git.corpit.ru/qemu.git mjt-iov
>
> for you to fetch changes up to 85c5ece9bc0bd30f0573e43d1ddd756154ac6849:
>
>    rewrite iov_send_recv() and move it to iov.c (2012-06-07 21:22:54 +0400)

Doesn't build:

   LINK  qemu-ga
cutils.o: In function `qemu_iovec_to_buf':
/home/anthony/git/qemu/cutils.c:226: undefined reference to `iov_to_buf'
cutils.o: In function `qemu_iovec_from_buf':
/home/anthony/git/qemu/cutils.c:232: undefined reference to `iov_from_buf'
cutils.o: In function `qemu_iovec_memset':
/home/anthony/git/qemu/cutils.c:238: undefined reference to `iov_memset'
collect2: ld returned 1 exit status
make: *** [qemu-ga] Error 1

Not sure why you didn't see this.  Are you not testing with a full build?

Regards,

Anthony Liguori

Re: [Qemu-devel] [PULL] iov* function rework (one more time)

[Michael Tokarev]

On 11.06.2012 22:31, Anthony Liguori wrote:
[]
> Doesn't build:
> 
>   LINK  qemu-ga
> cutils.o: In function `qemu_iovec_to_buf':
> /home/anthony/git/qemu/cutils.c:226: undefined reference to `iov_to_buf'

Indeed.  I'm sorry for this.  Just a makefile issue, I'm not really
sure which of the various *-objs to put this stuff at.  I guess it
should be placed into every place where cutils.o is referenced,
since functions from cutils are now just calling functions from
iov.

> Not sure why you didn't see this.  Are you not testing with a full build?

I tested every commit back and forth in turn, but only for
the main qemu binary build, not for any of the tools builds.
Even the user target does not build, due to the same issue.

I just tested complete builds and separate tools builds,
and verified the resulting qemu-nbd a bit too, it appears
to work fine.

This is commit bb09bc0a5e6eb36d67d8e5f1bddb52fa9c80d463,
"consolidate qemu_iovec_memset{,_skip}() into single function..."
which needs to be redone to include some more stuff in there.
It was due to me improperly rebasing changes to 1.1 -- I
used much more care when forward-ported the C changed, but
missed trivial Makefile changes.

I updated this commit, and pushed new branch mjt-iov2 to the
same location, git://git.corpit.ru/qemu.git .  The new pull
request is below.

Besides, with a few Reviewed-By and Signed-off-by, should
I add the corresponding coments to the commit messages as
well, when doing the rebase?  And should I also rebase to
the current qemu/master?

Please excuse me for for yet another wrong attempt.

Thanks,

/mjt

-----
The following changes since commit 45270ad8a86a80cca4c59dfa73d9a9ee0688d781:

  virtio-serial-bus: use correct lengths in control_out() message (2012-06-07 20:43:26 +0400)

are available in the git repository at:

  git://git.corpit.ru/qemu.git mjt-iov2

for you to fetch changes up to 25e5e4c7e9d5ec3e95c9526d1abaca40ada50ab0:

  rewrite iov_send_recv() and move it to iov.c (2012-06-11 23:12:11 +0400)

----------------------------------------------------------------
Michael Tokarev (10):
      change iov_* function prototypes to be more appropriate
      rewrite iov_* functions
      consolidate qemu_iovec_memset{,_skip}() into single function and use existing iov_memset()
      allow qemu_iovec_from_buffer() to specify offset from which to start copying
      consolidate qemu_iovec_copy() and qemu_iovec_concat() and make them consistent
      change qemu_iovec_to_buf() to match other to,from_buf functions
      rename qemu_sendv to iov_send, change proto and move declarations to iov.h
      export iov_send_recv() and use it in iov_send() and iov_recv()
      cleanup qemu_co_sendv(), qemu_co_recvv() and friends
      rewrite iov_send_recv() and move it to iov.c

 Makefile               |    3 +-
 Makefile.objs          |    4 +-
 block.c                |   12 ++--
 block/curl.c           |    6 +-
 block/iscsi.c          |    3 +-
 block/nbd.c            |   18 ++---
 block/qcow.c           |    4 +-
 block/qcow2.c          |   21 +++---
 block/qed.c            |   10 +--
 block/rbd.c            |    4 +-
 block/sheepdog.c       |    6 +-
 cutils.c               |  234 ++++++++-------------------------------------------------------
 hw/9pfs/virtio-9p.c    |    8 +--
 hw/rtl8139.c           |    2 +-
 hw/usb/core.c          |    6 +-
 hw/virtio-balloon.c    |    4 +-
 hw/virtio-net.c        |    4 +-
 hw/virtio-serial-bus.c |    6 +-
 iov.c                  |  192 +++++++++++++++++++++++++++++++++++++--------------
 iov.h                  |   77 +++++++++++++++++++--
 linux-aio.c            |    4 +-
 net.c                  |    2 +-
 posix-aio-compat.c     |    8 +--
 qemu-common.h          |   56 +++++++--------
 qemu-coroutine-io.c    |   83 +++++++----------------
 tests/Makefile         |    2 +
 tests/test-iov.c       |  260 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 27 files changed, 621 insertions(+), 418 deletions(-)
 create mode 100644 tests/test-iov.c

Re: [Qemu-devel] [PULL] iov* function rework (one more time)

[Michael Tokarev]

Ping?  Another month has passed without any reply...

Thanks,

/mjt

On 11.06.2012 23:19, Michael Tokarev wrote:
> On 11.06.2012 22:31, Anthony Liguori wrote:
> []
>> Doesn't build:
>>
>>   LINK  qemu-ga
>> cutils.o: In function `qemu_iovec_to_buf':
>> /home/anthony/git/qemu/cutils.c:226: undefined reference to `iov_to_buf'
> 
> Indeed.  I'm sorry for this.  Just a makefile issue, I'm not really
> sure which of the various *-objs to put this stuff at.  I guess it
> should be placed into every place where cutils.o is referenced,
> since functions from cutils are now just calling functions from
> iov.
> 
>> Not sure why you didn't see this.  Are you not testing with a full build?
> 
> I tested every commit back and forth in turn, but only for
> the main qemu binary build, not for any of the tools builds.
> Even the user target does not build, due to the same issue.
> 
> I just tested complete builds and separate tools builds,
> and verified the resulting qemu-nbd a bit too, it appears
> to work fine.
> 
> This is commit bb09bc0a5e6eb36d67d8e5f1bddb52fa9c80d463,
> "consolidate qemu_iovec_memset{,_skip}() into single function..."
> which needs to be redone to include some more stuff in there.
> It was due to me improperly rebasing changes to 1.1 -- I
> used much more care when forward-ported the C changed, but
> missed trivial Makefile changes.
> 
> I updated this commit, and pushed new branch mjt-iov2 to the
> same location, git://git.corpit.ru/qemu.git .  The new pull
> request is below.
> 
> Besides, with a few Reviewed-By and Signed-off-by, should
> I add the corresponding coments to the commit messages as
> well, when doing the rebase?  And should I also rebase to
> the current qemu/master?
> 
> Please excuse me for for yet another wrong attempt.
> 
> Thanks,
> 
> /mjt
> 
> -----
> The following changes since commit 45270ad8a86a80cca4c59dfa73d9a9ee0688d781:
> 
>   virtio-serial-bus: use correct lengths in control_out() message (2012-06-07 20:43:26 +0400)
> 
> are available in the git repository at:
> 
>   git://git.corpit.ru/qemu.git mjt-iov2
> 
> for you to fetch changes up to 25e5e4c7e9d5ec3e95c9526d1abaca40ada50ab0:
> 
>   rewrite iov_send_recv() and move it to iov.c (2012-06-11 23:12:11 +0400)
> 
> ----------------------------------------------------------------
> Michael Tokarev (10):
>       change iov_* function prototypes to be more appropriate
>       rewrite iov_* functions
>       consolidate qemu_iovec_memset{,_skip}() into single function and use existing iov_memset()
>       allow qemu_iovec_from_buffer() to specify offset from which to start copying
>       consolidate qemu_iovec_copy() and qemu_iovec_concat() and make them consistent
>       change qemu_iovec_to_buf() to match other to,from_buf functions
>       rename qemu_sendv to iov_send, change proto and move declarations to iov.h
>       export iov_send_recv() and use it in iov_send() and iov_recv()
>       cleanup qemu_co_sendv(), qemu_co_recvv() and friends
>       rewrite iov_send_recv() and move it to iov.c
> 
>  Makefile               |    3 +-
>  Makefile.objs          |    4 +-
>  block.c                |   12 ++--
>  block/curl.c           |    6 +-
>  block/iscsi.c          |    3 +-
>  block/nbd.c            |   18 ++---
>  block/qcow.c           |    4 +-
>  block/qcow2.c          |   21 +++---
>  block/qed.c            |   10 +--
>  block/rbd.c            |    4 +-
>  block/sheepdog.c       |    6 +-
>  cutils.c               |  234 ++++++++-------------------------------------------------------
>  hw/9pfs/virtio-9p.c    |    8 +--
>  hw/rtl8139.c           |    2 +-
>  hw/usb/core.c          |    6 +-
>  hw/virtio-balloon.c    |    4 +-
>  hw/virtio-net.c        |    4 +-
>  hw/virtio-serial-bus.c |    6 +-
>  iov.c                  |  192 +++++++++++++++++++++++++++++++++++++--------------
>  iov.h                  |   77 +++++++++++++++++++--
>  linux-aio.c            |    4 +-
>  net.c                  |    2 +-
>  posix-aio-compat.c     |    8 +--
>  qemu-common.h          |   56 +++++++--------
>  qemu-coroutine-io.c    |   83 +++++++----------------
>  tests/Makefile         |    2 +
>  tests/test-iov.c       |  260 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  27 files changed, 621 insertions(+), 418 deletions(-)
>  create mode 100644 tests/test-iov.c
> 

Re: [Qemu-devel] [PULL] iov* function rework (one more time)

[Anthony Liguori]

On 06/11/2012 02:19 PM, Michael Tokarev wrote:
> On 11.06.2012 22:31, Anthony Liguori wrote:
> []
>> Doesn't build:
>>
>>    LINK  qemu-ga
>> cutils.o: In function `qemu_iovec_to_buf':
>> /home/anthony/git/qemu/cutils.c:226: undefined reference to `iov_to_buf'
>
> Indeed.  I'm sorry for this.  Just a makefile issue, I'm not really
> sure which of the various *-objs to put this stuff at.  I guess it
> should be placed into every place where cutils.o is referenced,
> since functions from cutils are now just calling functions from
> iov.
>
>> Not sure why you didn't see this.  Are you not testing with a full build?
>
> I tested every commit back and forth in turn, but only for
> the main qemu binary build, not for any of the tools builds.
> Even the user target does not build, due to the same issue.
>
> I just tested complete builds and separate tools builds,
> and verified the resulting qemu-nbd a bit too, it appears
> to work fine.
>
> This is commit bb09bc0a5e6eb36d67d8e5f1bddb52fa9c80d463,
> "consolidate qemu_iovec_memset{,_skip}() into single function..."
> which needs to be redone to include some more stuff in there.
> It was due to me improperly rebasing changes to 1.1 -- I
> used much more care when forward-ported the C changed, but
> missed trivial Makefile changes.
>
> I updated this commit, and pushed new branch mjt-iov2 to the
> same location, git://git.corpit.ru/qemu.git .  The new pull
> request is below.
>
> Besides, with a few Reviewed-By and Signed-off-by, should
> I add the corresponding coments to the commit messages as
> well, when doing the rebase?  And should I also rebase to
> the current qemu/master?
>
> Please excuse me for for yet another wrong attempt.
>
> Thanks,
>
> /mjt
>
> -----
> The following changes since commit 45270ad8a86a80cca4c59dfa73d9a9ee0688d781:
>
>    virtio-serial-bus: use correct lengths in control_out() message (2012-06-07 20:43:26 +0400)
>
> are available in the git repository at:
>
>    git://git.corpit.ru/qemu.git mjt-iov2
>
> for you to fetch changes up to 25e5e4c7e9d5ec3e95c9526d1abaca40ada50ab0:
>
>    rewrite iov_send_recv() and move it to iov.c (2012-06-11 23:12:11 +0400)

Pulled.  Thanks.

Regards,

Anthony Liguori

>
> ----------------------------------------------------------------
> Michael Tokarev (10):
>        change iov_* function prototypes to be more appropriate
>        rewrite iov_* functions
>        consolidate qemu_iovec_memset{,_skip}() into single function and use existing iov_memset()
>        allow qemu_iovec_from_buffer() to specify offset from which to start copying
>        consolidate qemu_iovec_copy() and qemu_iovec_concat() and make them consistent
>        change qemu_iovec_to_buf() to match other to,from_buf functions
>        rename qemu_sendv to iov_send, change proto and move declarations to iov.h
>        export iov_send_recv() and use it in iov_send() and iov_recv()
>        cleanup qemu_co_sendv(), qemu_co_recvv() and friends
>        rewrite iov_send_recv() and move it to iov.c
>
>   Makefile               |    3 +-
>   Makefile.objs          |    4 +-
>   block.c                |   12 ++--
>   block/curl.c           |    6 +-
>   block/iscsi.c          |    3 +-
>   block/nbd.c            |   18 ++---
>   block/qcow.c           |    4 +-
>   block/qcow2.c          |   21 +++---
>   block/qed.c            |   10 +--
>   block/rbd.c            |    4 +-
>   block/sheepdog.c       |    6 +-
>   cutils.c               |  234 ++++++++-------------------------------------------------------
>   hw/9pfs/virtio-9p.c    |    8 +--
>   hw/rtl8139.c           |    2 +-
>   hw/usb/core.c          |    6 +-
>   hw/virtio-balloon.c    |    4 +-
>   hw/virtio-net.c        |    4 +-
>   hw/virtio-serial-bus.c |    6 +-
>   iov.c                  |  192 +++++++++++++++++++++++++++++++++++++--------------
>   iov.h                  |   77 +++++++++++++++++++--
>   linux-aio.c            |    4 +-
>   net.c                  |    2 +-
>   posix-aio-compat.c     |    8 +--
>   qemu-common.h          |   56 +++++++--------
>   qemu-coroutine-io.c    |   83 +++++++----------------
>   tests/Makefile         |    2 +
>   tests/test-iov.c       |  260 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>   27 files changed, 621 insertions(+), 418 deletions(-)
>   create mode 100644 tests/test-iov.c
>
>