From 4d3064c7333dca56e8dde9973ab43ca4c34e642f Mon Sep 17 00:00:00 2001 From: Gabor Apati-Nagy Date: Fri, 2 Oct 2015 18:14:00 +0100 Subject: [PATCH 1/4] CA-184521: On the Storage page of New VM wizard, the SR selection is ignored Signed-off-by: Gabor Apati-Nagy --- XenAdmin/Wizards/NewVMWizard/Page_Storage.cs | 42 +++++++++++++++----- 1 file changed, 33 insertions(+), 9 deletions(-) diff --git a/XenAdmin/Wizards/NewVMWizard/Page_Storage.cs b/XenAdmin/Wizards/NewVMWizard/Page_Storage.cs index 5191c70dc..982d4eb8a 100644 --- a/XenAdmin/Wizards/NewVMWizard/Page_Storage.cs +++ b/XenAdmin/Wizards/NewVMWizard/Page_Storage.cs @@ -421,7 +421,7 @@ namespace XenAdmin.Wizards.NewVMWizard Disk.virtual_size = long.Parse(diskNode.Attributes["size"].Value); SR sruuid = connection.Cache.Find_By_Uuid(diskNode.Attributes["sr"].Value); - SR sr = GetBeskDiskStorage(Connection, Disk.virtual_size, affinity, sruuid == null ? null : sruuid); + SR sr = GetBeskDiskStorage(Connection, Disk, affinity, sruuid == null ? null : sruuid); Disk.SR = new XenRef(sr != null ? sr.opaque_ref : Helper.NullOpaqueRef); Disk.type = (vdi_type)Enum.Parse(typeof(vdi_type), diskNode.Attributes["type"].Value); Device.userdevice = diskNode.Attributes["device"].Value; @@ -447,7 +447,7 @@ namespace XenAdmin.Wizards.NewVMWizard Connection = connection; Disk.virtual_size = vdi.virtual_size; - SR sr = GetBeskDiskStorage(Connection, Disk.virtual_size, affinity, Connection.Resolve(vdi.SR)); + SR sr = GetBeskDiskStorage(Connection, vdi, affinity, Connection.Resolve(vdi.SR)); Disk.SR = new XenRef(sr != null ? sr.opaque_ref : Helper.NullOpaqueRef); Disk.type = vdi.type; Device.userdevice = vbd.userdevice; @@ -519,16 +519,16 @@ namespace XenAdmin.Wizards.NewVMWizard /// /// returns null if nothing suitable /// - private static SR GetBeskDiskStorage(IXenConnection connection, long diskSize, Host affinity, SR suggestion) + private static SR GetBeskDiskStorage(IXenConnection connection, VDI disk, Host affinity, SR suggestedSR) { // try suggestion - if (suggestion != null && suggestion.FreeSpace > diskSize && suggestion.CanBeSeenFrom(affinity)) - return suggestion; + if (suggestedSR != null && suggestedSR.CanBeSeenFrom(affinity) && IsSufficientFreeSpaceAvailableOnSrForVdi(suggestedSR, disk)) + return suggestedSR; // try default sr - SR def_sr = connection.Resolve(Helpers.GetPoolOfOne(connection).default_SR); - if (def_sr != null && def_sr.FreeSpace > diskSize && def_sr.CanBeSeenFrom(affinity)) - return def_sr; + SR defaultSR = connection.Resolve(Helpers.GetPoolOfOne(connection).default_SR); + if (defaultSR != null && defaultSR.CanBeSeenFrom(affinity) && IsSufficientFreeSpaceAvailableOnSrForVdi(defaultSR, disk)) + return defaultSR; // pick an sr foreach (SR sr in connection.Cache.SRs) @@ -536,13 +536,37 @@ namespace XenAdmin.Wizards.NewVMWizard if (!sr.CanCreateVmOn()) continue; - if (sr.FreeSpace > diskSize && sr.CanBeSeenFrom(affinity)) + if (sr.IsThinProvisioned && sr.CanBeSeenFrom(affinity) && IsSufficientFreeSpaceAvailableOnSrForVdi(sr, disk)) return sr; } // there is nothing return null; } + + private static long GetRequiredSpaceToCreateVdiOnSr(SR sr, VDI disk) + { + if (sr == null) + throw new ArgumentNullException("sr"); + + long initialAllocationVdi = -1; + if (disk.sm_config != null && disk.sm_config.ContainsKey("initial_allocation")) + long.TryParse(disk.sm_config["initial_allocation"], out initialAllocationVdi); + + long initialAllocationSr = -1; + if (sr.IsThinProvisioned && sr.sm_config.ContainsKey("initial_allocation") && sr.sm_config != null) + long.TryParse(sr.sm_config["initial_allocation"], out initialAllocationSr); + + return (initialAllocationSr > -1 || initialAllocationVdi > -1) ? Math.Max(initialAllocationSr, initialAllocationVdi) : disk.virtual_size; + } + + private static bool IsSufficientFreeSpaceAvailableOnSrForVdi(SR sr, VDI disk) + { + if (sr == null) + return false; + + return sr.FreeSpace > GetRequiredSpaceToCreateVdiOnSr(sr, disk); + } } public enum DiskOverCommit : int From 80f855b98180eab0e2dbf6c6ee3b98f7b8dbf1fe Mon Sep 17 00:00:00 2001 From: Gabor Apati-Nagy Date: Tue, 6 Oct 2015 14:04:36 +0100 Subject: [PATCH 2/4] CA-184521: On the Storage page of New VM wizard, the SR selection is ignored Fixed issues where total initial allocation for an SR was not calculated correctly. Also some minor improvements. Signed-off-by: Gabor Apati-Nagy --- XenAdmin/Wizards/NewVMWizard/Page_Storage.cs | 38 +++++++------------- XenModel/Utils/Helpers.cs | 30 ++++++++++++++++ XenModel/XenAPI-Extensions/VDI.cs | 14 +------- 3 files changed, 44 insertions(+), 38 deletions(-) diff --git a/XenAdmin/Wizards/NewVMWizard/Page_Storage.cs b/XenAdmin/Wizards/NewVMWizard/Page_Storage.cs index 982d4eb8a..105e4b265 100644 --- a/XenAdmin/Wizards/NewVMWizard/Page_Storage.cs +++ b/XenAdmin/Wizards/NewVMWizard/Page_Storage.cs @@ -238,10 +238,11 @@ namespace XenAdmin.Wizards.NewVMWizard else totalDiskSize[sr.opaque_ref] = item.Disk.virtual_size; + var initialSpace = Helpers.GetRequiredSpaceToCreateVdiOnSr(sr, item.Disk); if (totalDiskInitialAllocation.ContainsKey(sr.opaque_ref)) - totalDiskInitialAllocation[sr.opaque_ref] += item.Disk.InitialAllocation; + totalDiskInitialAllocation[sr.opaque_ref] += initialSpace; else - totalDiskInitialAllocation[sr.opaque_ref] = item.Disk.InitialAllocation; + totalDiskInitialAllocation[sr.opaque_ref] = initialSpace; } DiskOverCommit overcommitedDisk = DiskOverCommit.None; foreach (DiskGridRowItem item in DisksGridView.Rows) @@ -517,7 +518,9 @@ namespace XenAdmin.Wizards.NewVMWizard } /// - /// returns null if nothing suitable + /// Tries to find the best SR for the given VDI considering the suggestedSR which has priority over other SRs in this check. + /// SuggestedSR, default SR, other SRs are checked. + /// Returns first suitable SR or NULL. /// private static SR GetBeskDiskStorage(IXenConnection connection, VDI disk, Host affinity, SR suggestedSR) { @@ -536,36 +539,21 @@ namespace XenAdmin.Wizards.NewVMWizard if (!sr.CanCreateVmOn()) continue; - if (sr.IsThinProvisioned && sr.CanBeSeenFrom(affinity) && IsSufficientFreeSpaceAvailableOnSrForVdi(sr, disk)) + if (sr.CanBeSeenFrom(affinity) && IsSufficientFreeSpaceAvailableOnSrForVdi(sr, disk)) return sr; } - // there is nothing - return null; + // there has been no suitable SR found + return null; } - private static long GetRequiredSpaceToCreateVdiOnSr(SR sr, VDI disk) - { - if (sr == null) - throw new ArgumentNullException("sr"); - - long initialAllocationVdi = -1; - if (disk.sm_config != null && disk.sm_config.ContainsKey("initial_allocation")) - long.TryParse(disk.sm_config["initial_allocation"], out initialAllocationVdi); - - long initialAllocationSr = -1; - if (sr.IsThinProvisioned && sr.sm_config.ContainsKey("initial_allocation") && sr.sm_config != null) - long.TryParse(sr.sm_config["initial_allocation"], out initialAllocationSr); - - return (initialAllocationSr > -1 || initialAllocationVdi > -1) ? Math.Max(initialAllocationSr, initialAllocationVdi) : disk.virtual_size; - } + /// + /// Checks whether there is enough space available on the SR to accommodate a VDI. + /// private static bool IsSufficientFreeSpaceAvailableOnSrForVdi(SR sr, VDI disk) { - if (sr == null) - return false; - - return sr.FreeSpace > GetRequiredSpaceToCreateVdiOnSr(sr, disk); + return sr != null && !sr.IsFull && sr.FreeSpace > Helpers.GetRequiredSpaceToCreateVdiOnSr(sr, disk); } } diff --git a/XenModel/Utils/Helpers.cs b/XenModel/Utils/Helpers.cs index a7b2f97fa..2a86adeb4 100755 --- a/XenModel/Utils/Helpers.cs +++ b/XenModel/Utils/Helpers.cs @@ -2113,5 +2113,35 @@ namespace XenAdmin.Core var master = GetMaster(connection); return CreamOrGreater(connection) && master != null && master.SuppPacks.Any(suppPack => suppPack.Name.ToLower().StartsWith("xscontainer")); } + + /// + /// This method returns the disk space required (bytes) on the provided SR for the provided VDI. + /// This method also considers thin provisioning. For thin provisioned SRs the provided sm_config in the VDI will be considered first, or it will use the values from the SR's sm_config in case the VDI does not have these set. For fully provisioned SRs the sm_config in the VDI will be ignored. + /// + /// Disk size required in bytes. + public static long GetRequiredSpaceToCreateVdiOnSr(SR sr, VDI vdi) + { + if (sr == null) + throw new ArgumentNullException("sr"); + + if (vdi == null) + throw new ArgumentNullException("vdi"); + + long initialAllocationVdi = -1; + if (vdi.sm_config != null && vdi.sm_config.ContainsKey("initial_allocation")) + long.TryParse(vdi.sm_config["initial_allocation"], out initialAllocationVdi); + + long initialAllocationSr = -1; + if (sr.IsThinProvisioned && sr.sm_config.ContainsKey("initial_allocation") && sr.sm_config != null) + long.TryParse(sr.sm_config["initial_allocation"], out initialAllocationSr); + + if (initialAllocationVdi > -1) + return initialAllocationVdi; + + if (initialAllocationSr > -1) + return initialAllocationSr; + + return vdi.virtual_size; + } } } diff --git a/XenModel/XenAPI-Extensions/VDI.cs b/XenModel/XenAPI-Extensions/VDI.cs index 046b4dac9..f3a953a28 100644 --- a/XenModel/XenAPI-Extensions/VDI.cs +++ b/XenModel/XenAPI-Extensions/VDI.cs @@ -361,18 +361,6 @@ namespace XenAPI NO_RO_IMAGE, // no part of the VDI is read-only => nothing to cache SR_OVERRIDE, // the feature has been explicitly disabled for the VDI's SR } - - public long InitialAllocation - { - get - { - long initialAllocation; - if (sm_config != null && sm_config.ContainsKey("initial_allocation") && long.TryParse(sm_config["initial_allocation"], out initialAllocation)) - { - return initialAllocation; - } - return virtual_size; - } - } + } } From ef02590008fb3c239f10e20467fedc215a8da49d Mon Sep 17 00:00:00 2001 From: Gabor Apati-Nagy Date: Tue, 6 Oct 2015 15:56:36 +0100 Subject: [PATCH 3/4] CA-184521: On the Storage page of New VM wizard, the SR selection is ignored Fixed code after code review --- XenModel/Utils/Helpers.cs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/XenModel/Utils/Helpers.cs b/XenModel/Utils/Helpers.cs index 2a86adeb4..d4f7f8b31 100755 --- a/XenModel/Utils/Helpers.cs +++ b/XenModel/Utils/Helpers.cs @@ -2127,6 +2127,9 @@ namespace XenAdmin.Core if (vdi == null) throw new ArgumentNullException("vdi"); + if (!sr.IsThinProvisioned) + return vdi.virtual_size; + long initialAllocationVdi = -1; if (vdi.sm_config != null && vdi.sm_config.ContainsKey("initial_allocation")) long.TryParse(vdi.sm_config["initial_allocation"], out initialAllocationVdi); From 3abedfe3bdb90c5addc455ac76de9328da56514a Mon Sep 17 00:00:00 2001 From: Gabor Apati-Nagy Date: Tue, 6 Oct 2015 16:01:31 +0100 Subject: [PATCH 4/4] CA-184521: Removed unnecessary code Signed-off-by: Gabor Apati-Nagy --- XenModel/Utils/Helpers.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/XenModel/Utils/Helpers.cs b/XenModel/Utils/Helpers.cs index d4f7f8b31..73a174182 100755 --- a/XenModel/Utils/Helpers.cs +++ b/XenModel/Utils/Helpers.cs @@ -2135,7 +2135,7 @@ namespace XenAdmin.Core long.TryParse(vdi.sm_config["initial_allocation"], out initialAllocationVdi); long initialAllocationSr = -1; - if (sr.IsThinProvisioned && sr.sm_config.ContainsKey("initial_allocation") && sr.sm_config != null) + if (sr.sm_config != null && sr.sm_config.ContainsKey("initial_allocation")) long.TryParse(sr.sm_config["initial_allocation"], out initialAllocationSr); if (initialAllocationVdi > -1)