On Thu, Jun 27, 2024 at 11:11:56 -0700, Jon Kohler wrote: > When enabling switchover-ack on qemu from libvirt, the .party value > was set to both source and target; however, qemuMigrationParamsCheck() > only takes that into account to validate that the remote side of the > migration supports the flag if it is marked optional or auto/always on. > > In the case of switchover-ack, when enabled on only the dst and not > the src, the migration will fail if the src qemu does not support > switchover-ack, as the dst qemu will issue a switchover-ack msg: > qemu/migration/savevm.c -> > loadvm_process_command -> > migrate_send_rp_switchover_ack(mis) -> > migrate_send_rp_message(mis, MIG_RP_MSG_SWITCHOVER_ACK, 0, NULL) > > Since the src qemu doesn't understand messages with header_type == > MIG_RP_MSG_SWITCHOVER_ACK, qemu will kill the migration with error: > qemu-kvm: RP: Received invalid message 0x0007 length 0x0000 > qemu-kvm: Unable to write to socket: Bad file descriptor > > Looking at the original commit [1] for optional migration capabilities, > it seems that the spirit of optional handling was to enhance a given > existing capability where possible. Given that switchover-ack > exclusively depends on return-path, adding it as optional to that cap > feels right. Oops, sorry for not spotting this in my review and thanks for fixing it. I guess we should add a possibility to check QEMU support for capabilities in case they don't depend on another capability and they are supposed to be enabled automatically. But that's a separate thing which may possibly become useful in the future. Reviewed-by: Jiri Denemark <jdenemar@xxxxxxxxxx>
