Kiu/xenadmin
1
0
Fork 0
mirror of https://github.com/xcp-ng/xenadmin.git synced 2025-01-20 15:29:26 +01:00
Code Issues Actions Packages Projects Releases Wiki Activity

Bad application of the code reusability principle in VMDestroyAction.

Browse Source 
Destroying a VM does not disrupt the connection, hence the use of the BestEffort
method is not warranted. After this correction, BestEffort is only used in the
DestroyedBondAction, hence I removed it completely and replaced it with simpler
exception handling in the latter class's Run method.

Signed-off-by: Konstantina Chremmou <konstantina.chremmou@citrix.com>
This commit is contained in:
Konstantina Chremmou 2020-11-30 13:40:16 +00:00
parent 7e6064eeb1
commit 84d0a2b8ae
3 changed files with 71 additions and 60 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

27
XenModel/Actions/AsyncAction.cs
View File

@ -335,33 +335,6 @@ namespace XenAdmin.Actions
}
}
protected static void BestEffort(ref Exception caught, bool expectDisruption, Action func)
{
try
{
func();
}
catch (Exception exn)
{
if (expectDisruption &&
exn is WebException && ((WebException)exn).Status == WebExceptionStatus.KeepAliveFailure) // ignore keep-alive failures if disruption is expected
{
return;
}
log.Error(exn, exn);
if (caught == null)
{
caught = exn;
}
}
}
protected void BestEffort(ref Exception caught, Action func)
{
BestEffort(ref caught, Connection != null && Connection.ExpectDisruption, func);
}
protected void AddCommonAPIMethodsToRoleCheck()
{
ApiMethodsToRoleCheck.AddRange(Role.CommonTaskApiList);

47
XenModel/Actions/Network/DestroyBondAction.cs
View File

@ -160,14 +160,30 @@ namespace XenAdmin.Actions
PercentComplete = 50;
Exception e = null;
var caughtExceptions = new List<Exception>();
int inc = Bonds.Count > 0 ? 40 / Bonds.Count : 0;
foreach (Bond bond in Bonds)
{
Bond bond1 = bond;
BestEffort(ref e, delegate() { RelatedTask = Bond.async_destroy(Session, bond1.opaque_ref);});
try
{
RelatedTask = Bond.async_destroy(Session, bond1.opaque_ref);
}
catch (Exception exn)
{
if (Connection != null && Connection.ExpectDisruption &&
exn is WebException webEx && webEx.Status == WebExceptionStatus.KeepAliveFailure)
{
//ignore
}
else
{
log.Error($"Failed to destroy bond {bond1.opaque_ref}", exn);
caughtExceptions.Add(exn);
}
}
PollToCompletion(PercentComplete, PercentComplete + inc);
}
@ -177,17 +193,30 @@ namespace XenAdmin.Actions
{
string oldNetworkName = Network.Name();
// Destroy the old network
log.DebugFormat("Destroying network {0} ({1})...", oldNetworkName, Network.uuid);
BestEffort(ref e, delegate()
try
{
XenAPI.Network.destroy(Session, Network.opaque_ref);
log.DebugFormat("Network {0} ({1}) destroyed.", oldNetworkName, Network.uuid);
}
catch (Exception exn)
{
if (Connection != null && Connection.ExpectDisruption &&
exn is WebException webEx && webEx.Status == WebExceptionStatus.KeepAliveFailure)
{
XenAPI.Network.destroy(Session, Network.opaque_ref);
log.DebugFormat("Network {0} ({1}) destroyed.", oldNetworkName, Network.uuid);
});
//ignore
}
else
{
log.Error($"Failed to destroy bond {Network.opaque_ref}", exn);
caughtExceptions.Add(exn);
}
}
}
if (e != null)
throw e;
if (caughtExceptions.Count > 0)
throw caughtExceptions[0];
Description = string.Format(Messages.ACTION_DESTROY_BOND_DONE, Name);
}

57
XenModel/Actions/VM/VMDestroyAction.cs
View File

@ -70,23 +70,24 @@ namespace XenAdmin.Actions.VMActions
private static void DestroyVM(Session session, VM vm, List<VBD> deleteDisks, IEnumerable<VM> deleteSnapshots)
{
Exception caught = null;
var caught = new List<Exception>();
foreach (VM snapshot in deleteSnapshots)
{
VM snap = snapshot;
BestEffort(ref caught, session.Connection.ExpectDisruption, () =>
{
if (snap.power_state == vm_power_state.Suspended)
{
XenAPI.VM.hard_shutdown(session, snap.opaque_ref);
}
DestroyVM(session, snap, true);
});
try
{
if (snap.power_state == vm_power_state.Suspended)
VM.hard_shutdown(session, snap.opaque_ref);
DestroyVM(session, snap, true);
}
catch (Exception e)
{
log.Error($"Failed to delete snapshot {snap.opaque_ref}", e);
caught.Add(e);
}
}
List<XenRef<VDI>> vdiRefs = new List<XenRef<VDI>>();
foreach (XenRef<VBD> vbdRef in vm.VBDs)
@ -95,7 +96,6 @@ namespace XenAdmin.Actions.VMActions
if (vbd == null)
continue;
if (deleteDisks.Contains(vbd))
{
if(vbd.Connection.Resolve(vbd.VDI)!=null)
@ -108,25 +108,34 @@ namespace XenAdmin.Actions.VMActions
if (suspendVDI != null)
vdiRefs.Add(vm.suspend_VDI);
XenAPI.VM.destroy(session, vm.opaque_ref);
VM.destroy(session, vm.opaque_ref);
foreach (XenRef<VDI> vdiRef in vdiRefs)
{
XenRef<VDI> vdi = vdiRef;
BestEffort(ref caught, session.Connection.ExpectDisruption, () => XenAPI.VDI.destroy(session, vdi.opaque_ref));
//CA-115249. XenAPI could have already deleted the VDI. Destroy suspended VM and destroy snapshot functions are affected.
var failure = caught as Failure;
if (failure != null && failure.ErrorDescription != null && failure.ErrorDescription.Count > 0 && failure.ErrorDescription[0] == "HANDLE_INVALID")
try
{
log.InfoFormat("VDI:{0} has already been deleted -- ignoring exception.", vdi.opaque_ref);
caught = null;
VDI.destroy(session, vdiRef.opaque_ref);
}
catch (Exception e)
{
//CA-115249. XenAPI could have already deleted the VDI.
//Destroy suspended VM and destroy snapshot functions are affected.
if (e is Failure failure && failure.ErrorDescription != null &&
failure.ErrorDescription.Count > 0 && failure.ErrorDescription[0] == "HANDLE_INVALID")
{
log.InfoFormat($"VDI {vdiRef.opaque_ref} has already been deleted; ignoring API failure.");
}
else
{
log.Error($"Failed to delete VDI {vdiRef.opaque_ref}", e);
caught.Add(e);
}
}
}
if (caught != null)
throw caught;
if (caught.Count > 0)
throw caught[0];
}
}
}