| Message ID | 1381559592-6309-1-git-send-email-famz@redhat.com |
|---|---|
| State | New |
| Headers | show |
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; }
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
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) {
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(-)