Discussion:
[Engine-patches] Change in ovirt-engine[master]: core: enable pinning to multiple hosts
a***@ovirt.org
2015-06-04 15:42:07 UTC
Permalink
***@ovirt.org has posted comments on this change.

Change subject: core: enable pinning to multiple hosts
......................................................................


Patch Set 1:

* Update tracker::#1107512::OK
* Check Bug-Url::OK
* Check Public Bug::#1107512::OK, public bug
* Check Product::#1107512::OK, Correct product Red Hat Enterprise Virtualization Manager
* Check TR::SKIP, not in a monitored branch (ovirt-engine-3.5 ovirt-engine-3.4 ovirt-engine-3.3 engine_3.2 engine_3.1 ovirt-engine-3.5.2 ovirt-engine-3.4.0 ovirt-engine-3.3.4 ovirt-engine-3.3.3 ovirt-engine-3.3.2 ovirt-engine-3.3.1)
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-engine-3.5', 'ovirt-engine-3.4', 'ovirt-engine-3.3', 'ovirt-engine-3.5.2', 'ovirt-engine-3.4.0', 'ovirt-engine-3.3.4', 'ovirt-engine-3.3.3', 'ovirt-engine-3.3.2', 'ovirt-engine-3.3.1'])
--
To view, visit https://gerrit.ovirt.org/41962
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I710bd8d3505552a2a8d6060194d94c68c05445db
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Dudi Maroshi <***@redhat.com>
Gerrit-Reviewer: ***@ovirt.org
Gerrit-HasComments: No
a***@ovirt.org
2015-06-04 15:56:42 UTC
Permalink
***@ovirt.org has posted comments on this change.

Change subject: core: enable pinning to multiple hosts
......................................................................


Patch Set 2:

* Update tracker::#1107512::OK
* Check Bug-Url::OK
* Check Public Bug::#1107512::OK, public bug
* Check Product::#1107512::OK, Correct product Red Hat Enterprise Virtualization Manager
* Check TR::SKIP, not in a monitored branch (ovirt-engine-3.5 ovirt-engine-3.4 ovirt-engine-3.3 engine_3.2 engine_3.1 ovirt-engine-3.5.2 ovirt-engine-3.4.0 ovirt-engine-3.3.4 ovirt-engine-3.3.3 ovirt-engine-3.3.2 ovirt-engine-3.3.1)
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-engine-3.5', 'ovirt-engine-3.4', 'ovirt-engine-3.3', 'ovirt-engine-3.5.2', 'ovirt-engine-3.4.0', 'ovirt-engine-3.3.4', 'ovirt-engine-3.3.3', 'ovirt-engine-3.3.2', 'ovirt-engine-3.3.1'])
--
To view, visit https://gerrit.ovirt.org/41962
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I710bd8d3505552a2a8d6060194d94c68c05445db
Gerrit-PatchSet: 2
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Dudi Maroshi <***@redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: ***@ovirt.org
Gerrit-HasComments: No
a***@ovirt.org
2015-06-04 22:05:03 UTC
Permalink
***@ovirt.org has posted comments on this change.

Change subject: core: enable pinning to multiple hosts
......................................................................


Patch Set 3:

* Update tracker::#1107512::OK
* Check Bug-Url::OK
* Check Public Bug::#1107512::OK, public bug
* Check Product::#1107512::OK, Correct product Red Hat Enterprise Virtualization Manager
* Check TR::SKIP, not in a monitored branch (ovirt-engine-3.5 ovirt-engine-3.4 ovirt-engine-3.3 engine_3.2 engine_3.1 ovirt-engine-3.5.2 ovirt-engine-3.4.0 ovirt-engine-3.3.4 ovirt-engine-3.3.3 ovirt-engine-3.3.2 ovirt-engine-3.3.1)
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-engine-3.5', 'ovirt-engine-3.4', 'ovirt-engine-3.3', 'ovirt-engine-3.5.2', 'ovirt-engine-3.4.0', 'ovirt-engine-3.3.4', 'ovirt-engine-3.3.3', 'ovirt-engine-3.3.2', 'ovirt-engine-3.3.1'])
--
To view, visit https://gerrit.ovirt.org/41962
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I710bd8d3505552a2a8d6060194d94c68c05445db
Gerrit-PatchSet: 3
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Dudi Maroshi <***@redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: ***@ovirt.org
Gerrit-HasComments: No
a***@ovirt.org
2015-06-04 22:22:16 UTC
Permalink
***@ovirt.org has posted comments on this change.

Change subject: core: enable pinning to multiple hosts
......................................................................


Patch Set 4:

* Update tracker::#1107512::OK
* Check Bug-Url::OK
* Check Public Bug::#1107512::OK, public bug
* Check Product::#1107512::OK, Correct product Red Hat Enterprise Virtualization Manager
* Check TR::SKIP, not in a monitored branch (ovirt-engine-3.5 ovirt-engine-3.4 ovirt-engine-3.3 engine_3.2 engine_3.1 ovirt-engine-3.5.2 ovirt-engine-3.4.0 ovirt-engine-3.3.4 ovirt-engine-3.3.3 ovirt-engine-3.3.2 ovirt-engine-3.3.1)
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-engine-3.5', 'ovirt-engine-3.4', 'ovirt-engine-3.3', 'ovirt-engine-3.5.2', 'ovirt-engine-3.4.0', 'ovirt-engine-3.3.4', 'ovirt-engine-3.3.3', 'ovirt-engine-3.3.2', 'ovirt-engine-3.3.1'])
--
To view, visit https://gerrit.ovirt.org/41962
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I710bd8d3505552a2a8d6060194d94c68c05445db
Gerrit-PatchSet: 4
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Dudi Maroshi <***@redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: ***@ovirt.org
Gerrit-HasComments: No
a***@ovirt.org
2015-06-07 14:47:33 UTC
Permalink
***@ovirt.org has posted comments on this change.

Change subject: core: enable pinning to multiple hosts
......................................................................


Patch Set 5:

* Update tracker::#1107512::OK
* Check Bug-Url::OK
* Check Public Bug::#1107512::OK, public bug
* Check Product::#1107512::OK, Correct product Red Hat Enterprise Virtualization Manager
* Check TR::SKIP, not in a monitored branch (ovirt-engine-3.5 ovirt-engine-3.4 ovirt-engine-3.3 engine_3.2 engine_3.1 ovirt-engine-3.5.2 ovirt-engine-3.4.0 ovirt-engine-3.3.4 ovirt-engine-3.3.3 ovirt-engine-3.3.2 ovirt-engine-3.3.1)
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-engine-3.5', 'ovirt-engine-3.4', 'ovirt-engine-3.3', 'ovirt-engine-3.5.2', 'ovirt-engine-3.4.0', 'ovirt-engine-3.3.4', 'ovirt-engine-3.3.3', 'ovirt-engine-3.3.2', 'ovirt-engine-3.3.1'])
--
To view, visit https://gerrit.ovirt.org/41962
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I710bd8d3505552a2a8d6060194d94c68c05445db
Gerrit-PatchSet: 5
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Dudi Maroshi <***@redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Roy Golan <***@redhat.com>
Gerrit-Reviewer: ***@ovirt.org
Gerrit-HasComments: No
a***@ovirt.org
2015-06-07 19:22:46 UTC
Permalink
***@ovirt.org has posted comments on this change.

Change subject: core: enable pinning to multiple hosts
......................................................................


Patch Set 6:

* Update tracker::#1107512::OK
* Check Bug-Url::OK
* Check Public Bug::#1107512::OK, public bug
* Check Product::#1107512::OK, Correct product Red Hat Enterprise Virtualization Manager
* Check TR::SKIP, not in a monitored branch (ovirt-engine-3.5 ovirt-engine-3.4 ovirt-engine-3.3 engine_3.2 engine_3.1 ovirt-engine-3.5.2 ovirt-engine-3.4.0 ovirt-engine-3.3.4 ovirt-engine-3.3.3 ovirt-engine-3.3.2 ovirt-engine-3.3.1)
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-engine-3.5', 'ovirt-engine-3.4', 'ovirt-engine-3.3', 'ovirt-engine-3.5.2', 'ovirt-engine-3.4.0', 'ovirt-engine-3.3.4', 'ovirt-engine-3.3.3', 'ovirt-engine-3.3.2', 'ovirt-engine-3.3.1'])
--
To view, visit https://gerrit.ovirt.org/41962
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I710bd8d3505552a2a8d6060194d94c68c05445db
Gerrit-PatchSet: 6
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Dudi Maroshi <***@redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Roy Golan <***@redhat.com>
Gerrit-Reviewer: ***@ovirt.org
Gerrit-HasComments: No
a***@ovirt.org
2015-06-08 09:09:30 UTC
Permalink
***@ovirt.org has posted comments on this change.

Change subject: core: enable pinning to multiple hosts
......................................................................


Patch Set 7:

* Update tracker::#1107512::OK
* Check Bug-Url::OK
* Check Public Bug::#1107512::OK, public bug
* Check Product::#1107512::OK, Correct product Red Hat Enterprise Virtualization Manager
* Check TR::SKIP, not in a monitored branch (ovirt-engine-3.5 ovirt-engine-3.4 ovirt-engine-3.3 engine_3.2 engine_3.1 ovirt-engine-3.5.2 ovirt-engine-3.4.0 ovirt-engine-3.3.4 ovirt-engine-3.3.3 ovirt-engine-3.3.2 ovirt-engine-3.3.1)
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-engine-3.5', 'ovirt-engine-3.4', 'ovirt-engine-3.3', 'ovirt-engine-3.5.2', 'ovirt-engine-3.4.0', 'ovirt-engine-3.3.4', 'ovirt-engine-3.3.3', 'ovirt-engine-3.3.2', 'ovirt-engine-3.3.1'])
--
To view, visit https://gerrit.ovirt.org/41962
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I710bd8d3505552a2a8d6060194d94c68c05445db
Gerrit-PatchSet: 7
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Dudi Maroshi <***@redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Roy Golan <***@redhat.com>
Gerrit-Reviewer: ***@ovirt.org
Gerrit-HasComments: No
o***@redhat.com
2015-06-08 15:22:59 UTC
Permalink
Omer Frenkel has posted comments on this change.

Change subject: core: enable pinning to multiple hosts
......................................................................


Patch Set 7:

(1 comment)

https://gerrit.ovirt.org/#/c/41962/7//COMMIT_MSG
Commit Message:

Line 6:
Line 7: core: enable pinning to multiple hosts
Line 8:
Line 9: First patch to rattle the core DAO and BLL layers.
Line 10: 1. We refactor all aspects of vm_static.preferred_host field
the name is dedicated_vm_for_vds
also, usually many-to-many relation is structured to a lookup table in the db, why using text?
Line 11: Converting the field from uuid to text. Holding CSV of guids.
Line 12: 2. We refactor all Vm_base.dedicatedHost references and implementation.
Line 13: From Single guid to List<Guid>
Line 14:
--
To view, visit https://gerrit.ovirt.org/41962
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I710bd8d3505552a2a8d6060194d94c68c05445db
Gerrit-PatchSet: 7
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Dudi Maroshi <***@redhat.com>
Gerrit-Reviewer: Arik Hadas <***@redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Omer Frenkel <***@redhat.com>
Gerrit-Reviewer: Roy Golan <***@redhat.com>
Gerrit-Reviewer: Tomer Saban <***@redhat.com>
Gerrit-Reviewer: ***@ovirt.org
Gerrit-HasComments: Yes
a***@ovirt.org
2015-06-08 16:56:15 UTC
Permalink
***@ovirt.org has posted comments on this change.

Change subject: core: enable pinning to multiple hosts
......................................................................


Patch Set 8:

* Update tracker::#1107512::OK
* Check Bug-Url::OK
* Check Public Bug::#1107512::OK, public bug
* Check Product::#1107512::OK, Correct product Red Hat Enterprise Virtualization Manager
* Check TR::SKIP, not in a monitored branch (ovirt-engine-3.5 ovirt-engine-3.4 ovirt-engine-3.3 engine_3.2 engine_3.1 ovirt-engine-3.5.2 ovirt-engine-3.4.0 ovirt-engine-3.3.4 ovirt-engine-3.3.3 ovirt-engine-3.3.2 ovirt-engine-3.3.1)
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-engine-3.5', 'ovirt-engine-3.4', 'ovirt-engine-3.3', 'ovirt-engine-3.5.2', 'ovirt-engine-3.4.0', 'ovirt-engine-3.3.4', 'ovirt-engine-3.3.3', 'ovirt-engine-3.3.2', 'ovirt-engine-3.3.1'])
--
To view, visit https://gerrit.ovirt.org/41962
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I710bd8d3505552a2a8d6060194d94c68c05445db
Gerrit-PatchSet: 8
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Dudi Maroshi <***@redhat.com>
Gerrit-Reviewer: Arik Hadas <***@redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Omer Frenkel <***@redhat.com>
Gerrit-Reviewer: Roy Golan <***@redhat.com>
Gerrit-Reviewer: Tomer Saban <***@redhat.com>
Gerrit-Reviewer: ***@ovirt.org
Gerrit-HasComments: No
m***@redhat.com
2015-06-09 13:26:57 UTC
Permalink
Martin Betak has posted comments on this change.

Change subject: core: enable pinning to multiple hosts
......................................................................


Patch Set 8: Code-Review-1

This patch breaks a lot of existing host device workflows. In "direct passthrough" host device code we strongly assume that there will be exactly one pinned host for VM with hostdev passthrough. This restriction will be hopefully dropped in the 4.0 timeframe, but I don't think we can make the careful analysis/rethink of host devices required in time for 3.6.

Adding -1 just for visibility
--
To view, visit https://gerrit.ovirt.org/41962
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I710bd8d3505552a2a8d6060194d94c68c05445db
Gerrit-PatchSet: 8
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Dudi Maroshi <***@redhat.com>
Gerrit-Reviewer: Alona Kaplan <***@redhat.com>
Gerrit-Reviewer: Arik Hadas <***@redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Betak <***@redhat.com>
Gerrit-Reviewer: Martin Sivák <***@redhat.com>
Gerrit-Reviewer: Omer Frenkel <***@redhat.com>
Gerrit-Reviewer: Roy Golan <***@redhat.com>
Gerrit-Reviewer: Tomer Saban <***@redhat.com>
Gerrit-Reviewer: ***@ovirt.org
Gerrit-HasComments: No
r***@redhat.com
2015-06-09 14:31:23 UTC
Permalink
Roy Golan has posted comments on this change.

Change subject: core: enable pinning to multiple hosts
......................................................................


Patch Set 8:

Martin we should probably make pin to hosts and host-devices mutually exclusive and block using one of them if one is already in use
--
To view, visit https://gerrit.ovirt.org/41962
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I710bd8d3505552a2a8d6060194d94c68c05445db
Gerrit-PatchSet: 8
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Dudi Maroshi <***@redhat.com>
Gerrit-Reviewer: Alona Kaplan <***@redhat.com>
Gerrit-Reviewer: Arik Hadas <***@redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Betak <***@redhat.com>
Gerrit-Reviewer: Martin Sivák <***@redhat.com>
Gerrit-Reviewer: Omer Frenkel <***@redhat.com>
Gerrit-Reviewer: Roy Golan <***@redhat.com>
Gerrit-Reviewer: Tomer Saban <***@redhat.com>
Gerrit-Reviewer: ***@ovirt.org
Gerrit-HasComments: No
m***@redhat.com
2015-06-09 14:36:05 UTC
Permalink
Martin Betak has posted comments on this change.

Change subject: core: enable pinning to multiple hosts
......................................................................


Patch Set 8:

Roy: yes, I agree that this is probably the best solution in the short term
--
To view, visit https://gerrit.ovirt.org/41962
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I710bd8d3505552a2a8d6060194d94c68c05445db
Gerrit-PatchSet: 8
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Dudi Maroshi <***@redhat.com>
Gerrit-Reviewer: Alona Kaplan <***@redhat.com>
Gerrit-Reviewer: Arik Hadas <***@redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Betak <***@redhat.com>
Gerrit-Reviewer: Martin Sivák <***@redhat.com>
Gerrit-Reviewer: Omer Frenkel <***@redhat.com>
Gerrit-Reviewer: Roy Golan <***@redhat.com>
Gerrit-Reviewer: Tomer Saban <***@redhat.com>
Gerrit-Reviewer: ***@ovirt.org
Gerrit-HasComments: No
r***@redhat.com
2015-06-09 16:19:11 UTC
Permalink
Roy Golan has posted comments on this change.

Change subject: core: enable pinning to multiple hosts
......................................................................


Patch Set 8:

(12 comments)

partial comments

https://gerrit.ovirt.org/#/c/41962/8/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmCommand.java
File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmCommand.java:

Line 1034: maroshi
no names in comment

lets discuss with mbetak that subjects and see where is this going


https://gerrit.ovirt.org/#/c/41962/8/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ChangeVMClusterCommand.java
File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ChangeVMClusterCommand.java:

Line 83: getDedicatedVmForVdsList()
if we went for a rename of that getter, lets just call it some other sane name

getHosts() seems perfectly fine as it depends on the migrationPolicy if its preffered or pinned.


https://gerrit.ovirt.org/#/c/41962/8/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ImportVmCommandBase.java
File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ImportVmCommandBase.java:

Line 274: new LinkedList<Guid>
why LinkedList and not ArrayList?


https://gerrit.ovirt.org/#/c/41962/8/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/MigrateVmCommand.java
File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/MigrateVmCommand.java:

Line 116: List<Guid> destinationHostGuidList = new LinkedList<>();
: if (getDestinationVdsId() != null){
: destinationHostGuidList.add(getDestinationVdsId());
: }
that code is duplicated in line 440. why can't it be a part of the old getDestinationVdsId?


https://gerrit.ovirt.org/#/c/41962/8/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/UpdateVmTemplateCommand.java
File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/UpdateVmTemplateCommand.java:

Line 361: if (getVmTemplate() != null && !isInstanceType() && !isBlankTemplate()) {
Line 362: // host-specific parameters can be changed by administration role only
Line 363: List<Guid> tmpltDdctHostsLst = getVmTemplate().getDedicatedVmForVdsList();
Line 364: List<Guid> prmTmpltDdctHostsLst = getParameters().getVmTemplateData().getDedicatedVmForVdsList();
Line 365: // tmpltDdctHostsLst.equals(prmTmpltDdctHostsLs is not good enough, lists order may change
this check is probably viable if we have only one host in the list and it was changed
Line 366: if (CollectionUtils.isEqualCollection(tmpltDdctHostsLst, prmTmpltDdctHostsLst) == false) {
Line 367: permissionList.add(
Line 368: new PermissionSubject(getParameters().getVmTemplateId(),
Line 369: VdcObjectType.VmTemplate,


https://gerrit.ovirt.org/#/c/41962/8/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VmManagementCommandBase.java
File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VmManagementCommandBase.java:

Line 108: if (vmStatic.getDedicatedVmForVdsList().isEmpty()){
Line 109: return failCanDoAction(VdcBllMessages.ACTION_TYPE_FAILED_VM_CANNOT_BE_PINNED_TO_CPU_WITH_UNDEFINED_HOST);
Line 110: }
Line 111: // TODO maroshi - validate, need to check each dedicated host? Maybe vmStatic.getRunOnVds() ?
Line 112: for (Guid dedicatedHostGuid : vmStatic.getDedicatedVmForVdsList()){
suggest refactoring this out, same as VmHandler.validateDedicatedVdsExistOnSameCluster
Line 113: dedicatedVds = getVds(dedicatedHostGuid);
Line 114:
Line 115: Collection<Integer> onlinePcpus = new HashSet<>();
Line 116:


https://gerrit.ovirt.org/#/c/41962/8/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/hostdev/AbstractVmHostDevicesCommand.java
File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/hostdev/AbstractVmHostDevicesCommand.java:

Line 12: import org.ovirt.engine.core.dao.HostDeviceDao;
Line 13: import org.ovirt.engine.core.dao.VmDeviceDAO;
Line 14:
Line 15: import javax.inject.Inject;
Line 16:
remove
Line 17: import java.util.ArrayList;
Line 18: import java.util.Collection;
Line 19: import java.util.Collections;
Line 20: import java.util.HashSet;


Line 105: }
Line 106: return null;
Line 107: }
Line 108:
Line 109: private HostDevice fetchHostDevice(String deviceName) {
again proposing this is valid when a single host
Line 110: // TODO maroshi - redesign query, return from all pinned hosts,
Line 111: // or from getVm().getRunOnVds(), or other hosts?
Line 112: for (Guid hostId : getVm().getDedicatedVmForVdsList()){
Line 113: HostDevice hostDevice = hostDeviceDao.getHostDeviceByHostIdAndDeviceName(hostId, deviceName);


https://gerrit.ovirt.org/#/c/41962/8/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/hostdev/HostDeviceManager.java
File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/hostdev/HostDeviceManager.java:

Line 69: * @param vm
Line 70: * @return true if the specified VM is pinned to a host and has host devices directly attached to it
Line 71: */
Line 72: public boolean checkVmNeedsDirectPassthrough(VM vm) {
Line 73: return vm.getDedicatedVmForVdsList().isEmpty() == false && supportsHostDevicePassthrough(vm) &&
that should be changed as well
Line 74: checkVmNeedsDirectPassthrough(vm.getId());
Line 75: }
Line 76:
Line 77: private boolean checkVmNeedsDirectPassthrough(Guid vmId) {


https://gerrit.ovirt.org/#/c/41962/8/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/numa/vm/AbstractVmNumaNodeCommand.java
File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/numa/vm/AbstractVmNumaNodeCommand.java:

Line 73: );
off topic but not completly -
we should fetch that config value with version -


Config.<Boolean> getValue(SupportNUMAMigration, vm.getCompatiblityVersion())


Line 71: }
Line 72:
Line 73: boolean pinHost = !Config.<Boolean> getValue(ConfigValues.SupportNUMAMigration);
Line 74: List<VdsNumaNode> hostNumaNodes = new ArrayList<>();
Line 75: for (Guid vdsId : getDedicatedHostList()) {
same thing with Numa - we should allow numa-pinning only when there is one host in the list, otherwise we can't determine how to use the UI of numa pinning
Line 76: if (pinHost && vdsId == null && getMigrationSupport() != MigrationSupport.PINNED_TO_HOST) {
Line 77: return failCanDoAction(VdcBllMessages.VM_NUMA_PINNED_VDS_NOT_EXIST);
Line 78: }
Line 79:


https://gerrit.ovirt.org/#/c/41962/8/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/scheduling/policyunits/PinToHostPolicyUnit.java
File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/scheduling/policyunits/PinToHostPolicyUnit.java:

Line 24: public List<VDS> filter(List<VDS> hosts, VM vm, Map<String, String> parameters, PerHostMessages messages) {
Line 25: if (vm.getMigrationSupport() == MigrationSupport.PINNED_TO_HOST) {
Line 26: // host has been specified for pin to host.
Line 27: if(vm.getDedicatedVmForVdsList().isEmpty() == false) {
Line 28: for (VDS host : hosts) {
I guess your next patches should take this into account and return all hosts which match
Line 29: if (vm.getDedicatedVmForVdsList().contains(host.getId())) {
Line 30: return Arrays.asList(host);
Line 31: }
Line 32: }
--
To view, visit https://gerrit.ovirt.org/41962
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I710bd8d3505552a2a8d6060194d94c68c05445db
Gerrit-PatchSet: 8
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Dudi Maroshi <***@redhat.com>
Gerrit-Reviewer: Alona Kaplan <***@redhat.com>
Gerrit-Reviewer: Arik Hadas <***@redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Betak <***@redhat.com>
Gerrit-Reviewer: Martin Sivák <***@redhat.com>
Gerrit-Reviewer: Omer Frenkel <***@redhat.com>
Gerrit-Reviewer: Roy Golan <***@redhat.com>
Gerrit-Reviewer: Tomer Saban <***@redhat.com>
Gerrit-Reviewer: ***@ovirt.org
Gerrit-HasComments: Yes
d***@redhat.com
2015-06-10 08:16:24 UTC
Permalink
Dudi Maroshi has posted comments on this change.

Change subject: core: enable pinning to multiple hosts
......................................................................


Patch Set 8:

(12 comments)

https://gerrit.ovirt.org/#/c/41962/8/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmCommand.java
File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmCommand.java:

Line 1030: protected void addVmNumaNodes() {
Line 1031: List<VmNumaNode> numaNodes = getParameters().getVmStaticData().getvNumaNodeList();
Line 1032: VmNumaNodeOperationParameters params = new VmNumaNodeOperationParameters(getVmId(), numaNodes);
Line 1033: params.setNumaTuneMode(getParameters().getVmStaticData().getNumaTuneMode());
Line 1034: // TODO maroshi - redesign NUMAfor multiple hosts pinning
Post by r***@redhat.com
no names in comment
Done
Line 1035: params.setDedicatedHostList(getParameters().getVmStaticData().getDedicatedVmForVdsList());
Line 1036: params.setMigrationSupport(getParameters().getVmStaticData().getMigrationSupport());
Line 1037: if (numaNodes == null || numaNodes.isEmpty()) {
Line 1038: return;


https://gerrit.ovirt.org/#/c/41962/8/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ChangeVMClusterCommand.java
File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ChangeVMClusterCommand.java:

Line 79: }
Line 80: }
Line 81: }
Line 82:
Line 83: if (vm.getDedicatedVmForVdsList().isEmpty() == false) {
Post by r***@redhat.com
if we went for a rename of that getter, lets just call it some other sane n
I agree, it is a good idea to rethink the property name.

I still want the name to reflect the multiplicity. Adding an (s) is not good enough and error prone (for reading and writing).
Line 84: vm.setDedicatedVmForVdsList(new LinkedList<Guid>());
Line 85: dedicatedHostWasCleared = true;
Line 86: }
Line 87:


https://gerrit.ovirt.org/#/c/41962/8/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ImportVmCommandBase.java
File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ImportVmCommandBase.java:

Line 270: getVm().getStaticData().setQuotaId(getParameters().getQuotaId());
Line 271:
Line 272: // if "run on host" field points to a non existent vds (in the current cluster) -> remove field and continue
Line 273: if (!VmHandler.validateDedicatedVdsExistOnSameCluster(getVm().getStaticData(), null)) {
Line 274: getVm().setDedicatedVmForVdsList(new LinkedList<Guid>());
Post by r***@redhat.com
why LinkedList and not ArrayList?
1. ArryList by default initially allocate 10 objects.

2. We do not access list items by index and it is not ordered (always iterator).


The comment is valid, we want to be consistent with LinkedList or ArrayList.
Will fix that.

We also want to insert static empty list.
Will fix that as well.
Line 275: }
Line 276:
Line 277: if (getVm().getOriginalTemplateGuid() != null && !VmTemplateHandler.BLANK_VM_TEMPLATE_ID.equals(getVm().getOriginalTemplateGuid())) {
Line 278: // no need to check this for blank


https://gerrit.ovirt.org/#/c/41962/8/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/MigrateVmCommand.java
File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/MigrateVmCommand.java:

Line 115: setVdsIdRef(getVm().getRunOnVds());
Line 116: List<Guid> destinationHostGuidList = new LinkedList<>();
Line 117: if (getDestinationVdsId() != null){
Line 118: destinationHostGuidList.add(getDestinationVdsId());
Line 119: }
Post by r***@redhat.com
that code is duplicated in line 440. why can't it be a part of the old getD
Done
Line 120: Guid vdsToRunOn =
Line 121: SchedulingManager.getInstance().schedule(getVdsGroup(),
Line 122: getVm(),
Line 123: getVdsBlackList(),


https://gerrit.ovirt.org/#/c/41962/8/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/UpdateVmTemplateCommand.java
File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/UpdateVmTemplateCommand.java:

Line 361: if (getVmTemplate() != null && !isInstanceType() && !isBlankTemplate()) {
Line 362: // host-specific parameters can be changed by administration role only
Line 363: List<Guid> tmpltDdctHostsLst = getVmTemplate().getDedicatedVmForVdsList();
Line 364: List<Guid> prmTmpltDdctHostsLst = getParameters().getVmTemplateData().getDedicatedVmForVdsList();
Line 365: // tmpltDdctHostsLst.equals(prmTmpltDdctHostsLs is not good enough, lists order may change
Post by r***@redhat.com
this check is probably viable if we have only one host in the list and it w
Need to discuss this
Line 366: if (CollectionUtils.isEqualCollection(tmpltDdctHostsLst, prmTmpltDdctHostsLst) == false) {
Line 367: permissionList.add(
Line 368: new PermissionSubject(getParameters().getVmTemplateId(),
Line 369: VdcObjectType.VmTemplate,


https://gerrit.ovirt.org/#/c/41962/8/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VmManagementCommandBase.java
File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VmManagementCommandBase.java:

Line 108: if (vmStatic.getDedicatedVmForVdsList().isEmpty()){
Line 109: return failCanDoAction(VdcBllMessages.ACTION_TYPE_FAILED_VM_CANNOT_BE_PINNED_TO_CPU_WITH_UNDEFINED_HOST);
Line 110: }
Line 111: // TODO maroshi - validate, need to check each dedicated host? Maybe vmStatic.getRunOnVds() ?
Line 112: for (Guid dedicatedHostGuid : vmStatic.getDedicatedVmForVdsList()){
Post by r***@redhat.com
suggest refactoring this out, same as VmHandler.validateDedicatedVdsExistOn
Done
Line 113: dedicatedVds = getVds(dedicatedHostGuid);
Line 114:
Line 115: Collection<Integer> onlinePcpus = new HashSet<>();
Line 116:


https://gerrit.ovirt.org/#/c/41962/8/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/hostdev/AbstractVmHostDevicesCommand.java
File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/hostdev/AbstractVmHostDevicesCommand.java:

Line 12: import org.ovirt.engine.core.dao.HostDeviceDao;
Line 13: import org.ovirt.engine.core.dao.VmDeviceDAO;
Line 14:
Line 15: import javax.inject.Inject;
Post by r***@redhat.com
remove
Done
Line 17: import java.util.ArrayList;
Line 18: import java.util.Collection;
Line 19: import java.util.Collections;
Line 20: import java.util.HashSet;


Line 105: }
Line 106: return null;
Line 107: }
Line 108:
Line 109: private HostDevice fetchHostDevice(String deviceName) {
Post by r***@redhat.com
again proposing this is valid when a single host
Done
Line 110: // TODO maroshi - redesign query, return from all pinned hosts,
Line 111: // or from getVm().getRunOnVds(), or other hosts?
Line 112: for (Guid hostId : getVm().getDedicatedVmForVdsList()){
Line 113: HostDevice hostDevice = hostDeviceDao.getHostDeviceByHostIdAndDeviceName(hostId, deviceName);


https://gerrit.ovirt.org/#/c/41962/8/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/hostdev/HostDeviceManager.java
File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/hostdev/HostDeviceManager.java:

Line 69: * @param vm
Line 70: * @return true if the specified VM is pinned to a host and has host devices directly attached to it
Line 71: */
Line 72: public boolean checkVmNeedsDirectPassthrough(VM vm) {
Line 73: return vm.getDedicatedVmForVdsList().isEmpty() == false && supportsHostDevicePassthrough(vm) &&
Post by r***@redhat.com
that should be changed as well
Done
Line 74: checkVmNeedsDirectPassthrough(vm.getId());
Line 75: }
Line 76:
Line 77: private boolean checkVmNeedsDirectPassthrough(Guid vmId) {


https://gerrit.ovirt.org/#/c/41962/8/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/numa/vm/AbstractVmNumaNodeCommand.java
File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/numa/vm/AbstractVmNumaNodeCommand.java:

Line 69: // if VM do not contain any NUMA node, skip checking
Line 70: return true;
Line 71: }
Line 72:
Line 73: boolean pinHost = !Config.<Boolean> getValue(ConfigValues.SupportNUMAMigration);
Post by r***@redhat.com
off topic but not completly -
Done
Line 74: List<VdsNumaNode> hostNumaNodes = new ArrayList<>();
Line 75: for (Guid vdsId : getDedicatedHostList()) {
Line 76: if (pinHost && vdsId == null && getMigrationSupport() != MigrationSupport.PINNED_TO_HOST) {
Line 77: return failCanDoAction(VdcBllMessages.VM_NUMA_PINNED_VDS_NOT_EXIST);


Line 71: }
Line 72:
Line 73: boolean pinHost = !Config.<Boolean> getValue(ConfigValues.SupportNUMAMigration);
Line 74: List<VdsNumaNode> hostNumaNodes = new ArrayList<>();
Line 75: for (Guid vdsId : getDedicatedHostList()) {
Post by r***@redhat.com
same thing with Numa - we should allow numa-pinning only when there is one
We need to define a clear policy for multiple pinning to NUMA.

This requires a deeper discussion.
Line 76: if (pinHost && vdsId == null && getMigrationSupport() != MigrationSupport.PINNED_TO_HOST) {
Line 77: return failCanDoAction(VdcBllMessages.VM_NUMA_PINNED_VDS_NOT_EXIST);
Line 78: }
Line 79:


https://gerrit.ovirt.org/#/c/41962/8/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/scheduling/policyunits/PinToHostPolicyUnit.java
File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/scheduling/policyunits/PinToHostPolicyUnit.java:

Line 24: public List<VDS> filter(List<VDS> hosts, VM vm, Map<String, String> parameters, PerHostMessages messages) {
Line 25: if (vm.getMigrationSupport() == MigrationSupport.PINNED_TO_HOST) {
Line 26: // host has been specified for pin to host.
Line 27: if(vm.getDedicatedVmForVdsList().isEmpty() == false) {
Line 28: for (VDS host : hosts) {
Post by r***@redhat.com
I guess your next patches should take this into account and return all host
Done
Line 29: if (vm.getDedicatedVmForVdsList().contains(host.getId())) {
Line 30: return Arrays.asList(host);
Line 31: }
Line 32: }
--
To view, visit https://gerrit.ovirt.org/41962
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I710bd8d3505552a2a8d6060194d94c68c05445db
Gerrit-PatchSet: 8
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Dudi Maroshi <***@redhat.com>
Gerrit-Reviewer: Alona Kaplan <***@redhat.com>
Gerrit-Reviewer: Arik Hadas <***@redhat.com>
Gerrit-Reviewer: Dudi Maroshi <***@redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Betak <***@redhat.com>
Gerrit-Reviewer: Martin Sivák <***@redhat.com>
Gerrit-Reviewer: Omer Frenkel <***@redhat.com>
Gerrit-Reviewer: Roy Golan <***@redhat.com>
Gerrit-Reviewer: Tomer Saban <***@redhat.com>
Gerrit-Reviewer: ***@ovirt.org
Gerrit-HasComments: Yes
a***@ovirt.org
2015-06-12 19:29:05 UTC
Permalink
***@ovirt.org has posted comments on this change.

Change subject: core: enable pinning to multiple hosts
......................................................................


Patch Set 9:

* Update tracker::#1107512::OK
* Check Bug-Url::OK
* Check Public Bug::#1107512::OK, public bug
* Check Product::#1107512::OK, Correct product Red Hat Enterprise Virtualization Manager
* Check TR::SKIP, not in a monitored branch (ovirt-engine-3.5 ovirt-engine-3.4 ovirt-engine-3.3 engine_3.2 engine_3.1 ovirt-engine-3.5.2 ovirt-engine-3.4.0 ovirt-engine-3.3.4 ovirt-engine-3.3.3 ovirt-engine-3.3.2 ovirt-engine-3.3.1)
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-engine-3.5', 'ovirt-engine-3.4', 'ovirt-engine-3.3', 'ovirt-engine-3.5.2', 'ovirt-engine-3.4.0', 'ovirt-engine-3.3.4', 'ovirt-engine-3.3.3', 'ovirt-engine-3.3.2', 'ovirt-engine-3.3.1'])
--
To view, visit https://gerrit.ovirt.org/41962
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I710bd8d3505552a2a8d6060194d94c68c05445db
Gerrit-PatchSet: 9
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Dudi Maroshi <***@redhat.com>
Gerrit-Reviewer: Alona Kaplan <***@redhat.com>
Gerrit-Reviewer: Arik Hadas <***@redhat.com>
Gerrit-Reviewer: Dudi Maroshi <***@redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Betak <***@redhat.com>
Gerrit-Reviewer: Martin Sivák <***@redhat.com>
Gerrit-Reviewer: Omer Frenkel <***@redhat.com>
Gerrit-Reviewer: Roy Golan <***@redhat.com>
Gerrit-Reviewer: Tomer Saban <***@redhat.com>
Gerrit-Reviewer: ***@ovirt.org
Gerrit-HasComments: No
a***@ovirt.org
2015-06-13 16:59:14 UTC
Permalink
***@ovirt.org has posted comments on this change.

Change subject: core: enable pinning to multiple hosts
......................................................................


Patch Set 10:

* Update tracker::#1107512::OK
* Check Bug-Url::OK
* Check Public Bug::#1107512::OK, public bug
* Check Product::#1107512::OK, Correct product Red Hat Enterprise Virtualization Manager
* Check TR::SKIP, not in a monitored branch (ovirt-engine-3.5 ovirt-engine-3.4 ovirt-engine-3.3 engine_3.2 engine_3.1 ovirt-engine-3.5.2 ovirt-engine-3.4.0 ovirt-engine-3.3.4 ovirt-engine-3.3.3 ovirt-engine-3.3.2 ovirt-engine-3.3.1)
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-engine-3.5', 'ovirt-engine-3.4', 'ovirt-engine-3.3', 'ovirt-engine-3.5.2', 'ovirt-engine-3.4.0', 'ovirt-engine-3.3.4', 'ovirt-engine-3.3.3', 'ovirt-engine-3.3.2', 'ovirt-engine-3.3.1'])
--
To view, visit https://gerrit.ovirt.org/41962
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I710bd8d3505552a2a8d6060194d94c68c05445db
Gerrit-PatchSet: 10
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Dudi Maroshi <***@redhat.com>
Gerrit-Reviewer: Alona Kaplan <***@redhat.com>
Gerrit-Reviewer: Arik Hadas <***@redhat.com>
Gerrit-Reviewer: Dudi Maroshi <***@redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Betak <***@redhat.com>
Gerrit-Reviewer: Martin Sivák <***@redhat.com>
Gerrit-Reviewer: Omer Frenkel <***@redhat.com>
Gerrit-Reviewer: Roy Golan <***@redhat.com>
Gerrit-Reviewer: Tomer Saban <***@redhat.com>
Gerrit-Reviewer: ***@ovirt.org
Gerrit-HasComments: No
a***@ovirt.org
2015-06-13 17:24:58 UTC
Permalink
***@ovirt.org has posted comments on this change.

Change subject: core: enable pinning to multiple hosts
......................................................................


Patch Set 11:

* Update tracker::#1107512::OK
* Check Bug-Url::OK
* Check Public Bug::#1107512::OK, public bug
* Check Product::#1107512::OK, Correct product Red Hat Enterprise Virtualization Manager
* Check TR::SKIP, not in a monitored branch (ovirt-engine-3.5 ovirt-engine-3.4 ovirt-engine-3.3 engine_3.2 engine_3.1 ovirt-engine-3.5.2 ovirt-engine-3.4.0 ovirt-engine-3.3.4 ovirt-engine-3.3.3 ovirt-engine-3.3.2 ovirt-engine-3.3.1)
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-engine-3.5', 'ovirt-engine-3.4', 'ovirt-engine-3.3', 'ovirt-engine-3.5.2', 'ovirt-engine-3.4.0', 'ovirt-engine-3.3.4', 'ovirt-engine-3.3.3', 'ovirt-engine-3.3.2', 'ovirt-engine-3.3.1'])
--
To view, visit https://gerrit.ovirt.org/41962
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I710bd8d3505552a2a8d6060194d94c68c05445db
Gerrit-PatchSet: 11
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Dudi Maroshi <***@redhat.com>
Gerrit-Reviewer: Alona Kaplan <***@redhat.com>
Gerrit-Reviewer: Arik Hadas <***@redhat.com>
Gerrit-Reviewer: Dudi Maroshi <***@redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Betak <***@redhat.com>
Gerrit-Reviewer: Martin Sivák <***@redhat.com>
Gerrit-Reviewer: Omer Frenkel <***@redhat.com>
Gerrit-Reviewer: Roy Golan <***@redhat.com>
Gerrit-Reviewer: Tomer Saban <***@redhat.com>
Gerrit-Reviewer: ***@ovirt.org
Gerrit-HasComments: No
r***@redhat.com
2015-06-15 10:56:44 UTC
Permalink
Roy Golan has posted comments on this change.

Change subject: core: enable pinning to multiple hosts
......................................................................


Patch Set 11:

(7 comments)

needs UI and REST

https://gerrit.ovirt.org/#/c/41962/11/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ChangeVMClusterCommand.java
File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ChangeVMClusterCommand.java:

Line 84: <Guid>
emptyList is generic method so the cast it unnecessary.


https://gerrit.ovirt.org/#/c/41962/11/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/UpdateVmCommand.java
File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/UpdateVmCommand.java:

Line 923: // vmList.equals(paramList) not good enough, the lists order could change
Line 924: if (vmList.size() != paramList.size()){
Line 925: return true;
Line 926: }
Line 927: for (Guid origGuid : vmList) {
need to make sure we ignore nulls
Line 928: if (paramList.contains(origGuid) == false){
Line 929: return true;
Line 930: }
Line 931: }


https://gerrit.ovirt.org/#/c/41962/11/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/numa/vm/AbstractVmNumaNodeCommand.java
File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/numa/vm/AbstractVmNumaNodeCommand.java:

Line 72: numaMigrationSupported
this rename is really awkward, really.


https://gerrit.ovirt.org/#/c/41962/11/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/scheduling/policyunits/PinToHostPolicyUnit.java
File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/scheduling/policyunits/PinToHostPolicyUnit.java:

Line 39: }
Line 40: }
Line 41:
Line 42: // if flow reaches here, the VM is pinned but there is no dedicated host.
Line 43: return Collections.<VDS>emptyList();
missing a space there and the Generic VDS decleration is redundant as emptyList is generic
Line 44: }
Line 45:
Line 46: return hosts;
Line 47: }


https://gerrit.ovirt.org/#/c/41962/11/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/BaseDAODbFacade.java
File backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/BaseDAODbFacade.java:

Line 218:
Line 219: return new ArrayList<String>(Arrays.asList(str.split(SEPARATOR)));
Line 220: }
Line 221:
Line 222: static public List<Guid> convertStringListToGuidList(List<String> stringList){
see GuidUtils.getGuidListFromStringArray
Line 223: List<Guid> guidsList = new ArrayList<>();
Line 224: if (stringList == null || stringList.isEmpty()){
Line 225: return guidsList;
Line 226: }


https://gerrit.ovirt.org/#/c/41962/11/backend/manager/modules/dal/src/main/resources/bundles/AppErrors.properties
File backend/manager/modules/dal/src/main/resources/bundles/AppErrors.properties:

Line 1445: ACTION_TYPE_FAILED_SCSI_RESERVATION_NOT_VALID_FOR_FLOATING_DISK=Cannot ${action} ${type}. SCSI reservation cannot be set when adding floating disks.
Line 1446: ACTION_TYPE_FAILED_SGIO_IS_FILTERED=Cannot ${action} ${type}. SCSI reservation can be set only when SGIO is unfiltered.
Line 1447:
Line 1448: ACTION_TYPE_FAILED_VM_NOT_PINNED_TO_HOST=Cannot ${action} ${type}. VM must be pinned to a host.
Line 1449: ACTION_TYPE_FAILED_VM_PINNED_TO_MULTIPLE_HOSTS=Cannot ${action} ${type}. VM must be pinned to a single host.
we need to add why its not allowed to multi-pin (numa|host-device)
Line 1450: ACTION_TYPE_FAILED_HOST_DEVICE_NOT_FOUND=Cannot ${action} ${type}. One or more of specified host devices not found.
Line 1451: ACTION_TYPE_FAILED_HOST_DEVICE_NOT_AVAILABLE=Cannot ${action} ${type}. One or more configured host devices are unavailable.
Line 1452:
Line 1453: # vm icons


https://gerrit.ovirt.org/#/c/41962/11/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/Linq.java
File frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/Linq.java:

Line 1655: }
Line 1656: });
Line 1657: }
Line 1658:
Line 1659: public static VDS findHostByIdFromIdList(Collection<VDS> items, List<Guid> hostIdList) {
consider changing this to varargs so the API still remains the same for callers with a single VM
Line 1660: for (VDS host : items) {
Line 1661: for (Guid hostId : hostIdList) {
Line 1662: if (host.getId().equals(hostId)) {
Line 1663: return host;
--
To view, visit https://gerrit.ovirt.org/41962
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I710bd8d3505552a2a8d6060194d94c68c05445db
Gerrit-PatchSet: 11
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Dudi Maroshi <***@redhat.com>
Gerrit-Reviewer: Alona Kaplan <***@redhat.com>
Gerrit-Reviewer: Arik Hadas <***@redhat.com>
Gerrit-Reviewer: Dudi Maroshi <***@redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Betak <***@redhat.com>
Gerrit-Reviewer: Martin Sivák <***@redhat.com>
Gerrit-Reviewer: Omer Frenkel <***@redhat.com>
Gerrit-Reviewer: Roy Golan <***@redhat.com>
Gerrit-Reviewer: Tomer Saban <***@redhat.com>
Gerrit-Reviewer: ***@ovirt.org
Gerrit-HasComments: Yes
d***@redhat.com
2015-06-15 14:54:41 UTC
Permalink
Dudi Maroshi has posted comments on this change.

Change subject: core: enable pinning to multiple hosts
......................................................................


Patch Set 11:

(7 comments)

https://gerrit.ovirt.org/#/c/41962/11/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ChangeVMClusterCommand.java
File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ChangeVMClusterCommand.java:

Line 84: <Guid>
Post by r***@redhat.com
emptyList is generic method so the cast it unnecessary.
vm.setDedicatedVmForVdsList(Collections.emptyList());

Is a compilation error:
The method setDedicatedVmForVdsList(List<Guid>) in the type VM is not applicable for the arguments (List<Object>)


https://gerrit.ovirt.org/#/c/41962/11/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/UpdateVmCommand.java
File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/UpdateVmCommand.java:

Line 923: // vmList.equals(paramList) not good enough, the lists order could change
Line 924: if (vmList.size() != paramList.size()){
Line 925: return true;
Line 926: }
Line 927: for (Guid origGuid : vmList) {
Post by r***@redhat.com
need to make sure we ignore nulls
Done
Line 928: if (paramList.contains(origGuid) == false){
Line 929: return true;
Line 930: }
Line 931: }


https://gerrit.ovirt.org/#/c/41962/11/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/numa/vm/AbstractVmNumaNodeCommand.java
File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/numa/vm/AbstractVmNumaNodeCommand.java:

Line 72: numaMigrationSupported
Post by r***@redhat.com
this rename is really awkward, really.
Done


https://gerrit.ovirt.org/#/c/41962/11/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/scheduling/policyunits/PinToHostPolicyUnit.java
File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/scheduling/policyunits/PinToHostPolicyUnit.java:

Line 39: }
Line 40: }
Line 41:
Line 42: // if flow reaches here, the VM is pinned but there is no dedicated host.
Line 43: return Collections.<VDS>emptyList();
Post by r***@redhat.com
missing a space there and the Generic VDS decleration is redundant as empty
Done
Line 44: }
Line 45:
Line 46: return hosts;
Line 47: }


https://gerrit.ovirt.org/#/c/41962/11/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/BaseDAODbFacade.java
File backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/BaseDAODbFacade.java:

Line 218:
Line 219: return new ArrayList<String>(Arrays.asList(str.split(SEPARATOR)));
Line 220: }
Line 221:
Line 222: static public List<Guid> convertStringListToGuidList(List<String> stringList){
Post by r***@redhat.com
see GuidUtils.getGuidListFromStringArray
Done
Line 223: List<Guid> guidsList = new ArrayList<>();
Line 224: if (stringList == null || stringList.isEmpty()){
Line 225: return guidsList;
Line 226: }


https://gerrit.ovirt.org/#/c/41962/11/backend/manager/modules/dal/src/main/resources/bundles/AppErrors.properties
File backend/manager/modules/dal/src/main/resources/bundles/AppErrors.properties:

Line 1445: ACTION_TYPE_FAILED_SCSI_RESERVATION_NOT_VALID_FOR_FLOATING_DISK=Cannot ${action} ${type}. SCSI reservation cannot be set when adding floating disks.
Line 1446: ACTION_TYPE_FAILED_SGIO_IS_FILTERED=Cannot ${action} ${type}. SCSI reservation can be set only when SGIO is unfiltered.
Line 1447:
Line 1448: ACTION_TYPE_FAILED_VM_NOT_PINNED_TO_HOST=Cannot ${action} ${type}. VM must be pinned to a host.
Line 1449: ACTION_TYPE_FAILED_VM_PINNED_TO_MULTIPLE_HOSTS=Cannot ${action} ${type}. VM must be pinned to a single host.
Post by r***@redhat.com
we need to add why its not allowed to multi-pin (numa|host-device)
The failed action is stated in ${action} ${type}
The affected commands are:
1. AddVmHostDeviceCommand
2. AddVmNumaNodesCommand
3. UpdateVmNumaNodeCommand

Yet AddVmCommand and UpdateVmCommand allow adding/updating NUMA nodes. We need to think about duplicate functionality.
Line 1450: ACTION_TYPE_FAILED_HOST_DEVICE_NOT_FOUND=Cannot ${action} ${type}. One or more of specified host devices not found.
Line 1451: ACTION_TYPE_FAILED_HOST_DEVICE_NOT_AVAILABLE=Cannot ${action} ${type}. One or more configured host devices are unavailable.
Line 1452:
Line 1453: # vm icons


https://gerrit.ovirt.org/#/c/41962/11/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/Linq.java
File frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/Linq.java:

Line 1655: }
Line 1656: });
Line 1657: }
Line 1658:
Line 1659: public static VDS findHostByIdFromIdList(Collection<VDS> items, List<Guid> hostIdList) {
Post by r***@redhat.com
consider changing this to varargs so the API still remains the same for cal
Refactored all callers (precisely 1) to this function, to use List<Guid>
Line 1660: for (VDS host : items) {
Line 1661: for (Guid hostId : hostIdList) {
Line 1662: if (host.getId().equals(hostId)) {
Line 1663: return host;
--
To view, visit https://gerrit.ovirt.org/41962
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I710bd8d3505552a2a8d6060194d94c68c05445db
Gerrit-PatchSet: 11
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Dudi Maroshi <***@redhat.com>
Gerrit-Reviewer: Alona Kaplan <***@redhat.com>
Gerrit-Reviewer: Arik Hadas <***@redhat.com>
Gerrit-Reviewer: Dudi Maroshi <***@redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Betak <***@redhat.com>
Gerrit-Reviewer: Martin Sivák <***@redhat.com>
Gerrit-Reviewer: Omer Frenkel <***@redhat.com>
Gerrit-Reviewer: Roy Golan <***@redhat.com>
Gerrit-Reviewer: Tomer Saban <***@redhat.com>
Gerrit-Reviewer: ***@ovirt.org
Gerrit-HasComments: Yes
a***@ovirt.org
2015-06-15 15:26:29 UTC
Permalink
***@ovirt.org has posted comments on this change.

Change subject: core: enable pinning to multiple hosts
......................................................................


Patch Set 12:

* Update tracker::#1107512::OK
* Check Bug-Url::OK
* Check Public Bug::#1107512::OK, public bug
* Check Product::#1107512::OK, Correct product Red Hat Enterprise Virtualization Manager
* Check TR::SKIP, not in a monitored branch (ovirt-engine-3.5 ovirt-engine-3.4 ovirt-engine-3.3 engine_3.2 engine_3.1 ovirt-engine-3.5.2 ovirt-engine-3.4.0 ovirt-engine-3.3.4 ovirt-engine-3.3.3 ovirt-engine-3.3.2 ovirt-engine-3.3.1)
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-engine-3.5', 'ovirt-engine-3.4', 'ovirt-engine-3.3', 'ovirt-engine-3.5.2', 'ovirt-engine-3.4.0', 'ovirt-engine-3.3.4', 'ovirt-engine-3.3.3', 'ovirt-engine-3.3.2', 'ovirt-engine-3.3.1'])
--
To view, visit https://gerrit.ovirt.org/41962
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I710bd8d3505552a2a8d6060194d94c68c05445db
Gerrit-PatchSet: 12
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Dudi Maroshi <***@redhat.com>
Gerrit-Reviewer: Alona Kaplan <***@redhat.com>
Gerrit-Reviewer: Arik Hadas <***@redhat.com>
Gerrit-Reviewer: Dudi Maroshi <***@redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Betak <***@redhat.com>
Gerrit-Reviewer: Martin Sivák <***@redhat.com>
Gerrit-Reviewer: Omer Frenkel <***@redhat.com>
Gerrit-Reviewer: Roy Golan <***@redhat.com>
Gerrit-Reviewer: Tomer Saban <***@redhat.com>
Gerrit-Reviewer: ***@ovirt.org
Gerrit-HasComments: No
r***@redhat.com
2015-06-15 16:53:06 UTC
Permalink
Roy Golan has posted comments on this change.

Change subject: core: enable pinning to multiple hosts
......................................................................


Patch Set 11:

(2 comments)

+1 BE except minor

https://gerrit.ovirt.org/#/c/41962/11/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ChangeVMClusterCommand.java
File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ChangeVMClusterCommand.java:

Line 84: <Guid>
Post by d***@redhat.com
vm.setDedicatedVmForVdsList(Collections.emptyList());
ok maybe here java has problem inferring that. I think jdk 8 fixed that


https://gerrit.ovirt.org/#/c/41962/11/backend/manager/modules/dal/src/main/resources/bundles/AppErrors.properties
File backend/manager/modules/dal/src/main/resources/bundles/AppErrors.properties:

Line 1445: ACTION_TYPE_FAILED_SCSI_RESERVATION_NOT_VALID_FOR_FLOATING_DISK=Cannot ${action} ${type}. SCSI reservation cannot be set when adding floating disks.
Line 1446: ACTION_TYPE_FAILED_SGIO_IS_FILTERED=Cannot ${action} ${type}. SCSI reservation can be set only when SGIO is unfiltered.
Line 1447:
Line 1448: ACTION_TYPE_FAILED_VM_NOT_PINNED_TO_HOST=Cannot ${action} ${type}. VM must be pinned to a host.
Line 1449: ACTION_TYPE_FAILED_VM_PINNED_TO_MULTIPLE_HOSTS=Cannot ${action} ${type}. VM must be pinned to a single host.
Post by d***@redhat.com
The failed action is stated in ${action} ${type}
ok
Line 1450: ACTION_TYPE_FAILED_HOST_DEVICE_NOT_FOUND=Cannot ${action} ${type}. One or more of specified host devices not found.
Line 1451: ACTION_TYPE_FAILED_HOST_DEVICE_NOT_AVAILABLE=Cannot ${action} ${type}. One or more configured host devices are unavailable.
Line 1452:
Line 1453: # vm icons
--
To view, visit https://gerrit.ovirt.org/41962
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I710bd8d3505552a2a8d6060194d94c68c05445db
Gerrit-PatchSet: 11
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Dudi Maroshi <***@redhat.com>
Gerrit-Reviewer: Alona Kaplan <***@redhat.com>
Gerrit-Reviewer: Arik Hadas <***@redhat.com>
Gerrit-Reviewer: Dudi Maroshi <***@redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Betak <***@redhat.com>
Gerrit-Reviewer: Martin Sivák <***@redhat.com>
Gerrit-Reviewer: Omer Frenkel <***@redhat.com>
Gerrit-Reviewer: Roy Golan <***@redhat.com>
Gerrit-Reviewer: Tomer Saban <***@redhat.com>
Gerrit-Reviewer: ***@ovirt.org
Gerrit-HasComments: Yes
r***@redhat.com
2015-06-15 16:53:16 UTC
Permalink
Roy Golan has posted comments on this change.

Change subject: core: enable pinning to multiple hosts
......................................................................


Patch Set 12: Code-Review+1

(2 comments)

https://gerrit.ovirt.org/#/c/41962/12/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/numa/vm/AbstractVmNumaNodeCommand.java
File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/numa/vm/AbstractVmNumaNodeCommand.java:

Line 72: supportedNUMAMigration
I said its weird because it is reads like that

x = !Config.get(x)

which is confusing. no?


https://gerrit.ovirt.org/#/c/41962/12/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/VmBaseDaoDbFacade.java
File backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/VmBaseDaoDbFacade.java:

Line 70: .addValue("kernel_url", entity.getKernelUrl())
Line 71: .addValue("kernel_params", entity.getKernelParams())
Line 72: .addValue("quota_id", entity.getQuotaId())
Line 73: .addValue("migration_support", entity.getMigrationSupport().getValue())
Line 74: .addValue("dedicated_vm_for_vds", presistDedicatedVmForVdsList)
TODO inline it, same as line 79
Line 75: .addValue("min_allocated_mem", entity.getMinAllocatedMem())
Line 76: .addValue("is_run_and_pause", entity.isRunAndPause())
Line 77: .addValue("created_by_user_id", entity.getCreatedByUserId())
Line 78: .addValue("migration_downtime", entity.getMigrationDowntime())
--
To view, visit https://gerrit.ovirt.org/41962
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I710bd8d3505552a2a8d6060194d94c68c05445db
Gerrit-PatchSet: 12
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Dudi Maroshi <***@redhat.com>
Gerrit-Reviewer: Alona Kaplan <***@redhat.com>
Gerrit-Reviewer: Arik Hadas <***@redhat.com>
Gerrit-Reviewer: Dudi Maroshi <***@redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Betak <***@redhat.com>
Gerrit-Reviewer: Martin Sivák <***@redhat.com>
Gerrit-Reviewer: Omer Frenkel <***@redhat.com>
Gerrit-Reviewer: Roy Golan <***@redhat.com>
Gerrit-Reviewer: Tomer Saban <***@redhat.com>
Gerrit-Reviewer: ***@ovirt.org
Gerrit-HasComments: Yes
a***@ovirt.org
2015-06-16 11:00:14 UTC
Permalink
***@ovirt.org has posted comments on this change.

Change subject: core: enable pinning to multiple hosts
......................................................................


Patch Set 13:

* Update tracker::#1107512::OK
* Check Bug-Url::OK
* Check Public Bug::#1107512::OK, public bug
* Check Product::#1107512::OK, Correct product Red Hat Enterprise Virtualization Manager
* Check TR::SKIP, not in a monitored branch (ovirt-engine-3.5 ovirt-engine-3.4 ovirt-engine-3.3 engine_3.2 engine_3.1 ovirt-engine-3.5.2 ovirt-engine-3.4.0 ovirt-engine-3.3.4 ovirt-engine-3.3.3 ovirt-engine-3.3.2 ovirt-engine-3.3.1)
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-engine-3.5', 'ovirt-engine-3.4', 'ovirt-engine-3.3', 'ovirt-engine-3.5.2', 'ovirt-engine-3.4.0', 'ovirt-engine-3.3.4', 'ovirt-engine-3.3.3', 'ovirt-engine-3.3.2', 'ovirt-engine-3.3.1'])
--
To view, visit https://gerrit.ovirt.org/41962
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I710bd8d3505552a2a8d6060194d94c68c05445db
Gerrit-PatchSet: 13
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Dudi Maroshi <***@redhat.com>
Gerrit-Reviewer: Alona Kaplan <***@redhat.com>
Gerrit-Reviewer: Arik Hadas <***@redhat.com>
Gerrit-Reviewer: Dudi Maroshi <***@redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Betak <***@redhat.com>
Gerrit-Reviewer: Martin Sivák <***@redhat.com>
Gerrit-Reviewer: Omer Frenkel <***@redhat.com>
Gerrit-Reviewer: Roy Golan <***@redhat.com>
Gerrit-Reviewer: Tomer Saban <***@redhat.com>
Gerrit-Reviewer: ***@ovirt.org
Gerrit-HasComments: No
a***@ovirt.org
2015-06-16 12:34:55 UTC
Permalink
***@ovirt.org has posted comments on this change.

Change subject: core: enable pinning to multiple hosts
......................................................................


Patch Set 14:

* Update tracker::#1107512::OK
* Check Bug-Url::OK
* Check Public Bug::#1107512::OK, public bug
* Check Product::#1107512::OK, Correct product Red Hat Enterprise Virtualization Manager
* Check TR::SKIP, not in a monitored branch (ovirt-engine-3.5 ovirt-engine-3.4 ovirt-engine-3.3 engine_3.2 engine_3.1 ovirt-engine-3.5.2 ovirt-engine-3.4.0 ovirt-engine-3.3.4 ovirt-engine-3.3.3 ovirt-engine-3.3.2 ovirt-engine-3.3.1)
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-engine-3.5', 'ovirt-engine-3.4', 'ovirt-engine-3.3', 'ovirt-engine-3.5.2', 'ovirt-engine-3.4.0', 'ovirt-engine-3.3.4', 'ovirt-engine-3.3.3', 'ovirt-engine-3.3.2', 'ovirt-engine-3.3.1'])
--
To view, visit https://gerrit.ovirt.org/41962
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I710bd8d3505552a2a8d6060194d94c68c05445db
Gerrit-PatchSet: 14
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Dudi Maroshi <***@redhat.com>
Gerrit-Reviewer: Alona Kaplan <***@redhat.com>
Gerrit-Reviewer: Arik Hadas <***@redhat.com>
Gerrit-Reviewer: Dudi Maroshi <***@redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Betak <***@redhat.com>
Gerrit-Reviewer: Martin Sivák <***@redhat.com>
Gerrit-Reviewer: Omer Frenkel <***@redhat.com>
Gerrit-Reviewer: Roy Golan <***@redhat.com>
Gerrit-Reviewer: Tomer Saban <***@redhat.com>
Gerrit-Reviewer: ***@ovirt.org
Gerrit-HasComments: No
d***@redhat.com
2015-06-16 12:37:51 UTC
Permalink
Dudi Maroshi has posted comments on this change.

Change subject: core: enable pinning to multiple hosts
......................................................................


Patch Set 14: Verified+1
--
To view, visit https://gerrit.ovirt.org/41962
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I710bd8d3505552a2a8d6060194d94c68c05445db
Gerrit-PatchSet: 14
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Dudi Maroshi <***@redhat.com>
Gerrit-Reviewer: Alona Kaplan <***@redhat.com>
Gerrit-Reviewer: Arik Hadas <***@redhat.com>
Gerrit-Reviewer: Dudi Maroshi <***@redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Betak <***@redhat.com>
Gerrit-Reviewer: Martin Sivák <***@redhat.com>
Gerrit-Reviewer: Omer Frenkel <***@redhat.com>
Gerrit-Reviewer: Roy Golan <***@redhat.com>
Gerrit-Reviewer: Tomer Saban <***@redhat.com>
Gerrit-Reviewer: ***@ovirt.org
Gerrit-HasComments: No
Loading...