Re: [Qemu-devel] [libvirt] [PATCH v2 1/4] config: Introduce <migration> for SPICE graphics

[Michal Privoznik]

On 19.09.2012 16:25, Daniel P. Berrange wrote:
> On Wed, Sep 19, 2012 at 03:26:49PM +0200, Michal Privoznik wrote:
>> On 15.09.2012 17:10, Daniel P. Berrange wrote:
>>> On Fri, Sep 14, 2012 at 05:23:16PM -0600, Eric Blake wrote:
>>>> [adding qemu]
>>>>
>>>> On 09/14/2012 11:47 AM, Daniel P. Berrange wrote:
>>>>> On Fri, Sep 14, 2012 at 07:34:50PM +0200, Michal Privoznik wrote:
>>>>>> With this element users will control how SPICE
>>>>>> server behaves upon migration. For now, there's
>>>>>> just one attribute 'seamless' turning seamless
>>>>>> migration on/off/default.
>>>>>
>>>>> Ewww, no. This information is a related to a API operation,
>>>>> not the VM configuration. It should be either auto-detected
>>>>> by libvirt to the best compatible setting, or passed as a
>>>>> flag to the virDomainMigrate API call if auto-detection is
>>>>> not possible.
>>>>
>>>> But with the current qemu implementation, there's no way to know if the
>>>> destination supports this until after you've started the source, and the
>>>> current implementation in qemu is that you must declare the semantics at
>>>> the time you start qemu, not at the time you send the 'migrate' monitor
>>>> command.  For libvirt autodetection to work without polluting the domain
>>>> XML, we'd need to be able to auto-detect at the time we start migration.
>>>>
>>>> This sounds like we need to enhance the 'migrate-set-capabilities'
>>>> command to enable or disable this feature on the fly, according to what
>>>> libvirt detects from the remote end, rather than hard-coding it to the
>>>> startup state of qemu on the source side.
>>>
>>> Hmm, my understanding of the QEMU flag was different. Based on
>>> the commit message:
>>>
>>>     spice: adding seamless-migration option to the command line
>>>     
>>>     The seamless-migration flag is required in order to identify
>>>     whether libvirt supports the new QEVENT_SPICE_MIGRATE_COMPLETED or not
>>>     (by default the flag is off).
>>>     New libvirt versions that wait for QEVENT_SPICE_MIGRATE_COMPLETED 
>>> should turn on this flag.
>>>     When this flag is off, spice fallbacks to its old migration method, 
>>> which
>>>     can result in data loss.
>>>
>>>
>>> This says to me that any libvirt which knows about the new
>>> SPICE_MIGRATE_COMPLETED event, should set the seamless-migration
>>> flag unconditionally, to indicate that it can handle the event
>>> and thus the new migration method. It says nothing about only
>>> setting this flag if the destination QEMU also supports it.
>>> As such, IMHO, we can & should set this flag unconditonally
>>> on all QEMUs we run which support it.
>>>
>>> If it turns out that this flag does indeed require that the
>>> destination QEMU also has the same setting, then IMHO this
>>> flag is a fatally flawed design. At time of starting any QEMU
>>> instance, we can't know whether the destination QEMU we want
>>> to migrate to will have the support or not. Compatibility
>>> checks of this kind can only be decided at time the migrate
>>> command is actually issued.
>>>
>>>
>>> Daniel
>>>
>>
>> From my investigation I was able to migrate between qemu where one had
>> seamless_migration=on and the other one didn't support such argument at
>> all. Literally, on source I've checked out 27af778828db9 (the last
>> commit in Yonit's patchset) on destination f5bb039c6d97e (the last
>> before the patchset). Then I've migrated and receive both event and flag
>> set in query-spice response. So I guess the design is okay.
> 
> Oh, BTW, when you tested did you have an actual SPICE client connected
> and did it migrate ok ?

That's correct.

Michal
> 
> Daniel
>