[libvirt] [PATCH 3/5] qemu_domain: Introduce qemuDomainObjBeginJobInstant

[John Ferlan]

[...]

>>>      qemuDomainObjPrivatePtr priv = obj->privateData;
>>>      unsigned long long now;
>>> @@ -6395,12 +6400,18 @@ qemuDomainObjBeginJobInternal(virQEMUDriverPtr driver,
>>>      }
>>>  
>>>      while (!nested && !qemuDomainNestedJobAllowed(priv, job)) {
>>> +        if (instant)
>>> +            goto cleanup;
>>> +
>>
>> If async is not supported for this (as I believe seen in the new API),
>> then is this path possible?
> 
> Of course. For instance, if there's an async job running and another
> thread is trying to start a sync, non-nested job that is not allowed for
> the async job. Say, thread A is doing a snapshot which sets job mask
> (=allowed sync jobs) to: query, destroy, abort, suspend, migration-op.
> Then there's a thread B which is executing qemuDomainSetUserPassword().
> This threads tries to grab modify job. However, since grabbing modify is
> not allowed (due to job mask) thread B sits and waits until thread A
> releases the async job. At that point, thread B can safely grab modify
> job because there's no other (nor async) job running.
> 
> Long story short, not only synchronous jobs serialize, also async (and
> any combination of those two) do.
> 

Ah... OK.

[...]

>>> +int
>>> +qemuDomainObjBeginJobInstant(virQEMUDriverPtr driver,
>>> +                             virDomainObjPtr obj,
>>> +                             qemuDomainJob job)
>>> +{
>>> +    return qemuDomainObjBeginJobInternal(driver, obj, job,
>>> +                                         QEMU_ASYNC_JOB_NONE, true);
>>                                             ^^^^^^^^^^^^^^^^^^^^
>> Doesn't this mean async jobs are not supported.
> 
> I'm not quite sure what do you mean. You mean whether grabbing a sync
> job while there's async job already running? This is supported. The fact
> that asyncJob is QEMU_ASYNC_JOB_NONE does not mean "set sync job to @job
> and async job to QEMU_ASYNC_JOB_NONE". In fact,
> qemuDomainObjBeginJobInternal() is capable of grabbing either sync or
> async job but not both at the same time. When grabbing a sync job,
> @asynJob == QEMU_ASYNC_JOB_NONE and when grabbing an async job, @job ==
> QEMU_JOB_ASYNC.
> 
> Anyway, the point of BeginJobInstant() is to be drop-in replacement for
> plain BeginJob(), so whatever the latter passes to BeginJobInternal()
> the former should mimic it. But then again, I'm not quite sure I
> understand what you mean.
> 

I should have used the [1] or something to signify that this particular
comment was related to the one above where I was asking about the need
for the bool/instant check for async jobs, but I think you figured that
out even though you left a small window of self doubt ;-)

John