Oved Ourfali has posted comments on this change.
Change subject: core: importing external and hosted-engine VMs to the engine
......................................................................
Patch Set 6:
(3 comments)
Regarding the getVm - I saw that I use it, and that several commands test for
it. Not sure why it isn't tested in all the VM commands.
Regarding performance improvements, it is a one-time shot here, so not sure it
is worth the effort, but let's discuss that.
Regarding the error messages - I'll add the VM name in the message.
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VdsEventListener.java
Line 312: @Override
Line 313: public void addExternallyManagedVms(List<VmStatic>
externalVmList) {
Line 314: for (VmStatic currVm : externalVmList) {
Line 315: AddVmFromScratchParameters params = new
AddVmFromScratchParameters(currVm, null, null);
Line 316: VdcReturnValueBase returnValue =
Backend.getInstance().runInternalAction(VdcActionType.AddVmFromScratch, params,
ExecutionHandler.createInternalJobContext());
Yes, but the use-case here would include a small amount of VMs, which will be
added once, so the improvement here is redundant.
Line 317: if (!returnValue.getSucceeded()) {
Line 318: log.debugFormat("Failed adding Externally managed VM
{0}", currVm.getName());
Line 319: }
Line 320: }
....................................................
File
backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/VdsUpdateRunTimeInfo.java
Line 1599: // Searching for External VMs that run on the host
Line 1600: for (VmInternalData vmInternalData : _runningVms.values()) {
Line 1601: VM currentVmData =
_vmDict.get(vmInternalData.getVmDynamic().getId());
Line 1602: if (currentVmData == null) {
Line 1603: if
(getDbFacade().getVmStaticDao().get(vmInternalData.getVmDynamic().getId()) ==
null) {
As I mentioned before, this would be a one-time querying for these VMs, so not
sure it is worth the effort, but if you think it does then I'll try to improve
that.
Line 1604: Guid vmId =
vmInternalData.getVmDynamic().getId();
Line 1605: vmsToQuery.add(vmId.toString());
Line 1606: }
Line 1607: }
....................................................
File
frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/AppErrors.java
Line 57:
Line 58: @DefaultStringValue("Cannot ${action} ${type}. The snapshot is
broken, and no further work can be done on it. Please remove this snapshot from
the VM.")
Line 59: String ACTION_TYPE_FAILED_VM_SNAPSHOT_IS_BROKEN();
Line 60:
Line 61: @DefaultStringValue("Cannot ${action} ${type}. This VM is not
managed by the engine.")
In most cases here the error is related straight to some operation the user
does.
In any way, I'll take a look at the different use-cases, as I think I can push
the VM name there.
Line 62: String ACTION_TYPE_FAILED_CANNOT_RUN_ACTION_ON_NON_MANAGED_VM();
Line 63:
Line 64: @DefaultStringValue("Storage Domain cannot be accessed.\nPossible
reasons:\nNo operational Host in Data Center or Data Center state is not Up.")
Line 65: String IMAGE_REPOSITORY_NOT_FOUND();
--
To view, visit http://gerrit.ovirt.org/17269
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I6b933df39aaf6897a675aef0564d3fb4922f12e7
Gerrit-PatchSet: 6
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Oved Ourfali <[email protected]>
Gerrit-Reviewer: Arik Hadas <[email protected]>
Gerrit-Reviewer: Doron Fediuck <[email protected]>
Gerrit-Reviewer: Eli Mesika <[email protected]>
Gerrit-Reviewer: Itamar Heim <[email protected]>
Gerrit-Reviewer: Michal Skrivanek <[email protected]>
Gerrit-Reviewer: Omer Frenkel <[email protected]>
Gerrit-Reviewer: Oved Ourfali <[email protected]>
Gerrit-Reviewer: Roy Golan <[email protected]>
Gerrit-Reviewer: Yair Zaslavsky <[email protected]>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches
