Merge remote-tracking branch 'mdroth/qga-pull-2013-05-30' into staging

# By Laszlo Ersek
# Via Michael Roth
* mdroth/qga-pull-2013-05-30:
  Makefile: create ".../var/run" when installing the POSIX guest agent
  qga: save state directory in ga_install_service()
  qga: remove undefined behavior in ga_install_service()
  qga: create state directory on win32
  configure: don't save any fixed local_statedir for win32
  qga: determine default state dir and pidfile dynamically
  osdep: add qemu_get_local_state_pathname()

Message-id: 1369940341-9043-1-git-send-email-mdroth@linux.vnet.ibm.com
Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>

8 files changed, 127 insertions, 22 deletions

diff --git a/Makefile b/Makefile
index 9695c9d14d..a96736b06b 100644
--- a/Makefile
+++ b/Makefile
@@ -318,13 +318,21 @@ endif
 install-datadir:
 	$(INSTALL_DIR) "$(DESTDIR)$(qemu_datadir)"
 
+install-localstatedir:
+ifdef CONFIG_POSIX
+ifneq (,$(findstring qemu-ga,$(TOOLS)))
+	$(INSTALL_DIR) "$(DESTDIR)$(qemu_localstatedir)"/run
+endif
+endif
+
 install-confdir:
 	$(INSTALL_DIR) "$(DESTDIR)$(qemu_confdir)"
 
 install-sysconfig: install-datadir install-confdir
 	$(INSTALL_DATA) $(SRC_PATH)/sysconfigs/target/target-x86_64.conf "$(DESTDIR)$(qemu_confdir)"
 
-install: all $(if $(BUILD_DOCS),install-doc) install-sysconfig install-datadir
+install: all $(if $(BUILD_DOCS),install-doc) install-sysconfig \
+install-datadir install-localstatedir
 	$(INSTALL_DIR) "$(DESTDIR)$(bindir)"
 ifneq ($(TOOLS),)
 	$(INSTALL_PROG) $(STRIP_OPT) $(TOOLS) "$(DESTDIR)$(bindir)"
diff --git a/configure b/configure
index eb74510940..918dc36e88 100755
--- a/configure
+++ b/configure
@@ -587,7 +587,7 @@ EOF
   qemu_docdir="\${prefix}"
   bindir="\${prefix}"
   sysconfdir="\${prefix}"
-  local_statedir="\${prefix}"
+  local_statedir=
   confsuffix=""
   libs_qga="-lws2_32 -lwinmm -lpowrprof $libs_qga"
 fi
@@ -1082,7 +1082,7 @@ echo "  --docdir=PATH            install documentation in PATH$confsuffix"
 echo "  --bindir=PATH            install binaries in PATH"
 echo "  --libdir=PATH            install libraries in PATH"
 echo "  --sysconfdir=PATH        install config in PATH$confsuffix"
-echo "  --localstatedir=PATH     install local state in PATH"
+echo "  --localstatedir=PATH     install local state in PATH (set at runtime on win32)"
 echo "  --with-confsuffix=SUFFIX suffix for QEMU data inside datadir and sysconfdir [$confsuffix]"
 echo "  --enable-debug-tcg       enable TCG debugging"
 echo "  --disable-debug-tcg      disable TCG debugging (default)"
@@ -3487,10 +3487,12 @@ echo "library directory `eval echo $libdir`"
 echo "libexec directory `eval echo $libexecdir`"
 echo "include directory `eval echo $includedir`"
 echo "config directory  `eval echo $sysconfdir`"
-echo "local state directory   `eval echo $local_statedir`"
 if test "$mingw32" = "no" ; then
+echo "local state directory   `eval echo $local_statedir`"
 echo "Manual directory  `eval echo $mandir`"
 echo "ELF interp prefix $interp_prefix"
+else
+echo "local state directory   queried at runtime"
 fi
 echo "Source path       $source_path"
 echo "C compiler        $cc"
@@ -3611,7 +3613,9 @@ echo "sysconfdir=$sysconfdir" >> $config_host_mak
 echo "qemu_confdir=$qemu_confdir" >> $config_host_mak
 echo "qemu_datadir=$qemu_datadir" >> $config_host_mak
 echo "qemu_docdir=$qemu_docdir" >> $config_host_mak
-echo "qemu_localstatedir=$local_statedir" >> $config_host_mak
+if test "$mingw32" = "no" ; then
+  echo "qemu_localstatedir=$local_statedir" >> $config_host_mak
+fi
 echo "qemu_helperdir=$libexecdir" >> $config_host_mak
 echo "extra_cflags=$EXTRA_CFLAGS" >> $config_host_mak
 echo "extra_ldflags=$EXTRA_LDFLAGS" >> $config_host_mak
diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
index 57d7b1fb4d..26136f16ec 100644
--- a/include/qemu/osdep.h
+++ b/include/qemu/osdep.h
@@ -204,4 +204,15 @@ const char *qemu_get_version(void);
 void fips_set_state(bool requested);
 bool fips_get_state(void);
 
+/* Return a dynamically allocated pathname denoting a file or directory that is
+ * appropriate for storing local state.
+ *
+ * @relative_pathname need not start with a directory separator; one will be
+ * added automatically.
+ *
+ * The caller is responsible for releasing the value returned with g_free()
+ * after use.
+ */
+char *qemu_get_local_state_pathname(const char *relative_pathname);
+
 #endif
diff --git a/qga/main.c b/qga/main.c
index 44a283686b..0e04e7395c 100644
--- a/qga/main.c
+++ b/qga/main.c
@@ -45,16 +45,21 @@
 
 #ifndef _WIN32
 #define QGA_VIRTIO_PATH_DEFAULT "/dev/virtio-ports/org.qemu.guest_agent.0"
+#define QGA_STATE_RELATIVE_DIR  "run"
 #else
 #define QGA_VIRTIO_PATH_DEFAULT "\\\\.\\Global\\org.qemu.guest_agent.0"
+#define QGA_STATE_RELATIVE_DIR  "qemu-ga"
 #endif
-#define QGA_STATEDIR_DEFAULT CONFIG_QEMU_LOCALSTATEDIR "/run"
-#define QGA_PIDFILE_DEFAULT QGA_STATEDIR_DEFAULT "/qemu-ga.pid"
 #ifdef CONFIG_FSFREEZE
 #define QGA_FSFREEZE_HOOK_DEFAULT CONFIG_QEMU_CONFDIR "/fsfreeze-hook"
 #endif
 #define QGA_SENTINEL_BYTE 0xFF
 
+static struct {
+    const char *state_dir;
+    const char *pidfile;
+} dfl_pathnames;
+
 typedef struct GAPersistentState {
 #define QGA_PSTATE_DEFAULT_FD_COUNTER 1000
     int64_t fd_counter;
@@ -106,6 +111,17 @@ DWORD WINAPI service_ctrl_handler(DWORD ctrl, DWORD type, LPVOID data,
 VOID WINAPI service_main(DWORD argc, TCHAR *argv[]);
 #endif
 
+static void
+init_dfl_pathnames(void)
+{
+    g_assert(dfl_pathnames.state_dir == NULL);
+    g_assert(dfl_pathnames.pidfile == NULL);
+    dfl_pathnames.state_dir = qemu_get_local_state_pathname(
+      QGA_STATE_RELATIVE_DIR);
+    dfl_pathnames.pidfile   = qemu_get_local_state_pathname(
+      QGA_STATE_RELATIVE_DIR G_DIR_SEPARATOR_S "qemu-ga.pid");
+}
+
 static void quit_handler(int sig)
 {
     /* if we're frozen, don't exit unless we're absolutely forced to,
@@ -198,11 +214,11 @@ static void usage(const char *cmd)
 "  -h, --help        display this help and exit\n"
 "\n"
 "Report bugs to <mdroth@linux.vnet.ibm.com>\n"
-    , cmd, QEMU_VERSION, QGA_VIRTIO_PATH_DEFAULT, QGA_PIDFILE_DEFAULT,
+    , cmd, QEMU_VERSION, QGA_VIRTIO_PATH_DEFAULT, dfl_pathnames.pidfile,
 #ifdef CONFIG_FSFREEZE
     QGA_FSFREEZE_HOOK_DEFAULT,
 #endif
-    QGA_STATEDIR_DEFAULT);
+    dfl_pathnames.state_dir);
 }
 
 static const char *ga_log_level_str(GLogLevelFlags level)
@@ -908,11 +924,11 @@ int main(int argc, char **argv)
     const char *sopt = "hVvdm:p:l:f:F::b:s:t:";
     const char *method = NULL, *path = NULL;
     const char *log_filepath = NULL;
-    const char *pid_filepath = QGA_PIDFILE_DEFAULT;
+    const char *pid_filepath;
 #ifdef CONFIG_FSFREEZE
     const char *fsfreeze_hook = NULL;
 #endif
-    const char *state_dir = QGA_STATEDIR_DEFAULT;
+    const char *state_dir;
 #ifdef _WIN32
     const char *service = NULL;
 #endif
@@ -942,6 +958,10 @@ int main(int argc, char **argv)
 
     module_call_init(MODULE_INIT_QAPI);
 
+    init_dfl_pathnames();
+    pid_filepath = dfl_pathnames.pidfile;
+    state_dir = dfl_pathnames.state_dir;
+
     while ((ch = getopt_long(argc, argv, sopt, lopt, &opt_ind)) != -1) {
         switch (ch) {
         case 'm':
@@ -1002,7 +1022,16 @@ int main(int argc, char **argv)
         case 's':
             service = optarg;
             if (strcmp(service, "install") == 0) {
-                return ga_install_service(path, log_filepath);
+                const char *fixed_state_dir;
+
+                /* If the user passed the "-t" option, we save that state dir
+                 * in the service. Otherwise we let the service fetch the state
+                 * dir from the environment when it starts.
+                 */
+                fixed_state_dir = (state_dir == dfl_pathnames.state_dir) ?
+                                  NULL :
+                                  state_dir;
+                return ga_install_service(path, log_filepath, fixed_state_dir);
             } else if (strcmp(service, "uninstall") == 0) {
                 return ga_uninstall_service();
             } else {
@@ -1021,6 +1050,20 @@ int main(int argc, char **argv)
         }
     }
 
+#ifdef _WIN32
+    /* On win32 the state directory is application specific (be it the default
+     * or a user override). We got past the command line parsing; let's create
+     * the directory (with any intermediate directories). If we run into an
+     * error later on, we won't try to clean up the directory, it is considered
+     * persistent.
+     */
+    if (g_mkdir_with_parents(state_dir, S_IRWXU) == -1) {
+        g_critical("unable to create (an ancestor of) the state directory"
+                   " '%s': %s", state_dir, strerror(errno));
+        return EXIT_FAILURE;
+    }
+#endif
+
     s = g_malloc0(sizeof(GAState));
     s->log_level = log_level;
     s->log_file = stderr;
diff --git a/qga/service-win32.c b/qga/service-win32.c
index 843398a6c6..02926abb28 100644
--- a/qga/service-win32.c
+++ b/qga/service-win32.c
@@ -35,38 +35,44 @@ static int printf_win_error(const char *text)
     return n;
 }
 
-int ga_install_service(const char *path, const char *logfile)
+int ga_install_service(const char *path, const char *logfile,
+                       const char *state_dir)
 {
     SC_HANDLE manager;
     SC_HANDLE service;
-    TCHAR cmdline[MAX_PATH];
+    TCHAR module_fname[MAX_PATH];
+    GString *cmdline;
 
-    if (GetModuleFileName(NULL, cmdline, MAX_PATH) == 0) {
+    if (GetModuleFileName(NULL, module_fname, MAX_PATH) == 0) {
         printf_win_error("No full path to service's executable");
         return EXIT_FAILURE;
     }
 
-    _snprintf(cmdline, MAX_PATH - strlen(cmdline), "%s -d", cmdline);
+    cmdline = g_string_new(module_fname);
+    g_string_append(cmdline, " -d");
 
     if (path) {
-        _snprintf(cmdline, MAX_PATH - strlen(cmdline), "%s -p %s", cmdline, path);
+        g_string_append_printf(cmdline, " -p %s", path);
     }
     if (logfile) {
-        _snprintf(cmdline, MAX_PATH - strlen(cmdline), "%s -l %s -v",
-            cmdline, logfile);
+        g_string_append_printf(cmdline, " -l %s -v", logfile);
+    }
+    if (state_dir) {
+        g_string_append_printf(cmdline, " -t %s", state_dir);
     }
 
-    g_debug("service's cmdline: %s", cmdline);
+    g_debug("service's cmdline: %s", cmdline->str);
 
     manager = OpenSCManager(NULL, NULL, SC_MANAGER_ALL_ACCESS);
     if (manager == NULL) {
         printf_win_error("No handle to service control manager");
+        g_string_free(cmdline, TRUE);
         return EXIT_FAILURE;
     }
 
     service = CreateService(manager, QGA_SERVICE_NAME, QGA_SERVICE_DISPLAY_NAME,
         SERVICE_ALL_ACCESS, SERVICE_WIN32_OWN_PROCESS, SERVICE_AUTO_START,
-        SERVICE_ERROR_NORMAL, cmdline, NULL, NULL, NULL, NULL, NULL);
+        SERVICE_ERROR_NORMAL, cmdline->str, NULL, NULL, NULL, NULL, NULL);
 
     if (service) {
         SERVICE_DESCRIPTION desc = { (char *)QGA_SERVICE_DESCRIPTION };
@@ -80,6 +86,7 @@ int ga_install_service(const char *path, const char *logfile)
     CloseServiceHandle(service);
     CloseServiceHandle(manager);
 
+    g_string_free(cmdline, TRUE);
     return (service == NULL);
 }
 
diff --git a/qga/service-win32.h b/qga/service-win32.h
index 99dfc53348..3b9e87024b 100644
--- a/qga/service-win32.h
+++ b/qga/service-win32.h
@@ -24,7 +24,8 @@ typedef struct GAService {
     SERVICE_STATUS_HANDLE status_handle;
 } GAService;
 
-int ga_install_service(const char *path, const char *logfile);
+int ga_install_service(const char *path, const char *logfile,
+                       const char *state_dir);
 int ga_uninstall_service(void);
 
 #endif
diff --git a/util/oslib-posix.c b/util/oslib-posix.c
index 631a1dea33..3dc8b1b074 100644
--- a/util/oslib-posix.c
+++ b/util/oslib-posix.c
@@ -47,6 +47,8 @@ extern int daemon(int, int);
 #  define QEMU_VMALLOC_ALIGN getpagesize()
 #endif
 
+#include <glib/gprintf.h>
+
 #include "config-host.h"
 #include "sysemu/sysemu.h"
 #include "trace.h"
@@ -232,3 +234,10 @@ int qemu_utimens(const char *path, const struct timespec *times)
 
     return utimes(path, &tv[0]);
 }
+
+char *
+qemu_get_local_state_pathname(const char *relative_pathname)
+{
+    return g_strdup_printf("%s/%s", CONFIG_QEMU_LOCALSTATEDIR,
+                           relative_pathname);
+}
diff --git a/util/oslib-win32.c b/util/oslib-win32.c
index df2ecbdffb..961fbf5e3d 100644
--- a/util/oslib-win32.c
+++ b/util/oslib-win32.c
@@ -26,12 +26,17 @@
  * THE SOFTWARE.
  */
 #include <windows.h>
+#include <glib.h>
+#include <stdlib.h>
 #include "config-host.h"
 #include "sysemu/sysemu.h"
 #include "qemu/main-loop.h"
 #include "trace.h"
 #include "qemu/sockets.h"
 
+/* this must come after including "trace.h" */
+#include <shlobj.h>
+
 void *qemu_oom_check(void *ptr)
 {
     if (ptr == NULL) {
@@ -160,3 +165,20 @@ int qemu_get_thread_id(void)
 {
     return GetCurrentThreadId();
 }
+
+char *
+qemu_get_local_state_pathname(const char *relative_pathname)
+{
+    HRESULT result;
+    char base_path[MAX_PATH+1] = "";
+
+    result = SHGetFolderPath(NULL, CSIDL_COMMON_APPDATA, NULL,
+                             /* SHGFP_TYPE_CURRENT */ 0, base_path);
+    if (result != S_OK) {
+        /* misconfigured environment */
+        g_critical("CSIDL_COMMON_APPDATA unavailable: %ld", (long)result);
+        abort();
+    }
+    return g_strdup_printf("%s" G_DIR_SEPARATOR_S "%s", base_path,
+                           relative_pathname);
+}