Andr0human commented on code in PR #13700:
URL: https://github.com/apache/cloudstack/pull/13700#discussion_r3709657428
##########
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:
Thanks for taking a look @DaanHoogland . I'd say let's leave it as is. The
wording is copied from the symmetric add path in
`addVmToNetworkThroughJobQueue`, which throws `The number of jobs to add
network %s to vm %s are %d`. I figured keeping both guards reading the same way
was more useful than fixing the grammar on just one of them.
I'm happy to reword both if you'd rather have it cleaned up, just let me
know.
--
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]
