On Mon, Apr 01, 2019 at 06:05:38PM +0200, Klemens Nanni wrote:
> As of now, `vmctl status test' will tell you whether the VM is running
> or not; except that "STATE" actually denotes whether the VCPU is
> currently running or haltet, not whether the VM is started/running or
> stopped.
>
> I tripped over this when trying to use
>
> vmctl status test | fgrep 'STATE: RUNNING'
>
> as reliable check, where it therefore only worked when the VM was busy
> CPU wise. According to mlarkin, this was only ever intended as internal
> debugging aid anyway.
>
> The following diff makes vmctl(8) switch it's state indicator from the
> actual VCPU state to whether the VM has a valid process ID, that is
> simply whether the respective host process runs.
>
> Since there's only ever one VCPU per VM for now, drop the constant
> "VCPU: 0" all together and stick with "STATE: RUNNING" or
> "STATE: STOPPED" on it's own line. That makes it easily scriptable.
>
>
> Given that the current behaviour is only vaguely documented in vmctl(8),
> I do not expect users to already depend on it; if so: now would be the
> time to change that into something consistent. I have not come up with
> a nice update to the manual yet, suggestions welcome.
>
>
> Feedback? OK?
>
I am ok with this. Go for it unless someone privately objected.
-ml
> Index: usr.sbin/vmctl//vmctl.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/vmctl/vmctl.c,v
> retrieving revision 1.65
> diff -u -p -r1.65 vmctl.c
> --- usr.sbin/vmctl//vmctl.c 6 Dec 2018 09:23:15 -0000 1.65
> +++ usr.sbin/vmctl//vmctl.c 1 Apr 2019 15:01:34 -0000
> @@ -716,12 +716,13 @@ print_vm_info(struct vmop_info_result *l
> {
> struct vm_info_result *vir;
> struct vmop_info_result *vmi;
> - size_t i, j;
> - char *vcpu_state, *tty;
> + size_t i;
> + char *tty;
> char curmem[FMT_SCALED_STRSIZE];
> char maxmem[FMT_SCALED_STRSIZE];
> char user[16], group[16];
> const char *name;
> + int running;
>
> printf("%5s %5s %5s %7s %7s %7s %12s %s\n", "ID", "PID", "VCPUS",
> "MAXMEM", "CURMEM", "TTY", "OWNER", "NAME");
> @@ -729,6 +730,7 @@ print_vm_info(struct vmop_info_result *l
> for (i = 0; i < ct; i++) {
> vmi = &list[i];
> vir = &vmi->vir_info;
> + running = (vir->vir_creator_pid != 0 && vir->vir_id != 0);
> if (check_info_id(vir->vir_name, vir->vir_id)) {
> /* get user name */
> name = user_from_uid(vmi->vir_uid, 1);
> @@ -757,7 +759,7 @@ print_vm_info(struct vmop_info_result *l
> (void)fmt_scaled(vir->vir_memory_size * 1024 * 1024,
> maxmem);
>
> - if (vir->vir_creator_pid != 0 && vir->vir_id != 0) {
> + if (running) {
> if (*vmi->vir_ttyname == '\0')
> tty = "-";
> /* get tty - skip /dev/ path */
> @@ -781,19 +783,8 @@ print_vm_info(struct vmop_info_result *l
> }
> }
> if (check_info_id(vir->vir_name, vir->vir_id) > 0) {
> - for (j = 0; j < vir->vir_ncpus; j++) {
> - if (vir->vir_vcpu_state[j] ==
> - VCPU_STATE_STOPPED)
> - vcpu_state = "STOPPED";
> - else if (vir->vir_vcpu_state[j] ==
> - VCPU_STATE_RUNNING)
> - vcpu_state = "RUNNING";
> - else
> - vcpu_state = "UNKNOWN";
> -
> - printf(" VCPU: %2zd STATE: %s\n",
> - j, vcpu_state);
> - }
> + printf("STATE: %s\n",
> + running ? "RUNNING" : "STOPPED");
> }
> }
> }
> ===================================================================
> Stats: --- 16 lines 430 chars
> Stats: +++ 7 lines 184 chars
> Stats: -9 lines
> Stats: -246 chars
>