| Message ID | 20170706163828.24082-2-pbonzini@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 07/06/2017 11:38 AM, Paolo Bonzini wrote: > Code refactoring only. > > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> > --- > block/write-threshold.c | 28 ++++++++++++++++------------ > 1 file changed, 16 insertions(+), 12 deletions(-) > Reviewed-by: Eric Blake <eblake@redhat.com>
On Thu, Jul 06, 2017 at 06:38:18PM +0200, Paolo Bonzini wrote: > Code refactoring only. > > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> > --- > block/write-threshold.c | 28 ++++++++++++++++------------ > 1 file changed, 16 insertions(+), 12 deletions(-) > > diff --git a/block/write-threshold.c b/block/write-threshold.c > index 0bd1a01c86..c8ebc32b4d 100644 > --- a/block/write-threshold.c > +++ b/block/write-threshold.c > @@ -37,18 +37,22 @@ static void write_threshold_disable(BlockDriverState *bs) > } > } > > +static uint64_t exceeded_amount(const BdrvTrackedRequest *req, > + uint64_t thres) Not a reason to respin, but I would prefer a more specific name so the intent of the code is easier to understand: exceeded_threshold() instead of exceeded_amount(). Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
On 10/07/2017 15:21, Stefan Hajnoczi wrote: > On Thu, Jul 06, 2017 at 06:38:18PM +0200, Paolo Bonzini wrote: >> Code refactoring only. >> >> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> >> --- >> block/write-threshold.c | 28 ++++++++++++++++------------ >> 1 file changed, 16 insertions(+), 12 deletions(-) >> >> diff --git a/block/write-threshold.c b/block/write-threshold.c >> index 0bd1a01c86..c8ebc32b4d 100644 >> --- a/block/write-threshold.c >> +++ b/block/write-threshold.c >> @@ -37,18 +37,22 @@ static void write_threshold_disable(BlockDriverState *bs) >> } >> } >> >> +static uint64_t exceeded_amount(const BdrvTrackedRequest *req, >> + uint64_t thres) > > Not a reason to respin, but I would prefer a more specific name so the > intent of the code is easier to understand: exceeded_threshold() instead > of exceeded_amount(). > > Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> It's the amount by which the request exceeds threshold... Not sure what name would be best. Paolo
On 07/10/2017 11:16 AM, Paolo Bonzini wrote: > On 10/07/2017 15:21, Stefan Hajnoczi wrote: >> On Thu, Jul 06, 2017 at 06:38:18PM +0200, Paolo Bonzini wrote: >>> Code refactoring only. >>> >>> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> >>> --- >>> block/write-threshold.c | 28 ++++++++++++++++------------ >>> 1 file changed, 16 insertions(+), 12 deletions(-) >>> >>> diff --git a/block/write-threshold.c b/block/write-threshold.c >>> index 0bd1a01c86..c8ebc32b4d 100644 >>> --- a/block/write-threshold.c >>> +++ b/block/write-threshold.c >>> @@ -37,18 +37,22 @@ static void write_threshold_disable(BlockDriverState *bs) >>> } >>> } >>> >>> +static uint64_t exceeded_amount(const BdrvTrackedRequest *req, >>> + uint64_t thres) >> >> Not a reason to respin, but I would prefer a more specific name so the >> intent of the code is easier to understand: exceeded_threshold() instead >> of exceeded_amount(). >> >> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> > > It's the amount by which the request exceeds threshold... Not sure what > name would be best. threshold_overage ?
diff --git a/block/write-threshold.c b/block/write-threshold.c index 0bd1a01c86..c8ebc32b4d 100644 --- a/block/write-threshold.c +++ b/block/write-threshold.c @@ -37,18 +37,22 @@ static void write_threshold_disable(BlockDriverState *bs) } } +static uint64_t exceeded_amount(const BdrvTrackedRequest *req, + uint64_t thres) +{ + if (thres > 0 && req->offset + req->bytes > thres) { + return req->offset + req->bytes - thres; + } else { + return 0; + } +} + uint64_t bdrv_write_threshold_exceeded(const BlockDriverState *bs, const BdrvTrackedRequest *req) { - if (bdrv_write_threshold_is_set(bs)) { - if (req->offset > bs->write_threshold_offset) { - return (req->offset - bs->write_threshold_offset) + req->bytes; - } - if ((req->offset + req->bytes) > bs->write_threshold_offset) { - return (req->offset + req->bytes) - bs->write_threshold_offset; - } - } - return 0; + uint64_t thres = bdrv_write_threshold_get(bs); + + return exceeded_amount(req, thres); } static int coroutine_fn before_write_notify(NotifierWithReturn *notifier, @@ -56,14 +60,14 @@ static int coroutine_fn before_write_notify(NotifierWithReturn *notifier, { BdrvTrackedRequest *req = opaque; BlockDriverState *bs = req->bs; - uint64_t amount = 0; + uint64_t thres = bdrv_write_threshold_get(bs); + uint64_t amount = exceeded_amount(req, thres); - amount = bdrv_write_threshold_exceeded(bs, req); if (amount > 0) { qapi_event_send_block_write_threshold( bs->node_name, amount, - bs->write_threshold_offset, + thres, &error_abort); /* autodisable to avoid flooding the monitor */
Code refactoring only. Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> --- block/write-threshold.c | 28 ++++++++++++++++------------ 1 file changed, 16 insertions(+), 12 deletions(-)