Re: vmctl: start: Require one interface at minimium with -i
On Sat, Oct 26, 2019 at 12:57:56AM +0200, Klemens Nanni wrote:
> It makes no sense to allow zero interfaces; either a positive count is
> given or -i is omitted entirely. vm.conf(5) does not allow interface
> configuration that results in zero interfaces either.
>
> $ doas vmctl start -Li0 -b ~/bsd.rd foo
> vmctl: starting without disks
> vmctl: started vm 6 successfully, tty /dev/ttyph
>
> Diff below raises the minimum to one and tells more about invalid counts
> with the usual strtonum(3) idiom:
>
> $ doas vmctl start -Li-1 -b ~/bsd.rd foo
> vmctl: invalid count "-1": too small
> vmctl: invalid interface count: -1
> $ doas ./obj/vmctl start -Li0 -b ~/bsd.rd foo
> vmctl: count is too small: 0
> vmctl: invalid interface count: 0
>
> Those duplicate errors aren't easily merged without breaking consistency
> with how all the command line flags are passed, so I did nothing about
> this.
>
> OK?
>
>
> Index: main.c
> ===
> RCS file: /cvs/src/usr.sbin/vmctl/main.c,v
> retrieving revision 1.58
> diff -u -p -r1.58 main.c
> --- main.c23 Aug 2019 07:55:20 - 1.58
> +++ main.c25 Oct 2019 22:48:49 -
> @@ -373,9 +373,9 @@ parse_ifs(struct parse_result *res, char
> const char *error;
>
> if (word != NULL) {
> - val = strtonum(word, 0, INT_MAX, );
> + val = strtonum(word, 1, INT_MAX, );
> if (error != NULL) {
> - warnx("invalid count \"%s\": %s", word, error);
> + warnx("count is %s: %s", error, word);
> return (-1);
> }
> }
>
ok mlarkin@ if you are still looking to do this.
Re: vmctl: start: Require one interface at minimium with -i
On Sat, Oct 26, 2019 at 10:18:21AM +, Reyk Floeter wrote: > - Do you want to prevent "-i 0" and "interfaces 0" as it is equal to > not specifying these options at all or do you want to prevent vmd from > even running a VM without interfaces? The first; VMs without network must be possible. > - It is true, the optional "interfaces" keyword in vm.conf cannot be > 0. The main reason is that it should prevent that the same keyword is > configured for multiple times. This could be the same for -i. It already is: $ doas vmctl start -Li1 -i0 -b ./bsd.rd test vmctl: interfaces specified multiple times > > - VMs can be started without interfaces: vmd prints a warning but > doesn't prevent it. While the use case without any interfaces seems > exotic, I also don't see a reason to prevent it. I did have PCs > without NICs in the past ;-) I use it all the time and do not want to change it.
Re: vmctl: start: Require one interface at minimium with -i
On Sat, Oct 26, 2019 at 12:57:56AM +0200, Klemens Nanni wrote:
> It makes no sense to allow zero interfaces; either a positive count is
> given or -i is omitted entirely. vm.conf(5) does not allow interface
> configuration that results in zero interfaces either.
>
> $ doas vmctl start -Li0 -b ~/bsd.rd foo
> vmctl: starting without disks
> vmctl: started vm 6 successfully, tty /dev/ttyph
>
I'm not sure if I agree with the context here.
- Do you want to prevent "-i 0" and "interfaces 0" as it is equal to
not specifying these options at all or do you want to prevent vmd from
even running a VM without interfaces?
- It is true, the optional "interfaces" keyword in vm.conf cannot be
0. The main reason is that it should prevent that the same keyword is
configured for multiple times. This could be the same for -i.
- VMs can be started without interfaces: vmd prints a warning but
doesn't prevent it. While the use case without any interfaces seems
exotic, I also don't see a reason to prevent it. I did have PCs
without NICs in the past ;-)
- Regarding -n and -L: The -i X option never lowers the number of
interfaces. It is a "minimum number of unconfigured interfaces"
option. Have a look at vmctl/main.c L913-919. So -Li0 will work just
fine, even if the -i0 is totally pointless.q
Besides that, I have no strong oppinion against the diff itself, OK.
Reyk
> Diff below raises the minimum to one and tells more about invalid counts
> with the usual strtonum(3) idiom:
>
> $ doas vmctl start -Li-1 -b ~/bsd.rd foo
> vmctl: invalid count "-1": too small
> vmctl: invalid interface count: -1
> $ doas ./obj/vmctl start -Li0 -b ~/bsd.rd foo
> vmctl: count is too small: 0
> vmctl: invalid interface count: 0
>
> Those duplicate errors aren't easily merged without breaking consistency
> with how all the command line flags are passed, so I did nothing about
> this.
>
> OK?
>
>
> Index: main.c
> ===
> RCS file: /cvs/src/usr.sbin/vmctl/main.c,v
> retrieving revision 1.58
> diff -u -p -r1.58 main.c
> --- main.c23 Aug 2019 07:55:20 - 1.58
> +++ main.c25 Oct 2019 22:48:49 -
> @@ -373,9 +373,9 @@ parse_ifs(struct parse_result *res, char
> const char *error;
>
> if (word != NULL) {
> - val = strtonum(word, 0, INT_MAX, );
> + val = strtonum(word, 1, INT_MAX, );
> if (error != NULL) {
> - warnx("invalid count \"%s\": %s", word, error);
> + warnx("count is %s: %s", error, word);
> return (-1);
> }
> }
>
vmctl: start: Require one interface at minimium with -i
It makes no sense to allow zero interfaces; either a positive count is
given or -i is omitted entirely. vm.conf(5) does not allow interface
configuration that results in zero interfaces either.
$ doas vmctl start -Li0 -b ~/bsd.rd foo
vmctl: starting without disks
vmctl: started vm 6 successfully, tty /dev/ttyph
Diff below raises the minimum to one and tells more about invalid counts
with the usual strtonum(3) idiom:
$ doas vmctl start -Li-1 -b ~/bsd.rd foo
vmctl: invalid count "-1": too small
vmctl: invalid interface count: -1
$ doas ./obj/vmctl start -Li0 -b ~/bsd.rd foo
vmctl: count is too small: 0
vmctl: invalid interface count: 0
Those duplicate errors aren't easily merged without breaking consistency
with how all the command line flags are passed, so I did nothing about
this.
OK?
Index: main.c
===
RCS file: /cvs/src/usr.sbin/vmctl/main.c,v
retrieving revision 1.58
diff -u -p -r1.58 main.c
--- main.c 23 Aug 2019 07:55:20 - 1.58
+++ main.c 25 Oct 2019 22:48:49 -
@@ -373,9 +373,9 @@ parse_ifs(struct parse_result *res, char
const char *error;
if (word != NULL) {
- val = strtonum(word, 0, INT_MAX, );
+ val = strtonum(word, 1, INT_MAX, );
if (error != NULL) {
- warnx("invalid count \"%s\": %s", word, error);
+ warnx("count is %s: %s", error, word);
return (-1);
}
}
