Francesco Romani has posted comments on this change.
Change subject: vm.py: State saving in hotunplugDisk.
......................................................................
Patch Set 4:
(2 comments)
https://gerrit.ovirt.org/#/c/45077/4//COMMIT_MSG
Commit Message:
Line 9: VM's state was saved before detatching a disk in hotunplugDisk.
Line 10:
Line 11: Saving vm's state should be performed only AFTER vdsm is actually detaching
Line 12: the disk, otherwise the disk will remain in xmlDesc and be calculated in
Line 13: the vm's hash.
Saving state does not effect xmlDesc.
It does, as side effect.
Please check the implementation of Vm.saveState().
Line 14: This causes a bug in the engine's VM monitoring: the engine
Line 15: gets the same hash as before the disk was unplugged, thus does not
Line 16: update the disk's status.
Line 17:
Line 12: the disk, otherwise the disk will remain in xmlDesc and be calculated in
Line 13: the vm's hash.
Line 14: This causes a bug in the engine's VM monitoring: the engine
Line 15: gets the same hash as before the disk was unplugged, thus does not
Line 16: update the disk's status.
How saving state before removal can cause engine to get same hash as
before
Most likely (Amit can correct me if I'm wrong) because the saveState
triggers the updateDomainDescriptor to refresh the XML from libvirt and the devices hash.
I commented this before in a past revision. This change can be good improvement to improve
the flow here, but for the issue described in the commit message the proper fix is to
update XML in a correct way, and to deentangle it from the saveState, at least in this
method.
Line 17:
Line 18: Change-Id: I2cf18186cbba33d7e74fd15651ffec3149c98e1d
Line 19:
Bug-Url:https://bugzilla.redhat.com/1206696
Line 20: Bug-Url:
https://bugzilla.redhat.com/1044466
--
To view, visit
https://gerrit.ovirt.org/45077
To unsubscribe, visit
https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I2cf18186cbba33d7e74fd15651ffec3149c98e1d
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Amit Aviram <aaviram(a)redhat.com>
Gerrit-Reviewer: Allon Mureinik <amureini(a)redhat.com>
Gerrit-Reviewer: Amit Aviram <aaviram(a)redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com>
Gerrit-Reviewer: Liron Aravot <laravot(a)redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-HasComments: Yes