[libvirt] [Qemu-devel] virDomainBlockJobAbort and block_job_cancel

[Stefan Hajnoczi]

On Thu, Nov 24, 2011 at 5:31 AM, Daniel Veillard <veillard at redhat.com> wrote:
> On Wed, Nov 23, 2011 at 09:04:50AM -0700, Eric Blake wrote:
>> On 11/23/2011 07:48 AM, Stefan Hajnoczi wrote:
>> > This means that virDomainBlockJobAbort() returns to the client without
>> > a guarantee that the job has completed.  If the client enumerates jobs
>> > it may still see a job that has not finished cancelling.  The client
>> > must register a handler for the BLOCK_JOB_CANCELLED event if it wants
>> > to know when the job really goes away.  The BLOCK_JOB_CANCELLED event
>> > has the same fields as the BLOCK_JOB_COMPLETED event, except it lacks
>> > the optional "error" message field.
>> >
>> > The impact on clients is that they need to add a BLOCK_JOB_CANCELLED
>> > handler if they really want to wait.  Most clients today (not many
>> > exist) will be fine without waiting for cancellation.
>> >
>> > Any objections or thoughts on this?
>>
>> virDomainBlockJobAbort() thankfully has an 'unsigned int flags'
>> argument.  For backwards-compatibility, I suggest we use it:
>>
>> calling virDomainBlockJobAbort(,0) maintains old blocking behavior, and
>> we document that blocking until things abort may render the rest of
>> interactions with the domain unresponsive.
>>
>> The new virDomainBlockJobAbort(,VIR_DOMAIN_BLOCK_JOB_ABORT_ASYNC) would
>> then implement your new proposed semantics of returning immediately once
>> the cancellation has been requested, even if it hasn't been acted on yet.
>>
>> Maybe you could convince me to swap the flags: have 0 change semantics
>> to non-blocking, and a new flag to request blocking, where callers that
>> care have to try the flag, and if the flag is unsupported then they know
>> they are talking to older libvirtd where the behavior is blocking by
>> default, but that's a bit riskier.
>
>  Agreed, I would rather not change the current call semantic,
> but an ASYNC flag would be a really good addition. We can document
> the risk of not using it in the function description and suggest
> new applications use ASYNC flag.

Yep, that's a nice suggestion and solves the problem.

Stefan