Copilot commented on code in PR #13628:
URL: https://github.com/apache/cloudstack/pull/13628#discussion_r3879133693
##########
engine/storage/datamotion/src/main/java/org/apache/cloudstack/storage/motion/AncientDataMotionStrategy.java:
##########
@@ -556,7 +556,15 @@ private boolean canBypassSecondaryStorage(DataObject
srcData, DataObject destDat
return true;
}
+ if (Hypervisor.HypervisorType.XenServer.equals(((VolumeInfo)
srcData).getHypervisorType())) {
+ return false;
+ }
+
if (destData instanceof VolumeInfo) {
+ if (Hypervisor.HypervisorType.XenServer.equals(((VolumeInfo)
destData).getHypervisorType())) {
+ return false;
+ }
Review Comment:
The new XenServer short-circuit changes behavior of
`canBypassSecondaryStorage` for volume-to-volume copies, but there is no unit
test asserting that XenServer volumes cannot bypass secondary storage. Adding a
focused test (e.g., src/dest VolumeInfo hypervisor type = XenServer with
otherwise-eligible scopes/pool types => expect `false`) would prevent
regressions.
##########
test/integration/smoke/test_deploy_vms_in_parallel.py:
##########
@@ -123,6 +125,40 @@ def update_resource_limit(self, max=1):
)
def tearDown(self):
+ # Deleting the account only soft-deletes it; the account row (and
+ # its "needs cleanup" state) is purged asynchronously by the
+ # account.cleanup.interval background task. Deleting the domain
+ # right after the account can therefore race with that task and
+ # fail with "Can't delete the domain yet because it has N
+ # accounts to cleanup". Delete the account first, then retry the
+ # domain deletion for a bit to ride out that race.
+ try:
+ self.cleanup_resources(self.apiclient, [self.account])
+ except Exception as e:
+ self.debug("Warning: Exception during account cleanup : %s" % e)
+
+ retries_left = 15
+ while True:
+ try:
+ self.domain.delete(self.apiclient)
+ break
+ except Exception as e:
+ retries_left -= 1
+ if "accounts to cleanup" not in str(e):
+ raise Exception("Warning: Exception during cleanup : %s" %
e)
Review Comment:
Reraising a new generic `Exception` here drops the original exception
type/stack trace, which makes this cleanup failure much harder to debug (and
can hide the real root cause in CI). Use exception chaining (`from e`) to
preserve the underlying error context.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
