|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH v3] libxl: reimplement buffer for bootloading and drop data if buffer is full
On Tue, 2011-10-18 at 12:17 +0100, Roger Pau Monne wrote:
> # HG changeset patch
> # User Roger Pau Monne <roger.pau@xxxxxxxxxxxxx>
> # Date 1318936435 -7200
> # Node ID 4a591584e20718b06f3e9dc72897ee2a87f43c3c
> # Parent 0a720316685a73e2d5aee56c1572b9ee8d98ab4e
> libxl: reimplement buffer for bootloading and drop data if buffer is full.
>
> Implement a buffer for the bootloading process that appends data to the end
> until it's full. Drop output from bootloader if a timeout has occurred and
> the buffer is full. Prevents the bootloader from getting stuck when using
> ptys with small buffers.
>
> Signed-off-by: Roger Pau Monne <roger.pau@xxxxxxxxxxxxx>
>
> diff -r 0a720316685a -r 4a591584e207 tools/libxl/libxl_bootloader.c
> --- a/tools/libxl/libxl_bootloader.c Tue Oct 18 11:26:43 2011 +0200
> +++ b/tools/libxl/libxl_bootloader.c Tue Oct 18 13:13:55 2011 +0200
> @@ -21,6 +21,7 @@
>
> #include <sys/stat.h>
> #include <sys/types.h>
> +#include <sys/ioctl.h>
>
> #include "libxl.h"
> #include "libxl_internal.h"
> @@ -28,7 +29,8 @@
> #include "flexarray.h"
>
> #define XENCONSOLED_BUF_SIZE 16
> -#define BOOTLOADER_BUF_SIZE 1024
> +#define BOOTLOADER_BUF_SIZE 4096
> +#define BOOTLOADER_TIMEOUT 1
>
> static char **make_bootloader_args(libxl__gc *gc,
> libxl_domain_build_info *info,
> @@ -165,10 +167,11 @@ static pid_t fork_exec_bootloader(int *m
> */
> static char * bootloader_interact(libxl__gc *gc, int xenconsoled_fd, int
> bootloader_fd, int fifo_fd)
> {
> - int ret;
> + int ret, read_ahead;
>
> size_t nr_out = 0, size_out = 0;
> char *output = NULL;
> + struct timeval wait;
>
> /* input from xenconsole. read on xenconsoled_fd write to bootloader_fd
> */
> int xenconsoled_prod = 0, xenconsoled_cons = 0;
> @@ -181,39 +184,74 @@ static char * bootloader_interact(libxl_
> fd_set wsel, rsel;
> int nfds;
>
> + /* Set timeout to 1s before starting to discard data */
> + wait.tv_sec = BOOTLOADER_TIMEOUT;
> + wait.tv_usec = 0;
> +
> if (xenconsoled_prod == xenconsoled_cons)
> xenconsoled_prod = xenconsoled_cons = 0;
> if (bootloader_prod == bootloader_cons)
> bootloader_prod = bootloader_cons = 0;
> + /* Move buffers around to drop already consumed data */
> + if (xenconsoled_cons > 0) {
> + xenconsoled_prod -= xenconsoled_cons;
> + memmove(xenconsoled_buf, &xenconsoled_buf[xenconsoled_cons],
> + xenconsoled_prod);
> + xenconsoled_cons = 0;
> + }
> + if (bootloader_cons > 0) {
> + bootloader_prod -= bootloader_cons;
> + memmove(bootloader_buf, &bootloader_buf[bootloader_cons],
> + bootloader_prod);
> + bootloader_cons = 0;
> + }
>
> FD_ZERO(&rsel);
> FD_SET(fifo_fd, &rsel);
> nfds = fifo_fd + 1;
> - if (xenconsoled_prod == 0 || (xenconsoled_prod < BOOTLOADER_BUF_SIZE
> && xenconsoled_cons == 0)) {
> + if (xenconsoled_prod < XENCONSOLED_BUF_SIZE) {
> FD_SET(xenconsoled_fd, &rsel);
> nfds = xenconsoled_fd + 1 > nfds ? xenconsoled_fd + 1 : nfds;
> - }
> - if (bootloader_prod == 0 || (bootloader_prod < BOOTLOADER_BUF_SIZE
> && bootloader_cons == 0)) {
> + }
> + if (bootloader_prod < BOOTLOADER_BUF_SIZE) {
> FD_SET(bootloader_fd, &rsel);
> nfds = bootloader_fd + 1 > nfds ? bootloader_fd + 1 : nfds;
> }
>
> FD_ZERO(&wsel);
> - if (bootloader_prod != bootloader_cons) {
> + if (bootloader_prod > 0) {
> FD_SET(xenconsoled_fd, &wsel);
> nfds = xenconsoled_fd + 1 > nfds ? xenconsoled_fd + 1 : nfds;
> }
> - if (xenconsoled_prod != xenconsoled_cons) {
> + if (xenconsoled_prod > 0) {
> FD_SET(bootloader_fd, &wsel);
> nfds = bootloader_fd + 1 > nfds ? bootloader_fd + 1 : nfds;
> }
>
> - ret = select(nfds, &rsel, &wsel, NULL, NULL);
> - if (ret < 0)
> + if (xenconsoled_prod == XENCONSOLED_BUF_SIZE ||
> + bootloader_prod == BOOTLOADER_BUF_SIZE)
> + ret = select(nfds, &rsel, &wsel, NULL, &wait);
> + else
> + ret = select(nfds, &rsel, &wsel, NULL, NULL);
> + if (ret < 0) {
> + if (errno == EINTR)
> + continue;
> goto out_err;
> + }
>
> /* Input from xenconsole, read xenconsoled_fd, write bootloader_fd */
> - if (FD_ISSET(xenconsoled_fd, &rsel)) {
> + if (FD_ISSET(xenconsoled_fd, &rsel) || ret == 0) {
I don't think these tests need to or should be combined. You don't need
to drop data from the buffer if xenconsole_fd is pending since if that
were the case then we must have added it to the select and therefore it
has space. Likewise if we have timed out we don't need to read from the
buffer, indeed we shouldn't because we can't be sure anything will be
available. If there does happen to be data available we will get it next
time.
I'm not sure what using FIONREAD buys us. If we have timed out we might
as well throw the whole buffer away.
IOW for each fd we do:
if (ret == 0 && xenconsoled_prod == XENCONSOLED_BUF_SIZE) {
timed out -> drop the buffer
} else if (FD_ISSET(xenconsoled_fd, &rsel)) {
read into the buffer
}
> + if (xenconsoled_prod == XENCONSOLED_BUF_SIZE) {
> + if (ioctl(xenconsoled_fd, FIONREAD, &read_ahead) < 0)
> + goto out_err;
> + if (read_ahead >= XENCONSOLED_BUF_SIZE)
> + /* The whole buffer will be overwritten */
> + read_ahead = XENCONSOLED_BUF_SIZE;
> + else
> + memmove(xenconsoled_buf, &xenconsoled_buf[read_ahead],
> + XENCONSOLED_BUF_SIZE - read_ahead);
> + xenconsoled_prod -= read_ahead;
> + }
> ret = read(xenconsoled_fd, &xenconsoled_buf[xenconsoled_prod],
> XENCONSOLED_BUF_SIZE - xenconsoled_prod);
> if (ret < 0 && errno != EIO && errno != EAGAIN)
> goto out_err;
> @@ -229,7 +267,18 @@ static char * bootloader_interact(libxl_
> }
>
> /* Input from bootloader, read bootloader_fd, write xenconsoled_fd */
> - if (FD_ISSET(bootloader_fd, &rsel)) {
> + if (FD_ISSET(bootloader_fd, &rsel) || ret == 0) {
> + if (bootloader_prod == BOOTLOADER_BUF_SIZE) {
> + if (ioctl(bootloader_fd, FIONREAD, &read_ahead) < 0)
> + goto out_err;
> + if (read_ahead >= BOOTLOADER_BUF_SIZE)
> + /* The whole buffer will be overwritten */
> + read_ahead = BOOTLOADER_BUF_SIZE;
> + else
> + memmove(bootloader_buf, &bootloader_buf[read_ahead],
> + BOOTLOADER_BUF_SIZE - read_ahead);
> + bootloader_prod -= read_ahead;
> + }
> ret = read(bootloader_fd, &bootloader_buf[bootloader_prod],
> BOOTLOADER_BUF_SIZE - bootloader_prod);
> if (ret < 0 && errno != EIO && errno != EAGAIN)
> goto out_err;
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@xxxxxxxxxxxxxxxxxxx
> http://lists.xensource.com/xen-devel
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |