[prev in list] [next in list] [prev in thread] [next in thread] 

List:       qemu-block
Subject:    Re: [Qemu-block] [Qemu-devel] [PATCH 2/5] qemu-io: Let command functions return error code
From:       Max Reitz <mreitz () redhat ! com>
Date:       2018-04-30 15:14:54
Message-ID: 829a7d2a-1202-c0ac-f3a2-b9de4afccff6 () redhat ! com
[Download RAW message or body]

[Attachment #2 (multipart/mixed)]


On 2018-04-30 17:08, Eric Blake wrote:
> 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 <mreitz@redhat.com>
>> ---
>>    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.
>   */

Yep, will do.

> With a comment added,
> Reviewed-by: Eric Blake <eblake@redhat.com>

Thanks!

Max



["signature.asc" (application/pgp-signature)]

[prev in list] [next in list] [prev in thread] [next in thread]