On Tue, Jul 03, 2018 at 08:59:29AM -0600, Theo de Raadt wrote:
> This incorrect codepath occurs in other places also. I think it
> ends up using an EINVAL from some other operation. Quite a ways
> earlier inside vmmaction(). Which is unrelated to the actual failure.
>
> The same problem is in vm_start_complete()
>
> pause_vm_complete() and unpause_vm_complete() use a different
> pattern to solve this detail:
>
> errno = res;
> warn("pause vm command failed");
>
> that may be preferrable to warnc?
>
> Anton Lindqvist <[email protected]> wrote:
>
> > Hi,
> > Stopping a VM owned by root as a non-root user fails with the following
> > error message:
> >
> > $ vmctl stop test
> > vmctl: terminate vm command failed: Invalid argument
> >
> > I think favoring warnc() with the appropriate errno number passed along
> > improves things. This of course is under the assumption that error codes
> > internal to vmd and errno:s are disjoint.
> >
> > $ ./obj/vmctl stop test
> > vmctl: terminate vm command failed: Operation not permitted
> >
> > Comments? OK?
> >
> > Index: vmctl.c
> > ===================================================================
> > RCS file: /cvs/src/usr.sbin/vmctl/vmctl.c,v
> > retrieving revision 1.49
> > diff -u -p -r1.49 vmctl.c
> > --- vmctl.c 19 Jun 2018 17:13:50 -0000 1.49
> > +++ vmctl.c 3 Jul 2018 08:46:25 -0000
> > @@ -465,7 +465,7 @@ terminate_vm_complete(struct imsg *imsg,
> > *ret = EIO;
> > break;
> > default:
> > - warn("terminate vm command failed");
> > + warnc(res, "terminate vm command failed");
> > *ret = EIO;
> > }
> > } else {
> >
>
In general, any effort spent here to improve things is welcome.
I think both approaches (warnc or making them look like the pause case as
pointed out by Theo) are fine. Anton, please choose one and go for it.
-ml
