You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cloudstack.apache.org by GitBox <gi...@apache.org> on 2022/12/02 12:43:29 UTC
[GitHub] [cloudstack] DaanHoogland commented on a diff in pull request #6939: When VM is created and ROOT volume is created it should emit a VOLUME.CREATE event
DaanHoogland commented on code in PR #6939:
URL: https://github.com/apache/cloudstack/pull/6939#discussion_r1038105245
##########
engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/VolumeOrchestrator.java:
##########
@@ -871,7 +879,16 @@ public DiskProfile allocateRawVolume(Type type, String name, DiskOffering offeri
_resourceLimitMgr.incrementResourceCount(vm.getAccountId(), ResourceType.volume, vol.isDisplayVolume());
_resourceLimitMgr.incrementResourceCount(vm.getAccountId(), ResourceType.primary_storage, vol.isDisplayVolume(), new Long(vol.getSize()));
}
- return toDiskProfile(vol, offering);
+ DiskProfile diskProfile = toDiskProfile(vol, offering);
+
+ // Set context information for VOLUME.CREATE event for ROOT disk.
+ CallContext volumeContext = CallContext.current();
+ if (type == Type.ROOT && volumeContext != null && volumeContext.getEventResourceType() == ApiCommandResourceType.Volume) {
+ volumeContext.setEventDetails("Volume Id: " + this._uuidMgr.getUuid(Volume.class, diskProfile.getVolumeId()) + " Vm Id: " + this._uuidMgr.getUuid(VirtualMachine.class, vm.getId()));
+ volumeContext.setEventResourceId(diskProfile.getVolumeId());
+ }
Review Comment:
can you extract this to a separate method?
##########
engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java:
##########
@@ -485,15 +486,22 @@ public void doInTransactionWithoutResult(final TransactionStatus status) throws
s_logger.debug("Allocating disks for " + vmFinal);
}
- String rootVolumeName = String.format("ROOT-%s", vmFinal.getId());
- if (template.getFormat() == ImageFormat.ISO) {
- volumeMgr.allocateRawVolume(Type.ROOT, rootVolumeName, rootDiskOfferingInfo.getDiskOffering(), rootDiskOfferingInfo.getSize(),
- rootDiskOfferingInfo.getMinIops(), rootDiskOfferingInfo.getMaxIops(), vmFinal, template, owner, null);
- } else if (template.getFormat() == ImageFormat.BAREMETAL) {
- s_logger.debug(String.format("%s has format [%s]. Skipping ROOT volume [%s] allocation.", template.toString(), ImageFormat.BAREMETAL, rootVolumeName));
- } else {
- volumeMgr.allocateTemplatedVolumes(Type.ROOT, rootVolumeName, rootDiskOfferingInfo.getDiskOffering(), rootDiskSizeFinal,
- rootDiskOfferingInfo.getMinIops(), rootDiskOfferingInfo.getMaxIops(), template, vmFinal, owner);
+ // Create new Volume context and inject event resource type, id and details to generate VOLUME.CREATE event for the ROOT disk.
+ CallContext volumeContext = CallContext.register(CallContext.current(), ApiCommandResourceType.Volume);
+ try {
+ String rootVolumeName = String.format("ROOT-%s", vmFinal.getId());
+ if (template.getFormat() == ImageFormat.ISO) {
+ volumeMgr.allocateRawVolume(Type.ROOT, rootVolumeName, rootDiskOfferingInfo.getDiskOffering(), rootDiskOfferingInfo.getSize(),
+ rootDiskOfferingInfo.getMinIops(), rootDiskOfferingInfo.getMaxIops(), vmFinal, template, owner, null);
+ } else if (template.getFormat() == ImageFormat.BAREMETAL) {
+ s_logger.debug(String.format("%s has format [%s]. Skipping ROOT volume [%s] allocation.", template.toString(), ImageFormat.BAREMETAL, rootVolumeName));
+ } else {
+ volumeMgr.allocateTemplatedVolumes(Type.ROOT, rootVolumeName, rootDiskOfferingInfo.getDiskOffering(), rootDiskSizeFinal,
+ rootDiskOfferingInfo.getMinIops(), rootDiskOfferingInfo.getMaxIops(), template, vmFinal, owner);
+ }
+ } finally {
+ // Remove volumeContext and pop vmContext back
+ CallContext.unregister();
}
Review Comment:
can you extract this to a separate method?
##########
engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/VolumeOrchestrator.java:
##########
@@ -1006,6 +1024,14 @@ public List<DiskProfile> allocateTemplatedVolumes(Type type, String name, DiskOf
profiles.add(diskProfile);
}
+ // Set context information for VOLUME.CREATE event for ROOT disk.
+ CallContext volumeContext = CallContext.current();
+ if (type == Type.ROOT && volumeContext != null && volumeContext.getEventResourceType() == ApiCommandResourceType.Volume) {
+ String volumeIds = profiles.stream().map(diskProfile -> this._uuidMgr.getUuid(Volume.class, diskProfile.getVolumeId())).collect(Collectors.joining(", "));
+ volumeContext.setEventDetails("Volume Id: " + volumeIds + " Vm Id: " + this._uuidMgr.getUuid(VirtualMachine.class, vm.getId()));
+ volumeContext.setEventResourceId(profiles.stream().findFirst().map(DiskProfile::getVolumeId).orElse(null));
+ }
Review Comment:
can you extract this to a separate method?
--
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