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

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

 



ä 2011å01æ04æ 23:51, Eric Blake åé:
On 01/04/2011 08:30 AM, Daniel P. Berrange wrote:
  virDomainGetConnect (virDomainPtr dom)
  {
-    DEBUG("dom=%p", dom);
+    const char *name = virDomainGetName(dom);
+
+    DEBUG("dom=%p, (VM: %s)", dom, NULLSTR(name));

Calling virDomainGetName() which is also a public API will pollute
the logs with messages about virDomainGetName being invoked, every
time.

So my fear about potential deadlock from invoking one public API from
within another was not quite right, but I was on the right track of
being worried about the use of a public API for debug purposes, because
it does introduce log pollution.  If this is the only problem, then a
possible solution would be to use a private API for getting the name for
logging purposes.

Logging the name alone is also potentially misleading, since
most of the API impls use the UUID and ignore the name.

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)");
        return entry;
    }

    name = domain->name;
    virUUIDFormat(domain->uuid, uuidstr);

    if(virAsprintf(&entry, "(VM: name=%s, uuid=%s)", name,
                   uuidstr) < 0)
         virReportOOMError();

    return NULLSTR(entry);
}

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));

    virResetLastError();
    ......
}

If do it like following:

virConnectPtr
virDomainGetConnect (virDomainPtr dom)
{
    const char *name_uuid = virDomainUUID(dom);

    DEBUG("dom=%p, %s", dom, name_uuid);
    VIR_FREE(name_uuid);

    virResetLastError();
    ......
}

Then the extra time is introduced when debugging is off, do you
have any good idea?

Thanks
Osier

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list



[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]