On Thu, Mar 07, 2019 at 08:52:45PM +0100, Klemens Nanni wrote:
> vmd(8) does not support numerical names with `start' and `receive'.
> It never worked, the manuals are now clearer about this, but error
> handling can still be improved:
>
> $ vmctl start 60 -t test -d 60.qcow2
> vmctl: start vm command failed: No such file or directory
>
> That's from vm_start_complete() in vmctl.c:254 where vmd(8) unexpectedly
> returns EPERM after it failed to create the VM.
>
> $ vmctl receive 60 </dev/null
> vmctl: invalid vm name
> vmctl: invalid id: 60
>
> Here, first parse_vmid(), then it's caller ctl_receive() complains.
>
> parse_vmid()'s last argument is `needname', indicating that the id to be
> parsed must not be numerical.
>
> The following diff makes the code check for a letter in the beginning,
> adjusts `start' and `receive' set this argument and print only one error.
This is not right. Each started (or defined if in /etc/vm.conf) VM is assigned
an ID. That ID can be used in place of a name. That behaviour needs to continue
to work.
My advice is to just clean up the man page and error messages, nothing more.
-ml
>
> $ ./obj/vmctl start 60 -t test -d 60.qcow2
> vmctl: invalid VM name
> $ ./obj/vmctl receive 60 </dev/null
> vmctl: invalid VM name
>
> "vm" -> "VM" for consistency with all other "invalid VM name" occurences
> throughout the sources.
>
> Feedback? OK?
>
> Index: usr.sbin/vmctl/main.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/vmctl/main.c,v
> retrieving revision 1.54
> diff -u -p -r1.54 main.c
> --- usr.sbin/vmctl/main.c 1 Mar 2019 12:47:36 -0000 1.54
> +++ usr.sbin/vmctl/main.c 7 Mar 2019 19:14:15 -0000
> @@ -524,7 +524,7 @@ parse_vmid(struct parse_result *res, cha
> id = strtonum(word, 0, UINT32_MAX, &error);
> if (error == NULL) {
> if (needname) {
> - warnx("invalid vm name");
> + warnx("invalid VM name");
> return (-1);
> } else {
> res->id = id;
> @@ -842,8 +842,8 @@ ctl_start(struct parse_result *res, int
> if (argc < 2)
> ctl_usage(res->ctl);
>
> - if (parse_vmid(res, argv[1], 0) == -1)
> - errx(1, "invalid id: %s", argv[1]);
> + if (parse_vmid(res, argv[1], 1) == -1)
> + exit(1);
>
> argc--;
> argv++;
> @@ -1046,7 +1046,7 @@ ctl_receive(struct parse_result *res, in
> err(1, "pledge");
> if (argc == 2) {
> if (parse_vmid(res, argv[1], 1) == -1)
> - errx(1, "invalid id: %s", argv[1]);
> + exit(1);
> } else if (argc != 2)
> ctl_usage(res->ctl);
>
> 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 7 Mar 2019 19:14:15 -0000
> @@ -154,12 +154,7 @@ vm_start(uint32_t start_id, const char *
> }
> }
> if (name != NULL) {
> - /*
> - * Allow VMs names with alphanumeric characters, dot, hyphen
> - * and underscore. But disallow dot, hyphen and underscore at
> - * the start.
> - */
> - if (*name == '-' || *name == '.' || *name == '_')
> + if (!isalpha(*name))
> errx(1, "invalid VM name");
>
> for (s = name; *s != '\0'; ++s) {
> Index: usr.sbin/vmd/vmd.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/vmd/vmd.c,v
> retrieving revision 1.108
> diff -u -p -r1.108 vmd.c
> --- usr.sbin/vmd/vmd.c 9 Dec 2018 12:26:38 -0000 1.108
> +++ usr.sbin/vmd/vmd.c 7 Mar 2019 19:14:15 -0000
> @@ -1257,11 +1257,7 @@ vm_register(struct privsep *ps, struct v
> vcp->vcp_ndisks == 0 && strlen(vcp->vcp_cdrom) == 0) {
> log_warnx("no kernel or disk/cdrom specified");
> goto fail;
> - } else if (strlen(vcp->vcp_name) == 0) {
> - log_warnx("invalid VM name");
> - goto fail;
> - } else if (*vcp->vcp_name == '-' || *vcp->vcp_name == '.' ||
> - *vcp->vcp_name == '_') {
> + } else if (strlen(vcp->vcp_name) == 0 || !isalpha(*vcp->vcp_name)) {
> log_warnx("invalid VM name");
> goto fail;
> } else {
> ===================================================================
> Stats: --- 15 lines 531 chars
> Stats: +++ 6 lines 185 chars
> Stats: -9 lines
> Stats: -346 chars
>
