[Qemu-devel] Dropping the MONITOR_CMD_ASYNC

[Qemu-devel] Dropping the MONITOR_CMD_ASYNC

[Luiz Capitulino]

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? :)

Re: [Qemu-devel] Dropping the MONITOR_CMD_ASYNC

[Alon Levy]

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.

Alon

Re: [Qemu-devel] Dropping the MONITOR_CMD_ASYNC

[Stefan Hajnoczi]

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?

Stefan

Re: [Qemu-devel] Dropping the MONITOR_CMD_ASYNC

[Gerd Hoffmann]

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.

cheers,
  Gerd

Re: [Qemu-devel] Dropping the MONITOR_CMD_ASYNC

[Stefan Hajnoczi]

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

Stefan

Re: [Qemu-devel] Dropping the MONITOR_CMD_ASYNC

[Gerd Hoffmann]

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.

cheers,
  Gerd

Re: [Qemu-devel] Dropping the MONITOR_CMD_ASYNC

[Stefan Hajnoczi]

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.

Stefan

Re: [Qemu-devel] Dropping the MONITOR_CMD_ASYNC

[Alon Levy]

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...
> 

Why has it got to be dropped? can't it be declared as deprecated first?

> 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? :)
> 

Re: [Qemu-devel] Dropping the MONITOR_CMD_ASYNC

[Alon Levy]

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, then we can implement a QMP variant of
client_migrate_info side by side, libvirt can learn to use it, and then
(seeing as we are the only user) M_C_A can be dropped.

> 
> Stefan
> 

Re: [Qemu-devel] Dropping the MONITOR_CMD_ASYNC

[Luiz Capitulino]

On Mon, 12 Dec 2011 17:50:46 +0200
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...
> > 
> 
> Why has it got to be dropped? can't it be declared as deprecated first?

Well, after this thread looks like it's what we'll have to do...

> 
> > 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? :)
> > 
> 

Re: [Qemu-devel] Dropping the MONITOR_CMD_ASYNC

[Anthony Liguori]

On 12/11/2011 04:29 AM, Alon Levy 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

Then it's completely broken.  CMD_ASYNC is broken and should have been removed a 
long time ago.

Unfortunately, we're going to have to remove this command until we can move to 
full QAPI.  I see this went in through Gerd's tree:

commit edc5cb1a52b2847201acf78b0fba67ab3c2464d5
Author: Yonit Halperin <yhalperi@redhat.com>
Date:   Mon Oct 17 10:03:18 2011 +0200

     spice: turn client_migrate_info to async

Changes to monitor commands really should at least carry an Acked-by from the 
QMP maintainer (Luiz).  That would have prevented this from happening.

I'll try to be more diligent about enforcing this in the future.

Regards,

Anthony Liguori

> 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.
>
> Alon
>
>

Re: [Qemu-devel] Dropping the MONITOR_CMD_ASYNC

[Anthony Liguori]

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

Re: [Qemu-devel] Dropping the MONITOR_CMD_ASYNC

[Anthony Liguori]

On 12/12/2011 10:08 AM, Luiz Capitulino wrote:
> On Mon, 12 Dec 2011 17:50:46 +0200
> 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...
>>>
>>
>> Why has it got to be dropped? can't it be declared as deprecated first?
>
> Well, after this thread looks like it's what we'll have to do...\

Nope, it has to be dropped.

Commands using CMD_ASYNC may fail in arbitrary ways because of the way error 
reporting is done.  This is an unfixable problem until we eliminate all uses of 
qerror_report().

We need to take the hit here and force the command to always fail.  libvirt will 
need logic to use a different command with new versions.  If we coordinate this 
with the libvirt folks, we can make the transition as smooth as possible.

Regards,

Anthony Liguori

>
>>
>>> 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? :)
>>>
>>
>
>

Re: [Qemu-devel] Dropping the MONITOR_CMD_ASYNC

[Alon Levy]

On Mon, Dec 12, 2011 at 10:24:53AM -0600, Anthony Liguori wrote:
> 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.
> 

Care to explain what is fundamental about it?

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

Gerd gave a very good explanation of the requirement, I don't think I
can add anything to it. What you suggest is to rewrite it, I just don't
understand why we can't leave the MONITOR_CMD_ASYNC around until there
is both an alternative and libvirt learns to use it.

> 
> Regards,
> 
> Anthony Liguori
> 

Re: [Qemu-devel] Dropping the MONITOR_CMD_ASYNC

[Anthony Liguori]

On 12/12/2011 11:22 AM, Alon Levy wrote:
> On Mon, Dec 12, 2011 at 10:24:53AM -0600, Anthony Liguori wrote:
>> 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.
>>
>
> Care to explain what is fundamental about it?

The commands are broken as they sit today.  If you issue an async command and an 
error occurs, the error will not be associated with the right command.

>
>> The command should have never been introduced in the first place to use async.
>
> Gerd gave a very good explanation of the requirement, I don't think I
> can add anything to it. What you suggest is to rewrite it, I just don't
> understand why we can't leave the MONITOR_CMD_ASYNC around until there
> is both an alternative and libvirt learns to use it.

It's broken and more importantly, the longer we leave it there the more likely 
people will use it and create more problems.

We aren't breaking an ABI here.  libvirt can detect that this command is gone 
and probe for the new command.

This shouldn't have gone in in the first place and yes, that's my responsibility 
to enforce.

Regards,

Anthony Liguori

>
>>
>> Regards,
>>
>> Anthony Liguori
>>
>

Re: [Qemu-devel] Dropping the MONITOR_CMD_ASYNC

[Alon Levy]

On Mon, Dec 12, 2011 at 11:53:28AM -0600, Anthony Liguori wrote:
> On 12/12/2011 11:22 AM, Alon Levy wrote:
> >On Mon, Dec 12, 2011 at 10:24:53AM -0600, Anthony Liguori wrote:
> >>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.
> >>
> >
> >Care to explain what is fundamental about it?
> 
> The commands are broken as they sit today.  If you issue an async
> command and an error occurs, the error will not be associated with
> the right command.
> 
> >
> >>The command should have never been introduced in the first place to use async.
> >
> >Gerd gave a very good explanation of the requirement, I don't think I
> >can add anything to it. What you suggest is to rewrite it, I just don't
> >understand why we can't leave the MONITOR_CMD_ASYNC around until there
> >is both an alternative and libvirt learns to use it.
> 
> It's broken and more importantly, the longer we leave it there the
> more likely people will use it and create more problems.
> 
> We aren't breaking an ABI here.  libvirt can detect that this
> command is gone and probe for the new command.

Obviously this will break current libvirt. So it means we need to make
sure we introduce the new command and libvirt's usage of it follows
ASAP. Still won't help with <=current libvirt.

How do you call the monitor interface btw? I thought it was a sort of
ABI.

> 
> This shouldn't have gone in in the first place and yes, that's my
> responsibility to enforce.
> 
> Regards,
> 
> Anthony Liguori
> 
> >
> >>
> >>Regards,
> >>
> >>Anthony Liguori
> >>
> >
> 
> 

Re: [Qemu-devel] Dropping the MONITOR_CMD_ASYNC

[Gerd Hoffmann]

  Hi,

> We aren't breaking an ABI here.  libvirt can detect that this command is
> gone and probe for the new command.

Please stop this.  There are libvirt versions in the wild which can't
deal with this and thus will break, no matter how hard you try to wave
it away.  The question is not whenever something breaks or not, the
question is how to deal with the breakage best.

Zapping async monitor commands *now* (i.e. early in the 1.1 release
cycle) is probably the best way to handle it.  It gives the libvirt
folks time to get a new release out of the door before 1.1-final is
released, which in turn allows distros to minimize the user-visible fallout.

Technically the switch is easy: migrate_connect_complete_cb() is called
from spice-server on completion, we just have to emit a qmp event there
instead of calling the monitor completion callback.

cheers,
  Gerd

Re: [Qemu-devel] Dropping the MONITOR_CMD_ASYNC

[Avi Kivity]

On 12/12/2011 06:26 PM, Anthony Liguori wrote:
>
> Nope, it has to be dropped.
>
> Commands using CMD_ASYNC may fail in arbitrary ways because of the way
> error reporting is done.  This is an unfixable problem until we
> eliminate all uses of qerror_report().
>

Why don't we eliminate all uses for qerror_report() then?  Breaking our
interfaces instead seems like a horrible idea.

> We need to take the hit here and force the command to always fail. 
> libvirt will need logic to use a different command with new versions. 
> If we coordinate this with the libvirt folks, we can make the
> transition as smooth as possible.

I thought we've outgrown this.  If we provide an interface, we should
support it.

-- 
error compiling committee.c: too many arguments to function

Re: [Qemu-devel] Dropping the MONITOR_CMD_ASYNC

[Anthony Liguori]

On 12/13/2011 03:03 AM, Alon Levy wrote:
> On Mon, Dec 12, 2011 at 11:53:28AM -0600, Anthony Liguori wrote:
>> On 12/12/2011 11:22 AM, Alon Levy wrote:
>>> On Mon, Dec 12, 2011 at 10:24:53AM -0600, Anthony Liguori wrote:
>>>> 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.
>>>>
>>>
>>> Care to explain what is fundamental about it?
>>
>> The commands are broken as they sit today.  If you issue an async
>> command and an error occurs, the error will not be associated with
>> the right command.
>>
>>>
>>>> The command should have never been introduced in the first place to use async.
>>>
>>> Gerd gave a very good explanation of the requirement, I don't think I
>>> can add anything to it. What you suggest is to rewrite it, I just don't
>>> understand why we can't leave the MONITOR_CMD_ASYNC around until there
>>> is both an alternative and libvirt learns to use it.
>>
>> It's broken and more importantly, the longer we leave it there the
>> more likely people will use it and create more problems.
>>
>> We aren't breaking an ABI here.  libvirt can detect that this
>> command is gone and probe for the new command.
>
> Obviously this will break current libvirt. So it means we need to make
> sure we introduce the new command and libvirt's usage of it follows
> ASAP. Still won't help with<=current libvirt.

libvirt broke with QEMU 1.0 anyway so only new libvirts will work with QEMU 1.0+ 
so it's all rather moot.

>
> How do you call the monitor interface btw? I thought it was a sort of
> ABI.

It is an ABI.  But you can negotiate features while still having an ABI.  We 
provide interfaces (query-commands) to list the features that are supported 
expressly for this purpose.

Worst case scenario, an old libvirt should be able to detect that the command is 
no longer valid and degrade gracefully.

Regards,

Anthony Liguori

>
>>
>> This shouldn't have gone in in the first place and yes, that's my
>> responsibility to enforce.
>>
>> Regards,
>>
>> Anthony Liguori
>>
>>>
>>>>
>>>> Regards,
>>>>
>>>> Anthony Liguori
>>>>
>>>
>>
>>
>

Re: [Qemu-devel] Dropping the MONITOR_CMD_ASYNC

[Anthony Liguori]

On 12/13/2011 05:06 AM, Avi Kivity wrote:
> On 12/12/2011 06:26 PM, Anthony Liguori wrote:
>>
>> Nope, it has to be dropped.
>>
>> Commands using CMD_ASYNC may fail in arbitrary ways because of the way
>> error reporting is done.  This is an unfixable problem until we
>> eliminate all uses of qerror_report().
>>
>
> Why don't we eliminate all uses for qerror_report() then?  Breaking our
> interfaces instead seems like a horrible idea.

That's what Luiz is doing right now.

Regards,

Anthony Liguori

>
>> We need to take the hit here and force the command to always fail.
>> libvirt will need logic to use a different command with new versions.
>> If we coordinate this with the libvirt folks, we can make the
>> transition as smooth as possible.
>
> I thought we've outgrown this.  If we provide an interface, we should
> support it.
>

Re: [Qemu-devel] Dropping the MONITOR_CMD_ASYNC

[Avi Kivity]

On 12/13/2011 03:36 PM, Anthony Liguori wrote:
> On 12/13/2011 05:06 AM, Avi Kivity wrote:
>> On 12/12/2011 06:26 PM, Anthony Liguori wrote:
>>>
>>> Nope, it has to be dropped.
>>>
>>> Commands using CMD_ASYNC may fail in arbitrary ways because of the way
>>> error reporting is done.  This is an unfixable problem until we
>>> eliminate all uses of qerror_report().
>>>
>>
>> Why don't we eliminate all uses for qerror_report() then?  Breaking our
>> interfaces instead seems like a horrible idea.
>
> That's what Luiz is doing right now.
>

So, the plan to drop MONITOR_CMD_ASYNC was dropped?

-- 
error compiling committee.c: too many arguments to function