Re: [Qemu-devel] [PATCH 2/7] qga: hang GAConfig/socket_activation off of GAState global

[Marc-André Lureau]

Hi

On Thu, Sep 27, 2018 at 11:40 AM Bishara AbuHattoum <address@hidden> wrote:
>
> From: Michael Roth <address@hidden>
>
> For w32 services we rely on the global GAState to access resources
> associated with the agent within service_main(). Currently this is
> sufficient for starting the agent since we open the channel once prior
> to calling service_main(), and simply start the GMainLoop to start the
> agent from within service_main().
>
> Eventually we want to be able to also [re-]open the communication
> channel from within service_main(), which requires access to
> config/socket_activation variables, so we hang them off GAState in
> preparation for that.
>
> Signed-off-by: Michael Roth <address@hidden>
> Message-Id: <address@hidden>

I am not sure you should keep the original message-id.

> Signed-off-by: Sameeh Jubran <address@hidden>
> ---
>  qga/main.c | 54 +++++++++++++++++++++++++++++-------------------------
>  1 file changed, 29 insertions(+), 25 deletions(-)
>
> diff --git a/qga/main.c b/qga/main.c
> index 7b7b837a14..7381053a0f 100644
> --- a/qga/main.c
> +++ b/qga/main.c
> @@ -69,6 +69,25 @@ typedef struct GAPersistentState {
>      int64_t fd_counter;
>  } GAPersistentState;
>
> +typedef struct GAConfig {
> +    char *channel_path;
> +    char *method;
> +    char *log_filepath;
> +    char *pid_filepath;
> +#ifdef CONFIG_FSFREEZE
> +    char *fsfreeze_hook;
> +#endif
> +    char *state_dir;
> +#ifdef _WIN32
> +    const char *service;
> +#endif
> +    gchar *bliststr; /* blacklist may point to this string */
> +    GList *blacklist;
> +    int daemonize;
> +    GLogLevelFlags log_level;
> +    int dumpconf;
> +} GAConfig;
> +
>  struct GAState {
>      JSONMessageParser parser;
>      GMainLoop *main_loop;
> @@ -94,6 +113,8 @@ struct GAState {
>  #endif
>      gchar *pstate_filepath;
>      GAPersistentState pstate;
> +    GAConfig *config;
> +    int socket_activation;
>  };
>
>  struct GAState *ga_state;
> @@ -939,25 +960,6 @@ static GList *split_list(const gchar *str, const gchar 
> *delim)
>      return list;
>  }
>
> -typedef struct GAConfig {
> -    char *channel_path;
> -    char *method;
> -    char *log_filepath;
> -    char *pid_filepath;
> -#ifdef CONFIG_FSFREEZE
> -    char *fsfreeze_hook;
> -#endif
> -    char *state_dir;
> -#ifdef _WIN32
> -    const char *service;
> -#endif
> -    gchar *bliststr; /* blacklist may point to this string */
> -    GList *blacklist;
> -    int daemonize;
> -    GLogLevelFlags log_level;
> -    int dumpconf;
> -} GAConfig;
> -

I would avoid moving the struct by simply moving the typedef.

>  static void config_load(GAConfig *config)
>  {
>      GError *gerr = NULL;
> @@ -1245,7 +1247,7 @@ static bool check_is_frozen(GAState *s)
>      return false;
>  }
>
> -static GAState *initialize_agent(GAConfig *config)
> +static GAState *initialize_agent(GAConfig *config, int socket_activation)
>  {
>      GAState *s = g_new0(GAState, 1);
>
> @@ -1338,6 +1340,8 @@ static GAState *initialize_agent(GAConfig *config)
>
>      s->main_loop = g_main_loop_new(NULL, false);
>
> +    s->config = config;
> +    s->socket_activation = socket_activation;
>      ga_state = s;
>      return s;
>  }
> @@ -1361,10 +1365,10 @@ static void cleanup_agent(GAState *s)
>      ga_state = NULL;
>  }
>
> -static int run_agent(GAState *s, GAConfig *config, int socket_activation)
> +static int run_agent(GAState *s)
>  {
> -    if (!channel_init(ga_state, config->method, config->channel_path,
> -                      socket_activation ? FIRST_SOCKET_ACTIVATION_FD : -1)) {
> +    if (!channel_init(s, s->config->method, s->config->channel_path,
> +                      s->socket_activation ? FIRST_SOCKET_ACTIVATION_FD : 
> -1)) {
>          g_critical("failed to initialize guest agent channel");
>          return EXIT_FAILURE;
>      }
> @@ -1459,12 +1463,12 @@ int main(int argc, char **argv)
>          goto end;
>      }
>
> -    s = initialize_agent(config);
> +    s = initialize_agent(config, socket_activation);
>      if (!s) {
>          g_critical("error initializing guest agent");
>          goto end;
>      }
> -    ret = run_agent(s, config, socket_activation);
> +    ret = run_agent(s);
>      cleanup_agent(s);
>
>  end:
> --
> 2.17.0
>
>


--
Marc-André Lureau