[libvirt] [PATCH] [v2] API: Improve log for domain related APIs

[Eric Blake]

On 01/05/2011 03:40 AM, Osier Yang wrote:
>> Then maybe the patch should be altered to output both name and UUID
>> (probably by introducing a helper function, which when given a
>> virDomainPtr outputs all three pieces of debug information rather than
>> the current %p).  I like the idea behind the patch, but only if we can
>> come up with the correct way of getting the extra information when
>> debugging is enabled, and without adding extra time when debugging is
>> off.  I'm not even sure what the right internal APIs for the job would
>> be, or if they even exist yet.
>>
> 
> Hi, Eric,
> 
> I implemented a helper function like so in src/libvirt.c:
> 
> static const char *
> virDomainNameUUID(virDomainPtr domain) {
>     char *entry = NULL;
>     const char *name = NULL;
>     char uuidstr[VIR_UUID_STRING_BUFLEN];
> 
>     if (!VIR_IS_DOMAIN(domain)) {
>         entry = strdup("(VM: invalid domain)");

malloc'd return...

>         return entry;
>     }
> 
>     name = domain->name;
>     virUUIDFormat(domain->uuid, uuidstr);
> 
>     if(virAsprintf(&entry, "(VM: name=%s, uuid=%s)", name,
>                    uuidstr) < 0)
>          virReportOOMError();
> 
>     return NULLSTR(entry);

and static return.  Ouch - that means the caller doesn't know whether to
call free or not.  You have to be consistent (the result is either NULL
or malloc'd; or the result is always static).

> }
> 
> But how to avoid extra time when debugging is off? Following
> is not correct way definitely, as malloc'ed "entry" need to be
> freed.
> 
> virConnectPtr
> virDomainGetConnect (virDomainPtr dom)
> {
>     DEBUG("dom=%p, %s", dom, virDomainNameUUID(dom));

When I envisioned a helper function, I'm thinking more like callers
using a simpler:

DEBUG_DOMAIN(dom)

and a helper like:

void
DEBUG_DOMAIN (virDomainPtr dom)
{
    if !DEBUG then early exit
    gather data
    DEBUG("dom=%p, %s", dom, virDomainNameUUID(dom));
    free any gathered data if needed
}

That would mean that your new helper function would need to look at the
guts of DEBUG to mirror the same decision of whether DEBUG will result
in any output.

-- 
Eric Blake   eblake at redhat.com    +1-801-349-2682
Libvirt virtualization library http://libvirt.org

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