Re: [PATCH v2 7/8] softmmu: move parsing of -runas, -chroot and -daemonize code

2022-03-04 Thread Eric Blake
On Fri, Mar 04, 2022 at 06:56:19PM +, Daniel P. Berrangé wrote:
> With the future intent to try to move to a fully QAPI driven
> configuration system, we want to have any current command
> parsing well isolated from logic that applies the resulting
> configuration.
> 
> We also don't want os-posix.c to contain code that is specific
> to the system emulators, as this file is linked to other binaries
> too.
> 
> To satisfy these goals, we move parsing of the -runas, -chroot and
> -daemonize code out of the os-posix.c helper code, and pass the
> parsed data into APIs in os-posix.c.
> 
> As a side benefit the 'os_daemonize()' function now lives up to
> its name and will always daemonize, instead of using global state
> to decide to be a no-op sometimes.
> 
> Signed-off-by: Daniel P. Berrangé 
> ---
>  include/sysemu/os-posix.h |   4 +-
>  include/sysemu/os-win32.h |   1 -
>  os-posix.c| 148 +++---
>  os-win32.c|   9 ---
>  softmmu/vl.c  |  86 ++
>  5 files changed, 117 insertions(+), 131 deletions(-)

> @@ -2780,6 +2811,14 @@ void qemu_init(int argc, char **argv, char **envp)
>  MachineClass *machine_class;
>  bool userconfig = true;
>  FILE *vmstate_dump_file = NULL;
> +bool daemonize = false;

Given this declaration,...

> +#ifndef WIN32
> +struct passwd *pwd;
> +uid_t runas_uid = -1;
> +gid_t runas_gid = -1;
> +g_autofree char *runas_name = NULL;
> +const char *chroot_dir = NULL;
> +#endif /* !WIN32 */
>  
>  qemu_add_opts(&qemu_drive_opts);
>  qemu_add_drive_opts(&qemu_legacy_drive_opts);
> @@ -3661,15 +3700,34 @@ void qemu_init(int argc, char **argv, char **envp)
>  case QEMU_OPTION_nouserconfig:
>  /* Nothing to be parsed here. Especially, do not error out 
> below. */
>  break;
> -default:
> -if (os_parse_cmd_args(popt->index, optarg)) {
> -error_report("Option not supported in this build");
> +#ifndef WIN32
> +case QEMU_OPTION_runas:
> +pwd = getpwnam(optarg);
> +if (pwd) {
> +runas_uid = pwd->pw_uid;
> +runas_gid = pwd->pw_gid;
> +runas_name = g_strdup(pwd->pw_name);
> +} else if (!os_parse_runas_uid_gid(optarg,
> +   &runas_uid,
> +   &runas_gid)) {
> +error_report("User \"%s\" doesn't exist"
> + " (and is not :)",
> + optarg);
>  exit(1);
>  }
> -if (is_daemonized()) {
> -qemu_log_stdio_disable();
> -qemu_chr_stdio_disable();
> -}
> +break;
> +case QEMU_OPTION_chroot:
> +chroot_dir = optarg;
> +break;
> +case QEMU_OPTION_daemonize:
> +daemonize = 1;

...this should s/1/true/. With that tweak,

Reviewed-by: Eric Blake 

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3266
Virtualization:  qemu.org | libvirt.org



[PATCH v2 7/8] softmmu: move parsing of -runas, -chroot and -daemonize code

2022-03-04 Thread Daniel P . Berrangé
With the future intent to try to move to a fully QAPI driven
configuration system, we want to have any current command
parsing well isolated from logic that applies the resulting
configuration.

We also don't want os-posix.c to contain code that is specific
to the system emulators, as this file is linked to other binaries
too.

To satisfy these goals, we move parsing of the -runas, -chroot and
-daemonize code out of the os-posix.c helper code, and pass the
parsed data into APIs in os-posix.c.

As a side benefit the 'os_daemonize()' function now lives up to
its name and will always daemonize, instead of using global state
to decide to be a no-op sometimes.

Signed-off-by: Daniel P. Berrangé 
---
 include/sysemu/os-posix.h |   4 +-
 include/sysemu/os-win32.h |   1 -
 os-posix.c| 148 +++---
 os-win32.c|   9 ---
 softmmu/vl.c  |  86 ++
 5 files changed, 117 insertions(+), 131 deletions(-)

diff --git a/include/sysemu/os-posix.h b/include/sysemu/os-posix.h
index 2edf33658a..390f3f8396 100644
--- a/include/sysemu/os-posix.h
+++ b/include/sysemu/os-posix.h
@@ -46,7 +46,9 @@ 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);
+void os_setup_post(const char *chroot_dir,
+   uid_t uid, gid_t gid,
+   const char *username);
 int os_mlock(void);
 
 #define closesocket(s) close(s)
diff --git a/include/sysemu/os-win32.h b/include/sysemu/os-win32.h
index 43f569b5c2..4879f8731d 100644
--- a/include/sysemu/os-win32.h
+++ b/include/sysemu/os-win32.h
@@ -61,7 +61,6 @@ struct tm *localtime_r(const time_t *timep, struct tm 
*result);
 
 static inline void os_setup_signal_handling(void) {}
 static inline void os_daemonize(void) {}
-static inline void os_setup_post(void) {}
 void os_set_line_buffering(void);
 static inline void os_set_proc_name(const char *dummy) {}
 
diff --git a/os-posix.c b/os-posix.c
index 30da1a1491..f598a52a4f 100644
--- a/os-posix.c
+++ b/os-posix.c
@@ -42,11 +42,6 @@
 #include 
 #endif
 
-static char *user_name;
-static uid_t user_uid = (uid_t)-1;
-static gid_t user_gid = (gid_t)-1;
-
-static const char *chroot_dir;
 static int daemonize;
 static int daemon_pipe;
 
@@ -96,69 +91,6 @@ void os_set_proc_name(const char *s)
 }
 
 
-static bool os_parse_runas_uid_gid(const char *optarg,
-   uid_t *runas_uid, gid_t *runas_gid)
-{
-unsigned long lv;
-const char *ep;
-uid_t got_uid;
-gid_t got_gid;
-int rc;
-
-rc = qemu_strtoul(optarg, &ep, 0, &lv);
-got_uid = lv; /* overflow here is ID in C99 */
-if (rc || *ep != ':' || got_uid != lv || got_uid == (uid_t)-1) {
-return false;
-}
-
-rc = qemu_strtoul(ep + 1, 0, 0, &lv);
-got_gid = lv; /* overflow here is ID in C99 */
-if (rc || got_gid != lv || got_gid == (gid_t)-1) {
-return false;
-}
-
-*runas_uid = got_uid;
-*runas_gid = got_gid;
-return true;
-}
-
-/*
- * Parse OS specific command line options.
- * return 0 if option handled, -1 otherwise
- */
-int os_parse_cmd_args(int index, const char *optarg)
-{
-struct passwd *user_pwd;
-
-switch (index) {
-case QEMU_OPTION_runas:
-user_pwd = getpwnam(optarg);
-if (user_pwd) {
-user_uid = user_pwd->pw_uid;
-user_gid = user_pwd->pw_gid;
-user_name = g_strdup(user_pwd->pw_name);
-} else if (!os_parse_runas_uid_gid(optarg,
-   &user_uid,
-   &user_gid)) {
-error_report("User \"%s\" doesn't exist"
- " (and is not :)",
- optarg);
-exit(1);
-}
-break;
-case QEMU_OPTION_chroot:
-chroot_dir = optarg;
-break;
-case QEMU_OPTION_daemonize:
-daemonize = 1;
-break;
-default:
-return -1;
-}
-
-return 0;
-}
-
 static void change_process_uid(uid_t uid, gid_t gid, const char *name)
 {
 if (setgid(gid) < 0) {
@@ -202,54 +134,56 @@ static void change_root(const char *root)
 
 void os_daemonize(void)
 {
-if (daemonize) {
-pid_t pid;
-int fds[2];
+pid_t pid;
+int fds[2];
 
-if (pipe(fds) == -1) {
-exit(1);
-}
+if (pipe(fds) == -1) {
+exit(1);
+}
 
-pid = fork();
-if (pid > 0) {
-uint8_t status;
-ssize_t len;
+pid = fork();
+if (pid > 0) {
+uint8_t status;
+ssize_t len;
 
-close(fds[1]);
+close(fds[1]);
 
-do {
-len = read(fds[0], &status, 1);
-} while (len < 0 && errno == EINTR);
+do {
+len = read(fds[0], &status, 1);
+} while (len < 0 && errno == EINTR);
 
-/* only