On Wed, Oct 17, 2012 at 08:17:16PM +0200, Miloslav Trmač wrote:
> This simplifies the top-level code, at the cost of using a little more
> stack space. The primary benefit is being able to send more fields
> without knowing in advance how many of them, and of which types, these
> fields will be, and without having to individually add buffer variables.
>
> The code imposes an upper limit on the total number of iovs/buffers
> used, and fields that wouldn't fit are silently dropped. This is not
> significant in this patch, but will affect the following one.
>
> Signed-off-by: Miloslav Trmač <mitr@xxxxxxxxxx>
> ---
> src/util/logging.c | 144 ++++++++++++++++++++++++++++++++---------------------
> 1 file changed, 87 insertions(+), 57 deletions(-)
>
> diff --git a/src/util/logging.c b/src/util/logging.c
> index a41ae8b..a6d00c9 100644
> --- a/src/util/logging.c
> +++ b/src/util/logging.c
> @@ -1043,19 +1043,78 @@ virLogAddOutputToSyslog(virLogPriority priority,
>
>
> # if USE_JOURNALD
> -# define IOVEC_SET_STRING(iov, str) \
> - do { \
> - struct iovec *_i = &(iov); \
> - _i->iov_base = (char*)str; \
> - _i->iov_len = strlen(str); \
> +# define IOVEC_SET(iov, data, size) \
> + do { \
> + struct iovec *_i = &(iov); \
> + _i->iov_base = (void*)(data); \
> + _i->iov_len = (size); \
> } while (0)
>
> -# define IOVEC_SET_INT(iov, buf, val) \
> - do { \
> - struct iovec *_i = &(iov); \
> - _i->iov_base = virFormatIntDecimal(buf, sizeof(buf), val); \
> - _i->iov_len = strlen(buf); \
> - } while (0)
> +# define IOVEC_SET_STRING(iov, str) IOVEC_SET(iov, str, strlen(str))
> +
> +/* Used for conversion of numbers to strings, and for length of binary data */
> +#define JOURNAL_BUF_SIZE (MAX(INT_BUFSIZE_BOUND(int), sizeof(uint64_t)))
> +
> +struct journalState
> +{
> + struct iovec *iov, *iov_end;
> + char (*bufs)[JOURNAL_BUF_SIZE], (*bufs_end)[JOURNAL_BUF_SIZE];
> +};
> +
> +static void
> +journalAddString(struct journalState *state, const char *field,
> + const char *value)
> +{
> + static const char newline = '\n', equals = '=';
> +
> + if (strchr(value, '\n') != NULL) {
> + uint64_t nstr;
> +
> + /* If 'str' contains a newline, then we must
> + * encode the string length, since we can't
> + * rely on the newline for the field separator
> + */
> + if (state->iov_end - state->iov < 5 || state->bufs == state->bufs_end)
> + return; /* Silently drop */
> + nstr = htole64(strlen(value));
> + memcpy(state->bufs[0], &nstr, sizeof(nstr));
> +
> + IOVEC_SET_STRING(state->iov[0], field);
> + IOVEC_SET(state->iov[1], &newline, sizeof(newline));
> + IOVEC_SET(state->iov[2], state->bufs[0], sizeof(nstr));
> + state->bufs++;
> + state->iov += 3;
> + } else {
> + if (state->iov_end - state->iov < 4)
> + return; /* Silently drop */
> + IOVEC_SET_STRING(state->iov[0], field);
> + IOVEC_SET(state->iov[1], (void *)&equals, sizeof(equals));
> + state->iov += 2;
> + }
> + IOVEC_SET_STRING(state->iov[0], value);
> + IOVEC_SET(state->iov[1], (void *)&newline, sizeof(newline));
> + state->iov += 2;
> +}
> +
> +static void
> +journalAddInt(struct journalState *state, const char *field, int value)
> +{
> + static const char newline = '\n', equals = '=';
> +
> + char *num;
> +
> + if (state->iov_end - state->iov < 4 || state->bufs == state->bufs_end)
> + return; /* Silently drop */
> +
> + num = virFormatIntDecimal(state->bufs[0], sizeof(state->bufs[0]), value);
> +
> + IOVEC_SET_STRING(state->iov[0], field);
> + IOVEC_SET(state->iov[1], (void *)&equals, sizeof(equals));
> + IOVEC_SET_STRING(state->iov[2], num);
> + IOVEC_SET(state->iov[3], (void *)&newline, sizeof(newline));
> + state->bufs++;
> + state->iov += 4;
> +}
>
> static int journalfd = -1;
>
> @@ -1074,7 +1133,6 @@ virLogOutputToJournald(virLogSource source,
> {
> virCheckFlags(VIR_LOG_STACK_TRACE,);
> int buffd = -1;
> - size_t niov = 0;
> struct msghdr mh;
> struct sockaddr_un sa;
> union {
> @@ -1086,52 +1144,24 @@ virLogOutputToJournald(virLogSource source,
> * be a tmpfs, and one that is available from early boot on
> * and where unprivileged users can create files. */
> char path[] = "/dev/shm/journal.XXXXXX";
> - char priostr[INT_BUFSIZE_BOUND(priority)];
> - char linestr[INT_BUFSIZE_BOUND(linenr)];
> -
> - /* First message takes up to 4 iovecs, and each
> - * other field needs 3, assuming they don't have
> - * newlines in them
> - */
> -# define IOV_SIZE (4 + (5 * 3))
> - struct iovec iov[IOV_SIZE];
> -
> - if (strchr(rawstr, '\n')) {
> - uint64_t nstr;
> - /* If 'str' contains a newline, then we must
> - * encode the string length, since we can't
> - * rely on the newline for the field separator
> - */
> - IOVEC_SET_STRING(iov[niov++], "MESSAGE\n");
> - nstr = htole64(strlen(rawstr));
> - iov[niov].iov_base = (char*)&nstr;
> - iov[niov].iov_len = sizeof(nstr);
> - niov++;
> - } else {
> - IOVEC_SET_STRING(iov[niov++], "MESSAGE=");
> - }
> - IOVEC_SET_STRING(iov[niov++], rawstr);
> - IOVEC_SET_STRING(iov[niov++], "\n");
> -
> - IOVEC_SET_STRING(iov[niov++], "PRIORITY=");
> - IOVEC_SET_INT(iov[niov++], priostr, priority);
> - IOVEC_SET_STRING(iov[niov++], "\n");
> -
> - IOVEC_SET_STRING(iov[niov++], "LIBVIRT_SOURCE=");
> - IOVEC_SET_STRING(iov[niov++], virLogSourceTypeToString(source));
> - IOVEC_SET_STRING(iov[niov++], "\n");
>
> - IOVEC_SET_STRING(iov[niov++], "CODE_FILE=");
> - IOVEC_SET_STRING(iov[niov++], filename);
> - IOVEC_SET_STRING(iov[niov++], "\n");
> +# define NUM_FIELDS 6
> + struct iovec iov[NUM_FIELDS * 5];
> + char iov_bufs[NUM_FIELDS][JOURNAL_BUF_SIZE];
> + struct journalState state;
>
> - IOVEC_SET_STRING(iov[niov++], "CODE_LINE=");
> - IOVEC_SET_INT(iov[niov++], linestr, linenr);
> - IOVEC_SET_STRING(iov[niov++], "\n");
> + state.iov = iov;
> + state.iov_end = iov + ARRAY_CARDINALITY(iov);
> + state.bufs = iov_bufs;
> + state.bufs_end = iov_bufs + ARRAY_CARDINALITY(iov_bufs);
>
> - IOVEC_SET_STRING(iov[niov++], "CODE_FUNC=");
> - IOVEC_SET_STRING(iov[niov++], funcname);
> - IOVEC_SET_STRING(iov[niov++], "\n");
> + journalAddString(&state ,"MESSAGE", rawstr);
> + journalAddInt(&state, "PRIORITY", priority);
> + journalAddString(&state, "LIBVIRT_SOURCE",
> + virLogSourceTypeToString(source));
> + journalAddString(&state, "CODE_FILE", filename);
> + journalAddInt(&state, "CODE_LINE", linenr);
> + journalAddString(&state, "CODE_FUNC", funcname);
>
> memset(&sa, 0, sizeof(sa));
> sa.sun_family = AF_UNIX;
> @@ -1142,7 +1172,7 @@ virLogOutputToJournald(virLogSource source,
> mh.msg_name = &sa;
> mh.msg_namelen = offsetof(struct sockaddr_un, sun_path) + strlen(sa.sun_path);
> mh.msg_iov = iov;
> - mh.msg_iovlen = niov;
> + mh.msg_iovlen = state.iov - iov;
>
> if (sendmsg(journalfd, &mh, MSG_NOSIGNAL) >= 0)
> return;
> @@ -1166,7 +1196,7 @@ virLogOutputToJournald(virLogSource source,
> if (unlink(path) < 0)
> goto cleanup;
>
> - if (writev(buffd, iov, niov) < 0)
> + if (writev(buffd, iov, state.iov - iov) < 0)
> goto cleanup;
>
> mh.msg_iov = NULL;
ACK
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list