All of lore.kernel.org
 help / color / mirror / Atom feed
From: Eric Blake <eblake@redhat.com>
To: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
Cc: kwolf@redhat.com, aliguori@us.ibm.com, stefanha@gmail.com,
	qemu-devel@nongnu.org, armbru@redhat.com, pbonzini@redhat.com
Subject: Re: [Qemu-devel] [PATCH V8 06/20] block: move collect_snapshots() and collect_image_info() to block/qapi.c
Date: Fri, 08 Mar 2013 15:04:36 -0700	[thread overview]
Message-ID: <513A6074.1070006@redhat.com> (raw)
In-Reply-To: <1362636445-7188-7-git-send-email-xiawenc@linux.vnet.ibm.com>

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

On 03/06/2013 11:07 PM, Wenchao Xia wrote:
>   This patch is just for making review easier, those two functions will
> be modified and renamed later.
> 
> Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
> ---

> +
> +void bdrv_collect_image_info(BlockDriverState *bs,
> +                             ImageInfo *info,
> +                             const char *fmt)
> +{

Three arguments here...

> +
> +void bdrv_collect_snapshots(BlockDriverState *bs , ImageInfo *info);
> +void bdrv_collect_image_info(BlockDriverState *bs,
> +                             ImageInfo *info,
> +                             const char *filename,
> +                             const char *fmt);

...but four here...

>  
> -static void collect_image_info(BlockDriverState *bs,
> -                   ImageInfo *info,
> -                   const char *filename)

...and moved from three arguments here...

>          info = g_new0(ImageInfo, 1);
> -        collect_image_info(bs, info, filename);
> -        collect_snapshots(bs, info);
> +        bdrv_collect_image_info(bs, info, filename, fmt);

...and your call site changes from 3 to 4 arguments.

How did you compile this?  Code motion must NOT make any semantic
changes - you should have exactly three arguments, preferably with the
same name, and save the addition of a fourth fmt argument until a later
patch.

Hint - a code motion patch should be easy to inspect with:
$ diff -u <(sed -n 's/^-//p' patch) <(sed -n 's/^\+//p' patch)

It's okay to have differences (such as 'static void collect_image_info'
becoming exported 'void bdrv_collect_image_info', and to see
reindentation to line up to the new function name), but the differences
should be trivially correct, and not a change between number of parameters.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


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

  reply	other threads:[~2013-03-08 22:06 UTC|newest]

Thread overview: 42+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-03-07  6:07 [Qemu-devel] [PATCH V8 00/20] qmp/hmp interfaces for internal snapshot info Wenchao Xia
2013-03-07  6:07 ` [Qemu-devel] [PATCH V8 01/20] build: add block/snapshot.c Wenchao Xia
2013-03-08 20:18   ` Eric Blake
2013-03-09  4:00     ` Wenchao Xia
2013-03-09 13:10       ` Eric Blake
2013-03-07  6:07 ` [Qemu-devel] [PATCH V8 02/20] build: add block/qapi.c Wenchao Xia
2013-03-08 20:22   ` Eric Blake
2013-03-09  4:05     ` Wenchao Xia
2013-03-07  6:07 ` [Qemu-devel] [PATCH V8 03/20] block: move bdrv_snapshot_find() to block/snapshot.c Wenchao Xia
2013-03-08 20:27   ` Eric Blake
2013-03-09  4:17     ` Wenchao Xia
2013-03-07  6:07 ` [Qemu-devel] [PATCH V8 04/20] block: distinguish id and name in bdrv_find_snapshot() Wenchao Xia
2013-03-07  6:07 ` [Qemu-devel] [PATCH V8 05/20] qemu-img: remove unused parameter in collect_image_info() Wenchao Xia
2013-03-08 20:34   ` Eric Blake
2013-03-09  4:18     ` Wenchao Xia
2013-03-07  6:07 ` [Qemu-devel] [PATCH V8 06/20] block: move collect_snapshots() and collect_image_info() to block/qapi.c Wenchao Xia
2013-03-08 22:04   ` Eric Blake [this message]
2013-03-09  4:20     ` Wenchao Xia
2013-03-07  6:07 ` [Qemu-devel] [PATCH V8 07/20] block: add snapshot info query function bdrv_query_snapshot_info_list() Wenchao Xia
2013-03-08 22:15   ` Eric Blake
2013-03-07  6:07 ` [Qemu-devel] [PATCH V8 08/20] block: add filter for vm snapshot in bdrv_query_snapshot_info_list() Wenchao Xia
2013-03-08 22:55   ` Eric Blake
2013-03-09  4:24     ` Wenchao Xia
2013-03-07  6:07 ` [Qemu-devel] [PATCH V8 09/20] block: add image info query function bdrv_query_image_info() Wenchao Xia
2013-03-08 23:08   ` Eric Blake
2013-03-09  4:26     ` Wenchao Xia
2013-03-07  6:07 ` [Qemu-devel] [PATCH V8 10/20] qmp: add interface query-snapshots Wenchao Xia
2013-03-08 23:30   ` Eric Blake
2013-03-09  4:28     ` Wenchao Xia
2013-03-07  6:07 ` [Qemu-devel] [PATCH V8 11/20] qmp: add interface query-images Wenchao Xia
2013-03-07  6:07 ` [Qemu-devel] [PATCH V8 12/20] hmp: add function hmp_info_snapshots() Wenchao Xia
2013-03-07  6:07 ` [Qemu-devel] [PATCH V8 13/20] hmp: switch snapshot info function to qmp based one Wenchao Xia
2013-03-07  6:07 ` [Qemu-devel] [PATCH V8 14/20] block: move dump_human_image_info() to block/qapi.c Wenchao Xia
2013-03-07  6:07 ` [Qemu-devel] [PATCH V8 15/20] block: dump to buffer for bdrv_image_info_dump() Wenchao Xia
2013-03-07  6:07 ` [Qemu-devel] [PATCH V8 16/20] hmp: add command info images Wenchao Xia
2013-03-07  6:07 ` [Qemu-devel] [PATCH V8 17/20] block: return bool for bdrv_can_snapshot() Wenchao Xia
2013-03-07  6:07 ` [Qemu-devel] [PATCH V8 18/20] block: move snapshot related functions to block/snapshot.c Wenchao Xia
2013-03-07  6:07 ` [Qemu-devel] [PATCH V8 19/20] block: move bdrv_snapshot_dump() to block/qapi.c Wenchao Xia
2013-03-07  6:07 ` [Qemu-devel] [PATCH V8 20/20] block: rename bdrv_query_info() to bdrv_query_block_info() Wenchao Xia
2013-03-07  7:36 ` [Qemu-devel] [PATCH V8 00/20] qmp/hmp interfaces for internal snapshot info Markus Armbruster
2013-03-11  1:24   ` Wenchao Xia
2013-03-11  8:34     ` Markus Armbruster

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=513A6074.1070006@redhat.com \
    --to=eblake@redhat.com \
    --cc=aliguori@us.ibm.com \
    --cc=armbru@redhat.com \
    --cc=kwolf@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=stefanha@gmail.com \
    --cc=xiawenc@linux.vnet.ibm.com \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.