* [Qemu-devel] [PATCH] block/raw-posix: Fix error_report of mounting message
@ 2016-06-01 7:08 Wei Jiangang
2016-06-01 7:23 ` Fam Zheng
2016-06-02 7:57 ` Markus Armbruster
0 siblings, 2 replies; 8+ messages in thread
From: Wei Jiangang @ 2016-06-01 7:08 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-block, qemu-trivial, kwolf, mreitz, Wei Jiangang
Use a single error_printf to replace triple error_report.
Signed-off-by: Wei Jiangang <weijg.fnst@cn.fujitsu.com>
---
block/raw-posix.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/block/raw-posix.c b/block/raw-posix.c
index a4f5a1b..141b01a 100644
--- a/block/raw-posix.c
+++ b/block/raw-posix.c
@@ -2061,11 +2061,11 @@ static bool setup_cdrom(char *bsd_path, Error **errp)
/* Prints directions on mounting and unmounting a device */
static void print_unmounting_directions(const char *file_name)
{
- error_report("If device %s is mounted on the desktop, unmount"
- " it first before using it in QEMU", file_name);
- error_report("Command to unmount device: diskutil unmountDisk %s",
- file_name);
- error_report("Command to mount device: diskutil mountDisk %s", file_name);
+ error_printf("If device %s is mounted on the desktop, unmount"
+ " it first before using it in QEMU\n"
+ "Command to unmount device: diskutil unmountDisk %s\n"
+ "Command to mount device: diskutil mountDisk %s\n",
+ file_name, file_name, file_name);
}
#endif /* defined(__APPLE__) && defined(__MACH__) */
--
1.9.3
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH] block/raw-posix: Fix error_report of mounting message
2016-06-01 7:08 [Qemu-devel] [PATCH] block/raw-posix: Fix error_report of mounting message Wei Jiangang
@ 2016-06-01 7:23 ` Fam Zheng
2016-06-02 3:04 ` Wei, Jiangang
2016-06-02 7:57 ` Markus Armbruster
1 sibling, 1 reply; 8+ messages in thread
From: Fam Zheng @ 2016-06-01 7:23 UTC (permalink / raw)
To: Wei Jiangang; +Cc: qemu-devel, qemu-trivial, kwolf, qemu-block, mreitz
On Wed, 06/01 15:08, Wei Jiangang wrote:
> Use a single error_printf to replace triple error_report.
>
> Signed-off-by: Wei Jiangang <weijg.fnst@cn.fujitsu.com>
> ---
> block/raw-posix.c | 10 +++++-----
> 1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/block/raw-posix.c b/block/raw-posix.c
> index a4f5a1b..141b01a 100644
> --- a/block/raw-posix.c
> +++ b/block/raw-posix.c
> @@ -2061,11 +2061,11 @@ static bool setup_cdrom(char *bsd_path, Error **errp)
> /* Prints directions on mounting and unmounting a device */
> static void print_unmounting_directions(const char *file_name)
> {
> - error_report("If device %s is mounted on the desktop, unmount"
> - " it first before using it in QEMU", file_name);
> - error_report("Command to unmount device: diskutil unmountDisk %s",
> - file_name);
> - error_report("Command to mount device: diskutil mountDisk %s", file_name);
> + error_printf("If device %s is mounted on the desktop, unmount"
> + " it first before using it in QEMU\n"
> + "Command to unmount device: diskutil unmountDisk %s\n"
> + "Command to mount device: diskutil mountDisk %s\n",
> + file_name, file_name, file_name);
I'd say it's less readable with a worse alignment to the parameters.
Fam
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH] block/raw-posix: Fix error_report of mounting message
2016-06-01 7:23 ` Fam Zheng
@ 2016-06-02 3:04 ` Wei, Jiangang
2016-06-02 3:15 ` Fam Zheng
0 siblings, 1 reply; 8+ messages in thread
From: Wei, Jiangang @ 2016-06-02 3:04 UTC (permalink / raw)
To: famz; +Cc: kwolf, qemu-devel, qemu-trivial, qemu-block, mreitz
On Wed, 2016-06-01 at 15:23 +0800, Fam Zheng wrote:
> On Wed, 06/01 15:08, Wei Jiangang wrote:
> > Use a single error_printf to replace triple error_report.
> >
> > Signed-off-by: Wei Jiangang <weijg.fnst@cn.fujitsu.com>
> > ---
> > block/raw-posix.c | 10 +++++-----
> > 1 file changed, 5 insertions(+), 5 deletions(-)
> >
> > diff --git a/block/raw-posix.c b/block/raw-posix.c
> > index a4f5a1b..141b01a 100644
> > --- a/block/raw-posix.c
> > +++ b/block/raw-posix.c
> > @@ -2061,11 +2061,11 @@ static bool setup_cdrom(char *bsd_path, Error **errp)
> > /* Prints directions on mounting and unmounting a device */
> > static void print_unmounting_directions(const char *file_name)
> > {
> > - error_report("If device %s is mounted on the desktop, unmount"
> > - " it first before using it in QEMU", file_name);
> > - error_report("Command to unmount device: diskutil unmountDisk %s",
> > - file_name);
> > - error_report("Command to mount device: diskutil mountDisk %s", file_name);
> > + error_printf("If device %s is mounted on the desktop, unmount"
> > + " it first before using it in QEMU\n"
> > + "Command to unmount device: diskutil unmountDisk %s\n"
> > + "Command to mount device: diskutil mountDisk %s\n",
> > + file_name, file_name, file_name);
>
> I'd say it's less readable with a worse alignment to the parameters.
Hi, Fam
Thanks for your comment.
You mean every error_report() followed by file_name is more readable ?
or my codes dosen't follow a good alignment format?
Wei
>
> Fam
>
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH] block/raw-posix: Fix error_report of mounting message
2016-06-02 3:04 ` Wei, Jiangang
@ 2016-06-02 3:15 ` Fam Zheng
0 siblings, 0 replies; 8+ messages in thread
From: Fam Zheng @ 2016-06-02 3:15 UTC (permalink / raw)
To: Wei, Jiangang; +Cc: kwolf, qemu-trivial, qemu-devel, qemu-block, mreitz
On Thu, 06/02 03:04, Wei, Jiangang wrote:
> On Wed, 2016-06-01 at 15:23 +0800, Fam Zheng wrote:
> > On Wed, 06/01 15:08, Wei Jiangang wrote:
> > > Use a single error_printf to replace triple error_report.
> > >
> > > Signed-off-by: Wei Jiangang <weijg.fnst@cn.fujitsu.com>
> > > ---
> > > block/raw-posix.c | 10 +++++-----
> > > 1 file changed, 5 insertions(+), 5 deletions(-)
> > >
> > > diff --git a/block/raw-posix.c b/block/raw-posix.c
> > > index a4f5a1b..141b01a 100644
> > > --- a/block/raw-posix.c
> > > +++ b/block/raw-posix.c
> > > @@ -2061,11 +2061,11 @@ static bool setup_cdrom(char *bsd_path, Error **errp)
> > > /* Prints directions on mounting and unmounting a device */
> > > static void print_unmounting_directions(const char *file_name)
> > > {
> > > - error_report("If device %s is mounted on the desktop, unmount"
> > > - " it first before using it in QEMU", file_name);
> > > - error_report("Command to unmount device: diskutil unmountDisk %s",
> > > - file_name);
> > > - error_report("Command to mount device: diskutil mountDisk %s", file_name);
> > > + error_printf("If device %s is mounted on the desktop, unmount"
> > > + " it first before using it in QEMU\n"
> > > + "Command to unmount device: diskutil unmountDisk %s\n"
> > > + "Command to mount device: diskutil mountDisk %s\n",
> > > + file_name, file_name, file_name);
> >
> > I'd say it's less readable with a worse alignment to the parameters.
> Hi, Fam
>
> Thanks for your comment.
> You mean every error_report() followed by file_name is more readable ?
> or my codes dosen't follow a good alignment format?
>
I'm happy with either way. :)
Fam
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH] block/raw-posix: Fix error_report of mounting message
2016-06-01 7:08 [Qemu-devel] [PATCH] block/raw-posix: Fix error_report of mounting message Wei Jiangang
2016-06-01 7:23 ` Fam Zheng
@ 2016-06-02 7:57 ` Markus Armbruster
2016-06-02 10:46 ` Wei, Jiangang
1 sibling, 1 reply; 8+ messages in thread
From: Markus Armbruster @ 2016-06-02 7:57 UTC (permalink / raw)
To: Wei Jiangang; +Cc: qemu-devel, qemu-trivial, kwolf, qemu-block, mreitz
Wei Jiangang <weijg.fnst@cn.fujitsu.com> writes:
> Use a single error_printf to replace triple error_report.
>
> Signed-off-by: Wei Jiangang <weijg.fnst@cn.fujitsu.com>
> ---
> block/raw-posix.c | 10 +++++-----
> 1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/block/raw-posix.c b/block/raw-posix.c
> index a4f5a1b..141b01a 100644
> --- a/block/raw-posix.c
> +++ b/block/raw-posix.c
> @@ -2061,11 +2061,11 @@ static bool setup_cdrom(char *bsd_path, Error **errp)
> /* Prints directions on mounting and unmounting a device */
> static void print_unmounting_directions(const char *file_name)
> {
> - error_report("If device %s is mounted on the desktop, unmount"
> - " it first before using it in QEMU", file_name);
> - error_report("Command to unmount device: diskutil unmountDisk %s",
> - file_name);
> - error_report("Command to mount device: diskutil mountDisk %s", file_name);
> + error_printf("If device %s is mounted on the desktop, unmount"
> + " it first before using it in QEMU\n"
> + "Command to unmount device: diskutil unmountDisk %s\n"
> + "Command to mount device: diskutil mountDisk %s\n",
> + file_name, file_name, file_name);
> }
>
> #endif /* defined(__APPLE__) && defined(__MACH__) */
This is less wrong :)
Beware, I have no suitable machine to actually test this, so the
following is based on code inspection.
print_unmounting_directions() is used by hdev_open(). hdev_open() is a
BlockDriver method bdrv_file_open(), and as such returns errors to its
caller via parameter Error **errp. The caller may or may not report
errors it gets from hdev_open().
If it reports them, then print_unmounting_directions()'s output
"overtakes" the error message, i.e. the "If device ... is mounted ..."
hint is printed before the error message. Wrong, it should be printed
after the error message.
If it doesn't report them, the hint gets printed anyway. More wrong.
In any case, printing each line of the hint with error_report() is also
wrong, because that prefixes each line with program name and location
information. Your patch fixes this part.
A more complete fix uses error_append_hint(errp, ...) instead of
error_printf(). hdev_open() needs to pass errp to
print_unmounting_directions() for that. errp must have an error set, or
else error_append_hint() makes no sense.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH] block/raw-posix: Fix error_report of mounting message
2016-06-02 7:57 ` Markus Armbruster
@ 2016-06-02 10:46 ` Wei, Jiangang
2016-06-03 9:00 ` Fam Zheng
0 siblings, 1 reply; 8+ messages in thread
From: Wei, Jiangang @ 2016-06-02 10:46 UTC (permalink / raw)
To: armbru; +Cc: kwolf, qemu-devel, qemu-trivial, qemu-block, mreitz
On Thu, 2016-06-02 at 09:57 +0200, Markus Armbruster wrote:
> Wei Jiangang <weijg.fnst@cn.fujitsu.com> writes:
>
> > Use a single error_printf to replace triple error_report.
> >
> > Signed-off-by: Wei Jiangang <weijg.fnst@cn.fujitsu.com>
> > ---
> > block/raw-posix.c | 10 +++++-----
> > 1 file changed, 5 insertions(+), 5 deletions(-)
> >
> > diff --git a/block/raw-posix.c b/block/raw-posix.c
> > index a4f5a1b..141b01a 100644
> > --- a/block/raw-posix.c
> > +++ b/block/raw-posix.c
> > @@ -2061,11 +2061,11 @@ static bool setup_cdrom(char *bsd_path, Error **errp)
> > /* Prints directions on mounting and unmounting a device */
> > static void print_unmounting_directions(const char *file_name)
> > {
> > - error_report("If device %s is mounted on the desktop, unmount"
> > - " it first before using it in QEMU", file_name);
> > - error_report("Command to unmount device: diskutil unmountDisk %s",
> > - file_name);
> > - error_report("Command to mount device: diskutil mountDisk %s", file_name);
> > + error_printf("If device %s is mounted on the desktop, unmount"
> > + " it first before using it in QEMU\n"
> > + "Command to unmount device: diskutil unmountDisk %s\n"
> > + "Command to mount device: diskutil mountDisk %s\n",
> > + file_name, file_name, file_name);
> > }
> >
> > #endif /* defined(__APPLE__) && defined(__MACH__) */
>
> This is less wrong :)
>
> Beware, I have no suitable machine to actually test this, so the
> following is based on code inspection.
>
> print_unmounting_directions() is used by hdev_open(). hdev_open() is a
> BlockDriver method bdrv_file_open(), and as such returns errors to its
> caller via parameter Error **errp. The caller may or may not report
> errors it gets from hdev_open().
>
> If it reports them, then print_unmounting_directions()'s output
> "overtakes" the error message, i.e. the "If device ... is mounted ..."
> hint is printed before the error message. Wrong, it should be printed
> after the error message.
>
> If it doesn't report them, the hint gets printed anyway. More wrong.
>
> In any case, printing each line of the hint with error_report() is also
> wrong, because that prefixes each line with program name and location
> information. Your patch fixes this part.
>
> A more complete fix uses error_append_hint(errp, ...) instead of
> error_printf(). hdev_open() needs to pass errp to
> print_unmounting_directions() for that. errp must have an error set, or
> else error_append_hint() makes no sense.
I think you gave me a wonderful lesson about how to deal with error
report.
Thank you very much.
My intention is only to avoid the output of multiple location
information.
I'll check it again.
Thanks,
wei
>
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH] block/raw-posix: Fix error_report of mounting message
2016-06-02 10:46 ` Wei, Jiangang
@ 2016-06-03 9:00 ` Fam Zheng
2016-06-03 9:56 ` Wei, Jiangang
0 siblings, 1 reply; 8+ messages in thread
From: Fam Zheng @ 2016-06-03 9:00 UTC (permalink / raw)
To: Wei, Jiangang; +Cc: qemu-devel, qemu-block
On Thu, 06/02 10:46, Wei, Jiangang wrote:
> My intention is only to avoid the output of multiple location
> information.
Then I think this should have gone into the commit message.
Fam
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH] block/raw-posix: Fix error_report of mounting message
2016-06-03 9:00 ` Fam Zheng
@ 2016-06-03 9:56 ` Wei, Jiangang
0 siblings, 0 replies; 8+ messages in thread
From: Wei, Jiangang @ 2016-06-03 9:56 UTC (permalink / raw)
To: famz; +Cc: qemu-devel, qemu-block
On Fri, 2016-06-03 at 17:00 +0800, Fam Zheng wrote:
> On Thu, 06/02 10:46, Wei, Jiangang wrote:
> > My intention is only to avoid the output of multiple location
> > information.
>
> Then I think this should have gone into the commit message.
OK, I'll make more detailed description in next version.
>
> Fam
>
>
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2016-06-03 9:57 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-06-01 7:08 [Qemu-devel] [PATCH] block/raw-posix: Fix error_report of mounting message Wei Jiangang
2016-06-01 7:23 ` Fam Zheng
2016-06-02 3:04 ` Wei, Jiangang
2016-06-02 3:15 ` Fam Zheng
2016-06-02 7:57 ` Markus Armbruster
2016-06-02 10:46 ` Wei, Jiangang
2016-06-03 9:00 ` Fam Zheng
2016-06-03 9:56 ` Wei, Jiangang
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.