From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:52529) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZUHRT-0004QC-0B for qemu-devel@nongnu.org; Tue, 25 Aug 2015 12:51:24 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZUHRP-0004kd-HY for qemu-devel@nongnu.org; Tue, 25 Aug 2015 12:51:22 -0400 Received: from e34.co.us.ibm.com ([32.97.110.152]:33218) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZUHRP-0004kS-8H for qemu-devel@nongnu.org; Tue, 25 Aug 2015 12:51:19 -0400 Received: from /spool/local by e34.co.us.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Tue, 25 Aug 2015 10:51:18 -0600 Received: from b03cxnp08028.gho.boulder.ibm.com (b03cxnp08028.gho.boulder.ibm.com [9.17.130.20]) by d03dlp02.boulder.ibm.com (Postfix) with ESMTP id E49393E40041 for ; Tue, 25 Aug 2015 10:51:15 -0600 (MDT) Received: from d03av04.boulder.ibm.com (d03av04.boulder.ibm.com [9.17.195.170]) by b03cxnp08028.gho.boulder.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id t7PGoGeb35061890 for ; Tue, 25 Aug 2015 09:50:16 -0700 Received: from d03av04.boulder.ibm.com (loopback [127.0.0.1]) by d03av04.boulder.ibm.com (8.14.4/8.14.4/NCO v10.0 AVout) with ESMTP id t7PGpF7L027716 for ; Tue, 25 Aug 2015 10:51:15 -0600 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable From: Michael Roth In-Reply-To: <1435751267-26378-9-git-send-email-marcandre.lureau@gmail.com> References: <1435751267-26378-1-git-send-email-marcandre.lureau@gmail.com> <1435751267-26378-9-git-send-email-marcandre.lureau@gmail.com> Message-ID: <20150825165108.11069.21240@loki> Date: Tue, 25 Aug 2015 11:51:08 -0500 Subject: Re: [Qemu-devel] [PATCH 08/12] qga: move agent run in a seperate function List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: =?utf-8?q?Marc-Andr=C3=A9_Lureau?= , qemu-devel@nongnu.org Quoting Marc-Andr=C3=A9 Lureau (2015-07-01 06:47:43) > Once the options are populated, move the running state to > a run_agent() function. > = > Signed-off-by: Marc-Andr=C3=A9 Lureau > --- > qga/main.c | 123 +++++++++++++++++++++++++++++++++----------------------= ------ > 1 file changed, 67 insertions(+), 56 deletions(-) > = > diff --git a/qga/main.c b/qga/main.c > index 5575637..aaf0e10 100644 > --- a/qga/main.c > +++ b/qga/main.c > @@ -1042,39 +1042,13 @@ static void option_parse(int argc, char **argv) > } > } > = > -int main(int argc, char **argv) > +static int run_agent(GAState *s) > { > - GAState *s; > - > - module_call_init(MODULE_INIT_QAPI); > - > - init_dfl_pathnames(); > - option_parse(argc, argv); > - > - if (pid_filepath =3D=3D NULL) { > - pid_filepath =3D g_strdup(dfl_pathnames.pidfile); > - } > - > - if (state_dir =3D=3D NULL) { > - state_dir =3D g_strdup(dfl_pathnames.state_dir); > - } > - > - if (method =3D=3D NULL) { > - method =3D g_strdup("virtio-serial"); > - } > + ga_state =3D s; > = > - if (device_path =3D=3D NULL) { > - if (strcmp(method, "virtio-serial") =3D=3D 0) { > - /* try the default path for the virtio-serial port */ > - device_path =3D g_strdup(QGA_VIRTIO_PATH_DEFAULT); > - } else if (strcmp(method, "isa-serial") =3D=3D 0) { > - /* try the default path for the serial port - COM1 */ > - device_path =3D g_strdup(QGA_SERIAL_PATH_DEFAULT); > - } else { > - g_critical("must specify a path for this channel"); > - goto out_bad; > - } > - } > + g_log_set_default_handler(ga_log, s); > + g_log_set_fatal_mask(NULL, G_LOG_LEVEL_ERROR); > + ga_enable_logging(s); > = > #ifdef _WIN32 > /* On win32 the state directory is application specific (be it the d= efault > @@ -1090,20 +1064,6 @@ int main(int argc, char **argv) > } > #endif > = > - s =3D g_malloc0(sizeof(GAState)); > - s->log_level =3D log_level; > - s->log_file =3D stderr; > -#ifdef CONFIG_FSFREEZE > - s->fsfreeze_hook =3D fsfreeze_hook; > -#endif > - g_log_set_default_handler(ga_log, s); > - g_log_set_fatal_mask(NULL, G_LOG_LEVEL_ERROR); > - ga_enable_logging(s); > - s->state_filepath_isfrozen =3D g_strdup_printf("%s/qga.state.isfroze= n", > - state_dir); > - s->pstate_filepath =3D g_strdup_printf("%s/qga.state", state_dir); > - s->frozen =3D false; > - > #ifndef _WIN32 > /* check if a previous instance of qemu-ga exited with filesystems' = state > * marked as frozen. this could be a stale value (a non-qemu-ga proc= ess > @@ -1154,7 +1114,7 @@ int main(int argc, char **argv) > if (!log_file) { > g_critical("unable to open specified log file: %s", > strerror(errno)); > - goto out_bad; > + return EXIT_FAILURE; > } > s->log_file =3D log_file; > } > @@ -1165,7 +1125,7 @@ int main(int argc, char **argv) > s->pstate_filepath, > ga_is_frozen(s))) { > g_critical("failed to load persistent state"); > - goto out_bad; > + return EXIT_FAILURE; > } > = > blacklist =3D ga_command_blacklist_init(blacklist); > @@ -1185,14 +1145,14 @@ int main(int argc, char **argv) > #ifndef _WIN32 > if (!register_signal_handlers()) { > g_critical("failed to register signal handlers"); > - goto out_bad; > + return EXIT_FAILURE; > } > #endif > = > s->main_loop =3D g_main_loop_new(NULL, false); > if (!channel_init(ga_state, method, device_path)) { > g_critical("failed to initialize guest agent channel"); > - goto out_bad; > + return EXIT_FAILURE; > } > #ifndef _WIN32 > g_main_loop_run(ga_state->main_loop); > @@ -1206,15 +1166,65 @@ int main(int argc, char **argv) > } > #endif > = > - ga_command_state_cleanup_all(ga_state->command_state); > - ga_channel_free(ga_state->channel); > + return EXIT_SUCCESS; > +} > = > - if (daemonize) { > - unlink(pid_filepath); > +int main(int argc, char **argv) > +{ > + int ret =3D EXIT_SUCCESS; > + GAState *s =3D g_new0(GAState, 1); > + > + module_call_init(MODULE_INIT_QAPI); > + > + init_dfl_pathnames(); > + option_parse(argc, argv); > + > + if (pid_filepath =3D=3D NULL) { > + pid_filepath =3D g_strdup(dfl_pathnames.pidfile); > + } > + > + if (state_dir =3D=3D NULL) { > + state_dir =3D g_strdup(dfl_pathnames.state_dir); > + } > + > + if (method =3D=3D NULL) { > + method =3D g_strdup("virtio-serial"); > + } > + > + if (device_path =3D=3D NULL) { > + if (strcmp(method, "virtio-serial") =3D=3D 0) { > + /* try the default path for the virtio-serial port */ > + device_path =3D g_strdup(QGA_VIRTIO_PATH_DEFAULT); > + } else if (strcmp(method, "isa-serial") =3D=3D 0) { > + /* try the default path for the serial port - COM1 */ > + device_path =3D g_strdup(QGA_SERIAL_PATH_DEFAULT); > + } else { > + g_critical("must specify a path for this channel"); > + ret =3D EXIT_FAILURE; > + goto end; > + } > + } > + > + s->log_level =3D log_level; > + s->log_file =3D stderr; > +#ifdef CONFIG_FSFREEZE > + s->fsfreeze_hook =3D fsfreeze_hook; > +#endif > + s->state_filepath_isfrozen =3D g_strdup_printf("%s/qga.state.isfroze= n", > + state_dir); > + s->pstate_filepath =3D g_strdup_printf("%s/qga.state", state_dir); > + s->frozen =3D false; It's hard to draw the line between what should be in main() as opposed to run_agent(), but we have some s->frozen initialization happening here, and then the statefile-based setting s->frozen values being set in run_agent(), so at least in this case we should move those back to main(). That means we probabably have to move the checks for the statefile existence/creation back to main() as well since s->frozen depends on it. I think that's the right place anyway, makes sense to load/init the persistent state file in the same location we handle the config file. > + > + ret =3D run_agent(s); > + > +end: > + if (s->command_state) { > + ga_command_state_cleanup_all(s->command_state); > + } > + if (s->channel) { > + ga_channel_free(s->channel); > } > - return 0; > = > -out_bad: > if (daemonize) { > unlink(pid_filepath); > } > @@ -1227,6 +1237,7 @@ out_bad: > #ifdef CONFIG_FSFREEZE > g_free(fsfreeze_hook); > #endif > + g_free(s); > = > - return EXIT_FAILURE; > + return ret; > } > -- = > 2.4.3 >=20