Re: [Qemu-devel] [PATCH 13/35] nbd: mark coroutine_fn

[Eric Blake <eblake@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 3661 bytes --]

On 07/04/2017 05:03 PM, Marc-André Lureau wrote:
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>  block/nbd-client.h | 10 +++++-----
>  block/nbd-client.c | 24 ++++++++++++++++--------
>  block/nbd.c        |  3 ++-
>  nbd/server.c       |  3 ++-
>  4 files changed, 25 insertions(+), 15 deletions(-)
> 

> diff --git a/block/nbd-client.h b/block/nbd-client.h
> index 49636bc621..473d1f88fd 100644
> --- a/block/nbd-client.h
> +++ b/block/nbd-client.h
> @@ -42,13 +42,13 @@ int nbd_client_init(BlockDriverState *bs,
>                      Error **errp);
>  void nbd_client_close(BlockDriverState *bs);
>  
> -int nbd_client_co_pdiscard(BlockDriverState *bs, int64_t offset, int bytes);
> -int nbd_client_co_flush(BlockDriverState *bs);
> -int nbd_client_co_pwritev(BlockDriverState *bs, uint64_t offset,
> +int coroutine_fn nbd_client_co_pdiscard(BlockDriverState *bs, int64_t offset, int bytes);
> +int coroutine_fn nbd_client_co_flush(BlockDriverState *bs);
> +int coroutine_fn nbd_client_co_pwritev(BlockDriverState *bs, uint64_t offset,
>                            uint64_t bytes, QEMUIOVector *qiov, int flags);

Indentation is off.

> -int nbd_client_co_pwrite_zeroes(BlockDriverState *bs, int64_t offset,
> +int coroutine_fn nbd_client_co_pwrite_zeroes(BlockDriverState *bs, int64_t offset,
>                                  int bytes, BdrvRequestFlags flags);
> -int nbd_client_co_preadv(BlockDriverState *bs, uint64_t offset,
> +int coroutine_fn nbd_client_co_preadv(BlockDriverState *bs, uint64_t offset,
>                           uint64_t bytes, QEMUIOVector *qiov, int flags);
>  

and some more

>  void nbd_client_detach_aio_context(BlockDriverState *bs);
> diff --git a/block/nbd-client.c b/block/nbd-client.c
> index 02e928142e..63c0210c37 100644
> --- a/block/nbd-client.c
> +++ b/block/nbd-client.c
> @@ -111,7 +111,8 @@ static coroutine_fn void nbd_read_reply_entry(void *opaque)
>      s->read_reply_co = NULL;
>  }
>  
> -static int nbd_co_send_request(BlockDriverState *bs,
> +static int coroutine_fn
> +nbd_co_send_request(BlockDriverState *bs,
>                                 NBDRequest *request,
>                                 QEMUIOVector *qiov)
>  {

I know some project specifically like the newline between return type
and function name (it becomes easier to search for function
implementations if ALL function names start in the first column), but it
looks a bit odd compared to the usual qemu style.  I guess you did it
for line length reasons.  But once you do it, now the indentation is off
on the remaining parameters.

> @@ -158,7 +159,8 @@ static int nbd_co_send_request(BlockDriverState *bs,
>      return rc;
>  }
>  
> -static void nbd_co_receive_reply(NBDClientSession *s,
> +static void coroutine_fn
> +nbd_co_receive_reply(NBDClientSession *s,
>                                   NBDRequest *request,
>                                   NBDReply *reply,
>                                   QEMUIOVector *qiov)

Recurring theme of whitespace.

> @@ -185,7 +187,8 @@ static void nbd_co_receive_reply(NBDClientSession *s,
>      }
>  }
>  
> -static void nbd_coroutine_end(BlockDriverState *bs,
> +static void coroutine_fn
> +nbd_coroutine_end(BlockDriverState *bs,
>                                NBDRequest *request)

This even fits on one line now.

Whitespace fixes are trivial enough, so I'm still okay if you add:

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]