[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH] BlockJob: Support sync/async virDomainBlockJobAbort



On 01/19/2012 02:00 PM, Adam Litke wrote:
>>> Without the VIR_DOMAIN_BLOCK_JOB_ABORT_ASYNC flag, libvirt will
>>> internally poll
>>> using qemu's "query-block-jobs" API and will not return until the
>>> operation has
>>> been completed.
>>
>> Why do you raise the event VIR_DOMAIN_BLOCK_JOB_CANCELLED also in the
>> case where libvirt takes care of it internally?
>> Is there a specific use case for this?
> 
> We just raise the events we receive from qemu and qemu will always generate
> these.  IMO, there is no harm in always raising the events.  If a user doesn't
> care about them, then they don't have to register for notifications.

I guess I reviewed under the assumption that you'd only raise the
libvirt event if the user is expecting it; but I guess I can see your
point of always raising the libvirt event when the qemu event happens.
This works fine for the new semantics of the qemu command.

The problem is the when the user has the old qemu that only had the
blocking semantics, but requests ASYNC operation, at which point, they
still expect the libvirt event.  Which means that libvirt must raise the
event by itself; but now if the user upgrades to new qemu, and we hit
the race where libvirt can't tell whether the command completed because
it was the old blocking version or whether it was async but completed
before we queried, then converting the qemu event to a libvirt event
_and_ having libvirt raise the event itself would result in double
events to the user.

Hence, I still think that being smarter and only converting qemu event
to libvirt event when there is a pending async cancel, and discarding it
otherwise, is the way to go.

> 
>>
>>> API users are advised that this operation is
>>> unbounded and
>>> further interaction with the domain during this period may block.
>>
>> Do you have an example of what operation may block?
>> Is this the common behavior for all the other async tasks that libvirt
>> is managing internally?
> 
> Any operation that needs to issue a monitor command would block.  This is a
> large list.  That being said, the existing implementation already has this
> potential issue with the ASYNC flag being the workaround.
> 
> I cannot answer definitively as to whether all libvirt async tasks can block.  I
> suspect the answer depends on whether the operation relies on a monitor command
> that can block in qemu.  As far as I know the only other command like this
> (coincidentally it is being improved in qemu by Luiz) is virDomainMemoryStats().

Jiri Denemark worked on some code to classify qemu monitor commands into
categories, so that when an async monitor command is in effect, other
safe commands can be done in the meantime between times that the async
job regains control to issue another monitor command to poll its status.
 This would involve grabbing an async job around the time where we wait
for job cancellation to complete, and dropping the lock each time we
sleep until the next poll.  Waiting for migration is an example of
setting up an async job.  But I'm okay if we save that for future
improvements (and get Jirka's help), rather than trying to complicate
this initial implementation with the additional semantics of using an
async monitor job.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]