On 11/12/2015 12:19 PM, Daniel P. Berrange wrote:
> Add the virLogManager API which allows for communication with
> the virtlogd daemon to RPC program. This provides the client
> side API to open log files for guest domains.
>
> The virtlogd daemon is setup to auto-spawn on first use when
> running unprivileged. For privileged usage, systemd socket
> activation is used instead.
>
> Signed-off-by: Daniel P. Berrange <berrange@xxxxxxxxxx>
> ---
> po/POTFILES.in | 2 +
> src/Makefile.am | 4 +-
> src/libvirt_private.syms | 8 ++
> src/logging/log_daemon_dispatch.c | 7 +
> src/logging/log_handler.c | 7 +-
> src/logging/log_manager.c | 283 ++++++++++++++++++++++++++++++++++++++
> src/logging/log_manager.h | 61 ++++++++
> src/logging/log_protocol.x | 1 +
> 8 files changed, 370 insertions(+), 3 deletions(-)
> create mode 100644 src/logging/log_manager.c
> create mode 100644 src/logging/log_manager.h
>
[...]
> diff --git a/src/logging/log_daemon_dispatch.c b/src/logging/log_daemon_dispatch.c
> index f612be1..203fdc4 100644
> --- a/src/logging/log_daemon_dispatch.c
> +++ b/src/logging/log_daemon_dispatch.c
> @@ -116,6 +116,13 @@ virLogManagerProtocolDispatchDomainReadLogFile(virNetServerPtr server ATTRIBUTE_
> int rv = -1;
> char *data;
>
> + if (args->maxlen > VIR_LOG_MANAGER_PROTOCOL_STRING_MAX) {
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("Requested data len %zu is larger than maximum %d"),
> + args->maxlen, VIR_LOG_MANAGER_PROTOCOL_STRING_MAX);
> + goto cleanup;
> + }
> +
This feels like it should have been in prior patch...
> if ((data = virLogHandlerDomainReadLogFile(virLogDaemonGetHandler(logDaemon),
> args->driver,
> (unsigned char *)args->dom.uuid,
> diff --git a/src/logging/log_handler.c b/src/logging/log_handler.c
> index 8853fd0..1465c5a 100644
> --- a/src/logging/log_handler.c
> +++ b/src/logging/log_handler.c
> @@ -442,6 +442,7 @@ char *virLogHandlerDomainReadLogFile(virLogHandlerPtr handler,
> char *path;
> virRotatingFileReaderPtr file = NULL;
> char *data = NULL;
> + ssize_t got;
>
> if (!(path = virLogHandlerGetLogFilePathForDomain(handler,
> driver,
> @@ -455,11 +456,13 @@ char *virLogHandlerDomainReadLogFile(virLogHandlerPtr handler,
> if (virRotatingFileReaderSeek(file, inode, offset) < 0)
> goto error;
>
> - if (VIR_ALLOC_N(data, maxlen) < 0)
> + if (VIR_ALLOC_N(data, maxlen + 1) < 0)
> goto error;
>
> - if (virRotatingFileReaderConsume(file, data, maxlen) < 0)
> + got = virRotatingFileReaderConsume(file, data, maxlen);
> + if (got < 0)
> goto error;
> + data[got] = '\0';
Perhaps another prior patch..
>
> virRotatingFileReaderFree(file);
> return data;
[...] Nothing jumped out at me in log_manager.{ch}
> diff --git a/src/logging/log_protocol.x b/src/logging/log_protocol.x
> index 2702beb..de57c69 100644
> --- a/src/logging/log_protocol.x
> +++ b/src/logging/log_protocol.x
> @@ -57,6 +57,7 @@ struct virLogManagerProtocolDomainReadLogFileArgs {
> virLogManagerProtocolDomain dom;
> virLogManagerProtocolLogFilePosition pos;
> unsigned hyper maxlen;
> + unsigned int flags;
should this perhaps have been in the prior patch?
> };
>
> struct virLogManagerProtocolDomainReadLogFileRet {
>
ACK - nothing major here - your call on whether things should move.
John
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list