From bfcdfd982fc3e7001c94102d0b052fa679581504 Mon Sep 17 00:00:00 2001 From: Konstantina Chremmou Date: Fri, 14 Jul 2017 17:44:39 +0100 Subject: [PATCH 1/3] The event registered on the HistoryPage is ConnectionsManager.History.CollectionChanged, not ConnectionsManager.XenConnections.CollectionChanged. --- XenAdmin/TabPages/HistoryPage.Designer.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/XenAdmin/TabPages/HistoryPage.Designer.cs b/XenAdmin/TabPages/HistoryPage.Designer.cs index 2339681fe..134ae7abd 100644 --- a/XenAdmin/TabPages/HistoryPage.Designer.cs +++ b/XenAdmin/TabPages/HistoryPage.Designer.cs @@ -13,7 +13,7 @@ namespace XenAdmin.TabPages /// true if managed resources should be disposed; otherwise, false. protected override void Dispose(bool disposing) { - ConnectionsManager.XenConnections.CollectionChanged -= History_CollectionChanged; + ConnectionsManager.History.CollectionChanged -= History_CollectionChanged; XenAdmin.Actions.ActionBase.NewAction -= Action_NewAction; if (disposing && (components != null)) From ad7e2fa6a64132aeb456db38527796e65724c241 Mon Sep 17 00:00:00 2001 From: Konstantina Chremmou Date: Fri, 14 Jul 2017 18:08:54 +0100 Subject: [PATCH 2/3] CA-257927: Changed ChangeableList.RemoveAll method to fire the CollectionChangedEvent once for all the items to be removed, passing in their list as argument. This method, and the method Clear calling it, are used for alerts, updates, events and connections, hence changed the handling of the CollectionChanged event in these cases to expect either a single item or a collection. In the case of alerts, updates and events this means that we can rebuild the whole list instead of removing single rows which causes the application to hang for big numbers of items. --- .../MainWindowControls/NavigationView.cs | 28 ++++- .../CloseXenCenterWarningDialog.cs | 13 ++- XenAdmin/MainWindow.cs | 109 ++++++++++++------ XenAdmin/TabPages/AlertSummaryPage.cs | 26 +++-- XenAdmin/TabPages/HistoryPage.cs | 13 ++- XenAdmin/TabPages/ManageUpdatesPage.cs | 20 ++-- XenAdmin/TabPages/SrStoragePage.cs | 41 +++++-- XenCenterLib/ChangeableList.cs | 3 +- XenModel/DockerContainers.cs | 44 +++---- XenModel/Folders.cs | 38 +++--- XenModel/OtherConfigWatcher.cs | 39 +++++-- 11 files changed, 253 insertions(+), 121 deletions(-) diff --git a/XenAdmin/Controls/MainWindowControls/NavigationView.cs b/XenAdmin/Controls/MainWindowControls/NavigationView.cs index 356fc806c..74464e949 100644 --- a/XenAdmin/Controls/MainWindowControls/NavigationView.cs +++ b/XenAdmin/Controls/MainWindowControls/NavigationView.cs @@ -131,19 +131,37 @@ namespace XenAdmin.Controls.MainWindowControls public void XenConnectionCollectionChanged(CollectionChangeEventArgs e) { - IXenConnection connection = (IXenConnection)e.Element; - if (connection == null) - return; + IXenConnection connection = e.Element as IXenConnection; if (e.Action == CollectionChangeAction.Add) { + if (connection == null) + return; + connection.BeforeMajorChange += Connection_BeforeMajorChange; connection.AfterMajorChange += Connection_AfterMajorChange; } else if (e.Action == CollectionChangeAction.Remove) { - connection.BeforeMajorChange -= Connection_BeforeMajorChange; - connection.AfterMajorChange -= Connection_AfterMajorChange; + var range = new List(); + if (connection != null) + { + range.Add(connection); + } + else + { + var r = e.Element as List; + if (r != null) + range = r; + else + return; + } + + foreach (var con in range) + { + con.BeforeMajorChange -= Connection_BeforeMajorChange; + con.AfterMajorChange -= Connection_AfterMajorChange; + } } } diff --git a/XenAdmin/Dialogs/WarningDialogs/CloseXenCenterWarningDialog.cs b/XenAdmin/Dialogs/WarningDialogs/CloseXenCenterWarningDialog.cs index ed8cda5ea..fb5d6c4b0 100644 --- a/XenAdmin/Dialogs/WarningDialogs/CloseXenCenterWarningDialog.cs +++ b/XenAdmin/Dialogs/WarningDialogs/CloseXenCenterWarningDialog.cs @@ -196,7 +196,7 @@ namespace XenAdmin.Dialogs.WarningDialogs { Program.BeginInvoke(this, () => { - ActionBase action = (ActionBase)e.Element; + ActionBase action = e.Element as ActionBase; switch (e.Action) { case CollectionChangeAction.Add: @@ -211,7 +211,16 @@ namespace XenAdmin.Dialogs.WarningDialogs } break; case CollectionChangeAction.Remove: - RemoveActionRow(action); + if (action != null) + { + RemoveActionRow(action); + } + else + { + var range = e.Element as List; + if (range != null) + BuildList(); + } break; case CollectionChangeAction.Refresh: BuildList(); diff --git a/XenAdmin/MainWindow.cs b/XenAdmin/MainWindow.cs index ee3f0f951..bba8034f4 100644 --- a/XenAdmin/MainWindow.cs +++ b/XenAdmin/MainWindow.cs @@ -393,14 +393,15 @@ namespace XenAdmin Program.BeginInvoke(Program.MainWindow, () => { - ActionBase action = (ActionBase)e.Element; - if (action == null) - return; - + ActionBase action = e.Element as ActionBase; + switch (e.Action) { case CollectionChangeAction.Add: { + if (action == null) + return; + var meddlingAction = action as MeddlingAction; if (meddlingAction == null) { @@ -419,9 +420,28 @@ namespace XenAdmin } case CollectionChangeAction.Remove: { - action.Changed -= actionChanged; - action.Completed -= actionCompleted; - + if (action != null) + { + action.Changed -= actionChanged; + action.Completed -= actionCompleted; + } + else + { + var range = e.Element as List; + if (range != null) + { + foreach (var a in range) + { + a.Changed -= actionChanged; + a.Completed -= actionCompleted; + } + } + else + { + return; + } + } + int errors = ConnectionsManager.History.Count(a => a.IsCompleted && !a.Succeeded); navigationPane.UpdateNotificationsButton(NotificationsSubMode.Events, errors); @@ -752,14 +772,15 @@ namespace XenAdmin { try { - IXenConnection connection = (IXenConnection)e.Element; - if (connection == null) - return; + IXenConnection connection = e.Element as IXenConnection; navigationPane.XenConnectionCollectionChanged(e); if (e.Action == CollectionChangeAction.Add) { + if (connection == null) + return; + connection.ClearingCache += connection_ClearingCache; connection.ConnectionResult += Connection_ConnectionResult; connection.ConnectionLost += Connection_ConnectionLost; @@ -779,38 +800,54 @@ namespace XenAdmin } else if (e.Action == CollectionChangeAction.Remove) { - connection.ClearingCache -= connection_ClearingCache; - connection.ConnectionResult -= Connection_ConnectionResult; - connection.ConnectionLost -= Connection_ConnectionLost; - connection.ConnectionClosed -= Connection_ConnectionClosed; - connection.ConnectionReconnecting -= connection_ConnectionReconnecting; - connection.XenObjectsUpdated -= Connection_XenObjectsUpdated; - connection.Cache.DeregisterCollectionChanged(MessageCollectionChangedWithInvoke); - connection.Cache.DeregisterCollectionChanged(PoolCollectionChangedWithInvoke); - connection.Cache.DeregisterCollectionChanged(HostCollectionChangedWithInvoke); - connection.Cache.DeregisterCollectionChanged(VMCollectionChangedWithInvoke); - connection.Cache.DeregisterCollectionChanged(SRCollectionChangedWithInvoke); - connection.Cache.DeregisterCollectionChanged(FolderCollectionChangedWithInvoke); - - connection.Cache.DeregisterCollectionChanged(TaskCollectionChangedWithInvoke); - - connection.CachePopulated -= connection_CachePopulated; - - foreach (VM vm in connection.Cache.VMs) + var range = new List(); + if (connection != null) { - ConsolePanel.closeVNCForSource(vm); + range.Add(connection); + } + else + { + var r = e.Element as List; + if (r != null) + range = r; + else + return; } - foreach (Host host in connection.Cache.Hosts) + foreach (var con in range) { - ConsolePanel.closeVNCForSource(host.ControlDomainZero); + con.ClearingCache -= connection_ClearingCache; + con.ConnectionResult -= Connection_ConnectionResult; + con.ConnectionLost -= Connection_ConnectionLost; + con.ConnectionClosed -= Connection_ConnectionClosed; + con.ConnectionReconnecting -= connection_ConnectionReconnecting; + con.XenObjectsUpdated -= Connection_XenObjectsUpdated; + con.Cache.DeregisterCollectionChanged(MessageCollectionChangedWithInvoke); + con.Cache.DeregisterCollectionChanged(PoolCollectionChangedWithInvoke); + con.Cache.DeregisterCollectionChanged(HostCollectionChangedWithInvoke); + con.Cache.DeregisterCollectionChanged(VMCollectionChangedWithInvoke); + con.Cache.DeregisterCollectionChanged(SRCollectionChangedWithInvoke); + con.Cache.DeregisterCollectionChanged(FolderCollectionChangedWithInvoke); - foreach (VM vm in host.OtherControlDomains) - CvmConsolePanel.closeVNCForSource(vm); + con.Cache.DeregisterCollectionChanged(TaskCollectionChangedWithInvoke); + + con.CachePopulated -= connection_CachePopulated; + + foreach (VM vm in con.Cache.VMs) + { + ConsolePanel.closeVNCForSource(vm); + } + + foreach (Host host in con.Cache.Hosts) + { + ConsolePanel.closeVNCForSource(host.ControlDomainZero); + + foreach (VM vm in host.OtherControlDomains) + CvmConsolePanel.closeVNCForSource(vm); + } + + con.EndConnect(); } - - connection.EndConnect(); - RequestRefreshTreeView(); //CA-41228 refresh submenu items when there are no connections SelectionManager.RefreshSelection(); diff --git a/XenAdmin/TabPages/AlertSummaryPage.cs b/XenAdmin/TabPages/AlertSummaryPage.cs index a8f76b7f5..3830f6d73 100644 --- a/XenAdmin/TabPages/AlertSummaryPage.cs +++ b/XenAdmin/TabPages/AlertSummaryPage.cs @@ -40,8 +40,6 @@ using System.Windows.Forms; using XenAdmin.Controls; using XenAdmin.Core; using XenAdmin.Dialogs; -using XenAdmin.Network; -using XenAPI; using XenAdmin.Alerts; using XenAdmin.Help; using System.Threading; @@ -411,7 +409,6 @@ namespace XenAdmin.TabPages } } - #endregion private DataGridViewRow FindAlertRow(ToolStripMenuItem toolStripMenuItem) { @@ -425,6 +422,8 @@ namespace XenAdmin.TabPages select row).FirstOrDefault(); } + #endregion + private void ToolStripMenuItemHelp_Click(object sender, EventArgs e) { // We should only be here if one item is selected, we dont do multi-help @@ -585,24 +584,27 @@ namespace XenAdmin.TabPages private void AlertsCollectionChanged(object sender, CollectionChangeEventArgs e) { Program.AssertOnEventThread(); - if (e.Element == null) - { - // We take the null element to mean there has been a batch remove - Rebuild(); - return; - } - Alert a = e.Element as Alert; + switch (e.Action) { case CollectionChangeAction.Add: Rebuild(); // rebuild entire alert list to ensure filtering and sorting break; case CollectionChangeAction.Remove: - RemoveAlertRow(a); + + var a = e.Element as Alert; + if (a != null) + RemoveAlertRow(a); + else + { + var range = e.Element as List; + if (range != null) + Rebuild(); + } break; } } - + private void UpdateActionEnablement() { toolStripButtonExportAll.Enabled = Alert.NonDismissingAlertCount > 0; diff --git a/XenAdmin/TabPages/HistoryPage.cs b/XenAdmin/TabPages/HistoryPage.cs index 91dce18b1..e2197e721 100644 --- a/XenAdmin/TabPages/HistoryPage.cs +++ b/XenAdmin/TabPages/HistoryPage.cs @@ -93,7 +93,7 @@ namespace XenAdmin.TabPages { Program.BeginInvoke(this, () => { - ActionBase action = (ActionBase)e.Element; + ActionBase action = e.Element as ActionBase; switch (e.Action) { case CollectionChangeAction.Add: @@ -104,7 +104,16 @@ namespace XenAdmin.TabPages InsertActionRow(index, action); break; case CollectionChangeAction.Remove: - RemoveActionRow(action); + if (action != null) + { + RemoveActionRow(action); + } + else + { + var range = e.Element as List; + if (range != null) + BuildRowList(); + } break; case CollectionChangeAction.Refresh: BuildRowList(); diff --git a/XenAdmin/TabPages/ManageUpdatesPage.cs b/XenAdmin/TabPages/ManageUpdatesPage.cs index e94f2807d..f166e4104 100644 --- a/XenAdmin/TabPages/ManageUpdatesPage.cs +++ b/XenAdmin/TabPages/ManageUpdatesPage.cs @@ -91,20 +91,24 @@ namespace XenAdmin.TabPages private void UpdatesCollectionChanged(object sender, CollectionChangeEventArgs e) { Program.AssertOnEventThread(); - if (e.Element == null) - { - // We take the null element to mean there has been a batch remove - Rebuild(); - return; - } - Alert a = e.Element as Alert; + switch (e.Action) { case CollectionChangeAction.Add: Rebuild(); // rebuild entire alert list to ensure filtering and sorting break; case CollectionChangeAction.Remove: - RemoveUpdateRow(a); + Alert a = e.Element as Alert; + if (a != null) + { + RemoveUpdateRow(a); + } + else + { + var range = e.Element as List; + if (range != null) + Rebuild(); + } break; } } diff --git a/XenAdmin/TabPages/SrStoragePage.cs b/XenAdmin/TabPages/SrStoragePage.cs index 8997dbb54..572b7e000 100644 --- a/XenAdmin/TabPages/SrStoragePage.cs +++ b/XenAdmin/TabPages/SrStoragePage.cs @@ -230,19 +230,40 @@ namespace XenAdmin.TabPages void History_CollectionChanged(object sender, CollectionChangeEventArgs e) { Program.BeginInvoke(Program.MainWindow, () => - { - SrRefreshAction a = e.Element as SrRefreshAction; - if (a == null) - return; + { + SrRefreshAction a = e.Element as SrRefreshAction; - if (e.Action == CollectionChangeAction.Add) - a.Completed += a_Completed; + if (e.Action == CollectionChangeAction.Add) + { + if (a != null) + a.Completed += a_Completed; + else + return; + } - if (e.Action == CollectionChangeAction.Remove) - a.Completed -= a_Completed; + if (e.Action == CollectionChangeAction.Remove) + { + if (a != null) + { + a.Completed -= a_Completed; + } + else + { + var range = e.Element as List; + if (range != null) + { + foreach (var sra in range) + sra.Completed -= a_Completed; + } + else + { + return; + } + } + } - RefreshButtons(); - }); + RefreshButtons(); + }); } void a_Completed(ActionBase sender) diff --git a/XenCenterLib/ChangeableList.cs b/XenCenterLib/ChangeableList.cs index a56f3f9df..b9d95dc4d 100644 --- a/XenCenterLib/ChangeableList.cs +++ b/XenCenterLib/ChangeableList.cs @@ -84,7 +84,8 @@ namespace XenAdmin.Core { var toRemove = FindAll(match); base.RemoveAll(match); - toRemove.ForEach(item => OnCollectionChanged(new CollectionChangeEventArgs(CollectionChangeAction.Remove, item))); + if (toRemove.Count > 0) + OnCollectionChanged(new CollectionChangeEventArgs(CollectionChangeAction.Remove, toRemove)); return toRemove.Count; } diff --git a/XenModel/DockerContainers.cs b/XenModel/DockerContainers.cs index 9f45f5bb0..e50f26c94 100644 --- a/XenModel/DockerContainers.cs +++ b/XenModel/DockerContainers.cs @@ -46,10 +46,6 @@ namespace XenAdmin.Model { public class DockerContainers { - static DockerContainers() - { - } - public static void InitDockerContainers() { Trace.Assert(InvokeHelper.Synchronizer != null); @@ -64,22 +60,31 @@ namespace XenAdmin.Model private static void XenConnections_CollectionChanged(object sender, CollectionChangeEventArgs e) { InvokeHelper.BeginInvoke(() => - { - IXenConnection connection = e.Element as IXenConnection; - if (connection == null) - return; + { + IXenConnection connection = e.Element as IXenConnection; - switch (e.Action) - { - case CollectionChangeAction.Add: - AddConnection(connection); - break; + switch (e.Action) + { + case CollectionChangeAction.Add: + if (connection != null) + AddConnection(connection); + break; - case CollectionChangeAction.Remove: - RemoveConnection(connection); - break; - } - }); + case CollectionChangeAction.Remove: + if (connection != null) + { + RemoveConnection(connection); + } + else + { + var range = e.Element as List; + if (range != null) + foreach (var con in range) + RemoveConnection(con); + } + break; + } + }); } private static CollectionChangeEventHandler CollectionChangedWithInvoke; @@ -106,9 +111,8 @@ namespace XenAdmin.Model { InvokeHelper.AssertOnEventThread(); - Trace.Assert(e.Element is VM); - var vm = e.Element as VM; + Trace.Assert(vm != null, "The item changed is not a VM"); switch (e.Action) { diff --git a/XenModel/Folders.cs b/XenModel/Folders.cs index 0598795ec..cd19a580d 100644 --- a/XenModel/Folders.cs +++ b/XenModel/Folders.cs @@ -74,24 +74,32 @@ namespace XenAdmin.Model private static void XenConnections_CollectionChanged(object sender, CollectionChangeEventArgs e) { - //Program.AssertOnEventThread(); InvokeHelper.BeginInvoke(() => - { - IXenConnection connection = e.Element as IXenConnection; - if (connection == null) - return; + { + IXenConnection connection = e.Element as IXenConnection; - switch (e.Action) - { - case CollectionChangeAction.Add: - AddConnection(connection); - break; + switch (e.Action) + { + case CollectionChangeAction.Add: + if (connection != null) + AddConnection(connection); + break; - case CollectionChangeAction.Remove: - RemoveConnection(connection); - break; - } - }); + case CollectionChangeAction.Remove: + if (connection != null) + { + RemoveConnection(connection); + } + else + { + var range = e.Element as List; + if (range != null) + foreach (var con in range) + RemoveConnection(con); + } + break; + } + }); } private static CollectionChangeEventHandler CollectionChangedWithInvoke; diff --git a/XenModel/OtherConfigWatcher.cs b/XenModel/OtherConfigWatcher.cs index e1499fea2..30b36aade 100644 --- a/XenModel/OtherConfigWatcher.cs +++ b/XenModel/OtherConfigWatcher.cs @@ -30,6 +30,7 @@ */ using System; +using System.Collections.Generic; using System.ComponentModel; using XenAdmin.Network; using XenAPI; @@ -93,11 +94,12 @@ namespace XenAdmin return; IXenConnection connection = e.Element as IXenConnection; - if (connection == null) - return; if (e.Action == CollectionChangeAction.Add) { + if (connection == null) + return; + connection.Cache.RegisterCollectionChanged(PoolCollectionChangedWithInvoke); connection.Cache.RegisterCollectionChanged(HostCollectionChangedWithInvoke); connection.Cache.RegisterCollectionChanged(VMCollectionChangedWithInvoke); @@ -112,15 +114,32 @@ namespace XenAdmin } else if (e.Action == CollectionChangeAction.Remove) { - connection.Cache.DeregisterCollectionChanged(PoolCollectionChangedWithInvoke); - connection.Cache.DeregisterCollectionChanged(HostCollectionChangedWithInvoke); - connection.Cache.DeregisterCollectionChanged(VMCollectionChangedWithInvoke); - connection.Cache.DeregisterCollectionChanged(SRCollectionChangedWithInvoke); - connection.Cache.DeregisterCollectionChanged(VDICollectionChangedWithInvoke); - connection.Cache.DeregisterCollectionChanged(NetworkCollectionChangedWithInvoke); + var range = new List(); + if (connection != null) + { + range.Add(connection); + } + else + { + var r = e.Element as List; + if (r != null) + range = r; + else + return; + } - connection.XenObjectsUpdated -= connection_XenObjectsUpdated; - connection.ConnectionStateChanged -= connection_ConnectionStateChanged; + foreach (var con in range) + { + con.Cache.DeregisterCollectionChanged(PoolCollectionChangedWithInvoke); + con.Cache.DeregisterCollectionChanged(HostCollectionChangedWithInvoke); + con.Cache.DeregisterCollectionChanged(VMCollectionChangedWithInvoke); + con.Cache.DeregisterCollectionChanged(SRCollectionChangedWithInvoke); + con.Cache.DeregisterCollectionChanged(VDICollectionChangedWithInvoke); + con.Cache.DeregisterCollectionChanged(NetworkCollectionChangedWithInvoke); + + con.XenObjectsUpdated -= connection_XenObjectsUpdated; + con.ConnectionStateChanged -= connection_ConnectionStateChanged; + } } MarkEventsReadyToFire(true); From 6debb9de4c488b82e4440a9f988cd2fc990e8628 Mon Sep 17 00:00:00 2001 From: Konstantina Chremmou Date: Wed, 26 Jul 2017 08:11:47 +0100 Subject: [PATCH 3/3] CA-257927: If the list of items to be removed contains only 1 item, keep firing the CollectionChangedEvent for a single item. Signed-off-by: Konstantina Chremmou --- XenCenterLib/ChangeableList.cs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/XenCenterLib/ChangeableList.cs b/XenCenterLib/ChangeableList.cs index b9d95dc4d..68bbb656b 100644 --- a/XenCenterLib/ChangeableList.cs +++ b/XenCenterLib/ChangeableList.cs @@ -84,8 +84,10 @@ namespace XenAdmin.Core { var toRemove = FindAll(match); base.RemoveAll(match); - if (toRemove.Count > 0) + if (toRemove.Count > 1) OnCollectionChanged(new CollectionChangeEventArgs(CollectionChangeAction.Remove, toRemove)); + else if (toRemove.Count == 1) + OnCollectionChanged(new CollectionChangeEventArgs(CollectionChangeAction.Remove, toRemove[0])); return toRemove.Count; }