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

[Wenchao Xia <xiawenc@linux.vnet.ibm.com>]

于 2013/10/2 20:23, Stefan Hajnoczi 写道:
> On Mon, Sep 30, 2013 at 04:08:53PM -0600, Eric Blake wrote:
>> 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.
> I agree with Eric.  Use Error* and make snapshot commands print a
> detailed error to the monitor.
>
> When diagnostics are compiled out we can't help troubleshoot user
> problems.
>
> Stefan
>
Make sense, will add *errp to report the errors.