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).
I'll definitely split into 2 patches; the VIR_DOMAIN_DEBUG motion in one
(since it was less controversial) and the potential virLib*Error cleanup
in the other. v2 coming up later today.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list