Commit d72915c6 authored by Stefan Hajnoczi's avatar Stefan Hajnoczi
Browse files

throttle: make throttle_config(throttle_get_config()) symmetric 



Throttling has a weird property that throttle_get_config() does not
always return the same throttling settings that were given with
throttle_config().  In other words, the set and get functions aren't
symmetric.

If .max is 0 then the throttling code assigns a default value of .avg /
10 in throttle_config().  This is an implementation detail of the
throttling algorithm.  When throttle_get_config() is called the .max
value returned should still be 0.

Users are exposed to this quirk via "info block" or "query-block"
monitor commands.  This has caused confusion because it looks like a bug
when an unexpected value is reported.

This patch hides the .max value adjustment in throttle_get_config() and
updates test-throttle.c appropriately.

Reported-by: default avatarNini Gu <ngu@redhat.com>
Signed-off-by: default avatarStefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: default avatarAlberto Garcia <berto@igalia.com>
Message-id: 20170301115026.22621-4-stefanha@redhat.com
Signed-off-by: default avatarStefan Hajnoczi <stefanha@redhat.com>
parent ab08aec4

tests/test-throttle.c

+4 −4
Original line number Diff line number Diff line
@@ -205,8 +205,8 @@ static void test_config_functions(void) 
    orig_cfg.buckets[THROTTLE_OPS_READ].avg  = 69;

    orig_cfg.buckets[THROTTLE_OPS_WRITE].avg = 23;



    orig_cfg.buckets[THROTTLE_BPS_TOTAL].max = 0;  /* should be corrected */

    orig_cfg.buckets[THROTTLE_BPS_READ].max  = 56; /* should not be corrected */

    orig_cfg.buckets[THROTTLE_BPS_TOTAL].max = 0;

    orig_cfg.buckets[THROTTLE_BPS_READ].max  = 56;

    orig_cfg.buckets[THROTTLE_BPS_WRITE].max = 120;



    orig_cfg.buckets[THROTTLE_OPS_TOTAL].max = 150;
@@ -246,8 +246,8 @@ static void test_config_functions(void) 
    g_assert(final_cfg.buckets[THROTTLE_OPS_READ].avg  == 69);

    g_assert(final_cfg.buckets[THROTTLE_OPS_WRITE].avg == 23);



    g_assert(final_cfg.buckets[THROTTLE_BPS_TOTAL].max == 15.3); /* fixed */

    g_assert(final_cfg.buckets[THROTTLE_BPS_READ].max  == 56);   /* not fixed */

    g_assert(final_cfg.buckets[THROTTLE_BPS_TOTAL].max == 0);

    g_assert(final_cfg.buckets[THROTTLE_BPS_READ].max  == 56);

    g_assert(final_cfg.buckets[THROTTLE_BPS_WRITE].max == 120);



    g_assert(final_cfg.buckets[THROTTLE_OPS_TOTAL].max == 150);

util/throttle.c

+14 −0
Original line number Diff line number Diff line
@@ -380,6 +380,14 @@ static void throttle_fix_bucket(LeakyBucket *bkt) 
    }

}



/* undo internal bucket parameter changes (see throttle_fix_bucket()) */

static void throttle_unfix_bucket(LeakyBucket *bkt)

{

    if (bkt->max < bkt->avg) {

        bkt->max = 0;

    }

}



/* take care of canceling a timer */

static void throttle_cancel_timer(QEMUTimer *timer)

{
@@ -420,7 +428,13 @@ void throttle_config(ThrottleState *ts, 
 */

void throttle_get_config(ThrottleState *ts, ThrottleConfig *cfg)

{

    int i;



    *cfg = ts->cfg;



    for (i = 0; i < BUCKETS_COUNT; i++) {

        throttle_unfix_bucket(&cfg->buckets[i]);

    }

}