Re: [PATCH 5/6] qga: Add timeout for fsfreeze

[Alexander Ivanov <alexander.ivanov@virtuozzo.com>]

On 10/26/23 11:16, Konstantin Kostiuk wrote:
>
> I think it is better to check that timeout <= 10 sec in the case of 
> Windows.
> Anyway this is a VSS limitation and FS will be unfrozen earlier if 
> timeout > 10 sec,
> this can cause some misunderstanding from a user.
Thank you, will pay attention.
>
> timeout option sounds good in the guest-fsfreeze-freeze command.
> In guest-fsfreeze-freeze-list, it looks strange to me. Command returns
> list but takes timeout option. Can you explain timeout usage in this 
> command?
The second command doesn't return a list, it takes a list of mountpoints.
Both of the commands freeze local guest filesystems. The first one 
freezes all the FS,
the second one freeze only FS from the given list.
>
> On Wed, Oct 25, 2023 at 5:01 PM Alexander Ivanov 
> <alexander.ivanov@virtuozzo.com> wrote:
>
>     In some cases it would be useful to thaw a filesystem by timeout after
>     freezing this filesystem by guest-fsfreeze-freeze-list. Add an
>     optional
>     argument "timeout" to the command.
>
>     Signed-off-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com>
>     ---
>      qga/commands-posix.c   | 21 ++++++++++++++++++---
>      qga/commands-win32.c   | 16 ++++++++++++++--
>      qga/guest-agent-core.h |  3 ++-
>      qga/main.c             | 19 ++++++++++++++++++-
>      qga/qapi-schema.json   |  9 ++++++++-
>      5 files changed, 60 insertions(+), 8 deletions(-)
>
>     diff --git a/qga/commands-posix.c b/qga/commands-posix.c
>     index 26711a1a72..e8a79e0a41 100644
>     --- a/qga/commands-posix.c
>     +++ b/qga/commands-posix.c
>     @@ -707,13 +707,17 @@ GuestFsfreezeStatus
>     qmp_guest_fsfreeze_status(Error **errp)
>          return GUEST_FSFREEZE_STATUS_THAWED;
>      }
>
>     -int64_t qmp_guest_fsfreeze_freeze(Error **errp)
>     +int64_t qmp_guest_fsfreeze_freeze(bool has_timeout, int64_t timeout,
>     +                                  Error **errp)
>      {
>     -    return qmp_guest_fsfreeze_freeze_list(false, NULL, errp);
>     +    return qmp_guest_fsfreeze_freeze_list(false, NULL,
>     has_timeout, timeout,
>     +                                          errp);
>      }
>
>      int64_t qmp_guest_fsfreeze_freeze_list(bool has_mountpoints,
>                                             strList *mountpoints,
>     +                                       bool has_timeout,
>     +                                       int64_t timeout,
>                                             Error **errp)
>      {
>          int ret;
>     @@ -734,8 +738,11 @@ int64_t qmp_guest_fsfreeze_freeze_list(bool
>     has_mountpoints,
>              return -1;
>          }
>
>     +    if (!has_timeout || timeout < 0) {
>     +        timeout = 0;
>     +    }
>          /* cannot risk guest agent blocking itself on a write in this
>     state */
>     -    ga_set_frozen(ga_state);
>     +    ga_set_frozen(ga_state, timeout);
>
>          ret = qmp_guest_fsfreeze_do_freeze_list(has_mountpoints,
>     mountpoints,
>                                                  mounts, errp);
>     @@ -780,6 +787,12 @@ static void guest_fsfreeze_cleanup(void)
>              }
>          }
>      }
>     +
>     +gboolean ga_frozen_timeout_cb(gpointer data)
>     +{
>     +    guest_fsfreeze_cleanup();
>     +    return G_SOURCE_REMOVE;
>     +}
>      #endif
>
>      /* linux-specific implementations. avoid this if at all possible. */
>     @@ -3119,6 +3132,8 @@ int64_t qmp_guest_fsfreeze_freeze(Error **errp)
>
>      int64_t qmp_guest_fsfreeze_freeze_list(bool has_mountpoints,
>                                             strList *mountpoints,
>     +                                       bool has_timeout,
>     +                                       int64_t timeout,
>                                             Error **errp)
>      {
>          error_setg(errp, QERR_UNSUPPORTED);
>     diff --git a/qga/commands-win32.c b/qga/commands-win32.c
>     index 618d862c00..51fd6dcd58 100644
>     --- a/qga/commands-win32.c
>     +++ b/qga/commands-win32.c
>     @@ -1221,13 +1221,16 @@ GuestFsfreezeStatus
>     qmp_guest_fsfreeze_status(Error **errp)
>       * Freeze local file systems using Volume Shadow-copy Service.
>       * The frozen state is limited for up to 10 seconds by VSS.
>       */
>     -int64_t qmp_guest_fsfreeze_freeze(Error **errp)
>     +int64_t qmp_guest_fsfreeze_freeze(bool has_timeout, int64_t timeout,
>     +                                  Error **errp)
>      {
>          return qmp_guest_fsfreeze_freeze_list(false, NULL, errp);
>      }
>
>      int64_t qmp_guest_fsfreeze_freeze_list(bool has_mountpoints,
>                                             strList *mountpoints,
>     +                                       bool has_timeout,
>     +                                       int64_t timeout,
>                                             Error **errp)
>      {
>          int i;
>     @@ -1240,8 +1243,11 @@ int64_t qmp_guest_fsfreeze_freeze_list(bool
>     has_mountpoints,
>
>          slog("guest-fsfreeze called");
>
>     +    if (!has_timeout || timeout < 0) {
>     +        timeout = 0;
>     +    }
>          /* cannot risk guest agent blocking itself on a write in this
>     state */
>     -    ga_set_frozen(ga_state);
>     +    ga_set_frozen(ga_state, timeout);
>
>          qga_vss_fsfreeze(&i, true, mountpoints, &local_err);
>          if (local_err) {
>     @@ -1299,6 +1305,12 @@ static void guest_fsfreeze_cleanup(void)
>          vss_deinit(true);
>      }
>
>     +gboolean ga_frozen_timeout_cb(gpointer data)
>     +{
>     +    guest_fsfreeze_cleanup();
>     +    return G_SOURCE_REMOVE;
>     +}
>     +
>      /*
>       * Walk list of mounted file systems in the guest, and discard unused
>       * areas.
>     diff --git a/qga/guest-agent-core.h b/qga/guest-agent-core.h
>     index b4e7c52c61..d8d1bb9505 100644
>     --- a/qga/guest-agent-core.h
>     +++ b/qga/guest-agent-core.h
>     @@ -39,8 +39,9 @@ void ga_enable_logging(GAState *s);
>      void G_GNUC_PRINTF(1, 2) slog(const gchar *fmt, ...);
>      void ga_set_response_delimited(GAState *s);
>      bool ga_is_frozen(GAState *s);
>     -void ga_set_frozen(GAState *s);
>     +void ga_set_frozen(GAState *s, int64_t timeout);
>      void ga_unset_frozen(GAState *s);
>     +gboolean ga_frozen_timeout_cb(gpointer data);
>      const char *ga_fsfreeze_hook(GAState *s);
>      int64_t ga_get_fd_handle(GAState *s, Error **errp);
>      int ga_parse_whence(GuestFileWhence *whence, Error **errp);
>     diff --git a/qga/main.c b/qga/main.c
>     index 8668b9f3d3..6c7c7d68d8 100644
>     --- a/qga/main.c
>     +++ b/qga/main.c
>     @@ -94,6 +94,7 @@ struct GAState {
>              const char *pid_filepath;
>          } deferred_options;
>      #ifdef CONFIG_FSFREEZE
>     +    guint frozen_timeout_id;
>          const char *fsfreeze_hook;
>      #endif
>          gchar *pstate_filepath;
>     @@ -478,7 +479,7 @@ bool ga_is_frozen(GAState *s)
>          return s->frozen;
>      }
>
>     -void ga_set_frozen(GAState *s)
>     +void ga_set_frozen(GAState *s, int64_t timeout)
>      {
>          if (ga_is_frozen(s)) {
>              return;
>     @@ -492,6 +493,15 @@ void ga_set_frozen(GAState *s)
>              g_warning("unable to create %s, fsfreeze may not function
>     properly",
>                        s->state_filepath_isfrozen);
>          }
>     +#ifdef CONFIG_FSFREEZE
>     +    if (timeout) {
>     +        s->frozen_timeout_id = g_timeout_add_seconds(timeout,
>     +  ga_frozen_timeout_cb,
>     +                                                     NULL);
>     +    } else {
>     +        s->frozen_timeout_id = 0;
>     +    }
>     +#endif
>      }
>
>      void ga_unset_frozen(GAState *s)
>     @@ -500,6 +510,13 @@ void ga_unset_frozen(GAState *s)
>              return;
>          }
>
>     +#ifdef CONFIG_FSFREEZE
>     +    /* remove timeout callback */
>     +    if (s->frozen_timeout_id) {
>     +        g_source_remove(s->frozen_timeout_id);
>     +    }
>     +#endif
>     +
>          /* if we delayed creation/opening of pid/log files due to being
>           * in a frozen state at start up, do it now
>           */
>     diff --git a/qga/qapi-schema.json b/qga/qapi-schema.json
>     index e96d463639..29ad342f0a 100644
>     --- a/qga/qapi-schema.json
>     +++ b/qga/qapi-schema.json
>     @@ -440,6 +440,9 @@
>      # command succeeded, you may call @guest-fsfreeze-thaw later to
>      # unfreeze.
>      #
>     +# @timeout: after this period in seconds filesystems will be thawed
>     +#     (since 8.2)
>     +#
>      # Note: On Windows, the command is implemented with the help of a
>      #     Volume Shadow-copy Service DLL helper.  The frozen state is
>      #     limited for up to 10 seconds by VSS.
>     @@ -452,6 +455,7 @@
>      # Since: 0.15.0
>      ##
>      { 'command': 'guest-fsfreeze-freeze',
>     +  'data':    { '*timeout': 'int' },
>        'returns': 'int' }
>
>      ##
>     @@ -464,13 +468,16 @@
>      #     If omitted, every mounted filesystem is frozen. Invalid mount
>      #     points are ignored.
>      #
>     +# @timeout: after this period in seconds filesystems will be thawed
>     +#     (since 8.2)
>     +#
>      # Returns: Number of file systems currently frozen.  On error, all
>      #     filesystems will be thawed.
>      #
>      # Since: 2.2
>      ##
>      { 'command': 'guest-fsfreeze-freeze-list',
>     -  'data':    { '*mountpoints': ['str'] },
>     +  'data':    { '*mountpoints': ['str'], '*timeout': 'int' },
>        'returns': 'int' }
>
>      ##
>     -- 
>     2.34.1
>

-- 
Best regards,
Alexander Ivanov