Re: [Qemu-devel] [PATCH 1/6] snapshot: export function in block.c

[Stefan Hajnoczi]

On Mon, Dec 17, 2012 at 02:25:04PM +0800, Wenchao Xia wrote:
>   This patch moves bdrv_snapshotfind from savevm.c to block.c and export
> it, also added bdrv_deappend in block.c.

I suggest naming the patch "block: export bdrv_snapshot_find()" because
it is more specific than "snapshot: export function in block.c".

> +/* revert the action */
> +void bdrv_deappend(BlockDriverState *bs_new, BlockDriverState *bs_top)
> +{
> +    bdrv_swap(bs_new, bs_top);
> +    /* this is enough? */
> +}

Please document this function in the same level of detail as
bdrv_append().  These functions are tricky and you must explain the
semantics so anyone else reading the code can safely use or modify this
function.

We also need to resolve the "this is enough?" comment.  I'll think it
through when I see how bdrv_deappend() is used later.

Stefan