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

[Eric Blake]

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

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 621 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20131219/2a803c11/attachment-0001.sig>