[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH V3 6/7] qcow2: print message for error path in s
From: |
Stefan Hajnoczi |
Subject: |
Re: [Qemu-devel] [PATCH V3 6/7] qcow2: print message for error path in snapshot creation |
Date: |
Wed, 2 Oct 2013 14:23:24 +0200 |
User-agent: |
Mutt/1.5.21 (2010-09-15) |
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
- Re: [Qemu-devel] [PATCH V3 6/7] qcow2: print message for error path in snapshot creation,
Stefan Hajnoczi <=