conmon icon indicating copy to clipboard operation
conmon copied to clipboard

Adjustments to container logging to address concerns from #262

Open portante opened this issue 3 years ago • 2 comments

There are two commits in this PR:


Move WRITEV_BUFFER_N_IOV to src/ctr_logging.c

The WRITEV_BUFFER_N_IOV constant is not part of the ctr_logging interface, and is only used internal to src/ctr_logging.c.


Match read(2) buffer size to pipe size.

Address a number of conditions flagged in issue #262.

The general thrust of this commit is to move closer to a one-to-one, read(2) system call to writev(2) system call ratio by creating a memory buffer for reading sized to match the stdio pipes.

The commit also allocates a maximum number of I/O vectors sized to the read buffer size, calculating 2 I/O vectors per line, defaulting to a target line size of 25 bytes (defined as the constant, AVG_LOG_LINE_TARGET).

The commit also now allocates the I/O vectors and read buffer memory using mmap() so that we don't cross page boundaries for I/O operations.

As a result, the commit removes the STDIO_BUF_SIZE constance, using fcntl(F_GETPIPE_SZ) calls to size all buffers for reading from pipes.

The stdpipe_t value is now passed to write_to_remote_consoles() so that we do not have to play games with pre-/post- byte allocations to buffers.

portante avatar May 31 '21 02:05 portante

I opened https://github.com/cri-o/cri-o/pull/5013 to test these changes with the kubernetes e2e and cri-o

haircommander avatar Jun 17 '21 16:06 haircommander

Seems to be some packaging issues: /usr/bin/ld: src/ctr_logging.o: in function writev_buffer_init': ctr_logging.c:(.text+0xd5a): undefined reference to ceilf' /usr/bin/ld: ctr_logging.c:(.text+0xdd9): undefined reference to `ceilf'

Also fmt test is not happy.

rhatdan avatar Jun 18 '21 14:06 rhatdan