On 03/22/2013 07:16 AM, Pavel Hrdina wrote: > If we provide error message, we should also provide a return code. > In some cases we could not care about any error message and the return > code is enough for as. > > Signed-off-by: Pavel Hrdina <phrd...@redhat.com> > ---
> +++ b/qemu-img.c
> @@ -1943,6 +1943,7 @@ static int img_snapshot(int argc, char **argv)
> int action = 0;
> qemu_timeval tv;
> bool quiet = false;
> + Error *local_err;
>
> bdrv_oflags = BDRV_O_FLAGS | BDRV_O_RDWR;
> /* Parse commandline parameters */
> @@ -2019,10 +2020,10 @@ static int img_snapshot(int argc, char **argv)
> sn.date_sec = tv.tv_sec;
> sn.date_nsec = tv.tv_usec * 1000;
>
> - ret = bdrv_snapshot_create(bs, &sn, NULL);
> + local_err = NULL;
> + ret = bdrv_snapshot_create(bs, &sn, &local_err);
> if (ret) {
Isn't it true that 'ret' is non-zero iff 'local_err' is no longer NULL?
In which case, why not have the functions return 'void' instead of an
integer, and use the null-ness of local_err be your test of whether
bdrv_snapshot_create failed.
> - error_report("Could not create snapshot '%s': %d (%s)",
> - snapshot_name, ret, strerror(-ret));
> + qemu_img_handle_error(local_err);
Same question as in 8/12 - you have already guaranteed that
qemu_img_handle_error will only be called if local_err is nonnull, so
does that helper function have to do a sanity check?
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
