[libvirt] [PATCH 0/8] Cleanup improper VIR_ERR_NO_SUPPORT use
by Osier Yang
Error code VIR_ERR_NO_SUPPORT will be translated to "this function
is not supported by the connection driver", however, it's used
across the whole projects, this patch is trying to cleanup all
the improper use in the source tree.
The modification can be grouped to 3 following groups:
1) The error intends to tell user it's invalid operation.
s/VIR_ERR_NO_SUPPORT/VIR_ERR_OPERATION_INVALID.
2) The error intends to tell the configuration of domain
is not supported.
s/VIR_ERR_NO_SUPPORT/VIR_ERR_CONFIG_UNSUPPORTED/
3) The error intends to tell the function is not implemented
on some platform.
* s/VIR_ERR_NO_SUPPORT/VIR_ERR_OPERATION_INVALID/
* and add error strings
e.g.
static int
lxcDomainInterfaceStats(virDomainPtr dom,
const char *path ATTRIBUTE_UNUSED,
struct _virDomainInterfaceStats *stats ATTRIBUTE_UNUSED)
-lxcError(VIR_ERR_NO_SUPPORT, __FUNCTION__);
+lxcError(VIR_ERR_OPERATION_INVALID, "%s",
+ _("interface stats not implemented on this platform"));
return -1;
}
[PATCH 1/8] conf: Cleanup improper use of VIR_ERR_NO_SUPPORT in
[PATCH 2/8] lxc: Cleanup improper VIR_ERR_NO_SUPPORT use
[PATCH 3/8] nodeinfo: Cleanup improper VIR_ERR_NO_SUPPORT use
[PATCH 4/8] qemu: Cleanup improper VIR_ERR_NO_SUPPORT use
[PATCH 5/8] remote: Cleanup improper VIR_ERR_NO_SUPPORT use
[PATCH 6/8] storage: Cleanup improper VIR_ERR_NO_SUPPORT use
[PATCH 7/8] test: Cleanup improper VIR_ERR_NO_SUPPORT use
[PATCH 8/8] xen: Cleanup improper VIR_ERR_NO_SUPPORT use
Regards
Osier
13 years, 7 months
[libvirt] [PATCH] Fix tracking of RPC messages wrt streams
by Daniel P. Berrange
From: "Daniel P. Berrange" <berrange(a)redhat.com>
Commit 2c85644b0b51fbe5b6244e6773531af29933a727 attempted to
fix a problem with tracking RPC messages from streams by doing
- if (msg->header.type == VIR_NET_REPLY) {
+ if (msg->header.type == VIR_NET_REPLY ||
+ (msg->header.type == VIR_NET_STREAM &&
+ msg->header.status != VIR_NET_CONTINUE)) {
client->nrequests--;
In other words any stream packet, with status NET_OK or NET_ERROR
would cause nrequests to be decremented. This is great if the
packet from from a synchronous virStreamFinish or virStreamAbort
API call, but wildly wrong if from a server initiated abort.
The latter resulted in 'nrequests' being decremented below zero.
This then causes all I/O for that client to be stopped.
Instead of trying to infer whether we need to decrement the
nrequests field, from the message type/status, introduce an
explicit 'bool tracked' field to mark whether the virNetMessagePtr
object is subject to tracking.
Also add a virNetMessageClear function to allow a message
contents to be cleared out, without adversely impacting the
'tracked' field as a naive memset() would do
* src/rpc/virnetmessage.c, src/rpc/virnetmessage.h: Add
a 'bool tracked' field and virNetMessageClear() API
* daemon/remote.c, daemon/stream.c, src/rpc/virnetclientprogram.c,
src/rpc/virnetclientstream.c, src/rpc/virnetserverclient.c,
src/rpc/virnetserverprogram.c: Switch over to use
virNetMessageClear() and pass in the 'bool tracked' value
when creating messages.
---
daemon/remote.c | 2 +-
daemon/stream.c | 10 +++++-----
src/rpc/virnetclientprogram.c | 2 +-
src/rpc/virnetclientstream.c | 4 ++--
src/rpc/virnetmessage.c | 14 ++++++++++++--
src/rpc/virnetmessage.h | 6 +++++-
src/rpc/virnetserverclient.c | 12 +++++-------
src/rpc/virnetserverprogram.c | 2 +-
8 files changed, 32 insertions(+), 20 deletions(-)
diff --git a/daemon/remote.c b/daemon/remote.c
index 34c6364..d5ead81 100644
--- a/daemon/remote.c
+++ b/daemon/remote.c
@@ -2495,7 +2495,7 @@ remoteDispatchDomainEventSend(virNetServerClientPtr client,
{
virNetMessagePtr msg;
- if (!(msg = virNetMessageNew()))
+ if (!(msg = virNetMessageNew(false)))
goto cleanup;
msg->header.prog = virNetServerProgramGetID(program);
diff --git a/daemon/stream.c b/daemon/stream.c
index ba3adc2..e3214c2 100644
--- a/daemon/stream.c
+++ b/daemon/stream.c
@@ -207,7 +207,7 @@ daemonStreamEvent(virStreamPtr st, int events, void *opaque)
virNetError(VIR_ERR_RPC,
"%s", _("stream had I/O failure"));
- msg = virNetMessageNew();
+ msg = virNetMessageNew(false);
if (!msg) {
ret = -1;
} else {
@@ -344,7 +344,7 @@ int daemonFreeClientStream(virNetServerClientPtr client,
virNetMessagePtr tmp = msg->next;
if (client) {
/* Send a dummy reply to free up 'msg' & unblock client rx */
- memset(msg, 0, sizeof(*msg));
+ virNetMessageClear(msg);
msg->header.type = VIR_NET_REPLY;
if (virNetServerClientSendMessage(client, msg) < 0) {
virNetServerClientImmediateClose(client);
@@ -653,7 +653,7 @@ daemonStreamHandleWrite(virNetServerClientPtr client,
* its active request count / throttling
*/
if (msg->header.status == VIR_NET_CONTINUE) {
- memset(msg, 0, sizeof(*msg));
+ virNetMessageClear(msg);
msg->header.type = VIR_NET_REPLY;
if (virNetServerClientSendMessage(client, msg) < 0) {
virNetMessageFree(msg);
@@ -715,7 +715,7 @@ daemonStreamHandleRead(virNetServerClientPtr client,
memset(&rerr, 0, sizeof(rerr));
- if (!(msg = virNetMessageNew()))
+ if (!(msg = virNetMessageNew(false)))
ret = -1;
else
ret = virNetServerProgramSendStreamError(remoteProgram,
@@ -729,7 +729,7 @@ daemonStreamHandleRead(virNetServerClientPtr client,
stream->tx = 0;
if (ret == 0)
stream->recvEOF = 1;
- if (!(msg = virNetMessageNew()))
+ if (!(msg = virNetMessageNew(false)))
ret = -1;
if (msg) {
diff --git a/src/rpc/virnetclientprogram.c b/src/rpc/virnetclientprogram.c
index c39520a..a07b744 100644
--- a/src/rpc/virnetclientprogram.c
+++ b/src/rpc/virnetclientprogram.c
@@ -272,7 +272,7 @@ int virNetClientProgramCall(virNetClientProgramPtr prog,
{
virNetMessagePtr msg;
- if (!(msg = virNetMessageNew()))
+ if (!(msg = virNetMessageNew(false)))
return -1;
msg->header.prog = prog->program;
diff --git a/src/rpc/virnetclientstream.c b/src/rpc/virnetclientstream.c
index fe15acd..2cc84d4 100644
--- a/src/rpc/virnetclientstream.c
+++ b/src/rpc/virnetclientstream.c
@@ -328,7 +328,7 @@ int virNetClientStreamSendPacket(virNetClientStreamPtr st,
bool wantReply;
VIR_DEBUG("st=%p status=%d data=%p nbytes=%zu", st, status, data, nbytes);
- if (!(msg = virNetMessageNew()))
+ if (!(msg = virNetMessageNew(false)))
return -1;
virMutexLock(&st->lock);
@@ -390,7 +390,7 @@ int virNetClientStreamRecvPacket(virNetClientStreamPtr st,
goto cleanup;
}
- if (!(msg = virNetMessageNew())) {
+ if (!(msg = virNetMessageNew(false))) {
virReportOOMError();
goto cleanup;
}
diff --git a/src/rpc/virnetmessage.c b/src/rpc/virnetmessage.c
index 0725491..a1ae9c4 100644
--- a/src/rpc/virnetmessage.c
+++ b/src/rpc/virnetmessage.c
@@ -32,7 +32,7 @@
virReportErrorHelper(VIR_FROM_THIS, code, __FILE__, \
__FUNCTION__, __LINE__, __VA_ARGS__)
-virNetMessagePtr virNetMessageNew(void)
+virNetMessagePtr virNetMessageNew(bool tracked)
{
virNetMessagePtr msg;
@@ -41,11 +41,21 @@ virNetMessagePtr virNetMessageNew(void)
return NULL;
}
- VIR_DEBUG("msg=%p", msg);
+ msg->tracked = tracked;
+ VIR_DEBUG("msg=%p tracked=%d", msg, tracked);
return msg;
}
+
+void virNetMessageClear(virNetMessagePtr msg)
+{
+ bool tracked = msg->tracked;
+ memset(msg, 0, sizeof(*msg));
+ msg->tracked = tracked;
+}
+
+
void virNetMessageFree(virNetMessagePtr msg)
{
if (!msg)
diff --git a/src/rpc/virnetmessage.h b/src/rpc/virnetmessage.h
index 2aae3f6..307a041 100644
--- a/src/rpc/virnetmessage.h
+++ b/src/rpc/virnetmessage.h
@@ -35,6 +35,8 @@ typedef void (*virNetMessageFreeCallback)(virNetMessagePtr msg, void *opaque);
* use virNetMessageNew() to allocate on the heap
*/
struct _virNetMessage {
+ bool tracked;
+
char buffer[VIR_NET_MESSAGE_MAX + VIR_NET_MESSAGE_LEN_MAX];
size_t bufferLength;
size_t bufferOffset;
@@ -48,7 +50,9 @@ struct _virNetMessage {
};
-virNetMessagePtr virNetMessageNew(void);
+virNetMessagePtr virNetMessageNew(bool tracked);
+
+void virNetMessageClear(virNetMessagePtr);
void virNetMessageFree(virNetMessagePtr msg);
diff --git a/src/rpc/virnetserverclient.c b/src/rpc/virnetserverclient.c
index a73b06d..412814d 100644
--- a/src/rpc/virnetserverclient.c
+++ b/src/rpc/virnetserverclient.c
@@ -277,7 +277,7 @@ virNetServerClientCheckAccess(virNetServerClientPtr client)
return -1;
}
- if (!(confirm = virNetMessageNew()))
+ if (!(confirm = virNetMessageNew(false)))
return -1;
/* Checks have succeeded. Write a '\1' byte back to the client to
@@ -323,7 +323,7 @@ virNetServerClientPtr virNetServerClientNew(virNetSocketPtr sock,
virNetTLSContextRef(tls);
/* Prepare one for packet receive */
- if (!(client->rx = virNetMessageNew()))
+ if (!(client->rx = virNetMessageNew(true)))
goto error;
client->rx->bufferLength = VIR_NET_MESSAGE_LEN_MAX;
client->nrequests = 1;
@@ -805,7 +805,7 @@ readmore:
/* Possibly need to create another receive buffer */
if (client->nrequests < client->nrequests_max) {
- if (!(client->rx = virNetMessageNew())) {
+ if (!(client->rx = virNetMessageNew(true))) {
client->wantClose = true;
} else {
client->rx->bufferLength = VIR_NET_MESSAGE_LEN_MAX;
@@ -885,16 +885,14 @@ virNetServerClientDispatchWrite(virNetServerClientPtr client)
/* Get finished msg from head of tx queue */
msg = virNetMessageQueueServe(&client->tx);
- if (msg->header.type == VIR_NET_REPLY ||
- (msg->header.type == VIR_NET_STREAM &&
- msg->header.status != VIR_NET_CONTINUE)) {
+ if (msg->tracked) {
client->nrequests--;
/* See if the recv queue is currently throttled */
if (!client->rx &&
client->nrequests < client->nrequests_max) {
/* Ready to recv more messages */
+ virNetMessageClear(msg);
client->rx = msg;
- memset(client->rx, 0, sizeof(*client->rx));
client->rx->bufferLength = VIR_NET_MESSAGE_LEN_MAX;
msg = NULL;
client->nrequests++;
diff --git a/src/rpc/virnetserverprogram.c b/src/rpc/virnetserverprogram.c
index 2e9e3f7..643a97d 100644
--- a/src/rpc/virnetserverprogram.c
+++ b/src/rpc/virnetserverprogram.c
@@ -284,7 +284,7 @@ int virNetServerProgramDispatch(virNetServerProgramPtr prog,
VIR_INFO("Ignoring unexpected stream data serial=%d proc=%d status=%d",
msg->header.serial, msg->header.proc, msg->header.status);
/* Send a dummy reply to free up 'msg' & unblock client rx */
- memset(msg, 0, sizeof(*msg));
+ virNetMessageClear(msg);
msg->header.type = VIR_NET_REPLY;
if (virNetServerClientSendMessage(client, msg) < 0) {
ret = -1;
--
1.7.6
13 years, 7 months
[libvirt] [PATCH] Avoid use-after-free on streams, due to message callbacks
by Daniel P. Berrange
From: "Daniel P. Berrange" <berrange(a)redhat.com>
When sending outbound stream RPC messages, a callback is
used to re-enable stream data transmission. If the stream
aborts while one of these messages is outstanding, the
stream may have been free'd by the time it is invoked. This
results in a use-after-free error
* daemon/stream.c: Ref-count streams to avoid use-after-free
---
daemon/stream.c | 10 +++++++++-
1 files changed, 9 insertions(+), 1 deletions(-)
diff --git a/daemon/stream.c b/daemon/stream.c
index 7d2b367..ba3adc2 100644
--- a/daemon/stream.c
+++ b/daemon/stream.c
@@ -38,6 +38,7 @@
struct daemonClientStream {
daemonClientPrivatePtr priv;
+ int refs;
virNetServerProgramPtr prog;
@@ -102,6 +103,8 @@ daemonStreamMessageFinished(virNetMessagePtr msg,
stream->tx = 1;
daemonStreamUpdateEvents(stream);
+
+ daemonFreeClientStream(NULL, stream);
}
@@ -299,6 +302,7 @@ daemonCreateClientStream(virNetServerClientPtr client,
return NULL;
}
+ stream->refs = 1;
stream->priv = priv;
stream->prog = prog;
stream->procedure = header->proc;
@@ -326,6 +330,10 @@ int daemonFreeClientStream(virNetServerClientPtr client,
if (!stream)
return 0;
+ stream->refs--;
+ if (stream->refs)
+ return 0;
+
VIR_DEBUG("client=%p, proc=%d, serial=%d",
client, stream->procedure, stream->serial);
@@ -727,7 +735,7 @@ daemonStreamHandleRead(virNetServerClientPtr client,
if (msg) {
msg->cb = daemonStreamMessageFinished;
msg->opaque = stream;
- virNetServerClientRef(client);
+ stream->refs++;
ret = virNetServerProgramSendStreamData(remoteProgram,
client,
msg,
--
1.7.6
13 years, 7 months
[libvirt] Python stream callback removal
by Dave Allan
I'm trying to write an example serial console implementation in python
(attached), but I'm having some trouble getting stream events to do
what I want. The console itself works fine as long as the domain
stays up, but as soon as the domain shuts down the python script goes
into a tight loop repeatedly calling the stream event callback.
Debugging indicates that the stream event callback is being requested
to be removed, but it never actually is removed which makes me think I
am not properly releasing some resource, but I was under the
impression that an error on a stream resulting in the stream aborting
was supposed to free all the resources for me. Is that not correct?
Thanks for any help anybody can provide,
Dave
13 years, 7 months
[libvirt] [PATCH] Fix memory leak dispatching domain events
by Daniel P. Berrange
From: "Daniel P. Berrange" <berrange(a)redhat.com>
When dispatching domain events we will create an XDR struct
containing the event info. Some of this data may be allocated
on the heap and so must be freed. The graphics event dispatcher
had a broken attempt to free one field, but missed others. All
the events have a dom->name string that needs freeing. The code
should have used the xdr_free() procedure for doing all this
* daemon/remote.c: Use xdr_free after dispatching events
---
daemon/remote.c | 6 +++---
1 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/daemon/remote.c b/daemon/remote.c
index 0f088c6..34c6364 100644
--- a/daemon/remote.c
+++ b/daemon/remote.c
@@ -334,8 +334,6 @@ static int remoteRelayDomainEventGraphics(virConnectPtr conn ATTRIBUTE_UNUSED,
REMOTE_PROC_DOMAIN_EVENT_GRAPHICS,
(xdrproc_t)xdr_remote_domain_event_graphics_msg, &data);
- VIR_FREE(data.subject.subject_val);
-
return 0;
}
@@ -2498,7 +2496,7 @@ remoteDispatchDomainEventSend(virNetServerClientPtr client,
virNetMessagePtr msg;
if (!(msg = virNetMessageNew()))
- return;
+ goto cleanup;
msg->header.prog = virNetServerProgramGetID(program);
msg->header.vers = virNetServerProgramGetVersion(program);
@@ -2516,10 +2514,12 @@ remoteDispatchDomainEventSend(virNetServerClientPtr client,
VIR_DEBUG("Queue event %d %zu", procnr, msg->bufferLength);
virNetServerClientSendMessage(client, msg);
+ xdr_free(proc, data);
return;
cleanup:
virNetMessageFree(msg);
+ xdr_free(proc, data);
}
static int
--
1.7.6
13 years, 7 months
[libvirt] [PATCH] Fix memory leak parsing 'relabel' attribute in domain security XML
by Daniel P. Berrange
From: "Daniel P. Berrange" <berrange(a)redhat.com>
* src/conf/domain_conf.c: Free the 'relabel' attribute
---
src/conf/domain_conf.c | 2 ++
1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 44212cf..00212db 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -5300,8 +5300,10 @@ virSecurityLabelDefParseXML(const virDomainDefPtr def,
} else {
virDomainReportError(VIR_ERR_XML_ERROR,
_("invalid security relabel value %s"), p);
+ VIR_FREE(p);
goto error;
}
+ VIR_FREE(p);
if (def->seclabel.type == VIR_DOMAIN_SECLABEL_DYNAMIC &&
def->seclabel.norelabel) {
virDomainReportError(VIR_ERR_CONFIG_UNSUPPORTED,
--
1.7.6
13 years, 7 months
[libvirt] [PATCH] Stop libxl driver polluting logs on non-Xen hosts
by Daniel P. Berrange
From: "Daniel P. Berrange" <berrange(a)redhat.com>
If the libxl driver is compiled in, then everytime libvirtd
starts up on a non-Xen Dom0 host, it logs a error message.
Since this is an expected condition, we should not log at
'error' level, only 'info'.
* src/libxl/libxl_driver.c: Lower log level for certain
expected errors during driver init
---
src/libxl/libxl_driver.c | 6 +++---
1 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index d6e0c28..3cffb5d 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -963,19 +963,19 @@ libxlStartup(int privileged) {
libxl_driver->logger =
(xentoollog_logger *)xtl_createlogger_stdiostream(libxl_driver->logger_file, XTL_DEBUG, 0);
if (!libxl_driver->logger) {
- VIR_ERROR(_("cannot create logger for libxenlight"));
+ VIR_INFO(_("cannot create logger for libxenlight, disabling driver"));
goto fail;
}
if (libxl_ctx_init(&libxl_driver->ctx,
LIBXL_VERSION,
libxl_driver->logger)) {
- VIR_ERROR(_("cannot initialize libxenlight context"));
+ VIR_INFO(_("cannot initialize libxenlight context, probably not running in a Xen Dom0, disabling driver"));
goto fail;
}
if ((ver_info = libxl_get_version_info(&libxl_driver->ctx)) == NULL) {
- VIR_ERROR(_("cannot version information from libxenlight"));
+ VIR_INFO(_("cannot version information from libxenlight, disabling driver"));
goto fail;
}
libxl_driver->version = (ver_info->xen_version_major * 1000000) +
--
1.7.6
13 years, 7 months
[libvirt] [PATCH] Don't leak memory if a cgroup is mounted multiple times
by Daniel P. Berrange
From: "Daniel P. Berrange" <berrange(a)redhat.com>
It is possible (expected/likely in Fedora 15) for a cgroup controller
to be mounted in multiple locations at the same time, due to bind
mounts. Currently we leak memory if this happens, because we overwrite
the previous 'mountPoint' string. Instead just accept the first match
we find.
* src/util/cgroup.c: Only accept first match for a cgroup
controller mount
---
src/util/cgroup.c | 5 +++++
1 files changed, 5 insertions(+), 0 deletions(-)
diff --git a/src/util/cgroup.c b/src/util/cgroup.c
index f3ec80f..c8d1f33 100644
--- a/src/util/cgroup.c
+++ b/src/util/cgroup.c
@@ -131,7 +131,12 @@ static int virCgroupDetectMounts(virCgroupPtr group)
} else {
len = strlen(tmp);
}
+ /* NB, the same controller can appear >1 time in mount list
+ * due to bind mounts from one location to another. Pick the
+ * first entry only
+ */
if (typelen == len && STREQLEN(typestr, tmp, len) &&
+ !group->controllers[i].mountPoint &&
!(group->controllers[i].mountPoint = strdup(entry.mnt_dir)))
goto no_memory;
tmp = next;
--
1.7.6
13 years, 7 months
[libvirt] [PATCH] stream: remove redundant reference to client while sending stream data
by Guannan Ren
*daemon/stream.c: remove virNetServerClientRef()
---
daemon/stream.c | 1 -
1 files changed, 0 insertions(+), 1 deletions(-)
diff --git a/daemon/stream.c b/daemon/stream.c
index 7d2b367..0333dfd 100644
--- a/daemon/stream.c
+++ b/daemon/stream.c
@@ -727,7 +727,6 @@ daemonStreamHandleRead(virNetServerClientPtr client,
if (msg) {
msg->cb = daemonStreamMessageFinished;
msg->opaque = stream;
- virNetServerClientRef(client);
ret = virNetServerProgramSendStreamData(remoteProgram,
client,
msg,
--
1.7.1
13 years, 7 months
