[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 02/18] block: add error parameter to bdrv_snapsh
From: |
Luiz Capitulino |
Subject: |
Re: [Qemu-devel] [PATCH 02/18] block: add error parameter to bdrv_snapshot_create() and related functions |
Date: |
Fri, 31 Aug 2012 10:13:59 -0300 |
On Fri, 31 Aug 2012 14:18:55 +0200
Kevin Wolf <address@hidden> wrote:
> Am 31.08.2012 08:26, schrieb Markus Armbruster:
> > Luiz Capitulino <address@hidden> writes:
> >
> >> On Wed, 15 Aug 2012 09:41:43 +0200
> >> Pavel Hrdina <address@hidden> wrote:
> >>
> >>> Signed-off-by: Pavel Hrdina <address@hidden>
> >>> ---
> >>> block.c | 25 +++++++++++++++++--------
> >>> block.h | 3 ++-
> >>> block/qcow2-snapshot.c | 9 ++++++++-
> >>> block/qcow2.h | 4 +++-
> >>> block/rbd.c | 20 ++++++++++++++------
> >>> block/sheepdog.c | 17 +++++++++--------
> >>> block_int.h | 3 ++-
> >>> qemu-img.c | 2 +-
> >>> savevm.c | 2 +-
> >>> 9 files changed, 57 insertions(+), 28 deletions(-)
> >>>
> >>> diff --git a/block.c b/block.c
> >>> index 016858b..8bc49b7 100644
> >>> --- a/block.c
> >>> +++ b/block.c
> >>> @@ -2661,16 +2661,25 @@ BlockDriverState *bdrv_snapshots(void)
> >>> }
> >>>
> >>> int bdrv_snapshot_create(BlockDriverState *bs,
> >>> - QEMUSnapshotInfo *sn_info)
> >>> + QEMUSnapshotInfo *sn_info,
> >>> + Error **errp)
> >>> {
> >>> BlockDriver *drv = bs->drv;
> >>> - if (!drv)
> >>> - return -ENOMEDIUM;
> >>> - if (drv->bdrv_snapshot_create)
> >>> - return drv->bdrv_snapshot_create(bs, sn_info);
> >>> - if (bs->file)
> >>> - return bdrv_snapshot_create(bs->file, sn_info);
> >>> - return -ENOTSUP;
> >>> + int ret;
> >>> +
> >>> + if (!drv) {
> >>> + error_set(errp, QERR_DEVICE_HAS_NO_MEDIUM,
> >>> bdrv_get_device_name(bs));
> >>
> >> We should only use QERR_ macros for the errors listed in the ErrorClass
> >> enum
> >> (except GenericError), all other errors should generally use error_setg(),
> >> like
> >> this:
> >>
> >> error_setg(errp, "device '%s' has no medium);
> >>
> >>> + ret = -ENOMEDIUM;
> >>
> >> And, usually, we should get rid of errno propagation. There are two cases
> >> here:
> >
> > The block layer consistently[*] uses -errno return values. Its
> > consistency is valuable, and I'm a bit reluctant to break it. Maybe a
> > new rule "returns -errno, except when it has an Error ** argument" could
> > work. I'd like to hear Kevin's advice on this.
>
> I'd rather not remove existing -errno returns if there's not good reason
> why we can't provide them (or can't do so easily at least).
Yes, I agree. Let me evaluate this calmly. I'll try to come with a better
proposal soon.
[Qemu-devel] [PATCH 05/18] block: add error parameter to bdrv_snapshot_list() and related functions, Pavel Hrdina, 2012/08/15
[Qemu-devel] [PATCH 04/18] block: add error parameter to bdrv_snapshot_delete() and related functions, Pavel Hrdina, 2012/08/15
[Qemu-devel] [PATCH 06/18] block: add error parameter to bdrv_snapshot_find(), Pavel Hrdina, 2012/08/15
[Qemu-devel] [PATCH 07/18] block: add error parameter to del_existing_snapshots(), Pavel Hrdina, 2012/08/15
[Qemu-devel] [PATCH 03/18] block: add error parameter to bdrv_snapshot_goto() and related functions, Pavel Hrdina, 2012/08/15
[Qemu-devel] [PATCH 08/18] savevm: add error parameter to qemu_savevm_state_begin(), Pavel Hrdina, 2012/08/15
[Qemu-devel] [PATCH 12/18] savevm: add error parameter to qemu_loadvm_state(), Pavel Hrdina, 2012/08/15