Re: svn commit: r336188 - head/usr.sbin/bhyve

2018-07-10 Thread Marcelo Araujo
Hi Ravi,

Yes, you are correct! I have discussed about it in the review and the
approach you mentioned is exactly I'm gonna do. Although with this patch
the intention for now is make exit(1) more unique for "powered off".

Snipped from the review:
"OK, I will update the manpage and commit it as-is just to make the exit(1)
more unique for now. I have intention to revisit it and improve all the
exit returns. I will replace fprintf/perror + exit as soon as I finish the
drivers analysis to make sure what can be improved as error, exit and
return code."

The full discussion is at that review.

Best,

2018-07-11 11:28 GMT+08:00 Ravi Pokala :

> Hi Marcelo,
>
> If the intention is to have specific exit codes have specific meanings,
> wouldn't it be useful to set up defines or an enum or something, so a
> symbolic value can be used rather than a bare integer?
>
> Thanks,
>
> Ravi (rpokala@)
>
> -Original Message-
> From:  on behalf of Marcelo Araujo
> 
> Date: 2018-07-10, Tuesday at 20:23
> To: , , <
> svn-src-h...@freebsd.org>
> Subject: svn commit: r336188 - head/usr.sbin/bhyve
>
> Author: araujo
> Date: Wed Jul 11 03:23:09 2018
> New Revision: 336188
> URL: https://svnweb.freebsd.org/changeset/base/336188
>
> Log:
>   Improve bhyve exit(3) error code.
>
>   The bhyve(8) exit status indicates how the VM was terminated:
>
>   0 rebooted
>   1 powered off
>   2 halted
>   3 triple fault
>
>   The problem is when we have wrappers around bhyve that parses the exit
>   error code and gets an exit(1) for an error but interprets it as
> "powered off".
>   So to mitigate this issue and makes it less error prone for third part
>   applications, I have added a new exit code 4 that is "exited due to an
> error".
>
>   For now the bhyve(8) exit status are:
>   0 rebooted
>   1 powered off
>   2 halted
>   3 triple fault
>   4 exited due to an error
>
>   Reviewed by:  @jhb
>   MFC after:2 weeks.
>   Sponsored by: iXsystems Inc.
>   Differential Revision:https://reviews.freebsd.org/D16161
>
> Modified:
>   head/usr.sbin/bhyve/bhyve.8
>   head/usr.sbin/bhyve/bhyverun.c
>   head/usr.sbin/bhyve/dbgport.c
>   head/usr.sbin/bhyve/fwctl.c
>   head/usr.sbin/bhyve/mevent_test.c
>   head/usr.sbin/bhyve/pci_e82545.c
>
> Modified: head/usr.sbin/bhyve/bhyve.8
> 
> ==
> --- head/usr.sbin/bhyve/bhyve.8 Wed Jul 11 02:32:06 2018(r336187)
> +++ head/usr.sbin/bhyve/bhyve.8 Wed Jul 11 03:23:09 2018(r336188)
> @@ -24,7 +24,7 @@
>  .\"
>  .\" $FreeBSD$
>  .\"
> -.Dd Jul 05, 2018
> +.Dd Jul 11, 2018
>  .Dt BHYVE 8
>  .Os
>  .Sh NAME
> @@ -520,6 +520,8 @@ powered off
>  halted
>  .It 3
>  triple fault
> +.It 4
> +exited due to an error
>  .El
>  .Sh EXAMPLES
>  If not using a boot ROM, the guest operating system must have been loaded
> with
>
> Modified: head/usr.sbin/bhyve/bhyverun.c
> 
> ==
> --- head/usr.sbin/bhyve/bhyverun.c  Wed Jul 11 02:32:06 2018
> (r336187)
> +++ head/usr.sbin/bhyve/bhyverun.c  Wed Jul 11 03:23:09 2018
> (r336188)
> @@ -397,7 +397,7 @@ fbsdrun_deletecpu(struct vmctx *ctx, int vcpu)
>
> if (!CPU_ISSET(vcpu, )) {
> fprintf(stderr, "Attempting to delete unknown cpu %d\n",
> vcpu);
> -   exit(1);
> +   exit(4);
> }
>
> CPU_CLR_ATOMIC(vcpu, );
> @@ -742,7 +742,7 @@ vm_loop(struct vmctx *ctx, int vcpu, uint64_t startrip
> if (exitcode >= VM_EXITCODE_MAX || handler[exitcode] ==
> NULL) {
> fprintf(stderr, "vm_loop: unexpected exitcode
> 0x%x\n",
> exitcode);
> -   exit(1);
> +   exit(4);
> }
>
> rc = (*handler[exitcode])(ctx, [vcpu], );
> @@ -753,7 +753,7 @@ vm_loop(struct vmctx *ctx, int vcpu, uint64_t startrip
> case VMEXIT_ABORT:
> abort();
> default:
> -   exit(1);
> +   exit(4);
> }
> }
> fprintf(stderr, "vm_run error %d, errno %d\n", error, errno);
> @@ -785,7 +785,7 @@ fbsdrun_set_capabilities(struct vmctx *ctx, int cpu)
> err = vm_get_capability(ctx, cpu, VM_CAP_HALT_EXIT, );
> if (err < 0) {
> fprintf(stderr, "

Re: svn commit: r336188 - head/usr.sbin/bhyve

2018-07-10 Thread Ravi Pokala
Hi Marcelo,

If the intention is to have specific exit codes have specific meanings, 
wouldn't it be useful to set up defines or an enum or something, so a symbolic 
value can be used rather than a bare integer?

Thanks,

Ravi (rpokala@)

-Original Message-
From:  on behalf of Marcelo Araujo 

Date: 2018-07-10, Tuesday at 20:23
To: , , 

Subject: svn commit: r336188 - head/usr.sbin/bhyve

Author: araujo
Date: Wed Jul 11 03:23:09 2018
New Revision: 336188
URL: https://svnweb.freebsd.org/changeset/base/336188

Log:
  Improve bhyve exit(3) error code.
  
  The bhyve(8) exit status indicates how the VM was terminated:
  
  0 rebooted
  1 powered off
  2 halted
  3 triple fault
  
  The problem is when we have wrappers around bhyve that parses the exit
  error code and gets an exit(1) for an error but interprets it as "powered 
off".
  So to mitigate this issue and makes it less error prone for third part
  applications, I have added a new exit code 4 that is "exited due to an error".
  
  For now the bhyve(8) exit status are:
  0 rebooted
  1 powered off
  2 halted
  3 triple fault
  4 exited due to an error
  
  Reviewed by:  @jhb
  MFC after:2 weeks.
  Sponsored by: iXsystems Inc.
  Differential Revision:https://reviews.freebsd.org/D16161

Modified:
  head/usr.sbin/bhyve/bhyve.8
  head/usr.sbin/bhyve/bhyverun.c
  head/usr.sbin/bhyve/dbgport.c
  head/usr.sbin/bhyve/fwctl.c
  head/usr.sbin/bhyve/mevent_test.c
  head/usr.sbin/bhyve/pci_e82545.c

Modified: head/usr.sbin/bhyve/bhyve.8
==
--- head/usr.sbin/bhyve/bhyve.8 Wed Jul 11 02:32:06 2018(r336187)
+++ head/usr.sbin/bhyve/bhyve.8 Wed Jul 11 03:23:09 2018(r336188)
@@ -24,7 +24,7 @@
 .\"
 .\" $FreeBSD$
 .\"
-.Dd Jul 05, 2018
+.Dd Jul 11, 2018
 .Dt BHYVE 8
 .Os
 .Sh NAME
@@ -520,6 +520,8 @@ powered off
 halted
 .It 3
 triple fault
+.It 4
+exited due to an error
 .El
 .Sh EXAMPLES
 If not using a boot ROM, the guest operating system must have been loaded with

Modified: head/usr.sbin/bhyve/bhyverun.c
==
--- head/usr.sbin/bhyve/bhyverun.c  Wed Jul 11 02:32:06 2018
(r336187)
+++ head/usr.sbin/bhyve/bhyverun.c  Wed Jul 11 03:23:09 2018
(r336188)
@@ -397,7 +397,7 @@ fbsdrun_deletecpu(struct vmctx *ctx, int vcpu)
 
if (!CPU_ISSET(vcpu, )) {
fprintf(stderr, "Attempting to delete unknown cpu %d\n", vcpu);
-   exit(1);
+   exit(4);
}
 
CPU_CLR_ATOMIC(vcpu, );
@@ -742,7 +742,7 @@ vm_loop(struct vmctx *ctx, int vcpu, uint64_t startrip
if (exitcode >= VM_EXITCODE_MAX || handler[exitcode] == NULL) {
fprintf(stderr, "vm_loop: unexpected exitcode 0x%x\n",
exitcode);
-   exit(1);
+   exit(4);
}
 
rc = (*handler[exitcode])(ctx, [vcpu], );
@@ -753,7 +753,7 @@ vm_loop(struct vmctx *ctx, int vcpu, uint64_t startrip
case VMEXIT_ABORT:
abort();
default:
-   exit(1);
+   exit(4);
}
}
fprintf(stderr, "vm_run error %d, errno %d\n", error, errno);
@@ -785,7 +785,7 @@ fbsdrun_set_capabilities(struct vmctx *ctx, int cpu)
err = vm_get_capability(ctx, cpu, VM_CAP_HALT_EXIT, );
if (err < 0) {
fprintf(stderr, "VM exit on HLT not supported\n");
-   exit(1);
+   exit(4);
}
vm_set_capability(ctx, cpu, VM_CAP_HALT_EXIT, 1);
if (cpu == BSP)
@@ -800,7 +800,7 @@ fbsdrun_set_capabilities(struct vmctx *ctx, int cpu)
if (err < 0) {
fprintf(stderr,
"SMP mux requested, no pause support\n");
-   exit(1);
+   exit(4);
}
vm_set_capability(ctx, cpu, VM_CAP_PAUSE_EXIT, 1);
if (cpu == BSP)
@@ -814,7 +814,7 @@ fbsdrun_set_capabilities(struct vmctx *ctx, int cpu)
 
if (err) {
fprintf(stderr, "Unable to set x2apic state (%d)\n", err);
-   exit(1);
+   exit(4);
}
 
vm_set_capability(ctx, cpu, VM_CAP_ENABLE_INVPCID, 1);
@@ -850,7 +850,7 @@ do_open(const char *vmname)
}
} else {
perror("vm_create");
-   exit(1);
+   exit(4);
}
} else {
if (!romboot) {
@@ -859,14 +859,14 @@ do_open(const char *vmname)
 * bootrom m

svn commit: r336188 - head/usr.sbin/bhyve

2018-07-10 Thread Marcelo Araujo
Author: araujo
Date: Wed Jul 11 03:23:09 2018
New Revision: 336188
URL: https://svnweb.freebsd.org/changeset/base/336188

Log:
  Improve bhyve exit(3) error code.
  
  The bhyve(8) exit status indicates how the VM was terminated:
  
  0 rebooted
  1 powered off
  2 halted
  3 triple fault
  
  The problem is when we have wrappers around bhyve that parses the exit
  error code and gets an exit(1) for an error but interprets it as "powered 
off".
  So to mitigate this issue and makes it less error prone for third part
  applications, I have added a new exit code 4 that is "exited due to an error".
  
  For now the bhyve(8) exit status are:
  0 rebooted
  1 powered off
  2 halted
  3 triple fault
  4 exited due to an error
  
  Reviewed by:  @jhb
  MFC after:2 weeks.
  Sponsored by: iXsystems Inc.
  Differential Revision:https://reviews.freebsd.org/D16161

Modified:
  head/usr.sbin/bhyve/bhyve.8
  head/usr.sbin/bhyve/bhyverun.c
  head/usr.sbin/bhyve/dbgport.c
  head/usr.sbin/bhyve/fwctl.c
  head/usr.sbin/bhyve/mevent_test.c
  head/usr.sbin/bhyve/pci_e82545.c

Modified: head/usr.sbin/bhyve/bhyve.8
==
--- head/usr.sbin/bhyve/bhyve.8 Wed Jul 11 02:32:06 2018(r336187)
+++ head/usr.sbin/bhyve/bhyve.8 Wed Jul 11 03:23:09 2018(r336188)
@@ -24,7 +24,7 @@
 .\"
 .\" $FreeBSD$
 .\"
-.Dd Jul 05, 2018
+.Dd Jul 11, 2018
 .Dt BHYVE 8
 .Os
 .Sh NAME
@@ -520,6 +520,8 @@ powered off
 halted
 .It 3
 triple fault
+.It 4
+exited due to an error
 .El
 .Sh EXAMPLES
 If not using a boot ROM, the guest operating system must have been loaded with

Modified: head/usr.sbin/bhyve/bhyverun.c
==
--- head/usr.sbin/bhyve/bhyverun.c  Wed Jul 11 02:32:06 2018
(r336187)
+++ head/usr.sbin/bhyve/bhyverun.c  Wed Jul 11 03:23:09 2018
(r336188)
@@ -397,7 +397,7 @@ fbsdrun_deletecpu(struct vmctx *ctx, int vcpu)
 
if (!CPU_ISSET(vcpu, )) {
fprintf(stderr, "Attempting to delete unknown cpu %d\n", vcpu);
-   exit(1);
+   exit(4);
}
 
CPU_CLR_ATOMIC(vcpu, );
@@ -742,7 +742,7 @@ vm_loop(struct vmctx *ctx, int vcpu, uint64_t startrip
if (exitcode >= VM_EXITCODE_MAX || handler[exitcode] == NULL) {
fprintf(stderr, "vm_loop: unexpected exitcode 0x%x\n",
exitcode);
-   exit(1);
+   exit(4);
}
 
rc = (*handler[exitcode])(ctx, [vcpu], );
@@ -753,7 +753,7 @@ vm_loop(struct vmctx *ctx, int vcpu, uint64_t startrip
case VMEXIT_ABORT:
abort();
default:
-   exit(1);
+   exit(4);
}
}
fprintf(stderr, "vm_run error %d, errno %d\n", error, errno);
@@ -785,7 +785,7 @@ fbsdrun_set_capabilities(struct vmctx *ctx, int cpu)
err = vm_get_capability(ctx, cpu, VM_CAP_HALT_EXIT, );
if (err < 0) {
fprintf(stderr, "VM exit on HLT not supported\n");
-   exit(1);
+   exit(4);
}
vm_set_capability(ctx, cpu, VM_CAP_HALT_EXIT, 1);
if (cpu == BSP)
@@ -800,7 +800,7 @@ fbsdrun_set_capabilities(struct vmctx *ctx, int cpu)
if (err < 0) {
fprintf(stderr,
"SMP mux requested, no pause support\n");
-   exit(1);
+   exit(4);
}
vm_set_capability(ctx, cpu, VM_CAP_PAUSE_EXIT, 1);
if (cpu == BSP)
@@ -814,7 +814,7 @@ fbsdrun_set_capabilities(struct vmctx *ctx, int cpu)
 
if (err) {
fprintf(stderr, "Unable to set x2apic state (%d)\n", err);
-   exit(1);
+   exit(4);
}
 
vm_set_capability(ctx, cpu, VM_CAP_ENABLE_INVPCID, 1);
@@ -850,7 +850,7 @@ do_open(const char *vmname)
}
} else {
perror("vm_create");
-   exit(1);
+   exit(4);
}
} else {
if (!romboot) {
@@ -859,14 +859,14 @@ do_open(const char *vmname)
 * bootrom must be configured to boot it.
 */
fprintf(stderr, "virtual machine cannot be booted\n");
-   exit(1);
+   exit(4);
}
}
 
ctx = vm_open(vmname);
if (ctx == NULL) {
perror("vm_open");
-   exit(1);
+   exit(4);
}
 
 #ifndef WITHOUT_CAPSICUM
@@ -888,7 +888,7 @@ do_open(const char *vmname)
error = vm_reinit(ctx);