From owner-svn-src-head@freebsd.org Wed Jul 11 03:37:26 2018 Return-Path: Delivered-To: svn-src-head@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 451AD102AE39; Wed, 11 Jul 2018 03:37:26 +0000 (UTC) (envelope-from araujobsdport@gmail.com) Received: from mail-lf0-x242.google.com (mail-lf0-x242.google.com [IPv6:2a00:1450:4010:c07::242]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (Client CN "smtp.gmail.com", Issuer "Google Internet Authority G2" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 808F97A8C0; Wed, 11 Jul 2018 03:37:25 +0000 (UTC) (envelope-from araujobsdport@gmail.com) Received: by mail-lf0-x242.google.com with SMTP id v22-v6so6574727lfe.8; Tue, 10 Jul 2018 20:37:25 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:reply-to:in-reply-to:references:from:date:message-id :subject:to:cc; bh=YNoqBzuBQ3+kXqhOQPnvQvh45x9XsGsOHGfaqekJWjs=; b=f2NaxJ+5ChSqRRV/8gF0O7R1B9f7kpoTXmvwe+ax9lAqIZgaJv8aaSVubqxiPlSLEb 69Td52iSHQni4fHV4UQ1mcJx9YTBaoRQ3ou1Rdr7EQJF2RAFfh46Xj9TYuTx5cRRodGh Diu1UcOD1GGvVwTawDuU7H+/3V7mmpEAkukmFfXGNvJ+R6XADeUcduAwcSSZ4GOr+Hrc tJI5Dte/fALU+PEFMJF5rhkMVYwAA9YtTDKaTMsJSNQ6Mt50J5yYiacpIS5QurwBo0pX u4LbNWfQmmLZnCjLoaBDOeZujA+I6VMK7SHfByRdler7cJIO9nhZ3xEjjccR5Sh05RT1 2xSQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:reply-to:in-reply-to:references :from:date:message-id:subject:to:cc; bh=YNoqBzuBQ3+kXqhOQPnvQvh45x9XsGsOHGfaqekJWjs=; b=XiAlK1NYqMNABGrIk+74lH/RaU/Q/6F2Ae91rDDC8tNVMWYVqDhUvDEo7AKUoKLiBs C8toQHBS3fVSauBcNeo4zvfvi2Urh73b9WwWYWYPYrJD73PbNfbHSw1JRwAY/eh11OLO IuZmmEsBB6jcu++UQqVMFj7J3q7pPAoKwdgmK4ACY/K8k7j/Phfty9F7IN+SV3Geg39P yqwOwO7YhI9yxA8QMTGRRNGbNFkIEAoBQvAWOyhfwB0XvJ0EUDoIZAmzXvJF4Rp8xQ+8 TnHOiclVNxJzcjXdZIASRS4mJZGz611FQ0qBBqf1MqqKeKejMKVL8b7tPHIGR33Fvvoe li+Q== X-Gm-Message-State: APt69E1HMexYnYbtG6Oer7xQZkhMo2O3KFA2JfiYzEw4BF91rldlb9M/ rFfTyUZTR/yA8GqmctKdEqNuFzGl7YG49bw1Fcr2RA== X-Google-Smtp-Source: AAOMgpeEvUT9AudGFRjpROQkg5GqRp34yqdc0u8rvjC2x2q79hNU0PwU3/n1G3HP3Fciwi4AjG3uU1nIYkGDA8NlhMI= X-Received: by 2002:a19:2207:: with SMTP id i7-v6mr4312787lfi.69.1531280242976; Tue, 10 Jul 2018 20:37:22 -0700 (PDT) MIME-Version: 1.0 Received: by 2002:a19:1f0d:0:0:0:0:0 with HTTP; Tue, 10 Jul 2018 20:37:21 -0700 (PDT) Reply-To: araujo@freebsd.org In-Reply-To: <3D1CA66E-9F52-435E-A5A4-E22C6A4931F5@panasas.com> References: <201807110323.w6B3N9FO003639@repo.freebsd.org> <3D1CA66E-9F52-435E-A5A4-E22C6A4931F5@panasas.com> From: Marcelo Araujo Date: Wed, 11 Jul 2018 11:37:21 +0800 Message-ID: Subject: Re: svn commit: r336188 - head/usr.sbin/bhyve To: Ravi Pokala Cc: src-committers , svn-src-all@freebsd.org, svn-src-head@freebsd.org Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Content-Filtered-By: Mailman/MimeDel 2.1.27 X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.27 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 11 Jul 2018 03:37:26 -0000 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@) > > =EF=BB=BF-----Original Message----- > From: on behalf of Marcelo Araujo > > Date: 2018-07-10, Tuesday at 20:23 > To: , , < > svn-src-head@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 > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D > --- 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 loade= d > with > > Modified: head/usr.sbin/bhyve/bhyverun.c > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D > --- 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, &cpumask)) { > fprintf(stderr, "Attempting to delete unknown cpu %d\n", > vcpu); > - exit(1); > + exit(4); > } > > CPU_CLR_ATOMIC(vcpu, &cpumask); > @@ -742,7 +742,7 @@ vm_loop(struct vmctx *ctx, int vcpu, uint64_t startri= p > if (exitcode >=3D VM_EXITCODE_MAX || handler[exitcode] = =3D=3D > NULL) { > fprintf(stderr, "vm_loop: unexpected exitcode > 0x%x\n", > exitcode); > - exit(1); > + exit(4); > } > > rc =3D (*handler[exitcode])(ctx, &vmexit[vcpu], &vcpu); > @@ -753,7 +753,7 @@ vm_loop(struct vmctx *ctx, int vcpu, uint64_t startri= p > 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 =3D vm_get_capability(ctx, cpu, VM_CAP_HALT_EXIT, &tm= p); > 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 =3D=3D 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 =3D=3D 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 =3D vm_open(vmname); > if (ctx =3D=3D NULL) { > perror("vm_open"); > - exit(1); > + exit(4); > } > > #ifndef WITHOUT_CAPSICUM > @@ -888,7 +888,7 @@ do_open(const char *vmname) > error =3D vm_reinit(ctx); > if (error) { > perror("vm_reinit"); > - exit(1); > + exit(4); > } > } > error =3D vm_set_topology(ctx, sockets, cores, threads, maxcpus); > @@ -967,7 +967,7 @@ main(int argc, char *argv[]) > break; > case 's': > if (pci_parse_slot(optarg) !=3D 0) > - exit(1); > + exit(4); > else > break; > case 'S': > @@ -1033,7 +1033,7 @@ main(int argc, char *argv[]) > if (guest_ncpus > max_vcpus) { > fprintf(stderr, "%d vCPUs requested but only %d > available\n", > guest_ncpus, max_vcpus); > - exit(1); > + exit(4); > } > > fbsdrun_set_capabilities(ctx, BSP); > @@ -1042,13 +1042,13 @@ main(int argc, char *argv[]) > err =3D vm_setup_memory(ctx, memsize, VM_MMAP_ALL); > if (err) { > fprintf(stderr, "Unable to setup memory (%d)\n", errno); > - exit(1); > + exit(4); > } > > error =3D init_msr(); > if (error) { > fprintf(stderr, "init_msr error %d", error); > - exit(1); > + exit(4); > } > > init_mem(); > @@ -1063,8 +1063,10 @@ main(int argc, char *argv[]) > /* > * Exit if a device emulation finds an error in its initilization > */ > - if (init_pci(ctx) !=3D 0) > - exit(1); > + if (init_pci(ctx) !=3D 0) { > + perror("device emulation initialization error"); > + exit(4); > + } > > if (dbg_port !=3D 0) > init_dbgport(dbg_port); > @@ -1079,7 +1081,7 @@ main(int argc, char *argv[]) > if (vm_set_capability(ctx, BSP, VM_CAP_UNRESTRICTED_GUEST= , > 1)) { > fprintf(stderr, "ROM boot failed: unrestricted > guest " > "capability not available\n"); > - exit(1); > + exit(4); > } > error =3D vcpu_reset(ctx, BSP); > assert(error =3D=3D 0); > @@ -1093,8 +1095,10 @@ main(int argc, char *argv[]) > */ > if (mptgen) { > error =3D mptable_build(ctx, guest_ncpus); > - if (error) > - exit(1); > + if (error) { > + perror("error to build the guest tables"); > + exit(4); > + } > } > > error =3D smbios_build(ctx); > @@ -1133,5 +1137,5 @@ main(int argc, char *argv[]) > */ > mevent_dispatch(); > > - exit(1); > + exit(4); > } > > Modified: head/usr.sbin/bhyve/dbgport.c > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D > --- head/usr.sbin/bhyve/dbgport.c Wed Jul 11 02:32:06 2018 > (r336187) > +++ head/usr.sbin/bhyve/dbgport.c Wed Jul 11 03:23:09 2018 > (r336188) > @@ -139,8 +139,8 @@ init_dbgport(int sport) > conn_fd =3D -1; > > if ((listen_fd =3D socket(AF_INET, SOCK_STREAM, 0)) < 0) { > - perror("socket"); > - exit(1); > + perror("cannot create socket"); > + exit(4); > } > > sin.sin_len =3D sizeof(sin); > @@ -151,18 +151,18 @@ init_dbgport(int sport) > reuse =3D 1; > if (setsockopt(listen_fd, SOL_SOCKET, SO_REUSEADDR, &reuse, > sizeof(reuse)) < 0) { > - perror("setsockopt"); > - exit(1); > + perror("cannot set socket options"); > + exit(4); > } > > if (bind(listen_fd, (struct sockaddr *)&sin, sizeof(sin)) < 0) { > - perror("bind"); > - exit(1); > + perror("cannot bind socket"); > + exit(4); > } > > if (listen(listen_fd, 1) < 0) { > - perror("listen"); > - exit(1); > + perror("cannot listen socket"); > + exit(4); > } > > #ifndef WITHOUT_CAPSICUM > > Modified: head/usr.sbin/bhyve/fwctl.c > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D > --- head/usr.sbin/bhyve/fwctl.c Wed Jul 11 02:32:06 2018 (r336187) > +++ head/usr.sbin/bhyve/fwctl.c Wed Jul 11 03:23:09 2018 (r336188) > @@ -375,7 +375,7 @@ fwctl_request(uint32_t value) > /* Verify size */ > if (value < 12) { > printf("msg size error"); > - exit(1); > + exit(4); > } > rinfo.req_size =3D value; > rinfo.req_count =3D 1; > > Modified: head/usr.sbin/bhyve/mevent_test.c > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D > --- head/usr.sbin/bhyve/mevent_test.c Wed Jul 11 02:32:06 2018 > (r336187) > +++ head/usr.sbin/bhyve/mevent_test.c Wed Jul 11 03:23:09 2018 > (r336188) > @@ -143,7 +143,7 @@ echoer(void *param) > mev =3D mevent_add(fd, EVF_READ, echoer_callback, &sync); > if (mev =3D=3D NULL) { > printf("Could not allocate echoer event\n"); > - exit(1); > + exit(4); > } > > while (!pthread_cond_wait(&sync.e_cond, &sync.e_mt)) { > @@ -200,8 +200,8 @@ acceptor(void *param) > static int first; > > if ((s =3D socket(AF_INET, SOCK_STREAM, 0)) < 0) { > - perror("socket"); > - exit(1); > + perror("cannot create socket"); > + exit(4); > } > > sin.sin_len =3D sizeof(sin); > @@ -210,13 +210,13 @@ acceptor(void *param) > sin.sin_port =3D htons(TEST_PORT); > > if (bind(s, (struct sockaddr *)&sin, sizeof(sin)) < 0) { > - perror("bind"); > - exit(1); > + perror("cannot bind socket"); > + exit(4); > } > > if (listen(s, 1) < 0) { > - perror("listen"); > - exit(1); > + perror("cannot listen socket"); > + exit(4); > } > > (void) mevent_add(s, EVF_READ, acceptor_callback, NULL); > > Modified: head/usr.sbin/bhyve/pci_e82545.c > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D > --- head/usr.sbin/bhyve/pci_e82545.c Wed Jul 11 02:32:06 2018 > (r336187) > +++ head/usr.sbin/bhyve/pci_e82545.c Wed Jul 11 03:23:09 2018 > (r336188) > @@ -2224,7 +2224,7 @@ e82545_open_tap(struct e82545_softc *sc, char *opts= ) > sc->esc_tapfd =3D open(tbuf, O_RDWR); > if (sc->esc_tapfd =3D=3D -1) { > DPRINTF("unable to open tap device %s\n", opts); > - exit(1); > + exit(4); > } > > /* > > > > --=20 --=20 Marcelo Araujo (__)araujo@FreeBSD.org \\\'',)http://www.FreeBSD.org \/ \ ^ Power To Server. .\. /_)