You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cloudstack.apache.org by "SadiJr (via GitHub)" <gi...@apache.org> on 2023/02/15 16:46:41 UTC
[GitHub] [cloudstack] SadiJr opened a new pull request, #7239: [Veeam] Fix restore of reimported VMs
SadiJr opened a new pull request, #7239:
URL: https://github.com/apache/cloudstack/pull/7239
### Description
Using the VMware hypervisor with the Veeam plugin active, when restoring a VM that has been unmanaged from ACS and reimported, an error occurs because ACS duplicates the register in the database of this VM. This PR aims to fix this behavior.
### Types of changes
- [ ] Breaking change (fix or feature that would cause existing functionality to change)
- [ ] New feature (non-breaking change which adds functionality)
- [x] Bug fix (non-breaking change which fixes an issue)
- [ ] Enhancement (improves an existing feature and functionality)
- [ ] Cleanup (Code refactoring and cleanup, that may add test cases)
### Feature/Enhancement Scale or Bug Severity
#### Bug Severity
- [ ] BLOCKER
- [ ] Critical
- [ ] Major
- [x] Minor
- [ ] Trivial
### Screenshots (if appropriate):
### How Has This Been Tested?
It was tested in a local lab:
1. I created one new VM via ACS, unmanaged this VM, and then reimported this VM again to ACS;
2. I added this VM to one backup offering and made one manual backup;
3. I tried to restore this VM backup;
4. Before ACS failed to restore the VM and duplicate the VM in the UI and database;
5. ACS now completes the restore successfully, and there is no VM duplication.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] sonarcloud[bot] commented on pull request #7239: [Veeam] Fix restore of reimported VMs
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #7239:
URL: https://github.com/apache/cloudstack/pull/7239#issuecomment-1431778220
SonarCloud Quality Gate failed. [](https://sonarcloud.io/dashboard?id=apache_cloudstack&pullRequest=7239)
[](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=7239&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=7239&resolved=false&types=BUG) [1 Bug](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=7239&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=7239&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=7239&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=7239&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_cloudstack&pullRequest=7239&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_cloudstack&pullRequest=7239&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_cloudstack&pullRequest=7239&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=7239&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=7239&resolved=false&types=CODE_SMELL) [1 Code Smell](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=7239&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=7239&metric=new_coverage&view=list) [0.0% Coverage](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=7239&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=7239&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=7239&metric=new_duplicated_lines_density&view=list)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [Veeam] Fix restore of reimported VMs [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #7239:
URL: https://github.com/apache/cloudstack/pull/7239#issuecomment-2185110311
Packaging result [SF]: ✔️ el7 ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 10076
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] harikrishna-patnala commented on a diff in pull request #7239: [Veeam] Fix restore of reimported VMs
Posted by "harikrishna-patnala (via GitHub)" <gi...@apache.org>.
harikrishna-patnala commented on code in PR #7239:
URL: https://github.com/apache/cloudstack/pull/7239#discussion_r1247367912
##########
plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/guru/VMwareGuru.java:
##########
@@ -661,30 +660,29 @@ private Long getImportingVMTemplate(List<VirtualDisk> virtualDisks, DatacenterMO
* If VM exists: update VM
*/
private VMInstanceVO getVM(String vmInternalName, long templateId, long guestOsId, long serviceOfferingId, long zoneId, long accountId, long userId, long domainId) {
- s_logger.debug(String.format("Trying to get VM with specs: [vmInternalName: %s, templateId: %s, guestOsId: %s, serviceOfferingId: %s].", vmInternalName,
- templateId, guestOsId, serviceOfferingId));
- VMInstanceVO vm = virtualMachineDao.findVMByInstanceNameIncludingRemoved(vmInternalName);
- if (vm != null) {
- s_logger.debug(String.format("Found an existing VM [id: %s, removed: %s] with internalName: [%s].", vm.getUuid(), vm.getRemoved() != null ? "yes" : "no", vmInternalName));
- vm.setState(VirtualMachine.State.Stopped);
- vm.setPowerState(VirtualMachine.PowerState.PowerOff);
- virtualMachineDao.update(vm.getId(), vm);
- if (vm.getRemoved() != null) {
- virtualMachineDao.unremove(vm.getId());
- UsageEventUtils.publishUsageEvent(EventTypes.EVENT_VM_CREATE, accountId, vm.getDataCenterId(), vm.getId(), vm.getHostName(), vm.getServiceOfferingId(), vm.getTemplateId(),
- vm.getHypervisorType().toString(), VirtualMachine.class.getName(), vm.getUuid(), vm.isDisplayVm());
- }
- return virtualMachineDao.findById(vm.getId());
- } else {
+ s_logger.debug(String.format("Trying to get VM with specs: [vmInternalName: %s], and in states [%s, %s].", vmInternalName, VirtualMachine.State.Running, VirtualMachine.State.Stopped));
+ VMInstanceVO vm = virtualMachineDao.findVMInStatesAndWithInternalNameIncludingRemoved(vmInternalName, VirtualMachine.State.Running, VirtualMachine.State.Stopped);
+ if (vm == null) {
+ s_logger.debug(String.format("Cannot find any existing VM with internalName [%s] in any of these [%s, %s] states. Assuming VM is destroyed in ACS and recreated in restore process.",
+ vmInternalName, VirtualMachine.State.Running, VirtualMachine.State.Stopped));
+
long id = userVmDao.getNextInSequence(Long.class, "id");
s_logger.debug(String.format("Can't find an existing VM with internalName: [%s]. Creating a new VM with: [id: %s, name: %s, templateId: %s, guestOsId: %s, serviceOfferingId: %s].",
vmInternalName, id, vmInternalName, templateId, guestOsId, serviceOfferingId));
UserVmVO vmInstanceVO = new UserVmVO(id, vmInternalName, vmInternalName, templateId, HypervisorType.VMware, guestOsId, false, false, domainId, accountId, userId,
serviceOfferingId, null, null, null, vmInternalName);
vmInstanceVO.setDataCenterId(zoneId);
- return userVmDao.persist(vmInstanceVO);
+ vm = userVmDao.persist(vmInstanceVO);
+ UsageEventUtils.publishUsageEvent(EventTypes.EVENT_VM_CREATE, accountId, vm.getDataCenterId(), vm.getId(), vm.getHostName(), vm.getServiceOfferingId(), vm.getTemplateId(),
+ vm.getHypervisorType().toString(), VirtualMachine.class.getName(), vm.getUuid(), vm.isDisplayVm());
+ } else {
+ s_logger.debug(String.format("Found an existing VM [id: %s, removed: %s] with internalName [%s], and with state [%s].", vm.getUuid(), vm.getRemoved() != null ? "yes" : "no", vmInternalName, vm.getState()));
Review Comment:
This block will run only when findVMInStatesAndWithInternalNameIncludingRemoved() returns a VM, which means only when VMs in Stopped and Running states are found. am I right?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [Veeam] Fix restore of reimported VMs [cloudstack]
Posted by "sureshanaparti (via GitHub)" <gi...@apache.org>.
sureshanaparti commented on PR #7239:
URL: https://github.com/apache/cloudstack/pull/7239#issuecomment-1991301786
Hi @SadiJr Please resolve conflicts, and let me know if this PR can be targeted for 4.19.1 (if so, rebase with 4.19 branch).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] SadiJr commented on a diff in pull request #7239: [Veeam] Fix restore of reimported VMs
Posted by "SadiJr (via GitHub)" <gi...@apache.org>.
SadiJr commented on code in PR #7239:
URL: https://github.com/apache/cloudstack/pull/7239#discussion_r1198010189
##########
plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/guru/VMwareGuru.java:
##########
@@ -661,30 +660,29 @@ private Long getImportingVMTemplate(List<VirtualDisk> virtualDisks, DatacenterMO
* If VM exists: update VM
*/
private VMInstanceVO getVM(String vmInternalName, long templateId, long guestOsId, long serviceOfferingId, long zoneId, long accountId, long userId, long domainId) {
- s_logger.debug(String.format("Trying to get VM with specs: [vmInternalName: %s, templateId: %s, guestOsId: %s, serviceOfferingId: %s].", vmInternalName,
- templateId, guestOsId, serviceOfferingId));
- VMInstanceVO vm = virtualMachineDao.findVMByInstanceNameIncludingRemoved(vmInternalName);
- if (vm != null) {
- s_logger.debug(String.format("Found an existing VM [id: %s, removed: %s] with internalName: [%s].", vm.getUuid(), vm.getRemoved() != null ? "yes" : "no", vmInternalName));
- vm.setState(VirtualMachine.State.Stopped);
- vm.setPowerState(VirtualMachine.PowerState.PowerOff);
- virtualMachineDao.update(vm.getId(), vm);
- if (vm.getRemoved() != null) {
- virtualMachineDao.unremove(vm.getId());
- UsageEventUtils.publishUsageEvent(EventTypes.EVENT_VM_CREATE, accountId, vm.getDataCenterId(), vm.getId(), vm.getHostName(), vm.getServiceOfferingId(), vm.getTemplateId(),
- vm.getHypervisorType().toString(), VirtualMachine.class.getName(), vm.getUuid(), vm.isDisplayVm());
- }
- return virtualMachineDao.findById(vm.getId());
- } else {
+ s_logger.debug(String.format("Trying to get VM with specs: [vmInternalName: %s], and in states [%s, %s].", vmInternalName, VirtualMachine.State.Running, VirtualMachine.State.Stopped));
+ VMInstanceVO vm = virtualMachineDao.findVMInStatesAndWithInternalNameIncludingRemoved(vmInternalName, VirtualMachine.State.Running, VirtualMachine.State.Stopped);
+ if (vm == null) {
+ s_logger.debug(String.format("Cannot find any existing VM with internalName [%s] in any of this [%s, %s] states. Assuming VM is destroyed in ACS and recreated in restore process.",
Review Comment:
Done, thanks.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [Veeam] Fix restore of reimported VMs [cloudstack]
Posted by "JoaoJandre (via GitHub)" <gi...@apache.org>.
JoaoJandre commented on PR #7239:
URL: https://github.com/apache/cloudstack/pull/7239#issuecomment-2186441861
> > @weizhouapache that is strange. Your results differ a lot from mine. I review the changes in #8241, but, from what I understand, they should not impact this behavior. I will repeat my tests, using the most recent community version, this PR of mine and #8241, and I will return an opinion. Unfortunately, this may take some time as I am without my VMware environment at the moment.
>
> Hi @SadiJr Any update of the tests, Is it good for 4.19.1? cc @weizhouapache
@sureshanaparti I'll try to repeat the described tests using the community version and, if I see that the described problem remains, I will test this PR to see if it still fixes the issue after #8241 was merged.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [Veeam] Fix restore of reimported VMs [cloudstack]
Posted by "sureshanaparti (via GitHub)" <gi...@apache.org>.
sureshanaparti commented on PR #7239:
URL: https://github.com/apache/cloudstack/pull/7239#issuecomment-2185131937
@blueorangutan test alma9 vmware-70u3
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [Veeam] Fix restore of reimported VMs [cloudstack]
Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on PR #7239:
URL: https://github.com/apache/cloudstack/pull/7239#issuecomment-2058900341
ping @harikrishna-patnala
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] harikrishna-patnala commented on a diff in pull request #7239: [Veeam] Fix restore of reimported VMs
Posted by "harikrishna-patnala (via GitHub)" <gi...@apache.org>.
harikrishna-patnala commented on code in PR #7239:
URL: https://github.com/apache/cloudstack/pull/7239#discussion_r1247368828
##########
plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/guru/VMwareGuru.java:
##########
@@ -661,30 +660,29 @@ private Long getImportingVMTemplate(List<VirtualDisk> virtualDisks, DatacenterMO
* If VM exists: update VM
*/
private VMInstanceVO getVM(String vmInternalName, long templateId, long guestOsId, long serviceOfferingId, long zoneId, long accountId, long userId, long domainId) {
- s_logger.debug(String.format("Trying to get VM with specs: [vmInternalName: %s, templateId: %s, guestOsId: %s, serviceOfferingId: %s].", vmInternalName,
- templateId, guestOsId, serviceOfferingId));
- VMInstanceVO vm = virtualMachineDao.findVMByInstanceNameIncludingRemoved(vmInternalName);
- if (vm != null) {
- s_logger.debug(String.format("Found an existing VM [id: %s, removed: %s] with internalName: [%s].", vm.getUuid(), vm.getRemoved() != null ? "yes" : "no", vmInternalName));
- vm.setState(VirtualMachine.State.Stopped);
- vm.setPowerState(VirtualMachine.PowerState.PowerOff);
- virtualMachineDao.update(vm.getId(), vm);
- if (vm.getRemoved() != null) {
- virtualMachineDao.unremove(vm.getId());
- UsageEventUtils.publishUsageEvent(EventTypes.EVENT_VM_CREATE, accountId, vm.getDataCenterId(), vm.getId(), vm.getHostName(), vm.getServiceOfferingId(), vm.getTemplateId(),
- vm.getHypervisorType().toString(), VirtualMachine.class.getName(), vm.getUuid(), vm.isDisplayVm());
- }
- return virtualMachineDao.findById(vm.getId());
- } else {
+ s_logger.debug(String.format("Trying to get VM with specs: [vmInternalName: %s], and in states [%s, %s].", vmInternalName, VirtualMachine.State.Running, VirtualMachine.State.Stopped));
+ VMInstanceVO vm = virtualMachineDao.findVMInStatesAndWithInternalNameIncludingRemoved(vmInternalName, VirtualMachine.State.Running, VirtualMachine.State.Stopped);
+ if (vm == null) {
+ s_logger.debug(String.format("Cannot find any existing VM with internalName [%s] in any of these [%s, %s] states. Assuming VM is destroyed in ACS and recreated in restore process.",
+ vmInternalName, VirtualMachine.State.Running, VirtualMachine.State.Stopped));
+
long id = userVmDao.getNextInSequence(Long.class, "id");
s_logger.debug(String.format("Can't find an existing VM with internalName: [%s]. Creating a new VM with: [id: %s, name: %s, templateId: %s, guestOsId: %s, serviceOfferingId: %s].",
vmInternalName, id, vmInternalName, templateId, guestOsId, serviceOfferingId));
UserVmVO vmInstanceVO = new UserVmVO(id, vmInternalName, vmInternalName, templateId, HypervisorType.VMware, guestOsId, false, false, domainId, accountId, userId,
serviceOfferingId, null, null, null, vmInternalName);
vmInstanceVO.setDataCenterId(zoneId);
- return userVmDao.persist(vmInstanceVO);
+ vm = userVmDao.persist(vmInstanceVO);
+ UsageEventUtils.publishUsageEvent(EventTypes.EVENT_VM_CREATE, accountId, vm.getDataCenterId(), vm.getId(), vm.getHostName(), vm.getServiceOfferingId(), vm.getTemplateId(),
Review Comment:
any reason why the publishUsageEvent() method call is reversed in if else? Previously it is called when a VM entry is found.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #7239: [Veeam] Fix restore of reimported VMs
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #7239:
URL: https://github.com/apache/cloudstack/pull/7239#issuecomment-1432755462
Packaging result: :heavy_check_mark: el7 :heavy_check_mark: el8 :heavy_check_mark: el9 :heavy_check_mark: debian :heavy_check_mark: suse15. SL-JID 5592
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [Veeam] Fix restore of reimported VMs [cloudstack]
Posted by "SadiJr (via GitHub)" <gi...@apache.org>.
SadiJr commented on code in PR #7239:
URL: https://github.com/apache/cloudstack/pull/7239#discussion_r1423016788
##########
plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/guru/VMwareGuru.java:
##########
@@ -661,30 +660,29 @@ private Long getImportingVMTemplate(List<VirtualDisk> virtualDisks, DatacenterMO
* If VM exists: update VM
*/
private VMInstanceVO getVM(String vmInternalName, long templateId, long guestOsId, long serviceOfferingId, long zoneId, long accountId, long userId, long domainId) {
- s_logger.debug(String.format("Trying to get VM with specs: [vmInternalName: %s, templateId: %s, guestOsId: %s, serviceOfferingId: %s].", vmInternalName,
- templateId, guestOsId, serviceOfferingId));
- VMInstanceVO vm = virtualMachineDao.findVMByInstanceNameIncludingRemoved(vmInternalName);
- if (vm != null) {
- s_logger.debug(String.format("Found an existing VM [id: %s, removed: %s] with internalName: [%s].", vm.getUuid(), vm.getRemoved() != null ? "yes" : "no", vmInternalName));
- vm.setState(VirtualMachine.State.Stopped);
- vm.setPowerState(VirtualMachine.PowerState.PowerOff);
- virtualMachineDao.update(vm.getId(), vm);
- if (vm.getRemoved() != null) {
- virtualMachineDao.unremove(vm.getId());
- UsageEventUtils.publishUsageEvent(EventTypes.EVENT_VM_CREATE, accountId, vm.getDataCenterId(), vm.getId(), vm.getHostName(), vm.getServiceOfferingId(), vm.getTemplateId(),
- vm.getHypervisorType().toString(), VirtualMachine.class.getName(), vm.getUuid(), vm.isDisplayVm());
- }
- return virtualMachineDao.findById(vm.getId());
- } else {
+ s_logger.debug(String.format("Trying to get VM with specs: [vmInternalName: %s], and in states [%s, %s].", vmInternalName, VirtualMachine.State.Running, VirtualMachine.State.Stopped));
+ VMInstanceVO vm = virtualMachineDao.findVMInStatesAndWithInternalNameIncludingRemoved(vmInternalName, VirtualMachine.State.Running, VirtualMachine.State.Stopped);
+ if (vm == null) {
+ s_logger.debug(String.format("Cannot find any existing VM with internalName [%s] in any of these [%s, %s] states. Assuming VM is destroyed in ACS and recreated in restore process.",
+ vmInternalName, VirtualMachine.State.Running, VirtualMachine.State.Stopped));
+
long id = userVmDao.getNextInSequence(Long.class, "id");
s_logger.debug(String.format("Can't find an existing VM with internalName: [%s]. Creating a new VM with: [id: %s, name: %s, templateId: %s, guestOsId: %s, serviceOfferingId: %s].",
vmInternalName, id, vmInternalName, templateId, guestOsId, serviceOfferingId));
UserVmVO vmInstanceVO = new UserVmVO(id, vmInternalName, vmInternalName, templateId, HypervisorType.VMware, guestOsId, false, false, domainId, accountId, userId,
serviceOfferingId, null, null, null, vmInternalName);
vmInstanceVO.setDataCenterId(zoneId);
- return userVmDao.persist(vmInstanceVO);
+ vm = userVmDao.persist(vmInstanceVO);
+ UsageEventUtils.publishUsageEvent(EventTypes.EVENT_VM_CREATE, accountId, vm.getDataCenterId(), vm.getId(), vm.getHostName(), vm.getServiceOfferingId(), vm.getTemplateId(),
+ vm.getHypervisorType().toString(), VirtualMachine.class.getName(), vm.getUuid(), vm.isDisplayVm());
+ } else {
+ s_logger.debug(String.format("Found an existing VM [id: %s, removed: %s] with internalName [%s], and with state [%s].", vm.getUuid(), vm.getRemoved() != null ? "yes" : "no", vmInternalName, vm.getState()));
Review Comment:
Yes, but remember, if the VM is not in state `Stopped` or `Destroyed`, the restore fails at the begin:
https://github.com/apache/cloudstack/blob/031b4c675c2bbf76568e9cfa6653d4abc851563e/server/src/main/java/org/apache/cloudstack/backup/BackupManagerImpl.java#L591C27-L591C27
Please tell me if I misunderstood your question.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [Veeam] Fix restore of reimported VMs [cloudstack]
Posted by "weizhouapache (via GitHub)" <gi...@apache.org>.
weizhouapache commented on PR #7239:
URL: https://github.com/apache/cloudstack/pull/7239#issuecomment-2206959692
>
> Hey @sureshanaparti, @weizhouapache and @DaanHoogland
>
> After testing, I was unable to reproduce the issue on 4.19.1.0. It was probably fixed in #8241 or somewhere else; therefore, I'll be closing this PR.
@JoaoJandre
Thanks for your update
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [Veeam] Fix restore of reimported VMs [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #7239:
URL: https://github.com/apache/cloudstack/pull/7239#issuecomment-2185137963
@sureshanaparti a [SL] Trillian-Jenkins test job (alma9 mgmt + vmware-70u3) 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 to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #7239: [Veeam] Fix restore of reimported VMs
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #7239:
URL: https://github.com/apache/cloudstack/pull/7239#issuecomment-1432679417
@DaanHoogland a Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. 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 to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] weizhouapache commented on pull request #7239: [Veeam] Fix restore of reimported VMs
Posted by "weizhouapache (via GitHub)" <gi...@apache.org>.
weizhouapache commented on PR #7239:
URL: https://github.com/apache/cloudstack/pull/7239#issuecomment-1678814651
@SadiJr
if you want this in 4.18.1.0, can you rebase with 4.18 branch ? thanks
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [Veeam] Fix restore of reimported VMs [cloudstack]
Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on PR #7239:
URL: https://github.com/apache/cloudstack/pull/7239#issuecomment-1782496893
@SadiJr can you answer @harikrishna-patnala 's concerns?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [Veeam] Fix restore of reimported VMs [cloudstack]
Posted by "SadiJr (via GitHub)" <gi...@apache.org>.
SadiJr commented on PR #7239:
URL: https://github.com/apache/cloudstack/pull/7239#issuecomment-1847100306
@weizhouapache can you please share the state of VMs `admin-001-001` and `i-2-162-VM` in your database after doing your test? And about fixing an issue with Veeam by changing the unmanage/manage process, I think having two VMs with same `instance_name` in database is a inconsistency that is being generated by the manage/import VMware VMs. The internal name is expected to be unique.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [Veeam] Fix restore of reimported VMs [cloudstack]
Posted by "JoaoJandre (via GitHub)" <gi...@apache.org>.
JoaoJandre commented on PR #7239:
URL: https://github.com/apache/cloudstack/pull/7239#issuecomment-2206762330
> > > > > @weizhouapache that is strange. Your results differ a lot from mine. I review the changes in #8241, but, from what I understand, they should not impact this behavior. I will repeat my tests, using the most recent community version, this PR of mine and #8241, and I will return an opinion. Unfortunately, this may take some time as I am without my VMware environment at the moment.
> > > >
> > > >
> > > > Hi @SadiJr Any update of the tests, Is it good for 4.19.1? cc @weizhouapache
> > >
> > >
> > > @sureshanaparti I'll try to repeat the described tests using the community version and, if I see that the described problem remains, I will test this PR to see if it still fixes the issue after #8241 was merged.
> >
> >
> > cool, looking forward to your testing results @JoaoJandre
>
> Hi @JoaoJandre Can you confirm/share your test results please.
Hey @sureshanaparti, @weizhouapache and @DaanHoogland
After testing, I was unable to reproduce the issue on 4.19.1.0. It was probably fixed in #8241 or somewhere else; therefore, I'll be closing this PR.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [Veeam] Fix restore of reimported VMs [cloudstack]
Posted by "SadiJr (via GitHub)" <gi...@apache.org>.
SadiJr commented on code in PR #7239:
URL: https://github.com/apache/cloudstack/pull/7239#discussion_r1423016422
##########
plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/guru/VMwareGuru.java:
##########
@@ -661,30 +660,29 @@ private Long getImportingVMTemplate(List<VirtualDisk> virtualDisks, DatacenterMO
* If VM exists: update VM
*/
private VMInstanceVO getVM(String vmInternalName, long templateId, long guestOsId, long serviceOfferingId, long zoneId, long accountId, long userId, long domainId) {
- s_logger.debug(String.format("Trying to get VM with specs: [vmInternalName: %s, templateId: %s, guestOsId: %s, serviceOfferingId: %s].", vmInternalName,
- templateId, guestOsId, serviceOfferingId));
- VMInstanceVO vm = virtualMachineDao.findVMByInstanceNameIncludingRemoved(vmInternalName);
- if (vm != null) {
- s_logger.debug(String.format("Found an existing VM [id: %s, removed: %s] with internalName: [%s].", vm.getUuid(), vm.getRemoved() != null ? "yes" : "no", vmInternalName));
- vm.setState(VirtualMachine.State.Stopped);
- vm.setPowerState(VirtualMachine.PowerState.PowerOff);
- virtualMachineDao.update(vm.getId(), vm);
- if (vm.getRemoved() != null) {
- virtualMachineDao.unremove(vm.getId());
- UsageEventUtils.publishUsageEvent(EventTypes.EVENT_VM_CREATE, accountId, vm.getDataCenterId(), vm.getId(), vm.getHostName(), vm.getServiceOfferingId(), vm.getTemplateId(),
- vm.getHypervisorType().toString(), VirtualMachine.class.getName(), vm.getUuid(), vm.isDisplayVm());
- }
- return virtualMachineDao.findById(vm.getId());
- } else {
+ s_logger.debug(String.format("Trying to get VM with specs: [vmInternalName: %s], and in states [%s, %s].", vmInternalName, VirtualMachine.State.Running, VirtualMachine.State.Stopped));
+ VMInstanceVO vm = virtualMachineDao.findVMInStatesAndWithInternalNameIncludingRemoved(vmInternalName, VirtualMachine.State.Running, VirtualMachine.State.Stopped);
Review Comment:
@harikrishna-patnala As you can see in this comment https://github.com/apache/cloudstack/pull/7239#issuecomment-1846056272, when removing a VM from ACS management and reimporting it, two records, with the same `instance_name` (which should be unique), are created, so the query, even though it filtered for VMs already removed, did not consider that there is another VM, not removed, with the same `instance_name`, which caused the VM that had already been removed to be removed, while that the reimported VM, as it was not removed, remains in the database, which causes the duplication of such VMs, something that should not occur, I believe.
Please tell me if my explanation wasn't good enough (I'm not good at talking to people).
##########
plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/guru/VMwareGuru.java:
##########
@@ -661,30 +660,29 @@ private Long getImportingVMTemplate(List<VirtualDisk> virtualDisks, DatacenterMO
* If VM exists: update VM
*/
private VMInstanceVO getVM(String vmInternalName, long templateId, long guestOsId, long serviceOfferingId, long zoneId, long accountId, long userId, long domainId) {
- s_logger.debug(String.format("Trying to get VM with specs: [vmInternalName: %s, templateId: %s, guestOsId: %s, serviceOfferingId: %s].", vmInternalName,
- templateId, guestOsId, serviceOfferingId));
- VMInstanceVO vm = virtualMachineDao.findVMByInstanceNameIncludingRemoved(vmInternalName);
- if (vm != null) {
- s_logger.debug(String.format("Found an existing VM [id: %s, removed: %s] with internalName: [%s].", vm.getUuid(), vm.getRemoved() != null ? "yes" : "no", vmInternalName));
- vm.setState(VirtualMachine.State.Stopped);
- vm.setPowerState(VirtualMachine.PowerState.PowerOff);
- virtualMachineDao.update(vm.getId(), vm);
- if (vm.getRemoved() != null) {
- virtualMachineDao.unremove(vm.getId());
- UsageEventUtils.publishUsageEvent(EventTypes.EVENT_VM_CREATE, accountId, vm.getDataCenterId(), vm.getId(), vm.getHostName(), vm.getServiceOfferingId(), vm.getTemplateId(),
- vm.getHypervisorType().toString(), VirtualMachine.class.getName(), vm.getUuid(), vm.isDisplayVm());
- }
- return virtualMachineDao.findById(vm.getId());
- } else {
+ s_logger.debug(String.format("Trying to get VM with specs: [vmInternalName: %s], and in states [%s, %s].", vmInternalName, VirtualMachine.State.Running, VirtualMachine.State.Stopped));
+ VMInstanceVO vm = virtualMachineDao.findVMInStatesAndWithInternalNameIncludingRemoved(vmInternalName, VirtualMachine.State.Running, VirtualMachine.State.Stopped);
+ if (vm == null) {
+ s_logger.debug(String.format("Cannot find any existing VM with internalName [%s] in any of these [%s, %s] states. Assuming VM is destroyed in ACS and recreated in restore process.",
+ vmInternalName, VirtualMachine.State.Running, VirtualMachine.State.Stopped));
+
long id = userVmDao.getNextInSequence(Long.class, "id");
s_logger.debug(String.format("Can't find an existing VM with internalName: [%s]. Creating a new VM with: [id: %s, name: %s, templateId: %s, guestOsId: %s, serviceOfferingId: %s].",
vmInternalName, id, vmInternalName, templateId, guestOsId, serviceOfferingId));
UserVmVO vmInstanceVO = new UserVmVO(id, vmInternalName, vmInternalName, templateId, HypervisorType.VMware, guestOsId, false, false, domainId, accountId, userId,
serviceOfferingId, null, null, null, vmInternalName);
vmInstanceVO.setDataCenterId(zoneId);
- return userVmDao.persist(vmInstanceVO);
+ vm = userVmDao.persist(vmInstanceVO);
+ UsageEventUtils.publishUsageEvent(EventTypes.EVENT_VM_CREATE, accountId, vm.getDataCenterId(), vm.getId(), vm.getHostName(), vm.getServiceOfferingId(), vm.getTemplateId(),
Review Comment:
Because, as you said in https://github.com/apache/cloudstack/pull/7239/files#r1247364501, there are no VMs removed in `Stopped` or `Running` states unless they are manually marked in the database. Therefore, if this check does not find any VM, it means that it must be created, therefore the event must be launched.
Again, please let me know if my explanation was good enough, and I apologize for the delay in my answer.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [Veeam] Fix restore of reimported VMs [cloudstack]
Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on PR #7239:
URL: https://github.com/apache/cloudstack/pull/7239#issuecomment-1812689393
ping @SadiJr
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] harikrishna-patnala commented on a diff in pull request #7239: [Veeam] Fix restore of reimported VMs
Posted by "harikrishna-patnala (via GitHub)" <gi...@apache.org>.
harikrishna-patnala commented on code in PR #7239:
URL: https://github.com/apache/cloudstack/pull/7239#discussion_r1247364501
##########
plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/guru/VMwareGuru.java:
##########
@@ -661,30 +660,29 @@ private Long getImportingVMTemplate(List<VirtualDisk> virtualDisks, DatacenterMO
* If VM exists: update VM
*/
private VMInstanceVO getVM(String vmInternalName, long templateId, long guestOsId, long serviceOfferingId, long zoneId, long accountId, long userId, long domainId) {
- s_logger.debug(String.format("Trying to get VM with specs: [vmInternalName: %s, templateId: %s, guestOsId: %s, serviceOfferingId: %s].", vmInternalName,
- templateId, guestOsId, serviceOfferingId));
- VMInstanceVO vm = virtualMachineDao.findVMByInstanceNameIncludingRemoved(vmInternalName);
- if (vm != null) {
- s_logger.debug(String.format("Found an existing VM [id: %s, removed: %s] with internalName: [%s].", vm.getUuid(), vm.getRemoved() != null ? "yes" : "no", vmInternalName));
- vm.setState(VirtualMachine.State.Stopped);
- vm.setPowerState(VirtualMachine.PowerState.PowerOff);
- virtualMachineDao.update(vm.getId(), vm);
- if (vm.getRemoved() != null) {
- virtualMachineDao.unremove(vm.getId());
- UsageEventUtils.publishUsageEvent(EventTypes.EVENT_VM_CREATE, accountId, vm.getDataCenterId(), vm.getId(), vm.getHostName(), vm.getServiceOfferingId(), vm.getTemplateId(),
- vm.getHypervisorType().toString(), VirtualMachine.class.getName(), vm.getUuid(), vm.isDisplayVm());
- }
- return virtualMachineDao.findById(vm.getId());
- } else {
+ s_logger.debug(String.format("Trying to get VM with specs: [vmInternalName: %s], and in states [%s, %s].", vmInternalName, VirtualMachine.State.Running, VirtualMachine.State.Stopped));
+ VMInstanceVO vm = virtualMachineDao.findVMInStatesAndWithInternalNameIncludingRemoved(vmInternalName, VirtualMachine.State.Running, VirtualMachine.State.Stopped);
Review Comment:
@SadiJr can you please help me understand how this solved the problem of duplicate entries with the same VM name? Previously also we are looking for any VMs with that name (including removed) and then reusing that removed entry.
Here in this fix, findVMInStatesAndWithInternalNameIncludingRemoved() method is fetching only VMs with states Running and Stopped. There won't be any VM entries that are removed with Running and Stopped states unless they are marked as removed in the DB manually. This means we are ignoring the actually removed entries.
Please correct me if I'm wrong here.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] github-actions[bot] commented on pull request #7239: [Veeam] Fix restore of reimported VMs
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #7239:
URL: https://github.com/apache/cloudstack/pull/7239#issuecomment-1678954975
This pull request has merge conflicts. Dear author, please fix the conflicts and sync your branch with the base branch.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #7239: [Veeam] Fix restore of reimported VMs
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #7239:
URL: https://github.com/apache/cloudstack/pull/7239#issuecomment-1434096103
<b>Trillian test result (tid-6189)</b>
Environment: vmware-67u3 (x2), Advanced Networking with Mgmt server r8
Total time taken: 44569 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr7239-t6189-vmware-67u3.zip
Smoke tests completed. 108 look OK, 0 have errors, 0 did not run
Only failed and skipped 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 to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] weizhouapache commented on pull request #7239: [Veeam] Fix restore of reimported VMs
Posted by "weizhouapache (via GitHub)" <gi...@apache.org>.
weizhouapache commented on PR #7239:
URL: https://github.com/apache/cloudstack/pull/7239#issuecomment-1691235028
moved to 4.19.0.0 as the target branch is `main`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [Veeam] Fix restore of reimported VMs [cloudstack]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #7239:
URL: https://github.com/apache/cloudstack/pull/7239#issuecomment-1934109973
This pull request has merge conflicts. Dear author, please fix the conflicts and sync your branch with the base branch.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [Veeam] Fix restore of reimported VMs [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #7239:
URL: https://github.com/apache/cloudstack/pull/7239#issuecomment-2185591853
<b>[SF] Trillian test result (tid-10576)</b>
Environment: vmware-70u3 (x2), Advanced Networking with Mgmt server a9
Total time taken: 42102 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr7239-t10576-vmware-70u3.zip
Smoke tests completed. 105 look OK, 10 have errors, 16 did not run
Only failed and skipped tests results shown below:
Test | Result | Time (s) | Test File
--- | --- | --- | ---
test_01_restore_vm | `Error` | 3614.41 | test_restore_vm.py
test_02_restore_vm_allocated_root | `Error` | 7.75 | test_restore_vm.py
ContextSuite context=TestRestoreVM>:teardown | `Error` | 15.11 | test_restore_vm.py
test_01_prepare_and_cancel_shutdown | `Error` | 2.54 | test_safe_shutdown.py
test_02_trigger_shutdown | `Failure` | 301.27 | test_safe_shutdown.py
test_01_scale_vm | `Error` | 0.02 | test_scale_vm.py
test_02_scale_vm_negative_offering_disable_scaling | `Error` | 0.02 | test_scale_vm.py
test_03_scale_vm_negative_vm_disable_scaling | `Error` | 0.02 | test_scale_vm.py
test_04_scale_vm_with_user_account | `Error` | 2.99 | test_scale_vm.py
test_04_scale_vm_with_user_account | `Error` | 2.99 | test_scale_vm.py
test_05_scale_vm_dont_allow_disk_offering_change | `Error` | 0.08 | test_scale_vm.py
ContextSuite context=TestScaleVm>:teardown | `Error` | 0.22 | test_scale_vm.py
ContextSuite context=TestServiceOfferings>:setup | `Error` | 2.87 | test_service_offerings.py
ContextSuite context=TestSetSourceNatIp>:setup | `Error` | 0.00 | test_set_sourcenat.py
ContextSuite context=TestSnapshotRootDisk>:setup | `Error` | 0.00 | test_snapshots.py
ContextSuite context=TestSnapshotStandaloneBackup>:setup | `Error` | 0.00 | test_snapshots.py
test_05_stop_ssvm | `Error` | 0.12 | test_ssvm.py
test_06_stop_cpvm | `Error` | 0.11 | test_ssvm.py
test_07_reboot_ssvm | `Error` | 0.11 | test_ssvm.py
test_08_reboot_cpvm | `Error` | 0.11 | test_ssvm.py
test_09_reboot_ssvm_forced | `Error` | 0.11 | test_ssvm.py
test_10_reboot_cpvm_forced | `Error` | 0.10 | test_ssvm.py
test_11_destroy_ssvm | `Error` | 0.09 | test_ssvm.py
test_12_destroy_cpvm | `Error` | 0.09 | test_ssvm.py
test_01_import_storage_policies | `Error` | 5.08 | test_storage_policy.py
ContextSuite context=TestVMWareStoragePolicies>:teardown | `Error` | 5.11 | test_storage_policy.py
ContextSuite context=TestCopyAndDeleteTemplatesAcrossZones>:setup | `Error` | 0.00 | test_templates.py
ContextSuite context=TestCreateTemplate>:setup | `Error` | 0.00 | test_templates.py
ContextSuite context=TestTemplates>:setup | `Error` | 5.75 | test_templates.py
test_01_positive_tests_usage | `Failure` | 2.78 | test_usage_events.py
test_01_positive_tests_usage | `Error` | 2.78 | test_usage_events.py
all_test_usage | `Skipped` | --- | test_usage.py
all_test_vm_autoscaling | `Skipped` | --- | test_vm_autoscaling.py
all_test_vm_deployment_planner | `Skipped` | --- | test_vm_deployment_planner.py
all_test_vm_life_cycle | `Skipped` | --- | test_vm_life_cycle.py
all_test_vm_lifecycle_unmanage_import | `Skipped` | --- | test_vm_lifecycle_unmanage_import.py
all_test_vm_schedule | `Skipped` | --- | test_vm_schedule.py
all_test_vm_snapshot_kvm | `Skipped` | --- | test_vm_snapshot_kvm.py
all_test_vm_snapshots | `Skipped` | --- | test_vm_snapshots.py
all_test_vnf_templates | `Skipped` | --- | test_vnf_templates.py
all_test_volumes | `Skipped` | --- | test_volumes.py
all_test_vpc_ipv6 | `Skipped` | --- | test_vpc_ipv6.py
all_test_vpc_redundant | `Skipped` | --- | test_vpc_redundant.py
all_test_vpc_router_nics | `Skipped` | --- | test_vpc_router_nics.py
all_test_vpc_vpn | `Skipped` | --- | test_vpc_vpn.py
all_test_host_maintenance | `Skipped` | --- | test_host_maintenance.py
all_test_hostha_kvm | `Skipped` | --- | test_hostha_kvm.py
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [Veeam] Fix restore of reimported VMs [cloudstack]
Posted by "weizhouapache (via GitHub)" <gi...@apache.org>.
weizhouapache commented on PR #7239:
URL: https://github.com/apache/cloudstack/pull/7239#issuecomment-1846056272
@SadiJr @harikrishna-patnala @DaanHoogland
I tried to reproduce the issue on veeam12 environment with some changes (see https://github.com/apache/cloudstack/pull/8241)
However, the restore of backup succeeds in my testing
- unmanage a running vm
- import it to cloudstack
got two VMs with same instance name
```
+-----+---------------+--------------------------------------+---------------+---------------+---------------------+---------------------+
| id | name | uuid | instance_name | display_name | created | removed |
+-----+---------------+--------------------------------------+---------------+---------------+---------------------+---------------------+
| 162 | admin-001-001 | c13f4662-dd08-4f24-abd3-c1b581a3ecd2 | i-2-162-VM | admin-001-001 | 2023-12-01 08:22:55 | 2023-12-07 20:00:16 |
| 225 | i-2-162-VM | a974a38d-5b9a-40d2-a0fb-feb80ef88762 | i-2-162-VM | NULL | 2023-12-07 20:01:01 | NULL |
+-----+---------------+--------------------------------------+---------------+---------------+---------------------+---------------------+
2 rows in set (0.00 sec)
```
- assign backup offering
- create a backup
- stop the vm, and restore the backup.
I checked the description and code again, I think we should not fix an issue with veeam backup/restore by changing the processes of unmanage/manage VM.
We should fix the issue in veeam plugin, if the issue can be reproduced.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [Veeam] Fix restore of reimported VMs [cloudstack]
Posted by "weizhouapache (via GitHub)" <gi...@apache.org>.
weizhouapache commented on PR #7239:
URL: https://github.com/apache/cloudstack/pull/7239#issuecomment-1847107331
> @weizhouapache can you please share the state of VMs `admin-001-001` and `i-2-162-VM` in your database after doing your test? And about fixing an issue with Veeam by changing the unmanage/manage process, I think having two VMs with same `instance_name` in database is a inconsistency that is being generated by the manage/import VMware VMs. The internal name is expected to be unique.
@SadiJr
here you are
```
mysql> select id,name,uuid,instance_name,display_name,state,created,removed from vm_instance where id in (162,225);
+-----+---------------+--------------------------------------+---------------+---------------+-----------+---------------------+---------------------+
| id | name | uuid | instance_name | display_name | state | created | removed |
+-----+---------------+--------------------------------------+---------------+---------------+-----------+---------------------+---------------------+
| 162 | admin-001-001 | c13f4662-dd08-4f24-abd3-c1b581a3ecd2 | i-2-162-VM | admin-001-001 | Expunging | 2023-12-01 08:22:55 | 2023-12-07 20:00:16 |
| 225 | i-2-162-VM | a974a38d-5b9a-40d2-a0fb-feb80ef88762 | i-2-162-VM | NULL | Stopped | 2023-12-07 20:01:01 | NULL |
+-----+---------------+--------------------------------------+---------------+---------------+-----------+---------------------+---------------------+
2 rows in set (0.00 sec)
```
the original one is in `Expunging` state and `removed` field is not empty. so it does not impact the new vm.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [Veeam] Fix restore of reimported VMs [cloudstack]
Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on PR #7239:
URL: https://github.com/apache/cloudstack/pull/7239#issuecomment-1999629859
@harikrishna-patnala , can you check if your queries are answerred?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] sonarcloud[bot] commented on pull request #7239: [Veeam] Fix restore of reimported VMs
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #7239:
URL: https://github.com/apache/cloudstack/pull/7239#issuecomment-1438345113
SonarCloud Quality Gate failed. [](https://sonarcloud.io/dashboard?id=apache_cloudstack&pullRequest=7239)
[](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=7239&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=7239&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=7239&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=7239&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=7239&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=7239&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_cloudstack&pullRequest=7239&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_cloudstack&pullRequest=7239&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_cloudstack&pullRequest=7239&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=7239&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=7239&resolved=false&types=CODE_SMELL) [1 Code Smell](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=7239&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=7239&metric=new_coverage&view=list) [0.0% Coverage](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=7239&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=7239&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=7239&metric=new_duplicated_lines_density&view=list)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] DaanHoogland commented on pull request #7239: [Veeam] Fix restore of reimported VMs
Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on PR #7239:
URL: https://github.com/apache/cloudstack/pull/7239#issuecomment-1432678825
@blueorangutan package
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] DaanHoogland commented on a diff in pull request #7239: [Veeam] Fix restore of reimported VMs
Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on code in PR #7239:
URL: https://github.com/apache/cloudstack/pull/7239#discussion_r1109576710
##########
plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/guru/VMwareGuru.java:
##########
@@ -661,30 +660,31 @@ private Long getImportingVMTemplate(List<VirtualDisk> virtualDisks, DatacenterMO
* If VM exists: update VM
*/
private VMInstanceVO getVM(String vmInternalName, long templateId, long guestOsId, long serviceOfferingId, long zoneId, long accountId, long userId, long domainId) {
- s_logger.debug(String.format("Trying to get VM with specs: [vmInternalName: %s, templateId: %s, guestOsId: %s, serviceOfferingId: %s].", vmInternalName,
- templateId, guestOsId, serviceOfferingId));
- VMInstanceVO vm = virtualMachineDao.findVMByInstanceNameIncludingRemoved(vmInternalName);
- if (vm != null) {
- s_logger.debug(String.format("Found an existing VM [id: %s, removed: %s] with internalName: [%s].", vm.getUuid(), vm.getRemoved() != null ? "yes" : "no", vmInternalName));
- vm.setState(VirtualMachine.State.Stopped);
- vm.setPowerState(VirtualMachine.PowerState.PowerOff);
- virtualMachineDao.update(vm.getId(), vm);
- if (vm.getRemoved() != null) {
- virtualMachineDao.unremove(vm.getId());
- UsageEventUtils.publishUsageEvent(EventTypes.EVENT_VM_CREATE, accountId, vm.getDataCenterId(), vm.getId(), vm.getHostName(), vm.getServiceOfferingId(), vm.getTemplateId(),
- vm.getHypervisorType().toString(), VirtualMachine.class.getName(), vm.getUuid(), vm.isDisplayVm());
- }
- return virtualMachineDao.findById(vm.getId());
- } else {
+ s_logger.debug(String.format("Trying to get VM with specs: [vmInternalName: %s], and in states [%s, %s].", vmInternalName, VirtualMachine.State.Running, VirtualMachine.State.Stopped));
+ VMInstanceVO vm = virtualMachineDao.findVMInStatesAndWithInternalNameIncludingRemoved(vmInternalName, VirtualMachine.State.Running, VirtualMachine.State.Stopped);
+ if (vm == null) {
+ s_logger.debug(String.format("Cannot find any existing VM with internalName [%s] in any of this [%s, %s] states. Assuming VM is destroyed in ACS and recreated in restore process.",
+ vmInternalName, VirtualMachine.State.Running, VirtualMachine.State.Stopped));
+
long id = userVmDao.getNextInSequence(Long.class, "id");
s_logger.debug(String.format("Can't find an existing VM with internalName: [%s]. Creating a new VM with: [id: %s, name: %s, templateId: %s, guestOsId: %s, serviceOfferingId: %s].",
vmInternalName, id, vmInternalName, templateId, guestOsId, serviceOfferingId));
UserVmVO vmInstanceVO = new UserVmVO(id, vmInternalName, vmInternalName, templateId, HypervisorType.VMware, guestOsId, false, false, domainId, accountId, userId,
serviceOfferingId, null, null, null, vmInternalName);
vmInstanceVO.setDataCenterId(zoneId);
- return userVmDao.persist(vmInstanceVO);
+ vm = userVmDao.persist(vmInstanceVO);
+ if (vm != null) {
Review Comment:
so the vm can still be null? and we later return it?
Can you explain what the scenario for that would be?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] DaanHoogland commented on pull request #7239: [Veeam] Fix restore of reimported VMs
Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on PR #7239:
URL: https://github.com/apache/cloudstack/pull/7239#issuecomment-1433316507
@blueorangutan test rocky8 vmware-67u3
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [Veeam] Fix restore of reimported VMs [cloudstack]
Posted by "weizhouapache (via GitHub)" <gi...@apache.org>.
weizhouapache commented on PR #7239:
URL: https://github.com/apache/cloudstack/pull/7239#issuecomment-2185698458
@blueorangutan test rocky8 vmware-70u3
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [Veeam] Fix restore of reimported VMs [cloudstack]
Posted by "JoaoJandre (via GitHub)" <gi...@apache.org>.
JoaoJandre closed pull request #7239: [Veeam] Fix restore of reimported VMs
URL: https://github.com/apache/cloudstack/pull/7239
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [Veeam] Fix restore of reimported VMs [cloudstack]
Posted by "sureshanaparti (via GitHub)" <gi...@apache.org>.
sureshanaparti commented on PR #7239:
URL: https://github.com/apache/cloudstack/pull/7239#issuecomment-2186310447
> @weizhouapache that is strange. Your results differ a lot from mine. I review the changes in #8241, but, from what I understand, they should not impact this behavior. I will repeat my tests, using the most recent community version, this PR of mine and #8241, and I will return an opinion. Unfortunately, this may take some time as I am without my VMware environment at the moment.
Hi @SadiJr Any update of the tests, Is it good for 4.19.1? cc @weizhouapache
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [Veeam] Fix restore of reimported VMs [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #7239:
URL: https://github.com/apache/cloudstack/pull/7239#issuecomment-2185701496
@weizhouapache a [SL] Trillian-Jenkins test job (rocky8 mgmt + vmware-70u3) 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 to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [Veeam] Fix restore of reimported VMs [cloudstack]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #7239:
URL: https://github.com/apache/cloudstack/pull/7239#issuecomment-2191579573
This pull request has merge conflicts. Dear author, please fix the conflicts and sync your branch with the base branch.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [Veeam] Fix restore of reimported VMs [cloudstack]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #7239:
URL: https://github.com/apache/cloudstack/pull/7239#issuecomment-2191582705
This pull request has merge conflicts. Dear author, please fix the conflicts and sync your branch with the base branch.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [Veeam] Fix restore of reimported VMs [cloudstack]
Posted by "shwstppr (via GitHub)" <gi...@apache.org>.
shwstppr commented on PR #7239:
URL: https://github.com/apache/cloudstack/pull/7239#issuecomment-1752541207
@SadiJr can you please check review comments
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [Veeam] Fix restore of reimported VMs [cloudstack]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #7239:
URL: https://github.com/apache/cloudstack/pull/7239#issuecomment-1759437435
This pull request has merge conflicts. Dear author, please fix the conflicts and sync your branch with the base branch.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [Veeam] Fix restore of reimported VMs [cloudstack]
Posted by "SadiJr (via GitHub)" <gi...@apache.org>.
SadiJr commented on PR #7239:
URL: https://github.com/apache/cloudstack/pull/7239#issuecomment-1850756527
@weizhouapache that is strange. Your results differ a lot from mine. I review the changes in #8241, but, from what I understand, they should not impact this behavior. I will repeat my tests, using the most recent community version, this PR of mine and #8241, and I will return an opinion. Unfortunately, this may take some time as I am without my VMware environment at the moment.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [Veeam] Fix restore of reimported VMs [cloudstack]
Posted by "SadiJr (via GitHub)" <gi...@apache.org>.
SadiJr commented on PR #7239:
URL: https://github.com/apache/cloudstack/pull/7239#issuecomment-1821428677
@harikrishna-patnala @DaanHoogland Sorry for the delay, I will check the suggestions and respond/work on this PR
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #7239: [Veeam] Fix restore of reimported VMs
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #7239:
URL: https://github.com/apache/cloudstack/pull/7239#issuecomment-1433317501
@DaanHoogland a Trillian-Jenkins test job (rocky8 mgmt + vmware-67u3) 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 to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] SadiJr commented on a diff in pull request #7239: [Veeam] Fix restore of reimported VMs
Posted by "SadiJr (via GitHub)" <gi...@apache.org>.
SadiJr commented on code in PR #7239:
URL: https://github.com/apache/cloudstack/pull/7239#discussion_r1112884849
##########
plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/guru/VMwareGuru.java:
##########
@@ -661,30 +660,31 @@ private Long getImportingVMTemplate(List<VirtualDisk> virtualDisks, DatacenterMO
* If VM exists: update VM
*/
private VMInstanceVO getVM(String vmInternalName, long templateId, long guestOsId, long serviceOfferingId, long zoneId, long accountId, long userId, long domainId) {
- s_logger.debug(String.format("Trying to get VM with specs: [vmInternalName: %s, templateId: %s, guestOsId: %s, serviceOfferingId: %s].", vmInternalName,
- templateId, guestOsId, serviceOfferingId));
- VMInstanceVO vm = virtualMachineDao.findVMByInstanceNameIncludingRemoved(vmInternalName);
- if (vm != null) {
- s_logger.debug(String.format("Found an existing VM [id: %s, removed: %s] with internalName: [%s].", vm.getUuid(), vm.getRemoved() != null ? "yes" : "no", vmInternalName));
- vm.setState(VirtualMachine.State.Stopped);
- vm.setPowerState(VirtualMachine.PowerState.PowerOff);
- virtualMachineDao.update(vm.getId(), vm);
- if (vm.getRemoved() != null) {
- virtualMachineDao.unremove(vm.getId());
- UsageEventUtils.publishUsageEvent(EventTypes.EVENT_VM_CREATE, accountId, vm.getDataCenterId(), vm.getId(), vm.getHostName(), vm.getServiceOfferingId(), vm.getTemplateId(),
- vm.getHypervisorType().toString(), VirtualMachine.class.getName(), vm.getUuid(), vm.isDisplayVm());
- }
- return virtualMachineDao.findById(vm.getId());
- } else {
+ s_logger.debug(String.format("Trying to get VM with specs: [vmInternalName: %s], and in states [%s, %s].", vmInternalName, VirtualMachine.State.Running, VirtualMachine.State.Stopped));
+ VMInstanceVO vm = virtualMachineDao.findVMInStatesAndWithInternalNameIncludingRemoved(vmInternalName, VirtualMachine.State.Running, VirtualMachine.State.Stopped);
+ if (vm == null) {
+ s_logger.debug(String.format("Cannot find any existing VM with internalName [%s] in any of this [%s, %s] states. Assuming VM is destroyed in ACS and recreated in restore process.",
+ vmInternalName, VirtualMachine.State.Running, VirtualMachine.State.Stopped));
+
long id = userVmDao.getNextInSequence(Long.class, "id");
s_logger.debug(String.format("Can't find an existing VM with internalName: [%s]. Creating a new VM with: [id: %s, name: %s, templateId: %s, guestOsId: %s, serviceOfferingId: %s].",
vmInternalName, id, vmInternalName, templateId, guestOsId, serviceOfferingId));
UserVmVO vmInstanceVO = new UserVmVO(id, vmInternalName, vmInternalName, templateId, HypervisorType.VMware, guestOsId, false, false, domainId, accountId, userId,
serviceOfferingId, null, null, null, vmInternalName);
vmInstanceVO.setDataCenterId(zoneId);
- return userVmDao.persist(vmInstanceVO);
+ vm = userVmDao.persist(vmInstanceVO);
+ if (vm != null) {
Review Comment:
@DaanHoogland sorry, my mistake. Added this check unnecessarily, thanks for taking that detail, I'll fix it :)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] codecov[bot] commented on pull request #7239: [Veeam] Fix restore of reimported VMs
Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #7239:
URL: https://github.com/apache/cloudstack/pull/7239#issuecomment-1431787333
# [Codecov](https://codecov.io/gh/apache/cloudstack/pull/7239?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#7239](https://codecov.io/gh/apache/cloudstack/pull/7239?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (5b870cd) into [main](https://codecov.io/gh/apache/cloudstack/commit/597a8030b7c3235e59207041634206bead379c08?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (597a803) will **increase** coverage by `0.00%`.
> The diff coverage is `18.18%`.
```diff
@@ Coverage Diff @@
## main #7239 +/- ##
=========================================
Coverage 12.67% 12.67%
Complexity 8639 8639
=========================================
Files 2716 2716
Lines 256112 256118 +6
Branches 39926 39927 +1
=========================================
+ Hits 32456 32460 +4
- Misses 219528 219530 +2
Partials 4128 4128
```
| [Impacted Files](https://codecov.io/gh/apache/cloudstack/pull/7239?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...ain/java/com/cloud/hypervisor/guru/VMwareGuru.java](https://codecov.io/gh/apache/cloudstack/pull/7239?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGx1Z2lucy9oeXBlcnZpc29ycy92bXdhcmUvc3JjL21haW4vamF2YS9jb20vY2xvdWQvaHlwZXJ2aXNvci9ndXJ1L1ZNd2FyZUd1cnUuamF2YQ==) | `1.06% <0.00%> (+<0.01%)` | :arrow_up: |
| [.../main/java/com/cloud/vm/dao/VMInstanceDaoImpl.java](https://codecov.io/gh/apache/cloudstack/pull/7239?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZW5naW5lL3NjaGVtYS9zcmMvbWFpbi9qYXZhL2NvbS9jbG91ZC92bS9kYW8vVk1JbnN0YW5jZURhb0ltcGwuamF2YQ==) | `26.01% <50.00%> (+0.35%)` | :arrow_up: |
:mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] JoaoJandre commented on a diff in pull request #7239: [Veeam] Fix restore of reimported VMs
Posted by "JoaoJandre (via GitHub)" <gi...@apache.org>.
JoaoJandre commented on code in PR #7239:
URL: https://github.com/apache/cloudstack/pull/7239#discussion_r1116192215
##########
plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/guru/VMwareGuru.java:
##########
@@ -661,30 +660,29 @@ private Long getImportingVMTemplate(List<VirtualDisk> virtualDisks, DatacenterMO
* If VM exists: update VM
*/
private VMInstanceVO getVM(String vmInternalName, long templateId, long guestOsId, long serviceOfferingId, long zoneId, long accountId, long userId, long domainId) {
- s_logger.debug(String.format("Trying to get VM with specs: [vmInternalName: %s, templateId: %s, guestOsId: %s, serviceOfferingId: %s].", vmInternalName,
- templateId, guestOsId, serviceOfferingId));
- VMInstanceVO vm = virtualMachineDao.findVMByInstanceNameIncludingRemoved(vmInternalName);
- if (vm != null) {
- s_logger.debug(String.format("Found an existing VM [id: %s, removed: %s] with internalName: [%s].", vm.getUuid(), vm.getRemoved() != null ? "yes" : "no", vmInternalName));
- vm.setState(VirtualMachine.State.Stopped);
- vm.setPowerState(VirtualMachine.PowerState.PowerOff);
- virtualMachineDao.update(vm.getId(), vm);
- if (vm.getRemoved() != null) {
- virtualMachineDao.unremove(vm.getId());
- UsageEventUtils.publishUsageEvent(EventTypes.EVENT_VM_CREATE, accountId, vm.getDataCenterId(), vm.getId(), vm.getHostName(), vm.getServiceOfferingId(), vm.getTemplateId(),
- vm.getHypervisorType().toString(), VirtualMachine.class.getName(), vm.getUuid(), vm.isDisplayVm());
- }
- return virtualMachineDao.findById(vm.getId());
- } else {
+ s_logger.debug(String.format("Trying to get VM with specs: [vmInternalName: %s], and in states [%s, %s].", vmInternalName, VirtualMachine.State.Running, VirtualMachine.State.Stopped));
+ VMInstanceVO vm = virtualMachineDao.findVMInStatesAndWithInternalNameIncludingRemoved(vmInternalName, VirtualMachine.State.Running, VirtualMachine.State.Stopped);
+ if (vm == null) {
+ s_logger.debug(String.format("Cannot find any existing VM with internalName [%s] in any of this [%s, %s] states. Assuming VM is destroyed in ACS and recreated in restore process.",
Review Comment:
```suggestion
s_logger.debug(String.format("Cannot find any existing VM with internalName [%s] in any of these [%s, %s] states. Assuming VM is destroyed in ACS and recreated in restore process.",
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org