You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cloudstack.apache.org by rhtyd <gi...@git.apache.org> on 2016/12/02 07:17:02 UTC

[GitHub] cloudstack pull request #1807: CLOUDSTACK-9633: Revert addition of `vhd` ext...

GitHub user rhtyd opened a pull request:

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

    CLOUDSTACK-9633: Revert addition of `vhd` extention to snapshots

    This reverts commit f1fd325c085cd61336ac616ba76e2c1f3f916cd1 and changes
    introduced in commit f46651e6721106941deeb6b5e6bf51d7e9efc61c that changed the
    snapshot file name to include a `vhd` extension.
    
    With this change, CloudStack users should not hit upgrade issues.
    
    Reference: https://github.com/apache/cloudstack/pull/1600#pullrequestreview-10955963

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

    $ git pull https://github.com/shapeblue/cloudstack xen-vhd-extension-fix

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

    https://github.com/apache/cloudstack/pull/1807.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 #1807
    
----
commit 2f24ebb7773653de9e389ca6637c47530e7eba46
Author: Rohit Yadav <ro...@shapeblue.com>
Date:   2016-12-02T07:14:16Z

    CLOUDSTACK-9633: Revert addition of `vhd` extention to snapshots
    
    This reverts commit f1fd325c085cd61336ac616ba76e2c1f3f916cd1 and changes
    introduced in commit f46651e6721106941deeb6b5e6bf51d7e9efc61c that changed the
    snapshot file name to include a `vhd` extension.
    
    With this change, CloudStack users should not hit upgrade issues.
    
    Signed-off-by: Rohit Yadav <ro...@shapeblue.com>

----


---
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 #1807: CLOUDSTACK-9633: Revert addition of `vhd` extention ...

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

    https://github.com/apache/cloudstack/pull/1807
  
    @abhinandanprateek @syed I'm seeing known intermittent failures with vpc/vr related tests, can I get LGTM on this? 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 #1807: CLOUDSTACK-9633: Revert addition of `vhd` extention ...

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

    https://github.com/apache/cloudstack/pull/1807
  
    @syed @swill @rhtyd @serg38 I think i missed some updates. This PR will only break the original PR. The patch suggested by @serg38 looks good. 
    @syed in a upgraded setup, I think removal of a older snapshot will fail for one, as these donot have a .vhd extension.


---
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 #1807: CLOUDSTACK-9633: Revert addition of `vhd` extention ...

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

    https://github.com/apache/cloudstack/pull/1807
  
    @rhtyd 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 #1807: CLOUDSTACK-9633: Revert addition of `vhd` extention ...

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

    https://github.com/apache/cloudstack/pull/1807
  
    @rhtyd a Trillian-Jenkins test job (centos7 mgmt + xenserver-65sp1) 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 #1807: CLOUDSTACK-9633: Revert addition of `vhd` extention ...

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

    https://github.com/apache/cloudstack/pull/1807
  
    @abhinandanprateek I will check this. I will start with a non-upgraded cloudstack, create snapshots, upgrade cloudstack to have #1600 and then try to delete older snapshots. From the code it looks like it should delete irrespective of weather the snapshot has the `vhd` extension or not (see [here] (https://github.com/apache/cloudstack/blob/master/services/secondary-storage/server/src/org/apache/cloudstack/storage/resource/NfsSecondaryStorageResource.java#L1546) ) because it does a wildcard delete of the directory before.
    
    Are you sure that delete will break? Can you point out where this might happen?


---
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 #1807: CLOUDSTACK-9633: Revert addition of `vhd` extention ...

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

    https://github.com/apache/cloudstack/pull/1807
  
    @syed @rhtyd @abhinandanprateek Why cant we simply rename all install paths in DB for existing snapshots instead. It can be as simple as adding this query to schema upgrade path
    UPDATE cloud.snapshots s
            JOIN
        cloud.snapshot_store_ref ssr ON ssr.snapshot_id = s.id 
    SET 
        ssr.install_path = CONCAT(ssr.install_path, '.vhd')
    WHERE
        s.hypervisor_type = 'XenServer'
            AND RIGHT(ssr.install_path, 4) != '.vhd';


---
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 #1807: CLOUDSTACK-9633: Revert addition of `vhd` extention ...

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

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


---
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 #1807: CLOUDSTACK-9633: Revert addition of `vhd` extention ...

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

    https://github.com/apache/cloudstack/pull/1807
  
    @rhtyd I understand that we want to avoid breaking upgrades. But I have upgraded my setup and I did not find any problems. I am trying to understand what will break if upgrades happen, given the fact that this change will definitely break the newer functionality that we have added.


---
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 #1807: CLOUDSTACK-9633: Revert addition of `vhd` extention ...

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

    https://github.com/apache/cloudstack/pull/1807
  
    <b>Trillian test result (tid-578)</b>
    Environment: xenserver-65sp1 (x2), Advanced Networking with Mgmt server 7
    Total time taken: 37415 seconds
    Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr1807-t578-xenserver-65sp1.zip
    Test completed. 48 look ok, 1 have error(s)
    
    
    Test | Result | Time (s) | Test File
    --- | --- | --- | ---
    test_05_rvpc_multi_tiers | `Failure` | 516.08 | test_vpc_redundant.py
    test_04_rvpc_network_garbage_collector_nics | `Failure` | 1356.90 | test_vpc_redundant.py
    test_01_create_redundant_VPC_2tiers_4VMs_4IPs_4PF_ACL | `Failure` | 615.09 | test_vpc_redundant.py
    test_01_vpc_site2site_vpn | Success | 361.88 | test_vpc_vpn.py
    test_01_vpc_remote_access_vpn | Success | 176.89 | test_vpc_vpn.py
    test_01_redundant_vpc_site2site_vpn | Success | 673.53 | test_vpc_vpn.py
    test_02_VPC_default_routes | Success | 341.19 | test_vpc_router_nics.py
    test_01_VPC_nics_after_destroy | Success | 760.63 | test_vpc_router_nics.py
    test_03_create_redundant_VPC_1tier_2VMs_2IPs_2PF_ACL_reboot_routers | Success | 891.92 | test_vpc_redundant.py
    test_02_redundant_VPC_default_routes | Success | 1102.85 | test_vpc_redundant.py
    test_09_delete_detached_volume | Success | 20.94 | test_volumes.py
    test_08_resize_volume | Success | 111.07 | test_volumes.py
    test_07_resize_fail | Success | 116.09 | test_volumes.py
    test_06_download_detached_volume | Success | 20.32 | test_volumes.py
    test_05_detach_volume | Success | 100.27 | test_volumes.py
    test_04_delete_attached_volume | Success | 10.20 | test_volumes.py
    test_03_download_attached_volume | Success | 15.28 | test_volumes.py
    test_02_attach_volume | Success | 16.47 | test_volumes.py
    test_01_create_volume | Success | 393.70 | test_volumes.py
    test_03_delete_vm_snapshots | Success | 280.20 | test_vm_snapshots.py
    test_02_revert_vm_snapshots | Success | 197.71 | test_vm_snapshots.py
    test_01_create_vm_snapshots | Success | 141.88 | test_vm_snapshots.py
    test_deploy_vm_multiple | Success | 289.05 | test_vm_life_cycle.py
    test_deploy_vm | Success | 0.04 | test_vm_life_cycle.py
    test_advZoneVirtualRouter | Success | 0.03 | test_vm_life_cycle.py
    test_10_attachAndDetach_iso | Success | 46.82 | test_vm_life_cycle.py
    test_09_expunge_vm | Success | 125.18 | test_vm_life_cycle.py
    test_08_migrate_vm | Success | 136.56 | test_vm_life_cycle.py
    test_07_restore_vm | Success | 0.11 | test_vm_life_cycle.py
    test_06_destroy_vm | Success | 15.20 | test_vm_life_cycle.py
    test_03_reboot_vm | Success | 20.25 | test_vm_life_cycle.py
    test_02_start_vm | Success | 25.27 | test_vm_life_cycle.py
    test_01_stop_vm | Success | 30.29 | test_vm_life_cycle.py
    test_CreateTemplateWithDuplicateName | Success | 166.28 | test_templates.py
    test_08_list_system_templates | Success | 0.04 | test_templates.py
    test_07_list_public_templates | Success | 0.04 | test_templates.py
    test_05_template_permissions | Success | 0.11 | test_templates.py
    test_04_extract_template | Success | 5.16 | test_templates.py
    test_03_delete_template | Success | 5.12 | test_templates.py
    test_02_edit_template | Success | 90.10 | test_templates.py
    test_01_create_template | Success | 65.63 | test_templates.py
    test_10_destroy_cpvm | Success | 261.86 | test_ssvm.py
    test_09_destroy_ssvm | Success | 229.85 | test_ssvm.py
    test_08_reboot_cpvm | Success | 157.00 | test_ssvm.py
    test_07_reboot_ssvm | Success | 144.08 | test_ssvm.py
    test_06_stop_cpvm | Success | 172.18 | test_ssvm.py
    test_05_stop_ssvm | Success | 139.45 | test_ssvm.py
    test_04_cpvm_internals | Success | 1.12 | test_ssvm.py
    test_03_ssvm_internals | Success | 3.50 | test_ssvm.py
    test_02_list_cpvm_vm | Success | 0.13 | test_ssvm.py
    test_01_list_sec_storage_vm | Success | 0.14 | test_ssvm.py
    test_01_snapshot_root_disk | Success | 21.32 | test_snapshots.py
    test_04_change_offering_small | Success | 129.16 | 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.08 | 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_01_scale_vm | Success | 5.21 | test_scale_vm.py
    test_09_reboot_router | Success | 75.60 | test_routers.py
    test_08_start_router | Success | 60.96 | test_routers.py
    test_07_stop_router | Success | 15.22 | 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.57 | test_routers.py
    test_03_restart_network_cleanup | Success | 141.10 | test_routers.py
    test_02_router_internal_adv | Success | 0.90 | test_routers.py
    test_01_router_internal_basic | Success | 0.52 | test_routers.py
    test_router_dns_guestipquery | Success | 46.83 | test_router_dns.py
    test_router_dns_externalipquery | Success | 0.09 | test_router_dns.py
    test_router_dhcphosts | Success | 117.88 | test_router_dhcphosts.py
    test_router_dhcp_opts | Success | 31.64 | test_router_dhcphosts.py
    test_01_updatevolumedetail | Success | 0.09 | test_resource_detail.py
    test_01_reset_vm_on_reboot | Success | 45.48 | test_reset_vm_on_reboot.py
    test_createRegion | Success | 0.05 | test_regions.py
    test_create_pvlan_network | Success | 5.26 | test_pvlan.py
    test_dedicatePublicIpRange | Success | 0.49 | test_public_ip_range.py
    test_04_rvpc_privategw_static_routes | Success | 1128.85 | test_privategw_acl.py
    test_03_vpc_privategw_restart_vpc_cleanup | Success | 1082.28 | test_privategw_acl.py
    test_02_vpc_privategw_static_routes | Success | 715.25 | test_privategw_acl.py
    test_01_vpc_privategw_acl | Success | 153.81 | test_privategw_acl.py
    test_01_primary_storage_nfs | Success | 39.07 | test_primary_storage.py
    test_01_primary_storage_iscsi | Success | 79.63 | test_primary_storage.py
    test_createPortablePublicIPRange | Success | 15.22 | test_portable_publicip.py
    test_createPortablePublicIPAcquire | Success | 15.55 | test_portable_publicip.py
    test_isolate_network_password_server | Success | 36.37 | test_password_server.py
    test_UpdateStorageOverProvisioningFactor | Success | 0.18 | test_over_provisioning.py
    test_oobm_zchange_password | Success | 31.06 | test_outofbandmanagement.py
    test_oobm_multiple_mgmt_server_ownership | Success | 16.41 | test_outofbandmanagement.py
    test_oobm_issue_power_status | Success | 10.47 | test_outofbandmanagement.py
    test_oobm_issue_power_soft | Success | 15.51 | test_outofbandmanagement.py
    test_oobm_issue_power_reset | Success | 15.49 | test_outofbandmanagement.py
    test_oobm_issue_power_on | Success | 15.45 | test_outofbandmanagement.py
    test_oobm_issue_power_off | Success | 15.51 | test_outofbandmanagement.py
    test_oobm_issue_power_cycle | Success | 15.39 | test_outofbandmanagement.py
    test_oobm_enabledisable_across_clusterzones | Success | 87.91 | test_outofbandmanagement.py
    test_oobm_enable_feature_valid | Success | 5.23 | test_outofbandmanagement.py
    test_oobm_enable_feature_invalid | Success | 0.11 | test_outofbandmanagement.py
    test_oobm_disable_feature_valid | Success | 5.22 | test_outofbandmanagement.py
    test_oobm_disable_feature_invalid | Success | 0.11 | test_outofbandmanagement.py
    test_oobm_configure_invalid_driver | Success | 0.09 | test_outofbandmanagement.py
    test_oobm_configure_default_driver | Success | 0.12 | test_outofbandmanagement.py
    test_oobm_background_powerstate_sync | Success | 29.55 | test_outofbandmanagement.py
    test_extendPhysicalNetworkVlan | Success | 15.40 | test_non_contigiousvlan.py
    test_01_nic | Success | 667.87 | test_nic.py
    test_releaseIP | Success | 375.32 | test_network.py
    test_reboot_router | Success | 586.44 | test_network.py
    test_public_ip_user_account | Success | 10.28 | test_network.py
    test_public_ip_admin_account | Success | 40.32 | test_network.py
    test_network_rules_acquired_public_ip_3_Load_Balancer_Rule | Success | 86.80 | test_network.py
    test_network_rules_acquired_public_ip_2_nat_rule | Success | 76.93 | test_network.py
    test_network_rules_acquired_public_ip_1_static_nat_rule | Success | 101.29 | test_network.py
    test_delete_account | Success | 389.80 | test_network.py
    test_02_port_fwd_on_non_src_nat | Success | 70.82 | test_network.py
    test_01_port_fwd_on_src_nat | Success | 83.84 | test_network.py
    test_nic_secondaryip_add_remove | Success | 269.11 | test_multipleips_per_nic.py
    login_test_saml_user | Success | 24.69 | test_login.py
    test_assign_and_removal_lb | Success | 149.12 | test_loadbalance.py
    test_02_create_lb_rule_non_nat | Success | 207.89 | test_loadbalance.py
    test_01_create_lb_rule_src_nat | Success | 209.33 | test_loadbalance.py
    test_03_list_snapshots | Success | 0.11 | test_list_ids_parameter.py
    test_02_list_templates | Success | 0.06 | test_list_ids_parameter.py
    test_01_list_volumes | Success | 0.04 | test_list_ids_parameter.py
    test_07_list_default_iso | Success | 0.07 | test_iso.py
    test_05_iso_permissions | Success | 0.07 | test_iso.py
    test_04_extract_Iso | Success | 5.24 | test_iso.py
    test_03_delete_iso | Success | 95.20 | test_iso.py
    test_02_edit_iso | Success | 0.08 | test_iso.py
    test_01_create_iso | Success | 22.30 | test_iso.py
    test_04_rvpc_internallb_haproxy_stats_on_all_interfaces | Success | 526.43 | test_internal_lb.py
    test_03_vpc_internallb_haproxy_stats_on_all_interfaces | Success | 375.00 | test_internal_lb.py
    test_02_internallb_roundrobin_1RVPC_3VM_HTTP_port80 | Success | 1087.11 | test_internal_lb.py
    test_01_internallb_roundrobin_1VPC_3VM_HTTP_port80 | Success | 730.71 | test_internal_lb.py
    test_dedicateGuestVlanRange | Success | 10.36 | test_guest_vlan_range.py
    test_UpdateConfigParamWithScope | Success | 0.36 | test_global_settings.py
    test_rolepermission_lifecycle_update | Success | 7.57 | test_dynamicroles.py
    test_rolepermission_lifecycle_list | Success | 7.54 | test_dynamicroles.py
    test_rolepermission_lifecycle_delete | Success | 7.18 | test_dynamicroles.py
    test_rolepermission_lifecycle_create | Success | 7.27 | test_dynamicroles.py
    test_rolepermission_lifecycle_concurrent_updates | Success | 7.42 | test_dynamicroles.py
    test_role_lifecycle_update_role_inuse | Success | 7.43 | test_dynamicroles.py
    test_role_lifecycle_update | Success | 12.31 | test_dynamicroles.py
    test_role_lifecycle_list | Success | 7.12 | test_dynamicroles.py
    test_role_lifecycle_delete | Success | 12.23 | test_dynamicroles.py
    test_role_lifecycle_create | Success | 7.53 | test_dynamicroles.py
    test_role_inuse_deletion | Success | 7.15 | test_dynamicroles.py
    test_role_account_acls_multiple_mgmt_servers | Success | 11.04 | test_dynamicroles.py
    test_role_account_acls | Success | 12.28 | test_dynamicroles.py
    test_default_role_deletion | Success | 7.92 | test_dynamicroles.py
    test_04_create_fat_type_disk_offering | Success | 0.11 | test_disk_offerings.py
    test_03_delete_disk_offering | Success | 0.06 | test_disk_offerings.py
    test_02_edit_disk_offering | Success | 0.08 | test_disk_offerings.py
    test_02_create_sparse_type_disk_offering | Success | 0.11 | test_disk_offerings.py
    test_01_create_disk_offering | Success | 0.14 | test_disk_offerings.py
    test_deployvm_userdispersing | Success | 67.19 | test_deploy_vms_with_varied_deploymentplanners.py
    test_deployvm_userconcentrated | Success | 182.61 | test_deploy_vms_with_varied_deploymentplanners.py
    test_deployvm_firstfit | Success | 268.37 | test_deploy_vms_with_varied_deploymentplanners.py
    test_deployvm_userdata_post | Success | 127.43 | test_deploy_vm_with_userdata.py
    test_deployvm_userdata | Success | 151.86 | test_deploy_vm_with_userdata.py
    test_02_deploy_vm_root_resize | Success | 7.30 | test_deploy_vm_root_resize.py
    test_01_deploy_vm_root_resize | Success | 7.40 | test_deploy_vm_root_resize.py
    test_00_deploy_vm_root_resize | Success | 7.76 | test_deploy_vm_root_resize.py
    test_deploy_vm_from_iso | Success | 375.92 | test_deploy_vm_iso.py
    test_DeployVmAntiAffinityGroup | Success | 273.09 | test_affinity_groups.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.03 | test_staticroles.py
    test_11_ss_nfs_version_on_ssvm | Skipped | 0.02 | test_ssvm.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 #1807: CLOUDSTACK-9633: Revert addition of `vhd` extention ...

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

    https://github.com/apache/cloudstack/pull/1807
  
    @serg38 I think we can do that, but will be also need some tooling to rename the actual files on the SRs? /cc @abhinandanprateek 


---
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 #1807: CLOUDSTACK-9633: Revert addition of `vhd` extention ...

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

    https://github.com/apache/cloudstack/pull/1807
  
    @rhtyd a Trillian-Jenkins test job (centos7 mgmt + xenserver-65sp1) 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 #1807: CLOUDSTACK-9633: Revert addition of `vhd` extention ...

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

    https://github.com/apache/cloudstack/pull/1807
  
    @rhtyd If you merge this you will break #1600.  We may want to do some more testing on the upgrade path, but I believe @syed has done some testing of the upgrade path.  Maybe we can put together a list of items to test for the upgrade path so we can confirm that this revert is not needed.


---
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 #1807: CLOUDSTACK-9633: Revert addition of `vhd` extention ...

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

    https://github.com/apache/cloudstack/pull/1807
  
    Thanks @abhinandanprateek @syed 


---
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.
---

Re: [GitHub] cloudstack issue #1807: CLOUDSTACK-9633: Revert addition of `vhd` extention ...

Posted by Rohit Yadav <ro...@shapeblue.com>.
Syed,


The aim of this PR #1807 is to avoid breaking upgrades for users.


With introduction of your PR (#1600), an extension was seen to be added to snapshot files. The PR I've shared removes '.vhd' extension from the snapshot file names, while regression smoke tests against XenServer65sp1 seem to be passing -- we're not sure if this will have any side-effects in your code (i.e. changes from your PR#1600).


Can you validate and review the PR and run tests on your side (i.e. test master branch including PR#1807)?

If we fail to do that this week, otherwise to avoid further delaying the release work we'll have to revert your previous PR merge (#1600), request you to submit it again.


Regards.

________________________________
From: Syed Ahmed <sa...@cloudops.com>
Sent: 06 December 2016 10:20:45
To: dev@cloudstack.apache.org
Subject: Re: [GitHub] cloudstack issue #1807: CLOUDSTACK-9633: Revert addition of `vhd` extention ...

I see. I was more interested in the functionality that is broken on
Cloudstack. I have verified that older operations are still working
correctly. Did I miss anything?
On Mon, Dec 5, 2016 at 23:44 Abhinandan Prateek <
abhinandan.prateek@shapeblue.com> wrote:

> Syed,
>
>   Any test that is calling is_snapshot_on_nfs from utils.py will fail for
> Xen as it ends up appending an additional “.vhd” to the install_path. There
> are several of them both in smoke and component folder. The reason this is
> under scrutiny is die to marvin test failures.
>
>
>
> On 06/12/16, 9:33 AM, "Syed Ahmed" <sa...@cloudops.com> wrote:
>
> >Yes this will break managed snapshot archives. I want to understand what
> >this revert is trying to fix and what tests were failing.
> >On Mon, Dec 5, 2016 at 22:38 abhinandanprateek <gi...@git.apache.org>
> wrote:
> >
> >> Github user abhinandanprateek commented on the issue:
> >>
> >>     https://github.com/apache/cloudstack/pull/1807
> >>
> >>     @syed can you look at this 'revert' i guess this is going to be an
> >> issue with the managed storage change that has been put in. cc @rhtyd
> >>
> >>
> >> ---
> >> 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.
> >> ---
> >>
>
> abhinandan.prateek@shapeblue.com
> www.shapeblue.com<http://www.shapeblue.com>
> 53 Chandos Place, Covent Garden, London  WC2N 4HSUK
> @shapeblue
>
>
>
>

rohit.yadav@shapeblue.com 
www.shapeblue.com
53 Chandos Place, Covent Garden, London  WC2N 4HSUK
@shapeblue
  
 


Re: [GitHub] cloudstack issue #1807: CLOUDSTACK-9633: Revert addition of `vhd` extention ...

Posted by Syed Ahmed <sa...@cloudops.com>.
I see. I was more interested in the functionality that is broken on
Cloudstack. I have verified that older operations are still working
correctly. Did I miss anything?
On Mon, Dec 5, 2016 at 23:44 Abhinandan Prateek <
abhinandan.prateek@shapeblue.com> wrote:

> Syed,
>
>   Any test that is calling is_snapshot_on_nfs from utils.py will fail for
> Xen as it ends up appending an additional “.vhd” to the install_path. There
> are several of them both in smoke and component folder. The reason this is
> under scrutiny is die to marvin test failures.
>
>
>
> On 06/12/16, 9:33 AM, "Syed Ahmed" <sa...@cloudops.com> wrote:
>
> >Yes this will break managed snapshot archives. I want to understand what
> >this revert is trying to fix and what tests were failing.
> >On Mon, Dec 5, 2016 at 22:38 abhinandanprateek <gi...@git.apache.org>
> wrote:
> >
> >> Github user abhinandanprateek commented on the issue:
> >>
> >>     https://github.com/apache/cloudstack/pull/1807
> >>
> >>     @syed can you look at this 'revert' i guess this is going to be an
> >> issue with the managed storage change that has been put in. cc @rhtyd
> >>
> >>
> >> ---
> >> 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.
> >> ---
> >>
>
> abhinandan.prateek@shapeblue.com
> www.shapeblue.com
> 53 Chandos Place, Covent Garden, London  WC2N 4HSUK
> @shapeblue
>
>
>
>

Re: [GitHub] cloudstack issue #1807: CLOUDSTACK-9633: Revert addition of `vhd` extention ...

Posted by Abhinandan Prateek <ab...@shapeblue.com>.
Syed,

  Any test that is calling is_snapshot_on_nfs from utils.py will fail for Xen as it ends up appending an additional “.vhd” to the install_path. There are several of them both in smoke and component folder. The reason this is under scrutiny is die to marvin test failures.



On 06/12/16, 9:33 AM, "Syed Ahmed" <sa...@cloudops.com> wrote:

>Yes this will break managed snapshot archives. I want to understand what
>this revert is trying to fix and what tests were failing.
>On Mon, Dec 5, 2016 at 22:38 abhinandanprateek <gi...@git.apache.org> wrote:
>
>> Github user abhinandanprateek commented on the issue:
>>
>>     https://github.com/apache/cloudstack/pull/1807
>>
>>     @syed can you look at this 'revert' i guess this is going to be an
>> issue with the managed storage change that has been put in. cc @rhtyd
>>
>>
>> ---
>> 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.
>> ---
>>

abhinandan.prateek@shapeblue.com 
www.shapeblue.com
53 Chandos Place, Covent Garden, London  WC2N 4HSUK
@shapeblue
  
 


Re: [GitHub] cloudstack issue #1807: CLOUDSTACK-9633: Revert addition of `vhd` extention ...

Posted by Syed Ahmed <sa...@cloudops.com>.
Yes this will break managed snapshot archives. I want to understand what
this revert is trying to fix and what tests were failing.
On Mon, Dec 5, 2016 at 22:38 abhinandanprateek <gi...@git.apache.org> wrote:

> Github user abhinandanprateek commented on the issue:
>
>     https://github.com/apache/cloudstack/pull/1807
>
>     @syed can you look at this 'revert' i guess this is going to be an
> issue with the managed storage change that has been put in. cc @rhtyd
>
>
> ---
> 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 #1807: CLOUDSTACK-9633: Revert addition of `vhd` extention ...

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

    https://github.com/apache/cloudstack/pull/1807
  
    @syed can you look at this 'revert' i guess this is going to be an issue with the managed storage change that has been put in. cc @rhtyd 


---
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 #1807: CLOUDSTACK-9633: Revert addition of `vhd` extention ...

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

    https://github.com/apache/cloudstack/pull/1807
  
    Pinging for review -- @jburwell @abhinandanprateek @borisstoyanov 
    @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 #1807: CLOUDSTACK-9633: Revert addition of `vhd` extention ...

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

    https://github.com/apache/cloudstack/pull/1807
  
    @syed Thanks, so just to confirm... with this PR, is your work with #1600 works and is not broken?


---
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 #1807: CLOUDSTACK-9633: Revert addition of `vhd` extention ...

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

    https://github.com/apache/cloudstack/pull/1807
  
    @blueorangutan test centos7 xenserver-65sp1



---
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 #1807: CLOUDSTACK-9633: Revert addition of `vhd` extention ...

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

    https://github.com/apache/cloudstack/pull/1807
  
    @blueorangutan test centos7 xenserver-65sp1


---
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 #1807: CLOUDSTACK-9633: Revert addition of `vhd` extention ...

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

    https://github.com/apache/cloudstack/pull/1807
  
    @syed please do a check of various snapshot functions on a upgraded setup. 
    
    @rhtyd this PR can be closed.


---
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 #1807: CLOUDSTACK-9633: Revert addition of `vhd` extention ...

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

    https://github.com/apache/cloudstack/pull/1807
  
    @syed yes I assume that PR in itself is good. You need to consider the broken test and broken upgrade ,where upgrading users will have path without the extension.


---
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 #1807: CLOUDSTACK-9633: Revert addition of `vhd` ext...

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

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


---
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 #1807: CLOUDSTACK-9633: Revert addition of `vhd` ext...

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

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


---
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 #1807: CLOUDSTACK-9633: Revert addition of `vhd` extention ...

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

    https://github.com/apache/cloudstack/pull/1807
  
    LGTM


---
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 #1807: CLOUDSTACK-9633: Revert addition of `vhd` extention ...

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

    https://github.com/apache/cloudstack/pull/1807
  
    @syed @swill the change is good. What needs to be considered is what happens with old snapshots, from a older version upgraded to this version, that are without the .vhd extension. If it can be checked that in a upgraded version where the older snapshots are without .vhd extension and new ones with .vhd extension, that cloudstack operations will work fine. Otherwise during upgrade we need to add .vhd to older snapshots for Xen.
    Another thing is that the marvin tests still assume that there is no .vhd extension and adds it. This needs to be fixed. We were seeing some marvin test fail on Xenserver due to a additonal .vhd extension. Look at the test_01_snapshot_root_disk  failure for Xen in this PR for example: https://github.com/apache/cloudstack/pull/1754 and the cause that @serg38  pointed out.


---
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 #1807: CLOUDSTACK-9633: Revert addition of `vhd` extention ...

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

    https://github.com/apache/cloudstack/pull/1807
  
    @abhinandanprateek the snapshot path is something which is internal to Cloudstack, as an end user, I would not be able to tell the difference before and after the upgrade, unless I missed something. 


---
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.
---