Github user karuturi commented on the pull request:

    https://github.com/apache/cloudstack/pull/660#issuecomment-128250223
  
    if we dont persist the deviceId at the time of generation, as you said 
multiple calls around the same time will still get the same id and the attach 
would fail (which would bring back the original problem).
    
    which is why I think, its better to persist the deviceId as soon as we 
generate and move the generation of deviceID logic to GlobalLlock. 
    
    something like:
    ```
    -        deviceId = getDeviceId(vmId, deviceId);
    +        GlobalLock lock =GlobalLock.getInternLock("Get-DeviceId-VM-" + 
vmId);
    +        try {
    +            if (lock.lock(120)) {
    +                try {
    +                    deviceId = getDeviceId(vmId, deviceId);
    +                    _volsDao.updateDeviceId(volumeId, vmId, deviceId);
    +                } finally {
    +                    lock.unlock();
    +                }
    +            }
    +        } finally {
    +            lock.releaseRef();
    +        }
    ```
    ```
    -        return attachVolumeToVM(command.getVirtualMachineId(), 
command.getId(), command.getDeviceId());
    +        try {
    +            return attachVolumeToVM(command.getVirtualMachineId(), 
command.getId(), command.getDeviceId());
    +        } catch(Exception e) {
    +            _volsDao.updateDeviceId(command.getId(), null, null);
    +            throw e;
    +        }
    ```


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

Reply via email to