Re: [Qemu-devel] [PATCH V3 6/7] qcow2: print message for error path in snapshot creation

[Eric Blake <eblake@redhat.com>]

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

On 09/08/2013 08:58 PM, Wenchao Xia wrote:
> The message will be print out with a macro enabled, which can

s/print/printed/

> be used to check which error path is taken.
> 
> Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
> ---
>  block/qcow2-snapshot.c |   46 ++++++++++++++++++++++++++++++++++++++++++++++
>  1 files changed, 46 insertions(+), 0 deletions(-)
> 

> @@ -381,12 +413,20 @@ int qcow2_snapshot_create(BlockDriverState *bs, QEMUSnapshotInfo *sn_info)
>      ret = qcow2_pre_write_overlap_check(bs, QCOW2_OL_DEFAULT,
>              sn->l1_table_offset, s->l1_size * sizeof(uint64_t));
>      if (ret < 0) {
> +#ifdef QCOW2_SNAPSHOT_PRINT_ERROR_PATH
> +        printf("qcow2: Failed in overlap check before update L1 table for "
> +               "snapshot\n");
> +#endif
>          goto dealloc_cluster;
>      }
>  
> +    BLKDBG_EVENT(bs->file, BLKDBG_SNAPSHOT_L1_UPDATE);

Should this BLKDBG be part of patch 5?

In general, the move to avoid fprintf except under recompilation seems
okay; but it seems odd to be removing the diagnosis altogether.  If you
had gone one step further and refactored the code to wire in Error*
support, then you could change fprintf to passing the Error back up the
stack to the caller rather than losing it except during a debug build.

-- 
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 --]