[libvirt] [PATCH] [v2] API: Improve log for domain related APIs
Add VM name/UUID in log for domain related APIs. Format: "param0=%p, param1=%p, (VM: %s)" * src/libvirt.c --- src/libvirt.c | 293 +++++++++++++++++++++++++++++++++++++++++++-------------- 1 files changed, 220 insertions(+), 73 deletions(-) diff --git a/src/libvirt.c b/src/libvirt.c index ee2495a..cd1cf6e 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -1961,7 +1961,9 @@ error: virConnectPtr virDomainGetConnect (virDomainPtr dom) { - DEBUG("dom=%p", dom); + const char *name = virDomainGetName(dom); + + DEBUG("dom=%p, (VM: %s)", dom, NULLSTR(name)); virResetLastError(); @@ -2100,7 +2102,10 @@ error: virDomainPtr virDomainLookupByUUID(virConnectPtr conn, const unsigned char *uuid) { - DEBUG("conn=%p, uuid=%s", conn, uuid); + char uuidstr[VIR_UUID_STRING_BUFLEN]; + virUUIDFormat(uuid, uuidstr); + + DEBUG("conn=%p, uuid=%s", conn, uuidstr); virResetLastError(); @@ -2226,8 +2231,9 @@ int virDomainDestroy(virDomainPtr domain) { virConnectPtr conn; + const char *name = virDomainGetName(domain); - DEBUG("domain=%p", domain); + DEBUG("domain=%p, (VM: %s)", domain, NULLSTR(name)); virResetLastError(); @@ -2270,7 +2276,9 @@ error: int virDomainFree(virDomainPtr domain) { - DEBUG("domain=%p", domain); + const char *name = virDomainGetName(domain); + + DEBUG("domain=%p, (VM: %s)", domain, NULLSTR(name)); virResetLastError(); @@ -2306,13 +2314,15 @@ virDomainFree(virDomainPtr domain) int virDomainRef(virDomainPtr domain) { + const char *name = virDomainGetName(domain); + if ((!VIR_IS_CONNECTED_DOMAIN(domain))) { virLibConnError(NULL, VIR_ERR_INVALID_ARG, __FUNCTION__); virDispatchError(NULL); return(-1); } virMutexLock(&domain->conn->lock); - DEBUG("domain=%p refs=%d", domain, domain->refs); + DEBUG("domain=%p refs=%d, (VM: %s)", domain, domain->refs, NULLSTR(name)); domain->refs++; virMutexUnlock(&domain->conn->lock); return 0; @@ -2335,7 +2345,9 @@ int virDomainSuspend(virDomainPtr domain) { virConnectPtr conn; - DEBUG("domain=%p", domain); + const char *name = virDomainGetName(domain); + + DEBUG("domain=%p, (VM: %s)", domain, NULLSTR(name)); virResetLastError(); @@ -2380,7 +2392,9 @@ int virDomainResume(virDomainPtr domain) { virConnectPtr conn; - DEBUG("domain=%p", domain); + const char *name = virDomainGetName(domain); + + DEBUG("domain=%p, (VM: %s)", domain, NULLSTR(name)); virResetLastError(); @@ -2428,7 +2442,9 @@ virDomainSave(virDomainPtr domain, const char *to) { char filepath[4096]; virConnectPtr conn; - DEBUG("domain=%p, to=%s", domain, to); + const char *name = virDomainGetName(domain); + + DEBUG("domain=%p, to=%s, (VM: %s)", domain, to, NULLSTR(name)); virResetLastError(); @@ -2570,7 +2586,10 @@ virDomainCoreDump(virDomainPtr domain, const char *to, int flags) { char filepath[4096]; virConnectPtr conn; - DEBUG("domain=%p, to=%s, flags=%d", domain, to, flags); + const char *name = virDomainGetName(domain); + + DEBUG("domain=%p, to=%s, flags=%d, (VM: %s)", domain, to, flags, + NULLSTR(name)); virResetLastError(); @@ -2647,7 +2666,9 @@ int virDomainShutdown(virDomainPtr domain) { virConnectPtr conn; - DEBUG("domain=%p", domain); + const char *name = virDomainGetName(domain); + + DEBUG("domain=%p, (VM: %s)", domain, NULLSTR(name)); virResetLastError(); @@ -2693,7 +2714,9 @@ int virDomainReboot(virDomainPtr domain, unsigned int flags) { virConnectPtr conn; - DEBUG("domain=%p, flags=%u", domain, flags); + const char *name = virDomainGetName(domain); + + DEBUG("domain=%p, flags=%u, (VM: %s)", domain, flags, NULLSTR(name)); virResetLastError(); @@ -2760,7 +2783,9 @@ virDomainGetName(virDomainPtr domain) int virDomainGetUUID(virDomainPtr domain, unsigned char *uuid) { - DEBUG("domain=%p, uuid=%p", domain, uuid); + const char *name = virDomainGetName(domain); + + DEBUG("domain=%p, uuid=%p, (VM: %s)", domain, uuid, NULLSTR(name)); virResetLastError(); @@ -2794,7 +2819,9 @@ int virDomainGetUUIDString(virDomainPtr domain, char *buf) { unsigned char uuid[VIR_UUID_BUFLEN]; - DEBUG("domain=%p, buf=%p", domain, buf); + const char *name = virDomainGetName(domain); + + DEBUG("domain=%p, buf=%p, (VM: %s)", domain, buf, NULLSTR(name)); virResetLastError(); @@ -2830,7 +2857,9 @@ error: unsigned int virDomainGetID(virDomainPtr domain) { - DEBUG("domain=%p", domain); + const char *name = virDomainGetName(domain); + + DEBUG("domain=%p, (VM: %s)", domain, NULLSTR(name)); virResetLastError(); @@ -2855,7 +2884,9 @@ char * virDomainGetOSType(virDomainPtr domain) { virConnectPtr conn; - DEBUG("domain=%p", domain); + const char *name = virDomainGetName(domain); + + DEBUG("domain=%p, (VM: %s)", domain, NULLSTR(name)); virResetLastError(); @@ -2896,7 +2927,9 @@ unsigned long virDomainGetMaxMemory(virDomainPtr domain) { virConnectPtr conn; - DEBUG("domain=%p", domain); + const char *name = virDomainGetName(domain); + + DEBUG("domain=%p, (VM: %s)", domain, NULLSTR(name)); virResetLastError(); @@ -2942,7 +2975,9 @@ int virDomainSetMaxMemory(virDomainPtr domain, unsigned long memory) { virConnectPtr conn; - DEBUG("domain=%p, memory=%lu", domain, memory); + const char *name = virDomainGetName(domain); + + DEBUG("domain=%p, memory=%lu, (VM: %s)", domain, memory, NULLSTR(name)); virResetLastError(); @@ -2995,7 +3030,9 @@ int virDomainSetMemory(virDomainPtr domain, unsigned long memory) { virConnectPtr conn; - DEBUG("domain=%p, memory=%lu", domain, memory); + const char *name = virDomainGetName(domain); + + DEBUG("domain=%p, memory=%lu, (VM: %s)", domain, memory, NULLSTR(name)); virResetLastError(); @@ -3049,7 +3086,10 @@ virDomainSetMemoryParameters(virDomainPtr domain, int nparams, unsigned int flags) { virConnectPtr conn; - DEBUG("domain=%p, params=%p, nparams=%d, flags=%u", domain, params, nparams, flags); + const char *name = virDomainGetName(domain); + + DEBUG("domain=%p, params=%p, nparams=%d, flags=%u, (VM: %s)", + domain, params, nparams, flags, NULLSTR(name)); virResetLastError(); @@ -3123,7 +3163,10 @@ virDomainGetMemoryParameters(virDomainPtr domain, int *nparams, unsigned int flags) { virConnectPtr conn; - DEBUG("domain=%p, params=%p, nparams=%d, flags=%u", domain, params, (nparams)?*nparams:-1, flags); + const char *name = virDomainGetName(domain); + + DEBUG("domain=%p, params=%p, nparams=%d, flags=%u, (VM: %s)", + domain, params, (nparams)?*nparams:-1, flags, NULLSTR(name)); virResetLastError(); @@ -3167,7 +3210,9 @@ int virDomainGetInfo(virDomainPtr domain, virDomainInfoPtr info) { virConnectPtr conn; - DEBUG("domain=%p, info=%p", domain, info); + const char *name = virDomainGetName(domain); + + DEBUG("domain=%p, info=%p, (VM: %s)", domain, info, NULLSTR(name)); virResetLastError(); @@ -3215,7 +3260,9 @@ char * virDomainGetXMLDesc(virDomainPtr domain, int flags) { virConnectPtr conn; - DEBUG("domain=%p, flags=%d", domain, flags); + const char *name = virDomainGetName(domain); + + DEBUG("domain=%p, flags=%d, (VM: %s)", domain, flags, NULLSTR(name)); virResetLastError(); @@ -3662,8 +3709,10 @@ virDomainMigrate (virDomainPtr domain, unsigned long bandwidth) { virDomainPtr ddomain = NULL; - DEBUG("domain=%p, dconn=%p, flags=%lu, dname=%s, uri=%s, bandwidth=%lu", - domain, dconn, flags, NULLSTR(dname), NULLSTR(uri), bandwidth); + const char *name = virDomainGetName(domain); + + DEBUG("domain=%p, dconn=%p, flags=%lu, dname=%s, uri=%s, bandwidth=%lu, (VM: %s)", + domain, dconn, flags, NULLSTR(dname), NULLSTR(uri), bandwidth, NULLSTR(name)); virResetLastError(); @@ -3811,8 +3860,10 @@ virDomainMigrateToURI (virDomainPtr domain, const char *dname, unsigned long bandwidth) { - DEBUG("domain=%p, duri=%p, flags=%lu, dname=%s, bandwidth=%lu", - domain, NULLSTR(duri), flags, NULLSTR(dname), bandwidth); + const char *name = virDomainGetName(domain); + + DEBUG("domain=%p, duri=%p, flags=%lu, dname=%s, bandwidth=%lu, (VM: %s)", + domain, NULLSTR(duri), flags, NULLSTR(dname), bandwidth, NULLSTR(name)); virResetLastError(); @@ -3924,9 +3975,11 @@ virDomainMigratePerform (virDomainPtr domain, unsigned long bandwidth) { virConnectPtr conn; + const char *name = virDomainGetName(domain); + VIR_DEBUG("domain=%p, cookie=%p, cookielen=%d, uri=%s, flags=%lu, " - "dname=%s, bandwidth=%lu", domain, cookie, cookielen, uri, flags, - NULLSTR(dname), bandwidth); + "dname=%s, bandwidth=%lu, (VM: %s)", domain, cookie, + cookielen, uri, flags, NULLSTR(dname), bandwidth, NULLSTR(name)); virResetLastError(); @@ -4290,7 +4343,9 @@ virDomainGetSchedulerType(virDomainPtr domain, int *nparams) { virConnectPtr conn; char *schedtype; - DEBUG("domain=%p, nparams=%p", domain, nparams); + const char *name = virDomainGetName(domain); + + DEBUG("domain=%p, nparams=%p, (VM: %s)", domain, nparams, NULLSTR(name)); virResetLastError(); @@ -4335,7 +4390,10 @@ virDomainGetSchedulerParameters(virDomainPtr domain, virSchedParameterPtr params, int *nparams) { virConnectPtr conn; - DEBUG("domain=%p, params=%p, nparams=%p", domain, params, nparams); + const char *name = virDomainGetName(domain); + + DEBUG("domain=%p, params=%p, nparams=%p, (VM: %s)", domain, + params, nparams, NULLSTR(name)); virResetLastError(); @@ -4378,7 +4436,10 @@ virDomainSetSchedulerParameters(virDomainPtr domain, virSchedParameterPtr params, int nparams) { virConnectPtr conn; - DEBUG("domain=%p, params=%p, nparams=%d", domain, params, nparams); + const char *name = virDomainGetName(domain); + + DEBUG("domain=%p, params=%p, nparams=%d, (VM: %s)", domain, + params, nparams, NULLSTR(name)); virResetLastError(); @@ -4438,7 +4499,10 @@ virDomainBlockStats (virDomainPtr dom, const char *path, { virConnectPtr conn; struct _virDomainBlockStats stats2 = { -1, -1, -1, -1, -1 }; - DEBUG("domain=%p, path=%s, stats=%p, size=%zi", dom, path, stats, size); + const char *name = virDomainGetName(dom); + + DEBUG("domain=%p, path=%s, stats=%p, size=%zi, (VM: %s)", dom, + path, stats, size, NULLSTR(name)); virResetLastError(); @@ -4496,7 +4560,10 @@ virDomainInterfaceStats (virDomainPtr dom, const char *path, virConnectPtr conn; struct _virDomainInterfaceStats stats2 = { -1, -1, -1, -1, -1, -1, -1, -1 }; - DEBUG("domain=%p, path=%s, stats=%p, size=%zi", dom, path, stats, size); + const char *name = virDomainGetName(dom); + + DEBUG("domain=%p, path=%s, stats=%p, size=%zi, (VM: %s)", dom, + path, stats, size, NULLSTR(name)); virResetLastError(); @@ -4561,7 +4628,10 @@ int virDomainMemoryStats (virDomainPtr dom, virDomainMemoryStatPtr stats, { virConnectPtr conn; unsigned long nr_stats_ret = 0; - DEBUG("domain=%p, stats=%p, nr_stats=%u", dom, stats, nr_stats); + const char *name = virDomainGetName(dom); + + DEBUG("domain=%p, stats=%p, nr_stats=%u, (VM: %s)", dom, stats, nr_stats, + NULLSTR(name)); if (flags != 0) { virLibDomainError (dom, VIR_ERR_INVALID_ARG, @@ -4645,8 +4715,10 @@ virDomainBlockPeek (virDomainPtr dom, unsigned int flags) { virConnectPtr conn; - DEBUG("domain=%p, path=%s, offset=%lld, size=%zi, buffer=%p", - dom, path, offset, size, buffer); + const char *name = virDomainGetName(dom); + + DEBUG("domain=%p, path=%s, offset=%lld, size=%zi, buffer=%p, (VM: %s)", + dom, path, offset, size, buffer, NULLSTR(name)); virResetLastError(); @@ -4736,8 +4808,10 @@ virDomainMemoryPeek (virDomainPtr dom, unsigned int flags) { virConnectPtr conn; - DEBUG ("domain=%p, start=%lld, size=%zi, buffer=%p, flags=%d", - dom, start, size, buffer, flags); + const char *name = virDomainGetName(dom); + + DEBUG ("domain=%p, start=%lld, size=%zi, buffer=%p, flags=%d, (VM: %s)", + dom, start, size, buffer, flags, NULLSTR(name)); virResetLastError(); @@ -4821,7 +4895,9 @@ int virDomainGetBlockInfo(virDomainPtr domain, const char *path, virDomainBlockInfoPtr info, unsigned int flags) { virConnectPtr conn; - DEBUG("domain=%p, info=%p flags=%u", domain, info, flags); + const char *name = virDomainGetName(domain); + + DEBUG("domain=%p, info=%p flags=%u, (VM: %s)", domain, info, flags, NULLSTR(name)); virResetLastError(); @@ -4919,7 +4995,9 @@ error: int virDomainUndefine(virDomainPtr domain) { virConnectPtr conn; - DEBUG("domain=%p", domain); + const char *name = virDomainGetName(domain); + + DEBUG("domain=%p name=%s", domain, NULLSTR(name)); virResetLastError(); @@ -5041,7 +5119,9 @@ error: int virDomainCreate(virDomainPtr domain) { virConnectPtr conn; - DEBUG("domain=%p", domain); + const char *name = virDomainGetName(domain); + + DEBUG("domain=%p, (VM :%s)", domain, NULLSTR(name)); virResetLastError(); @@ -5084,7 +5164,9 @@ error: int virDomainCreateWithFlags(virDomainPtr domain, unsigned int flags) { virConnectPtr conn; - DEBUG("domain=%p, flags=%d", domain, flags); + const char *name = virDomainGetName(domain); + + DEBUG("domain=%p, flags=%d, (VM: %s)", domain, flags, NULLSTR(name)); virResetLastError(); @@ -5130,7 +5212,9 @@ virDomainGetAutostart(virDomainPtr domain, int *autostart) { virConnectPtr conn; - DEBUG("domain=%p, autostart=%p", domain, autostart); + const char *name = virDomainGetName(domain); + + DEBUG("domain=%p, autostart=%p, (VM: %s)", domain, autostart, NULLSTR(name)); virResetLastError(); @@ -5176,7 +5260,9 @@ virDomainSetAutostart(virDomainPtr domain, int autostart) { virConnectPtr conn; - DEBUG("domain=%p, autostart=%d", domain, autostart); + const char *name = virDomainGetName(domain); + + DEBUG("domain=%p, autostart=%d, (VM: %s)", domain, autostart, NULLSTR(name)); virResetLastError(); @@ -5230,7 +5316,9 @@ int virDomainSetVcpus(virDomainPtr domain, unsigned int nvcpus) { virConnectPtr conn; - DEBUG("domain=%p, nvcpus=%u", domain, nvcpus); + const char *name = virDomainGetName(domain); + + DEBUG("domain=%p, nvcpus=%u, (VM: %s)", domain, nvcpus, NULLSTR(name)); virResetLastError(); @@ -5296,7 +5384,10 @@ virDomainSetVcpusFlags(virDomainPtr domain, unsigned int nvcpus, unsigned int flags) { virConnectPtr conn; - DEBUG("domain=%p, nvcpus=%u, flags=%u", domain, nvcpus, flags); + const char *name = virDomainGetName(domain); + + DEBUG("domain=%p, nvcpus=%u, flags=%u, (VM: %s)", domain, + nvcpus, flags, NULLSTR(name)); virResetLastError(); @@ -5359,7 +5450,9 @@ int virDomainGetVcpusFlags(virDomainPtr domain, unsigned int flags) { virConnectPtr conn; - DEBUG("domain=%p, flags=%u", domain, flags); + const char *name = virDomainGetName(domain); + + DEBUG("domain=%p, flags=%u, (VM: %s)", domain, flags, NULLSTR(name)); virResetLastError(); @@ -5417,7 +5510,10 @@ virDomainPinVcpu(virDomainPtr domain, unsigned int vcpu, unsigned char *cpumap, int maplen) { virConnectPtr conn; - DEBUG("domain=%p, vcpu=%u, cpumap=%p, maplen=%d", domain, vcpu, cpumap, maplen); + const char *name = virDomainGetName(domain); + + DEBUG("domain=%p, vcpu=%u, cpumap=%p, maplen=%d, (VM: %s)", + domain, vcpu, cpumap, maplen, NULLSTR(name)); virResetLastError(); @@ -5480,7 +5576,10 @@ virDomainGetVcpus(virDomainPtr domain, virVcpuInfoPtr info, int maxinfo, unsigned char *cpumaps, int maplen) { virConnectPtr conn; - DEBUG("domain=%p, info=%p, maxinfo=%d, cpumaps=%p, maplen=%d", domain, info, maxinfo, cpumaps, maplen); + const char *name = virDomainGetName(domain); + + DEBUG("domain=%p, info=%p, maxinfo=%d, cpumaps=%p, maplen=%d, (VM: %s)", + domain, info, maxinfo, cpumaps, maplen, NULLSTR(name)); virResetLastError(); @@ -5536,7 +5635,9 @@ int virDomainGetMaxVcpus(virDomainPtr domain) { virConnectPtr conn; - DEBUG("domain=%p", domain); + const char *name = virDomainGetName(domain); + + DEBUG("domain=%p, (VM: %s)", domain, NULLSTR(name)); virResetLastError(); @@ -5665,7 +5766,9 @@ int virDomainAttachDevice(virDomainPtr domain, const char *xml) { virConnectPtr conn; - DEBUG("domain=%p, xml=%s", domain, xml); + const char *name = virDomainGetName(domain); + + DEBUG("domain=%p, xml=%s, (VM: %s)", domain, xml, NULLSTR(name)); virResetLastError(); @@ -5724,7 +5827,10 @@ virDomainAttachDeviceFlags(virDomainPtr domain, const char *xml, unsigned int flags) { virConnectPtr conn; - DEBUG("domain=%p, xml=%s, flags=%d", domain, xml, flags); + const char *name = virDomainGetName(domain); + + DEBUG("domain=%p, xml=%s, flags=%d, (VM: %s)", domain, xml, + flags, NULLSTR(name)); virResetLastError(); @@ -5767,7 +5873,9 @@ int virDomainDetachDevice(virDomainPtr domain, const char *xml) { virConnectPtr conn; - DEBUG("domain=%p, xml=%s", domain, xml); + const char *name = virDomainGetName(domain); + + DEBUG("domain=%p, xml=%s, (VM: %s)", domain, xml, NULLSTR(name)); virResetLastError(); @@ -5822,7 +5930,10 @@ virDomainDetachDeviceFlags(virDomainPtr domain, const char *xml, unsigned int flags) { virConnectPtr conn; - DEBUG("domain=%p, xml=%s, flags=%d", domain, xml, flags); + const char *name = virDomainGetName(domain); + + DEBUG("domain=%p, xml=%s, flags=%d, (VM: %s)", domain, xml, flags, + NULLSTR(name)); virResetLastError(); @@ -5880,7 +5991,10 @@ virDomainUpdateDeviceFlags(virDomainPtr domain, const char *xml, unsigned int flags) { virConnectPtr conn; - DEBUG("domain=%p, xml=%s, flags=%d", domain, xml, flags); + const char *name = virDomainGetName(domain); + + DEBUG("domain=%p, xml=%s, flags=%d, (VM: %s)", domain, xml, flags, + NULLSTR(name)); virResetLastError(); @@ -6206,7 +6320,10 @@ error: virNetworkPtr virNetworkLookupByUUID(virConnectPtr conn, const unsigned char *uuid) { - DEBUG("conn=%p, uuid=%s", conn, uuid); + char uuidstr[VIR_UUID_STRING_BUFLEN]; + virUUIDFormat(uuid, uuidstr); + + DEBUG("conn=%p, uuid=%s", conn, uuidstr); virResetLastError(); @@ -11431,7 +11548,9 @@ error: */ int virDomainIsPersistent(virDomainPtr dom) { - DEBUG("dom=%p", dom); + const char *name = virDomainGetName(dom); + + DEBUG("domain=%p, (VM: %s)", dom, NULLSTR(name)); virResetLastError(); @@ -11464,7 +11583,9 @@ error: */ int virDomainIsUpdated(virDomainPtr dom) { - DEBUG("dom=%p", dom); + const char *name = virDomainGetName(dom); + + DEBUG("domain=%p, (VM: %s)", dom, NULLSTR(name)); virResetLastError(); @@ -12353,7 +12474,9 @@ int virDomainGetJobInfo(virDomainPtr domain, virDomainJobInfoPtr info) { virConnectPtr conn; - DEBUG("domain=%p, info=%p", domain, info); + const char *name = virDomainGetName(domain); + + DEBUG("domain=%p, info=%p, (VM: %s)", domain, info, NULLSTR(name)); virResetLastError(); @@ -12401,8 +12524,9 @@ int virDomainAbortJob(virDomainPtr domain) { virConnectPtr conn; + const char *name = virDomainGetName(domain); - DEBUG("domain=%p", domain); + DEBUG("domain=%p, (VM: %s)", domain, NULLSTR(name)); virResetLastError(); @@ -12452,8 +12576,10 @@ virDomainMigrateSetMaxDowntime(virDomainPtr domain, unsigned int flags) { virConnectPtr conn; + const char *name = virDomainGetName(domain); - DEBUG("domain=%p, downtime=%llu, flags=%u", domain, downtime, flags); + DEBUG("domain=%p, downtime=%llu, flags=%u, (VM: %s)", domain, + downtime, flags, NULLSTR(name)); virResetLastError(); @@ -12522,7 +12648,10 @@ virConnectDomainEventRegisterAny(virConnectPtr conn, void *opaque, virFreeCallback freecb) { - DEBUG("conn=%p dom=%p, eventID=%d, cb=%p, opaque=%p, freecb=%p", conn, dom, eventID, cb, opaque, freecb); + const char *name = virDomainGetName(dom); + + DEBUG("conn=%p dom=%p, eventID=%d, cb=%p, opaque=%p, freecb=%p, (VM: %s)", + conn, dom, eventID, cb, opaque, freecb, NULLSTR(name)); virResetLastError(); if (!VIR_IS_CONNECT(conn)) { @@ -12614,8 +12743,9 @@ error: int virDomainManagedSave(virDomainPtr dom, unsigned int flags) { virConnectPtr conn; + const char *name = virDomainGetName(dom); - VIR_DEBUG("dom=%p, flags=%u", dom, flags); + VIR_DEBUG("dom%p, flags=%u, (VM: %s)", dom, flags, NULLSTR(name)); virResetLastError(); @@ -12662,8 +12792,9 @@ error: int virDomainHasManagedSaveImage(virDomainPtr dom, unsigned int flags) { virConnectPtr conn; + const char *name = virDomainGetName(dom); - VIR_DEBUG("dom=%p, flags=%u", dom, flags); + VIR_DEBUG("dom=%p, flags=%u, (VM: %s)", dom, flags, NULLSTR(name)); virResetLastError(); @@ -12703,8 +12834,9 @@ error: int virDomainManagedSaveRemove(virDomainPtr dom, unsigned int flags) { virConnectPtr conn; + const char *name = virDomainGetName(dom); - VIR_DEBUG("dom=%p, flags=%u", dom, flags); + VIR_DEBUG("dom=%p, flags=%u, (VM: %s)", dom, flags, NULLSTR(name)); virResetLastError(); @@ -12753,8 +12885,10 @@ virDomainSnapshotCreateXML(virDomainPtr domain, unsigned int flags) { virConnectPtr conn; + const char *name = virDomainGetName(domain); - DEBUG("domain=%p, xmlDesc=%s, flags=%u", domain, xmlDesc, flags); + DEBUG("domain=%p, xmlDesc=%s, flags=%u, (VM: %s)", domain, xmlDesc, + flags, NULLSTR(name)); virResetLastError(); @@ -12845,7 +12979,9 @@ int virDomainSnapshotNum(virDomainPtr domain, unsigned int flags) { virConnectPtr conn; - DEBUG("domain=%p", domain); + const char *name = virDomainGetName(domain); + + DEBUG("domain=%p, (VM: %s)", domain, NULLSTR(name)); virResetLastError(); @@ -12887,9 +13023,10 @@ virDomainSnapshotListNames(virDomainPtr domain, char **names, int nameslen, unsigned int flags) { virConnectPtr conn; + const char *name = virDomainGetName(domain); - DEBUG("domain=%p, names=%p, nameslen=%d, flags=%u", - domain, names, nameslen, flags); + DEBUG("domain=%p, names=%p, nameslen=%d, flags=%u, (VM: %s)", + domain, names, nameslen, flags, NULLSTR(name)); virResetLastError(); @@ -12938,7 +13075,10 @@ virDomainSnapshotLookupByName(virDomainPtr domain, unsigned int flags) { virConnectPtr conn; - DEBUG("domain=%p, name=%s, flags=%u", domain, name, flags); + const char *domname = virDomainGetName(domain); + + DEBUG("domain=%p, name=%s, flags=%u, (VM: %s)", domain, domname, + flags, NULLSTR(name)); virResetLastError(); @@ -12982,7 +13122,9 @@ int virDomainHasCurrentSnapshot(virDomainPtr domain, unsigned int flags) { virConnectPtr conn; - DEBUG("domain=%p, flags=%u", domain, flags); + const char *name = virDomainGetName(domain); + + DEBUG("domain=%p, flags=%u, (VM: %s)", domain, flags, NULLSTR(name)); virResetLastError(); @@ -13023,7 +13165,9 @@ virDomainSnapshotCurrent(virDomainPtr domain, unsigned int flags) { virConnectPtr conn; - DEBUG("domain=%p, flags=%u", domain, flags); + const char *name = virDomainGetName(domain); + + DEBUG("domain=%p, flags=%u, (VM: %s)", domain, flags, NULLSTR(name)); virResetLastError(); @@ -13187,7 +13331,10 @@ int virDomainOpenConsole(virDomainPtr dom, unsigned int flags) { virConnectPtr conn; - DEBUG("dom=%p devname=%s, st=%p flags=%u", dom, NULLSTR(devname), st, flags); + const char *name = virDomainGetName(dom); + + DEBUG("dom=%p, devname=%s, st=%p, flags=%u, (VM: %s)", dom, + NULLSTR(devname), st, flags, NULLSTR(name)); virResetLastError(); -- 1.7.3.2
On Fri, Dec 24, 2010 at 10:31:20AM +0800, Osier Yang wrote:
Add VM name/UUID in log for domain related APIs. Format: "param0=%p, param1=%p, (VM: %s)"
* src/libvirt.c --- src/libvirt.c | 293 +++++++++++++++++++++++++++++++++++++++++++-------------- 1 files changed, 220 insertions(+), 73 deletions(-)
diff --git a/src/libvirt.c b/src/libvirt.c index ee2495a..cd1cf6e 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -1961,7 +1961,9 @@ error: virConnectPtr 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. Logging the name alone is also potentially misleading, since most of the API impls use the UUID and ignore the name. Daniel
于 2011年01月04日 23:30, Daniel P. Berrange 写道:
On Fri, Dec 24, 2010 at 10:31:20AM +0800, Osier Yang wrote:
Add VM name/UUID in log for domain related APIs. Format: "param0=%p, param1=%p, (VM: %s)"
* src/libvirt.c --- src/libvirt.c | 293 +++++++++++++++++++++++++++++++++++++++++++-------------- 1 files changed, 220 insertions(+), 73 deletions(-)
diff --git a/src/libvirt.c b/src/libvirt.c index ee2495a..cd1cf6e 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -1961,7 +1961,9 @@ error: virConnectPtr 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. Logging the name alone is also potentially misleading, since most of the API impls use the UUID and ignore the name.
hmm, yes, thanks for the reviewing, will make a v3 patch. Regards Osier
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. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
于 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.
[snip] I'm not even sure what the right internal APIs for the job would
be, or if they even exist yet. [/snip]
I'm also thinking about it, anyone has further idea? Regards Osier
于 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
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@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
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) NB, this relies on 'fmt' being a literal string in the caller, to avoid malloc'ing, but that's already true. This should be low enough overhead that we don't need to have "if ! DEBUG then return" - at least not seriously worse than our existing overhead - its only adding a single virUUIDFormat call So int virDomainSetMaxMemory(virDomainPtr domain, unsigned long memory) { virConnectPtr conn; DEBUG("domain=%p, memory=%lu", domain, memory); would thus become int virDomainSetMaxMemory(virDomainPtr domain, unsigned long memory) { virConnectPtr conn; DEBUG_DOMAIN(domain, "memory=%lu", memory); Daniel
于 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
participants (3)
-
Daniel P. Berrange -
Eric Blake -
Osier Yang