On Tue, 2011-10-18 at 14:38 +0100, Roger Pau Monne wrote:
> # HG changeset patch
> # User Roger Pau Monne <roger.pau@xxxxxxxxxxxxx>
> # Date 1318944898 -7200
> # Node ID f7b311b85973f5862c323fed4e89c20b7af139a8
> # Parent 6fd16bcdd3e55bf8cc6e6e90e910f32e37654fd7
> 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 the whole buffer 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 6fd16bcdd3e5 -r f7b311b85973 tools/libxl/libxl_bootloader.c
> --- a/tools/libxl/libxl_bootloader.c Tue Oct 18 15:10:04 2011 +0200
> +++ b/tools/libxl/libxl_bootloader.c Tue Oct 18 15:34:58 2011 +0200
> @@ -28,7 +28,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,
> @@ -169,6 +170,7 @@ static char * bootloader_interact(libxl_
>
> 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 +183,66 @@ 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) {
It's a little less obvious now that this test means "there is data to
consume", a comment might be appropriate.
> 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 (ret == 0 && xenconsoled_prod == XENCONSOLED_BUF_SIZE) {
> + /* Drop the buffer */
> + xenconsoled_prod = 0;
Do you also need to reset cons here? I expect we could prove it was
always zero anyway but it might be more obvious to just reset it.
Otherwise this looks good, thanks.
> + } else if (FD_ISSET(xenconsoled_fd, &rsel)) {
> 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 +258,10 @@ static char * bootloader_interact(libxl_
> }
>
> /* Input from bootloader, read bootloader_fd, write xenconsoled_fd */
> - if (FD_ISSET(bootloader_fd, &rsel)) {
> + if (ret == 0 && bootloader_prod == BOOTLOADER_BUF_SIZE) {
> + /* Drop the buffer */
> + bootloader_prod = 0;
> + } else if (FD_ISSET(bootloader_fd, &rsel)) {
> 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
|
Home