Vinzenz Feenstra has posted comments on this change. Change subject: vm: Fix guest channel symlink creation handling ......................................................................
Patch Set 1: (3 comments) http://gerrit.ovirt.org/#/c/26882/1//COMMIT_MSG Commit Message: Line 7: vm: Fix guest channel symlink creation handling Line 8: Line 9: In case of a restart of VDSM the symlinks and sockets won't get cleaned up. Line 10: Commit 4a7af02 partially fixed the issue, however it did not consider the Line 11: case of crashing a crashing VDSM. This patch handles these cases and adds > s/of crashing/of / actually it is that the symlink and the socket exist both, and we try to make a symlink which already exists and fail on that existing State when entering the function: name.socket <- uuid.socket os.symlink('name.socket', 'uuid.socket') => Raises OSError(File Exists) This causes the recovery to be broken and the VM instance is not correctly initialized and reports its state falsely as 'Paused' and does not start the statistic thread etc. Line 12: a bit more of documentation to the code to improve the clarity of the cases Line 13: it handles. Line 14: Line 15: Change-Id: I24e5d8e7b7e338c683d4064585539444044bb77a http://gerrit.ovirt.org/#/c/26882/1/vdsm/virt/vm.py File vdsm/virt/vm.py: Line 2682: This is necessary to prevent incoming migrations, restoring of VMs and Line 2683: the upgrade of VDSM with running VMs to fail on this. Line 2684: Line 2685: Scenarios where this is necessary: Line 2686: OLD: VDSM 4.13 > > I suppose that I think I am going to drop this section and just add the information about version more local to the specific workaround area. Line 2687: NEW: VDSM 4.13 <= Line 2688: - Old VM gets migrated to new vdsm Line 2689: - New VDSM with old VMs get unexpectedly restarted and starts Line 2690: recovering Line 2713: os.path.unlink(uuidPath) Line 2714: try: Line 2715: os.symlink(path, uuidPath) Line 2716: except OSError: Line 2717: # We swallow OSError exceptions here, we rather have no > I'm reluctant to approve this point of view. Which OSError do you expect to Actually this fix should prevent 'File Exists' errors. I am wondering if I should rather check if it exists and if so, not do anything but log an error to at least allow the VM to start/recover correctly without bringing the whole VM into a broken state during recovery Line 2718: # connection to the guest agent than a broken VM instance Line 2719: self.log.exception("Failed to make a agent channel " Line 2720: "symlink from %s -> %s for channel %s", Line 2721: path, uuidPath, name) -- To view, visit http://gerrit.ovirt.org/26882 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I24e5d8e7b7e338c683d4064585539444044bb77a Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Vinzenz Feenstra <[email protected]> Gerrit-Reviewer: Dan Kenigsberg <[email protected]> Gerrit-Reviewer: Michal Skrivanek <[email protected]> Gerrit-Reviewer: Vinzenz Feenstra <[email protected]> Gerrit-Reviewer: [email protected] Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list [email protected] https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
