On 18.04.24 18:43, Daniel P. Berrangé wrote:
On Thu, Apr 18, 2024 at 06:40:38PM +0300, Vladimir Sementsov-Ogievskiy wrote:
On 18.04.24 17:37, Daniel P. Berrangé wrote:
On Thu, Apr 18, 2024 at 01:13:29AM +0300, Vladimir Sementsov-Ogievskiy wrote:
We do set MIGRATION_FAILED state, but don't give a chance to
orchestrator to query migration state and get the error.
Let's report an error through QAPI like we do on outgoing migration.
migration-test is updated correspondingly.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsement...@yandex-team.ru>
---
Doubt: is exiting on failure a contract? Will this commit break
something in Libvirt? Finally, could we just change the logic, or I need
and additional migration-parameter for new behavior?
There's a decent risk that this could break apps, whether
libvirt or something else, especially if the app is just
launching QEMU with '-incoming URI', rather than using
'-incoming defer' and then explicitly using QMP to start the
incoming migration.
I'd say that with '-incoming defer' we should *not* exit on
migration error, because that arg implies the app explicitly
wants to be using QMP to control migration.
With the legacy '-incoming URI' it is probably best to keep
exit on error, as that's comparatively more likely to be used
in adhoc scenarios where the app/user is ignoring QMP on the
dst side.
None the less, I think we need to check how libvirt behaves
with this patch to be sure of no surprises.
Sounds reasonable, thanks! I'll rework it to behave the new
way only with "-incoming defer", and check how libvirt behave with it.
If there are problems and/or we want to be super safe wrt
backcompat, we could add a new '-incoming managed' as
being equivalent to '-incoming defer' but without the
implicit exit.
Probably, that's the best variant. As I can check libvirt in some case, but not
at all cases. And libvirt is not the only vm manager finally.
And we can in the same time deprecate "-incoming defer" in favor of new
behavior.
--
Best regards,
Vladimir
