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

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.