|
| From: | Mars.cao |
| Subject: | Re: [Qemu-devel] [PATCH V2] Add stdio char device on windows |
| Date: | Thu, 29 Sep 2011 09:28:16 +0800 |
| User-agent: | Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.2.22) Gecko/20110904 Red Hat/3.1.14-1.el6_1 Thunderbird/3.1.14 |
On 09/29/2011 12:25 AM, Fabien Chouteau wrote:
Sorry for that,I did not notice the qemu_del_wait_object() func after the while(1). :)On 28/09/2011 11:57, Mars.cao wrote:On 09/27/2011 11:42 PM, Fabien Chouteau wrote:Simple implementation of an stdio char device on Windows. Signed-off-by: Fabien Chouteau<address@hidden> --- 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.I just removed the useless "0" initialization of this global variable.+ #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.I don't see why, can you explain?
It is just not necessary in this case,but I think the structure is better for maintain and expand.+ 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.Good catch!+ + 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.I see your point but in this case the structure is just not necessary.
But now in this case it is not wrong. :)
+ + 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.Please wait for v3. Thanks for the review!
Thanks,Waiting for v3!
| [Prev in Thread] | Current Thread | [Next in Thread] |