[Qemu-trivial] [PATCH] migration: reflect incoming failure to shell

[Qemu-trivial] [PATCH] migration: reflect incoming failure to shell

[Eric Blake]

Management apps like libvirt don't know to pay attention to
stderr unless there is a non-zero exit status.

* migration.c (process_incoming_migration_co): Exit with non-zero
status on failure.

Signed-off-by: Eric Blake <eblake@redhat.com>
---

Noticed while reviewing:
https://lists.gnu.org/archive/html/qemu-devel/2013-04/msg03293.html
and it seems blatant enough to fix now, rather than waiting for
Pavel's series to stabilize.

Side note: libvirt explicitly forbids all use of exit({0,1}), and
instead encourages exit(EXIT_{SUCCESS,FAILURE}), precisely because
it makes it harder to slip in unintentional successful exits.

 migration.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/migration.c b/migration.c
index 3b4b467..3eb0fad 100644
--- a/migration.c
+++ b/migration.c
@@ -99,7 +99,7 @@ static void process_incoming_migration_co(void *opaque)
     qemu_fclose(f);
     if (ret < 0) {
         fprintf(stderr, "load of migration failed\n");
-        exit(0);
+        exit(EXIT_FAILURE);
     }
     qemu_announce_self();
     DPRINTF("successfully loaded vm state\n");
-- 
1.8.1.4

[Qemu-devel] [PATCH] migration: reflect incoming failure to shell

[Eric Blake]

Management apps like libvirt don't know to pay attention to
stderr unless there is a non-zero exit status.

* migration.c (process_incoming_migration_co): Exit with non-zero
status on failure.

Signed-off-by: Eric Blake <eblake@redhat.com>
---

Noticed while reviewing:
https://lists.gnu.org/archive/html/qemu-devel/2013-04/msg03293.html
and it seems blatant enough to fix now, rather than waiting for
Pavel's series to stabilize.

Side note: libvirt explicitly forbids all use of exit({0,1}), and
instead encourages exit(EXIT_{SUCCESS,FAILURE}), precisely because
it makes it harder to slip in unintentional successful exits.

 migration.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/migration.c b/migration.c
index 3b4b467..3eb0fad 100644
--- a/migration.c
+++ b/migration.c
@@ -99,7 +99,7 @@ static void process_incoming_migration_co(void *opaque)
     qemu_fclose(f);
     if (ret < 0) {
         fprintf(stderr, "load of migration failed\n");
-        exit(0);
+        exit(EXIT_FAILURE);
     }
     qemu_announce_self();
     DPRINTF("successfully loaded vm state\n");
-- 
1.8.1.4

Re: [Qemu-trivial] [Qemu-devel] [PATCH] migration: reflect incoming failure to shell

[Anthony Liguori]

Eric Blake <eblake@redhat.com> writes:

> Management apps like libvirt don't know to pay attention to
> stderr unless there is a non-zero exit status.
>
> * migration.c (process_incoming_migration_co): Exit with non-zero
> status on failure.
>
> Signed-off-by: Eric Blake <eblake@redhat.com>

It looks like the migration coroutine cleans itself so there's no
obvious way to propagate the error.  A nicer cleanup would be to switch
the fprintf to error_report() too but this is certainly better than what
we have now.

Maybe we need an error_report_fatal()...

Reviewed-by: Anthony Liguori <aliguori@us.ibm.com>

Regards,

Anthony Liguori


> ---
>
> Noticed while reviewing:
> https://lists.gnu.org/archive/html/qemu-devel/2013-04/msg03293.html
> and it seems blatant enough to fix now, rather than waiting for
> Pavel's series to stabilize.
>
> Side note: libvirt explicitly forbids all use of exit({0,1}), and
> instead encourages exit(EXIT_{SUCCESS,FAILURE}), precisely because
> it makes it harder to slip in unintentional successful exits.
>
>  migration.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/migration.c b/migration.c
> index 3b4b467..3eb0fad 100644
> --- a/migration.c
> +++ b/migration.c
> @@ -99,7 +99,7 @@ static void process_incoming_migration_co(void *opaque)
>      qemu_fclose(f);
>      if (ret < 0) {
>          fprintf(stderr, "load of migration failed\n");
> -        exit(0);
> +        exit(EXIT_FAILURE);
>      }
>      qemu_announce_self();
>      DPRINTF("successfully loaded vm state\n");
> -- 
> 1.8.1.4

Re: [Qemu-devel] [PATCH] migration: reflect incoming failure to shell

[Anthony Liguori]

Eric Blake <eblake@redhat.com> writes:

> Management apps like libvirt don't know to pay attention to
> stderr unless there is a non-zero exit status.
>
> * migration.c (process_incoming_migration_co): Exit with non-zero
> status on failure.
>
> Signed-off-by: Eric Blake <eblake@redhat.com>

It looks like the migration coroutine cleans itself so there's no
obvious way to propagate the error.  A nicer cleanup would be to switch
the fprintf to error_report() too but this is certainly better than what
we have now.

Maybe we need an error_report_fatal()...

Reviewed-by: Anthony Liguori <aliguori@us.ibm.com>

Regards,

Anthony Liguori


> ---
>
> Noticed while reviewing:
> https://lists.gnu.org/archive/html/qemu-devel/2013-04/msg03293.html
> and it seems blatant enough to fix now, rather than waiting for
> Pavel's series to stabilize.
>
> Side note: libvirt explicitly forbids all use of exit({0,1}), and
> instead encourages exit(EXIT_{SUCCESS,FAILURE}), precisely because
> it makes it harder to slip in unintentional successful exits.
>
>  migration.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/migration.c b/migration.c
> index 3b4b467..3eb0fad 100644
> --- a/migration.c
> +++ b/migration.c
> @@ -99,7 +99,7 @@ static void process_incoming_migration_co(void *opaque)
>      qemu_fclose(f);
>      if (ret < 0) {
>          fprintf(stderr, "load of migration failed\n");
> -        exit(0);
> +        exit(EXIT_FAILURE);
>      }
>      qemu_announce_self();
>      DPRINTF("successfully loaded vm state\n");
> -- 
> 1.8.1.4

Re: [Qemu-trivial] [PATCH] migration: reflect incoming failure to shell

[Anthony Liguori]

Applied.  Thanks.

Regards,

Anthony Liguori

Re: [Qemu-devel] [PATCH] migration: reflect incoming failure to shell

[Anthony Liguori]

Applied.  Thanks.

Regards,

Anthony Liguori