From 025e92bc7d44101646d80f6e2dfc56f75041bc9c Mon Sep 17 00:00:00 2001 From: Mihaela Stoica Date: Fri, 4 May 2018 15:40:42 +0100 Subject: [PATCH] CA-286291: Some more refactoring in VMOperationToolStripManuItem (including the removal of the HostListUpdater class) Signed-off-by: Mihaela Stoica --- .../Controls/VMOperationToolStripMenuItem.cs | 425 ++++++++---------- .../Commands/Controls/WlbRecommendations.cs | 40 -- 2 files changed, 196 insertions(+), 269 deletions(-) diff --git a/XenAdmin/Commands/Controls/VMOperationToolStripMenuItem.cs b/XenAdmin/Commands/Controls/VMOperationToolStripMenuItem.cs index 4cf33cfb4..06ce05c33 100644 --- a/XenAdmin/Commands/Controls/VMOperationToolStripMenuItem.cs +++ b/XenAdmin/Commands/Controls/VMOperationToolStripMenuItem.cs @@ -36,7 +36,6 @@ using System.Windows.Forms; using XenAdmin.Controls; using XenAPI; using XenAdmin.Core; -using System.Threading; using XenAdmin.Actions; using XenAdmin.Network; @@ -52,7 +51,6 @@ namespace XenAdmin.Commands private static readonly log4net.ILog log = log4net.LogManager.GetLogger(System.Reflection.MethodBase.GetCurrentMethod().DeclaringType); private readonly vm_operations _operation; private readonly bool _resumeAfter; - private HostListUpdater hostListUpdater; protected VMOperationToolStripMenuItem(Command command, bool inContextMenu, vm_operations operation) : base(command, inContextMenu) @@ -72,8 +70,7 @@ namespace XenAdmin.Commands protected override void OnDropDownClosed(EventArgs e) { base.OnDropDownClosed(e); - if (hostListUpdater != null) - hostListUpdater.Stop(); + Stop(); } @@ -112,280 +109,250 @@ namespace XenAdmin.Commands item.Tag = host; base.DropDownItems.Add(item); } - - SelectedItemCollection selection = Command.GetSelection(); - Session session = selection[0].Connection.DuplicateSession(); - hostListUpdater = new HostListUpdater(); - hostListUpdater.updateHostList(this, session); - + UpdateHostList(); } /// /// Hook to add additional members to the menu item /// Note: Called on main window thread by executing code /// - /// protected virtual void AddAdditionalMenuItems(SelectedItemCollection selection) { return; } - private class HostListUpdater + #region UpdateHostList + + private ProduceConsumerQueue workerQueueWithoutWlb; + readonly object locker = new object(); + private bool stopped; + + private bool Stopped { - private ProduceConsumerQueue workerQueueWithoutWlb; - readonly object _locker = new object(); - private bool _stopped; - - private bool Stopped + set { - set + lock (locker) { - lock (_locker) - { - _stopped = value; - } - } - get - { - lock (_locker) - { - return _stopped; - } + stopped = value; } } - - public void Stop() + get { - Stopped = true; - if (workerQueueWithoutWlb != null) - workerQueueWithoutWlb.CancelWorkers(false); + lock (locker) + { + return stopped; + } } + } - public void updateHostList(VMOperationToolStripMenuItem menu, Session session) + private void Stop() + { + Stopped = true; + if (workerQueueWithoutWlb != null) + workerQueueWithoutWlb.CancelWorkers(false); + } + + private void UpdateHostList() + { + Stopped = false; + + var selection = Command.GetSelection(); + var connection = selection[0].Connection; + + if (Helpers.WlbEnabled(connection)) { - Stopped = false; - - SelectedItemCollection selection = menu.Command.GetSelection(); - var connection = selection[0].Connection; var vms = selection.AsXenObjects(); - - if (Helpers.WlbEnabled(connection)) + var retrieveVmRecommendationsAction = new WlbRetrieveVmRecommendationsAction(connection, vms); + retrieveVmRecommendationsAction.Completed += delegate { - var retrieveVmRecommendationsAction = new WlbRetrieveVmRecommendationsAction(connection, vms); - retrieveVmRecommendationsAction.Completed += delegate + if (Stopped || retrieveVmRecommendationsAction.Cancelled || + !retrieveVmRecommendationsAction.Succeeded) + return; + + var recommendations = new WlbRecommendations(vms, retrieveVmRecommendationsAction.Recommendations); + + Program.Invoke(Program.MainWindow, delegate { - if (Stopped || retrieveVmRecommendationsAction.Cancelled || !retrieveVmRecommendationsAction.Succeeded) - return; + if (recommendations.IsError) + EnableAppropriateHostsNoWlb(); + else + EnableAppropriateHostsWlb(recommendations); + }); + }; + retrieveVmRecommendationsAction.RunAsync(); + } + else + { + EnableAppropriateHostsNoWlb(); + } + } - var recommendations = new WlbRecommendations(vms, retrieveVmRecommendationsAction.Recommendations); + private void EnableAppropriateHostsWlb(WlbRecommendations recommendations) + { + if (Stopped || DropDownItems.Count == 0) + return; - Program.Invoke(Program.MainWindow, delegate - { - if (recommendations.IsError) - EnableAppropriateHostsNoWlb(menu, session); - else - EnableAppropriateHostsWlb(menu, session, recommendations); - }); - }; - retrieveVmRecommendationsAction.RunAsync(); - } - else + // set the first menu item to be the WLB optimal server menu item + var firstItem = DropDownItems[0] as VMOperationToolStripMenuSubItem; + if (firstItem == null) + return; + + var selection = Command.GetSelection(); + + var firstItemCmd = new VMOperationWlbOptimalServerCommand(Command.MainWindowCommandInterface, + selection, _operation, recommendations); + + firstItem.Command = firstItemCmd; + firstItem.Enabled = firstItemCmd.CanExecute(); + + var hostMenuItems = new List(); + foreach (var item in DropDownItems) + { + var hostMenuItem = item as VMOperationToolStripMenuSubItem; + if (hostMenuItem == null) + continue; + + var host = hostMenuItem.Tag as Host; + if (host != null) { - EnableAppropriateHostsNoWlb(menu, session); + var cmd = new VMOperationWlbHostCommand(Command.MainWindowCommandInterface, selection, host, + _operation, recommendations.GetStarRating(host)); + + hostMenuItem.Command = cmd; + hostMenuItem.Enabled = cmd.CanExecute(); + + hostMenuItems.Add(hostMenuItem); } } + // sort the hostMenuItems by star rating + hostMenuItems.Sort(new WlbHostStarCompare()); - private void EnableAppropriateHostsWlb(VMOperationToolStripMenuItem menu, Session session, WlbRecommendations recommendations) + // refresh the drop-down-items from the menuItems. + foreach (VMOperationToolStripMenuSubItem menuItem in hostMenuItems) { + DropDownItems.Insert(hostMenuItems.IndexOf(menuItem) + 1, menuItem); + } + + AddAdditionalMenuItems(selection); + } + + private void EnableAppropriateHostsNoWlb() + { + if (Stopped || DropDownItems.Count == 0) + return; + + var firstItem = DropDownItems[0] as VMOperationToolStripMenuSubItem; + if (firstItem == null) + return; + + // API calls could happen in CanExecute(), which take time to wait. So a Producer-Consumer-Queue with size 25 is used here to : + // 1. Make API calls for different menu items happen in parallel; + // 2. Limit the count of concurrent threads (now it's 25). + workerQueueWithoutWlb = new ProduceConsumerQueue(25); + + var selection = Command.GetSelection(); + var connection = selection[0].Connection; + var session = connection.DuplicateSession(); + + var affinityHost = connection.Resolve(((VM) selection[0].XenObject).affinity); + + EnqueueHostMenuItem(this, session, affinityHost, firstItem, true); + + var hostMenuItems = DropDownItems.OfType().ToList(); + + if (Stopped) + return; + + // Adds the migrate wizard button, do this before the enable checks on the other items + AddAdditionalMenuItems(selection); + + foreach (VMOperationToolStripMenuSubItem item in hostMenuItems) + { + var host = item.Tag as Host; + if (host != null) + { + var tempItem = item; + EnqueueHostMenuItem(this, session, host, tempItem, false); + } + } + } + + private void EnqueueHostMenuItem(VMOperationToolStripMenuItem menu, Session session, Host host, VMOperationToolStripMenuSubItem hostMenuItem, bool isHomeServer) + { + workerQueueWithoutWlb.EnqueueItem(() => + { + var selection = menu.Command.GetSelection(); + var cmd = isHomeServer + ? new VMOperationHomeServerCommand(menu.Command.MainWindowCommandInterface, selection, menu._operation, session) + : new VMOperationHostCommand(menu.Command.MainWindowCommandInterface, selection, delegate { return host; }, host.Name().EscapeAmpersands(), menu._operation, session); + + var oldMigrateCmdCanRun = cmd.CanExecute(); if (Stopped) return; - SelectedItemCollection selection = menu.Command.GetSelection(); - // set the first menu item to be the WLB optimal server menu item - if (menu.DropDownItems.Count == 0) - return; - - var firstItem = menu.DropDownItems[0] as VMOperationToolStripMenuSubItem; - if (firstItem == null) - return; - - var firstItemCmd = new VMOperationWlbOptimalServerCommand(menu.Command.MainWindowCommandInterface, selection, menu._operation, recommendations); - var firstItemCmdCanExecute = firstItemCmd.CanExecute(); - - - firstItem.Command = firstItemCmd; - firstItem.Enabled = firstItemCmdCanExecute; - - List hostMenuItems = new List(); - foreach (var item in menu.DropDownItems) + if (host == null || menu._operation == vm_operations.start_on || oldMigrateCmdCanRun) { - var hostMenuItem = item as VMOperationToolStripMenuSubItem; - if (hostMenuItem == null) - continue; - - Host host = hostMenuItem.Tag as Host; - if (host != null) + Program.Invoke(Program.MainWindow, delegate { - var cmd = new VMOperationWlbHostCommand(menu.Command.MainWindowCommandInterface, selection, host, menu._operation, recommendations.GetStarRating(host)); - var canExecute = cmd.CanExecute(); - hostMenuItem.Command = cmd; - hostMenuItem.Enabled = canExecute; - - hostMenuItems.Add(hostMenuItem); - } - } - - // Shuffle the list to make it look cool - // Helpers.ShuffleList(hostMenuItems); - - // sort the hostMenuItems by star rating - hostMenuItems.Sort(new WlbHostStarCompare()); - - // refresh the drop-down-items from the menuItems. - foreach (VMOperationToolStripMenuSubItem menuItem in hostMenuItems) - { - menu.DropDownItems.Insert(hostMenuItems.IndexOf(menuItem) + 1, menuItem); - } - - menu.AddAdditionalMenuItems(selection); - - } - - private void EnableAppropriateHostsNoWlb(VMOperationToolStripMenuItem menu, Session session) - { - if (Stopped) - return; - - if (menu.DropDownItems.Count == 0) - return; - - var firstItem = menu.DropDownItems[0] as VMOperationToolStripMenuSubItem; - if (firstItem == null) - return; - - SelectedItemCollection selection = menu.Command.GetSelection(); - IXenConnection connection = selection[0].Connection; - workerQueueWithoutWlb = new ProduceConsumerQueue(25); - VMOperationCommand cmdHome = new VMOperationHomeServerCommand(menu.Command.MainWindowCommandInterface, selection, menu._operation, session); - Host affinityHost = connection.Resolve(((VM)selection[0].XenObject).affinity); - - bool oldMigrateToHomeCmdCanRun = cmdHome.CanExecute(); - if (affinityHost == null || menu._operation == vm_operations.start_on || oldMigrateToHomeCmdCanRun) - { - firstItem.Command = cmdHome; - firstItem.Enabled = oldMigrateToHomeCmdCanRun; + hostMenuItem.Enabled = oldMigrateCmdCanRun; + }); } else { - VMOperationCommand cpmCmdHome = new CrossPoolMigrateToHomeCommand(menu.Command.MainWindowCommandInterface, selection, affinityHost); + var cpmCmd = isHomeServer + ? new CrossPoolMigrateToHomeCommand(menu.Command.MainWindowCommandInterface, selection, host) + : new CrossPoolMigrateCommand(menu.Command.MainWindowCommandInterface, selection, host, menu._resumeAfter); - if (cpmCmdHome.CanExecute()) + var crossPoolMigrateCmdCanRun = cpmCmd.CanExecute(); + if (Stopped) + return; + + Program.Invoke(Program.MainWindow, delegate { - firstItem.Command = cpmCmdHome; - firstItem.Enabled = true; - } - else - { - firstItem.Command = cmdHome; - firstItem.Enabled = false; - } - } - - List hostMenuItems = menu.DropDownItems.OfType().ToList(); - - if (Stopped) - return; - - // Adds the migrate wizard button, do this before the enable checks on the other items - menu.AddAdditionalMenuItems(selection); - - foreach (VMOperationToolStripMenuSubItem item in hostMenuItems) - { - - Host host = item.Tag as Host; - if (host != null) - { - // API calls could happen in CanExecute(), which take time to wait. - // So a Producer-Consumer-Queue with size 25 is used here to : - // 1. Make API calls for different menu items happen in parallel; - // 2. Limit the count of concurrent threads (now it's 25). - workerQueueWithoutWlb.EnqueueItem(() => + if (crossPoolMigrateCmdCanRun || !string.IsNullOrEmpty(cpmCmd.CantExecuteReason)) { - VMOperationCommand cmd = new VMOperationHostCommand(menu.Command.MainWindowCommandInterface, selection, delegate { return host; }, host.Name().EscapeAmpersands(), menu._operation, session); - CrossPoolMigrateCommand cpmCmd = new CrossPoolMigrateCommand(menu.Command.MainWindowCommandInterface, selection, host, menu._resumeAfter); - - VMOperationToolStripMenuSubItem tempItem = item; - bool oldMigrateCmdCanRun = cmd.CanExecute(); - if ((menu._operation == vm_operations.start_on) || oldMigrateCmdCanRun) - { - if (Stopped) - return; - - Program.Invoke(Program.MainWindow, delegate - { - tempItem.Command = cmd; - tempItem.Enabled = oldMigrateCmdCanRun; - }); - } - else - { - bool crossPoolMigrateCmdCanRun = cpmCmd.CanExecute(); - if (crossPoolMigrateCmdCanRun || !string.IsNullOrEmpty(cpmCmd.CantExecuteReason)) - { - if (Stopped) - return; - - Program.Invoke(Program.MainWindow, delegate - { - tempItem.Command = cpmCmd; - tempItem.Enabled = crossPoolMigrateCmdCanRun; - }); - } - else - { - if (Stopped) - return; - - Program.Invoke(Program.MainWindow, delegate - { - tempItem.Command = cmd; - tempItem.Enabled = oldMigrateCmdCanRun; - }); - } - } - }); - } + hostMenuItem.Command = cpmCmd; + hostMenuItem.Enabled = crossPoolMigrateCmdCanRun; + } + else + { + hostMenuItem.Command = cmd; + hostMenuItem.Enabled = false; + } + }); } - } + }); + } + + #endregion - /// - /// This class is an implementation of the 'IComparer' interface - /// for sorting vm placement menuItem List when wlb is enabled - /// - private class WlbHostStarCompare : IComparer + /// + /// This class is an implementation of the 'IComparer' interface + /// for sorting vm placement menuItem List when wlb is enabled + /// + private class WlbHostStarCompare : IComparer + { + public int Compare(VMOperationToolStripMenuSubItem x, VMOperationToolStripMenuSubItem y) { - public int Compare(VMOperationToolStripMenuSubItem x, VMOperationToolStripMenuSubItem y) - { - int result = 0; + int result = 0; - // if x and y are enabled, compare their start rating - if (x.Enabled && y.Enabled) - result = y.StarRating.CompareTo(x.StarRating); + // if x and y are enabled, compare their start rating + if (x.Enabled && y.Enabled) + result = y.StarRating.CompareTo(x.StarRating); - // if x and y are disabled, they are equal - else if (!x.Enabled && !y.Enabled) - result = 0; + // if x and y are disabled, they are equal + else if (!x.Enabled && !y.Enabled) + result = 0; - // if x is disabled, y is greater - else if (!x.Enabled) - result = 1; + // if x is disabled, y is greater + else if (!x.Enabled) + result = 1; - // if y is disabled, x is greater - else if (!y.Enabled) - result = -1; + // if y is disabled, x is greater + else if (!y.Enabled) + result = -1; - return result; - } + return result; } } } diff --git a/XenAdmin/Commands/Controls/WlbRecommendations.cs b/XenAdmin/Commands/Controls/WlbRecommendations.cs index 9a2e552db..d47937a26 100644 --- a/XenAdmin/Commands/Controls/WlbRecommendations.cs +++ b/XenAdmin/Commands/Controls/WlbRecommendations.cs @@ -31,7 +31,6 @@ using System; using System.Collections.Generic; -using System.Text; using XenAPI; using System.Collections.ObjectModel; using XenAdmin.Core; @@ -45,7 +44,6 @@ namespace XenAdmin.Commands internal class WlbRecommendations { private readonly ReadOnlyCollection _vms; - private readonly bool _initialized; private readonly Dictionary, string[]>> _recommendations; private readonly bool _isError; @@ -61,41 +59,7 @@ namespace XenAdmin.Commands _vms = new ReadOnlyCollection(vms); _recommendations = recommendations; _isError = recommendations == null; - _initialized = recommendations != null; } - /* - /// - /// Calls VM.retrieve_wlb_recommendations for each of the VMs specified in the constructor. - /// - public void Initialize() - { - if (_initialized) - { - throw new InvalidOperationException("Already initialized"); - } - - _initialized = true; - - if (Helpers.WlbEnabled(_vms[0].Connection)) - { - try - { - foreach (VM vm in _vms) - { - _recommendations[vm] = VM.retrieve_wlb_recommendations(_session, vm.opaque_ref); - } - } - catch (Exception e) - { - log.Error("Error getting WLB recommendations", e); - _isError = true; - } - } - else - { - _isError = true; - } - }*/ /// /// Gets a value indicating whether an exception was thrown when calling VM.retrieve_wlb_recommendations. @@ -115,10 +79,6 @@ namespace XenAdmin.Commands { throw new InvalidOperationException("There was an error getting the WLB recommendations."); } - if (!_initialized) - { - throw new InvalidOperationException("Initialize() has not been called."); - } } public Host GetOptimalServer(VM vm)