[PATCH v3 18/22] qga: don't disable fsfreeze commands if vss_init fails

Daniel P. Berrangé posted 22 patches 4 months, 2 weeks ago
Maintainers: Michael Roth <michael.roth@amd.com>, Konstantin Kostiuk <kkostiuk@redhat.com>, Paolo Bonzini <pbonzini@redhat.com>, "Marc-André Lureau" <marcandre.lureau@redhat.com>, "Daniel P. Berrangé" <berrange@redhat.com>, Thomas Huth <thuth@redhat.com>, "Philippe Mathieu-Daudé" <philmd@linaro.org>
[PATCH v3 18/22] qga: don't disable fsfreeze commands if vss_init fails
Posted by Daniel P. Berrangé 4 months, 2 weeks ago
The fsfreeze commands are already written to report an error if
vss_init() fails. Reporting a more specific error message is more
helpful than a generic "command is disabled" message, which cannot
between an admin config decision and lack of platform support.

Reviewed-by: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 qga/commands-win32.c | 18 +++---------------
 qga/main.c           |  4 ++++
 2 files changed, 7 insertions(+), 15 deletions(-)

diff --git a/qga/commands-win32.c b/qga/commands-win32.c
index 2533e4c748..5866cc2e3c 100644
--- a/qga/commands-win32.c
+++ b/qga/commands-win32.c
@@ -1203,7 +1203,7 @@ GuestFilesystemInfoList *qmp_guest_get_fsinfo(Error **errp)
 GuestFsfreezeStatus qmp_guest_fsfreeze_status(Error **errp)
 {
     if (!vss_initialized()) {
-        error_setg(errp, QERR_UNSUPPORTED);
+        error_setg(errp, "fsfreeze not possible as VSS failed to initialize");
         return 0;
     }
 
@@ -1231,7 +1231,7 @@ int64_t qmp_guest_fsfreeze_freeze_list(bool has_mountpoints,
     Error *local_err = NULL;
 
     if (!vss_initialized()) {
-        error_setg(errp, QERR_UNSUPPORTED);
+        error_setg(errp, "fsfreeze not possible as VSS failed to initialize");
         return 0;
     }
 
@@ -1266,7 +1266,7 @@ int64_t qmp_guest_fsfreeze_thaw(Error **errp)
     int i;
 
     if (!vss_initialized()) {
-        error_setg(errp, QERR_UNSUPPORTED);
+        error_setg(errp, "fsfreeze not possible as VSS failed to initialize");
         return 0;
     }
 
@@ -1961,18 +1961,6 @@ done:
 /* add unsupported commands to the list of blocked RPCs */
 GList *ga_command_init_blockedrpcs(GList *blockedrpcs)
 {
-    if (!vss_init(true)) {
-        g_debug("vss_init failed, vss commands are going to be disabled");
-        const char *list[] = {
-            "guest-get-fsinfo", "guest-fsfreeze-status",
-            "guest-fsfreeze-freeze", "guest-fsfreeze-thaw", NULL};
-        char **p = (char **)list;
-
-        while (*p) {
-            blockedrpcs = g_list_append(blockedrpcs, g_strdup(*p++));
-        }
-    }
-
     return blockedrpcs;
 }
 
diff --git a/qga/main.c b/qga/main.c
index f4d5f15bb3..17b6ce18ac 100644
--- a/qga/main.c
+++ b/qga/main.c
@@ -1395,6 +1395,10 @@ static GAState *initialize_agent(GAConfig *config, int socket_activation)
                    " '%s': %s", config->state_dir, strerror(errno));
         return NULL;
     }
+
+    if (!vss_init(true)) {
+        g_debug("vss_init failed, vss commands will not function");
+    }
 #endif
 
     if (ga_is_frozen(s)) {
-- 
2.45.1


Re: [PATCH v3 18/22] qga: don't disable fsfreeze commands if vss_init fails
Posted by Konstantin Kostiuk 4 months, 1 week ago
Reviewed-by: Konstantin Kostiuk <kkostiuk@redhat.com>

On Fri, Jul 12, 2024 at 4:26 PM Daniel P. Berrangé <berrange@redhat.com>
wrote:

> The fsfreeze commands are already written to report an error if
> vss_init() fails. Reporting a more specific error message is more
> helpful than a generic "command is disabled" message, which cannot
> between an admin config decision and lack of platform support.
>
> Reviewed-by: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
>  qga/commands-win32.c | 18 +++---------------
>  qga/main.c           |  4 ++++
>  2 files changed, 7 insertions(+), 15 deletions(-)
>
> diff --git a/qga/commands-win32.c b/qga/commands-win32.c
> index 2533e4c748..5866cc2e3c 100644
> --- a/qga/commands-win32.c
> +++ b/qga/commands-win32.c
> @@ -1203,7 +1203,7 @@ GuestFilesystemInfoList *qmp_guest_get_fsinfo(Error
> **errp)
>  GuestFsfreezeStatus qmp_guest_fsfreeze_status(Error **errp)
>  {
>      if (!vss_initialized()) {
> -        error_setg(errp, QERR_UNSUPPORTED);
> +        error_setg(errp, "fsfreeze not possible as VSS failed to
> initialize");
>          return 0;
>      }
>
> @@ -1231,7 +1231,7 @@ int64_t qmp_guest_fsfreeze_freeze_list(bool
> has_mountpoints,
>      Error *local_err = NULL;
>
>      if (!vss_initialized()) {
> -        error_setg(errp, QERR_UNSUPPORTED);
> +        error_setg(errp, "fsfreeze not possible as VSS failed to
> initialize");
>          return 0;
>      }
>
> @@ -1266,7 +1266,7 @@ int64_t qmp_guest_fsfreeze_thaw(Error **errp)
>      int i;
>
>      if (!vss_initialized()) {
> -        error_setg(errp, QERR_UNSUPPORTED);
> +        error_setg(errp, "fsfreeze not possible as VSS failed to
> initialize");
>          return 0;
>      }
>
> @@ -1961,18 +1961,6 @@ done:
>  /* add unsupported commands to the list of blocked RPCs */
>  GList *ga_command_init_blockedrpcs(GList *blockedrpcs)
>  {
> -    if (!vss_init(true)) {
> -        g_debug("vss_init failed, vss commands are going to be disabled");
> -        const char *list[] = {
> -            "guest-get-fsinfo", "guest-fsfreeze-status",
> -            "guest-fsfreeze-freeze", "guest-fsfreeze-thaw", NULL};
> -        char **p = (char **)list;
> -
> -        while (*p) {
> -            blockedrpcs = g_list_append(blockedrpcs, g_strdup(*p++));
> -        }
> -    }
> -
>      return blockedrpcs;
>  }
>
> diff --git a/qga/main.c b/qga/main.c
> index f4d5f15bb3..17b6ce18ac 100644
> --- a/qga/main.c
> +++ b/qga/main.c
> @@ -1395,6 +1395,10 @@ static GAState *initialize_agent(GAConfig *config,
> int socket_activation)
>                     " '%s': %s", config->state_dir, strerror(errno));
>          return NULL;
>      }
> +
> +    if (!vss_init(true)) {
> +        g_debug("vss_init failed, vss commands will not function");
> +    }
>  #endif
>
>      if (ga_is_frozen(s)) {
> --
> 2.45.1
>
>