Re: [PATCH 1/8] Add API for thread cancellation

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, Mar 07, 2013 at 05:10:36PM -0700, Eric Blake wrote:
> On 03/06/2013 05:49 AM, Daniel P. Berrange wrote:
> > From: "Daniel P. Berrange" <berrange@xxxxxxxxxx>
> > 
> > Add a virThreadCancel function. This functional is inherantly
> 
> s/inherantly/inherently/
> 
> > dangerous and not something we want to use in general, but
> > integration with SELinux requires that we provide this stub.
> > We leave out any Win32 impl to discourage further use and
> > because obviously SELinux isn't enabled on Win32
> > 
> > Signed-off-by: Daniel P. Berrange <berrange@xxxxxxxxxx>
> > ---
> >  src/libvirt_private.syms    | 1 +
> >  src/util/virthread.h        | 1 +
> >  src/util/virthreadpthread.c | 5 +++++
> >  3 files changed, 7 insertions(+)
> > 
> 
> > +++ b/src/util/virthreadpthread.c
> > @@ -236,6 +236,11 @@ void virThreadJoin(virThreadPtr thread)
> >      pthread_join(thread->thread, NULL);
> >  }
> >  
> > +void virThreadCancel(virThreadPtr thread)
> 
> It would help to add documentation right before this implementation that
> mentions that the function exists SOLELY for use by threads created by
> third-party libraries which are prepared for cancellation, and that most
> libvirt threads do not qualify.  Having the discouraging comment tucked
> away solely in the commit message won't help a future coder who doesn't
> realize the dangers.
> 
> ACK with the comment added.

I'll add it in the header file since that's where people will probably
find the function first.

Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list


[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]