Am 27.09.2012 14:27, schrieb Paolo Bonzini: > Il 27/09/2012 14:18, Kevin Wolf ha scritto: >>>> >>>> Signed-off-by: Paolo Bonzini <pbonz...@redhat.com> >> I think there's a problem with terminology at least. What does "paused" >> really mean? Is it that the job has been requested to pause, or that it >> has actually yielded and is inactive? >> >> The commit message seems to use the latter semantics (which I would >> consider the intuitive one), > > You mean this: "Paused jobs cannot be canceled without first resuming > them". I can add a specification, like "(even if the job actually has > not reached the sleeping point and thus is still running)".
I actually meant "pause happens at the next sleeping point", which isn't unspecific at all. >> the QMP documentation leaves it unclear, >> but the code actually implements the former semantics. > > This code comment is clear: > > /** > * Set to true if the job is either paused, or will pause itself > * as soon as possible (if busy == true). > */ > bool paused; Yes, this one is a good and clear comment (and possibly I wouldn't even have noticed without this comment) > but this one can indeed use some improvement. > > /** > * block_job_is_paused: > * @job: The job being queried. > * > * Returns whether the job is currently paused. > */ > bool block_job_is_paused(BlockJob *job); > > > From the QMP client's point of view it doesn't really matter, does it? > > - even after a job that writes to disk X has "really" paused, you cannot > read or write disk X. It's still owned by QEMU, it hasn't been flushed, > it may play games like lazy refcounts. I'm not sure about this one. Consider things like a built-in NBD server. Probably we'll find more cases in the future, where some monitor command might seem to be safe while a job is paused. It makes me nervous that clients could make assumptions based on the paused state despite having no way to make sure that a job is actually stopped - the documentation doesn't even tell them about the fact that "paused" doesn't really mean what they think it means. > - what matters is that a resume undoes a pause, even if it is still > pending (which it does). Agreed, this part looks okay. Kevin