Kevin Wolf <kw...@redhat.com> writes: > Am 18.10.2012 15:49, schrieb Luiz Capitulino: >> On Thu, 18 Oct 2012 14:11:40 +0200 >> Kevin Wolf <kw...@redhat.com> wrote: >> >>> Am 17.10.2012 21:35, schrieb Luiz Capitulino: >>>> Signed-off-by: Luiz Capitulino <lcapitul...@redhat.com> >>>> --- >>>> qemu-img.c | 10 +++++++--- >>>> 1 file changed, 7 insertions(+), 3 deletions(-) >>>> >>>> diff --git a/qemu-img.c b/qemu-img.c >>>> index 12fb6c2..dfde588 100644 >>>> --- a/qemu-img.c >>>> +++ b/qemu-img.c >>>> @@ -302,6 +302,7 @@ static int img_create(int argc, char **argv) >>>> const char *base_filename = NULL; >>>> char *options = NULL; >>>> QEMUOptionParameter *params = NULL; >>>> + Error *local_err = NULL; >>>> >>>> for(;;) { >>>> c = getopt(argc, argv, "F:b:f:he6o:"); >>>> @@ -362,9 +363,12 @@ static int img_create(int argc, char **argv) >>>> goto out; >>>> } >>>> >>>> - ret = bdrv_img_create(filename, fmt, base_filename, base_fmt, >>>> - options, img_size, BDRV_O_FLAGS, ¶ms, NULL); >>>> - if (ret < 0) { >>>> + bdrv_img_create(filename, fmt, base_filename, base_fmt, >>>> + options, img_size, BDRV_O_FLAGS, ¶ms, &local_err); >>>> + if (error_is_set(&local_err)) { >>>> + fprintf(stderr, "qemu-img: %s\n", error_get_pretty(local_err)); >>> >>> This should use error_report() instead of adding the "qemu-img:" manually. >> >> If you want to do it for consistency with the current code then ok, >> I'll do it for v2. But I disagree qemu-img should keep using error_report(), >> printing to hmp for example is something beyond the interests of a tool like >> qemu-img. > > qemu-img doesn't have an HMP monitor, so it doesn't hurt either. If you > want to replace it, replace it with a copy of qemu-error.c that only > removes the monitor_vprintf() case. That is, in particular, leave all of > the loc_*() functionality there, because this is something that is meant > for use in command line parsers.
Strongly agreed on use of error_report(). Buys us uniform error messages that can point to the location causing the error. The fact that error_report() does the right thing in monitor context is detail. I don't see why purging a few monitor references from tools is worth copying the file. Stubbing out cur_mon and monitor_vprintf() in tools is just fine, in my opinion. > That qemu-img doesn't use these functions yet should be fixed. It has > some good use cases for them. Indeed.