xapi_vm_migrate: Avoid duplicate, overly-strict CBT check on VDIs

[last-genius]

There is already a call to assert_can_migrate_vdis present in assert_can_migrate, which checks that none of the VDIs that are going to be moved have CBT enabled. There is no need to additionally check that none of the VDIs in general have CBT enabled.

Some clients, like XenOrchestra, will turn off CBT on VDIs and retry migration after getting the VDI_CBT_ENABLED error on live migration. Dropping this overly strict check allows not stripping CBT when VDI will not be moved (when it's on a shared SR).
In addition, during rolling pool upgrades, disabling CBT is not allowed, hence the live migration operation wouldn't be able to continue. Avoiding the strict check fixes that as well.

Closes #6400

[last-genius]

Tested manually, live migrated a VM with a CBT-enabled VDI on a shared SR a few times.

[psafont]

As explained in the original issue, I don't see a reason to have this strict check.

[Vincent-lau]

The function assert_can_migrate_vdis says

(** Check that none of the VDIs that are mapped to a different SR have CBT
or encryption enabled. This function must be called with the complete
[vdi_map], which contains all the VDIs of the VM.
[check_vdi_map] should be called before this function to verify that this
is the case. *)

which means it's going to check all the VDIs of a VM to see if they have cbt. And the List.iter you removed checks all the VDIs of a VM as well, through the vm_vdis list. So in what case will there be a VDI in vm_vdis but no in vdi_map? In other words, why is the List.iter check you removed a stricter check than assert_can_migrate_vdis?

[last-genius]

which means it's going to check all the VDIs of a VM to see if they have cbt. And the List.iter you removed checks all the VDIs of a VM as well, through the vm_vdis list. So in what case will there be a VDI in vm_vdis but no in vdi_map? In other words, why is the List.iter check you removed a stricter check than assert_can_migrate_vdis?

the key part is "that are mapped to a different SR". the List.iter will check all VDIs. the assert will only check those VDIs that will have to move. I can amend the commit message to make it clearer, perhaps?

[Vincent-lau]

LGTM, the other test worth doing is to migrate a VM with multiple VDIs from local SRs, some cbt-enabled, and some cbt-disabled and check that they are correctly blocked, with this patch

[lindig]

Just in case we are coming back to this in the future: how about leaving a comment that this check was removed? Often we re-discover reasons why some code was present.

[last-genius]

LGTM, the other test worth doing is to migrate a VM with multiple VDIs from local SRs, some cbt-enabled, and some cbt-disabled and check that they are correctly blocked, with this patch

Verified this manually, migration with CBT-enabled VDIs that are going to be moved is still blocked.

[last-genius]

Just in case we are coming back to this in the future: how about leaving a comment that this check was removed? Often we re-discover reasons why some code was present.

Added a short comment