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

2023-10-30 Thread Konstantin Kostiuk
On Mon, Oct 30, 2023 at 6:32 PM Alexander Ivanov <
alexander.iva...@virtuozzo.com> wrote:

>
>
> 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.
>

Oh, sorry, my mistake.
Just comment about VSS limitation.


Best Regards,
Konstantin Kostiuk.


> >
> > On Wed, Oct 25, 2023 at 5:01 PM Alexander Ivanov
> >  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 
> > ---
> >  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,
> 

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

2023-10-30 Thread Alexander Ivanov




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 
 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 
---
 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);
+    

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

2023-10-26 Thread Konstantin Kostiuk
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.

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?

On Wed, Oct 25, 2023 at 5:01 PM Alexander Ivanov <
alexander.iva...@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 
> ---
>  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(, true, mountpoints, _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 

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

2023-10-25 Thread Alexander Ivanov
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 
---
 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(, true, mountpoints, _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