[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [Qemu-block] [PATCH v3 1/2] blockdev: Error out on nega
From: |
Fam Zheng |
Subject: |
Re: [Qemu-devel] [Qemu-block] [PATCH v3 1/2] blockdev: Error out on negative throttling option values |
Date: |
Fri, 15 Jan 2016 10:06:46 +0800 |
User-agent: |
Mutt/1.5.21 (2010-09-15) |
On Thu, 01/14 16:50, Max Reitz wrote:
> On 14.01.2016 16:46, Max Reitz wrote:
> > On 14.01.2016 05:08, Fam Zheng wrote:
> >> The implicit casting from unsigned int to double changes negative values
> >> into large positive numbers and accepts them. We should instead print
> >> an error.
> >>
> >> Check the number range so this case is caught and reported.
> >>
> >> Signed-off-by: Fam Zheng <address@hidden>
> >> ---
> >> blockdev.c | 3 ++-
> >> include/qemu/throttle.h | 2 ++
> >> util/throttle.c | 16 ++++++----------
> >> 3 files changed, 10 insertions(+), 11 deletions(-)
> >
> > Reviewed-by: Max Reitz <address@hidden>
> >
> >> diff --git a/blockdev.c b/blockdev.c
> >> index 2df0c6d..1afef87 100644
> >> --- a/blockdev.c
> >> +++ b/blockdev.c
> >> @@ -348,7 +348,8 @@ static bool check_throttle_config(ThrottleConfig *cfg,
> >> Error **errp)
> >> }
> >>
> >> if (!throttle_is_valid(cfg)) {
> >> - error_setg(errp, "bps/iops/maxs values must be 0 or greater");
> >> + error_setg(errp, "bps/iops/maxs values must be within [0, %"
> >> PRId64
>
> Pre-existing, but you might want to fix this to "bps/iops/max" while
> touching this line.
Makes sense, I'll apply your rev-by and fix this in v4.
Fam
>
> Max
>
> >> + ")", (int64_t)THROTTLE_VALUE_MAX);
> >
> > I personally would have liked a simpler %lli and no cast, but I can see
> > why you want an explicit int64_t here.
> >
> >> return false;
> >> }
> >>
> >> diff --git a/include/qemu/throttle.h b/include/qemu/throttle.h
> >> index 12faaad..d0c98ed 100644
> >> --- a/include/qemu/throttle.h
> >> +++ b/include/qemu/throttle.h
> >> @@ -29,6 +29,8 @@
> >> #include "qemu-common.h"
> >> #include "qemu/timer.h"
> >>
> >> +#define THROTTLE_VALUE_MAX 1000000000000000LL
> >
> > <pedantic>
> > But then you could use UINT64_C(1000000000000000) here.
> > </pedantic>
> >
>
>