On Fri, Jul 28, 2017 at 11:31:27AM +0200, Michal Privoznik wrote: > On 07/26/2017 10:45 AM, Erik Skultety wrote: > > Commit @4cb719b2dc moved the driver locks around since these have become > > unnecessary at spots where the code handles now self-lockable object > > list, but missed the possible double unlock if udevEnumerateDevices > > fails, because at that point the driver lock had been already dropped. > > > > Signed-off-by: Erik Skultety <eskultet@xxxxxxxxxx> > > --- > > src/node_device/node_device_udev.c | 17 ++++++++++------- > > 1 file changed, 10 insertions(+), 7 deletions(-) > > > > I know you already pushed this but what's the point of node driver lock > anyway? There are only two places where the driver lock is used - init To answer the question: to protect the access to driver's private data which are mutable. Once we introduce an event handler thread for udev events, you'll get concurrent access to the udev monitor. As you say, the driver's lock doesn't serve much purpose except for accessing the udev monitor. This wasn't discussed on a mailing list unfortunately, but I asked about the need to perform any kinds of sanity checks on the udev_monitor on IRC, because noone can change that one except ourselves - let's say we dropped the sanity checks, then there's basically no need to have the driver lock (except for maybe consistency among other drivers) at all. But I recall some other opinions in favour of keeping the sanity checks, don't remember the specifics though. Erik > and cleanup. Moreover with the current code still racy, except not > really beacuse init and cleanup will never be called concurrently. > Therefore the lock can be dropped entirely. > > Michal -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
