Allon Mureinik has posted comments on this change. Change subject: engine: Check for conflicting address when hotplugging a disk ......................................................................
Patch Set 6: (4 comments) http://gerrit.ovirt.org/#/c/26598/6//COMMIT_MSG Commit Message: Line 5: CommitDate: 2014-04-23 10:26:36 +0200 Line 6: Line 7: engine: Check for conflicting address when hotplugging a disk Line 8: Line 9: When a disk image is hotplugged and it has a stored address we have to This is not DiskImage specific - it should also apply to a LUN disk. Line 10: check if that address is not taken by another plugged disk and if that's Line 11: the case we should remove the address from the disk that's going to be Line 12: hotplugged Line 13: Line 8: Line 9: When a disk image is hotplugged and it has a stored address we have to Line 10: check if that address is not taken by another plugged disk and if that's Line 11: the case we should remove the address from the disk that's going to be Line 12: hotplugged missing "." at the end of a sentence. Line 13: Line 14: The rationale behind this is: when a disk is hotunplugged qemu may free the Line 15: disk's address for further use. If another disk is hotplugged that Line 16: address may be given to the new device. Thus would cause that when the http://gerrit.ovirt.org/#/c/26598/6/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/HotPlugDiskToVmCommand.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/HotPlugDiskToVmCommand.java: Line 126: // We need to check if the assigned address for that device is already taken when we try to hotplug it, Line 127: // and in that case, clear the address. Line 128: if (!oldVmDevice.getIsPlugged()) { Line 129: clearAddressAlreadyInUse(); Line 130: } > Perhaps it would be nicer if this entire code will be extended to a method, +1. Line 131: performPlugCommand(getPlugAction(), getDisk(), oldVmDevice); Line 132: } Line 133: Line 134: // At this point disk is already plugged to or unplugged from VM (depends on the command), Line 148: Line 149: private void clearAddressAlreadyInUse() { Line 150: List<VmDevice> devices = getVmDeviceDao().getVmDeviceByVmIdAndType(getVmId(), oldVmDevice.getType()); Line 151: for (VmDevice device : devices) { Line 152: if (device.getId().equals(oldVmDevice.getId())) { > You can remove that if condition and just add following to the second condi +1. Line 153: continue; Line 154: } Line 155: if (device.getIsPlugged() && oldVmDevice.getAddress().equals(device.getAddress())) { Line 156: oldVmDevice.setAddress(""); -- To view, visit http://gerrit.ovirt.org/26598 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Id5850fd4c15a230dbc27e8c22d47935083da21c2 Gerrit-PatchSet: 6 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Xavi Francisco <xfran...@redhat.com> Gerrit-Reviewer: Allon Mureinik <amure...@redhat.com> Gerrit-Reviewer: Arik Hadas <aha...@redhat.com> Gerrit-Reviewer: Federico Simoncelli <fsimo...@redhat.com> Gerrit-Reviewer: Liron Ar <lara...@redhat.com> Gerrit-Reviewer: Maor Lipchuk <mlipc...@redhat.com> Gerrit-Reviewer: Omer Frenkel <ofren...@redhat.com> Gerrit-Reviewer: Xavi Francisco <xfran...@redhat.com> Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches
