[libvirt] [PATCH 2/2] maint: improve debug of libvirt-{qemu, lxc} apis

[Daniel P. Berrange]

On Thu, Dec 19, 2013 at 09:35:23AM -0700, Eric Blake wrote:
> On 12/19/2013 08:17 AM, Daniel P. Berrange wrote:
> > On Thu, Dec 19, 2013 at 08:13:36AM -0700, Eric Blake wrote:
> >> diff --git a/src/libvirt_internal.h b/src/libvirt_internal.h
> >> index 115d8d1..b8c842d 100644
> >> --- a/src/libvirt_internal.h
> >> +++ b/src/libvirt_internal.h
> >> @@ -27,6 +27,113 @@
> >>
> >>  # include "internal.h"
> >>
> 
> >> +#define VIR_DOMAIN_DEBUG(...)                           \
> >> +    VIR_DOMAIN_DEBUG_EXPAND(VIR_DOMAIN_DEBUG_,          \
> >> +                            VIR_HAS_COMMA(__VA_ARGS__), \
> >> +                            __VA_ARGS__)
> > 
> > I'd suggest these go in  datatypes.h
> 
> Good idea.
> 
> > 
> > 
> >> +
> >> +/**
> >> + * VIR_UUID_DEBUG:
> >> + * @conn: connection
> >> + * @uuid: possibly null UUID array
> >> + */
> >> +#define VIR_UUID_DEBUG(conn, uuid)                              \
> >> +    do {                                                        \
> >> +        if (uuid) {                                             \
> >> +            char _uuidstr[VIR_UUID_STRING_BUFLEN];              \
> >> +            virUUIDFormat(uuid, _uuidstr);                      \
> >> +            VIR_DEBUG("conn=%p, uuid=%s", conn, _uuidstr);      \
> >> +        } else {                                                \
> >> +            VIR_DEBUG("conn=%p, uuid=(null)", conn);            \
> >> +        }                                                       \
> >> +    } while (0)
> > 
> > 
> > And this in viruuid.h
> 
> Sure, although VIR_DOMAIN_DEBUG requires the use of viruuid.h in the
> first place.
> 
> >> +#define virLibDomainSnapshotError(code, ...)                       \
> >> +    virReportErrorHelper(VIR_FROM_DOMAIN_SNAPSHOT, code, __FILE__, \
> >> +                         __FUNCTION__, __LINE__, __VA_ARGS__)
> > 
> > I'd venture to sugggest that these functions really have little
> > to no benefit / reason to exist. They aren't really simplifying
> > like, just making it different. They're historical baggage from
> > the old time when they were actual functions which did extra
> > sprintf formatting work. Could we just remove them ??
> 
> Perhaps, by using virReportError instead.  I can give that a try; but
> just looking at it, virReportError() hardcodes VIR_FROM_THIS as its
> first argument, whereas virLib*Error() sets different VIR_FROM_*
> categories.  Worse, we use multiple calls from within a single function
> (so continually redefining VIR_FROM_THIS would get painful) - for
> example, virDomainSnapshotCreateXML() uses both virLibDomainError()
> (VIR_FROM_DOMAIN) and virLibConnError() (VIR_FROM_NONE).

Oh true I'd forgotten it relied on VIR_FROM_THIS.

How about just renaming them s/Lib/Report/
 eg virLibDomainError ->  virReportDomainError and having
them in datatypes.h too. 

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 :|