Re: [Qemu-devel] [PATCH 2/5] qemu-io: Let command functions return error code

[Eric Blake]

On 04/28/2018 09:41 AM, Max Reitz wrote:
> This is basically what everything else in the qemu code base does, so we
> can do it here, too.
>
> Signed-off-by: Max Reitz <address@hidden>
> ---
>   include/qemu-io.h |   4 +-
>   qemu-io-cmds.c    | 346 ++++++++++++++++++++++++++++++++----------------------
>   qemu-io.c         |  34 ++++--
>   3 files changed, 227 insertions(+), 157 deletions(-)
>
> diff --git a/include/qemu-io.h b/include/qemu-io.h
> index 06cdfbf660..380724ad59 100644
> --- a/include/qemu-io.h
> +++ b/include/qemu-io.h
> @@ -22,7 +22,7 @@
>   
>   #define CMD_FLAG_GLOBAL ((int)0x80000000) /* don't iterate "args" */
>   
> -typedef void (*cfunc_t)(BlockBackend *blk, int argc, char **argv);
> +typedef int (*cfunc_t)(BlockBackend *blk, int argc, char **argv);

Quite a bit of churn on the definition here; squashing 1 and 2 might 
make for a smaller patch.  But I agree that doing it this way (2 
separate patches) forces us to audit (with the compiler's help) that all 
callers are caught and adjusted to the new semantics.

However, a comment before the typedef would be useful, so that writing a 
new command knows what semantics it must provide.  It can be as simple as:

/* Implement a qemu-io command.
 * Operate on @blk using @argc/@argv as the command's arguments, and
 * return 0 on success or negative errno on failure.
 */

With a comment added,
Reviewed-by: Eric Blake <address@hidden>

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