skip a queued volume detach when the volume is no longer on that vm

server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java

@@ -3555,10 +3555,28 @@ public Volume detachVolumeViaDestroyVM(long vmId, long volumeId) {
         return orchestrateDetachVolumeFromVM(vmId, volumeId);
     }
 
+    /**
+     * A queued detach can run after the volume was already detached and attached elsewhere.
+     * In that case the detach must be skipped, so the newer attachment is left alone.
+     *
+     * @return true if the volume is no longer attached to the given VM
+     */
+    protected boolean shouldDetachBeSkipped(Volume volume, VMInstanceVO vm) {
+        if (volume.getInstanceId() == null || volume.getInstanceId() != vm.getId()) {
+            logger.warn("Volume {} is no longer attached to VM {}, skipping detach", volume, vm);
+            return true;
+        }
+        return false;
+    }
+
     private Volume orchestrateDetachVolumeFromVM(long vmId, long volumeId) {
         Volume volume = _volsDao.findById(volumeId);
         VMInstanceVO vm = _vmInstanceDao.findById(vmId);
 
+        if (shouldDetachBeSkipped(volume, vm)) {
+            return volume;
+        }
+
         String errorMsg = "Failed to detach volume " + volume.getName() + " from VM " + vm.getHostName();
         boolean sendCommand = vm.getState() == State.Running;
 

server/src/test/java/com/cloud/storage/VolumeApiServiceImplTest.java

@@ -144,6 +144,7 @@
 import com.cloud.vm.UserVmVO;
 import com.cloud.vm.VirtualMachine;
 import com.cloud.vm.VirtualMachine.State;
+import com.cloud.vm.VMInstanceVO;
 import com.cloud.vm.VirtualMachineManager;
 import com.cloud.vm.dao.UserVmDao;
 import com.cloud.vm.dao.VMInstanceDao;
@@ -619,6 +620,50 @@ public void testDetachVolumeFromStoppedXenVm() throws NoSuchFieldException, Ille
         volumeApiServiceImpl.detachVolumeFromVM(detachCmd);
     }
 
+    @Test
+    public void testDetachVolumeSkippedWhenAttachedToAnotherVm() {
+        VolumeVO volume = Mockito.mock(VolumeVO.class);
+        Mockito.when(volume.getInstanceId()).thenReturn(200L);
+        Mockito.when(volumeDaoMock.findById(100L)).thenReturn(volume);
+
+        Volume result = volumeApiServiceImpl.detachVolumeViaDestroyVM(300L, 100L);
+
+        Assert.assertEquals(volume, result);
+        Mockito.verify(volumeDaoMock, Mockito.never()).detachVolume(Mockito.anyLong());
+    }
+
+    @Test
+    public void testDetachVolumeSkippedWhenAlreadyDetached() {
+        VolumeVO volume = Mockito.mock(VolumeVO.class);
+        Mockito.when(volume.getInstanceId()).thenReturn(null);
+        Mockito.when(volumeDaoMock.findById(100L)).thenReturn(volume);
+
+        Volume result = volumeApiServiceImpl.detachVolumeViaDestroyVM(300L, 100L);
+
+        Assert.assertEquals(volume, result);
+        Mockito.verify(volumeDaoMock, Mockito.never()).detachVolume(Mockito.anyLong());
+    }
+
+    @Test
+    public void testShouldDetachBeSkippedFalseWhenAttachedToSameVm() {
+        VolumeVO volume = Mockito.mock(VolumeVO.class);
+        Mockito.when(volume.getInstanceId()).thenReturn(300L);
+        VMInstanceVO vm = Mockito.mock(VMInstanceVO.class);
+        Mockito.when(vm.getId()).thenReturn(300L);
+
+        Assert.assertFalse(volumeApiServiceImpl.shouldDetachBeSkipped(volume, vm));
+    }
+
+    @Test
+    public void testShouldDetachBeSkippedTrueWhenAttachedToAnotherVm() {
+        VolumeVO volume = Mockito.mock(VolumeVO.class);
+        Mockito.when(volume.getInstanceId()).thenReturn(200L);
+        VMInstanceVO vm = Mockito.mock(VMInstanceVO.class);
+        Mockito.when(vm.getId()).thenReturn(300L);
+
+        Assert.assertTrue(volumeApiServiceImpl.shouldDetachBeSkipped(volume, vm));
+    }
+
     /**
      * TESTS FOR ATTACH ROOT VOLUME, COUNT=7
      */