Omer Frenkel has posted comments on this change.
Change subject: core: importing external VMs to the engine
......................................................................
Patch Set 1: (4 inline comments)
some minor comments inside,
in addition i think origin field should be used instead of adding a new field,
we can add "External" origin and use it.
the new field has the same meanning
....................................................
File
backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/VdsUpdateRunTimeInfo.java
Line 1458: _externalVmsToAdd.clear();
Line 1459: for (VmInternalData vmInternalData : _runningVms.values()) {
Line 1460: VM currentVmData =
_vmDict.get(vmInternalData.getVmDynamic().getId());
Line 1461: if (currentVmData == null) {
Line 1462: if
(getDbFacade().getVmDao().get(vmInternalData.getVmDynamic().getId()) == null) {
it would be better to get only vm static instead of all vm object
Line 1463: Guid vmId =
vmInternalData.getVmDynamic().getId();
Line 1464: Map vmConfiguration =
getVmInfo(vmId.toString())[0];
Line 1465: VmStatic vmStatic = new VmStatic();
Line 1466:
vmStatic.setOrigin(OriginType.valueOf(Config.<String>GetValue(ConfigValues.OriginType)));
Line 1460: VM currentVmData =
_vmDict.get(vmInternalData.getVmDynamic().getId());
Line 1461: if (currentVmData == null) {
Line 1462: if
(getDbFacade().getVmDao().get(vmInternalData.getVmDynamic().getId()) == null) {
Line 1463: Guid vmId =
vmInternalData.getVmDynamic().getId();
Line 1464: Map vmConfiguration =
getVmInfo(vmId.toString())[0];
maybe it would be better to call getVmInfo only once with all vms required, as
this goes to vdsm
Line 1465: VmStatic vmStatic = new VmStatic();
Line 1466:
vmStatic.setOrigin(OriginType.valueOf(Config.<String>GetValue(ConfigValues.OriginType)));
Line 1467: vmStatic.setId(vmId);
Line 1468: vmStatic.setQuotaId(null);
Line 1464: Map vmConfiguration =
getVmInfo(vmId.toString())[0];
Line 1465: VmStatic vmStatic = new VmStatic();
Line 1466:
vmStatic.setOrigin(OriginType.valueOf(Config.<String>GetValue(ConfigValues.OriginType)));
Line 1467: vmStatic.setId(vmId);
Line 1468: vmStatic.setQuotaId(null);
setting to null is as not setting at all, no?
Line 1469: vmStatic.setCreationDate(new Date());
Line 1470: vmStatic.setVdsGroupId(_vds.getVdsGroupId());
Line 1471: String vmNameOnHost = (String)
vmConfiguration.get(VdsProperties.vm_name);
Line 1472: String vmName = "external-" + vmNameOnHost;
Line 1474:
vmStatic.setNumOfSockets(Integer.parseInt((String)
vmConfiguration.get(VdsProperties.num_of_cpus)));
Line 1475: vmStatic.setMemSizeMb(Integer.parseInt((String)
vmConfiguration.get(VdsProperties.mem_size_mb)));
Line 1476: vmStatic.setExternallyManaged(true);
Line 1477: _externalVmsToAdd.add(vmStatic);
Line 1478: log.infoFormat("VDS::UpdateVmRunTimeInfo:
Importing VM {0} as {1}, as it is running on the on Host, but does not exist in
the engine.", vmNameOnHost, vmName);
no need to log "VDS::UpdateVmRunTimeInfo: " its part of the logger anyway
Line 1479: }
Line 1480: }
Line 1481: }
Line 1482: }
--
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: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Oved Ourfali <[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: oVirt Jenkins CI Server
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches
