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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
