Roger Pau Monne
2011-Oct-18 11:17 UTC
[Xen-devel] [PATCH v3] libxl: reimplement buffer for bootloading and drop data if buffer is full
# HG changeset patch
# User Roger Pau Monne <roger.pau@entel.upc.edu>
# 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@entel.upc.edu>
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) {
+ 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@lists.xensource.com
http://lists.xensource.com/xen-devel
Ian Campbell
2011-Oct-18 13:05 UTC
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@entel.upc.edu> > # 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@entel.upc.edu> > > 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@lists.xensource.com > http://lists.xensource.com/xen-devel_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel