On Fri, 31 Aug 2012 14:18:55 +0200 Kevin Wolf <kw...@redhat.com> wrote:
> Am 31.08.2012 08:26, schrieb Markus Armbruster: > > Luiz Capitulino <lcapitul...@redhat.com> writes: > > > >> On Wed, 15 Aug 2012 09:41:43 +0200 > >> Pavel Hrdina <phrd...@redhat.com> wrote: > >> > >>> Signed-off-by: Pavel Hrdina <phrd...@redhat.com> > >>> --- > >>> 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.