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 :|
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list