qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Max Reitz <mreitz@redhat.com>
To: Eric Blake <eblake@redhat.com>, qemu-block@nongnu.org
Cc: Kevin Wolf <kwolf@redhat.com>, qemu-devel@nongnu.org
Subject: Re: [Qemu-devel] [PATCH 2/5] qemu-io: Let command functions return error code
Date: Mon, 30 Apr 2018 17:14:54 +0200	[thread overview]
Message-ID: <829a7d2a-1202-c0ac-f3a2-b9de4afccff6@redhat.com> (raw)
In-Reply-To: <26182b94-a34a-ad71-255b-f18dd677efd6@redhat.com>

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

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



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

  reply	other threads:[~2018-04-30 15:15 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-04-28 14:41 [Qemu-devel] [PATCH 0/5] qemu-io: Exit with error when a command failed Max Reitz
2018-04-28 14:41 ` [Qemu-devel] [PATCH 1/5] qemu-io: Drop command functions' return values Max Reitz
2018-04-30 14:58   ` Eric Blake
2018-04-28 14:41 ` [Qemu-devel] [PATCH 2/5] qemu-io: Let command functions return error code Max Reitz
2018-04-30 15:08   ` Eric Blake
2018-04-30 15:14     ` Max Reitz [this message]
2018-04-28 14:41 ` [Qemu-devel] [PATCH 3/5] qemu-io: Exit with error when a command failed Max Reitz
2018-04-30 16:38   ` Eric Blake
2018-04-28 14:41 ` [Qemu-devel] [PATCH 4/5] iotests.py: Add qemu_io_silent Max Reitz
2018-04-30 16:40   ` Eric Blake
2018-04-28 14:41 ` [Qemu-devel] [PATCH 5/5] iotests: Let 216 make use of qemu-io's exit code Max Reitz
2018-04-30 16:44   ` Eric Blake

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=829a7d2a-1202-c0ac-f3a2-b9de4afccff6@redhat.com \
    --to=mreitz@redhat.com \
    --cc=eblake@redhat.com \
    --cc=kwolf@redhat.com \
    --cc=qemu-block@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).