11:32 a.m.
On 12/28/2013 11:11 AM, Eric Blake wrote:
Most of our public APIs emit a debug log on entry, prior to anything
else. There were a few exceptions where obvious failures were not
logged, so fix those. Many of the fixes are made more careful to
s/careful/carefully/
avoid a NULL deref if the user passes NULL for the object.
* src/libvirt.c (virConnectOpen, virConnectOpenReadOnly)
(virConnectOpenAuth, virConnectRef, virDomainRef)
(virNetworkRef, virInterfaceRef, virStoragePoolRef)
(virStorageVolRef, virNodeDeviceRef, virSecretRef, virStreamRef)
(virNWFilterRef, virDomainSnapshotRef): Debug on function entry.
* src/libvirt-lxc.c (virDomainLxcEnterNamespace)
(virDomainLxcEnterSecurityLabel): Likewise.
Signed-off-by: Eric Blake <eblake(a)redhat.com>
---
src/libvirt-lxc.c | 7 +++++++
src/libvirt.c | 53 +++++++++++++++++++++++++++++++++++------------------
2 files changed, 42 insertions(+), 18 deletions(-)
diff --git a/src/libvirt-lxc.c b/src/libvirt-lxc.c
index 5cbe8bf..7ffed3e 100644
--- a/src/libvirt-lxc.c
+++ b/src/libvirt-lxc.c
@@ -138,6 +138,10 @@ virDomainLxcEnterNamespace(virDomainPtr domain,
{
size_t i;
+ VIR_DOMAIN_DEBUG(domain, "nfdlist=%d, fdlist=%p, "
+ "noldfdlist=%p, oldfdlist=%p, flags=%x",
+ nfdlist, fdlist, noldfdlist, oldfdlist, flags);
+
virCheckFlagsGoto(0, error);
if (noldfdlist && oldfdlist) {
@@ -196,6 +200,9 @@ virDomainLxcEnterSecurityLabel(virSecurityModelPtr model,
virSecurityLabelPtr oldlabel,
unsigned int flags)
{
+ VIR_DEBUG("model=%p, label=%p, oldlabel=%p, flags=%x",
+ model, label, oldlabel, flags);
+
virCheckFlagsGoto(0, error);
virCheckNonNullArgGoto(model, error);
diff --git a/src/libvirt.c b/src/libvirt.c
index f145d74..815dd64 100644
--- a/src/libvirt.c
+++ b/src/libvirt.c
@@ -1367,10 +1367,11 @@ virConnectOpen(const char *name)
{
virConnectPtr ret = NULL;
+ VIR_DEBUG("name=%s", NULLSTR(name));
+
if (virInitialize() < 0)
goto error;
- VIR_DEBUG("name=%s", NULLSTR(name));
For this and other virConnect*()'s being moved before virInitialize()...
Something tells me there's some "historical reason" for the VIR_DEBUG()
after virInitialize(). Something that perhaps some assumption that
virLogMessage() or what it calls may assume has been initialized properly...
Although I suppose, since virGetVersion() already will VIR_DEBUG before
virInitialize(), it's perhaps just an overly paranoid observation based
on a previous job where debug/output logs got filled with messages
because due to a similar change and some thread was waiting for
initialization to complete and expected a failure from an entry routine
such as this. Having 100's of entries in the log was "of concern".
virResetLastError();
ret = do_open(name, NULL, 0);
if (!ret)
@@ -1403,10 +1404,11 @@ virConnectOpenReadOnly(const char *name)
{
virConnectPtr ret = NULL;
+ VIR_DEBUG("name=%s", NULLSTR(name));
+
if (virInitialize() < 0)
goto error;
- VIR_DEBUG("name=%s", NULLSTR(name));
virResetLastError();
ret = do_open(name, NULL, VIR_CONNECT_RO);
if (!ret)
@@ -1443,10 +1445,11 @@ virConnectOpenAuth(const char *name,
{
virConnectPtr ret = NULL;
+ VIR_DEBUG("name=%s, auth=%p, flags=%x", NULLSTR(name), auth, flags);
+
if (virInitialize() < 0)
goto error;
- VIR_DEBUG("name=%s, auth=%p, flags=%x", NULLSTR(name), auth, flags);
virResetLastError();
ret = do_open(name, auth, flags);
if (!ret)
@@ -1530,12 +1533,13 @@ error:
int
virConnectRef(virConnectPtr conn)
{
+ VIR_DEBUG("conn=%p refs=%d", conn, conn ? conn->object.u.s.refs : 0);
+
if ((!VIR_IS_CONNECT(conn))) {
virLibConnError(VIR_ERR_INVALID_CONN, __FUNCTION__);
virDispatchError(NULL);
return -1;
}
- VIR_DEBUG("conn=%p refs=%d", conn, conn->object.u.s.refs);
virObjectRef(conn);
return 0;
}
@@ -2457,13 +2461,14 @@ virDomainFree(virDomainPtr domain)
int
virDomainRef(virDomainPtr domain)
{
+ VIR_DOMAIN_DEBUG(domain, "refs=%d", domain ? domain->object.u.s.refs :
0);
+
if ((!VIR_IS_CONNECTED_DOMAIN(domain))) {
virLibConnError(VIR_ERR_INVALID_DOMAIN, __FUNCTION__);
virDispatchError(NULL);
return -1;
}
- VIR_DOMAIN_DEBUG(domain, "refs=%d", domain->object.u.s.refs);
virObjectRef(domain);
return 0;
}
@@ -7033,7 +7038,6 @@ virDomainMigratePrepareTunnel3(virConnectPtr conn,
const char *dname,
unsigned long bandwidth,
const char *dom_xml)
-
Seems to be something more relevant to patch 1 (downside of peeking at
finished product for me I guess).
{
VIR_DEBUG("conn=%p, stream=%p, cookiein=%p, cookieinlen=%d, cookieout=%p,
"
"cookieoutlen=%p, flags=%lx, dname=%s, bandwidth=%lu, "
@@ -7362,7 +7366,6 @@ virDomainMigratePrepareTunnel3Params(virConnectPtr conn,
char **cookieout,
int *cookieoutlen,
unsigned int flags)
-
Same here w/r/t patch1
{
VIR_DEBUG("conn=%p, stream=%p, params=%p, nparams=%d, cookiein=%p, "
"cookieinlen=%d, cookieout=%p, cookieoutlen=%p, flags=%x",
@@ -7898,7 +7901,6 @@ virNodeSuspendForDuration(virConnectPtr conn,
unsigned long long duration,
unsigned int flags)
{
-
Same here w/r/t patch1
The rest is mechanical code movement and the extra checks for printing
of "->object.u.s.refs"
ACK - as long as you're satisfied that calling VIR_DEBUG before
virInitialize is ok
John
VIR_DEBUG("conn=%p, target=%d, duration=%lld,
flags=%x",
conn, target, duration, flags);
@@ -12078,12 +12080,14 @@ virNetworkFree(virNetworkPtr network)
int
virNetworkRef(virNetworkPtr network)
{
+ VIR_DEBUG("network=%p refs=%d", network,
+ network ? network->object.u.s.refs : 0);
+
if ((!VIR_IS_CONNECTED_NETWORK(network))) {
virLibConnError(VIR_ERR_INVALID_NETWORK, __FUNCTION__);
virDispatchError(NULL);
return -1;
}
- VIR_DEBUG("network=%p refs=%d", network, network->object.u.s.refs);
virObjectRef(network);
return 0;
}
@@ -13046,12 +13050,13 @@ error:
int
virInterfaceRef(virInterfacePtr iface)
{
+ VIR_DEBUG("iface=%p refs=%d", iface, iface ? iface->object.u.s.refs :
0);
+
if ((!VIR_IS_CONNECTED_INTERFACE(iface))) {
virLibConnError(VIR_ERR_INVALID_INTERFACE, __FUNCTION__);
virDispatchError(NULL);
return -1;
}
- VIR_DEBUG("iface=%p refs=%d", iface, iface->object.u.s.refs);
virObjectRef(iface);
return 0;
}
@@ -14116,12 +14121,13 @@ virStoragePoolFree(virStoragePoolPtr pool)
int
virStoragePoolRef(virStoragePoolPtr pool)
{
+ VIR_DEBUG("pool=%p refs=%d", pool, pool ? pool->object.u.s.refs : 0);
+
if ((!VIR_IS_CONNECTED_STORAGE_POOL(pool))) {
virLibConnError(VIR_ERR_INVALID_STORAGE_POOL, __FUNCTION__);
virDispatchError(NULL);
return -1;
}
- VIR_DEBUG("pool=%p refs=%d", pool, pool->object.u.s.refs);
virObjectRef(pool);
return 0;
}
@@ -15242,12 +15248,13 @@ virStorageVolFree(virStorageVolPtr vol)
int
virStorageVolRef(virStorageVolPtr vol)
{
+ VIR_DEBUG("vol=%p refs=%d", vol, vol ? vol->object.u.s.refs : 0);
+
if ((!VIR_IS_CONNECTED_STORAGE_VOL(vol))) {
virLibConnError(VIR_ERR_INVALID_STORAGE_VOL, __FUNCTION__);
virDispatchError(NULL);
return -1;
}
- VIR_DEBUG("vol=%p refs=%d", vol, vol->object.u.s.refs);
virObjectRef(vol);
return 0;
}
@@ -15947,12 +15954,13 @@ virNodeDeviceFree(virNodeDevicePtr dev)
int
virNodeDeviceRef(virNodeDevicePtr dev)
{
+ VIR_DEBUG("dev=%p refs=%d", dev, dev ? dev->object.u.s.refs : 0);
+
if ((!VIR_IS_CONNECTED_NODE_DEVICE(dev))) {
virLibConnError(VIR_ERR_INVALID_NODE_DEVICE, __FUNCTION__);
virDispatchError(NULL);
return -1;
}
- VIR_DEBUG("dev=%p refs=%d", dev, dev->object.u.s.refs);
virObjectRef(dev);
return 0;
}
@@ -17074,12 +17082,14 @@ error:
int
virSecretRef(virSecretPtr secret)
{
+ VIR_DEBUG("secret=%p refs=%d", secret,
+ secret ? secret->object.u.s.refs : 0);
+
if (!VIR_IS_CONNECTED_SECRET(secret)) {
virLibSecretError(VIR_ERR_INVALID_SECRET, __FUNCTION__);
virDispatchError(NULL);
return -1;
}
- VIR_DEBUG("secret=%p refs=%d", secret, secret->object.u.s.refs);
virObjectRef(secret);
return 0;
}
@@ -17169,12 +17179,14 @@ virStreamNew(virConnectPtr conn,
int
virStreamRef(virStreamPtr stream)
{
+ VIR_DEBUG("stream=%p refs=%d", stream,
+ stream ? stream->object.u.s.refs : 0);
+
if ((!VIR_IS_CONNECTED_STREAM(stream))) {
virLibConnError(VIR_ERR_INVALID_STREAM, __FUNCTION__);
virDispatchError(NULL);
return -1;
}
- VIR_DEBUG("stream=%p refs=%d", stream, stream->object.u.s.refs);
virObjectRef(stream);
return 0;
}
@@ -17597,7 +17609,8 @@ virStreamEventAddCallback(virStreamPtr stream,
void *opaque,
virFreeCallback ff)
{
- VIR_DEBUG("stream=%p, events=%d, cb=%p, opaque=%p, ff=%p", stream, events,
cb, opaque, ff);
+ VIR_DEBUG("stream=%p, events=%d, cb=%p, opaque=%p, ff=%p",
+ stream, events, cb, opaque, ff);
virResetLastError();
@@ -18606,12 +18619,14 @@ error:
int
virNWFilterRef(virNWFilterPtr nwfilter)
{
+ VIR_DEBUG("nwfilter=%p refs=%d", nwfilter,
+ nwfilter ? nwfilter->object.u.s.refs : 0);
+
if ((!VIR_IS_CONNECTED_NWFILTER(nwfilter))) {
virLibConnError(VIR_ERR_INVALID_NWFILTER, __FUNCTION__);
virDispatchError(NULL);
return -1;
}
- VIR_DEBUG("nwfilter=%p refs=%d", nwfilter, nwfilter->object.u.s.refs);
virObjectRef(nwfilter);
return 0;
}
@@ -20944,13 +20959,15 @@ error:
int
virDomainSnapshotRef(virDomainSnapshotPtr snapshot)
{
+ VIR_DEBUG("snapshot=%p, refs=%d", snapshot,
+ snapshot ? snapshot->object.u.s.refs : 0);
+
if ((!VIR_IS_DOMAIN_SNAPSHOT(snapshot))) {
virLibDomainSnapshotError(VIR_ERR_INVALID_DOMAIN_SNAPSHOT,
__FUNCTION__);
virDispatchError(NULL);
return -1;
}
- VIR_DEBUG("snapshot=%p, refs=%d", snapshot,
snapshot->object.u.s.refs);
virObjectRef(snapshot);
return 0;
}