[ 
https://issues.apache.org/jira/browse/CLOUDSTACK-9794?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15876885#comment-15876885
 ] 

ASF GitHub Bot commented on CLOUDSTACK-9794:
--------------------------------------------

Github user HrWiggles commented on a diff in the pull request:

    https://github.com/apache/cloudstack/pull/1953#discussion_r102336557
  
    --- Diff: server/src/com/cloud/storage/VolumeApiServiceImpl.java ---
    @@ -2639,22 +2639,23 @@ private int getMaxDataVolumesSupported(UserVmVO vm) 
{
             return maxDataVolumesSupported.intValue();
         }
     
    -    private Long getDeviceId(long vmId, Long deviceId) {
    +    private Long getDeviceId(UserVmVO vm, Long deviceId) {
             // allocate deviceId
    -        List<VolumeVO> vols = _volsDao.findByInstance(vmId);
    +        int maxDataVolumesSupported = getMaxDataVolumesSupported(vm);
    +        List<VolumeVO> vols = _volsDao.findByInstance(vm.getId());
             if (deviceId != null) {
    -            if (deviceId.longValue() > 15 || deviceId.longValue() == 3) {
    -                throw new RuntimeException("deviceId should be 1,2,4-15");
    +            if (deviceId.longValue() > maxDataVolumesSupported || 
deviceId.longValue() == 3) {
    +                throw new RuntimeException("deviceId should be 1,2,4-" + 
maxDataVolumesSupported);
                 }
                 for (VolumeVO vol : vols) {
                     if (vol.getDeviceId().equals(deviceId)) {
    -                    throw new RuntimeException("deviceId " + deviceId + " 
is used by vm" + vmId);
    +                    throw new RuntimeException("deviceId " + deviceId + " 
is used by vm" + vm.getId());
                     }
                 }
             } else {
                 // allocate deviceId here
                 List<String> devIds = new ArrayList<String>();
    -            for (int i = 1; i < 15; i++) {
    +            for (int i = 1; i < maxDataVolumesSupported; i++) {
    --- End diff --
    
    @sureshanaparti If the condition really should be `i < 
maxDataVolumesSupported` (which would make the maximum device id returned by 
the method be `maxDataVolumesSupported - 1`), then the check + error message 
above
    ```
    if (deviceId.longValue() <= 0 || deviceId.longValue() > 
maxDataVolumesSupported || deviceId.longValue() == 3) {
        throw new RuntimeException("deviceId should be 1,2,4-" + 
maxDataVolumesSupported);
    ```
    need to be changed so as not to include the value of 
`maxDataVolumesSupported` itself.
    Otherwise, when `maxDataVolumesSupported` has value `6` (for example), the 
method would not ever return `6` when parameter `deviceId` is specified as 
`null` but would return `6` when parameter `deviceId` is specified as `6` 
(assuming device id `6` is not already in use).  Also, the error message would 
state "deviceId should be 1,2,4-6" whenever parameter `deviceId` would be 
specified as an invalid value, which would not be correct (as `5` should be the 
highest valid device id).


> Unable to attach more than 14 devices to a VM
> ---------------------------------------------
>
>                 Key: CLOUDSTACK-9794
>                 URL: https://issues.apache.org/jira/browse/CLOUDSTACK-9794
>             Project: CloudStack
>          Issue Type: Bug
>      Security Level: Public(Anyone can view this level - this is the 
> default.) 
>          Components: Volumes
>            Reporter: Suresh Kumar Anaparti
>            Assignee: Suresh Kumar Anaparti
>             Fix For: 4.10
>
>
> A limit of 13 disks is set in hypervisor_capabilities for VMware hypervisor. 
> Changed this limit to a higher value in the DB directly for the VMware and 
> tried attaching more than 14 disks. This was failing with the below exception:
> {noformat}
> 2016-08-12 18:42:53,694 ERROR [c.c.a.ApiAsyncJobDispatcher] 
> (API-Job-Executor-40:ctx-56068c6b job-1015) (logid:b22938fd) Unexpected 
> exception while executing 
> org.apache.cloudstack.api.command.admin.volume.AttachVolumeCmdByAdmin
> java.util.NoSuchElementException
>       at java.util.ArrayList$Itr.next(ArrayList.java:794)
>       at 
> com.cloud.storage.VolumeApiServiceImpl.getDeviceId(VolumeApiServiceImpl.java:2439)
>       at 
> com.cloud.storage.VolumeApiServiceImpl.attachVolumeToVM(VolumeApiServiceImpl.java:1308)
>       at 
> com.cloud.storage.VolumeApiServiceImpl.attachVolumeToVM(VolumeApiServiceImpl.java:1173)
>       at sun.reflect.GeneratedMethodAccessor248.invoke(Unknown Source)
>       at 
> sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
>       at java.lang.reflect.Method.invoke(Method.java:601)
>       at 
> org.springframework.aop.support.AopUtils.invokeJoinpointUsingReflection(AopUtils.java:317)
>       at 
> org.springframework.aop.framework.ReflectiveMethodInvocation.invokeJoinpoint(ReflectiveMethodInvocation.java:183)
>       at 
> org.springframework.aop.framework.ReflectiveMethodInvocation.proceed(ReflectiveMethodInvocation.java:150)
>       at 
> org.apache.cloudstack.network.contrail.management.EventUtils$EventInterceptor.invoke(EventUtils.java:106)
> {noformat}
> There was a hardcoded limit of 15 on the number of devices for a VM.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

Reply via email to