[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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



于 2011年01月06日 00:55, Daniel P. Berrange 写道:
On Wed, Jan 05, 2011 at 09:34:20AM -0700, Eric Blake wrote:
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.

Ideally it'd be a macro that accepts extra args too eg
perhaps

  #define DEBUG_DOMAIN(dom, fmt, ...) \
     do {
        char uuidstr[VIR_UUID_STRING_BUFLEN];
        char *name;
        if (VIR_IS_DOMAIN(dom) {
               virUUIDFormat(uuidstr, dom->uuid);
               name=dom->name;
        } else {
               memset(uuidstr, 0, sizeof(uuidstr));
        }
        DEBUG("dom=%p (name=%s, uuid=%s) " fmt,
              NULLSTR(name), uuidstr, __VA_ARGS__)
     } while(0)


Cool, I prefer this one, personally, as with it the work is
easier, don't need to introduce new statements, and don't need
to worry about malloc'ing.

Thanks, Eric, and Daniel.

Regards
Osier


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]