From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:60671) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1R8quR-0003cR-4O for qemu-devel@nongnu.org; Wed, 28 Sep 2011 05:58:36 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1R8quP-0000Qg-Nf for qemu-devel@nongnu.org; Wed, 28 Sep 2011 05:58:35 -0400 Received: from e28smtp04.in.ibm.com ([122.248.162.4]:49962) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1R8quO-0000QL-NB for qemu-devel@nongnu.org; Wed, 28 Sep 2011 05:58:33 -0400 Received: from d28relay01.in.ibm.com (d28relay01.in.ibm.com [9.184.220.58]) by e28smtp04.in.ibm.com (8.14.4/8.13.1) with ESMTP id p8S9wQ6Q006261 for ; Wed, 28 Sep 2011 15:28:26 +0530 Received: from d28av05.in.ibm.com (d28av05.in.ibm.com [9.184.220.67]) by d28relay01.in.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id p8S9wPgD4206630 for ; Wed, 28 Sep 2011 15:28:26 +0530 Received: from d28av05.in.ibm.com (loopback [127.0.0.1]) by d28av05.in.ibm.com (8.14.4/8.13.1/NCO v10.0 AVout) with ESMTP id p8SFRl2T004691 for ; Thu, 29 Sep 2011 01:27:47 +1000 Message-ID: <4E82EF9C.6010904@linux.vnet.ibm.com> Date: Wed, 28 Sep 2011 17:57:48 +0800 From: "Mars.cao" MIME-Version: 1.0 References: <1317138177-2195-1-git-send-email-chouteau@adacore.com> In-Reply-To: <1317138177-2195-1-git-send-email-chouteau@adacore.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH V2] Add stdio char device on windows List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Fabien Chouteau Cc: pbonzini@redhat.com, qemu-devel@nongnu.org On 09/27/2011 11:42 PM, Fabien Chouteau wrote: > Simple implementation of an stdio char device on Windows. > > Signed-off-by: Fabien Chouteau > --- > qemu-char.c | 199 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++- > 1 files changed, 197 insertions(+), 2 deletions(-) > > diff --git a/qemu-char.c b/qemu-char.c > index 09d2309..46acf1c 100644 > --- a/qemu-char.c > +++ b/qemu-char.c > @@ -538,6 +538,9 @@ int send_all(int fd, const void *_buf, int len1) > } > #endif /* !_WIN32 */ > > +#define STDIO_MAX_CLIENTS 1 > +static int stdio_nb_clients; Why change the default initial value (0) of stdio_nb_clients? Did you mean we should support multiple stdio clients? But I did not found any other stdio backend in the code. > + > #ifndef _WIN32 > > typedef struct { > @@ -545,8 +548,6 @@ typedef struct { > int max_size; > } FDCharDriver; > > -#define STDIO_MAX_CLIENTS 1 > -static int stdio_nb_clients = 0; > > static int fd_chr_write(CharDriverState *chr, const uint8_t *buf, int len) > { > @@ -1451,6 +1452,8 @@ static int qemu_chr_open_pp(QemuOpts *opts, CharDriverState **_chr) > > #else /* _WIN32 */ > > +static CharDriverState *stdio_clients[STDIO_MAX_CLIENTS]; > + > typedef struct { > int max_size; > HANDLE hcom, hrecv, hsend; > @@ -1809,6 +1812,197 @@ static int qemu_chr_open_win_file_out(QemuOpts *opts, CharDriverState **_chr) > > return qemu_chr_open_win_file(fd_out, _chr); > } > + > +static int win_stdio_write(CharDriverState *chr, const uint8_t *buf, int len) > +{ > + HANDLE *hStdOut = GetStdHandle(STD_OUTPUT_HANDLE); > + DWORD dwSize; > + int len1; > + > + len1 = len; > + > + while (len1> 0) { > + if (!WriteFile(hStdOut, buf, len1,&dwSize, NULL)) { > + break; > + } > + buf += dwSize; > + len1 -= dwSize; > + } > + > + return len - len1; > +} > + > +static HANDLE *hStdIn; > + > +static void win_stdio_wait_func(void *opaque) > +{ > + CharDriverState *chr = opaque; > + INPUT_RECORD buf[4]; > + int ret; > + DWORD dwSize; > + int i; > + > + ret = ReadConsoleInput(hStdIn, buf, sizeof(buf)/sizeof(*buf),&dwSize); > + > + if (!ret) { > + /* Avoid error storm */ > + qemu_del_wait_object(hStdIn, NULL, NULL); > + return; > + } > + > + for (i = 0; i< dwSize; i++) { > + KEY_EVENT_RECORD *kev =&buf[i].Event.KeyEvent; > + > + if (buf[i].EventType == KEY_EVENT&& kev->bKeyDown) { > + int j; > + if (kev->uChar.AsciiChar != 0) { > + for (j = 0; j< kev->wRepeatCount; j++) > + if (qemu_chr_be_can_write(chr)) { > + uint8_t c = kev->uChar.AsciiChar; > + qemu_chr_be_write(chr,&c, 1); > + } > + } > + } > + } > +} > + > +static HANDLE hInputReadyEvent; > +static HANDLE hInputDoneEvent; > +static uint8_t win_stdio_buf; > + > +static DWORD WINAPI win_stdio_thread(LPVOID param) > +{ > + int ret; > + DWORD dwSize; > + > + while (1) { > + > + /* Wait for one byte */ > + ret = ReadFile(hStdIn,&win_stdio_buf, 1,&dwSize, NULL); > + > + /* Exit in case of error, continue if nothing read */ > + if (!ret) { > + break; > + } I think a qemu_del_wait_object() should be added before break. > + if (!dwSize) { > + continue; > + } > + > + /* Some terminal emulator returns \r\n for Enter, just pass \n */ > + if (win_stdio_buf == '\r') { > + continue; > + } > + > + /* Signal the main thread and wait until the byte was eaten */ > + if (!SetEvent(hInputReadyEvent)) { > + break; > + } > + if (WaitForSingleObject(hInputDoneEvent, INFINITE) != WAIT_OBJECT_0) { > + break; > + } > + } > + > + qemu_del_wait_object(hInputReadyEvent, NULL, NULL); > + return 0; > +} > + > +static void win_stdio_thread_wait_func(void *opaque) > +{ > + CharDriverState *chr = opaque; > + > + if (qemu_chr_be_can_write(chr)) { > + qemu_chr_be_write(chr,&win_stdio_buf, 1); > + } > + > + SetEvent(hInputDoneEvent); > +} > + > +static void qemu_chr_set_echo_win_stdio(CharDriverState *chr, bool echo) > +{ > + DWORD dwMode = 0; > + > + GetConsoleMode(hStdIn,&dwMode); > + > + if (echo) { > + SetConsoleMode(hStdIn, dwMode | ENABLE_ECHO_INPUT); > + } else { > + SetConsoleMode(hStdIn, dwMode& ~ENABLE_ECHO_INPUT); > + } > +} > + > +static int qemu_chr_open_win_stdio(QemuOpts *opts, > + CharDriverState **_chr) > +{ > + CharDriverState *chr; > + DWORD dwMode; > + int is_console = 0; > + > + hStdIn = GetStdHandle(STD_INPUT_HANDLE); > + if (hStdIn == INVALID_HANDLE_VALUE) { > + fprintf(stderr, "cannot open stdio: invalid handle\n"); > + exit(1); > + } > + > + is_console = GetConsoleMode(hStdIn,&dwMode) != 0; > + > + if (stdio_nb_clients>= STDIO_MAX_CLIENTS > + || ((display_type != DT_NOGRAPHIC)&& (stdio_nb_clients != 0))) { > + return -EIO; > + } > + > + chr = g_malloc0(sizeof(CharDriverState)); > + if (!chr) { > + return -ENOMEM; > + } I have not found any g_free for the CharDriverStart chr. > + > + chr->chr_write = win_stdio_write; > + > + if (stdio_nb_clients == 0) { > + if (is_console) { > + if (qemu_add_wait_object(hStdIn, > + win_stdio_wait_func, chr)) { > + fprintf(stderr, "qemu_add_wait_object: failed\n"); > + } > + } else { > + DWORD dwId; > + HANDLE *hInputThread; > + > + hInputReadyEvent = CreateEvent(NULL, FALSE, FALSE, NULL); > + hInputDoneEvent = CreateEvent(NULL, FALSE, FALSE, NULL); > + hInputThread = CreateThread(NULL, 0, win_stdio_thread, > + chr, 0,&dwId); I do not think it is a good idea to create 3 static global variant for the 2 events and the thread. Creating a new structure to contain the elements may be better. > + > + if ( hInputThread == INVALID_HANDLE_VALUE > + || hInputReadyEvent == INVALID_HANDLE_VALUE > + || hInputDoneEvent == INVALID_HANDLE_VALUE) { > + fprintf(stderr, "cannot create stdio thread or event\n"); > + exit(1); > + } > + if (qemu_add_wait_object(hInputReadyEvent, > + win_stdio_thread_wait_func, chr)) { > + fprintf(stderr, "qemu_add_wait_object: failed\n"); > + } > + } > + } > + > + dwMode |= ENABLE_LINE_INPUT; > + > + stdio_clients[stdio_nb_clients++] = chr; > + if (stdio_nb_clients == 1&& is_console) { > + /* set the terminal in raw mode */ > + /* ENABLE_QUICK_EDIT_MODE | ENABLE_EXTENDED_FLAGS */ > + dwMode |= ENABLE_PROCESSED_INPUT; > + } > + > + SetConsoleMode(hStdIn, dwMode); > + > + chr->chr_set_echo = qemu_chr_set_echo_win_stdio; > + qemu_chr_fe_set_echo(chr, false); > + > + *_chr = chr; > + > + return 0; > +} > #endif /* !_WIN32 */ > > /***********************************************************/ > @@ -2519,6 +2713,7 @@ static const struct { > { .name = "pipe", .open = qemu_chr_open_win_pipe }, > { .name = "console", .open = qemu_chr_open_win_con }, > { .name = "serial", .open = qemu_chr_open_win }, > + { .name = "stdio", .open = qemu_chr_open_win_stdio }, > #else > { .name = "file", .open = qemu_chr_open_file_out }, > { .name = "pipe", .open = qemu_chr_open_pipe }, I am a new rookie in qemu-devel,so forgive my misunderstanding. I will test your patch in future 2 days. Reviewed by: Mars.Cao