[Qemu-devel] [PATCH] mirror: drop local_err in mirror_compelte

[Qemu-devel] [PATCH] mirror: drop local_err in mirror_compelte

[Fam Zheng]

There is errp passed in, so no need for local_err and error_propagate.

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 block/mirror.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/block/mirror.c b/block/mirror.c
index 7b95acf..f2e9558 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -505,15 +505,13 @@ static void mirror_iostatus_reset(BlockJob *job)
 static void mirror_complete(BlockJob *job, Error **errp)
 {
     MirrorBlockJob *s = container_of(job, MirrorBlockJob, common);
-    Error *local_err = NULL;
     int ret;
 
-    ret = bdrv_open_backing_file(s->target, NULL, &local_err);
+    ret = bdrv_open_backing_file(s->target, NULL, errp);
     if (ret < 0) {
         char backing_filename[PATH_MAX];
         bdrv_get_full_backing_filename(s->target, backing_filename,
                                        sizeof(backing_filename));
-        error_propagate(errp, local_err);
         return;
     }
     if (!s->synced) {
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH] mirror: drop local_err in mirror_compelte

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 1455 bytes --]

On 10/12/2013 12:33 AM, Fam Zheng wrote:
> There is errp passed in, so no need for local_err and error_propagate.

s/compelte/complete/ in the subject

> 
> Signed-off-by: Fam Zheng <famz@redhat.com>
> ---
>  block/mirror.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)

> 
> diff --git a/block/mirror.c b/block/mirror.c
> index 7b95acf..f2e9558 100644
> --- a/block/mirror.c
> +++ b/block/mirror.c
> @@ -505,15 +505,13 @@ static void mirror_iostatus_reset(BlockJob *job)
>  static void mirror_complete(BlockJob *job, Error **errp)
>  {
>      MirrorBlockJob *s = container_of(job, MirrorBlockJob, common);
> -    Error *local_err = NULL;
>      int ret;
>  
> -    ret = bdrv_open_backing_file(s->target, NULL, &local_err);
> +    ret = bdrv_open_backing_file(s->target, NULL, errp);
>      if (ret < 0) {
>          char backing_filename[PATH_MAX];
>          bdrv_get_full_backing_filename(s->target, backing_filename,
>                                         sizeof(backing_filename));
> -        error_propagate(errp, local_err);
>          return;

What purpose does backing_filename serve?  It looks like it was rendered
dead code with the introduction of commit 34b5d2c, and that you could
simplify further to:

if (bdrv_open_backing_file(s->target, NULL, errp) < 0) {
    return;
}

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 621 bytes --]

Re: [Qemu-devel] [PATCH] mirror: drop local_err in mirror_compelte

[Fam Zheng]

On Mon, 10/14 07:52, Eric Blake wrote:
> On 10/12/2013 12:33 AM, Fam Zheng wrote:
> > There is errp passed in, so no need for local_err and error_propagate.
> 
> s/compelte/complete/ in the subject
> 
> > 
> > Signed-off-by: Fam Zheng <famz@redhat.com>
> > ---
> >  block/mirror.c | 4 +---
> >  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> > 
> > diff --git a/block/mirror.c b/block/mirror.c
> > index 7b95acf..f2e9558 100644
> > --- a/block/mirror.c
> > +++ b/block/mirror.c
> > @@ -505,15 +505,13 @@ static void mirror_iostatus_reset(BlockJob *job)
> >  static void mirror_complete(BlockJob *job, Error **errp)
> >  {
> >      MirrorBlockJob *s = container_of(job, MirrorBlockJob, common);
> > -    Error *local_err = NULL;
> >      int ret;
> >  
> > -    ret = bdrv_open_backing_file(s->target, NULL, &local_err);
> > +    ret = bdrv_open_backing_file(s->target, NULL, errp);
> >      if (ret < 0) {
> >          char backing_filename[PATH_MAX];
> >          bdrv_get_full_backing_filename(s->target, backing_filename,
> >                                         sizeof(backing_filename));
> > -        error_propagate(errp, local_err);
> >          return;
> 
> What purpose does backing_filename serve?  It looks like it was rendered
> dead code with the introduction of commit 34b5d2c, and that you could
> simplify further to:
> 
> if (bdrv_open_backing_file(s->target, NULL, errp) < 0) {
>     return;
> }
> 

Good idea, thanks!

Fam