Dan Kenigsberg has posted comments on this change.

Change subject: vm: Fix guest channel symlink creation handling
......................................................................


Patch Set 1: Code-Review-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 /

sorry for this not being clear to me, but which is the broken condition that 
this commit fixes? a stale link? an inaccessible socket?
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

 OLD: Vdsm < 4.13

is more readable.
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 
see? Can you swallow only specific one, instead of possibly ignoring system 
bugs?
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: [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

Reply via email to