DaanHoogland commented on code in PR #13700:
URL: https://github.com/apache/cloudstack/pull/13700#discussion_r3709824156
##########
engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java:
##########
@@ -5938,14 +5938,25 @@ public Outcome<VirtualMachine>
removeNicFromVmThroughJobQueue(
final VirtualMachine vm, final Nic nic) {
Long vmId = vm.getId();
String commandName = VmWorkRemoveNicFromVm.class.getName();
- Pair<VmWorkJobVO, Long> pendingWorkJob = retrievePendingWorkJob(vmId,
commandName);
- VmWorkJobVO workJob = pendingWorkJob.first();
+ // The nic uuid must be part of the pending-job lookup key. Without
it, a concurrent request
+ // to remove a different nic from the same vm matches this
still-pending job and joins it
+ // instead of submitting its own, so only one nic is removed while
both callers wait on the
+ // single job and both receive its success. Mirrors the symmetric
addVmToNetworkThroughJobQueue.
+ final List<VmWorkJobVO> pendingWorkJobs =
_workJobDao.listPendingWorkJobs(
+ VirtualMachine.Type.Instance, vmId, commandName,
nic.getUuid());
- if (workJob == null) {
+ VmWorkJobVO workJob;
+ if (pendingWorkJobs != null && pendingWorkJobs.size() > 0) {
+ if (pendingWorkJobs.size() > 1) {
+ throw new CloudRuntimeException(String.format("The number of
jobs to remove nic %s from vm %s are %d", nic.getUuid(), vm.getInstanceName(),
pendingWorkJobs.size()));
Review Comment:
ok, my preference is to never have the exact same text be thrown in more
places. It will induce the need for the stack trace to diagnose (more easily).
about english syntax, I don’t care too much (not *my* language ;)
your choice in the end
--
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]
