Re: [Qemu-devel] Dropping the MONITOR_CMD_ASYNC

[Anthony Liguori <anthony@codemonkey.ws>]

On 12/12/2011 10:00 AM, Alon Levy wrote:
> On Mon, Dec 12, 2011 at 03:23:35PM +0000, Stefan Hajnoczi wrote:
>> On Mon, Dec 12, 2011 at 12:39 PM, Gerd Hoffmann<kraxel@redhat.com>  wrote:
>>> On 12/12/11 13:10, Stefan Hajnoczi wrote:
>>>> On Mon, Dec 12, 2011 at 11:29 AM, Gerd Hoffmann<kraxel@redhat.com>  wrote:
>>>>> On 12/12/11 11:18, Stefan Hajnoczi wrote:
>>>>>> On Sun, Dec 11, 2011 at 10:29 AM, Alon Levy<alevy@redhat.com>  wrote:
>>>>>>> On Thu, Dec 08, 2011 at 05:45:44PM -0200, Luiz Capitulino wrote:
>>>>>>>> Hi there,
>>>>>>>>
>>>>>>>> I'm about to completely drop the MONITOR_CMD_ASYNC API, but it turns out that
>>>>>>>> the command client_migrate_info uses it. That's a legacy interface and has to
>>>>>>>> be dropped, no command should be using it...
>>>>>>>>
>>>>>>>> Something tells me that if I just drop it (and change the command to use the
>>>>>>>> regular interface), bad things will happen. Am I right? :)
>>>>>>>>
>>>>>>>
>>>>>>> The monitor command client_migrate_info needs to complete after getting
>>>>>>> an ACK message from the currently connected spice client (this is the
>>>>>>> only case where this is required - if there is no client then the
>>>>>>> MONITOR_CMD_ASYNC API won't be used). This in turn requires the main
>>>>>>> thread to perform select and call the callback that will process this
>>>>>>> ACK. That's why the MONITOR_CMD_ASYNC API was used.
>>>>>>>
>>>>>>> I'm not aware of any other way to do this, I'll be glad for any help
>>>>>>> here. Also, I understand this is not what is not true async, since one
>>>>>>> would expect a true async interface to support multiple in flight
>>>>>>> monitor commands. If there is any ETA or existing way to do this we
>>>>>>> could change the implementation of client_migrate_info.
>>>>>>
>>>>>> Is it possible to use a QMP event to signal completion instead of a
>>>>>> MONITOR_CMD_ASYNC command?
>>>>>
>>>>> Problem is this breaks the qemu<->  libvirt interface.
>>>>
>>>> I had the same issue with the block_job_cancel command, which Adam
>>>> Litke and Eric Blake helped us fix and find a backward-compatible
>>>> libvirt solution for:
>>>>
>>>> http://www.redhat.com/archives/libvir-list/2011-November/msg01351.html
>>>
>>> Isn't going to fly as waiting for completion isn't optional in that
>>> case.  Workflow is this:
>>>
>>> (1) libvirt issues client_migrate_info command.
>>> (2) qemu forwards it to spice-server, which in turn forwards it to
>>>     the spice client (if connected).
>>> (3) spice client connects to the migration target machine.
>>> (4) spice client signals completion to spice-server, which in turn
>>>     notifies qemu.
>>> (5) qemu calls the monitor completion callback, libvirt gets
>>>     client_migrate_info result.
>>> (6) libvirt issues migrate command.
>>>
>>> The problem is that (3) must be finished before (6) because qemu on the
>>> target side doesn't accept incoming tcp connections any more once the
>>> migration started.
>>>
>>> I don't see a way to switch this to qmp events without breaking old
>>> libvirt versions managing new qemu.
>>
>> I don't see a solution in this case either.  Looks like libvirt
>> supports this command since 0.9.2 so it's not a good idea to just yank
>> it.
>>
>> It might be possible for the QEMU client_migrate_info handler to run a
>> nested event loop in the legacy libvirt case.  This would suck since
>> the VM is unresponsive while waiting for spice migration to complete.
>> New libvirt would call the async version of the command which is
>> well-behaved and uses a QMP event to signal completion.
>
> I agree that a nested event loop would be a bad solution, the point is
> to let the guest continue to work while waiting, otherwise you destroy
> the live migration experience, might as well disconnect the client from
> the source and have it connect to the target.
>
> I thought the whole point of MONITOR_CMD_ASYNC was to allow this
> scenario. So iiuc QMP is the alternative but it would require a rewrite,
> i.e. break existing users like libvirt. Hence my suggestion as a reply
> to Luiz's initial email that we just deprecate MONITOR_CMD_ASYNC right
> now instead of dropping it,

It *has* to be dropped.  Any command using it is fundamentally broken.

The command should have never been introduced in the first place to use async.

Regards,

Anthony Liguori