From 3b773c738f8528dcaa63be58fffbb5f793885260 Mon Sep 17 00:00:00 2001 From: Konstantina Chremmou Date: Tue, 14 Nov 2023 13:25:34 +0000 Subject: [PATCH] CA-383483: Rewrote the migration logic to fix regression (#3241) * Simplified the class name by renaming CrossPoolMigrateCanMigrateFilter to CrossPoolMigrateFilter. * CA-383483: Rewrote (again) the migration logic because the fix to CA-294370 regressed migration from local to local storage. Also, minor refactoring to some methods for more efficient calculations. Signed-off-by: Konstantina Chremmou --- XenAdmin/Commands/CrossPoolMigrateCommand.cs | 14 +- XenAdmin/Commands/VMOperationHostCommand.cs | 62 ++++-- .../CrossPoolMigrateDestinationPage.cs | 4 +- ...ateFilter.cs => CrossPoolMigrateFilter.cs} | 209 +++++++++--------- XenAdmin/XenAdmin.csproj | 2 +- 5 files changed, 159 insertions(+), 132 deletions(-) rename XenAdmin/Wizards/CrossPoolMigrateWizard/Filters/{CrossPoolMigrateCanMigrateFilter.cs => CrossPoolMigrateFilter.cs} (52%) diff --git a/XenAdmin/Commands/CrossPoolMigrateCommand.cs b/XenAdmin/Commands/CrossPoolMigrateCommand.cs index 66838cec1..bd66d6f68 100644 --- a/XenAdmin/Commands/CrossPoolMigrateCommand.cs +++ b/XenAdmin/Commands/CrossPoolMigrateCommand.cs @@ -119,14 +119,16 @@ public static bool CanRun(VM vm, Host preselectedHost, out string failureReason, return false; } - var vms = new List {vm}; + if (preselectedHost != null) + { + var vms = new List {vm}; - if (preselectedHost != null && new ResidentHostIsSameAsSelectionFilter(preselectedHost, vms).FailureFound(out failureReason)) - return false; + if (new ResidentHostIsSameAsSelectionFilter(preselectedHost, vms).FailureFound(out failureReason)) + return false; - if (preselectedHost != null && new CrossPoolMigrateCanMigrateFilter(preselectedHost, new List {vm}, - WizardMode.Migrate, cache).FailureFound(out failureReason)) - return false; + if (new CrossPoolMigrateFilter(preselectedHost, vms, WizardMode.Migrate, cache).FailureFound(out failureReason)) + return false; + } failureReason = string.Empty; return true; diff --git a/XenAdmin/Commands/VMOperationHostCommand.cs b/XenAdmin/Commands/VMOperationHostCommand.cs index dd7a1ba44..ef74a0779 100644 --- a/XenAdmin/Commands/VMOperationHostCommand.cs +++ b/XenAdmin/Commands/VMOperationHostCommand.cs @@ -46,7 +46,7 @@ internal class VMOperationHostCommand : VMOperationCommand { public delegate Host GetHostForVM(VM vm); - private readonly static log4net.ILog log = log4net.LogManager.GetLogger(System.Reflection.MethodBase.GetCurrentMethod().DeclaringType); + private static readonly log4net.ILog log = log4net.LogManager.GetLogger(System.Reflection.MethodBase.GetCurrentMethod().DeclaringType); private readonly Dictionary _cantBootReasons = new Dictionary(); private readonly bool _noneCanBoot = true; private readonly string _text; @@ -65,9 +65,7 @@ public VMOperationHostCommand(IMainWindow mainWindow, IEnumerable { VM vm = (VM)item.XenObject; - string reason = GetVmCannotBootOnHostReason(vm, GetHost(vm), session, operation); - - if (reason == null) + if (VmCanBootOnHost(vm, GetHost(vm), session, operation, out var reason)) _noneCanBoot = false; else _cantBootReasons[vm] = reason; @@ -107,26 +105,38 @@ protected override bool CanRun(VM vm) return vm != null && !_cantBootReasons.ContainsKey(vm); } - internal static string GetVmCannotBootOnHostReason(VM vm, Host host, Session session, vm_operations operation) + internal static bool VmCanBootOnHost(VM vm, Host host, Session session, vm_operations operation, out string cannotBootReason) { - Host residentHost = vm.Connection.Resolve(vm.resident_on); - if (host == null) - return Messages.NO_HOME_SERVER; + { + cannotBootReason = Messages.NO_HOME_SERVER; + return false; + } - if (vm.power_state == vm_power_state.Running && residentHost != null - && Helpers.ProductVersionCompare(Helpers.HostProductVersion(host), Helpers.HostProductVersion(residentHost)) < 0) + if (vm.power_state == vm_power_state.Running) { - // This will be a migrate menu if powerstate if running - return Messages.OLDER_THAN_CURRENT_SERVER; + var residentHost = vm.Connection.Resolve(vm.resident_on); + + if (residentHost != null) + { + if (host.opaque_ref == residentHost.opaque_ref) + { + cannotBootReason = Messages.HOST_MENU_CURRENT_SERVER; + return false; + } + + if (Helpers.ProductVersionCompare(Helpers.HostProductVersion(host), Helpers.HostProductVersion(residentHost)) < 0) + { + cannotBootReason = Messages.OLDER_THAN_CURRENT_SERVER; + return false; + } + } } - - if (vm.power_state == vm_power_state.Running && residentHost != null && host.opaque_ref == residentHost.opaque_ref) - return Messages.HOST_MENU_CURRENT_SERVER; if ((operation == vm_operations.pool_migrate || operation == vm_operations.resume_on) && VmCpuIncompatibleWithHost(host, vm)) { - return FriendlyErrorNames.VM_INCOMPATIBLE_WITH_THIS_HOST; + cannotBootReason = FriendlyErrorNames.VM_INCOMPATIBLE_WITH_THIS_HOST; + return false; } try @@ -137,20 +147,27 @@ internal static string GetVmCannotBootOnHostReason(VM vm, Host host, Session ses { if (f.ErrorDescription.Count > 2 && f.ErrorDescription[0] == Failure.VM_REQUIRES_SR) { - SR sr = host.Connection.Resolve((new XenRef(f.ErrorDescription[2]))); + SR sr = host.Connection.Resolve(new XenRef(f.ErrorDescription[2])); if (sr != null && sr.content_type == SR.Content_Type_ISO) - return Messages.MIGRATE_PLEASE_EJECT_YOUR_CD; + { + cannotBootReason = Messages.MIGRATE_PLEASE_EJECT_YOUR_CD; + return false; + } } - return f.ShortMessage; + + cannotBootReason = f.ShortMessage; + return false; } catch (Exception e) { log.ErrorFormat("There was an error calling assert_can_boot_here on host {0}: {1}", host.Name(), e.Message); - return Messages.HOST_MENU_UNKNOWN_ERROR; + cannotBootReason = Messages.HOST_MENU_UNKNOWN_ERROR; + return false; } - return null; + cannotBootReason = null; + return true; } protected override CommandErrorDialog GetErrorDialogCore(IDictionary cantRunReasons) @@ -160,8 +177,7 @@ protected override CommandErrorDialog GetErrorDialogCore(IDictionary { new ResidentHostIsSameAsSelectionFilter(xenItem, selectedVMs), - new CrossPoolMigrateCanMigrateFilter(xenItem, selectedVMs, wizardMode, migrateFilterCache) + new CrossPoolMigrateFilter(xenItem, selectedVMs, wizardMode, migrateFilterCache) }; return new DelayLoadingOptionComboBoxItem(xenItem, filters); } @@ -139,7 +139,7 @@ protected override List CreateTargetServerFilterList(IXenObject vmList.Add(selectedVMs.Find(vm => vm.opaque_ref == opaqueRef)); filters.Add(new ResidentHostIsSameAsSelectionFilter(xenObject, vmList)); - filters.Add(new CrossPoolMigrateCanMigrateFilter(xenObject, vmList, wizardMode, migrateFilterCache)); + filters.Add(new CrossPoolMigrateFilter(xenObject, vmList, wizardMode, migrateFilterCache)); } return filters; diff --git a/XenAdmin/Wizards/CrossPoolMigrateWizard/Filters/CrossPoolMigrateCanMigrateFilter.cs b/XenAdmin/Wizards/CrossPoolMigrateWizard/Filters/CrossPoolMigrateFilter.cs similarity index 52% rename from XenAdmin/Wizards/CrossPoolMigrateWizard/Filters/CrossPoolMigrateCanMigrateFilter.cs rename to XenAdmin/Wizards/CrossPoolMigrateWizard/Filters/CrossPoolMigrateFilter.cs index 604fda6ef..2e64a8a6f 100644 --- a/XenAdmin/Wizards/CrossPoolMigrateWizard/Filters/CrossPoolMigrateCanMigrateFilter.cs +++ b/XenAdmin/Wizards/CrossPoolMigrateWizard/Filters/CrossPoolMigrateFilter.cs @@ -38,16 +38,16 @@ namespace XenAdmin.Wizards.CrossPoolMigrateWizard.Filters { - public class CrossPoolMigrateCanMigrateFilter : ReasoningFilter + public class CrossPoolMigrateFilter : ReasoningFilter { private static readonly log4net.ILog log = log4net.LogManager.GetLogger(System.Reflection.MethodBase.GetCurrentMethod().DeclaringType); private readonly WizardMode _wizardMode; private readonly List _preSelectedVMs; - private Dictionary> _cache; + private readonly Dictionary> _cache; private bool _canceled; private static readonly object _cacheLock = new object(); - public CrossPoolMigrateCanMigrateFilter(IXenObject itemAddedToFilterOn, List preSelectedVMs, WizardMode wizardMode, Dictionary> cache = null) + public CrossPoolMigrateFilter(IXenObject itemAddedToFilterOn, List preSelectedVMs, WizardMode wizardMode, Dictionary> cache = null) : base(itemAddedToFilterOn) { _wizardMode = wizardMode; @@ -90,109 +90,71 @@ protected override bool FailureFoundFor(IXenObject itemToFilterOn, out string fa } } - var hostMemoryUnavailableFailures = 0; foreach (Host host in targets) { if (_canceled) return false; - if (vmCache.TryGetValue(host.opaque_ref, out var reason) && !string.IsNullOrEmpty(reason)) + if (vmCache.TryGetValue(host.opaque_ref, out var reason)) { - failureReason = reason; + if (!string.IsNullOrEmpty(reason)) + failureReason = reason; continue; } - try - { - //Skip the resident host as there's a filter for it and - //if not then you could exclude intrapool migration - //CA-205799: do not offer the host the VM is currently on - Host homeHost = vm.Home(); - if (homeHost != null && homeHost.opaque_ref == host.opaque_ref) - continue; - - //if pool_migrate can be done, then we will allow it in the wizard, even if storage migration is not allowed (i.e. users can use the wizard to live-migrate a VM inside the pool) - if (_wizardMode == WizardMode.Migrate && vmPool != null && targetPool != null && vmPool.opaque_ref == targetPool.opaque_ref) - { - failureReason = VMOperationHostCommand.GetVmCannotBootOnHostReason(vm, host, vm.Connection.Session, vm_operations.pool_migrate); - - if (string.IsNullOrEmpty(failureReason)) - break; - else - continue; - } - - //check if the destination host is older than the source host - var destinationVersion = Helpers.HostPlatformVersion(host); - var sourceVersion = Helpers.HostPlatformVersion(vm.Home() ?? Helpers.GetCoordinator(vmPool)); - - if (Helpers.ProductVersionCompare(destinationVersion, sourceVersion) < 0) - { - failureReason = Messages.OLDER_THAN_CURRENT_SERVER; - continue; - } - - if (Host.RestrictDMC(host) && - (vm.power_state == vm_power_state.Running || - vm.power_state == vm_power_state.Paused || - vm.power_state == vm_power_state.Suspended) && - (vm.memory_static_min > vm.memory_dynamic_min || //corner case, probably unlikely - vm.memory_dynamic_min != vm.memory_dynamic_max || - vm.memory_dynamic_max != vm.memory_static_max)) - { - failureReason = FriendlyErrorNames.DYNAMIC_MEMORY_CONTROL_UNAVAILABLE; - continue; - } - - PIF managementPif = host.Connection.Cache.PIFs.First(p => p.management); - XenAPI.Network managementNetwork = host.Connection.Cache.Resolve(managementPif.network); - - Session session = host.Connection.DuplicateSession(); - Dictionary receiveMapping = Host.migrate_receive(session, host.opaque_ref, managementNetwork.opaque_ref, new Dictionary()); - - var targetSrs = host.Connection.Cache.SRs.Where(sr => sr.SupportsStorageMigration()).ToList(); - var targetNetwork = GetANetwork(host); + //CA-205799: do not offer the host the VM is currently on + Host homeHost = vm.Home(); + if (homeHost != null && homeHost.opaque_ref == host.opaque_ref) + continue; - VM.assert_can_migrate(vm.Connection.Session, - vm.opaque_ref, - receiveMapping, - true, - GetVdiMap(vm, targetSrs), - vm.Connection == host.Connection ? new Dictionary, XenRef>() : GetVifMap(vm, targetNetwork), - new Dictionary()); + //check if the destination host is older than the source host + var destinationVersion = Helpers.HostPlatformVersion(host); + var sourceVersion = Helpers.HostPlatformVersion(vm.Home() ?? Helpers.GetCoordinator(vmPool)); - break; + if (Helpers.ProductVersionCompare(destinationVersion, sourceVersion) < 0) + { + failureReason = Messages.OLDER_THAN_CURRENT_SERVER; + continue; } - catch (Failure failure) + + if (Host.RestrictDMC(host) && + (vm.power_state == vm_power_state.Running || + vm.power_state == vm_power_state.Paused || + vm.power_state == vm_power_state.Suspended) && + (vm.memory_static_min > vm.memory_dynamic_min || //corner case, probably unlikely + vm.memory_dynamic_min != vm.memory_dynamic_max || + vm.memory_dynamic_max != vm.memory_static_max)) { - // CA-359124 VM is migratable if a snapshot has more VIFs than the VM. As long as the mapping takes this into account. - if (failure.ErrorDescription.Count > 0 && failure.ErrorDescription[0] == Failure.VIF_NOT_IN_MAP && - SnapshotsContainExtraVIFs(vm)) - break; - - if (failure.ErrorDescription.Count > 0 && failure.ErrorDescription[0] == Failure.RBAC_PERMISSION_DENIED) - failureReason = failure.Message.Split('\n')[0].TrimEnd('\r'); // we want the first line only - else if (failure.ErrorDescription.Count > 1 && failure.ErrorDescription[1].Contains(Failure.DYNAMIC_MEMORY_CONTROL_UNAVAILABLE)) - failureReason = FriendlyErrorNames.DYNAMIC_MEMORY_CONTROL_UNAVAILABLE; - // CA-378758: Ensure all hosts in pool hit `HOST_NOT_ENOUGH_FREE_MEMORY` before preventing migration - else if (!failure.ErrorDescription.Contains(Failure.HOST_NOT_ENOUGH_FREE_MEMORY) || ++hostMemoryUnavailableFailures >= targets.Count) - failureReason = failure.Message; - - log.InfoFormat("VM {0} cannot be migrated to {1}. Reason: {2};", vm.Name(), host.Name(), failure.Message); + failureReason = FriendlyErrorNames.DYNAMIC_MEMORY_CONTROL_UNAVAILABLE; + continue; } - catch (Exception e) + + if (CanDoStorageMigration(vm, host, out failureReason)) { - log.Error($"There was an error while asserting the VM {vm.Name()} can be migrated to {itemToFilterOn.Name()}:", e); - failureReason = Messages.HOST_MENU_UNKNOWN_ERROR; + lock (_cacheLock) + vmCache[host.opaque_ref] = null; + break; } - finally + + //if pool_migrate can be done, then we will allow it in the wizard, even if storage migration is not allowed + //(i.e. users can use the wizard to live-migrate a VM inside the pool) + + string reason2 = null; + + if (_wizardMode == WizardMode.Migrate && + vmPool != null && targetPool != null && vmPool.opaque_ref == targetPool.opaque_ref && + VMOperationHostCommand.VmCanBootOnHost(vm, host, vm.Connection.Session, vm_operations.pool_migrate, out reason2)) { lock (_cacheLock) - { - if (!string.IsNullOrEmpty(failureReason)) - vmCache[host.opaque_ref] = failureReason; - } + vmCache[host.opaque_ref] = null; + break; } + + if (!string.IsNullOrEmpty(reason2)) + failureReason += " - " + reason2; + + lock (_cacheLock) + vmCache[host.opaque_ref] = failureReason; } if (!string.IsNullOrEmpty(failureReason)) @@ -203,6 +165,55 @@ protected override bool FailureFoundFor(IXenObject itemToFilterOn, out string fa return false; } + private bool CanDoStorageMigration(VM vm, Host host, out string failureReason) + { + failureReason = null; + + try + { + PIF managementPif = host.Connection.Cache.PIFs.First(p => p.management); + XenAPI.Network managementNetwork = host.Connection.Cache.Resolve(managementPif.network); + + Session session = host.Connection.DuplicateSession(); + Dictionary receiveMapping = Host.migrate_receive(session, host.opaque_ref, managementNetwork.opaque_ref, new Dictionary()); + + var targetSrs = host.Connection.Cache.SRs.Where(sr => sr.SupportsStorageMigration()).ToList(); + var targetNetwork = GetANetwork(host); + + VM.assert_can_migrate(vm.Connection.Session, + vm.opaque_ref, + receiveMapping, + true, + GetVdiMap(vm, targetSrs), + vm.Connection == host.Connection ? new Dictionary, XenRef>() : GetVifMap(vm, targetNetwork), + new Dictionary()); + + return true; + } + catch (Failure failure) + { + // CA-359124 VM is migratable if a snapshot has more VIFs than the VM. As long as the mapping takes this into account. + if (failure.ErrorDescription.Count > 0 && failure.ErrorDescription[0] == Failure.VIF_NOT_IN_MAP && SnapshotsContainExtraVIFs(vm)) + return true; + + if (failure.ErrorDescription.Count > 0 && failure.ErrorDescription[0] == Failure.RBAC_PERMISSION_DENIED) + failureReason = failure.Message.Split('\n')[0].TrimEnd('\r'); // we want the first line only + else if (failure.ErrorDescription.Count > 1 && failure.ErrorDescription[1].Contains(Failure.DYNAMIC_MEMORY_CONTROL_UNAVAILABLE)) + failureReason = FriendlyErrorNames.DYNAMIC_MEMORY_CONTROL_UNAVAILABLE; + else + failureReason = failure.Message; + + log.InfoFormat("VM {0} cannot be migrated to {1}. Reason: {2};", vm.Name(), host.Name(), failure.Message); + return false; + } + catch (Exception e) + { + log.Error($"There was an error while asserting the VM {vm.Name()} can be migrated to {host.Name()}:", e); + failureReason = Messages.HOST_MENU_UNKNOWN_ERROR; + return false; + } + } + /// /// Check if a VM's snapshots contain a reference to a VIF not present in the VM. /// @@ -214,25 +225,23 @@ private static bool SnapshotsContainExtraVIFs(VM vm) return snapVIFs.Any(snapVIF => !vm.VIFs.Contains(snapVIF)); } - private List CollateHosts(IXenObject itemToFilterOn, out Pool thePool) + private List CollateHosts(IXenObject itemToFilterOn, out Pool targetPool) { - thePool = null; + targetPool = null; + var targetHosts = new List(); - List target = new List(); - if (itemToFilterOn is Host) + if (itemToFilterOn is Host host) { - target.Add(itemToFilterOn as Host); - thePool = Helpers.GetPoolOfOne(itemToFilterOn.Connection); + targetHosts.Add(host); + targetPool = Helpers.GetPoolOfOne(host.Connection); } - - if (itemToFilterOn is Pool) + else if (itemToFilterOn is Pool pool) { - Pool pool = itemToFilterOn as Pool; - target.AddRange(pool.Connection.Cache.Hosts); - target.Sort(); - thePool = pool; + targetHosts.AddRange(pool.Connection.Cache.Hosts); + targetHosts.Sort(); + targetPool = pool; } - return target; + return targetHosts; } private Dictionary, XenRef> GetVdiMap(VM vm, List targetSrs) diff --git a/XenAdmin/XenAdmin.csproj b/XenAdmin/XenAdmin.csproj index e17cbd15f..6a936276a 100755 --- a/XenAdmin/XenAdmin.csproj +++ b/XenAdmin/XenAdmin.csproj @@ -3790,7 +3790,7 @@ WlbReportSubscriptionDialog.cs - + UserControl