From fcb63995355801c4b55abf1c265706e9d0d76932 Mon Sep 17 00:00:00 2001 From: Mihaela Stoica Date: Wed, 28 Mar 2018 16:17:08 +0100 Subject: [PATCH] CP-17099: Changes following code review Signed-off-by: Mihaela Stoica --- .../NewSRWizard_Pages/Frontends/LVMoHBA.cs | 2 +- .../NewSRWizard_Pages/Frontends/LVMoISCSI.cs | 32 +++---- .../Actions/SR/ISCSIPopulateIQNsAction.cs | 73 ++++++++-------- .../Actions/SR/ISCSIPopulateLunsAction.cs | 84 +++++++++---------- 4 files changed, 86 insertions(+), 105 deletions(-) diff --git a/XenAdmin/Wizards/NewSRWizard_Pages/Frontends/LVMoHBA.cs b/XenAdmin/Wizards/NewSRWizard_Pages/Frontends/LVMoHBA.cs index 6c0243f96..906891636 100644 --- a/XenAdmin/Wizards/NewSRWizard_Pages/Frontends/LVMoHBA.cs +++ b/XenAdmin/Wizards/NewSRWizard_Pages/Frontends/LVMoHBA.cs @@ -57,7 +57,7 @@ namespace XenAdmin.Wizards.NewSRWizard_Pages.Frontends SrType = SR.SRTypes.lvmohba; } - public virtual SR.SRTypes SrType { get; set; } + public SR.SRTypes SrType { get; set; } public virtual bool ShowNicColumn { get { return false; } } diff --git a/XenAdmin/Wizards/NewSRWizard_Pages/Frontends/LVMoISCSI.cs b/XenAdmin/Wizards/NewSRWizard_Pages/Frontends/LVMoISCSI.cs index 161662f1f..2e7d80c4b 100644 --- a/XenAdmin/Wizards/NewSRWizard_Pages/Frontends/LVMoISCSI.cs +++ b/XenAdmin/Wizards/NewSRWizard_Pages/Frontends/LVMoISCSI.cs @@ -470,17 +470,13 @@ namespace XenAdmin.Wizards.NewSRWizard_Pages.Frontends UpdateButtons(); - if (IscsiUseChapCheckBox.Checked) - { - IscsiPopulateIqnsAction = new ISCSIPopulateIQNsAction(Connection, - getIscsiHost(), getIscsiPort(), IScsiChapUserTextBox.Text, IScsiChapSecretTextBox.Text, SrType); - } - else - { - IscsiPopulateIqnsAction = new ISCSIPopulateIQNsAction(Connection, - getIscsiHost(), getIscsiPort(), null, null, SrType); - } + var chapUser = IscsiUseChapCheckBox.Checked ? IScsiChapUserTextBox.Text : null; + var chapPwd = IscsiUseChapCheckBox.Checked ? IScsiChapSecretTextBox.Text : null; + IscsiPopulateIqnsAction = SrType == SR.SRTypes.gfs2 + ? new Gfs2PopulateIQNsAction(Connection, getIscsiHost(), getIscsiPort(), chapUser, chapPwd) + : new ISCSIPopulateIQNsAction(Connection, getIscsiHost(), getIscsiPort(), chapUser, chapPwd); + IscsiPopulateIqnsAction.Completed += IscsiPopulateIqnsAction_Completed; controlDisabler.Reset(); @@ -599,17 +595,13 @@ namespace XenAdmin.Wizards.NewSRWizard_Pages.Frontends comboBoxIscsiLuns.Items.Clear(); LunMap.Clear(); - if (IscsiUseChapCheckBox.Checked) - { - IscsiPopulateLunsAction = new ISCSIPopulateLunsAction(Connection, - getIscsiHost(), getIscsiPort(), getIscsiIQN(), IScsiChapUserTextBox.Text, IScsiChapSecretTextBox.Text, SrType); - } - else - { - IscsiPopulateLunsAction = new ISCSIPopulateLunsAction(Connection, - getIscsiHost(), getIscsiPort(), getIscsiIQN(), null, null, SrType); - } + var chapUser = IscsiUseChapCheckBox.Checked ? IScsiChapUserTextBox.Text : null; + var chapPwd = IscsiUseChapCheckBox.Checked ? IScsiChapSecretTextBox.Text : null; + IscsiPopulateLunsAction = SrType == SR.SRTypes.gfs2 + ? new Gfs2PopulateLunsAction(Connection, getIscsiHost(), getIscsiPort(), getIscsiIQN(), chapUser, chapPwd) + : new ISCSIPopulateLunsAction(Connection, getIscsiHost(), getIscsiPort(), getIscsiIQN(), chapUser, chapPwd); + IscsiPopulateLunsAction.Completed += IscsiPopulateLunsAction_Completed; controlDisabler.Reset(); diff --git a/XenModel/Actions/SR/ISCSIPopulateIQNsAction.cs b/XenModel/Actions/SR/ISCSIPopulateIQNsAction.cs index d1187874d..622b3a952 100644 --- a/XenModel/Actions/SR/ISCSIPopulateIQNsAction.cs +++ b/XenModel/Actions/SR/ISCSIPopulateIQNsAction.cs @@ -38,7 +38,6 @@ using XenAdmin.Core; using XenAdmin.Network; using XenAPI; using System.Globalization; -using System.Web.Script.Serialization; using XenCenterLib; namespace XenAdmin.Actions @@ -48,13 +47,12 @@ namespace XenAdmin.Actions /// public class ISCSIPopulateIQNsAction : AsyncAction { - private readonly string targetHost; - private readonly UInt16 targetPort; - private readonly string chapUsername; - private readonly string chapPassword; - private readonly SR.SRTypes srType; + protected readonly string targetHost; + protected readonly UInt16 targetPort; + protected readonly string chapUsername; + protected readonly string chapPassword; - private IScsiIqnInfo[] _iqns; + protected IScsiIqnInfo[] _iqns; /// /// Will be null if the scan has not yet successfully returned. /// @@ -67,14 +65,13 @@ namespace XenAdmin.Actions } public ISCSIPopulateIQNsAction(IXenConnection connection, string targetHost, - UInt16 targetPort, string chapUsername, string chapPassword, SR.SRTypes srType = SR.SRTypes.lvmoiscsi) + UInt16 targetPort, string chapUsername, string chapPassword) : base(connection, string.Format(Messages.ACTION_ISCSI_IQN_SCANNING, targetHost), null, true) { this.targetHost = targetHost; this.targetPort = targetPort; this.chapUsername = chapUsername; this.chapPassword = chapPassword; - this.srType = srType; } public class NoIQNsFoundException : Exception @@ -119,12 +116,6 @@ namespace XenAdmin.Actions if (pool == null) throw new Failure(Failure.INTERNAL_ERROR, Messages.POOL_GONE); - if (srType == SR.SRTypes.gfs2) - { - DoProbeExt(pool.master); - return; - } - Dictionary settings = new Dictionary(); settings["target"] = targetHost; settings["port"] = targetPort.ToString(CultureInfo.InvariantCulture); @@ -140,7 +131,7 @@ namespace XenAdmin.Actions // containing the list of SRs on the filer. RelatedTask = SR.async_create(Session, pool.master, settings, 0, Helpers.GuiTempObjectPrefix, Messages.ISCSI_SHOULD_NO_BE_CREATED, - srType.ToString(), "user", true, new Dictionary()); + SR.SRTypes.lvmoiscsi.ToString(), "user", true, new Dictionary()); this.PollToCompletion(); // Create should always fail and never get here @@ -210,9 +201,20 @@ namespace XenAdmin.Actions throw new NoIQNsFoundException(targetHost); } } + } - private void DoProbeExt(XenRef host) + public class Gfs2PopulateIQNsAction : ISCSIPopulateIQNsAction + { + public Gfs2PopulateIQNsAction(IXenConnection connection, string targetHost, + UInt16 targetPort, string chapUsername, string chapPassword) + : base(connection, targetHost, targetPort, chapUsername, chapPassword) + { } + + protected override void Run() { + Pool pool = Helpers.GetPoolOfOne(Connection); + if (pool == null) + throw new Failure(Failure.INTERNAL_ERROR, Messages.POOL_GONE); var deviceConfig = new Dictionary(); deviceConfig["provider"] = "iscsi"; deviceConfig["ips"] = targetHost; @@ -222,38 +224,31 @@ namespace XenAdmin.Actions deviceConfig["chapuser"] = chapUsername; deviceConfig["chappassword"] = chapPassword; } - - var probeResults = SR.probe_ext(Session, host.opaque_ref, - deviceConfig, srType.ToString().ToLowerInvariant(), new Dictionary()); + + var probeResults = SR.probe_ext(Session, pool.master.opaque_ref, + deviceConfig, SR.SRTypes.gfs2.ToString(), new Dictionary()); var results = new List(); - try + var index = -1; + foreach (var probeResult in probeResults) { - var index = -1; - foreach (var probeResult in probeResults) - { - index++; - var address = probeResult.configuration.ContainsKey("ips") ? probeResult.configuration["ips"] : ""; - UInt16 port; - if (!probeResult.configuration.ContainsKey("port") || !UInt16.TryParse(probeResult.configuration["port"], out port)) - port = Util.DEFAULT_ISCSI_PORT; - var targetIQN = probeResult.configuration.ContainsKey("iqns") ? probeResult.configuration["iqns"] : ""; + index++; + var address = probeResult.configuration.ContainsKey("ips") ? probeResult.configuration["ips"] : ""; + UInt16 port; + if (!probeResult.configuration.ContainsKey("port") || !UInt16.TryParse(probeResult.configuration["port"], out port)) + port = Util.DEFAULT_ISCSI_PORT; + var targetIQN = probeResult.configuration.ContainsKey("iqns") ? probeResult.configuration["iqns"] : ""; - results.Add(new IScsiIqnInfo(index, targetIQN, address, port)); - } - results.Sort(); - _iqns = results.ToArray(); - } - catch - { - throw new BadServerResponse(targetHost); + results.Add(new IScsiIqnInfo(index, targetIQN, address, port)); } + results.Sort(); + _iqns = results.ToArray(); if (_iqns.Length < 1) throw new NoIQNsFoundException(targetHost); } } - + public struct IScsiIqnInfo : IComparable, IEquatable { public readonly int Index; diff --git a/XenModel/Actions/SR/ISCSIPopulateLunsAction.cs b/XenModel/Actions/SR/ISCSIPopulateLunsAction.cs index b9d05074c..5a7b92e3e 100644 --- a/XenModel/Actions/SR/ISCSIPopulateLunsAction.cs +++ b/XenModel/Actions/SR/ISCSIPopulateLunsAction.cs @@ -32,8 +32,6 @@ using System; using System.Collections.Generic; using System.Globalization; -using System.Text; -using System.Threading; using System.Xml; using XenAdmin.Core; @@ -48,14 +46,13 @@ namespace XenAdmin.Actions /// public class ISCSIPopulateLunsAction : AsyncAction { - private readonly string targetHost; - private readonly UInt16 targetPort; - private readonly string targetIQN; - private readonly string chapUsername; - private readonly string chapPassword; - private readonly SR.SRTypes srType; + protected readonly string targetHost; + protected readonly UInt16 targetPort; + protected readonly string targetIQN; + protected readonly string chapUsername; + protected readonly string chapPassword; - private ISCSIInfo[] _luns; + protected ISCSIInfo[] _luns; /// /// Will be null if the scan has not yet successfully returned. /// @@ -68,7 +65,7 @@ namespace XenAdmin.Actions } public ISCSIPopulateLunsAction(IXenConnection connection, string targetHost, - UInt16 targetPort, string targetIQN, string chapUsername, string chapPassword, SR.SRTypes srType = SR.SRTypes.lvmoiscsi) + UInt16 targetPort, string targetIQN, string chapUsername, string chapPassword) : base(connection, string.Format(Messages.ACTION_ISCSI_LUN_SCANNING, targetHost)) { this.targetHost = targetHost; @@ -76,7 +73,6 @@ namespace XenAdmin.Actions this.targetIQN = targetIQN; this.chapUsername = chapUsername; this.chapPassword = chapPassword; - this.srType = srType; } public class NoLUNsFoundException : Exception @@ -103,12 +99,6 @@ namespace XenAdmin.Actions if (pool == null) throw new Failure(Failure.INTERNAL_ERROR, Messages.POOL_GONE); - if (srType == SR.SRTypes.gfs2) - { - DoProbeExt(pool.master); - return; - } - Dictionary settings = new Dictionary(); settings["target"] = targetHost; @@ -123,8 +113,8 @@ namespace XenAdmin.Actions try { RelatedTask = SR.async_create(Session, pool.master, settings, 0, - Helpers.GuiTempObjectPrefix, Messages.ISCSI_SHOULD_NO_BE_CREATED, - srType.ToString(), "user", true, new Dictionary()); + Helpers.GuiTempObjectPrefix, Messages.ISCSI_SHOULD_NO_BE_CREATED, + SR.SRTypes.lvmoiscsi.ToString(), "user", true, new Dictionary()); this.PollToCompletion(); throw new InvalidOperationException(Messages.ISCSI_FAIL); } @@ -196,9 +186,21 @@ namespace XenAdmin.Actions throw new NoLUNsFoundException(targetHost); } } - - private void DoProbeExt(XenRef host) + } + + public class Gfs2PopulateLunsAction : ISCSIPopulateLunsAction + { + public Gfs2PopulateLunsAction(IXenConnection connection, string targetHost, + UInt16 targetPort, string targetIQN, string chapUsername, string chapPassword) + : base(connection, targetHost, targetPort, targetIQN, chapUsername, chapPassword) + { } + + protected override void Run() { + Pool pool = Helpers.GetPoolOfOne(Connection); + if (pool == null) + throw new Failure(Failure.INTERNAL_ERROR, Messages.POOL_GONE); + var deviceConfig = new Dictionary(); deviceConfig["provider"] = "iscsi"; deviceConfig["ips"] = targetHost; @@ -209,35 +211,27 @@ namespace XenAdmin.Actions deviceConfig["chapuser"] = chapUsername; deviceConfig["chappassword"] = chapPassword; } - - var probeResults = SR.probe_ext(Session, host.opaque_ref, - deviceConfig, srType.ToString().ToLowerInvariant(), new Dictionary()); + + var probeResults = SR.probe_ext(Session, pool.master.opaque_ref, + deviceConfig, SR.SRTypes.gfs2.ToString().ToLowerInvariant(), new Dictionary()); var results = new List(); - try + foreach (var probeResult in probeResults) { - foreach (var probeResult in probeResults) - { - int lunid; - if (!probeResult.extra_info.ContainsKey("LUNid") || !int.TryParse(probeResult.extra_info["LUNid"], out lunid)) - lunid = -1; - var vendor = probeResult.extra_info.ContainsKey("vendor") ? probeResult.extra_info["vendor"] : ""; - var serial = probeResult.extra_info.ContainsKey("serial") ? probeResult.extra_info["serial"] : ""; - long size; - if (!probeResult.extra_info.ContainsKey("size") || !long.TryParse(probeResult.extra_info["size"], out size)) - size = -1; - var scsiid = probeResult.configuration.ContainsKey("ScsiId") ? probeResult.configuration["ScsiId"] : ""; + int lunid; + if (!probeResult.extra_info.ContainsKey("LUNid") || !int.TryParse(probeResult.extra_info["LUNid"], out lunid)) + lunid = -1; + var vendor = probeResult.extra_info.ContainsKey("vendor") ? probeResult.extra_info["vendor"] : ""; + var serial = probeResult.extra_info.ContainsKey("serial") ? probeResult.extra_info["serial"] : ""; + long size; + if (!probeResult.extra_info.ContainsKey("size") || !long.TryParse(probeResult.extra_info["size"], out size)) + size = -1; + var scsiid = probeResult.configuration.ContainsKey("ScsiId") ? probeResult.configuration["ScsiId"] : ""; - results.Add(new ISCSIInfo(scsiid, lunid, vendor, serial, size)); - - } - results.Sort(); - _luns = results.ToArray(); - } - catch - { - throw new BadServerResponse(targetHost); + results.Add(new ISCSIInfo(scsiid, lunid, vendor, serial, size)); } + results.Sort(); + _luns = results.ToArray(); if (_luns.Length < 1) throw new NoLUNsFoundException(targetHost);