| Message ID | 1384303994-26796-6-git-send-email-famz@redhat.com |
|---|---|
| State | New |
| Headers | show |
On Wed, Nov 13, 2013 at 08:53:14AM +0800, Fam Zheng wrote: > bdrv_get_info could fail. Add check before using the returned value. > > Signed-off-by: Fam Zheng <famz@redhat.com> > --- > block/mirror.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/block/mirror.c b/block/mirror.c > index 7b95acf..c0c321b 100644 > --- a/block/mirror.c > +++ b/block/mirror.c > @@ -320,8 +320,8 @@ static void coroutine_fn mirror_run(void *opaque) > bdrv_get_backing_filename(s->target, backing_filename, > sizeof(backing_filename)); > if (backing_filename[0] && !s->target->backing_hd) { > - bdrv_get_info(s->target, &bdi); > - if (s->granularity < bdi.cluster_size) { > + ret = bdrv_get_info(s->target, &bdi); > + if (ret == 0 && s->granularity < bdi.cluster_size) { For ret < 0, I think we should exit on error (at least in cases where ret < 0 && ret != -ENOTSUP). > s->buf_size = MAX(s->buf_size, bdi.cluster_size); > s->cow_bitmap = bitmap_new(length); > }
On 2013年11月13日 23:34, Jeff Cody wrote: > On Wed, Nov 13, 2013 at 08:53:14AM +0800, Fam Zheng wrote: >> bdrv_get_info could fail. Add check before using the returned value. >> >> Signed-off-by: Fam Zheng <famz@redhat.com> >> --- >> block/mirror.c | 4 ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/block/mirror.c b/block/mirror.c >> index 7b95acf..c0c321b 100644 >> --- a/block/mirror.c >> +++ b/block/mirror.c >> @@ -320,8 +320,8 @@ static void coroutine_fn mirror_run(void *opaque) >> bdrv_get_backing_filename(s->target, backing_filename, >> sizeof(backing_filename)); >> if (backing_filename[0] && !s->target->backing_hd) { >> - bdrv_get_info(s->target, &bdi); >> - if (s->granularity < bdi.cluster_size) { >> + ret = bdrv_get_info(s->target, &bdi); >> + if (ret == 0 && s->granularity < bdi.cluster_size) { > > For ret < 0, I think we should exit on error (at least in cases where > ret < 0 && ret != -ENOTSUP). > We only need the cluster_size here, which is optional to set up operation granularity. Exiting the block job because bdrv_get_info fails (which is not a critical error) is worse in fault tolerance sense, so I'd like to keep this way. Paolo, any ideas? Fam
diff --git a/block/mirror.c b/block/mirror.c index 7b95acf..c0c321b 100644 --- a/block/mirror.c +++ b/block/mirror.c @@ -320,8 +320,8 @@ static void coroutine_fn mirror_run(void *opaque) bdrv_get_backing_filename(s->target, backing_filename, sizeof(backing_filename)); if (backing_filename[0] && !s->target->backing_hd) { - bdrv_get_info(s->target, &bdi); - if (s->granularity < bdi.cluster_size) { + ret = bdrv_get_info(s->target, &bdi); + if (ret == 0 && s->granularity < bdi.cluster_size) { s->buf_size = MAX(s->buf_size, bdi.cluster_size); s->cow_bitmap = bitmap_new(length); }
bdrv_get_info could fail. Add check before using the returned value. Signed-off-by: Fam Zheng <famz@redhat.com> --- block/mirror.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)