[V2] Add stdio char device on windows

Simple implementation of an stdio char device on Windows.

Signed-off-by: Fabien Chouteau <chouteau@adacore.com>
---
 qemu-char.c |  199 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 files changed, 197 insertions(+), 2 deletions(-)

On 09/27/2011 11:42 PM, Fabien Chouteau wrote:
> Simple implementation of an stdio char device on Windows.
>
> Signed-off-by: Fabien Chouteau<chouteau@adacore.com>
> ---
>   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 <mars@linux.vnet.ibm.com>

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<chouteau@adacore.com>
>> ---
>>   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?

>> +        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.

>> +
>> +            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!

On 09/29/2011 12:25 AM, Fabien Chouteau wrote:
> 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<chouteau@adacore.com>
>>> ---
>>>    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?
Sorry for that,I did not notice the qemu_del_wait_object() func after 
the while(1).    :)
>>> +        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.
It is just not necessary in this case,but I think the structure is 
better for maintain and expand.
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!