* [Qemu-devel] [PATCH] qemu-ga: isa-serial support on Windows
@ 2014-01-05 17:18 Miki Mishael
2014-01-08 18:01 ` Michael Roth
0 siblings, 1 reply; 2+ messages in thread
From: Miki Mishael @ 2014-01-05 17:18 UTC (permalink / raw)
To: qemu-devel
Cc: Yan Vugenfirer, Michael Roth, Adam Litke, Ronen Hod,
Anthony Liguori, Dmitry Fleytman, Miki Mishael
Add support for isa-serial method for qemu-ga on Windows,
Added -p command line parameter for serial port name
specification, e.g. "-p COM15".
Signed-off-by: Miki Mishael <mmishael@redhat.com>
Signed-off-by: Dmitry Fleytman <dfleytma@redhat.com>
---
qga/channel-win32.c | 27 +++++++++++++++++++++++++--
qga/main.c | 14 +++++++++++---
2 files changed, 36 insertions(+), 5 deletions(-)
diff --git a/qga/channel-win32.c b/qga/channel-win32.c
index 8a303f3..fd42460 100644
--- a/qga/channel-win32.c
+++ b/qga/channel-win32.c
@@ -284,15 +284,32 @@ GIOStatus ga_channel_write_all(GAChannel *c, const char *buf, size_t size)
return status;
}
+static void ga_serial_path_correction( gchar* newpath, const gchar *path, size_t maxdestlen)
+{
+ gchar *prefix = "\\\\.\\";
+ g_strlcpy(newpath, prefix, maxdestlen);
+ g_strlcat(newpath, path, maxdestlen);
+}
+
static gboolean ga_channel_open(GAChannel *c, GAChannelMethod method,
const gchar *path)
{
- if (method != GA_CHANNEL_VIRTIO_SERIAL) {
+ COMMTIMEOUTS comTimeOut = {0};
+ gchar newpath[MAXPATHLEN] = {0};
+ comTimeOut.ReadIntervalTimeout = 1;
+
+ if (method != GA_CHANNEL_VIRTIO_SERIAL && method != GA_CHANNEL_ISA_SERIAL) {
g_critical("unsupported communication method");
return false;
}
- c->handle = CreateFile(path, GENERIC_READ | GENERIC_WRITE, 0, NULL,
+ if (method == GA_CHANNEL_ISA_SERIAL){
+ ga_serial_path_correction(newpath, path, sizeof(newpath));
+ }else {
+ g_strlcpy(newpath, path, sizeof(newpath));
+ }
+
+ c->handle = CreateFile(newpath, GENERIC_READ | GENERIC_WRITE, 0, NULL,
OPEN_EXISTING,
FILE_FLAG_NO_BUFFERING | FILE_FLAG_OVERLAPPED, NULL);
if (c->handle == INVALID_HANDLE_VALUE) {
@@ -300,6 +317,12 @@ static gboolean ga_channel_open(GAChannel *c, GAChannelMethod method,
return false;
}
+ if (method == GA_CHANNEL_ISA_SERIAL && !SetCommTimeouts(c->handle,&comTimeOut)) {
+ g_critical("error SetCommTimeouts: %d",GetLastError());
+ CloseHandle(c->handle);
+ return false;
+ }
+
return true;
}
diff --git a/qga/main.c b/qga/main.c
index c58b26a..ad21061 100644
--- a/qga/main.c
+++ b/qga/main.c
@@ -47,9 +47,11 @@
#ifndef _WIN32
#define QGA_VIRTIO_PATH_DEFAULT "/dev/virtio-ports/org.qemu.guest_agent.0"
#define QGA_STATE_RELATIVE_DIR "run"
+#define QGA_SERIAL_PATH_DEFAULT ""
#else
#define QGA_VIRTIO_PATH_DEFAULT "\\\\.\\Global\\org.qemu.guest_agent.0"
#define QGA_STATE_RELATIVE_DIR "qemu-ga"
+#define QGA_SERIAL_PATH_DEFAULT "COM1"
#endif
#ifdef CONFIG_FSFREEZE
#define QGA_FSFREEZE_HOOK_DEFAULT CONFIG_QEMU_CONFDIR "/fsfreeze-hook"
@@ -659,12 +661,18 @@ static gboolean channel_init(GAState *s, const gchar *method, const gchar *path)
}
if (path == NULL) {
- if (strcmp(method, "virtio-serial") != 0) {
+ if (strcmp(method, "virtio-serial") == 0 ) {
+ /* try the default path for the virtio-serial port */
+ path = QGA_VIRTIO_PATH_DEFAULT;
+ }
+ else if (strcmp(method, "isa-serial") == 0){
+ /* try the default path for the serial port - COM1 */
+ path = QGA_SERIAL_PATH_DEFAULT;
+ }
+ else {
g_critical("must specify a path for this channel");
return false;
}
- /* try the default path for the virtio-serial port */
- path = QGA_VIRTIO_PATH_DEFAULT;
}
if (strcmp(method, "virtio-serial") == 0) {
--
1.8.3.1
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [Qemu-devel] [PATCH] qemu-ga: isa-serial support on Windows
2014-01-05 17:18 [Qemu-devel] [PATCH] qemu-ga: isa-serial support on Windows Miki Mishael
@ 2014-01-08 18:01 ` Michael Roth
0 siblings, 0 replies; 2+ messages in thread
From: Michael Roth @ 2014-01-08 18:01 UTC (permalink / raw)
To: Miki Mishael, qemu-devel
Cc: Yan Vugenfirer, Adam Litke, Ronen Hod, Anthony Liguori, Dmitry Fleytman
Quoting Miki Mishael (2014-01-05 11:18:51)
> Add support for isa-serial method for qemu-ga on Windows,
> Added -p command line parameter for serial port name
> specification, e.g. "-p COM15".
>
> Signed-off-by: Miki Mishael <mmishael@redhat.com>
> Signed-off-by: Dmitry Fleytman <dfleytma@redhat.com>
> ---
> qga/channel-win32.c | 27 +++++++++++++++++++++++++--
> qga/main.c | 14 +++++++++++---
> 2 files changed, 36 insertions(+), 5 deletions(-)
>
> diff --git a/qga/channel-win32.c b/qga/channel-win32.c
> index 8a303f3..fd42460 100644
> --- a/qga/channel-win32.c
> +++ b/qga/channel-win32.c
> @@ -284,15 +284,32 @@ GIOStatus ga_channel_write_all(GAChannel *c, const char *buf, size_t size)
> return status;
> }
>
> +static void ga_serial_path_correction( gchar* newpath, const gchar *path, size_t maxdestlen)
> +{
> + gchar *prefix = "\\\\.\\";
> + g_strlcpy(newpath, prefix, maxdestlen);
> + g_strlcat(newpath, path, maxdestlen);
> +}
I think this can be simplified a bit, perhaps:
#define QGA_SERIAL_PATH_PREFIX "\\\\.\\"
static void ga_serial_path_correction(gchar* newpath, const gchar *path)
{
sprintf(newpath, QGA_SERIAL_PATH_PREFIX"%d", path);
}
At that point it probably makes more sense to just drop the func and move
this into ga_channel_open, since it's a bit clearer to understand what the
"correction" entails looking at sprintf's format parameter.
> +
> static gboolean ga_channel_open(GAChannel *c, GAChannelMethod method,
> const gchar *path)
> {
> - if (method != GA_CHANNEL_VIRTIO_SERIAL) {
> + COMMTIMEOUTS comTimeOut = {0};
> + gchar newpath[MAXPATHLEN] = {0};
> + comTimeOut.ReadIntervalTimeout = 1;
> +
> + if (method != GA_CHANNEL_VIRTIO_SERIAL && method != GA_CHANNEL_ISA_SERIAL) {
> g_critical("unsupported communication method");
> return false;
> }
>
> - c->handle = CreateFile(path, GENERIC_READ | GENERIC_WRITE, 0, NULL,
> + if (method == GA_CHANNEL_ISA_SERIAL){
> + ga_serial_path_correction(newpath, path, sizeof(newpath));
> + }else {
> + g_strlcpy(newpath, path, sizeof(newpath));
> + }
> +
> + c->handle = CreateFile(newpath, GENERIC_READ | GENERIC_WRITE, 0, NULL,
> OPEN_EXISTING,
> FILE_FLAG_NO_BUFFERING | FILE_FLAG_OVERLAPPED, NULL);
> if (c->handle == INVALID_HANDLE_VALUE) {
> @@ -300,6 +317,12 @@ static gboolean ga_channel_open(GAChannel *c, GAChannelMethod method,
> return false;
> }
>
> + if (method == GA_CHANNEL_ISA_SERIAL && !SetCommTimeouts(c->handle,&comTimeOut)) {
> + g_critical("error SetCommTimeouts: %d",GetLastError());
Perhaps something a little more human readable, like "error setting timeout for
comm port"
> + CloseHandle(c->handle);
> + return false;
> + }
> +
> return true;
> }
>
> diff --git a/qga/main.c b/qga/main.c
> index c58b26a..ad21061 100644
> --- a/qga/main.c
> +++ b/qga/main.c
> @@ -47,9 +47,11 @@
> #ifndef _WIN32
> #define QGA_VIRTIO_PATH_DEFAULT "/dev/virtio-ports/org.qemu.guest_agent.0"
> #define QGA_STATE_RELATIVE_DIR "run"
> +#define QGA_SERIAL_PATH_DEFAULT ""
We know this will fail on linux, so we should either attempt to define some
kind of default like "/dev/ttyS0", or only attempt the default path on w32
and fail explicitly for linux.
I'm not sure setting a default makes sense for w32 either, serial is already
a non-default configuration, so requiring explicit paths seems reasonable
there, and less likely to confuse users by guessing the wrong port. Though
I guess serial might be a bit more common on w32 than linux, so I don't
really have a strong opinion either way.
> #else
> #define QGA_VIRTIO_PATH_DEFAULT "\\\\.\\Global\\org.qemu.guest_agent.0"
> #define QGA_STATE_RELATIVE_DIR "qemu-ga"
> +#define QGA_SERIAL_PATH_DEFAULT "COM1"
> #endif
> #ifdef CONFIG_FSFREEZE
> #define QGA_FSFREEZE_HOOK_DEFAULT CONFIG_QEMU_CONFDIR "/fsfreeze-hook"
> @@ -659,12 +661,18 @@ static gboolean channel_init(GAState *s, const gchar *method, const gchar *path)
> }
>
> if (path == NULL) {
> - if (strcmp(method, "virtio-serial") != 0) {
> + if (strcmp(method, "virtio-serial") == 0 ) {
> + /* try the default path for the virtio-serial port */
> + path = QGA_VIRTIO_PATH_DEFAULT;
> + }
> + else if (strcmp(method, "isa-serial") == 0){
> + /* try the default path for the serial port - COM1 */
> + path = QGA_SERIAL_PATH_DEFAULT;
> + }
> + else {
In QEMU we generally do:
if (...) {
...
} else if (...) {
...
} else {
...
}
> g_critical("must specify a path for this channel");
> return false;
> }
> - /* try the default path for the virtio-serial port */
> - path = QGA_VIRTIO_PATH_DEFAULT;
> }
>
> if (strcmp(method, "virtio-serial") == 0) {
> --
> 1.8.3.1
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2014-01-08 18:01 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-01-05 17:18 [Qemu-devel] [PATCH] qemu-ga: isa-serial support on Windows Miki Mishael
2014-01-08 18:01 ` Michael Roth
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.