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

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

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 1404 bytes --]

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

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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 617 bytes --]

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

[Daniel P. Berrange]

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
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|

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

[Michal Privoznik]

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.

Having said that - what's desired solution here? Unconditionally set
seamless_migration=on on enough new qemu and don't expose it in XML at all?

Michal

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

[Daniel P. Berrange]

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.
> 
> Having said that - what's desired solution here? Unconditionally set
> seamless_migration=on on enough new qemu and don't expose it in XML at all?

Yes, that's my desired impl.

Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|

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

[Daniel P. Berrange]

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 ?

Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|

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
>