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<kra...@redhat.com> wrote: > >>>On 12/12/11 13:10, Stefan Hajnoczi wrote: > >>>>On Mon, Dec 12, 2011 at 11:29 AM, Gerd Hoffmann<kra...@redhat.com> wrote: > >>>>>On 12/12/11 11:18, Stefan Hajnoczi wrote: > >>>>>>On Sun, Dec 11, 2011 at 10:29 AM, Alon Levy<al...@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 >
