On 12/11/2017 07:08 PM, Jeff Cody wrote: > On Mon, Dec 11, 2017 at 06:46:09PM -0500, John Snow wrote: >> If users set an unreasonably low speed (like one byte per second), the >> calculated delay may exceed many hours. While we like to punish users >> for asking for stupid things, we do also like to allow users to correct >> their wicked ways. >> >> When a user provides a new speed, kick the job to allow it to recalculate >> its delay. >> >> Signed-off-by: John Snow <js...@redhat.com> >> --- >> blockjob.c | 5 +++++ >> 1 file changed, 5 insertions(+) >> >> diff --git a/blockjob.c b/blockjob.c >> index 715c2c2680..43f01ad190 100644 >> --- a/blockjob.c >> +++ b/blockjob.c >> @@ -483,6 +483,7 @@ static void block_job_completed_txn_success(BlockJob >> *job) >> void block_job_set_speed(BlockJob *job, int64_t speed, Error **errp) >> { >> Error *local_err = NULL; >> + int64_t old_speed = job->speed; >> >> if (!job->driver->set_speed) { >> error_setg(errp, QERR_UNSUPPORTED); >> @@ -495,6 +496,10 @@ void block_job_set_speed(BlockJob *job, int64_t speed, >> Error **errp) >> } >> >> job->speed = speed; >> + /* Kick the job to recompute its delay */ >> + if ((speed > old_speed) && timer_pending(&job->sleep_timer)) { > > job->sleep_timer is protected by block_job_mutex (via > block_job_lock/unlock); is it safe for us to check it here outside the > mutex? >
My hunch is that in this specific case that it is; but only because of assumptions about holding the aio_context and the QEMU global mutex here. > But in any case, I think we could get rid of the timer_pending check, and > just always kick the job if we have a speed increase. block_job_enter() > should do the right thing (mutex protected check on job->busy and > job->sleep_timer). > I could lock it for inarguable correctness; I just didn't want to kick a job that didn't actually require any kicking to limit any potential problems from that interaction. (I'm fond of the extra conditional because I feel like it makes the intent of the kick explicit.) I can remove it. >> + block_job_enter(job); >> + } >> } >> >> void block_job_complete(BlockJob *job, Error **errp) >> -- >> 2.14.3 >> >