[Qemu-devel] [PATCH 00/16] clean up vl.c code

[Qemu-devel] [PATCH 00/16] clean up vl.c code

[Jes.Sorensen]

From: Jes Sorensen <Jes.Sorensen@redhat.com>

Hi,

I have been working on a set of patches to clean up the vl.c code, by
separating out OS specific code into OS specific files. Basically it
introduces two header files: qemu-os-win32.h and qemu-os-posix.h as
well as os-win32.c and os-posix.c.

I have tried to be as careful as I can to not break non Linux support,
but as I only have a Linux build environment handy, I would appreciate
it if people with other OSes could check that I didn't break anything
for them. In particular I would like to know if win32 still builds.

Thanks,
Jes


Jes Sorensen (16):
  vl.c: Remove double include of netinet/in.h for Solaris
  Create qemu-os-win32.h and move WIN32 specific declarations there
  Introduce os-win32.c and move polling functions from vl.c
  vl.c: Move host_main_loop_wait() to OS specific files.
  Introduce os-posix.c and create os_setup_signal_handling()
  Move win32 early signal handling setup to os_setup_signal_handling()
  Rename os_setup_signal_handling() to os_setup_early_signal_handling()
  Move main signal handler setup to os specificfiles.
  Move find_datadir to OS specific files.
  Introduce OS specific cmdline argument handling and move SMB arg to
    os-posix.c
  Move runas handling from vl.c to OS specific files.
  Move chroot handling to OS specific files.
  Move daemonize handling to OS specific files
  Make os_change_process_uid and os_change_root os-posix.c local
  Move line-buffering setup to OS specific files.
  Move set_proc_name() to OS specific files.

 Makefile.objs   |    2 +
 os-posix.c      |  344 ++++++++++++++++++++++++++++++++++++++
 os-win32.c      |  233 ++++++++++++++++++++++++++
 qemu-os-posix.h |   39 +++++
 qemu-os-win32.h |   52 ++++++
 sysemu.h        |   35 ++--
 vl.c            |  490 ++-----------------------------------------------------
 7 files changed, 703 insertions(+), 492 deletions(-)
 create mode 100644 os-posix.c
 create mode 100644 os-win32.c
 create mode 100644 qemu-os-posix.h
 create mode 100644 qemu-os-win32.h

[Qemu-devel] [PATCH 01/16] vl.c: Remove double include of netinet/in.h for Solaris

[Jes.Sorensen]

From: Jes Sorensen <Jes.Sorensen@redhat.com>

vl.c: netinet/in.h is already included once above for the generic
non win32 code.

Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com>
---
 vl.c |    1 -
 1 files changed, 0 insertions(+), 1 deletions(-)

diff --git a/vl.c b/vl.c
index 417554f..7c4298a 100644
--- a/vl.c
+++ b/vl.c
@@ -70,7 +70,6 @@
 #include <sys/ethernet.h>
 #include <sys/sockio.h>
 #include <netinet/arp.h>
-#include <netinet/in.h>
 #include <netinet/in_systm.h>
 #include <netinet/ip.h>
 #include <netinet/ip_icmp.h> // must come after ip.h
-- 
1.6.5.2

[Qemu-devel] [PATCH 02/16] Create qemu-os-win32.h and move WIN32 specific declarations there

[Jes.Sorensen]

From: Jes Sorensen <Jes.Sorensen@redhat.com>

Create qemu-os-win32.h for WIN32 specific declarations. Move polling
handling declaration into this file from sysemu.h

Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com>
---
 qemu-os-win32.h |   43 +++++++++++++++++++++++++++++++++++++++++++
 sysemu.h        |   17 +----------------
 2 files changed, 44 insertions(+), 16 deletions(-)
 create mode 100644 qemu-os-win32.h

diff --git a/qemu-os-win32.h b/qemu-os-win32.h
new file mode 100644
index 0000000..be108ad
--- /dev/null
+++ b/qemu-os-win32.h
@@ -0,0 +1,43 @@
+/*
+ * win32 specific declarations
+ *
+ * Copyright (c) 2003-2008 Fabrice Bellard
+ * Copyright (c) 2010 Jes Sorensen <Jes.Sorensen@redhat.com>
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+#ifndef QEMU_OS_WIN32_H
+#define QEMU_OS_WIN32_H
+
+/* Polling handling */
+
+/* return TRUE if no sleep should be done afterwards */
+typedef int PollingFunc(void *opaque);
+
+int qemu_add_polling_cb(PollingFunc *func, void *opaque);
+void qemu_del_polling_cb(PollingFunc *func, void *opaque);
+
+/* Wait objects handling */
+typedef void WaitObjectFunc(void *opaque);
+
+int qemu_add_wait_object(HANDLE handle, WaitObjectFunc *func, void *opaque);
+void qemu_del_wait_object(HANDLE handle, WaitObjectFunc *func, void *opaque);
+
+#endif
diff --git a/sysemu.h b/sysemu.h
index 879446a..13fc9a9 100644
--- a/sysemu.h
+++ b/sysemu.h
@@ -9,6 +9,7 @@
 
 #ifdef _WIN32
 #include <windows.h>
+#include "qemu-os-win32.h"
 #endif
 
 /* vl.c */
@@ -71,22 +72,6 @@ int qemu_savevm_state_complete(Monitor *mon, QEMUFile *f);
 void qemu_savevm_state_cancel(Monitor *mon, QEMUFile *f);
 int qemu_loadvm_state(QEMUFile *f);
 
-#ifdef _WIN32
-/* Polling handling */
-
-/* return TRUE if no sleep should be done afterwards */
-typedef int PollingFunc(void *opaque);
-
-int qemu_add_polling_cb(PollingFunc *func, void *opaque);
-void qemu_del_polling_cb(PollingFunc *func, void *opaque);
-
-/* Wait objects handling */
-typedef void WaitObjectFunc(void *opaque);
-
-int qemu_add_wait_object(HANDLE handle, WaitObjectFunc *func, void *opaque);
-void qemu_del_wait_object(HANDLE handle, WaitObjectFunc *func, void *opaque);
-#endif
-
 /* SLIRP */
 void do_info_slirp(Monitor *mon);
 
-- 
1.6.5.2

[Qemu-devel] [PATCH 03/16] Introduce os-win32.c and move polling functions from vl.c

[Jes.Sorensen]

From: Jes Sorensen <Jes.Sorensen@redhat.com>

This introduces os-win32.c. It is meant to carry win32 specific
functions thata are not relevant for all of QEMU as well as win32
versions of various pieces like signal handling etc.

Move win32 polling handler helper functions from vl.c to os-win32.c

Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com>
---
 Makefile.objs |    1 +
 os-win32.c    |  111 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 vl.c          |   80 -----------------------------------------
 3 files changed, 112 insertions(+), 80 deletions(-)
 create mode 100644 os-win32.c

diff --git a/Makefile.objs b/Makefile.objs
index 9796dcb..58fdb03 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -144,6 +144,7 @@ hw-obj-$(CONFIG_ECC) += ecc.o
 hw-obj-$(CONFIG_NAND) += nand.o
 hw-obj-$(CONFIG_PFLASH_CFI01) += pflash_cfi01.o
 hw-obj-$(CONFIG_PFLASH_CFI02) += pflash_cfi02.o
+hw-obj-$(CONFIG_WIN32) += os-win32.o
 
 hw-obj-$(CONFIG_M48T59) += m48t59.o
 hw-obj-$(CONFIG_ESCC) += escc.o
diff --git a/os-win32.c b/os-win32.c
new file mode 100644
index 0000000..5a464cc
--- /dev/null
+++ b/os-win32.c
@@ -0,0 +1,111 @@
+/*
+ * os-win32.c
+ *
+ * Copyright (c) 2003-2008 Fabrice Bellard
+ * Copyright (c) 2010 Red Hat, Inc.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+#include <windows.h>
+#include <unistd.h>
+#include <fcntl.h>
+#include <signal.h>
+#include <time.h>
+#include <errno.h>
+#include <sys/time.h>
+#include "config-host.h"
+#include "sysemu.h"
+
+/***********************************************************/
+/* Polling handling */
+
+typedef struct PollingEntry {
+    PollingFunc *func;
+    void *opaque;
+    struct PollingEntry *next;
+} PollingEntry;
+
+static PollingEntry *first_polling_entry;
+
+int qemu_add_polling_cb(PollingFunc *func, void *opaque)
+{
+    PollingEntry **ppe, *pe;
+    pe = qemu_mallocz(sizeof(PollingEntry));
+    pe->func = func;
+    pe->opaque = opaque;
+    for(ppe = &first_polling_entry; *ppe != NULL; ppe = &(*ppe)->next);
+    *ppe = pe;
+    return 0;
+}
+
+void qemu_del_polling_cb(PollingFunc *func, void *opaque)
+{
+    PollingEntry **ppe, *pe;
+    for(ppe = &first_polling_entry; *ppe != NULL; ppe = &(*ppe)->next) {
+        pe = *ppe;
+        if (pe->func == func && pe->opaque == opaque) {
+            *ppe = pe->next;
+            qemu_free(pe);
+            break;
+        }
+    }
+}
+
+/***********************************************************/
+/* Wait objects support */
+typedef struct WaitObjects {
+    int num;
+    HANDLE events[MAXIMUM_WAIT_OBJECTS + 1];
+    WaitObjectFunc *func[MAXIMUM_WAIT_OBJECTS + 1];
+    void *opaque[MAXIMUM_WAIT_OBJECTS + 1];
+} WaitObjects;
+
+static WaitObjects wait_objects = {0};
+
+int qemu_add_wait_object(HANDLE handle, WaitObjectFunc *func, void *opaque)
+{
+    WaitObjects *w = &wait_objects;
+
+    if (w->num >= MAXIMUM_WAIT_OBJECTS)
+        return -1;
+    w->events[w->num] = handle;
+    w->func[w->num] = func;
+    w->opaque[w->num] = opaque;
+    w->num++;
+    return 0;
+}
+
+void qemu_del_wait_object(HANDLE handle, WaitObjectFunc *func, void *opaque)
+{
+    int i, found;
+    WaitObjects *w = &wait_objects;
+
+    found = 0;
+    for (i = 0; i < w->num; i++) {
+        if (w->events[i] == handle)
+            found = 1;
+        if (found) {
+            w->events[i] = w->events[i + 1];
+            w->func[i] = w->func[i + 1];
+            w->opaque[i] = w->opaque[i + 1];
+        }
+    }
+    if (found)
+        w->num--;
+}
diff --git a/vl.c b/vl.c
index 7c4298a..afbb26c 100644
--- a/vl.c
+++ b/vl.c
@@ -1497,86 +1497,6 @@ int qemu_set_fd_handler(int fd,
     return qemu_set_fd_handler2(fd, NULL, fd_read, fd_write, opaque);
 }
 
-#ifdef _WIN32
-/***********************************************************/
-/* Polling handling */
-
-typedef struct PollingEntry {
-    PollingFunc *func;
-    void *opaque;
-    struct PollingEntry *next;
-} PollingEntry;
-
-static PollingEntry *first_polling_entry;
-
-int qemu_add_polling_cb(PollingFunc *func, void *opaque)
-{
-    PollingEntry **ppe, *pe;
-    pe = qemu_mallocz(sizeof(PollingEntry));
-    pe->func = func;
-    pe->opaque = opaque;
-    for(ppe = &first_polling_entry; *ppe != NULL; ppe = &(*ppe)->next);
-    *ppe = pe;
-    return 0;
-}
-
-void qemu_del_polling_cb(PollingFunc *func, void *opaque)
-{
-    PollingEntry **ppe, *pe;
-    for(ppe = &first_polling_entry; *ppe != NULL; ppe = &(*ppe)->next) {
-        pe = *ppe;
-        if (pe->func == func && pe->opaque == opaque) {
-            *ppe = pe->next;
-            qemu_free(pe);
-            break;
-        }
-    }
-}
-
-/***********************************************************/
-/* Wait objects support */
-typedef struct WaitObjects {
-    int num;
-    HANDLE events[MAXIMUM_WAIT_OBJECTS + 1];
-    WaitObjectFunc *func[MAXIMUM_WAIT_OBJECTS + 1];
-    void *opaque[MAXIMUM_WAIT_OBJECTS + 1];
-} WaitObjects;
-
-static WaitObjects wait_objects = {0};
-
-int qemu_add_wait_object(HANDLE handle, WaitObjectFunc *func, void *opaque)
-{
-    WaitObjects *w = &wait_objects;
-
-    if (w->num >= MAXIMUM_WAIT_OBJECTS)
-        return -1;
-    w->events[w->num] = handle;
-    w->func[w->num] = func;
-    w->opaque[w->num] = opaque;
-    w->num++;
-    return 0;
-}
-
-void qemu_del_wait_object(HANDLE handle, WaitObjectFunc *func, void *opaque)
-{
-    int i, found;
-    WaitObjects *w = &wait_objects;
-
-    found = 0;
-    for (i = 0; i < w->num; i++) {
-        if (w->events[i] == handle)
-            found = 1;
-        if (found) {
-            w->events[i] = w->events[i + 1];
-            w->func[i] = w->func[i + 1];
-            w->opaque[i] = w->opaque[i + 1];
-        }
-    }
-    if (found)
-        w->num--;
-}
-#endif
-
 /***********************************************************/
 /* machine registration */
 
-- 
1.6.5.2

[Qemu-devel] [PATCH 04/16] vl.c: Move host_main_loop_wait() to OS specific files.

[Jes.Sorensen]

From: Jes Sorensen <Jes.Sorensen@redhat.com>

Move host_main_loop_wait() to OS specific files. Create
qemu-os-posix.h and provide empty inline for the POSIX case.

Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com>
---
 os-win32.c      |   43 +++++++++++++++++++++++++++++++++++++++++++
 qemu-os-posix.h |   33 +++++++++++++++++++++++++++++++++
 qemu-os-win32.h |    1 +
 sysemu.h        |    4 ++++
 vl.c            |   52 +---------------------------------------------------
 5 files changed, 82 insertions(+), 51 deletions(-)
 create mode 100644 qemu-os-posix.h

diff --git a/os-win32.c b/os-win32.c
index 5a464cc..1f7e28b 100644
--- a/os-win32.c
+++ b/os-win32.c
@@ -109,3 +109,46 @@ void qemu_del_wait_object(HANDLE handle, WaitObjectFunc *func, void *opaque)
     if (found)
         w->num--;
 }
+
+void os_host_main_loop_wait(int *timeout)
+{
+    int ret, ret2, i;
+    PollingEntry *pe;
+
+    /* XXX: need to suppress polling by better using win32 events */
+    ret = 0;
+    for(pe = first_polling_entry; pe != NULL; pe = pe->next) {
+        ret |= pe->func(pe->opaque);
+    }
+    if (ret == 0) {
+        int err;
+        WaitObjects *w = &wait_objects;
+
+        ret = WaitForMultipleObjects(w->num, w->events, FALSE, *timeout);
+        if (WAIT_OBJECT_0 + 0 <= ret && ret <= WAIT_OBJECT_0 + w->num - 1) {
+            if (w->func[ret - WAIT_OBJECT_0])
+                w->func[ret - WAIT_OBJECT_0](w->opaque[ret - WAIT_OBJECT_0]);
+
+            /* Check for additional signaled events */
+            for(i = (ret - WAIT_OBJECT_0 + 1); i < w->num; i++) {
+
+                /* Check if event is signaled */
+                ret2 = WaitForSingleObject(w->events[i], 0);
+                if(ret2 == WAIT_OBJECT_0) {
+                    if (w->func[i])
+                        w->func[i](w->opaque[i]);
+                } else if (ret2 == WAIT_TIMEOUT) {
+                } else {
+                    err = GetLastError();
+                    fprintf(stderr, "WaitForSingleObject error %d %d\n", i, err);
+                }
+            }
+        } else if (ret == WAIT_TIMEOUT) {
+        } else {
+            err = GetLastError();
+            fprintf(stderr, "WaitForMultipleObjects error %d %d\n", ret, err);
+        }
+    }
+
+    *timeout = 0;
+}
diff --git a/qemu-os-posix.h b/qemu-os-posix.h
new file mode 100644
index 0000000..96d1036
--- /dev/null
+++ b/qemu-os-posix.h
@@ -0,0 +1,33 @@
+/*
+ * posix specific declarations
+ *
+ * Copyright (c) 2003-2008 Fabrice Bellard
+ * Copyright (c) 2010 Jes Sorensen <Jes.Sorensen@redhat.com>
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+#ifndef QEMU_OS_POSIX_H
+#define QEMU_OS_POSIX_H
+
+static inline void os_host_main_loop_wait(int *timeout)
+{
+}
+
+#endif
diff --git a/qemu-os-win32.h b/qemu-os-win32.h
index be108ad..4d1cac8 100644
--- a/qemu-os-win32.h
+++ b/qemu-os-win32.h
@@ -40,4 +40,5 @@ typedef void WaitObjectFunc(void *opaque);
 int qemu_add_wait_object(HANDLE handle, WaitObjectFunc *func, void *opaque);
 void qemu_del_wait_object(HANDLE handle, WaitObjectFunc *func, void *opaque);
 
+void os_host_main_loop_wait(int *timeout);
 #endif
diff --git a/sysemu.h b/sysemu.h
index 13fc9a9..5e4feae 100644
--- a/sysemu.h
+++ b/sysemu.h
@@ -12,6 +12,10 @@
 #include "qemu-os-win32.h"
 #endif
 
+#ifdef CONFIG_POSIX
+#include "qemu-os-posix.h"
+#endif
+
 /* vl.c */
 extern const char *bios_name;
 
diff --git a/vl.c b/vl.c
index afbb26c..c655582 100644
--- a/vl.c
+++ b/vl.c
@@ -1722,56 +1722,6 @@ void qemu_system_powerdown_request(void)
     qemu_notify_event();
 }
 
-#ifdef _WIN32
-static void host_main_loop_wait(int *timeout)
-{
-    int ret, ret2, i;
-    PollingEntry *pe;
-
-
-    /* XXX: need to suppress polling by better using win32 events */
-    ret = 0;
-    for(pe = first_polling_entry; pe != NULL; pe = pe->next) {
-        ret |= pe->func(pe->opaque);
-    }
-    if (ret == 0) {
-        int err;
-        WaitObjects *w = &wait_objects;
-
-        ret = WaitForMultipleObjects(w->num, w->events, FALSE, *timeout);
-        if (WAIT_OBJECT_0 + 0 <= ret && ret <= WAIT_OBJECT_0 + w->num - 1) {
-            if (w->func[ret - WAIT_OBJECT_0])
-                w->func[ret - WAIT_OBJECT_0](w->opaque[ret - WAIT_OBJECT_0]);
-
-            /* Check for additional signaled events */
-            for(i = (ret - WAIT_OBJECT_0 + 1); i < w->num; i++) {
-
-                /* Check if event is signaled */
-                ret2 = WaitForSingleObject(w->events[i], 0);
-                if(ret2 == WAIT_OBJECT_0) {
-                    if (w->func[i])
-                        w->func[i](w->opaque[i]);
-                } else if (ret2 == WAIT_TIMEOUT) {
-                } else {
-                    err = GetLastError();
-                    fprintf(stderr, "WaitForSingleObject error %d %d\n", i, err);
-                }
-            }
-        } else if (ret == WAIT_TIMEOUT) {
-        } else {
-            err = GetLastError();
-            fprintf(stderr, "WaitForMultipleObjects error %d %d\n", ret, err);
-        }
-    }
-
-    *timeout = 0;
-}
-#else
-static void host_main_loop_wait(int *timeout)
-{
-}
-#endif
-
 void main_loop_wait(int nonblocking)
 {
     IOHandlerRecord *ioh;
@@ -1787,7 +1737,7 @@ void main_loop_wait(int nonblocking)
         qemu_bh_update_timeout(&timeout);
     }
 
-    host_main_loop_wait(&timeout);
+    os_host_main_loop_wait(&timeout);
 
     /* poll any events */
     /* XXX: separate device handlers from system ones */
-- 
1.6.5.2

[Qemu-devel] [PATCH 05/16] Introduce os-posix.c and create os_setup_signal_handling()

[Jes.Sorensen]

From: Jes Sorensen <Jes.Sorensen@redhat.com>

Introcuce os-posix.c and move posix specific signal handling
there.

Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com>
---
 Makefile.objs |    1 +
 os-posix.c    |   41 +++++++++++++++++++++++++++++++++++++++++
 sysemu.h      |    3 +++
 vl.c          |    8 +-------
 4 files changed, 46 insertions(+), 7 deletions(-)
 create mode 100644 os-posix.c

diff --git a/Makefile.objs b/Makefile.objs
index 58fdb03..2d94677 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -145,6 +145,7 @@ hw-obj-$(CONFIG_NAND) += nand.o
 hw-obj-$(CONFIG_PFLASH_CFI01) += pflash_cfi01.o
 hw-obj-$(CONFIG_PFLASH_CFI02) += pflash_cfi02.o
 hw-obj-$(CONFIG_WIN32) += os-win32.o
+hw-obj-$(CONFIG_POSIX) += os-posix.o
 
 hw-obj-$(CONFIG_M48T59) += m48t59.o
 hw-obj-$(CONFIG_ESCC) += escc.o
diff --git a/os-posix.c b/os-posix.c
new file mode 100644
index 0000000..914a4d1
--- /dev/null
+++ b/os-posix.c
@@ -0,0 +1,41 @@
+/*
+ * os-posix.c
+ *
+ * Copyright (c) 2003-2008 Fabrice Bellard
+ * Copyright (c) 2010 Red Hat, Inc.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+#include <unistd.h>
+#include <fcntl.h>
+#include <signal.h>
+
+/* Needed early for CONFIG_BSD etc. */
+#include "config-host.h"
+#include "sysemu.h"
+
+void os_setup_signal_handling(void)
+{
+    struct sigaction act;
+    sigfillset(&act.sa_mask);
+    act.sa_flags = 0;
+    act.sa_handler = SIG_IGN;
+    sigaction(SIGPIPE, &act, NULL);
+}
diff --git a/sysemu.h b/sysemu.h
index 5e4feae..fc438c5 100644
--- a/sysemu.h
+++ b/sysemu.h
@@ -79,6 +79,9 @@ int qemu_loadvm_state(QEMUFile *f);
 /* SLIRP */
 void do_info_slirp(Monitor *mon);
 
+/* OS specific functions */
+void os_setup_signal_handling(void);
+
 typedef enum DisplayType
 {
     DT_DEFAULT,
diff --git a/vl.c b/vl.c
index c655582..7a46fee 100644
--- a/vl.c
+++ b/vl.c
@@ -2460,13 +2460,7 @@ int main(int argc, char **argv, char **envp)
 
     QLIST_INIT (&vm_change_state_head);
 #ifndef _WIN32
-    {
-        struct sigaction act;
-        sigfillset(&act.sa_mask);
-        act.sa_flags = 0;
-        act.sa_handler = SIG_IGN;
-        sigaction(SIGPIPE, &act, NULL);
-    }
+    os_setup_signal_handling();
 #else
     SetConsoleCtrlHandler(qemu_ctrl_handler, TRUE);
     /* Note: cpu_interrupt() is currently not SMP safe, so we force
-- 
1.6.5.2

Re: [Qemu-devel] [PATCH 05/16] Introduce os-posix.c and create os_setup_signal_handling()

[Richard Henderson]

On 06/03/2010 09:48 AM, Jes.Sorensen@redhat.com wrote:
> --- a/sysemu.h
> +++ b/sysemu.h
> @@ -79,6 +79,9 @@ int qemu_loadvm_state(QEMUFile *f);
>  /* SLIRP */
>  void do_info_slirp(Monitor *mon);
>  
> +/* OS specific functions */
> +void os_setup_signal_handling(void);
> +

Can this go in your qemu-os-posix.h?


r~

Re: [Qemu-devel] [PATCH 05/16] Introduce os-posix.c and create os_setup_signal_handling()

[Jes Sorensen]

On 06/03/10 22:50, Richard Henderson wrote:
> On 06/03/2010 09:48 AM, Jes.Sorensen@redhat.com wrote:
>> --- a/sysemu.h
>> +++ b/sysemu.h
>> @@ -79,6 +79,9 @@ int qemu_loadvm_state(QEMUFile *f);
>>  /* SLIRP */
>>  void do_info_slirp(Monitor *mon);
>>  
>> +/* OS specific functions */
>> +void os_setup_signal_handling(void);
>> +
> 
> Can this go in your qemu-os-posix.h?

Seems reasonable, must be a leftover from earlier.

Cheers,
Jes

[Qemu-devel] [PATCH 06/16] Move win32 early signal handling setup to os_setup_signal_handling()

[Jes.Sorensen]

From: Jes Sorensen <Jes.Sorensen@redhat.com>

Move win32 early signal handling setup to os_setup_signal_handling()

Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com>
---
 os-win32.c |   29 +++++++++++++++++++++++++++++
 vl.c       |   30 ------------------------------
 2 files changed, 29 insertions(+), 30 deletions(-)

diff --git a/os-win32.c b/os-win32.c
index 1f7e28b..dfa90bc 100644
--- a/os-win32.c
+++ b/os-win32.c
@@ -152,3 +152,32 @@ void os_host_main_loop_wait(int *timeout)
 
     *timeout = 0;
 }
+
+static BOOL WINAPI qemu_ctrl_handler(DWORD type)
+{
+    exit(STATUS_CONTROL_C_EXIT);
+    return TRUE;
+}
+
+void os_setup_signal_handling(void)
+{
+    /* Note: cpu_interrupt() is currently not SMP safe, so we force
+       QEMU to run on a single CPU */
+    HANDLE h;
+    DWORD mask, smask;
+    int i;
+
+    SetConsoleCtrlHandler(qemu_ctrl_handler, TRUE);
+
+    h = GetCurrentProcess();
+    if (GetProcessAffinityMask(h, &mask, &smask)) {
+        for(i = 0; i < 32; i++) {
+            if (mask & (1 << i))
+                break;
+        }
+        if (i != 32) {
+            mask = 1 << i;
+            SetProcessAffinityMask(h, mask);
+        }
+    }
+}
diff --git a/vl.c b/vl.c
index 7a46fee..f43456a 100644
--- a/vl.c
+++ b/vl.c
@@ -1986,14 +1986,6 @@ static int balloon_parse(const char *arg)
     return -1;
 }
 
-#ifdef _WIN32
-static BOOL WINAPI qemu_ctrl_handler(DWORD type)
-{
-    exit(STATUS_CONTROL_C_EXIT);
-    return TRUE;
-}
-#endif
-
 #ifndef _WIN32
 
 static void termsig_handler(int signal)
@@ -2459,29 +2451,7 @@ int main(int argc, char **argv, char **envp)
     qemu_cache_utils_init(envp);
 
     QLIST_INIT (&vm_change_state_head);
-#ifndef _WIN32
     os_setup_signal_handling();
-#else
-    SetConsoleCtrlHandler(qemu_ctrl_handler, TRUE);
-    /* Note: cpu_interrupt() is currently not SMP safe, so we force
-       QEMU to run on a single CPU */
-    {
-        HANDLE h;
-        DWORD mask, smask;
-        int i;
-        h = GetCurrentProcess();
-        if (GetProcessAffinityMask(h, &mask, &smask)) {
-            for(i = 0; i < 32; i++) {
-                if (mask & (1 << i))
-                    break;
-            }
-            if (i != 32) {
-                mask = 1 << i;
-                SetProcessAffinityMask(h, mask);
-            }
-        }
-    }
-#endif
 
     module_call_init(MODULE_INIT_MACHINE);
     machine = find_default_machine();
-- 
1.6.5.2

[Qemu-devel] [PATCH 07/16] Rename os_setup_signal_handling() to os_setup_early_signal_handling()

[Jes.Sorensen]

From: Jes Sorensen <Jes.Sorensen@redhat.com>

Rename os_setup_signal_handling() to os_setup_early_signal_handling()

Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com>
---
 os-posix.c |    2 +-
 os-win32.c |    2 +-
 sysemu.h   |    2 +-
 vl.c       |    2 +-
 4 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/os-posix.c b/os-posix.c
index 914a4d1..948f662 100644
--- a/os-posix.c
+++ b/os-posix.c
@@ -31,7 +31,7 @@
 #include "config-host.h"
 #include "sysemu.h"
 
-void os_setup_signal_handling(void)
+void os_setup_early_signal_handling(void)
 {
     struct sigaction act;
     sigfillset(&act.sa_mask);
diff --git a/os-win32.c b/os-win32.c
index dfa90bc..a936f7a 100644
--- a/os-win32.c
+++ b/os-win32.c
@@ -159,7 +159,7 @@ static BOOL WINAPI qemu_ctrl_handler(DWORD type)
     return TRUE;
 }
 
-void os_setup_signal_handling(void)
+void os_setup_early_signal_handling(void)
 {
     /* Note: cpu_interrupt() is currently not SMP safe, so we force
        QEMU to run on a single CPU */
diff --git a/sysemu.h b/sysemu.h
index fc438c5..79ffd9f 100644
--- a/sysemu.h
+++ b/sysemu.h
@@ -80,7 +80,7 @@ int qemu_loadvm_state(QEMUFile *f);
 void do_info_slirp(Monitor *mon);
 
 /* OS specific functions */
-void os_setup_signal_handling(void);
+void os_setup_early_signal_handling(void);
 
 typedef enum DisplayType
 {
diff --git a/vl.c b/vl.c
index f43456a..372f931 100644
--- a/vl.c
+++ b/vl.c
@@ -2451,7 +2451,7 @@ int main(int argc, char **argv, char **envp)
     qemu_cache_utils_init(envp);
 
     QLIST_INIT (&vm_change_state_head);
-    os_setup_signal_handling();
+    os_setup_early_signal_handling();
 
     module_call_init(MODULE_INIT_MACHINE);
     machine = find_default_machine();
-- 
1.6.5.2

[Qemu-devel] [PATCH 08/16] Move main signal handler setup to os specificfiles.

[Jes.Sorensen]

From: Jes Sorensen <Jes.Sorensen@redhat.com>

Move main signal handler setup to os specific files.

Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com>
---
 os-posix.c      |   27 +++++++++++++++++++++++++++
 qemu-os-posix.h |    2 ++
 qemu-os-win32.h |    3 +++
 vl.c            |   33 +--------------------------------
 4 files changed, 33 insertions(+), 32 deletions(-)

diff --git a/os-posix.c b/os-posix.c
index 948f662..01dbec2 100644
--- a/os-posix.c
+++ b/os-posix.c
@@ -26,6 +26,8 @@
 #include <unistd.h>
 #include <fcntl.h>
 #include <signal.h>
+#include <sys/types.h>
+#include <sys/wait.h>
 
 /* Needed early for CONFIG_BSD etc. */
 #include "config-host.h"
@@ -39,3 +41,28 @@ void os_setup_early_signal_handling(void)
     act.sa_handler = SIG_IGN;
     sigaction(SIGPIPE, &act, NULL);
 }
+
+static void termsig_handler(int signal)
+{
+    qemu_system_shutdown_request();
+}
+
+static void sigchld_handler(int signal)
+{
+    waitpid(-1, NULL, WNOHANG);
+}
+
+void os_setup_signal_handling(void)
+{
+    struct sigaction act;
+
+    memset(&act, 0, sizeof(act));
+    act.sa_handler = termsig_handler;
+    sigaction(SIGINT,  &act, NULL);
+    sigaction(SIGHUP,  &act, NULL);
+    sigaction(SIGTERM, &act, NULL);
+
+    act.sa_handler = sigchld_handler;
+    act.sa_flags = SA_NOCLDSTOP;
+    sigaction(SIGCHLD, &act, NULL);
+}
diff --git a/qemu-os-posix.h b/qemu-os-posix.h
index 96d1036..ff5adb1 100644
--- a/qemu-os-posix.h
+++ b/qemu-os-posix.h
@@ -30,4 +30,6 @@ static inline void os_host_main_loop_wait(int *timeout)
 {
 }
 
+void os_setup_signal_handling(void);
+
 #endif
diff --git a/qemu-os-win32.h b/qemu-os-win32.h
index 4d1cac8..4343c6d 100644
--- a/qemu-os-win32.h
+++ b/qemu-os-win32.h
@@ -41,4 +41,7 @@ int qemu_add_wait_object(HANDLE handle, WaitObjectFunc *func, void *opaque);
 void qemu_del_wait_object(HANDLE handle, WaitObjectFunc *func, void *opaque);
 
 void os_host_main_loop_wait(int *timeout);
+
+static inline void os_setup_signal_handling(void) {};
+
 #endif
diff --git a/vl.c b/vl.c
index 372f931..fc5e8d8 100644
--- a/vl.c
+++ b/vl.c
@@ -1986,35 +1986,6 @@ static int balloon_parse(const char *arg)
     return -1;
 }
 
-#ifndef _WIN32
-
-static void termsig_handler(int signal)
-{
-    qemu_system_shutdown_request();
-}
-
-static void sigchld_handler(int signal)
-{
-    waitpid(-1, NULL, WNOHANG);
-}
-
-static void sighandler_setup(void)
-{
-    struct sigaction act;
-
-    memset(&act, 0, sizeof(act));
-    act.sa_handler = termsig_handler;
-    sigaction(SIGINT,  &act, NULL);
-    sigaction(SIGHUP,  &act, NULL);
-    sigaction(SIGTERM, &act, NULL);
-
-    act.sa_handler = sigchld_handler;
-    act.sa_flags = SA_NOCLDSTOP;
-    sigaction(SIGCHLD, &act, NULL);
-}
-
-#endif
-
 #ifdef _WIN32
 /* Look for support files in the same directory as the executable.  */
 static char *find_datadir(const char *argv0)
@@ -3556,10 +3527,8 @@ int main(int argc, char **argv, char **envp)
 
     cpu_synchronize_all_post_init();
 
-#ifndef _WIN32
     /* must be after terminal init, SDL library changes signal handlers */
-    sighandler_setup();
-#endif
+    os_setup_signal_handling();
 
     set_numa_modes();
 
-- 
1.6.5.2

Re: [Qemu-devel] [PATCH 08/16] Move main signal handler setup to os specificfiles.

[Richard Henderson]

On 06/03/2010 09:48 AM, Jes.Sorensen@redhat.com wrote:
> --- a/qemu-os-win32.h
> +++ b/qemu-os-win32.h
> @@ -41,4 +41,7 @@ int qemu_add_wait_object(HANDLE handle, WaitObjectFunc *func, void *opaque);
>  void qemu_del_wait_object(HANDLE handle, WaitObjectFunc *func, void *opaque);
>  
>  void os_host_main_loop_wait(int *timeout);
> +
> +static inline void os_setup_signal_handling(void) {};

Stray ;


r~

Re: [Qemu-devel] [PATCH 08/16] Move main signal handler setup to os specificfiles.

[Jes Sorensen]

On 06/03/10 22:52, Richard Henderson wrote:
> On 06/03/2010 09:48 AM, Jes.Sorensen@redhat.com wrote:
>> --- a/qemu-os-win32.h
>> +++ b/qemu-os-win32.h
>> @@ -41,4 +41,7 @@ int qemu_add_wait_object(HANDLE handle, WaitObjectFunc *func, void *opaque);
>>  void qemu_del_wait_object(HANDLE handle, WaitObjectFunc *func, void *opaque);
>>  
>>  void os_host_main_loop_wait(int *timeout);
>> +
>> +static inline void os_setup_signal_handling(void) {};
> 
> Stray ;

Sorry, not sure what you mean here?

Jes

Re: [Qemu-devel] [PATCH 08/16] Move main signal handler setup to os specificfiles.

[Markus Armbruster]

Jes Sorensen <Jes.Sorensen@redhat.com> writes:

> On 06/03/10 22:52, Richard Henderson wrote:
>> On 06/03/2010 09:48 AM, Jes.Sorensen@redhat.com wrote:
>>> --- a/qemu-os-win32.h
>>> +++ b/qemu-os-win32.h
>>> @@ -41,4 +41,7 @@ int qemu_add_wait_object(HANDLE handle, WaitObjectFunc *func, void *opaque);
>>>  void qemu_del_wait_object(HANDLE handle, WaitObjectFunc *func, void *opaque);
>>>  
>>>  void os_host_main_loop_wait(int *timeout);
>>> +
>>> +static inline void os_setup_signal_handling(void) {};
>> 
>> Stray ;
>
> Sorry, not sure what you mean here?

There's a stray ';' after the function body's closing brace.  Please
drop it.

[Qemu-devel] [PATCH 09/16] Move find_datadir to OS specific files.

[Jes.Sorensen]

From: Jes Sorensen <Jes.Sorensen@redhat.com>

This moves the win32 and POSIX versions of find_datadir() to OS
specific files, and removes some #ifdef clutter from vl.c

Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com>
---
 os-posix.c |   64 +++++++++++++++++++++++++++++++++++++++
 os-win32.c |   23 ++++++++++++++
 sysemu.h   |    1 +
 vl.c       |   98 ++---------------------------------------------------------
 4 files changed, 92 insertions(+), 94 deletions(-)

diff --git a/os-posix.c b/os-posix.c
index 01dbec2..621ad06 100644
--- a/os-posix.c
+++ b/os-posix.c
@@ -28,6 +28,7 @@
 #include <signal.h>
 #include <sys/types.h>
 #include <sys/wait.h>
+#include <libgen.h>
 
 /* Needed early for CONFIG_BSD etc. */
 #include "config-host.h"
@@ -66,3 +67,66 @@ void os_setup_signal_handling(void)
     act.sa_flags = SA_NOCLDSTOP;
     sigaction(SIGCHLD, &act, NULL);
 }
+
+/* Find a likely location for support files using the location of the binary.
+   For installed binaries this will be "$bindir/../share/qemu".  When
+   running from the build tree this will be "$bindir/../pc-bios".  */
+#define SHARE_SUFFIX "/share/qemu"
+#define BUILD_SUFFIX "/pc-bios"
+char *os_find_datadir(const char *argv0)
+{
+    char *dir;
+    char *p = NULL;
+    char *res;
+    char buf[PATH_MAX];
+    size_t max_len;
+
+#if defined(__linux__)
+    {
+        int len;
+        len = readlink("/proc/self/exe", buf, sizeof(buf) - 1);
+        if (len > 0) {
+            buf[len] = 0;
+            p = buf;
+        }
+    }
+#elif defined(__FreeBSD__)
+    {
+        static int mib[4] = {CTL_KERN, KERN_PROC, KERN_PROC_PATHNAME, -1};
+        size_t len = sizeof(buf) - 1;
+
+        *buf = '\0';
+        if (!sysctl(mib, sizeof(mib)/sizeof(*mib), buf, &len, NULL, 0) &&
+            *buf) {
+            buf[sizeof(buf) - 1] = '\0';
+            p = buf;
+        }
+    }
+#endif
+    /* If we don't have any way of figuring out the actual executable
+       location then try argv[0].  */
+    if (!p) {
+        p = realpath(argv0, buf);
+        if (!p) {
+            return NULL;
+        }
+    }
+    dir = dirname(p);
+    dir = dirname(dir);
+
+    max_len = strlen(dir) +
+        MAX(strlen(SHARE_SUFFIX), strlen(BUILD_SUFFIX)) + 1;
+    res = qemu_mallocz(max_len);
+    snprintf(res, max_len, "%s%s", dir, SHARE_SUFFIX);
+    if (access(res, R_OK)) {
+        snprintf(res, max_len, "%s%s", dir, BUILD_SUFFIX);
+        if (access(res, R_OK)) {
+            qemu_free(res);
+            res = NULL;
+        }
+    }
+
+    return res;
+}
+#undef SHARE_SUFFIX
+#undef BUILD_SUFFIX
diff --git a/os-win32.c b/os-win32.c
index a936f7a..1758538 100644
--- a/os-win32.c
+++ b/os-win32.c
@@ -181,3 +181,26 @@ void os_setup_early_signal_handling(void)
         }
     }
 }
+
+/* Look for support files in the same directory as the executable.  */
+char *os_find_datadir(const char *argv0)
+{
+    char *p;
+    char buf[MAX_PATH];
+    DWORD len;
+
+    len = GetModuleFileName(NULL, buf, sizeof(buf) - 1);
+    if (len == 0) {
+        return NULL;
+    }
+
+    buf[len] = 0;
+    p = buf + len - 1;
+    while (p != buf && *p != '\\')
+        p--;
+    *p = 0;
+    if (access(buf, R_OK) == 0) {
+        return qemu_strdup(buf);
+    }
+    return NULL;
+}
diff --git a/sysemu.h b/sysemu.h
index 79ffd9f..72f3734 100644
--- a/sysemu.h
+++ b/sysemu.h
@@ -81,6 +81,7 @@ void do_info_slirp(Monitor *mon);
 
 /* OS specific functions */
 void os_setup_early_signal_handling(void);
+char *os_find_datadir(const char *argv0);
 
 typedef enum DisplayType
 {
diff --git a/vl.c b/vl.c
index fc5e8d8..7f22733 100644
--- a/vl.c
+++ b/vl.c
@@ -1986,95 +1986,6 @@ static int balloon_parse(const char *arg)
     return -1;
 }
 
-#ifdef _WIN32
-/* Look for support files in the same directory as the executable.  */
-static char *find_datadir(const char *argv0)
-{
-    char *p;
-    char buf[MAX_PATH];
-    DWORD len;
-
-    len = GetModuleFileName(NULL, buf, sizeof(buf) - 1);
-    if (len == 0) {
-        return NULL;
-    }
-
-    buf[len] = 0;
-    p = buf + len - 1;
-    while (p != buf && *p != '\\')
-        p--;
-    *p = 0;
-    if (access(buf, R_OK) == 0) {
-        return qemu_strdup(buf);
-    }
-    return NULL;
-}
-#else /* !_WIN32 */
-
-/* Find a likely location for support files using the location of the binary.
-   For installed binaries this will be "$bindir/../share/qemu".  When
-   running from the build tree this will be "$bindir/../pc-bios".  */
-#define SHARE_SUFFIX "/share/qemu"
-#define BUILD_SUFFIX "/pc-bios"
-static char *find_datadir(const char *argv0)
-{
-    char *dir;
-    char *p = NULL;
-    char *res;
-    char buf[PATH_MAX];
-    size_t max_len;
-
-#if defined(__linux__)
-    {
-        int len;
-        len = readlink("/proc/self/exe", buf, sizeof(buf) - 1);
-        if (len > 0) {
-            buf[len] = 0;
-            p = buf;
-        }
-    }
-#elif defined(__FreeBSD__)
-    {
-        static int mib[4] = {CTL_KERN, KERN_PROC, KERN_PROC_PATHNAME, -1};
-        size_t len = sizeof(buf) - 1;
-
-        *buf = '\0';
-        if (!sysctl(mib, sizeof(mib)/sizeof(*mib), buf, &len, NULL, 0) &&
-            *buf) {
-            buf[sizeof(buf) - 1] = '\0';
-            p = buf;
-        }
-    }
-#endif
-    /* If we don't have any way of figuring out the actual executable
-       location then try argv[0].  */
-    if (!p) {
-        p = realpath(argv0, buf);
-        if (!p) {
-            return NULL;
-        }
-    }
-    dir = dirname(p);
-    dir = dirname(dir);
-
-    max_len = strlen(dir) +
-        MAX(strlen(SHARE_SUFFIX), strlen(BUILD_SUFFIX)) + 1;
-    res = qemu_mallocz(max_len);
-    snprintf(res, max_len, "%s%s", dir, SHARE_SUFFIX);
-    if (access(res, R_OK)) {
-        snprintf(res, max_len, "%s%s", dir, BUILD_SUFFIX);
-        if (access(res, R_OK)) {
-            qemu_free(res);
-            res = NULL;
-        }
-    }
-
-    return res;
-}
-#undef SHARE_SUFFIX
-#undef BUILD_SUFFIX
-#endif
-
 char *qemu_find_file(int type, const char *name)
 {
     int len;
@@ -3223,11 +3134,10 @@ int main(int argc, char **argv, char **envp)
     /* If no data_dir is specified then try to find it relative to the
        executable path.  */
     if (!data_dir) {
-        data_dir = find_datadir(argv[0]);
-    }
-    /* If all else fails use the install patch specified when building.  */
-    if (!data_dir) {
-        data_dir = CONFIG_QEMU_SHAREDIR;
+        data_dir = os_find_datadir(argv[0]);
+        /* If all else fails use the install patch specified when building.  */
+        if (!data_dir)
+            data_dir = CONFIG_QEMU_SHAREDIR;
     }
 
     /*
-- 
1.6.5.2

[Qemu-devel] [PATCH 10/16] Introduce OS specific cmdline argument handling and move SMB arg to os-posix.c

[Jes.Sorensen]

From: Jes Sorensen <Jes.Sorensen@redhat.com>

Introduce OS specific cmdline argument handling by calling
os_parse_cmd_args() at the end of switch() statement.

In addition move SMB argument to os-posix.c

Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com>
---
 os-posix.c |   34 ++++++++++++++++++++++++++++++++++
 os-win32.c |   22 ++++++++++++++++++++++
 sysemu.h   |    9 +++++++++
 vl.c       |   15 ++-------------
 4 files changed, 67 insertions(+), 13 deletions(-)

diff --git a/os-posix.c b/os-posix.c
index 621ad06..66f2bf5 100644
--- a/os-posix.c
+++ b/os-posix.c
@@ -33,6 +33,7 @@
 /* Needed early for CONFIG_BSD etc. */
 #include "config-host.h"
 #include "sysemu.h"
+#include "net/slirp.h"
 
 void os_setup_early_signal_handling(void)
 {
@@ -130,3 +131,36 @@ char *os_find_datadir(const char *argv0)
 }
 #undef SHARE_SUFFIX
 #undef BUILD_SUFFIX
+
+/*
+ * Duplicate definition from vl.c to avoid messing up the entire build
+ */
+enum {
+#define DEF(option, opt_arg, opt_enum, opt_help, arch_mask)     \
+    opt_enum,
+#define DEFHEADING(text)
+#include "qemu-options.h"
+#undef DEF
+#undef DEFHEADING
+#undef GEN_DOCS
+};
+
+/*
+ * Parse OS specific command line options.
+ * return 0 if option handled, -1 otherwise
+ */
+int os_parse_cmd_args(const QEMUOption *popt, const char *optarg)
+{
+    int ret = 0;
+    switch (popt->index) {
+#ifdef CONFIG_SLIRP
+    case QEMU_OPTION_smb:
+        if (net_slirp_smb(optarg) < 0)
+            exit(1);
+        break;
+#endif
+    default:
+        ret = -1;
+    }
+    return ret;
+}
diff --git a/os-win32.c b/os-win32.c
index 1758538..a311a90 100644
--- a/os-win32.c
+++ b/os-win32.c
@@ -204,3 +204,25 @@ char *os_find_datadir(const char *argv0)
     }
     return NULL;
 }
+
+/*
+ * Duplicate definition from vl.c to avoid messing up the entire build
+ */
+enum {
+#define DEF(option, opt_arg, opt_enum, opt_help, arch_mask)     \
+    opt_enum,
+#define DEFHEADING(text)
+#include "qemu-options.h"
+#undef DEF
+#undef DEFHEADING
+#undef GEN_DOCS
+};
+
+/*
+ * Parse OS specific command line options.
+ * return 0 if option handled, -1 otherwise
+ */
+int os_parse_cmd_args(const QEMUOption *popt, const char *optarg)
+{
+    return -1;
+}
diff --git a/sysemu.h b/sysemu.h
index 72f3734..08ec323 100644
--- a/sysemu.h
+++ b/sysemu.h
@@ -79,9 +79,18 @@ int qemu_loadvm_state(QEMUFile *f);
 /* SLIRP */
 void do_info_slirp(Monitor *mon);
 
+/* This is needed for vl.c and the OS specific files */
+typedef struct QEMUOption {
+    const char *name;
+    int flags;
+    int index;
+    uint32_t arch_mask;
+} QEMUOption;
+
 /* OS specific functions */
 void os_setup_early_signal_handling(void);
 char *os_find_datadir(const char *argv0);
+int os_parse_cmd_args(const QEMUOption *popt, const char *optarg);
 
 typedef enum DisplayType
 {
diff --git a/vl.c b/vl.c
index 7f22733..838e109 100644
--- a/vl.c
+++ b/vl.c
@@ -1909,13 +1909,6 @@ enum {
 #undef GEN_DOCS
 };
 
-typedef struct QEMUOption {
-    const char *name;
-    int flags;
-    int index;
-    uint32_t arch_mask;
-} QEMUOption;
-
 static const QEMUOption qemu_options[] = {
     { "h", 0, QEMU_OPTION_h, QEMU_ARCH_ALL },
 #define DEF(option, opt_arg, opt_enum, opt_help, arch_mask)     \
@@ -2624,12 +2617,6 @@ int main(int argc, char **argv, char **envp)
             case QEMU_OPTION_bootp:
                 legacy_bootp_filename = optarg;
                 break;
-#ifndef _WIN32
-            case QEMU_OPTION_smb:
-                if (net_slirp_smb(optarg) < 0)
-                    exit(1);
-                break;
-#endif
             case QEMU_OPTION_redir:
                 if (net_slirp_redir(optarg) < 0)
                     exit(1);
@@ -3126,6 +3113,8 @@ int main(int argc, char **argv, char **envp)
                     fclose(fp);
                     break;
                 }
+            default:
+                os_parse_cmd_args(popt, optarg);
             }
         }
     }
-- 
1.6.5.2

Re: [Qemu-devel] [PATCH 10/16] Introduce OS specific cmdline argument handling and move SMB arg to os-posix.c

[Richard Henderson]

On 06/03/2010 09:48 AM, Jes.Sorensen@redhat.com wrote:
> +/*
> + * Duplicate definition from vl.c to avoid messing up the entire build
> + */
> +enum {
> +#define DEF(option, opt_arg, opt_enum, opt_help, arch_mask)     \
> +    opt_enum,
> +#define DEFHEADING(text)
> +#include "qemu-options.h"
> +#undef DEF
> +#undef DEFHEADING
> +#undef GEN_DOCS
> +};

There's no header file you can put this in?  Or invent to put this in?
Cause this is really kinda gross...

> +
> +/*
> + * Parse OS specific command line options.
> + * return 0 if option handled, -1 otherwise
> + */
> +int os_parse_cmd_args(const QEMUOption *popt, const char *optarg)
> +{
> +    int ret = 0;
> +    switch (popt->index) {
> +#ifdef CONFIG_SLIRP
> +    case QEMU_OPTION_smb:
> +        if (net_slirp_smb(optarg) < 0)
> +            exit(1);
> +        break;
> +#endif
> +    default:
> +        ret = -1;
> +    }
> +    return ret;
> +}

Why have a return value at all...

> +            default:
> +                os_parse_cmd_args(popt, optarg);

... if you're going to ignore the results?


r~

Re: [Qemu-devel] [PATCH 10/16] Introduce OS specific cmdline argument handling and move SMB arg to os-posix.c

[Jes Sorensen]

On 06/03/10 22:58, Richard Henderson wrote:
> On 06/03/2010 09:48 AM, Jes.Sorensen@redhat.com wrote:
>> +/*
>> + * Duplicate definition from vl.c to avoid messing up the entire build
>> + */
>> +enum {
>> +#define DEF(option, opt_arg, opt_enum, opt_help, arch_mask)     \
>> +    opt_enum,
>> +#define DEFHEADING(text)
>> +#include "qemu-options.h"
>> +#undef DEF
>> +#undef DEFHEADING
>> +#undef GEN_DOCS
>> +};
> 
> There's no header file you can put this in?  Or invent to put this in?
> Cause this is really kinda gross...
> 

The problem is that it requires qemu-options.h to be included, which
isn't included per default for all the files. If I put it into sysemu.h
at least it's going to require making every .c file build with those flags.

I agree it's gross, but I am not sure what would be a better solution.

>> +    default:
>> +        ret = -1;
>> +    }
>> +    return ret;
>> +}
> 
> Why have a return value at all...
> 
>> +            default:
>> +                os_parse_cmd_args(popt, optarg);
> 
> ... if you're going to ignore the results?

I was trying to make it forward looking, but yeah we can just kill that.

Cheers,
Jes

Re: [Qemu-devel] [PATCH 10/16] Introduce OS specific cmdline argument handling and move SMB arg to os-posix.c

[Richard Henderson]

On 06/03/2010 11:47 PM, Jes Sorensen wrote:
> On 06/03/10 22:58, Richard Henderson wrote:
>> On 06/03/2010 09:48 AM, Jes.Sorensen@redhat.com wrote:
>>> +/*
>>> + * Duplicate definition from vl.c to avoid messing up the entire build
>>> + */
>>> +enum {
>>> +#define DEF(option, opt_arg, opt_enum, opt_help, arch_mask)     \
>>> +    opt_enum,
>>> +#define DEFHEADING(text)
>>> +#include "qemu-options.h"
>>> +#undef DEF
>>> +#undef DEFHEADING
>>> +#undef GEN_DOCS
>>> +};
>>
>> There's no header file you can put this in?  Or invent to put this in?
>> Cause this is really kinda gross...
>>
> 
> The problem is that it requires qemu-options.h to be included, which
> isn't included per default for all the files. If I put it into sysemu.h
> at least it's going to require making every .c file build with those flags.
> 
> I agree it's gross, but I am not sure what would be a better solution.

One possible solution is to put this whole block in "qemu-options-enum.h"
(or whatever) and include that in the three places that you have this block.


r~

Re: [Qemu-devel] [PATCH 10/16] Introduce OS specific cmdline argument handling and move SMB arg to os-posix.c

[Jes Sorensen]

On 06/04/10 16:49, Richard Henderson wrote:
> On 06/03/2010 11:47 PM, Jes Sorensen wrote:
>> The problem is that it requires qemu-options.h to be included, which
>> isn't included per default for all the files. If I put it into sysemu.h
>> at least it's going to require making every .c file build with those flags.
>>
>> I agree it's gross, but I am not sure what would be a better solution.
> 
> One possible solution is to put this whole block in "qemu-options-enum.h"
> (or whatever) and include that in the three places that you have this block.

I ended up moving qemu-options.h to qemu-options.def and then creating a
proper qemu-options.h per Paolo's suggestion.

Let me know what you think of the v2 patch set, minus the duplicates
some idiot posted because he forgot to remove the Emacs backup files :)

Cheers,
Jes

Re: [Qemu-devel] [PATCH 10/16] Introduce OS specific cmdline argument handling and move SMB arg to os-posix.c

[Markus Armbruster]

Jes.Sorensen@redhat.com writes:

> From: Jes Sorensen <Jes.Sorensen@redhat.com>
>
> Introduce OS specific cmdline argument handling by calling
> os_parse_cmd_args() at the end of switch() statement.
>
> In addition move SMB argument to os-posix.c
>
> Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com>
> ---
>  os-posix.c |   34 ++++++++++++++++++++++++++++++++++
>  os-win32.c |   22 ++++++++++++++++++++++
>  sysemu.h   |    9 +++++++++
>  vl.c       |   15 ++-------------
>  4 files changed, 67 insertions(+), 13 deletions(-)
>
> diff --git a/os-posix.c b/os-posix.c
> index 621ad06..66f2bf5 100644
> --- a/os-posix.c
> +++ b/os-posix.c
> @@ -33,6 +33,7 @@
>  /* Needed early for CONFIG_BSD etc. */
>  #include "config-host.h"
>  #include "sysemu.h"
> +#include "net/slirp.h"
>  
>  void os_setup_early_signal_handling(void)
>  {
> @@ -130,3 +131,36 @@ char *os_find_datadir(const char *argv0)
>  }
>  #undef SHARE_SUFFIX
>  #undef BUILD_SUFFIX
> +
> +/*
> + * Duplicate definition from vl.c to avoid messing up the entire build
> + */
> +enum {
> +#define DEF(option, opt_arg, opt_enum, opt_help, arch_mask)     \
> +    opt_enum,
> +#define DEFHEADING(text)
> +#include "qemu-options.h"
> +#undef DEF
> +#undef DEFHEADING
> +#undef GEN_DOCS
> +};
> +
> +/*
> + * Parse OS specific command line options.
> + * return 0 if option handled, -1 otherwise
> + */
> +int os_parse_cmd_args(const QEMUOption *popt, const char *optarg)
> +{
> +    int ret = 0;
> +    switch (popt->index) {
> +#ifdef CONFIG_SLIRP
> +    case QEMU_OPTION_smb:
> +        if (net_slirp_smb(optarg) < 0)
> +            exit(1);
> +        break;
> +#endif

Was #ifndef _WIN32 before.  Impact?

> +    default:
> +        ret = -1;
> +    }
> +    return ret;
> +}
> diff --git a/os-win32.c b/os-win32.c
> index 1758538..a311a90 100644
> --- a/os-win32.c
> +++ b/os-win32.c
> @@ -204,3 +204,25 @@ char *os_find_datadir(const char *argv0)
>      }
>      return NULL;
>  }
> +
> +/*
> + * Duplicate definition from vl.c to avoid messing up the entire build
> + */
> +enum {
> +#define DEF(option, opt_arg, opt_enum, opt_help, arch_mask)     \
> +    opt_enum,
> +#define DEFHEADING(text)
> +#include "qemu-options.h"
> +#undef DEF
> +#undef DEFHEADING
> +#undef GEN_DOCS
> +};

I agree with Richard: this is gross.

> +
> +/*
> + * Parse OS specific command line options.
> + * return 0 if option handled, -1 otherwise
> + */
> +int os_parse_cmd_args(const QEMUOption *popt, const char *optarg)
> +{
> +    return -1;
> +}
> diff --git a/sysemu.h b/sysemu.h
> index 72f3734..08ec323 100644
> --- a/sysemu.h
> +++ b/sysemu.h
> @@ -79,9 +79,18 @@ int qemu_loadvm_state(QEMUFile *f);
>  /* SLIRP */
>  void do_info_slirp(Monitor *mon);
>  
> +/* This is needed for vl.c and the OS specific files */
> +typedef struct QEMUOption {
> +    const char *name;
> +    int flags;
> +    int index;
> +    uint32_t arch_mask;
> +} QEMUOption;
> +

Ugh.

>  /* OS specific functions */
>  void os_setup_early_signal_handling(void);
>  char *os_find_datadir(const char *argv0);
> +int os_parse_cmd_args(const QEMUOption *popt, const char *optarg);
>  
>  typedef enum DisplayType
>  {
> diff --git a/vl.c b/vl.c
> index 7f22733..838e109 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -1909,13 +1909,6 @@ enum {
>  #undef GEN_DOCS
>  };
>  
> -typedef struct QEMUOption {
> -    const char *name;
> -    int flags;
> -    int index;
> -    uint32_t arch_mask;
> -} QEMUOption;
> -
>  static const QEMUOption qemu_options[] = {
>      { "h", 0, QEMU_OPTION_h, QEMU_ARCH_ALL },
>  #define DEF(option, opt_arg, opt_enum, opt_help, arch_mask)     \
> @@ -2624,12 +2617,6 @@ int main(int argc, char **argv, char **envp)
>              case QEMU_OPTION_bootp:
>                  legacy_bootp_filename = optarg;
>                  break;
> -#ifndef _WIN32
> -            case QEMU_OPTION_smb:
> -                if (net_slirp_smb(optarg) < 0)
> -                    exit(1);
> -                break;
> -#endif
>              case QEMU_OPTION_redir:
>                  if (net_slirp_redir(optarg) < 0)
>                      exit(1);
> @@ -3126,6 +3113,8 @@ int main(int argc, char **argv, char **envp)
>                      fclose(fp);
>                      break;
>                  }
> +            default:
> +                os_parse_cmd_args(popt, optarg);
>              }
>          }
>      }

Is this minor improvement of vl.c really worth the headaches elsewhere?

Re: [Qemu-devel] [PATCH 10/16] Introduce OS specific cmdline argument handling and move SMB arg to os-posix.c

[Jes Sorensen]

On 06/04/10 10:15, Markus Armbruster wrote:
> Jes.Sorensen@redhat.com writes:
>> + * Parse OS specific command line options.
>> + * return 0 if option handled, -1 otherwise
>> + */
>> +int os_parse_cmd_args(const QEMUOption *popt, const char *optarg)
>> +{
>> +    int ret = 0;
>> +    switch (popt->index) {
>> +#ifdef CONFIG_SLIRP
>> +    case QEMU_OPTION_smb:
>> +        if (net_slirp_smb(optarg) < 0)
>> +            exit(1);
>> +        break;
>> +#endif
> 
> Was #ifndef _WIN32 before.  Impact?

It was moved to os-posix.c which is only built for non _WIN32, so it has
the same effect, except it's not full of ugly #ifdef's

>> +/*
>> + * Duplicate definition from vl.c to avoid messing up the entire build
>> + */
>> +enum {
>> +#define DEF(option, opt_arg, opt_enum, opt_help, arch_mask)     \
>> +    opt_enum,
>> +#define DEFHEADING(text)
>> +#include "qemu-options.h"
>> +#undef DEF
>> +#undef DEFHEADING
>> +#undef GEN_DOCS
>> +};
> 
> I agree with Richard: this is gross.

The enum creation is gross by itself. Only way to get around not
duplicating it is to create a new header file to hold just that?

>> +/* This is needed for vl.c and the OS specific files */
>> +typedef struct QEMUOption {
>> +    const char *name;
>> +    int flags;
>> +    int index;
>> +    uint32_t arch_mask;
>> +} QEMUOption;
>> +
> 
> Ugh.

What do you mean? The real ugh! here is that it was created as a
typedef. I can change the function to pass in just the index, but I
don't know if we will have cases where the rest is needed.

> Is this minor improvement of vl.c really worth the headaches elsewhere?

vl.c as it is today is gross and un-maintainable. This patch gets rid of
a lot of the ugly #ifdefs and makes the code easier to read and maintain.

Jes

[Qemu-devel] Re: [PATCH 10/16] Introduce OS specific cmdline argument handling and move SMB arg to os-posix.c

[Paolo Bonzini]

>>> +/*
>>> + * Duplicate definition from vl.c to avoid messing up the entire build
>>> + */
>>> +enum {
>>> +#define DEF(option, opt_arg, opt_enum, opt_help, arch_mask)     \
>>> +    opt_enum,
>>> +#define DEFHEADING(text)
>>> +#include "qemu-options.h"
>>> +#undef DEF
>>> +#undef DEFHEADING
>>> +#undef GEN_DOCS
>>> +};
>>
>> I agree with Richard: this is gross.
>
> The enum creation is gross by itself. Only way to get around not
> duplicating it is to create a new header file to hold just that?

I don't think it's particularly gross.  At least you don't have two 
files to keep in sync.

You could rename qemu-options.h to qemu-options.def, and make a real 
header file with the typedef and the enum.  Then include the header from 
vl.c and os-*.c.

BTW from Fedora 11 and newer you can easily build QEMU with a cross 
compiler.  (Running it is a bit harder).  These packages should suffice:

mingw32-w32api mingw32-cpp mingw32-termcap mingw32-runtime
mingw32-binutils mingw32-filesystem mingw32-SDL mingw32-gcc
mingw32-zlib

and you need to configure it with "--cross-prefix=i686-pc-mingw32-" 
(trailing dash included!).

Paolo

[Qemu-devel] Re: [PATCH 10/16] Introduce OS specific cmdline argument handling and move SMB arg to os-posix.c

[Jes Sorensen]

On 06/04/10 12:39, Paolo Bonzini wrote:
>>> I agree with Richard: this is gross.
>>
>> The enum creation is gross by itself. Only way to get around not
>> duplicating it is to create a new header file to hold just that?
> 
> I don't think it's particularly gross.  At least you don't have two
> files to keep in sync.
> 
> You could rename qemu-options.h to qemu-options.def, and make a real
> header file with the typedef and the enum.  Then include the header from
> vl.c and os-*.c.

I like this idea. I was looking at a qemu-options-somethingelse.h but I
couldn't really find any appropriate name for it. Renaming the current
qemu-options.h to qemu-options.def seems correct IMHO as it's not really
a header file that can be included into code by itself.

> BTW from Fedora 11 and newer you can easily build QEMU with a cross
> compiler.  (Running it is a bit harder).  These packages should suffice:
> 
> mingw32-w32api mingw32-cpp mingw32-termcap mingw32-runtime
> mingw32-binutils mingw32-filesystem mingw32-SDL mingw32-gcc
> mingw32-zlib
> 
> and you need to configure it with "--cross-prefix=i686-pc-mingw32-"
> (trailing dash included!).

Thanks! I'll have to take a look at this.

Cheers,
Jes

Re: [Qemu-devel] [PATCH 10/16] Introduce OS specific cmdline argument handling and move SMB arg to os-posix.c

[Markus Armbruster]

Jes Sorensen <Jes.Sorensen@redhat.com> writes:

> On 06/04/10 10:15, Markus Armbruster wrote:
>> Jes.Sorensen@redhat.com writes:
>>> + * Parse OS specific command line options.
>>> + * return 0 if option handled, -1 otherwise
>>> + */
>>> +int os_parse_cmd_args(const QEMUOption *popt, const char *optarg)
>>> +{
>>> +    int ret = 0;
>>> +    switch (popt->index) {
>>> +#ifdef CONFIG_SLIRP
>>> +    case QEMU_OPTION_smb:
>>> +        if (net_slirp_smb(optarg) < 0)
>>> +            exit(1);
>>> +        break;
>>> +#endif
>> 
>> Was #ifndef _WIN32 before.  Impact?
>
> It was moved to os-posix.c which is only built for non _WIN32, so it has
> the same effect, except it's not full of ugly #ifdef's

I missed the fact that it is under #ifdef CONFIG_SLIRP in the current
code.  Sorry for the noise.

>>> +/*
>>> + * Duplicate definition from vl.c to avoid messing up the entire build
>>> + */
>>> +enum {
>>> +#define DEF(option, opt_arg, opt_enum, opt_help, arch_mask)     \
>>> +    opt_enum,
>>> +#define DEFHEADING(text)
>>> +#include "qemu-options.h"
>>> +#undef DEF
>>> +#undef DEFHEADING
>>> +#undef GEN_DOCS
>>> +};
>> 
>> I agree with Richard: this is gross.
>
> The enum creation is gross by itself. Only way to get around not
> duplicating it is to create a new header file to hold just that?
>
>>> +/* This is needed for vl.c and the OS specific files */
>>> +typedef struct QEMUOption {
>>> +    const char *name;
>>> +    int flags;
>>> +    int index;
>>> +    uint32_t arch_mask;
>>> +} QEMUOption;
>>> +
>> 
>> Ugh.
>
> What do you mean? The real ugh! here is that it was created as a
> typedef. I can change the function to pass in just the index, but I
> don't know if we will have cases where the rest is needed.

Moving stuff out of the vl.c grabbag is cool.  Moving stuff into the
sysemu.h grabbag is very uncool.

>> Is this minor improvement of vl.c really worth the headaches elsewhere?
>
> vl.c as it is today is gross and un-maintainable. This patch gets rid of
> a lot of the ugly #ifdefs and makes the code easier to read and maintain.

I'm not arguing against your patch, just trying to help making it even
better.

Re: [Qemu-devel] [PATCH 10/16] Introduce OS specific cmdline argument handling and move SMB arg to os-posix.c

[Jes Sorensen]

On 06/04/10 14:04, Markus Armbruster wrote:
> Jes Sorensen <Jes.Sorensen@redhat.com> writes:
> 
>> On 06/04/10 10:15, Markus Armbruster wrote:
>> What do you mean? The real ugh! here is that it was created as a
>> typedef. I can change the function to pass in just the index, but I
>> don't know if we will have cases where the rest is needed.
> 
> Moving stuff out of the vl.c grabbag is cool.  Moving stuff into the
> sysemu.h grabbag is very uncool.

I agree, I have a new version of the patch coming up shortly. I just
want to apply Paolo's idea of moving qemu-options.h around a bit.

>>> Is this minor improvement of vl.c really worth the headaches elsewhere?
>>
>> vl.c as it is today is gross and un-maintainable. This patch gets rid of
>> a lot of the ugly #ifdefs and makes the code easier to read and maintain.
> 
> I'm not arguing against your patch, just trying to help making it even
> better.

I was gathering that, and your input is much appreciated.

Cheers,
Jes

[Qemu-devel] [PATCH 11/16] Move runas handling from vl.c to OS specific files.

[Jes.Sorensen]

From: Jes Sorensen <Jes.Sorensen@redhat.com>

Move code to handle runas, ie. change of user id of QEMU process
to OS specific files and provide dummy stub for Win32.

Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com>
---
 os-posix.c      |   28 ++++++++++++++++++++++++++++
 qemu-os-posix.h |    1 +
 qemu-os-win32.h |    1 +
 vl.c            |   29 +----------------------------
 4 files changed, 31 insertions(+), 28 deletions(-)

diff --git a/os-posix.c b/os-posix.c
index 66f2bf5..f8a092e 100644
--- a/os-posix.c
+++ b/os-posix.c
@@ -28,6 +28,7 @@
 #include <signal.h>
 #include <sys/types.h>
 #include <sys/wait.h>
+#include <pwd.h>
 #include <libgen.h>
 
 /* Needed early for CONFIG_BSD etc. */
@@ -35,6 +36,8 @@
 #include "sysemu.h"
 #include "net/slirp.h"
 
+static struct passwd *user_pwd;
+
 void os_setup_early_signal_handling(void)
 {
     struct sigaction act;
@@ -159,8 +162,33 @@ int os_parse_cmd_args(const QEMUOption *popt, const char *optarg)
             exit(1);
         break;
 #endif
+    case QEMU_OPTION_runas:
+        user_pwd = getpwnam(optarg);
+        if (!user_pwd) {
+            fprintf(stderr, "User \"%s\" doesn't exist\n", optarg);
+            exit(1);
+        }
+        break;
     default:
         ret = -1;
     }
     return ret;
 }
+
+void os_change_process_uid(void)
+{
+    if (user_pwd) {
+        if (setgid(user_pwd->pw_gid) < 0) {
+            fprintf(stderr, "Failed to setgid(%d)\n", user_pwd->pw_gid);
+            exit(1);
+        }
+        if (setuid(user_pwd->pw_uid) < 0) {
+            fprintf(stderr, "Failed to setuid(%d)\n", user_pwd->pw_uid);
+            exit(1);
+        }
+        if (setuid(0) != -1) {
+            fprintf(stderr, "Dropping privileges failed\n");
+            exit(1);
+        }
+    }
+}
diff --git a/qemu-os-posix.h b/qemu-os-posix.h
index ff5adb1..6d8cf79 100644
--- a/qemu-os-posix.h
+++ b/qemu-os-posix.h
@@ -31,5 +31,6 @@ static inline void os_host_main_loop_wait(int *timeout)
 }
 
 void os_setup_signal_handling(void);
+void os_change_process_uid(void);
 
 #endif
diff --git a/qemu-os-win32.h b/qemu-os-win32.h
index 4343c6d..9df0eda 100644
--- a/qemu-os-win32.h
+++ b/qemu-os-win32.h
@@ -43,5 +43,6 @@ void qemu_del_wait_object(HANDLE handle, WaitObjectFunc *func, void *opaque);
 void os_host_main_loop_wait(int *timeout);
 
 static inline void os_setup_signal_handling(void) {};
+static inline void os_change_process_uid(void) {};
 
 #endif
diff --git a/vl.c b/vl.c
index 838e109..d42be8d 100644
--- a/vl.c
+++ b/vl.c
@@ -34,7 +34,6 @@
 
 #ifndef _WIN32
 #include <libgen.h>
-#include <pwd.h>
 #include <sys/times.h>
 #include <sys/wait.h>
 #include <termios.h>
@@ -2312,9 +2311,7 @@ int main(int argc, char **argv, char **envp)
     const char *incoming = NULL;
 #ifndef _WIN32
     int fd = 0;
-    struct passwd *pwd = NULL;
     const char *chroot_dir = NULL;
-    const char *run_as = NULL;
 #endif
     int show_vnc_port = 0;
     int defconfig = 1;
@@ -3062,9 +3059,6 @@ int main(int argc, char **argv, char **envp)
             case QEMU_OPTION_chroot:
                 chroot_dir = optarg;
                 break;
-            case QEMU_OPTION_runas:
-                run_as = optarg;
-                break;
 #endif
             case QEMU_OPTION_xen_domid:
                 if (!(xen_available())) {
@@ -3554,14 +3548,6 @@ int main(int argc, char **argv, char **envp)
 	    exit(1);
     }
 
-    if (run_as) {
-        pwd = getpwnam(run_as);
-        if (!pwd) {
-            fprintf(stderr, "User \"%s\" doesn't exist\n", run_as);
-            exit(1);
-        }
-    }
-
     if (chroot_dir) {
         if (chroot(chroot_dir) < 0) {
             fprintf(stderr, "chroot failed\n");
@@ -3573,20 +3559,7 @@ int main(int argc, char **argv, char **envp)
         }
     }
 
-    if (run_as) {
-        if (setgid(pwd->pw_gid) < 0) {
-            fprintf(stderr, "Failed to setgid(%d)\n", pwd->pw_gid);
-            exit(1);
-        }
-        if (setuid(pwd->pw_uid) < 0) {
-            fprintf(stderr, "Failed to setuid(%d)\n", pwd->pw_uid);
-            exit(1);
-        }
-        if (setuid(0) != -1) {
-            fprintf(stderr, "Dropping privileges failed\n");
-            exit(1);
-        }
-    }
+    os_change_process_uid();
 
     if (daemonize) {
         dup2(fd, 0);
-- 
1.6.5.2

Re: [Qemu-devel] [PATCH 11/16] Move runas handling from vl.c to OS specific files.

[Richard Henderson]

On 06/03/2010 09:48 AM, Jes.Sorensen@redhat.com wrote:
> +static inline void os_change_process_uid(void) {};

Stray ;


r~

[Qemu-devel] [PATCH 12/16] Move chroot handling to OS specific files.

[Jes.Sorensen]

From: Jes Sorensen <Jes.Sorensen@redhat.com>

Move chroot handling to OS specific files.

Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com>
---
 os-posix.c      |   19 +++++++++++++++++++
 qemu-os-posix.h |    1 +
 qemu-os-win32.h |    1 +
 vl.c            |   18 +-----------------
 4 files changed, 22 insertions(+), 17 deletions(-)

diff --git a/os-posix.c b/os-posix.c
index f8a092e..a91e1f6 100644
--- a/os-posix.c
+++ b/os-posix.c
@@ -37,6 +37,7 @@
 #include "net/slirp.h"
 
 static struct passwd *user_pwd;
+static const char *chroot_dir;
 
 void os_setup_early_signal_handling(void)
 {
@@ -169,6 +170,9 @@ int os_parse_cmd_args(const QEMUOption *popt, const char *optarg)
             exit(1);
         }
         break;
+    case QEMU_OPTION_chroot:
+        chroot_dir = optarg;
+        break;
     default:
         ret = -1;
     }
@@ -192,3 +196,18 @@ void os_change_process_uid(void)
         }
     }
 }
+
+void os_change_root(void)
+{
+    if (chroot_dir) {
+        if (chroot(chroot_dir) < 0) {
+            fprintf(stderr, "chroot failed\n");
+            exit(1);
+        }
+        if (chdir("/")) {
+            perror("not able to chdir to /");
+            exit(1);
+        }
+    }
+
+}
diff --git a/qemu-os-posix.h b/qemu-os-posix.h
index 6d8cf79..91c7b68 100644
--- a/qemu-os-posix.h
+++ b/qemu-os-posix.h
@@ -32,5 +32,6 @@ static inline void os_host_main_loop_wait(int *timeout)
 
 void os_setup_signal_handling(void);
 void os_change_process_uid(void);
+void os_change_root(void);
 
 #endif
diff --git a/qemu-os-win32.h b/qemu-os-win32.h
index 9df0eda..245b188 100644
--- a/qemu-os-win32.h
+++ b/qemu-os-win32.h
@@ -44,5 +44,6 @@ void os_host_main_loop_wait(int *timeout);
 
 static inline void os_setup_signal_handling(void) {};
 static inline void os_change_process_uid(void) {};
+static inline void os_change_root(void) {};
 
 #endif
diff --git a/vl.c b/vl.c
index d42be8d..7173684 100644
--- a/vl.c
+++ b/vl.c
@@ -2311,7 +2311,6 @@ int main(int argc, char **argv, char **envp)
     const char *incoming = NULL;
 #ifndef _WIN32
     int fd = 0;
-    const char *chroot_dir = NULL;
 #endif
     int show_vnc_port = 0;
     int defconfig = 1;
@@ -3055,11 +3054,6 @@ int main(int argc, char **argv, char **envp)
                 default_cdrom = 0;
                 default_sdcard = 0;
                 break;
-#ifndef _WIN32
-            case QEMU_OPTION_chroot:
-                chroot_dir = optarg;
-                break;
-#endif
             case QEMU_OPTION_xen_domid:
                 if (!(xen_available())) {
                     printf("Option %s not supported for this target\n", popt->name);
@@ -3548,17 +3542,7 @@ int main(int argc, char **argv, char **envp)
 	    exit(1);
     }
 
-    if (chroot_dir) {
-        if (chroot(chroot_dir) < 0) {
-            fprintf(stderr, "chroot failed\n");
-            exit(1);
-        }
-        if (chdir("/")) {
-            perror("not able to chdir to /");
-            exit(1);
-        }
-    }
-
+    os_change_root();
     os_change_process_uid();
 
     if (daemonize) {
-- 
1.6.5.2

Re: [Qemu-devel] [PATCH 12/16] Move chroot handling to OS specific files.

[Richard Henderson]

On 06/03/2010 09:48 AM, Jes.Sorensen@redhat.com wrote:
> +static inline void os_change_root(void) {};

You really like the ";", don't you.  ;-)


r~

Re: [Qemu-devel] [PATCH 12/16] Move chroot handling to OS specific files.

[Jes Sorensen]

On 06/03/10 23:02, Richard Henderson wrote:
> On 06/03/2010 09:48 AM, Jes.Sorensen@redhat.com wrote:
>> +static inline void os_change_root(void) {};
> 
> You really like the ";", don't you.  ;-)

LOL now I get it.

Yes, ;'s are so pretty ;-)

I'll clean it up and send out a new version. Still not sure about the
enmu but the rest is straight forward to handle.

Cheers,
Jes

[Qemu-devel] [PATCH 13/16] Move daemonize handling to OS specific files

[Jes.Sorensen]

From: Jes Sorensen <Jes.Sorensen@redhat.com>

Move daemonize handling from vl.c to OS specific files. Provide dummy
stubs for Win32.

Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com>
---
 os-posix.c      |  102 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 os-win32.c      |    5 +++
 qemu-os-posix.h |    2 +
 qemu-os-win32.h |    2 +
 sysemu.h        |    1 +
 vl.c            |  106 ++-----------------------------------------------------
 6 files changed, 115 insertions(+), 103 deletions(-)

diff --git a/os-posix.c b/os-posix.c
index a91e1f6..8a9d102 100644
--- a/os-posix.c
+++ b/os-posix.c
@@ -38,6 +38,8 @@
 
 static struct passwd *user_pwd;
 static const char *chroot_dir;
+static int daemonize;
+static int fds[2];
 
 void os_setup_early_signal_handling(void)
 {
@@ -173,6 +175,9 @@ int os_parse_cmd_args(const QEMUOption *popt, const char *optarg)
     case QEMU_OPTION_chroot:
         chroot_dir = optarg;
         break;
+    case QEMU_OPTION_daemonize:
+        daemonize = 1;
+        break;
     default:
         ret = -1;
     }
@@ -211,3 +216,100 @@ void os_change_root(void)
     }
 
 }
+
+void os_daemonize(void)
+{
+    if (daemonize) {
+	pid_t pid;
+
+	if (pipe(fds) == -1)
+	    exit(1);
+
+	pid = fork();
+	if (pid > 0) {
+	    uint8_t status;
+	    ssize_t len;
+
+	    close(fds[1]);
+
+	again:
+            len = read(fds[0], &status, 1);
+            if (len == -1 && (errno == EINTR))
+                goto again;
+
+            if (len != 1)
+                exit(1);
+            else if (status == 1) {
+                fprintf(stderr, "Could not acquire pidfile: %s\n", strerror(errno));
+                exit(1);
+            } else
+                exit(0);
+	} else if (pid < 0)
+            exit(1);
+
+	close(fds[0]);
+	qemu_set_cloexec(fds[1]);
+
+	setsid();
+
+	pid = fork();
+	if (pid > 0)
+	    exit(0);
+	else if (pid < 0)
+	    exit(1);
+
+	umask(027);
+
+        signal(SIGTSTP, SIG_IGN);
+        signal(SIGTTOU, SIG_IGN);
+        signal(SIGTTIN, SIG_IGN);
+    }
+}
+
+void os_setup_post(void)
+{
+    int fd = 0;
+
+    if (daemonize) {
+	uint8_t status = 0;
+	ssize_t len;
+
+    again1:
+	len = write(fds[1], &status, 1);
+	if (len == -1 && (errno == EINTR))
+	    goto again1;
+
+	if (len != 1)
+	    exit(1);
+
+        if (chdir("/")) {
+            perror("not able to chdir to /");
+            exit(1);
+        }
+	TFR(fd = qemu_open("/dev/null", O_RDWR));
+	if (fd == -1)
+	    exit(1);
+    }
+
+    os_change_root();
+    os_change_process_uid();
+
+    if (daemonize) {
+        dup2(fd, 0);
+        dup2(fd, 1);
+        dup2(fd, 2);
+
+        close(fd);
+    }
+}
+
+void os_pidfile_error(void)
+{
+    if (daemonize) {
+        uint8_t status = 1;
+        if (write(fds[1], &status, 1) != 1) {
+            perror("daemonize. Writing to pipe\n");
+        }
+    } else
+        fprintf(stderr, "Could not acquire pid file: %s\n", strerror(errno));
+}
diff --git a/os-win32.c b/os-win32.c
index a311a90..86ff327 100644
--- a/os-win32.c
+++ b/os-win32.c
@@ -226,3 +226,8 @@ int os_parse_cmd_args(const QEMUOption *popt, const char *optarg)
 {
     return -1;
 }
+
+void os_pidfile_error(void)
+{
+    fprintf(stderr, "Could not acquire pid file: %s\n", strerror(errno));
+}
diff --git a/qemu-os-posix.h b/qemu-os-posix.h
index 91c7b68..9b07660 100644
--- a/qemu-os-posix.h
+++ b/qemu-os-posix.h
@@ -33,5 +33,7 @@ static inline void os_host_main_loop_wait(int *timeout)
 void os_setup_signal_handling(void);
 void os_change_process_uid(void);
 void os_change_root(void);
+void os_daemonize(void);
+void os_setup_post(void);
 
 #endif
diff --git a/qemu-os-win32.h b/qemu-os-win32.h
index 245b188..ccb9691 100644
--- a/qemu-os-win32.h
+++ b/qemu-os-win32.h
@@ -45,5 +45,7 @@ void os_host_main_loop_wait(int *timeout);
 static inline void os_setup_signal_handling(void) {};
 static inline void os_change_process_uid(void) {};
 static inline void os_change_root(void) {};
+static inline void os_daemonize(void) {};
+static inline void os_setup_post(void) {};
 
 #endif
diff --git a/sysemu.h b/sysemu.h
index 08ec323..aa44a20 100644
--- a/sysemu.h
+++ b/sysemu.h
@@ -91,6 +91,7 @@ typedef struct QEMUOption {
 void os_setup_early_signal_handling(void);
 char *os_find_datadir(const char *argv0);
 int os_parse_cmd_args(const QEMUOption *popt, const char *optarg);
+void os_pidfile_error(void);
 
 typedef enum DisplayType
 {
diff --git a/vl.c b/vl.c
index 7173684..bb8abbf 100644
--- a/vl.c
+++ b/vl.c
@@ -215,9 +215,6 @@ int no_shutdown = 0;
 int cursor_hide = 1;
 int graphic_rotate = 0;
 uint8_t irq0override = 1;
-#ifndef _WIN32
-int daemonize = 0;
-#endif
 const char *watchdog;
 const char *option_rom[MAX_OPTION_ROMS];
 int nb_option_roms;
@@ -2303,15 +2300,9 @@ int main(int argc, char **argv, char **envp)
     const char *loadvm = NULL;
     QEMUMachine *machine;
     const char *cpu_model;
-#ifndef _WIN32
-    int fds[2];
-#endif
     int tb_size;
     const char *pid_file = NULL;
     const char *incoming = NULL;
-#ifndef _WIN32
-    int fd = 0;
-#endif
     int show_vnc_port = 0;
     int defconfig = 1;
 
@@ -2977,11 +2968,6 @@ int main(int argc, char **argv, char **envp)
                     exit(1);
                 }
                 break;
-#ifndef _WIN32
-	    case QEMU_OPTION_daemonize:
-		daemonize = 1;
-		break;
-#endif
 	    case QEMU_OPTION_option_rom:
 		if (nb_option_roms >= MAX_OPTION_ROMS) {
 		    fprintf(stderr, "Too many option ROMs\n");
@@ -3195,64 +3181,10 @@ int main(int argc, char **argv, char **envp)
     }
 #endif
 
-#ifndef _WIN32
-    if (daemonize) {
-	pid_t pid;
-
-	if (pipe(fds) == -1)
-	    exit(1);
-
-	pid = fork();
-	if (pid > 0) {
-	    uint8_t status;
-	    ssize_t len;
-
-	    close(fds[1]);
-
-	again:
-            len = read(fds[0], &status, 1);
-            if (len == -1 && (errno == EINTR))
-                goto again;
-
-            if (len != 1)
-                exit(1);
-            else if (status == 1) {
-                fprintf(stderr, "Could not acquire pidfile: %s\n", strerror(errno));
-                exit(1);
-            } else
-                exit(0);
-	} else if (pid < 0)
-            exit(1);
-
-	close(fds[0]);
-	qemu_set_cloexec(fds[1]);
-
-	setsid();
-
-	pid = fork();
-	if (pid > 0)
-	    exit(0);
-	else if (pid < 0)
-	    exit(1);
-
-	umask(027);
-
-        signal(SIGTSTP, SIG_IGN);
-        signal(SIGTTOU, SIG_IGN);
-        signal(SIGTTIN, SIG_IGN);
-    }
-#endif
+    os_daemonize();
 
     if (pid_file && qemu_create_pidfile(pid_file) != 0) {
-#ifndef _WIN32
-        if (daemonize) {
-            uint8_t status = 1;
-            if (write(fds[1], &status, 1) != 1) {
-                perror("daemonize. Writing to pipe\n");
-            }
-        } else
-#endif
-            fprintf(stderr, "Could not acquire pid file: %s\n", strerror(errno));
+        os_pidfile_error();
         exit(1);
     }
 
@@ -3520,39 +3452,7 @@ int main(int argc, char **argv, char **envp)
         vm_start();
     }
 
-#ifndef _WIN32
-    if (daemonize) {
-	uint8_t status = 0;
-	ssize_t len;
-
-    again1:
-	len = write(fds[1], &status, 1);
-	if (len == -1 && (errno == EINTR))
-	    goto again1;
-
-	if (len != 1)
-	    exit(1);
-
-        if (chdir("/")) {
-            perror("not able to chdir to /");
-            exit(1);
-        }
-	TFR(fd = qemu_open("/dev/null", O_RDWR));
-	if (fd == -1)
-	    exit(1);
-    }
-
-    os_change_root();
-    os_change_process_uid();
-
-    if (daemonize) {
-        dup2(fd, 0);
-        dup2(fd, 1);
-        dup2(fd, 2);
-
-        close(fd);
-    }
-#endif
+    os_setup_post();
 
     main_loop();
     quit_timers();
-- 
1.6.5.2

[Qemu-devel] [PATCH 14/16] Make os_change_process_uid and os_change_root os-posix.c local

[Jes.Sorensen]

From: Jes Sorensen <Jes.Sorensen@redhat.com>

os_change_process_uid() and os_change_root() are now only called
from os-posix.c, so no need to keep win32 stubs for them.

Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com>
---
 os-posix.c      |    8 ++++----
 qemu-os-posix.h |    2 --
 qemu-os-win32.h |    2 --
 3 files changed, 4 insertions(+), 8 deletions(-)

diff --git a/os-posix.c b/os-posix.c
index 8a9d102..7ac6f07 100644
--- a/os-posix.c
+++ b/os-posix.c
@@ -184,7 +184,7 @@ int os_parse_cmd_args(const QEMUOption *popt, const char *optarg)
     return ret;
 }
 
-void os_change_process_uid(void)
+static void change_process_uid(void)
 {
     if (user_pwd) {
         if (setgid(user_pwd->pw_gid) < 0) {
@@ -202,7 +202,7 @@ void os_change_process_uid(void)
     }
 }
 
-void os_change_root(void)
+static void change_root(void)
 {
     if (chroot_dir) {
         if (chroot(chroot_dir) < 0) {
@@ -291,8 +291,8 @@ void os_setup_post(void)
 	    exit(1);
     }
 
-    os_change_root();
-    os_change_process_uid();
+    change_root();
+    change_process_uid();
 
     if (daemonize) {
         dup2(fd, 0);
diff --git a/qemu-os-posix.h b/qemu-os-posix.h
index 9b07660..8be583d 100644
--- a/qemu-os-posix.h
+++ b/qemu-os-posix.h
@@ -31,8 +31,6 @@ static inline void os_host_main_loop_wait(int *timeout)
 }
 
 void os_setup_signal_handling(void);
-void os_change_process_uid(void);
-void os_change_root(void);
 void os_daemonize(void);
 void os_setup_post(void);
 
diff --git a/qemu-os-win32.h b/qemu-os-win32.h
index ccb9691..facd3d6 100644
--- a/qemu-os-win32.h
+++ b/qemu-os-win32.h
@@ -43,8 +43,6 @@ void qemu_del_wait_object(HANDLE handle, WaitObjectFunc *func, void *opaque);
 void os_host_main_loop_wait(int *timeout);
 
 static inline void os_setup_signal_handling(void) {};
-static inline void os_change_process_uid(void) {};
-static inline void os_change_root(void) {};
 static inline void os_daemonize(void) {};
 static inline void os_setup_post(void) {};
 
-- 
1.6.5.2

[Qemu-devel] [PATCH 15/16] Move line-buffering setup to OS specific files.

[Jes.Sorensen]

From: Jes Sorensen <Jes.Sorensen@redhat.com>

Move line-buffering setup to OS specific files.

Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com>
---
 os-posix.c      |    5 +++++
 qemu-os-posix.h |    1 +
 qemu-os-win32.h |    2 ++
 vl.c            |    5 +----
 4 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/os-posix.c b/os-posix.c
index 7ac6f07..7530276 100644
--- a/os-posix.c
+++ b/os-posix.c
@@ -313,3 +313,8 @@ void os_pidfile_error(void)
     } else
         fprintf(stderr, "Could not acquire pid file: %s\n", strerror(errno));
 }
+
+void os_set_line_buffering(void)
+{
+    setvbuf(stdout, NULL, _IOLBF, 0);
+}
diff --git a/qemu-os-posix.h b/qemu-os-posix.h
index 8be583d..cb210ba 100644
--- a/qemu-os-posix.h
+++ b/qemu-os-posix.h
@@ -30,6 +30,7 @@ static inline void os_host_main_loop_wait(int *timeout)
 {
 }
 
+void os_set_line_buffering(void);
 void os_setup_signal_handling(void);
 void os_daemonize(void);
 void os_setup_post(void);
diff --git a/qemu-os-win32.h b/qemu-os-win32.h
index facd3d6..1709cf6 100644
--- a/qemu-os-win32.h
+++ b/qemu-os-win32.h
@@ -45,5 +45,7 @@ void os_host_main_loop_wait(int *timeout);
 static inline void os_setup_signal_handling(void) {};
 static inline void os_daemonize(void) {};
 static inline void os_setup_post(void) {};
+/* Win32 doesn't support line-buffering and requires size >= 2 */
+static inline void os_set_line_buffering(void) {};
 
 #endif
diff --git a/vl.c b/vl.c
index bb8abbf..3dbc789 100644
--- a/vl.c
+++ b/vl.c
@@ -3216,10 +3216,7 @@ int main(int argc, char **argv, char **envp)
         exit(1);
     }
 
-#ifndef _WIN32
-    /* Win32 doesn't support line-buffering and requires size >= 2 */
-    setvbuf(stdout, NULL, _IOLBF, 0);
-#endif
+    os_set_line_buffering();
 
     if (init_timer_alarm() < 0) {
         fprintf(stderr, "could not initialize alarm timer\n");
-- 
1.6.5.2

[Qemu-devel] [PATCH 16/16] Move set_proc_name() to OS specific files.

[Jes.Sorensen]

From: Jes Sorensen <Jes.Sorensen@redhat.com>

Move handling to change process name to POSIX specific files
plus add a better error message to cover the case where the
feature isn't supported.

Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com>
---
 os-posix.c      |   24 ++++++++++++++++++++++++
 qemu-os-posix.h |    1 +
 qemu-os-win32.h |    1 +
 vl.c            |   19 +------------------
 4 files changed, 27 insertions(+), 18 deletions(-)

diff --git a/os-posix.c b/os-posix.c
index 7530276..03105f7 100644
--- a/os-posix.c
+++ b/os-posix.c
@@ -36,6 +36,10 @@
 #include "sysemu.h"
 #include "net/slirp.h"
 
+#ifdef CONFIG_LINUX
+#include <sys/prctl.h>
+#endif
+
 static struct passwd *user_pwd;
 static const char *chroot_dir;
 static int daemonize;
@@ -138,6 +142,26 @@ char *os_find_datadir(const char *argv0)
 #undef SHARE_SUFFIX
 #undef BUILD_SUFFIX
 
+void os_set_proc_name(const char *s)
+{
+#if defined(PR_SET_NAME)
+    char name[16];
+    if (!s)
+        return;
+    name[sizeof(name) - 1] = 0;
+    strncpy(name, s, sizeof(name));
+    /* Could rewrite argv[0] too, but that's a bit more complicated.
+       This simple way is enough for `top'. */
+    if (prctl(PR_SET_NAME, name)) {
+        perror("unable to change process name");
+        exit(1);
+    }
+#else
+    fprintf(stderr, "Change of process name not supported by your OS\n");
+    exit(1);
+#endif    	
+}
+
 /*
  * Duplicate definition from vl.c to avoid messing up the entire build
  */
diff --git a/qemu-os-posix.h b/qemu-os-posix.h
index cb210ba..ed5c058 100644
--- a/qemu-os-posix.h
+++ b/qemu-os-posix.h
@@ -31,6 +31,7 @@ static inline void os_host_main_loop_wait(int *timeout)
 }
 
 void os_set_line_buffering(void);
+void os_set_proc_name(const char *s);
 void os_setup_signal_handling(void);
 void os_daemonize(void);
 void os_setup_post(void);
diff --git a/qemu-os-win32.h b/qemu-os-win32.h
index 1709cf6..bb7126b 100644
--- a/qemu-os-win32.h
+++ b/qemu-os-win32.h
@@ -47,5 +47,6 @@ static inline void os_daemonize(void) {};
 static inline void os_setup_post(void) {};
 /* Win32 doesn't support line-buffering and requires size >= 2 */
 static inline void os_set_line_buffering(void) {};
+static inline void os_set_proc_name(const char *dummy) {};
 
 #endif
diff --git a/vl.c b/vl.c
index 3dbc789..b77dce8 100644
--- a/vl.c
+++ b/vl.c
@@ -59,7 +59,6 @@
 #ifdef __linux__
 #include <pty.h>
 #include <malloc.h>
-#include <sys/prctl.h>
 
 #include <linux/ppdev.h>
 #include <linux/parport.h>
@@ -283,22 +282,6 @@ static int default_driver_check(QemuOpts *opts, void *opaque)
 }
 
 /***********************************************************/
-
-static void set_proc_name(const char *s)
-{
-#if defined(__linux__) && defined(PR_SET_NAME)
-    char name[16];
-    if (!s)
-        return;
-    name[sizeof(name) - 1] = 0;
-    strncpy(name, s, sizeof(name));
-    /* Could rewrite argv[0] too, but that's a bit more complicated.
-       This simple way is enough for `top'. */
-    prctl(PR_SET_NAME, name);
-#endif    	
-}
- 
-/***********************************************************/
 /* real time host monotonic timer */
 
 /* compute with 96 bit intermediate result: (a*b)/c */
@@ -2990,7 +2973,7 @@ int main(int argc, char **argv, char **envp)
 			    exit(1);
 			}
 			p += 8;
-			set_proc_name(p);
+			os_set_proc_name(p);
 		     }	
 		 }	
                 break;
-- 
1.6.5.2

Re: [Qemu-devel] [PATCH 00/16] clean up vl.c code

[Markus Armbruster]

Jes.Sorensen@redhat.com writes:

> From: Jes Sorensen <Jes.Sorensen@redhat.com>
>
> Hi,
>
> I have been working on a set of patches to clean up the vl.c code, by
> separating out OS specific code into OS specific files. Basically it
> introduces two header files: qemu-os-win32.h and qemu-os-posix.h as
> well as os-win32.c and os-posix.c.
>
> I have tried to be as careful as I can to not break non Linux support,
> but as I only have a Linux build environment handy, I would appreciate
> it if people with other OSes could check that I didn't break anything
> for them. In particular I would like to know if win32 still builds.

I like moving stuff out of vl.c in general.  Your moves of entire
functions look like a win to me.  I have doubts about spreading the
option switch over three files, though.

Re: [Qemu-devel] [PATCH 00/16] clean up vl.c code

[Jes Sorensen]

On 06/04/10 10:21, Markus Armbruster wrote:
> Jes.Sorensen@redhat.com writes:
>> I have tried to be as careful as I can to not break non Linux support,
>> but as I only have a Linux build environment handy, I would appreciate
>> it if people with other OSes could check that I didn't break anything
>> for them. In particular I would like to know if win32 still builds.
> 
> I like moving stuff out of vl.c in general.  Your moves of entire
> functions look like a win to me.  I have doubts about spreading the
> option switch over three files, though.

The problem is right now there are too many OS specific options, but
having the #ifdefs plastered all over to enable/disable them accordingly
is just a nightmare and is prone to leave in inconsistent behavior for
various OSes. See the set_proc_name() stuff for an example.

Cheers,
Jes

Re: [Qemu-devel] [PATCH 00/16] clean up vl.c code

[Markus Armbruster]

Jes Sorensen <Jes.Sorensen@redhat.com> writes:

> On 06/04/10 10:21, Markus Armbruster wrote:
>> Jes.Sorensen@redhat.com writes:
>>> I have tried to be as careful as I can to not break non Linux support,
>>> but as I only have a Linux build environment handy, I would appreciate
>>> it if people with other OSes could check that I didn't break anything
>>> for them. In particular I would like to know if win32 still builds.
>> 
>> I like moving stuff out of vl.c in general.  Your moves of entire
>> functions look like a win to me.  I have doubts about spreading the
>> option switch over three files, though.
>
> The problem is right now there are too many OS specific options, but
> having the #ifdefs plastered all over to enable/disable them accordingly
> is just a nightmare and is prone to leave in inconsistent behavior for
> various OSes. See the set_proc_name() stuff for an example.

I doubt spreading option code over separate files will help consistency.

I suspect the true root of the problem is having (too many) OS-specific
options in the first place.  What about parsing options the same
everywhere, calling out to OS-specific functions to do the actual work?
Let them fail with "can't do this on this OS".

Re: [Qemu-devel] [PATCH 00/16] clean up vl.c code

[Jes Sorensen]

On 06/04/10 13:54, Markus Armbruster wrote:
> Jes Sorensen <Jes.Sorensen@redhat.com> writes:
> 
>> On 06/04/10 10:21, Markus Armbruster wrote:
>>> I like moving stuff out of vl.c in general.  Your moves of entire
>>> functions look like a win to me.  I have doubts about spreading the
>>> option switch over three files, though.
>>
>> The problem is right now there are too many OS specific options, but
>> having the #ifdefs plastered all over to enable/disable them accordingly
>> is just a nightmare and is prone to leave in inconsistent behavior for
>> various OSes. See the set_proc_name() stuff for an example.
> 
> I doubt spreading option code over separate files will help consistency.
> 
> I suspect the true root of the problem is having (too many) OS-specific
> options in the first place.  What about parsing options the same
> everywhere, calling out to OS-specific functions to do the actual work?
> Let them fail with "can't do this on this OS".

That is a possibility which I did consider, but it would end up in far
more os specific functions for simple assignments etc. I modeled it the
way I did similar to how we handle ioctl calls in the kernel.

If there is strong feeling we should do it this way instead, I can
change the code to do it this way instead. I am not married to the
current approach, I just find it the lesser evil.

Cheers,
Jes

Re: [Qemu-devel] [PATCH 00/16] clean up vl.c code

[Markus Armbruster]

Jes Sorensen <Jes.Sorensen@redhat.com> writes:

> On 06/04/10 13:54, Markus Armbruster wrote:
>> Jes Sorensen <Jes.Sorensen@redhat.com> writes:
>> 
>>> On 06/04/10 10:21, Markus Armbruster wrote:
>>>> I like moving stuff out of vl.c in general.  Your moves of entire
>>>> functions look like a win to me.  I have doubts about spreading the
>>>> option switch over three files, though.
>>>
>>> The problem is right now there are too many OS specific options, but
>>> having the #ifdefs plastered all over to enable/disable them accordingly
>>> is just a nightmare and is prone to leave in inconsistent behavior for
>>> various OSes. See the set_proc_name() stuff for an example.
>> 
>> I doubt spreading option code over separate files will help consistency.
>> 
>> I suspect the true root of the problem is having (too many) OS-specific
>> options in the first place.  What about parsing options the same
>> everywhere, calling out to OS-specific functions to do the actual work?
>> Let them fail with "can't do this on this OS".
>
> That is a possibility which I did consider, but it would end up in far
> more os specific functions for simple assignments etc.

Far more?

option                  condition       what's needed to make it uncond.
QEMU_OPTION_smb         !_WIN32         net_slirp_smb() dummy.
QEMU_OPTION_fsdev       CONFIG_LINUX    fsdev_init_func() dummy
QEMU_OPTION_virtfs      CONFIG_LINUX    nothing, it's just short for
                                        -fsdev & -device
QEMU_OPTION_daemonize   !_WIN32         move the big #ifndef _WIN32
                                        chunk at the end of main()
                                        into a helper, + dummy
QEMU_OPTION_chroot      !_WIN32         ditto
QEMU_OPTION_runas       !_WIN32         ditto

>                                                        I modeled it the
> way I did similar to how we handle ioctl calls in the kernel.
>
> If there is strong feeling we should do it this way instead, I can
> change the code to do it this way instead. I am not married to the
> current approach, I just find it the lesser evil.

I'm making suggestions, not demands.  If you still prefer your way after
considering my suggestions, go with it.

For what it's worth, pushing OS-dependent bits out of the option parsing
allows for:

    qemu: --frobnicate: Can't frobnicate under Windows

instead of

    qemu: --frobnicate: invalid option

Re: [Qemu-devel] [PATCH 00/16] clean up vl.c code

[Jes Sorensen]

On 06/09/10 09:07, Markus Armbruster wrote:
> Jes Sorensen <Jes.Sorensen@redhat.com> writes:
>> On 06/04/10 13:54, Markus Armbruster wrote:
>> If there is strong feeling we should do it this way instead, I can
>> change the code to do it this way instead. I am not married to the
>> current approach, I just find it the lesser evil.
> 
> I'm making suggestions, not demands.  If you still prefer your way after
> considering my suggestions, go with it.
> 
> For what it's worth, pushing OS-dependent bits out of the option parsing
> allows for:
> 
>     qemu: --frobnicate: Can't frobnicate under Windows
> 
> instead of
> 
>     qemu: --frobnicate: invalid option

That is true, on the other hand I think it would be cleaner to not
advertise options on an OS where it is not supported. Ideally I would
like them to not show up in qemu --help at all.

Cheers,
Jes