[GitHub] niteshsarda commented on issue #2373: CLOUDSTACK-10202:createSnapshotPolicy API create multiple entries in DB for same volume.
niteshsarda commented on issue #2373: CLOUDSTACK-10202:createSnapshotPolicy API create multiple entries in DB for same volume. URL: https://github.com/apache/cloudstack/pull/2373#issuecomment-354246593 @rhtyd : Can you please re-run the test cases again ? This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] blueorangutan commented on issue #2344: CLOUDSTACK-10163: Component tests sanity
blueorangutan commented on issue #2344: CLOUDSTACK-10163: Component tests sanity URL: https://github.com/apache/cloudstack/pull/2344#issuecomment-354239316 Trillian test result (tid-1912) Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7 Total time taken: 249025 seconds Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr2344-t1912-kvm-centos7.zip Intermitten failure detected: /marvin/tests/smoke/test_deploy_virtio_scsi_vm.py Intermitten failure detected: /marvin/tests/smoke/test_password_server.py Intermitten failure detected: /marvin/tests/smoke/test_ssvm.py Smoke tests completed. 66 look OK, 0 have error(s) Only failed tests results shown below: Test | Result | Time (s) | Test File --- | --- | --- | --- Additional tests completed. 88 look ok, 41 have error(s) Test | Result | Time (s) | Test File --- | --- | --- | --- ContextSuite context=TestBrowseUploadVolume>:setup | `Error` | 0.00 | test_browse_volumes.py test_05_create_delete_lbrule_forvpc | `Error` | 51.98 | test_escalations_ipaddresses.py test_10_create_delete_portforwarding_forvpc | `Error` | 77.88 | test_escalations_ipaddresses.py test_15_enable_disable_staticnat_forvpc | `Error` | 51.92 | test_escalations_ipaddresses.py ContextSuite context=TestIpAddresses>:teardown | `Error` | 1.08 | test_escalations_ipaddresses.py test_02_internallb_rules_traffic | `Failure` | 655.07 | test_vpc_network_internal_lbrules.py test_03_internallb_rules_vpc_network_restarts_traffic | `Failure` | 484.25 | test_vpc_network_internal_lbrules.py test_04_internallb_appliance_operations_traffic | `Error` | 2084.74 | test_vpc_network_internal_lbrules.py test_01_list_templates_pagination | `Error` | 376.88 | test_escalations_templates.py test_02_download_template | `Error` | 516.10 | test_escalations_templates.py test_stop_start_vpc_router | `Failure` | 650.35 | test_vpc_network.py test_03_restart_vpcvr | `Failure` | 956.91 | test_vpc_network.py test_01_list_vpncustomergateways_pagination | `Error` | 0.13 | test_escalations_vpncustomergateways.py test_02_update_vpncustomergateways | `Error` | 5.14 | test_escalations_vpncustomergateways.py test_03_network_create | `Error` | 212.84 | test_project_resources.py test_operations_non_root_admin_api_client_2_SHARED | `Failure` | 1.18 | test_multiple_ips_per_nic.py test_disable_static_nat_2_SHARED | `Failure` | 0.93 | test_multiple_ips_per_nic.py test_network_restart_cleanup_true_2_SHARED | `Failure` | 0.87 | test_multiple_ips_per_nic.py test_reboot_router_VM_3_VPC | `Error` | 735.03 | test_multiple_ips_per_nic.py test_recover_vm_2_SHARED | `Failure` | 0.89 | test_multiple_ips_per_nic.py test_02_add_nw_stopped_vm_2_shared | `Error` | 131.11 | test_add_remove_network.py test_03_add_nw_multiple_times_2_shared | `Error` | 30.95 | test_add_remove_network.py test_06_add_nw_ipaddress_running_vm | `Error` | 35.88 | test_add_remove_network.py test_14_add_nw_different_account_1_isolated | `Failure` | 56.39 | test_add_remove_network.py test_14_add_nw_different_account_2_shared | `Failure` | 202.55 | test_add_remove_network.py test_14_add_nw_different_account_2_shared | `Error` | 212.79 | test_add_remove_network.py ContextSuite context=TestAddNetworkToVirtualMachine>:teardown | `Error` | 184.41 | test_add_remove_network.py test_01_reset_keypair_normal_user | `Failure` | 892.78 | test_reset_ssh_keypair.py test_02_reset_keypair_domain_admin | `Failure` | 827.18 | test_reset_ssh_keypair.py test_03_reset_keypair_root_admin | `Failure` | 877.57 | test_reset_ssh_keypair.py test_01_reset_ssh_keys | `Failure` | 874.09 | test_reset_ssh_keypair.py test_02_reset_ssh_key_password_enabled_template | `Failure` | 781.89 | test_reset_ssh_keypair.py test_03_reset_ssh_with_no_key | `Failure` | 786.55 | test_reset_ssh_keypair.py test_04_reset_key_passwd_enabled_no_key | `Failure` | 781.06 | test_reset_ssh_keypair.py test_01_host_ha_with_nfs_storagepool_with_vm | `Error` | 63.58 | test_host.py test_06_release_ip | `Failure` | 729.63 | test_haproxy.py test_07_restart_network_vm_running | `Failure` | 850.07 | test_vpc.py test_08_delete_vpc | `Failure` | 771.46 | test_vpc.py test_03_vmsnapshot__on_resized_rootvolume_vm | `Error` | 172.98 | test_rootvolume_resize.py test_04_vmreset_after_migrate_vm__rootvolume_resized | `Error` | 86.77 | test_rootvolume_resize.py test_22_disablePod_admin_deployVM | `Error` | 5.24 | test_organization_states.py test_42_disableHost_admin_deployVM | `Error` | 10.24 | test_organization_states.py test_43_disableHost_admin_deployVM | `Failure` | 5.21 | test_organization_states.py test_44_disableHost_admin_stop_startVM | `Failure` | 40.50 | test_organization_states.py test_46_disableHost_user_stop_startVM | `Failure` | 131.37 | test_organization_states.py
[GitHub] niteshsarda commented on a change in pull request #2373: CLOUDSTACK-10202:createSnapshotPolicy API create multiple entries in DB for same volume.
niteshsarda commented on a change in pull request #2373: CLOUDSTACK-10202:createSnapshotPolicy API create multiple entries in DB for same volume. URL: https://github.com/apache/cloudstack/pull/2373#discussion_r158901278 ## File path: server/src/com/cloud/storage/snapshot/SnapshotManagerImpl.java ## @@ -854,33 +856,39 @@ public SnapshotPolicyVO createPolicy(CreateSnapshotPolicyCmd cmd, Account policy throw new InvalidParameterValueException("Max number of snapshots shouldn't exceed the " + message + " level snapshot limit"); } } -SnapshotPolicyVO policy = _snapshotPolicyDao.findOneByVolumeInterval(volumeId, intvType); -if (policy == null) { -policy = new SnapshotPolicyVO(volumeId, cmd.getSchedule(), timezoneId, intvType, cmd.getMaxSnaps(), display); -policy = _snapshotPolicyDao.persist(policy); -_snapSchedMgr.scheduleNextSnapshotJob(policy); -} else { + +final GlobalLock createSnapshotPolicyLock = GlobalLock.getInternLock("createSnapshotPolicy_" + volumeId); +boolean isLockAcquired = createSnapshotPolicyLock.lock(5); +if (isLockAcquired) { +s_logger.debug("Acquired lock for creating snapshot policy of volume : " + volume.getName()); try { -boolean previousDisplay = policy.isDisplay(); -policy = _snapshotPolicyDao.acquireInLockTable(policy.getId()); -policy.setSchedule(cmd.getSchedule()); -policy.setTimezone(timezoneId); -policy.setInterval((short)intvType.ordinal()); -policy.setMaxSnaps(cmd.getMaxSnaps()); -policy.setActive(true); -policy.setDisplay(display); -_snapshotPolicyDao.update(policy.getId(), policy); - _snapSchedMgr.scheduleOrCancelNextSnapshotJobOnDisplayChange(policy, previousDisplay); -} finally { -if (policy != null) { -_snapshotPolicyDao.releaseFromLockTable(policy.getId()); +policy = _snapshotPolicyDao.findOneByVolumeInterval(volumeId, intvType); +if (policy == null) { +policy = new SnapshotPolicyVO(volumeId, cmd.getSchedule(), timezoneId, intvType, cmd.getMaxSnaps(), display); +policy = _snapshotPolicyDao.persist(policy); +_snapSchedMgr.scheduleNextSnapshotJob(policy); +} else { +boolean previousDisplay = policy.isDisplay(); +policy.setSchedule(cmd.getSchedule()); +policy.setTimezone(timezoneId); +policy.setInterval((short)intvType.ordinal()); +policy.setMaxSnaps(cmd.getMaxSnaps()); +policy.setActive(true); +policy.setDisplay(display); +_snapshotPolicyDao.update(policy.getId(), policy); + _snapSchedMgr.scheduleOrCancelNextSnapshotJobOnDisplayChange(policy, previousDisplay); } +} finally { +createSnapshotPolicyLock.unlock(); } +// TODO - Make createSnapshotPolicy - BaseAsyncCreate and remove this. +CallContext.current().putContextParameter(SnapshotPolicy.class, policy.getUuid()); +return policy; +} else { +s_logger.debug("Unable to acquire lock for creating snapshot policy of volume : " + volume.getName()); Review comment: @marcaurele : Changed the log level to warn. Please check latest code. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] marcaurele commented on a change in pull request #2373: CLOUDSTACK-10202:createSnapshotPolicy API create multiple entries in DB for same volume.
marcaurele commented on a change in pull request #2373: CLOUDSTACK-10202:createSnapshotPolicy API create multiple entries in DB for same volume. URL: https://github.com/apache/cloudstack/pull/2373#discussion_r158900038 ## File path: server/src/com/cloud/storage/snapshot/SnapshotManagerImpl.java ## @@ -854,33 +856,39 @@ public SnapshotPolicyVO createPolicy(CreateSnapshotPolicyCmd cmd, Account policy throw new InvalidParameterValueException("Max number of snapshots shouldn't exceed the " + message + " level snapshot limit"); } } -SnapshotPolicyVO policy = _snapshotPolicyDao.findOneByVolumeInterval(volumeId, intvType); -if (policy == null) { -policy = new SnapshotPolicyVO(volumeId, cmd.getSchedule(), timezoneId, intvType, cmd.getMaxSnaps(), display); -policy = _snapshotPolicyDao.persist(policy); -_snapSchedMgr.scheduleNextSnapshotJob(policy); -} else { + +final GlobalLock createSnapshotPolicyLock = GlobalLock.getInternLock("createSnapshotPolicy_" + volumeId); +boolean isLockAcquired = createSnapshotPolicyLock.lock(5); +if (isLockAcquired) { +s_logger.debug("Acquired lock for creating snapshot policy of volume : " + volume.getName()); try { -boolean previousDisplay = policy.isDisplay(); -policy = _snapshotPolicyDao.acquireInLockTable(policy.getId()); -policy.setSchedule(cmd.getSchedule()); -policy.setTimezone(timezoneId); -policy.setInterval((short)intvType.ordinal()); -policy.setMaxSnaps(cmd.getMaxSnaps()); -policy.setActive(true); -policy.setDisplay(display); -_snapshotPolicyDao.update(policy.getId(), policy); - _snapSchedMgr.scheduleOrCancelNextSnapshotJobOnDisplayChange(policy, previousDisplay); -} finally { -if (policy != null) { -_snapshotPolicyDao.releaseFromLockTable(policy.getId()); +policy = _snapshotPolicyDao.findOneByVolumeInterval(volumeId, intvType); +if (policy == null) { +policy = new SnapshotPolicyVO(volumeId, cmd.getSchedule(), timezoneId, intvType, cmd.getMaxSnaps(), display); +policy = _snapshotPolicyDao.persist(policy); +_snapSchedMgr.scheduleNextSnapshotJob(policy); +} else { +boolean previousDisplay = policy.isDisplay(); +policy.setSchedule(cmd.getSchedule()); +policy.setTimezone(timezoneId); +policy.setInterval((short)intvType.ordinal()); +policy.setMaxSnaps(cmd.getMaxSnaps()); +policy.setActive(true); +policy.setDisplay(display); +_snapshotPolicyDao.update(policy.getId(), policy); + _snapSchedMgr.scheduleOrCancelNextSnapshotJobOnDisplayChange(policy, previousDisplay); } +} finally { +createSnapshotPolicyLock.unlock(); } +// TODO - Make createSnapshotPolicy - BaseAsyncCreate and remove this. +CallContext.current().putContextParameter(SnapshotPolicy.class, policy.getUuid()); +return policy; +} else { +s_logger.debug("Unable to acquire lock for creating snapshot policy of volume : " + volume.getName()); Review comment: This log should be move to the `warn` level This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] blueorangutan commented on issue #2360: [CLOUDSTACK-10189] Adding nuage VSD managed network support to CloudStack.
blueorangutan commented on issue #2360: [CLOUDSTACK-10189] Adding nuage VSD managed network support to CloudStack. URL: https://github.com/apache/cloudstack/pull/2360#issuecomment-354162092 Trillian test result (tid-1934) Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7 Total time taken: 30678 seconds Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr2360-t1934-kvm-centos7.zip Intermitten failure detected: /marvin/tests/smoke/test_host_maintenance.py Smoke tests completed. 65 look OK, 1 have error(s) Only failed tests results shown below: Test | Result | Time (s) | Test File --- | --- | --- | --- test_02_cancel_host_maintenace_with_migration_jobs | `Error` | 5.53 | test_host_maintenance.py This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] khos2ow commented on a change in pull request #2365: CLOUDSTACK-10197: Rename xentools iso for XenServer 7.0+
khos2ow commented on a change in pull request #2365: CLOUDSTACK-10197: Rename xentools iso for XenServer 7.0+ URL: https://github.com/apache/cloudstack/pull/2365#discussion_r158855295 ## File path: plugins/hypervisors/xenserver/src/com/cloud/hypervisor/xenserver/discoverer/XcpServerDiscoverer.java ## @@ -529,13 +540,12 @@ public boolean processCommands(long agentId, long seq, Command[] commands) { } private void createXsToolsISO() { -String isoName = "xs-tools.iso"; -VMTemplateVO tmplt = _tmpltDao.findByTemplateName(isoName); +VMTemplateVO tmplt = _tmpltDao.findByTemplateName(_toolsIsoName); Review comment: That's what I was trying to do but the way I did it was wrong, hence the first option mentioned above: > we can register both xs-tools.iso and guest-tools.iso and ... later on (for example in createServerResource method) delete/deactivate the wrong iso This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] blueorangutan commented on issue #2035: CLOUDSTACK-9867:VM snapshot on primary storage usage metrics
blueorangutan commented on issue #2035: CLOUDSTACK-9867:VM snapshot on primary storage usage metrics URL: https://github.com/apache/cloudstack/pull/2035#issuecomment-354153798 Trillian test result (tid-1932) Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7 Total time taken: 32798 seconds Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr2035-t1932-kvm-centos7.zip Intermitten failure detected: /marvin/tests/smoke/test_deploy_virtio_scsi_vm.py Intermitten failure detected: /marvin/tests/smoke/test_internal_lb.py Intermitten failure detected: /marvin/tests/smoke/test_vpc_redundant.py Smoke tests completed. 64 look OK, 2 have error(s) Only failed tests results shown below: Test | Result | Time (s) | Test File --- | --- | --- | --- test_04_rvpc_internallb_haproxy_stats_on_all_interfaces | `Error` | 199.79 | test_internal_lb.py test_01_create_redundant_VPC_2tiers_4VMs_4IPs_4PF_ACL | `Failure` | 386.17 | test_vpc_redundant.py This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] blueorangutan commented on issue #2352: CLOUDSTACK-10175: prevent VPC list leakage
blueorangutan commented on issue #2352: CLOUDSTACK-10175: prevent VPC list leakage URL: https://github.com/apache/cloudstack/pull/2352#issuecomment-354142189 Trillian test result (tid-1931) Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7 Total time taken: 31360 seconds Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr2352-t1931-kvm-centos7.zip Intermitten failure detected: /marvin/tests/smoke/test_outofbandmanagement.py Smoke tests completed. 66 look OK, 0 have error(s) Only failed tests results shown below: Test | Result | Time (s) | Test File --- | --- | --- | --- This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] PranaliM commented on issue #2282: CLOUDSTACK-10104:Optimize database transactions in ListDomain API to improve performance
PranaliM commented on issue #2282: CLOUDSTACK-10104:Optimize database transactions in ListDomain API to improve performance URL: https://github.com/apache/cloudstack/pull/2282#issuecomment-354139467 I have fixed the conflicts @rhtyd This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] nitin-maharana commented on a change in pull request #2340: CLOUDSTACK-10106: GPU/vGPU Support on VMware
nitin-maharana commented on a change in pull request #2340: CLOUDSTACK-10106: GPU/vGPU Support on VMware URL: https://github.com/apache/cloudstack/pull/2340#discussion_r158800362 ## File path: vmware-base/src/com/cloud/hypervisor/vmware/mo/HostMO.java ## @@ -1184,4 +1206,261 @@ public ManagedObjectReference waitForPortGroup(String networkName, long timeOutM } return morNetwork; } + +public ManagedObjectReference getComputeResourceEnvironmentBrowser() throws Exception { +ManagedObjectReference morParent = getParentMor(); +ClusterMO clusterMo = new ClusterMO(_context, morParent); +return clusterMo.getComputeResourceEnvironmentBrowser(); +} + +public VirtualMachinePciPassthroughInfo getHostPciDeviceInfo(final String pciDeviceId) throws Exception { +VirtualMachinePciPassthroughInfo matchingPciPassthroughDevice = null; +ConfigTarget configTarget = _context.getService().queryConfigTarget(getComputeResourceEnvironmentBrowser(), _mor); +List pciPassthroughDevices = configTarget.getPciPassthrough(); +for (VirtualMachinePciPassthroughInfo pciPassthroughDevice : pciPassthroughDevices) { +HostPciDevice hostPciDevice = pciPassthroughDevice.getPciDevice(); +if (pciDeviceId.equals(hostPciDevice.getId())) { +matchingPciPassthroughDevice = pciPassthroughDevice; +break; +} +} +return matchingPciPassthroughDevice; +} + +public VirtualDevice prepareSharedPciPassthroughDevice(final String vGpuProfile) { +s_logger.debug("Preparing shared PCI device"); +VirtualPCIPassthrough virtualPciPassthrough = new VirtualPCIPassthrough(); +VirtualPCIPassthroughVmiopBackingInfo virtualPCIPassthroughVmiopBackingInfo = new VirtualPCIPassthroughVmiopBackingInfo(); +virtualPCIPassthroughVmiopBackingInfo.setVgpu(vGpuProfile); + virtualPciPassthrough.setBacking(virtualPCIPassthroughVmiopBackingInfo); +Description description = new Description(); +description.setLabel("vGPU device"); +description.setSummary("vGPU type: " + vGpuProfile); +virtualPciPassthrough.setDeviceInfo(description); +return virtualPciPassthrough; +} + +public VirtualDevice prepareDirectPciPassthroughDevice(final VirtualMachinePciPassthroughInfo hostPciDeviceInfo) { +// Ex: pciDeviceId is like ":08:00.0" composed of bus,slot,function +s_logger.debug("Preparing direct PCI device"); + +VirtualPCIPassthrough pciDevice = new VirtualPCIPassthrough(); +VirtualPCIPassthroughDeviceBackingInfo pciBacking = new VirtualPCIPassthroughDeviceBackingInfo(); +pciBacking.setId(hostPciDeviceInfo.getPciDevice().getId()); + pciBacking.setDeviceId(Integer.toHexString(hostPciDeviceInfo.getPciDevice().getDeviceId())); + pciBacking.setDeviceName(hostPciDeviceInfo.getPciDevice().getDeviceName()); +pciBacking.setVendorId(hostPciDeviceInfo.getPciDevice().getVendorId()); +pciBacking.setSystemId(hostPciDeviceInfo.getSystemId()); +pciDevice.setBacking(pciBacking); +return pciDevice; +} + +public String getPciIdForAvailableDirectPciPassthroughDevice() throws Exception { +String pciId = ""; +List hostGraphicsInfos = getHostGraphicsInfo(); +for (HostGraphicsInfo hostGraphicsInfo : hostGraphicsInfos) { +if (GPU.GPUType.direct.toString().equalsIgnoreCase(hostGraphicsInfo.getGraphicsType())) { +List vms = hostGraphicsInfo.getVm(); +if (CollectionUtils.isEmpty(vms)) { +pciId = hostGraphicsInfo.getPciId(); +break; +} +} +} +return pciId; +} + +/** + * It updates the info of each vGPU type in the NVidia GRID K1/K2 Card. + * @param gpuCapacity (The output is stored in this) + * @param groupName - (NVIDIAGRID K1 or NVIDIAGRID K2) + * @param countGridKSharedGPUs (Number of Enabled shared GPUs) + * @param graphicsInfo (Info regarding the card) + * @param sharedPassthruGpuTypes (All the enabled vGPU types) + * @param gridKGPUMemory (Video RAM of each GPU in the card) + * @throws Exception + */ +private void updateGpuCapacities(final HashMapgpuCapacity, final String groupName, final long countGridKSharedGPUs, final List graphicsInfo, final List sharedPassthruGpuTypes, final long gridKGPUMemory) throws Exception { +/* + * 0 - grid_k100 or grid_k200 + * 1 - grid_k120q or grid_k220q + * 2 - grid_k140q or grid_k240q + * 3 - grid_k160q or grid_k260q + * 4 - grid_k180q or grid_k280q + */ +final long remainingCapacities[] = new long[5]; + +remainingCapacities[0] = 8l * countGridKSharedGPUs; +remainingCapacities[1] = 8l *
[GitHub] sgoeminn commented on issue #2360: [CLOUDSTACK-10189] Adding nuage VSD managed network support to CloudStack.
sgoeminn commented on issue #2360: [CLOUDSTACK-10189] Adding nuage VSD managed network support to CloudStack. URL: https://github.com/apache/cloudstack/pull/2360#issuecomment-354098151 @borisstoyanov I added the JIRA ticket to the title. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] DaanHoogland opened a new pull request #2378: CLOUDSTACK-10205 LinkDomainToLdap returns UUID instead of internal id
DaanHoogland opened a new pull request #2378: CLOUDSTACK-10205 LinkDomainToLdap returns UUID instead of internal id URL: https://github.com/apache/cloudstack/pull/2378 the internal id is not usefull to the user. It is a bug to return it instead of a uuid. In the process of fixing the above "name" was deprecated in favour of "ldap_domain". This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] niteshsarda commented on a change in pull request #2373: CLOUDSTACK-10202:createSnapshotPolicy API create multiple entries in DB for same volume.
niteshsarda commented on a change in pull request #2373: CLOUDSTACK-10202:createSnapshotPolicy API create multiple entries in DB for same volume. URL: https://github.com/apache/cloudstack/pull/2373#discussion_r158793730 ## File path: server/src/com/cloud/storage/snapshot/SnapshotManagerImpl.java ## @@ -854,15 +856,18 @@ public SnapshotPolicyVO createPolicy(CreateSnapshotPolicyCmd cmd, Account policy throw new InvalidParameterValueException("Max number of snapshots shouldn't exceed the " + message + " level snapshot limit"); } } -SnapshotPolicyVO policy = _snapshotPolicyDao.findOneByVolumeInterval(volumeId, intvType); -if (policy == null) { -policy = new SnapshotPolicyVO(volumeId, cmd.getSchedule(), timezoneId, intvType, cmd.getMaxSnaps(), display); -policy = _snapshotPolicyDao.persist(policy); -_snapSchedMgr.scheduleNextSnapshotJob(policy); -} else { -try { + +final GlobalLock createSnapshotPolicyLock = GlobalLock.getInternLock("createSnapshotPolicy_" + volumeId); +createSnapshotPolicyLock.lock(5); Review comment: @marcaurele : Incorporated the changes suggested by you for handling the condition of lock not acquired. In case if lock is not acquired, null value will be returned. In CreateSnapshotPolicyCmd class, inside execute method from where this method is called, we have already handled the null condition, and in case of null value exception will be thrown. Please review the latest code. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] rafaelweingartner commented on a change in pull request #1740: CLOUDSTACK-9572 Snapshot on primary storage not cleaned up after Stor?
rafaelweingartner commented on a change in pull request #1740: CLOUDSTACK-9572 Snapshot on primary storage not cleaned up after Stor? URL: https://github.com/apache/cloudstack/pull/1740#discussion_r158788814 ## File path: server/src/com/cloud/storage/snapshot/SnapshotManagerImpl.java ## @@ -1331,6 +1331,21 @@ public boolean canOperateOnVolume(Volume volume) { return true; } +@Override +public void cleanupSnapshotsByVolume(Long volumeId) { +List infos = snapshotFactory.getSnapshots(volumeId, DataStoreRole.Primary); +for(SnapshotInfo info: infos) { +try { + if(info != null) { Review comment: Do you need this null check? This only happen if one uses the `set` method of List object to replace a position with a `Null` value This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] yvsubhash commented on a change in pull request #2350: Cloudstack 10170 - fixes resource tags security bugs and adds account tags support
yvsubhash commented on a change in pull request #2350: Cloudstack 10170 - fixes resource tags security bugs and adds account tags support URL: https://github.com/apache/cloudstack/pull/2350#discussion_r158785251 ## File path: server/src/com/cloud/tags/TaggedResourceManagerImpl.java ## @@ -279,83 +308,77 @@ public void doInTransactionWithoutResult(TransactionStatus status) { return resourceTags; } -@Override -public String getUuid(String resourceId, ResourceObjectType resourceType) { -if (!StringUtils.isNumeric(resourceId)) { -return resourceId; -} - -Class clazz = s_typeMap.get(resourceType); - -Object entity = _entityMgr.findById(clazz, resourceId); -if (entity != null && entity instanceof Identity) { -return ((Identity)entity).getUuid(); - } +private List searchResourceTags(List resourceIds, ResourceObjectType resourceType) { +List resourceUuids = resourceIds.stream().map(resourceId -> getUuid(resourceId, resourceType)).collect(Collectors.toList()); +SearchBuilder sb = _resourceTagDao.createSearchBuilder(); +sb.and("resourceUuid", sb.entity().getResourceUuid(), SearchCriteria.Op.IN); +sb.and("resourceType", sb.entity().getResourceType(), SearchCriteria.Op.EQ); - return resourceId; - } +SearchCriteria sc = sb.create(); +sc.setParameters("resourceUuid", resourceUuids.toArray()); +sc.setParameters("resourceType", resourceType); +return _resourceTagDao.search(sc, null); +} @Override @DB @ActionEvent(eventType = EventTypes.EVENT_TAGS_DELETE, eventDescription = "deleting resource tags") public boolean deleteTags(List resourceIds, ResourceObjectType resourceType, Maptags) { Account caller = CallContext.current().getCallingAccount(); - -SearchBuilder sb = _resourceTagDao.createSearchBuilder(); -sb.and().op("resourceId", sb.entity().getResourceId(), SearchCriteria.Op.IN); -sb.or("resourceUuid", sb.entity().getResourceUuid(), SearchCriteria.Op.IN); -sb.cp(); -sb.and("resourceType", sb.entity().getResourceType(), SearchCriteria.Op.EQ); - -SearchCriteria sc = sb.create(); -sc.setParameters("resourceId", resourceIds.toArray()); -sc.setParameters("resourceUuid", resourceIds.toArray()); -sc.setParameters("resourceType", resourceType); - -List resourceTags = _resourceTagDao.search(sc, null); -; -final List tagsToRemove = new ArrayList(); +if(s_logger.isDebugEnabled()) { +s_logger.debug("ResourceIds to Find " + String.join(", ", resourceIds)); +} +List resourceTags = searchResourceTags(resourceIds, resourceType); +final List tagsToDelete = new ArrayList<>(); // Finalize which tags should be removed for (ResourceTag resourceTag : resourceTags) { //1) validate the permissions +if(s_logger.isDebugEnabled()) { +s_logger.debug("Resource Tag Id: " + resourceTag.getResourceId()); +s_logger.debug("Resource Tag AccountId: " + resourceTag.getAccountId()); +} Account owner = _accountMgr.getAccount(resourceTag.getAccountId()); +if(s_logger.isDebugEnabled()) { +s_logger.debug("Resource Owner: " + owner); +} _accountMgr.checkAccess(caller, null, false, owner); //2) Only remove tag if it matches key value pairs -if (tags != null && !tags.isEmpty()) { +if (MapUtils.isEmpty(tags)) { +tagsToDelete.add(resourceTag); +} else { for (String key : tags.keySet()) { -boolean canBeRemoved = false; +boolean deleteTag = false; if (resourceTag.getKey().equalsIgnoreCase(key)) { String value = tags.get(key); if (value != null) { if (resourceTag.getValue().equalsIgnoreCase(value)) { -canBeRemoved = true; +deleteTag = true; } } else { -canBeRemoved = true; +deleteTag = true; } -if (canBeRemoved) { -tagsToRemove.add(resourceTag); +if (deleteTag) { +tagsToDelete.add(resourceTag); break; } } } -} else { -tagsToRemove.add(resourceTag); } } -if (tagsToRemove.isEmpty()) { -throw new InvalidParameterValueException("Unable
[GitHub] blueorangutan commented on issue #2035: CLOUDSTACK-9867:VM snapshot on primary storage usage metrics
blueorangutan commented on issue #2035: CLOUDSTACK-9867:VM snapshot on primary storage usage metrics URL: https://github.com/apache/cloudstack/pull/2035#issuecomment-354080953 @rhtyd a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] rhtyd commented on issue #2035: CLOUDSTACK-9867:VM snapshot on primary storage usage metrics
rhtyd commented on issue #2035: CLOUDSTACK-9867:VM snapshot on primary storage usage metrics URL: https://github.com/apache/cloudstack/pull/2035#issuecomment-354080905 @blueorangutan test This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] blueorangutan commented on issue #2035: CLOUDSTACK-9867:VM snapshot on primary storage usage metrics
blueorangutan commented on issue #2035: CLOUDSTACK-9867:VM snapshot on primary storage usage metrics URL: https://github.com/apache/cloudstack/pull/2035#issuecomment-354077301 Packaging result: ?centos6 ?centos7 ?debian. JID-1508 This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] blueorangutan commented on issue #2377: CLOUDSTACK-10207: updateVpnCustomerGateway: fix defaulting for option?
blueorangutan commented on issue #2377: CLOUDSTACK-10207: updateVpnCustomerGateway: fix defaulting for option? URL: https://github.com/apache/cloudstack/pull/2377#issuecomment-354075059 Packaging result: ?centos6 ?centos7 ?debian. JID-1507 This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] blueorangutan commented on issue #2102: CLOUDSTACK-9889 Dedication of guest vlan range to a domain
blueorangutan commented on issue #2102: CLOUDSTACK-9889 Dedication of guest vlan range to a domain URL: https://github.com/apache/cloudstack/pull/2102#issuecomment-354074892 Packaging result: ?centos6 ?centos7 ?debian. JID-1506 This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] blueorangutan commented on issue #2046: CLOUDSTACK-7958: Add configuration for limit to CIDRs for Admin API calls
blueorangutan commented on issue #2046: CLOUDSTACK-7958: Add configuration for limit to CIDRs for Admin API calls URL: https://github.com/apache/cloudstack/pull/2046#issuecomment-354074851 Packaging result: ?centos6 ?centos7 ?debian. JID-1505 This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] blueorangutan commented on issue #2035: CLOUDSTACK-9867:VM snapshot on primary storage usage metrics
blueorangutan commented on issue #2035: CLOUDSTACK-9867:VM snapshot on primary storage usage metrics URL: https://github.com/apache/cloudstack/pull/2035#issuecomment-354072935 @rhtyd a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] rhtyd commented on issue #2035: CLOUDSTACK-9867:VM snapshot on primary storage usage metrics
rhtyd commented on issue #2035: CLOUDSTACK-9867:VM snapshot on primary storage usage metrics URL: https://github.com/apache/cloudstack/pull/2035#issuecomment-354072828 @blueorangutan package This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] rhtyd commented on issue #2350: Cloudstack 10170 - fixes resource tags security bugs and adds account tags support
rhtyd commented on issue #2350: Cloudstack 10170 - fixes resource tags security bugs and adds account tags support URL: https://github.com/apache/cloudstack/pull/2350#issuecomment-354072725 @bwsw please fix the conflict, thanks. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] blueorangutan commented on issue #2352: CLOUDSTACK-10175: prevent VPC list leakage
blueorangutan commented on issue #2352: CLOUDSTACK-10175: prevent VPC list leakage URL: https://github.com/apache/cloudstack/pull/2352#issuecomment-354072682 @rhtyd a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] rhtyd commented on issue #2352: CLOUDSTACK-10175: prevent VPC list leakage
rhtyd commented on issue #2352: CLOUDSTACK-10175: prevent VPC list leakage URL: https://github.com/apache/cloudstack/pull/2352#issuecomment-354072615 @blueorangutan test This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services