Change in vdsm[master]: virt: Correct epoll unregistration usage in vmchannels

nsoffer at redhat.com nsoffer at redhat.com
Tue Jan 12 11:56:30 UTC 2016


Nir Soffer has posted comments on this change.

Change subject: virt: Correct epoll unregistration usage in vmchannels
......................................................................


Patch Set 8: Code-Review-1

(1 comment)

https://gerrit.ovirt.org/#/c/51521/8/vdsm/virt/vmchannels.py
File vdsm/virt/vmchannels.py:

Line 233:             # removed from epoll after the socket has been closed, which is
Line 234:             # incorrect
Line 235:             if (fileno not in self._add_channels and
Line 236:                     fileno not in self._unconnected):
Line 237:                 self._unregister_fd(fileno)
Can you explain this checks?

How can fileno be in self._add_channels or in self._unconnected when we unregister, and what will happen if we do not unregister the fd in this case?

I think we should always unregister the fd the caller ask to unregister.

If we track all fds, and we can detect when caller asked to unregister an fd we do not track, this should raise - this is a caller error.


-- 
To view, visit https://gerrit.ovirt.org/51521
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I9f7eab8318f41f653e0a24552c81bcd5b09d8690
Gerrit-PatchSet: 8
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Vinzenz Feenstra <vfeenstr at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani at redhat.com>
Gerrit-Reviewer: Gal Hammer <ghammer%redhat.com at gtempaccount.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Polednik <mpolednik at redhat.com>
Gerrit-Reviewer: Michal Skrivanek <michal.skrivanek at redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer at redhat.com>
Gerrit-Reviewer: Vinzenz Feenstra <vfeenstr at redhat.com>
Gerrit-Reviewer: gerrit-hooks <automation at ovirt.org>
Gerrit-HasComments: Yes


More information about the vdsm-patches mailing list