You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cloudstack.apache.org by sureshanaparti <gi...@git.apache.org> on 2017/02/18 20:26:20 UTC

[GitHub] cloudstack pull request #1953: CLOUDSTACK-9794: Unable to attach more than 1...

GitHub user sureshanaparti opened a pull request:

    https://github.com/apache/cloudstack/pull/1953

    CLOUDSTACK-9794: Unable to attach more than 14 devices to a VM

    Updated hardcoded value with max data volumes limit from hypervisor capabilities.

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/Accelerite/cloudstack CLOUDSTACK-9794

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/cloudstack/pull/1953.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #1953
    
----
commit cdf50c1e0f69297beb335f852ae82663bbc87fb4
Author: Suresh Kumar Anaparti <su...@accelerite.com>
Date:   2017-02-18T20:22:30Z

    CLOUDSTACK-9794: Unable to attach more than 14 devices to a VM
    
    Updated hardcoded value with max data volumes limit from hypervisor capabilities.

----


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack issue #1953: CLOUDSTACK-9794: Unable to attach more than 14 devic...

Posted by borisstoyanov <gi...@git.apache.org>.
Github user borisstoyanov commented on the issue:

    https://github.com/apache/cloudstack/pull/1953
  
    @blueorangutan package



---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request #1953: CLOUDSTACK-9794: Unable to attach more than 1...

Posted by sureshanaparti <gi...@git.apache.org>.
Github user sureshanaparti commented on a diff in the pull request:

    https://github.com/apache/cloudstack/pull/1953#discussion_r102303229
  
    --- 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 --
    
    @HrWiggles Thanks for pointing this. Addressed.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack issue #1953: CLOUDSTACK-9794: Unable to attach more than 14 devic...

Posted by blueorangutan <gi...@git.apache.org>.
Github user blueorangutan commented on the issue:

    https://github.com/apache/cloudstack/pull/1953
  
    @borisstoyanov a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack issue #1953: CLOUDSTACK-9794: Unable to attach more than 14 devic...

Posted by borisstoyanov <gi...@git.apache.org>.
Github user borisstoyanov commented on the issue:

    https://github.com/apache/cloudstack/pull/1953
  
    @blueorangutan test


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack issue #1953: CLOUDSTACK-9794: Unable to attach more than 14 devic...

Posted by borisstoyanov <gi...@git.apache.org>.
Github user borisstoyanov commented on the issue:

    https://github.com/apache/cloudstack/pull/1953
  
    @blueorangutan package


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request #1953: CLOUDSTACK-9794: Unable to attach more than 1...

Posted by sureshanaparti <gi...@git.apache.org>.
Github user sureshanaparti commented on a diff in the pull request:

    https://github.com/apache/cloudstack/pull/1953#discussion_r102302052
  
    --- 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) {
    --- End diff --
    
    @HrWiggles Addressed.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack issue #1953: CLOUDSTACK-9794: Unable to attach more than 14 devic...

Posted by sureshanaparti <gi...@git.apache.org>.
Github user sureshanaparti commented on the issue:

    https://github.com/apache/cloudstack/pull/1953
  
    @borisstoyanov Can you kick off Jenkins job on this PR.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request #1953: CLOUDSTACK-9794: Unable to attach more than 1...

Posted by HrWiggles <gi...@git.apache.org>.
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).


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack issue #1953: CLOUDSTACK-9794: Unable to attach more than 14 devic...

Posted by blueorangutan <gi...@git.apache.org>.
Github user blueorangutan commented on the issue:

    https://github.com/apache/cloudstack/pull/1953
  
    <b>Trillian test result (tid-894)</b>
    Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
    Total time taken: 26990 seconds
    Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr1953-t894-kvm-centos7.zip
    Intermitten failure detected: /marvin/tests/smoke/test_privategw_acl.py
    Intermitten failure detected: /marvin/tests/smoke/test_snapshots.py
    Test completed. 47 look ok, 2 have error(s)
    
    
    Test | Result | Time (s) | Test File
    --- | --- | --- | ---
    test_04_rvpc_privategw_static_routes | `Failure` | 320.55 | test_privategw_acl.py
    test_02_list_snapshots_with_removed_data_store | `Error` | 0.04 | test_snapshots.py
    test_01_vpc_site2site_vpn | Success | 144.79 | test_vpc_vpn.py
    test_01_vpc_remote_access_vpn | Success | 56.15 | test_vpc_vpn.py
    test_01_redundant_vpc_site2site_vpn | Success | 215.46 | test_vpc_vpn.py
    test_02_VPC_default_routes | Success | 253.84 | test_vpc_router_nics.py
    test_01_VPC_nics_after_destroy | Success | 480.33 | test_vpc_router_nics.py
    test_05_rvpc_multi_tiers | Success | 481.57 | test_vpc_redundant.py
    test_04_rvpc_network_garbage_collector_nics | Success | 1387.50 | test_vpc_redundant.py
    test_03_create_redundant_VPC_1tier_2VMs_2IPs_2PF_ACL_reboot_routers | Success | 526.87 | test_vpc_redundant.py
    test_02_redundant_VPC_default_routes | Success | 738.73 | test_vpc_redundant.py
    test_01_create_redundant_VPC_2tiers_4VMs_4IPs_4PF_ACL | Success | 1233.63 | test_vpc_redundant.py
    test_09_delete_detached_volume | Success | 156.56 | test_volumes.py
    test_08_resize_volume | Success | 151.63 | test_volumes.py
    test_07_resize_fail | Success | 156.49 | test_volumes.py
    test_06_download_detached_volume | Success | 151.54 | test_volumes.py
    test_05_detach_volume | Success | 150.74 | test_volumes.py
    test_04_delete_attached_volume | Success | 151.24 | test_volumes.py
    test_03_download_attached_volume | Success | 156.38 | test_volumes.py
    test_02_attach_volume | Success | 84.12 | test_volumes.py
    test_01_create_volume | Success | 621.05 | test_volumes.py
    test_03_delete_vm_snapshots | Success | 275.20 | test_vm_snapshots.py
    test_02_revert_vm_snapshots | Success | 100.60 | test_vm_snapshots.py
    test_01_create_vm_snapshots | Success | 163.72 | test_vm_snapshots.py
    test_deploy_vm_multiple | Success | 232.61 | test_vm_life_cycle.py
    test_deploy_vm | Success | 0.03 | test_vm_life_cycle.py
    test_advZoneVirtualRouter | Success | 0.03 | test_vm_life_cycle.py
    test_10_attachAndDetach_iso | Success | 26.54 | test_vm_life_cycle.py
    test_09_expunge_vm | Success | 125.24 | test_vm_life_cycle.py
    test_08_migrate_vm | Success | 35.96 | test_vm_life_cycle.py
    test_07_restore_vm | Success | 0.13 | test_vm_life_cycle.py
    test_06_destroy_vm | Success | 125.85 | test_vm_life_cycle.py
    test_03_reboot_vm | Success | 125.87 | test_vm_life_cycle.py
    test_02_start_vm | Success | 10.18 | test_vm_life_cycle.py
    test_01_stop_vm | Success | 35.30 | test_vm_life_cycle.py
    test_CreateTemplateWithDuplicateName | Success | 70.66 | test_templates.py
    test_08_list_system_templates | Success | 0.03 | test_templates.py
    test_07_list_public_templates | Success | 0.04 | test_templates.py
    test_05_template_permissions | Success | 0.06 | test_templates.py
    test_04_extract_template | Success | 5.13 | test_templates.py
    test_03_delete_template | Success | 5.11 | test_templates.py
    test_02_edit_template | Success | 90.11 | test_templates.py
    test_01_create_template | Success | 30.53 | test_templates.py
    test_10_destroy_cpvm | Success | 161.41 | test_ssvm.py
    test_09_destroy_ssvm | Success | 133.20 | test_ssvm.py
    test_08_reboot_cpvm | Success | 101.34 | test_ssvm.py
    test_07_reboot_ssvm | Success | 133.19 | test_ssvm.py
    test_06_stop_cpvm | Success | 131.45 | test_ssvm.py
    test_05_stop_ssvm | Success | 138.32 | test_ssvm.py
    test_04_cpvm_internals | Success | 0.97 | test_ssvm.py
    test_03_ssvm_internals | Success | 2.93 | test_ssvm.py
    test_02_list_cpvm_vm | Success | 0.14 | test_ssvm.py
    test_01_list_sec_storage_vm | Success | 0.14 | test_ssvm.py
    test_01_snapshot_root_disk | Success | 10.98 | test_snapshots.py
    test_04_change_offering_small | Success | 210.42 | test_service_offerings.py
    test_03_delete_service_offering | Success | 0.06 | test_service_offerings.py
    test_02_edit_service_offering | Success | 0.06 | test_service_offerings.py
    test_01_create_service_offering | Success | 0.12 | test_service_offerings.py
    test_02_sys_template_ready | Success | 0.14 | test_secondary_storage.py
    test_01_sys_vm_start | Success | 0.20 | test_secondary_storage.py
    test_09_reboot_router | Success | 35.31 | test_routers.py
    test_08_start_router | Success | 25.26 | test_routers.py
    test_07_stop_router | Success | 10.17 | test_routers.py
    test_06_router_advanced | Success | 0.06 | test_routers.py
    test_05_router_basic | Success | 0.04 | test_routers.py
    test_04_restart_network_wo_cleanup | Success | 5.58 | test_routers.py
    test_03_restart_network_cleanup | Success | 50.52 | test_routers.py
    test_02_router_internal_adv | Success | 0.85 | test_routers.py
    test_01_router_internal_basic | Success | 0.46 | test_routers.py
    test_router_dns_guestipquery | Success | 73.70 | test_router_dns.py
    test_router_dns_externalipquery | Success | 0.08 | test_router_dns.py
    test_router_dhcphosts | Success | 241.28 | test_router_dhcphosts.py
    test_router_dhcp_opts | Success | 21.51 | test_router_dhcphosts.py
    test_01_updatevolumedetail | Success | 0.08 | test_resource_detail.py
    test_01_reset_vm_on_reboot | Success | 151.06 | test_reset_vm_on_reboot.py
    test_createRegion | Success | 0.04 | test_regions.py
    test_create_pvlan_network | Success | 5.21 | test_pvlan.py
    test_dedicatePublicIpRange | Success | 0.43 | test_public_ip_range.py
    test_03_vpc_privategw_restart_vpc_cleanup | Success | 460.15 | test_privategw_acl.py
    test_02_vpc_privategw_static_routes | Success | 340.17 | test_privategw_acl.py
    test_01_vpc_privategw_acl | Success | 87.25 | test_privategw_acl.py
    test_01_primary_storage_nfs | Success | 35.79 | test_primary_storage.py
    test_createPortablePublicIPRange | Success | 15.21 | test_portable_publicip.py
    test_createPortablePublicIPAcquire | Success | 15.45 | test_portable_publicip.py
    test_isolate_network_password_server | Success | 59.22 | test_password_server.py
    test_UpdateStorageOverProvisioningFactor | Success | 0.13 | test_over_provisioning.py
    test_oobm_zchange_password | Success | 30.73 | test_outofbandmanagement.py
    test_oobm_multiple_mgmt_server_ownership | Success | 16.37 | test_outofbandmanagement.py
    test_oobm_issue_power_status | Success | 10.48 | test_outofbandmanagement.py
    test_oobm_issue_power_soft | Success | 15.36 | test_outofbandmanagement.py
    test_oobm_issue_power_reset | Success | 15.34 | test_outofbandmanagement.py
    test_oobm_issue_power_on | Success | 15.31 | test_outofbandmanagement.py
    test_oobm_issue_power_off | Success | 15.34 | test_outofbandmanagement.py
    test_oobm_issue_power_cycle | Success | 15.33 | test_outofbandmanagement.py
    test_oobm_enabledisable_across_clusterzones | Success | 87.71 | test_outofbandmanagement.py
    test_oobm_enable_feature_valid | Success | 5.16 | test_outofbandmanagement.py
    test_oobm_enable_feature_invalid | Success | 0.09 | test_outofbandmanagement.py
    test_oobm_disable_feature_valid | Success | 5.22 | test_outofbandmanagement.py
    test_oobm_disable_feature_invalid | Success | 0.10 | test_outofbandmanagement.py
    test_oobm_configure_invalid_driver | Success | 0.08 | test_outofbandmanagement.py
    test_oobm_configure_default_driver | Success | 0.07 | test_outofbandmanagement.py
    test_oobm_background_powerstate_sync | Success | 23.45 | test_outofbandmanagement.py
    test_extendPhysicalNetworkVlan | Success | 15.41 | test_non_contigiousvlan.py
    test_01_nic | Success | 399.17 | test_nic.py
    test_releaseIP | Success | 177.69 | test_network.py
    test_reboot_router | Success | 373.52 | test_network.py
    test_public_ip_user_account | Success | 10.26 | test_network.py
    test_public_ip_admin_account | Success | 40.28 | test_network.py
    test_network_rules_acquired_public_ip_3_Load_Balancer_Rule | Success | 66.81 | test_network.py
    test_network_rules_acquired_public_ip_2_nat_rule | Success | 61.61 | test_network.py
    test_network_rules_acquired_public_ip_1_static_nat_rule | Success | 123.73 | test_network.py
    test_delete_account | Success | 267.83 | test_network.py
    test_02_port_fwd_on_non_src_nat | Success | 60.68 | test_network.py
    test_01_port_fwd_on_src_nat | Success | 111.73 | test_network.py
    test_nic_secondaryip_add_remove | Success | 197.68 | test_multipleips_per_nic.py
    login_test_saml_user | Success | 19.31 | test_login.py
    test_assign_and_removal_lb | Success | 133.00 | test_loadbalance.py
    test_02_create_lb_rule_non_nat | Success | 187.37 | test_loadbalance.py
    test_01_create_lb_rule_src_nat | Success | 207.60 | test_loadbalance.py
    test_03_list_snapshots | Success | 0.10 | test_list_ids_parameter.py
    test_02_list_templates | Success | 0.04 | test_list_ids_parameter.py
    test_01_list_volumes | Success | 0.03 | test_list_ids_parameter.py
    test_07_list_default_iso | Success | 0.06 | test_iso.py
    test_05_iso_permissions | Success | 0.06 | test_iso.py
    test_04_extract_Iso | Success | 5.19 | test_iso.py
    test_03_delete_iso | Success | 95.20 | test_iso.py
    test_02_edit_iso | Success | 0.06 | test_iso.py
    test_01_create_iso | Success | 20.99 | test_iso.py
    test_04_rvpc_internallb_haproxy_stats_on_all_interfaces | Success | 173.93 | test_internal_lb.py
    test_03_vpc_internallb_haproxy_stats_on_all_interfaces | Success | 128.95 | test_internal_lb.py
    test_02_internallb_roundrobin_1RVPC_3VM_HTTP_port80 | Success | 499.86 | test_internal_lb.py
    test_01_internallb_roundrobin_1VPC_3VM_HTTP_port80 | Success | 414.24 | test_internal_lb.py
    test_dedicateGuestVlanRange | Success | 10.31 | test_guest_vlan_range.py
    test_UpdateConfigParamWithScope | Success | 0.18 | test_global_settings.py
    test_rolepermission_lifecycle_update | Success | 6.31 | test_dynamicroles.py
    test_rolepermission_lifecycle_list | Success | 6.03 | test_dynamicroles.py
    test_rolepermission_lifecycle_delete | Success | 5.88 | test_dynamicroles.py
    test_rolepermission_lifecycle_create | Success | 5.90 | test_dynamicroles.py
    test_rolepermission_lifecycle_concurrent_updates | Success | 6.12 | test_dynamicroles.py
    test_role_lifecycle_update_role_inuse | Success | 5.92 | test_dynamicroles.py
    test_role_lifecycle_update | Success | 10.99 | test_dynamicroles.py
    test_role_lifecycle_list | Success | 5.88 | test_dynamicroles.py
    test_role_lifecycle_delete | Success | 10.94 | test_dynamicroles.py
    test_role_lifecycle_create | Success | 5.91 | test_dynamicroles.py
    test_role_inuse_deletion | Success | 5.87 | test_dynamicroles.py
    test_role_account_acls_multiple_mgmt_servers | Success | 8.01 | test_dynamicroles.py
    test_role_account_acls | Success | 8.21 | test_dynamicroles.py
    test_default_role_deletion | Success | 5.96 | test_dynamicroles.py
    test_04_create_fat_type_disk_offering | Success | 0.07 | test_disk_offerings.py
    test_03_delete_disk_offering | Success | 0.04 | test_disk_offerings.py
    test_02_edit_disk_offering | Success | 0.05 | test_disk_offerings.py
    test_02_create_sparse_type_disk_offering | Success | 0.07 | test_disk_offerings.py
    test_01_create_disk_offering | Success | 0.10 | test_disk_offerings.py
    test_deployvm_userdispersing | Success | 20.58 | test_deploy_vms_with_varied_deploymentplanners.py
    test_deployvm_userconcentrated | Success | 20.73 | test_deploy_vms_with_varied_deploymentplanners.py
    test_deployvm_firstfit | Success | 65.80 | test_deploy_vms_with_varied_deploymentplanners.py
    test_deployvm_userdata_post | Success | 5.38 | test_deploy_vm_with_userdata.py
    test_deployvm_userdata | Success | 45.72 | test_deploy_vm_with_userdata.py
    test_02_deploy_vm_root_resize | Success | 5.97 | test_deploy_vm_root_resize.py
    test_01_deploy_vm_root_resize | Success | 6.00 | test_deploy_vm_root_resize.py
    test_00_deploy_vm_root_resize | Success | 202.56 | test_deploy_vm_root_resize.py
    test_deploy_vm_from_iso | Success | 202.43 | test_deploy_vm_iso.py
    test_DeployVmAntiAffinityGroup | Success | 56.05 | test_affinity_groups.py
    test_change_service_offering_for_vm_with_snapshots | Skipped | 0.00 | test_vm_snapshots.py
    test_01_test_vm_volume_snapshot | Skipped | 0.00 | test_vm_snapshots.py
    test_06_copy_template | Skipped | 0.00 | test_templates.py
    test_static_role_account_acls | Skipped | 0.02 | test_staticroles.py
    test_11_ss_nfs_version_on_ssvm | Skipped | 0.02 | test_ssvm.py
    test_01_scale_vm | Skipped | 0.00 | test_scale_vm.py
    test_01_primary_storage_iscsi | Skipped | 0.04 | test_primary_storage.py
    test_nested_virtualization_vmware | Skipped | 0.00 | test_nested_virtualization.py
    test_06_copy_iso | Skipped | 0.00 | test_iso.py
    test_deploy_vgpu_enabled_vm | Skipped | 0.03 | test_deploy_vgpu_enabled_vm.py
    test_3d_gpu_support | Skipped | 0.04 | test_deploy_vgpu_enabled_vm.py



---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack issue #1953: CLOUDSTACK-9794: Unable to attach more than 14 devic...

Posted by koushik-das <gi...@git.apache.org>.
Github user koushik-das commented on the issue:

    https://github.com/apache/cloudstack/pull/1953
  
    @karuturi This PR is not correct and is resulting in travis CI failures. It needs to be properly fixed.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request #1953: CLOUDSTACK-9794: Unable to attach more than 1...

Posted by sureshanaparti <gi...@git.apache.org>.
Github user sureshanaparti commented on a diff in the pull request:

    https://github.com/apache/cloudstack/pull/1953#discussion_r102311697
  
    --- Diff: plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtVMDef.java ---
    @@ -716,11 +734,6 @@ public DiskFmtType getDiskFormatType() {
                 return _diskFmtType;
             }
     
    --- End diff --
    
    Removed unused method _getDiskSeq()_.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack issue #1953: CLOUDSTACK-9794: Unable to attach more than 14 devic...

Posted by blueorangutan <gi...@git.apache.org>.
Github user blueorangutan commented on the issue:

    https://github.com/apache/cloudstack/pull/1953
  
    @borisstoyanov a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack issue #1953: CLOUDSTACK-9794: Unable to attach more than 14 devic...

Posted by blueorangutan <gi...@git.apache.org>.
Github user blueorangutan commented on the issue:

    https://github.com/apache/cloudstack/pull/1953
  
    Packaging result: \u2714centos6 \u2714centos7 \u2714debian. JID-535


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack issue #1953: CLOUDSTACK-9794: Unable to attach more than 14 devic...

Posted by sureshanaparti <gi...@git.apache.org>.
Github user sureshanaparti commented on the issue:

    https://github.com/apache/cloudstack/pull/1953
  
    @remibergsma @borisstoyanov Updated the KVM code to generate the valid device name above id 25.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack issue #1953: CLOUDSTACK-9794: Unable to attach more than 14 devic...

Posted by blueorangutan <gi...@git.apache.org>.
Github user blueorangutan commented on the issue:

    https://github.com/apache/cloudstack/pull/1953
  
    <b>Trillian test result (tid-877)</b>
    Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
    Total time taken: 26001 seconds
    Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr1953-t877-kvm-centos7.zip
    Intermitten failure detected: /marvin/tests/smoke/test_privategw_acl.py
    Intermitten failure detected: /marvin/tests/smoke/test_snapshots.py
    Test completed. 47 look ok, 2 have error(s)
    
    
    Test | Result | Time (s) | Test File
    --- | --- | --- | ---
    test_04_rvpc_privategw_static_routes | `Failure` | 303.85 | test_privategw_acl.py
    test_02_list_snapshots_with_removed_data_store | `Error` | 0.03 | test_snapshots.py
    test_01_vpc_site2site_vpn | Success | 134.10 | test_vpc_vpn.py
    test_01_vpc_remote_access_vpn | Success | 55.78 | test_vpc_vpn.py
    test_01_redundant_vpc_site2site_vpn | Success | 210.11 | test_vpc_vpn.py
    test_02_VPC_default_routes | Success | 242.58 | test_vpc_router_nics.py
    test_01_VPC_nics_after_destroy | Success | 466.50 | test_vpc_router_nics.py
    test_05_rvpc_multi_tiers | Success | 479.09 | test_vpc_redundant.py
    test_04_rvpc_network_garbage_collector_nics | Success | 1366.03 | test_vpc_redundant.py
    test_03_create_redundant_VPC_1tier_2VMs_2IPs_2PF_ACL_reboot_routers | Success | 521.09 | test_vpc_redundant.py
    test_02_redundant_VPC_default_routes | Success | 728.22 | test_vpc_redundant.py
    test_01_create_redundant_VPC_2tiers_4VMs_4IPs_4PF_ACL | Success | 1246.26 | test_vpc_redundant.py
    test_09_delete_detached_volume | Success | 156.10 | test_volumes.py
    test_08_resize_volume | Success | 156.06 | test_volumes.py
    test_07_resize_fail | Success | 156.10 | test_volumes.py
    test_06_download_detached_volume | Success | 155.99 | test_volumes.py
    test_05_detach_volume | Success | 150.56 | test_volumes.py
    test_04_delete_attached_volume | Success | 145.93 | test_volumes.py
    test_03_download_attached_volume | Success | 155.98 | test_volumes.py
    test_02_attach_volume | Success | 83.88 | test_volumes.py
    test_01_create_volume | Success | 620.53 | test_volumes.py
    test_03_delete_vm_snapshots | Success | 275.19 | test_vm_snapshots.py
    test_02_revert_vm_snapshots | Success | 95.62 | test_vm_snapshots.py
    test_01_create_vm_snapshots | Success | 128.64 | test_vm_snapshots.py
    test_deploy_vm_multiple | Success | 252.01 | test_vm_life_cycle.py
    test_deploy_vm | Success | 0.02 | test_vm_life_cycle.py
    test_advZoneVirtualRouter | Success | 0.02 | test_vm_life_cycle.py
    test_10_attachAndDetach_iso | Success | 26.37 | test_vm_life_cycle.py
    test_09_expunge_vm | Success | 125.17 | test_vm_life_cycle.py
    test_08_migrate_vm | Success | 35.63 | test_vm_life_cycle.py
    test_07_restore_vm | Success | 0.08 | test_vm_life_cycle.py
    test_06_destroy_vm | Success | 130.68 | test_vm_life_cycle.py
    test_03_reboot_vm | Success | 125.64 | test_vm_life_cycle.py
    test_02_start_vm | Success | 5.10 | test_vm_life_cycle.py
    test_01_stop_vm | Success | 35.23 | test_vm_life_cycle.py
    test_CreateTemplateWithDuplicateName | Success | 40.36 | test_templates.py
    test_08_list_system_templates | Success | 0.03 | test_templates.py
    test_07_list_public_templates | Success | 0.03 | test_templates.py
    test_05_template_permissions | Success | 0.04 | test_templates.py
    test_04_extract_template | Success | 5.10 | test_templates.py
    test_03_delete_template | Success | 5.08 | test_templates.py
    test_02_edit_template | Success | 90.07 | test_templates.py
    test_01_create_template | Success | 25.25 | test_templates.py
    test_10_destroy_cpvm | Success | 161.28 | test_ssvm.py
    test_09_destroy_ssvm | Success | 133.21 | test_ssvm.py
    test_08_reboot_cpvm | Success | 101.19 | test_ssvm.py
    test_07_reboot_ssvm | Success | 102.98 | test_ssvm.py
    test_06_stop_cpvm | Success | 131.35 | test_ssvm.py
    test_05_stop_ssvm | Success | 133.05 | test_ssvm.py
    test_04_cpvm_internals | Success | 0.98 | test_ssvm.py
    test_03_ssvm_internals | Success | 2.85 | test_ssvm.py
    test_02_list_cpvm_vm | Success | 0.09 | test_ssvm.py
    test_01_list_sec_storage_vm | Success | 0.10 | test_ssvm.py
    test_01_snapshot_root_disk | Success | 10.83 | test_snapshots.py
    test_04_change_offering_small | Success | 204.33 | test_service_offerings.py
    test_03_delete_service_offering | Success | 0.03 | test_service_offerings.py
    test_02_edit_service_offering | Success | 0.04 | test_service_offerings.py
    test_01_create_service_offering | Success | 0.08 | test_service_offerings.py
    test_02_sys_template_ready | Success | 0.09 | test_secondary_storage.py
    test_01_sys_vm_start | Success | 0.13 | test_secondary_storage.py
    test_09_reboot_router | Success | 35.23 | test_routers.py
    test_08_start_router | Success | 25.19 | test_routers.py
    test_07_stop_router | Success | 10.12 | test_routers.py
    test_06_router_advanced | Success | 0.04 | test_routers.py
    test_05_router_basic | Success | 0.03 | test_routers.py
    test_04_restart_network_wo_cleanup | Success | 5.52 | test_routers.py
    test_03_restart_network_cleanup | Success | 55.36 | test_routers.py
    test_02_router_internal_adv | Success | 0.83 | test_routers.py
    test_01_router_internal_basic | Success | 0.48 | test_routers.py
    test_router_dns_guestipquery | Success | 74.57 | test_router_dns.py
    test_router_dns_externalipquery | Success | 0.06 | test_router_dns.py
    test_router_dhcphosts | Success | 241.66 | test_router_dhcphosts.py
    test_router_dhcp_opts | Success | 21.31 | test_router_dhcphosts.py
    test_01_updatevolumedetail | Success | 0.05 | test_resource_detail.py
    test_01_reset_vm_on_reboot | Success | 130.70 | test_reset_vm_on_reboot.py
    test_createRegion | Success | 0.03 | test_regions.py
    test_create_pvlan_network | Success | 5.22 | test_pvlan.py
    test_dedicatePublicIpRange | Success | 0.30 | test_public_ip_range.py
    test_03_vpc_privategw_restart_vpc_cleanup | Success | 423.11 | test_privategw_acl.py
    test_02_vpc_privategw_static_routes | Success | 348.47 | test_privategw_acl.py
    test_01_vpc_privategw_acl | Success | 71.55 | test_privategw_acl.py
    test_01_primary_storage_nfs | Success | 35.66 | test_primary_storage.py
    test_createPortablePublicIPRange | Success | 15.15 | test_portable_publicip.py
    test_createPortablePublicIPAcquire | Success | 15.33 | test_portable_publicip.py
    test_isolate_network_password_server | Success | 55.76 | test_password_server.py
    test_UpdateStorageOverProvisioningFactor | Success | 0.10 | test_over_provisioning.py
    test_oobm_zchange_password | Success | 30.51 | test_outofbandmanagement.py
    test_oobm_multiple_mgmt_server_ownership | Success | 16.27 | test_outofbandmanagement.py
    test_oobm_issue_power_status | Success | 10.19 | test_outofbandmanagement.py
    test_oobm_issue_power_soft | Success | 15.25 | test_outofbandmanagement.py
    test_oobm_issue_power_reset | Success | 10.25 | test_outofbandmanagement.py
    test_oobm_issue_power_on | Success | 15.24 | test_outofbandmanagement.py
    test_oobm_issue_power_off | Success | 10.23 | test_outofbandmanagement.py
    test_oobm_issue_power_cycle | Success | 15.26 | test_outofbandmanagement.py
    test_oobm_enabledisable_across_clusterzones | Success | 92.27 | test_outofbandmanagement.py
    test_oobm_enable_feature_valid | Success | 5.13 | test_outofbandmanagement.py
    test_oobm_enable_feature_invalid | Success | 0.07 | test_outofbandmanagement.py
    test_oobm_disable_feature_valid | Success | 0.10 | test_outofbandmanagement.py
    test_oobm_disable_feature_invalid | Success | 0.07 | test_outofbandmanagement.py
    test_oobm_configure_invalid_driver | Success | 0.05 | test_outofbandmanagement.py
    test_oobm_configure_default_driver | Success | 0.06 | test_outofbandmanagement.py
    test_oobm_background_powerstate_sync | Success | 23.30 | test_outofbandmanagement.py
    test_extendPhysicalNetworkVlan | Success | 15.25 | test_non_contigiousvlan.py
    test_01_nic | Success | 398.40 | test_nic.py
    test_releaseIP | Success | 141.63 | test_network.py
    test_reboot_router | Success | 372.51 | test_network.py
    test_public_ip_user_account | Success | 10.21 | test_network.py
    test_public_ip_admin_account | Success | 40.22 | test_network.py
    test_network_rules_acquired_public_ip_3_Load_Balancer_Rule | Success | 66.45 | test_network.py
    test_network_rules_acquired_public_ip_2_nat_rule | Success | 61.34 | test_network.py
    test_network_rules_acquired_public_ip_1_static_nat_rule | Success | 120.46 | test_network.py
    test_delete_account | Success | 257.03 | test_network.py
    test_02_port_fwd_on_non_src_nat | Success | 55.47 | test_network.py
    test_01_port_fwd_on_src_nat | Success | 109.59 | test_network.py
    test_nic_secondaryip_add_remove | Success | 206.88 | test_multipleips_per_nic.py
    login_test_saml_user | Success | 17.85 | test_login.py
    test_assign_and_removal_lb | Success | 132.88 | test_loadbalance.py
    test_02_create_lb_rule_non_nat | Success | 186.82 | test_loadbalance.py
    test_01_create_lb_rule_src_nat | Success | 187.33 | test_loadbalance.py
    test_03_list_snapshots | Success | 0.04 | test_list_ids_parameter.py
    test_02_list_templates | Success | 0.03 | test_list_ids_parameter.py
    test_01_list_volumes | Success | 0.02 | test_list_ids_parameter.py
    test_07_list_default_iso | Success | 0.04 | test_iso.py
    test_05_iso_permissions | Success | 0.04 | test_iso.py
    test_04_extract_Iso | Success | 5.11 | test_iso.py
    test_03_delete_iso | Success | 95.15 | test_iso.py
    test_02_edit_iso | Success | 0.05 | test_iso.py
    test_01_create_iso | Success | 20.70 | test_iso.py
    test_04_rvpc_internallb_haproxy_stats_on_all_interfaces | Success | 172.31 | test_internal_lb.py
    test_03_vpc_internallb_haproxy_stats_on_all_interfaces | Success | 122.20 | test_internal_lb.py
    test_02_internallb_roundrobin_1RVPC_3VM_HTTP_port80 | Success | 478.43 | test_internal_lb.py
    test_01_internallb_roundrobin_1VPC_3VM_HTTP_port80 | Success | 402.29 | test_internal_lb.py
    test_dedicateGuestVlanRange | Success | 10.19 | test_guest_vlan_range.py
    test_UpdateConfigParamWithScope | Success | 0.11 | test_global_settings.py
    test_rolepermission_lifecycle_update | Success | 5.80 | test_dynamicroles.py
    test_rolepermission_lifecycle_list | Success | 5.68 | test_dynamicroles.py
    test_rolepermission_lifecycle_delete | Success | 5.58 | test_dynamicroles.py
    test_rolepermission_lifecycle_create | Success | 5.61 | test_dynamicroles.py
    test_rolepermission_lifecycle_concurrent_updates | Success | 5.70 | test_dynamicroles.py
    test_role_lifecycle_update_role_inuse | Success | 5.63 | test_dynamicroles.py
    test_role_lifecycle_update | Success | 10.70 | test_dynamicroles.py
    test_role_lifecycle_list | Success | 5.61 | test_dynamicroles.py
    test_role_lifecycle_delete | Success | 5.63 | test_dynamicroles.py
    test_role_lifecycle_create | Success | 5.61 | test_dynamicroles.py
    test_role_inuse_deletion | Success | 5.76 | test_dynamicroles.py
    test_role_account_acls_multiple_mgmt_servers | Success | 6.95 | test_dynamicroles.py
    test_role_account_acls | Success | 7.00 | test_dynamicroles.py
    test_default_role_deletion | Success | 5.66 | test_dynamicroles.py
    test_04_create_fat_type_disk_offering | Success | 0.05 | test_disk_offerings.py
    test_03_delete_disk_offering | Success | 0.03 | test_disk_offerings.py
    test_02_edit_disk_offering | Success | 0.03 | test_disk_offerings.py
    test_02_create_sparse_type_disk_offering | Success | 0.05 | test_disk_offerings.py
    test_01_create_disk_offering | Success | 0.07 | test_disk_offerings.py
    test_deployvm_userdispersing | Success | 15.39 | test_deploy_vms_with_varied_deploymentplanners.py
    test_deployvm_userconcentrated | Success | 45.58 | test_deploy_vms_with_varied_deploymentplanners.py
    test_deployvm_firstfit | Success | 55.48 | test_deploy_vms_with_varied_deploymentplanners.py
    test_deployvm_userdata_post | Success | 5.27 | test_deploy_vm_with_userdata.py
    test_deployvm_userdata | Success | 40.48 | test_deploy_vm_with_userdata.py
    test_02_deploy_vm_root_resize | Success | 5.65 | test_deploy_vm_root_resize.py
    test_01_deploy_vm_root_resize | Success | 5.67 | test_deploy_vm_root_resize.py
    test_00_deploy_vm_root_resize | Success | 186.66 | test_deploy_vm_root_resize.py
    test_deploy_vm_from_iso | Success | 196.76 | test_deploy_vm_iso.py
    test_DeployVmAntiAffinityGroup | Success | 50.63 | test_affinity_groups.py
    test_change_service_offering_for_vm_with_snapshots | Skipped | 0.00 | test_vm_snapshots.py
    test_01_test_vm_volume_snapshot | Skipped | 0.00 | test_vm_snapshots.py
    test_06_copy_template | Skipped | 0.00 | test_templates.py
    test_static_role_account_acls | Skipped | 0.02 | test_staticroles.py
    test_11_ss_nfs_version_on_ssvm | Skipped | 0.02 | test_ssvm.py
    test_01_scale_vm | Skipped | 0.00 | test_scale_vm.py
    test_01_primary_storage_iscsi | Skipped | 0.03 | test_primary_storage.py
    test_nested_virtualization_vmware | Skipped | 0.00 | test_nested_virtualization.py
    test_06_copy_iso | Skipped | 0.00 | test_iso.py
    test_deploy_vgpu_enabled_vm | Skipped | 0.02 | test_deploy_vgpu_enabled_vm.py
    test_3d_gpu_support | Skipped | 0.03 | test_deploy_vgpu_enabled_vm.py



---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request #1953: CLOUDSTACK-9794: Unable to attach more than 1...

Posted by rafaelweingartner <gi...@git.apache.org>.
Github user rafaelweingartner commented on a diff in the pull request:

    https://github.com/apache/cloudstack/pull/1953#discussion_r102291066
  
    --- 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) {
    --- End diff --
    
    big \U0001f44d for this request :)


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack issue #1953: CLOUDSTACK-9794: Unable to attach more than 14 devic...

Posted by blueorangutan <gi...@git.apache.org>.
Github user blueorangutan commented on the issue:

    https://github.com/apache/cloudstack/pull/1953
  
    @borisstoyanov a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack issue #1953: CLOUDSTACK-9794: Unable to attach more than 14 devic...

Posted by sureshanaparti <gi...@git.apache.org>.
Github user sureshanaparti commented on the issue:

    https://github.com/apache/cloudstack/pull/1953
  
    Changes in the PR #2019.
    
    Maximum data volumes supported field, in the _hypervisor_capabilities_ table, should be the attachable data volumes to the VM. This excludes the ROOT disk and CD-ROM. So, the hypervisor has to support max devices = maximum data volumes supported + 2 (ROOT and CD-ROM).
    
    0(zero) index is considered in `getDeviceId()` for ROOT disk as the ROOT disk attach cmd uses this method. This has to be refactored.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack issue #1953: CLOUDSTACK-9794: Unable to attach more than 14 devic...

Posted by sureshanaparti <gi...@git.apache.org>.
Github user sureshanaparti commented on the issue:

    https://github.com/apache/cloudstack/pull/1953
  
    @remibergsma The KVM support for max data volumes 32 was updated in the DB as part of CLOUDSTACK-8992 (PR #980). CloudStack supports till device Id 15, without this changes. Might be the limit 32 was tested manually in the VM.  As you pointed, in KVM, I see there is a problem with the device name after device id 25. As soon as the device name ends with suffix 'z' for device id 25, it should take the next suffix 'aa', 'ab',...so on. I think it is better to improve the code to generate a valid device name suffix with the given device id. As this is specific to KVM, I'll create a separate PR with these changes. Thanks.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack issue #1953: CLOUDSTACK-9794: Unable to attach more than 14 devic...

Posted by remibergsma <gi...@git.apache.org>.
Github user remibergsma commented on the issue:

    https://github.com/apache/cloudstack/pull/1953
  
    Thanks for the fix! 
    
    Although I think that after id=25 stuff breaks on KVM, as the code generates a deviceName that is invalid. Id 25 results in vdz, after that it continues with caracters like `{` etc. The default for KVM seems to be 32 (as defined here: https://github.com/apache/cloudstack/blob/87ef8137534fa798101f65c6691fcf71513ac978/setup/db/db/schema-452to460.sql#L422).
    
    See: https://github.com/apache/cloudstack/blob/master/plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtVMDef.java#L591
    
    So we either limit it to 25, or improve the code to generate valid device names above id 25. What do you think?


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request #1953: CLOUDSTACK-9794: Unable to attach more than 1...

Posted by HrWiggles <gi...@git.apache.org>.
Github user HrWiggles commented on a diff in the pull request:

    https://github.com/apache/cloudstack/pull/1953#discussion_r102181285
  
    --- 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++) {
                     devIds.add(String.valueOf(i));
                 }
                 devIds.remove("3");
    --- End diff --
    
    Not part of your changes but... there's a possible `NoSuchElementException` below, for line:
    ```
    deviceId = Long.parseLong(devIds.iterator().next());
    ```
    A check should be added for whether `devIds` is empty and, if so, throw a `RuntimeException` with an error message which specifies that all possible device ids are already in use by the vm (if only to be consistent with the rest of the method).


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack issue #1953: CLOUDSTACK-9794: Unable to attach more than 14 devic...

Posted by borisstoyanov <gi...@git.apache.org>.
Github user borisstoyanov commented on the issue:

    https://github.com/apache/cloudstack/pull/1953
  
    @blueorangutan package


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack issue #1953: CLOUDSTACK-9794: Unable to attach more than 14 devic...

Posted by blueorangutan <gi...@git.apache.org>.
Github user blueorangutan commented on the issue:

    https://github.com/apache/cloudstack/pull/1953
  
    Packaging result: \u2714centos6 \u2714centos7 \u2714debian. JID-507


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack issue #1953: CLOUDSTACK-9794: Unable to attach more than 14 devic...

Posted by sureshanaparti <gi...@git.apache.org>.
Github user sureshanaparti commented on the issue:

    https://github.com/apache/cloudstack/pull/1953
  
    @karuturi I'm working on the changes.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request #1953: CLOUDSTACK-9794: Unable to attach more than 1...

Posted by HrWiggles <gi...@git.apache.org>.
Github user HrWiggles commented on a diff in the pull request:

    https://github.com/apache/cloudstack/pull/1953#discussion_r102342529
  
    --- Diff: plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtVMDef.java ---
    @@ -584,18 +584,36 @@ public void defFileBasedDisk(String filePath, String diskLabel, DiskBus bus, Dis
     
             /* skip iso label */
             private String getDevLabel(int devId, DiskBus bus) {
    --- End diff --
    
    Would be great to have unit tests for either `getDevLabel(int devId, DiskBus bus)` or `getDevLabelSuffix(int deviceIndex)`, especially to test for the expected results when `devId` (or `deviceIndex`) are high enough to return a double-letter device label suffix.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request #1953: CLOUDSTACK-9794: Unable to attach more than 1...

Posted by sureshanaparti <gi...@git.apache.org>.
Github user sureshanaparti commented on a diff in the pull request:

    https://github.com/apache/cloudstack/pull/1953#discussion_r102539356
  
    --- 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 --
    
    @HrWiggles Correct. I've updated the code. Thanks.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack issue #1953: CLOUDSTACK-9794: Unable to attach more than 14 devic...

Posted by remibergsma <gi...@git.apache.org>.
Github user remibergsma commented on the issue:

    https://github.com/apache/cloudstack/pull/1953
  
    Tested it, works as expected! LGTM :+1:


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack issue #1953: CLOUDSTACK-9794: Unable to attach more than 14 devic...

Posted by blueorangutan <gi...@git.apache.org>.
Github user blueorangutan commented on the issue:

    https://github.com/apache/cloudstack/pull/1953
  
    @borisstoyanov a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request #1953: CLOUDSTACK-9794: Unable to attach more than 1...

Posted by rafaelweingartner <gi...@git.apache.org>.
Github user rafaelweingartner commented on a diff in the pull request:

    https://github.com/apache/cloudstack/pull/1953#discussion_r102294407
  
    --- 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);
    --- End diff --
    
    I think this is a good question. I would add that the ID `0` is reserved for the root device.
    
    So, I add the question if the `maxDataVolumesSupported` already accounts for the one already reserved for the root disk. 
    
    For instance, when configuring `getMaxDataVolumesSupported(vm)` with `6` for the hypervisor of the VM. Does that mean that the VM can have up to 7 devices? 1 root (id `0`), 1 CD-ROM (id `3`) and other 5 for extra disks/volumes.



---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack issue #1953: CLOUDSTACK-9794: Unable to attach more than 14 devic...

Posted by blueorangutan <gi...@git.apache.org>.
Github user blueorangutan commented on the issue:

    https://github.com/apache/cloudstack/pull/1953
  
    <b>Trillian test result (tid-855)</b>
    Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
    Total time taken: 30017 seconds
    Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr1953-t855-kvm-centos7.zip
    Intermitten failure detected: /marvin/tests/smoke/test_privategw_acl.py
    Intermitten failure detected: /marvin/tests/smoke/test_snapshots.py
    Test completed. 47 look ok, 2 have error(s)
    
    
    Test | Result | Time (s) | Test File
    --- | --- | --- | ---
    test_04_rvpc_privategw_static_routes | `Failure` | 325.73 | test_privategw_acl.py
    test_02_list_snapshots_with_removed_data_store | `Error` | 0.04 | test_snapshots.py
    test_01_vpc_site2site_vpn | Success | 165.60 | test_vpc_vpn.py
    test_01_vpc_remote_access_vpn | Success | 66.17 | test_vpc_vpn.py
    test_01_redundant_vpc_site2site_vpn | Success | 260.79 | test_vpc_vpn.py
    test_02_VPC_default_routes | Success | 277.42 | test_vpc_router_nics.py
    test_01_VPC_nics_after_destroy | Success | 565.35 | test_vpc_router_nics.py
    test_05_rvpc_multi_tiers | Success | 517.19 | test_vpc_redundant.py
    test_04_rvpc_network_garbage_collector_nics | Success | 1418.42 | test_vpc_redundant.py
    test_03_create_redundant_VPC_1tier_2VMs_2IPs_2PF_ACL_reboot_routers | Success | 563.39 | test_vpc_redundant.py
    test_02_redundant_VPC_default_routes | Success | 765.56 | test_vpc_redundant.py
    test_01_create_redundant_VPC_2tiers_4VMs_4IPs_4PF_ACL | Success | 1313.50 | test_vpc_redundant.py
    test_09_delete_detached_volume | Success | 156.82 | test_volumes.py
    test_08_resize_volume | Success | 156.52 | test_volumes.py
    test_07_resize_fail | Success | 161.54 | test_volumes.py
    test_06_download_detached_volume | Success | 156.37 | test_volumes.py
    test_05_detach_volume | Success | 155.90 | test_volumes.py
    test_04_delete_attached_volume | Success | 151.64 | test_volumes.py
    test_03_download_attached_volume | Success | 156.37 | test_volumes.py
    test_02_attach_volume | Success | 129.32 | test_volumes.py
    test_01_create_volume | Success | 711.37 | test_volumes.py
    test_03_delete_vm_snapshots | Success | 275.15 | test_vm_snapshots.py
    test_02_revert_vm_snapshots | Success | 95.72 | test_vm_snapshots.py
    test_01_create_vm_snapshots | Success | 158.76 | test_vm_snapshots.py
    test_deploy_vm_multiple | Success | 273.02 | test_vm_life_cycle.py
    test_deploy_vm | Success | 0.03 | test_vm_life_cycle.py
    test_advZoneVirtualRouter | Success | 0.03 | test_vm_life_cycle.py
    test_10_attachAndDetach_iso | Success | 26.67 | test_vm_life_cycle.py
    test_09_expunge_vm | Success | 125.17 | test_vm_life_cycle.py
    test_08_migrate_vm | Success | 41.01 | test_vm_life_cycle.py
    test_07_restore_vm | Success | 0.17 | test_vm_life_cycle.py
    test_06_destroy_vm | Success | 125.86 | test_vm_life_cycle.py
    test_03_reboot_vm | Success | 125.87 | test_vm_life_cycle.py
    test_02_start_vm | Success | 10.17 | test_vm_life_cycle.py
    test_01_stop_vm | Success | 40.33 | test_vm_life_cycle.py
    test_CreateTemplateWithDuplicateName | Success | 40.46 | test_templates.py
    test_08_list_system_templates | Success | 0.06 | test_templates.py
    test_07_list_public_templates | Success | 0.04 | test_templates.py
    test_05_template_permissions | Success | 0.06 | test_templates.py
    test_04_extract_template | Success | 5.16 | test_templates.py
    test_03_delete_template | Success | 5.24 | test_templates.py
    test_02_edit_template | Success | 90.14 | test_templates.py
    test_01_create_template | Success | 30.38 | test_templates.py
    test_10_destroy_cpvm | Success | 191.67 | test_ssvm.py
    test_09_destroy_ssvm | Success | 163.63 | test_ssvm.py
    test_08_reboot_cpvm | Success | 131.62 | test_ssvm.py
    test_07_reboot_ssvm | Success | 133.60 | test_ssvm.py
    test_06_stop_cpvm | Success | 131.75 | test_ssvm.py
    test_05_stop_ssvm | Success | 133.87 | test_ssvm.py
    test_04_cpvm_internals | Success | 1.22 | test_ssvm.py
    test_03_ssvm_internals | Success | 3.62 | test_ssvm.py
    test_02_list_cpvm_vm | Success | 0.15 | test_ssvm.py
    test_01_list_sec_storage_vm | Success | 0.13 | test_ssvm.py
    test_01_snapshot_root_disk | Success | 11.11 | test_snapshots.py
    test_04_change_offering_small | Success | 242.67 | test_service_offerings.py
    test_03_delete_service_offering | Success | 0.04 | test_service_offerings.py
    test_02_edit_service_offering | Success | 0.06 | test_service_offerings.py
    test_01_create_service_offering | Success | 0.14 | test_service_offerings.py
    test_02_sys_template_ready | Success | 0.13 | test_secondary_storage.py
    test_01_sys_vm_start | Success | 0.19 | test_secondary_storage.py
    test_09_reboot_router | Success | 35.35 | test_routers.py
    test_08_start_router | Success | 30.33 | test_routers.py
    test_07_stop_router | Success | 10.16 | test_routers.py
    test_06_router_advanced | Success | 0.06 | test_routers.py
    test_05_router_basic | Success | 0.04 | test_routers.py
    test_04_restart_network_wo_cleanup | Success | 5.72 | test_routers.py
    test_03_restart_network_cleanup | Success | 60.55 | test_routers.py
    test_02_router_internal_adv | Success | 1.04 | test_routers.py
    test_01_router_internal_basic | Success | 0.58 | test_routers.py
    test_router_dns_guestipquery | Success | 76.78 | test_router_dns.py
    test_router_dns_externalipquery | Success | 0.08 | test_router_dns.py
    test_router_dhcphosts | Success | 276.92 | test_router_dhcphosts.py
    test_router_dhcp_opts | Success | 21.87 | test_router_dhcphosts.py
    test_01_updatevolumedetail | Success | 0.08 | test_resource_detail.py
    test_01_reset_vm_on_reboot | Success | 130.96 | test_reset_vm_on_reboot.py
    test_createRegion | Success | 0.04 | test_regions.py
    test_create_pvlan_network | Success | 5.22 | test_pvlan.py
    test_dedicatePublicIpRange | Success | 0.47 | test_public_ip_range.py
    test_03_vpc_privategw_restart_vpc_cleanup | Success | 475.21 | test_privategw_acl.py
    test_02_vpc_privategw_static_routes | Success | 360.75 | test_privategw_acl.py
    test_01_vpc_privategw_acl | Success | 87.36 | test_privategw_acl.py
    test_01_primary_storage_nfs | Success | 35.80 | test_primary_storage.py
    test_createPortablePublicIPRange | Success | 15.19 | test_portable_publicip.py
    test_createPortablePublicIPAcquire | Success | 15.52 | test_portable_publicip.py
    test_isolate_network_password_server | Success | 89.41 | test_password_server.py
    test_UpdateStorageOverProvisioningFactor | Success | 0.14 | test_over_provisioning.py
    test_oobm_zchange_password | Success | 30.94 | test_outofbandmanagement.py
    test_oobm_multiple_mgmt_server_ownership | Success | 16.35 | test_outofbandmanagement.py
    test_oobm_issue_power_status | Success | 10.24 | test_outofbandmanagement.py
    test_oobm_issue_power_soft | Success | 10.39 | test_outofbandmanagement.py
    test_oobm_issue_power_reset | Success | 15.35 | test_outofbandmanagement.py
    test_oobm_issue_power_on | Success | 15.33 | test_outofbandmanagement.py
    test_oobm_issue_power_off | Success | 15.33 | test_outofbandmanagement.py
    test_oobm_issue_power_cycle | Success | 15.33 | test_outofbandmanagement.py
    test_oobm_enabledisable_across_clusterzones | Success | 87.76 | test_outofbandmanagement.py
    test_oobm_enable_feature_valid | Success | 5.43 | test_outofbandmanagement.py
    test_oobm_enable_feature_invalid | Success | 0.11 | test_outofbandmanagement.py
    test_oobm_disable_feature_valid | Success | 0.17 | test_outofbandmanagement.py
    test_oobm_disable_feature_invalid | Success | 0.10 | test_outofbandmanagement.py
    test_oobm_configure_invalid_driver | Success | 0.08 | test_outofbandmanagement.py
    test_oobm_configure_default_driver | Success | 0.08 | test_outofbandmanagement.py
    test_oobm_background_powerstate_sync | Success | 23.45 | test_outofbandmanagement.py
    test_extendPhysicalNetworkVlan | Success | 15.32 | test_non_contigiousvlan.py
    test_01_nic | Success | 434.39 | test_nic.py
    test_releaseIP | Success | 253.11 | test_network.py
    test_reboot_router | Success | 393.70 | test_network.py
    test_public_ip_user_account | Success | 10.33 | test_network.py
    test_public_ip_admin_account | Success | 40.30 | test_network.py
    test_network_rules_acquired_public_ip_3_Load_Balancer_Rule | Success | 66.96 | test_network.py
    test_network_rules_acquired_public_ip_2_nat_rule | Success | 61.78 | test_network.py
    test_network_rules_acquired_public_ip_1_static_nat_rule | Success | 124.30 | test_network.py
    test_delete_account | Success | 272.94 | test_network.py
    test_02_port_fwd_on_non_src_nat | Success | 55.65 | test_network.py
    test_01_port_fwd_on_src_nat | Success | 111.75 | test_network.py
    test_nic_secondaryip_add_remove | Success | 227.89 | test_multipleips_per_nic.py
    login_test_saml_user | Success | 19.36 | test_login.py
    test_assign_and_removal_lb | Success | 134.14 | test_loadbalance.py
    test_02_create_lb_rule_non_nat | Success | 187.28 | test_loadbalance.py
    test_01_create_lb_rule_src_nat | Success | 217.72 | test_loadbalance.py
    test_03_list_snapshots | Success | 0.07 | test_list_ids_parameter.py
    test_02_list_templates | Success | 0.04 | test_list_ids_parameter.py
    test_01_list_volumes | Success | 0.03 | test_list_ids_parameter.py
    test_07_list_default_iso | Success | 0.06 | test_iso.py
    test_05_iso_permissions | Success | 0.06 | test_iso.py
    test_04_extract_Iso | Success | 5.16 | test_iso.py
    test_03_delete_iso | Success | 95.14 | test_iso.py
    test_02_edit_iso | Success | 0.06 | test_iso.py
    test_01_create_iso | Success | 21.04 | test_iso.py
    test_04_rvpc_internallb_haproxy_stats_on_all_interfaces | Success | 203.16 | test_internal_lb.py
    test_03_vpc_internallb_haproxy_stats_on_all_interfaces | Success | 148.07 | test_internal_lb.py
    test_02_internallb_roundrobin_1RVPC_3VM_HTTP_port80 | Success | 525.63 | test_internal_lb.py
    test_01_internallb_roundrobin_1VPC_3VM_HTTP_port80 | Success | 440.91 | test_internal_lb.py
    test_dedicateGuestVlanRange | Success | 10.44 | test_guest_vlan_range.py
    test_UpdateConfigParamWithScope | Success | 0.14 | test_global_settings.py
    test_rolepermission_lifecycle_update | Success | 6.24 | test_dynamicroles.py
    test_rolepermission_lifecycle_list | Success | 6.03 | test_dynamicroles.py
    test_rolepermission_lifecycle_delete | Success | 5.89 | test_dynamicroles.py
    test_rolepermission_lifecycle_create | Success | 5.93 | test_dynamicroles.py
    test_rolepermission_lifecycle_concurrent_updates | Success | 6.18 | test_dynamicroles.py
    test_role_lifecycle_update_role_inuse | Success | 5.92 | test_dynamicroles.py
    test_role_lifecycle_update | Success | 11.09 | test_dynamicroles.py
    test_role_lifecycle_list | Success | 6.54 | test_dynamicroles.py
    test_role_lifecycle_delete | Success | 10.96 | test_dynamicroles.py
    test_role_lifecycle_create | Success | 5.91 | test_dynamicroles.py
    test_role_inuse_deletion | Success | 5.90 | test_dynamicroles.py
    test_role_account_acls_multiple_mgmt_servers | Success | 8.43 | test_dynamicroles.py
    test_role_account_acls | Success | 8.54 | test_dynamicroles.py
    test_default_role_deletion | Success | 6.06 | test_dynamicroles.py
    test_04_create_fat_type_disk_offering | Success | 0.07 | test_disk_offerings.py
    test_03_delete_disk_offering | Success | 0.04 | test_disk_offerings.py
    test_02_edit_disk_offering | Success | 0.06 | test_disk_offerings.py
    test_02_create_sparse_type_disk_offering | Success | 0.07 | test_disk_offerings.py
    test_01_create_disk_offering | Success | 0.11 | test_disk_offerings.py
    test_deployvm_userdispersing | Success | 25.65 | test_deploy_vms_with_varied_deploymentplanners.py
    test_deployvm_userconcentrated | Success | 50.81 | test_deploy_vms_with_varied_deploymentplanners.py
    test_deployvm_firstfit | Success | 55.68 | test_deploy_vms_with_varied_deploymentplanners.py
    test_deployvm_userdata_post | Success | 20.50 | test_deploy_vm_with_userdata.py
    test_deployvm_userdata | Success | 55.77 | test_deploy_vm_with_userdata.py
    test_02_deploy_vm_root_resize | Success | 6.17 | test_deploy_vm_root_resize.py
    test_01_deploy_vm_root_resize | Success | 6.01 | test_deploy_vm_root_resize.py
    test_00_deploy_vm_root_resize | Success | 217.87 | test_deploy_vm_root_resize.py
    test_deploy_vm_from_iso | Success | 212.74 | test_deploy_vm_iso.py
    test_DeployVmAntiAffinityGroup | Success | 76.50 | test_affinity_groups.py
    test_change_service_offering_for_vm_with_snapshots | Skipped | 0.00 | test_vm_snapshots.py
    test_01_test_vm_volume_snapshot | Skipped | 0.00 | test_vm_snapshots.py
    test_06_copy_template | Skipped | 0.00 | test_templates.py
    test_static_role_account_acls | Skipped | 0.02 | test_staticroles.py
    test_11_ss_nfs_version_on_ssvm | Skipped | 0.02 | test_ssvm.py
    test_01_scale_vm | Skipped | 0.00 | test_scale_vm.py
    test_01_primary_storage_iscsi | Skipped | 0.04 | test_primary_storage.py
    test_nested_virtualization_vmware | Skipped | 0.00 | test_nested_virtualization.py
    test_06_copy_iso | Skipped | 0.00 | test_iso.py
    test_deploy_vgpu_enabled_vm | Skipped | 0.04 | test_deploy_vgpu_enabled_vm.py
    test_3d_gpu_support | Skipped | 0.04 | test_deploy_vgpu_enabled_vm.py



---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack issue #1953: CLOUDSTACK-9794: Unable to attach more than 14 devic...

Posted by borisstoyanov <gi...@git.apache.org>.
Github user borisstoyanov commented on the issue:

    https://github.com/apache/cloudstack/pull/1953
  
    @blueorangutan test


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request #1953: CLOUDSTACK-9794: Unable to attach more than 1...

Posted by sureshanaparti <gi...@git.apache.org>.
Github user sureshanaparti commented on a diff in the pull request:

    https://github.com/apache/cloudstack/pull/1953#discussion_r102301645
  
    --- 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);
    --- End diff --
    
    @HrWiggles Thanks for the review. The max data volumes here is the actual hypervisor capability (which is posted in the db). The device id 3 is being reserved for something since long and I don't want that to be effected.
    When _getMaxDataVolumesSupported()_ returns 6, max 5 volumes can be attached to the VM and one device reserved (might be for virtual tools/CDROM). _maxDataVolumesSupported_ specifies the data volumes limit supported by hypervisor, nothing related to _maxDeviceId_.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request #1953: CLOUDSTACK-9794: Unable to attach more than 1...

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:

    https://github.com/apache/cloudstack/pull/1953


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack issue #1953: CLOUDSTACK-9794: Unable to attach more than 14 devic...

Posted by blueorangutan <gi...@git.apache.org>.
Github user blueorangutan commented on the issue:

    https://github.com/apache/cloudstack/pull/1953
  
    Packaging result: \u2714centos6 \u2714centos7 \u2714debian. JID-521


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request #1953: CLOUDSTACK-9794: Unable to attach more than 1...

Posted by sureshanaparti <gi...@git.apache.org>.
Github user sureshanaparti commented on a diff in the pull request:

    https://github.com/apache/cloudstack/pull/1953#discussion_r102303488
  
    --- 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>();
    --- End diff --
    
    @HrWiggles. Noted, not considering it for now.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request #1953: CLOUDSTACK-9794: Unable to attach more than 1...

Posted by HrWiggles <gi...@git.apache.org>.
Github user HrWiggles commented on a diff in the pull request:

    https://github.com/apache/cloudstack/pull/1953#discussion_r102187534
  
    --- 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) {
    --- End diff --
    
    How about adding unit tests for the method `getDeviceId(UserVmVO vm, Long deviceId)`?
    Things I can currently think of to test:
    - `RuntimeException` if param `deviceId` is specified as a negative value
    - `RuntimeException` if param `deviceId` is specified as `0L`
    - `RuntimeException` if param `deviceId` is specified as a value greater than the "max-device-id"
    - `RuntimeException` if param `deviceId` is specified as reserved id `3L`
    - `RuntimeException` if param `deviceId` is specified as an id that is already in use
    - `RuntimeException` if param `deviceId` is specified as `null` and all device ids are in use
    - returns id specified in param `deviceId` when not `null` and the id is not in use
    - returns lowest available id when param `deviceId` is specified as `null`
    
    (all of the above are from my understanding of how the method should behave)


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack issue #1953: CLOUDSTACK-9794: Unable to attach more than 14 devic...

Posted by blueorangutan <gi...@git.apache.org>.
Github user blueorangutan commented on the issue:

    https://github.com/apache/cloudstack/pull/1953
  
    @borisstoyanov a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack issue #1953: CLOUDSTACK-9794: Unable to attach more than 14 devic...

Posted by blueorangutan <gi...@git.apache.org>.
Github user blueorangutan commented on the issue:

    https://github.com/apache/cloudstack/pull/1953
  
    @borisstoyanov a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request #1953: CLOUDSTACK-9794: Unable to attach more than 1...

Posted by HrWiggles <gi...@git.apache.org>.
Github user HrWiggles commented on a diff in the pull request:

    https://github.com/apache/cloudstack/pull/1953#discussion_r102179249
  
    --- 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) {
    --- End diff --
    
    There's no check for whether `deviceId` is greater than `0`.  Should check for that here, as well.  Otherwise, the method will accept `0` and negative values as valid device ids.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request #1953: CLOUDSTACK-9794: Unable to attach more than 1...

Posted by HrWiggles <gi...@git.apache.org>.
Github user HrWiggles commented on a diff in the pull request:

    https://github.com/apache/cloudstack/pull/1953#discussion_r102184780
  
    --- 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>();
    --- End diff --
    
    Not part of your changes but... variable `devIds` should have type `List<Long>` instead of `List<String>`.
    All that conversion from `int` to `String` and then converting from `String` to `long` seems unnecessary.
    Should simply be able to do:
    ```
    List<Long> devIds = new ArrayList<>();
    for (long i = 1; i <= maxDataVolumesSupported; i++) {
        devIds.add(i);
    }
    devIds.remove(3L);
    for (VolumeVO vol : vols) {
        devIds.remove(vol.getDeviceId());
    }
    if (devIds.isEmpty()) {
        throw new RuntimeException("every available deviceId already in use by vm " + vm.getId());
    }
    deviceId = devIds.iterator().next();
    ```
    Note: my code above includes fixes to two other comments I made further down in the code.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack issue #1953: CLOUDSTACK-9794: Unable to attach more than 14 devic...

Posted by remibergsma <gi...@git.apache.org>.
Github user remibergsma commented on the issue:

    https://github.com/apache/cloudstack/pull/1953
  
    Thanks @sureshanaparti will test it soon! 


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request #1953: CLOUDSTACK-9794: Unable to attach more than 1...

Posted by sureshanaparti <gi...@git.apache.org>.
Github user sureshanaparti commented on a diff in the pull request:

    https://github.com/apache/cloudstack/pull/1953#discussion_r102301869
  
    --- 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) {
    --- End diff --
    
    @HrWiggles Will check if I can write a test for the same.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack issue #1953: CLOUDSTACK-9794: Unable to attach more than 14 devic...

Posted by borisstoyanov <gi...@git.apache.org>.
Github user borisstoyanov commented on the issue:

    https://github.com/apache/cloudstack/pull/1953
  
    @blueorangutan test


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request #1953: CLOUDSTACK-9794: Unable to attach more than 1...

Posted by HrWiggles <gi...@git.apache.org>.
Github user HrWiggles commented on a diff in the pull request:

    https://github.com/apache/cloudstack/pull/1953#discussion_r102177792
  
    --- 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);
    --- End diff --
    
    Is it enough to use max-data-volumes-supported as the max device id, seeing as device id 3 is not used? (or is reserved)
    
    E.g. let's assume that `getMaxDataVolumesSupported(vm)` returns `6`.  What device ids should be ok in that case?  Would they be `1`, `2`, `4`, `5`, and `6` (since `maxDataVolumesSupported` is `6`) which is a total of `5` data volumes, or should `maxDataVolumesSupported` be renamed as `maxDeviceId` and be assigned a value of `getMaxDataVolumesSupported(vm) + 1` to account for the unused/reserved id `3`?


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request #1953: CLOUDSTACK-9794: Unable to attach more than 1...

Posted by sureshanaparti <gi...@git.apache.org>.
Github user sureshanaparti commented on a diff in the pull request:

    https://github.com/apache/cloudstack/pull/1953#discussion_r102309898
  
    --- 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);
    --- End diff --
    
    @rafaelweingartner when configuring getMaxDataVolumesSupported(vm) with 6 for the hypervisor of the VM, the VM can have max 6 devices. 1 root (id 0), 1 CD-ROM (id 3) and other 4 for extra disks/volumes.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request #1953: CLOUDSTACK-9794: Unable to attach more than 1...

Posted by HrWiggles <gi...@git.apache.org>.
Github user HrWiggles commented on a diff in the pull request:

    https://github.com/apache/cloudstack/pull/1953#discussion_r102178630
  
    --- 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 --
    
    Since `maxDataVolumesSupported` is basically being used to indicate the "max-device-id", the conditional should be `i <= maxDataVolumesSupported`.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request #1953: CLOUDSTACK-9794: Unable to attach more than 1...

Posted by sureshanaparti <gi...@git.apache.org>.
Github user sureshanaparti commented on a diff in the pull request:

    https://github.com/apache/cloudstack/pull/1953#discussion_r102303566
  
    --- 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++) {
                     devIds.add(String.valueOf(i));
                 }
                 devIds.remove("3");
    --- End diff --
    
    Thanks. Added this.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request #1953: CLOUDSTACK-9794: Unable to attach more than 1...

Posted by sureshanaparti <gi...@git.apache.org>.
Github user sureshanaparti commented on a diff in the pull request:

    https://github.com/apache/cloudstack/pull/1953#discussion_r102539843
  
    --- Diff: plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtVMDef.java ---
    @@ -584,18 +584,36 @@ public void defFileBasedDisk(String filePath, String diskLabel, DiskBus bus, Dis
     
             /* skip iso label */
             private String getDevLabel(int devId, DiskBus bus) {
    --- End diff --
    
    @HrWiggles Will add unit tests.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---