On Tuesday 22 March 2016 19:05:24 Richard W.M. Jones wrote:
> This tedious workaround avoids a 0.26 second pause when using sgabios
> (the Serial Graphics Adapter). It's basically a workaround for buggy
> code in sgabios, but much easier than fixing the assembler.
> ---
> src/conn-socket.c | 32 +++++++++++++++++++++++++++++++-
> 1 file changed, 31 insertions(+), 1 deletion(-)
>
> diff --git a/src/conn-socket.c b/src/conn-socket.c
> index 5b6b80e..13cb39b 100644
> --- a/src/conn-socket.c
> +++ b/src/conn-socket.c
> @@ -33,6 +33,8 @@
> #include <assert.h>
> #include <libintl.h>
>
> +#include "ignore-value.h"
> +
> #include "guestfs.h"
> #include "guestfs-internal.h"
>
> @@ -314,6 +316,9 @@ handle_log_message (guestfs_h *g,
> {
> CLEANUP_FREE char *buf = safe_malloc (g, BUFSIZ);
> ssize_t n;
> + const char dsr_request[] = "\033[6n";
> + const char dsr_reply[] = "\033[24;80R";
> + const char dsr_reply_padding[] = "\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b";
>
> /* Carried over from ancient proto.c code. The comment there was:
> *
> @@ -341,7 +346,32 @@ handle_log_message (guestfs_h *g,
> return -1;
> }
>
> - /* It's an actual log message, send it upwards. */
> + /* It's an actual log message. */
> +
> + /* SGABIOS tries to query the "serial console" for its size using the
> + * ISO/IEC 6429 Device Status Report (ESC [ 6 n). If it doesn't
> + * read anything back, then it unfortunately hangs for 0.26 seconds.
> + * Therefore we detect this situation and send back a fake console
> + * size.
> + */
> + if (memmem (buf, n, dsr_request, sizeof dsr_request - 1) != NULL) {
> + /* Ignore any error from this write, as it's just an optimization.
> + * We can't even be sure that console_sock is a socket or that
> + * it's writable.
> + */
> + ignore_value (write (conn->console_sock, dsr_reply,
> + sizeof dsr_reply - 1));
> + /* Additionally, because of a bug in sgabios, it will still pause
> + * unless you write at least 14 bytes, so we have to pad the
> + * reply. We can't pad with NULs since sgabios's input routine
> + * ignores these, so we have to use some other safe padding
> + * characters. Backspace seems innocuous.
> + */
> + ignore_value (write (conn->console_sock, dsr_reply_padding,
> + sizeof dsr_reply_padding - 1));
> + }
> +
> + /* Send it upwards. */
> guestfs_int_log_message_callback (g, buf, n);
>
> return 1;If I read this correctly, the newly added memmem would be executed for every read data from the daemon socket: read_data -> handle_log_message -> memmem Wouldn't be better to limit it in accept_connection if possible? More than the overhead of memmem, I'm worried that this check might misdetect other kind of binary data coming from the daemon (e.g. download APIs) with the sgabios output. -- Pino Toscano
signature.asc
Description: This is a digitally signed message part.
_______________________________________________ Libguestfs mailing list [email protected] https://www.redhat.com/mailman/listinfo/libguestfs
