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

[Wenchao Xia]

于 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<address@hidden>
> > > ---
> > >   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.