[libvirt] [PATCH V2 2/3] add error report for virMutexInit virRWLockInit and virCondInit
Jincheng Miao
jmiao at redhat.com
Tue Jul 22 04:10:42 UTC 2014
----- Original Message -----
> On Mon, Jul 21, 2014 at 06:13:36PM +0800, Jincheng Miao wrote:
> > Implement vir{Mutex,RWLock,Cond}InitInternal functions which have
> > related error report when failure, and vir{Mutex,RWLock,Cond}Init
> > as macros. So that the caller no longer to print error message
> > explicitly.
> >
> > Signed-off-by: Jincheng Miao <jmiao at redhat.com>
> > ---
> > src/libvirt_private.syms | 7 +++---
> > src/util/virthread.c | 56
> > +++++++++++++++++++++---------------------------
> > src/util/virthread.h | 25 +++++++++++++++++----
> > 3 files changed, 49 insertions(+), 39 deletions(-)
> >
>
> >
> > +#define VIR_THREAD_ERR_EXIT(str) do { \
> > + errno = ret; \
> > + virReportSystemErrorFull(VIR_FROM_NONE, errno, \
> > + filename, funcname, linenr, \
> > + "%s", _(str)); \
> > + return -1; \
> > + } while (0)
>
> [snip]
>
> > -int virCondInit(virCondPtr cond)
> > +int virCondInitInternal(virCondPtr cond, const char *filename,
> > + const char *funcname, size_t linenr)
> > {
> > - int ret;
> > - if ((ret = pthread_cond_init(&cond->cond, NULL)) != 0) {
> > - errno = ret;
> > - return -1;
> > - }
> > + int ret = pthread_cond_init(&cond->cond, NULL);
> > + if (ret != 0)
> > + VIR_THREAD_ERR_EXIT("unable to init Condition");
> > +
> > return 0;
> > }
>
> [snip]
>
> > -int virCondInit(virCondPtr cond) ATTRIBUTE_RETURN_CHECK;
> > +int virCondInitInternal(virCondPtr cond, const char *filename,
> > + const char *funcname, size_t linenr)
> > + ATTRIBUTE_RETURN_CHECK;
> > +# define virCondInit(cond) \
> > + virCondInitInternal(cond, __FILE__, __FUNCTION__, __LINE__)
> > +
>
> I can see what you're trying todo here by passing in __FILE__,
> __FUNCTION__, __LINE__ through from the calling site, but I don't
> really think this is beneficial and is not the way we deal with
> error reporting in any other similar helper APIs. IMHO just
> remove all this passing around of source location and let it do
> the default thing. In fact the default behaviour is probably
> better since if we want to search for any thread related error
> messages, we can query the journald for the virthread.c file
> directly.
OK, it's fine to drop them, and thanks for your review.
>
> Regards,
> 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 :|
>
More information about the libvir-list
mailing list