* [Qemu-devel] [PATCH V2] Add stdio char device on windows
@ 2011-09-27 15:42 Fabien Chouteau
2011-09-28 9:57 ` Mars.cao
0 siblings, 1 reply; 4+ messages in thread
From: Fabien Chouteau @ 2011-09-27 15:42 UTC (permalink / raw)
To: qemu-devel; +Cc: pbonzini
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;
+
#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;
+ }
+ 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;
+ }
+
+ 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);
+
+ 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 },
--
1.7.4.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [Qemu-devel] [PATCH V2] Add stdio char device on windows
2011-09-27 15:42 [Qemu-devel] [PATCH V2] Add stdio char device on windows Fabien Chouteau
@ 2011-09-28 9:57 ` Mars.cao
2011-09-28 16:25 ` Fabien Chouteau
0 siblings, 1 reply; 4+ messages in thread
From: Mars.cao @ 2011-09-28 9:57 UTC (permalink / raw)
To: Fabien Chouteau; +Cc: pbonzini, qemu-devel
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>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [Qemu-devel] [PATCH V2] Add stdio char device on windows
2011-09-28 9:57 ` Mars.cao
@ 2011-09-28 16:25 ` Fabien Chouteau
2011-09-29 1:28 ` Mars.cao
0 siblings, 1 reply; 4+ messages in thread
From: Fabien Chouteau @ 2011-09-28 16:25 UTC (permalink / raw)
To: Mars.cao; +Cc: pbonzini, qemu-devel
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!
--
Fabien Chouteau
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [Qemu-devel] [PATCH V2] Add stdio char device on windows
2011-09-28 16:25 ` Fabien Chouteau
@ 2011-09-29 1:28 ` Mars.cao
0 siblings, 0 replies; 4+ messages in thread
From: Mars.cao @ 2011-09-29 1:28 UTC (permalink / raw)
To: Fabien Chouteau; +Cc: pbonzini, qemu-devel
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!
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2011-09-29 1:30 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-09-27 15:42 [Qemu-devel] [PATCH V2] Add stdio char device on windows Fabien Chouteau
2011-09-28 9:57 ` Mars.cao
2011-09-28 16:25 ` Fabien Chouteau
2011-09-29 1:28 ` Mars.cao
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).