> > Another thought --- is it necessary to call realpath at all?
> 
> Yes, vmd(8) will break if you remove it.
> 
> The caveat here is vmd(8) uses realpath and works with realpath, so the
> issue is really vmctl(8). I'm guessing in your test run you ran vmd from
> the directory with the disk image.

No, I ran it with rcctl stop vmd / rcctl start vmd.

I set up my vm.conf so that the path to the disk image includes a
symlink. It still worked.

vm "leth" {
  owner falsifian
  disable
  memory 4G
  disk "/home/falsifian/vm/leth/disk.qcow2"
  local interface
}

falsifian moth ~ $ ls -l /home/falsifian/vm/leth
lrwxr-xr-x  1 falsifian  falsifian  9 Mar 14 04:37 /home/falsifian/vm/leth -> 
leth.orig

And just to be sure I'm really using my changes to vmd, I tried adding
an "&& 0" to the "if (path[0] != '/') {" condition, and sure enough,
vmd fails to start my instance with
  vmctl: could not open disk image(s)

I also tried adding a second base image: disk.qcow2 is based on
disk_base.qcow2 which is based on disk_snapshot_2021-02-07.qcow2. vmd
still works, with my no-realpath patch.

Looking at the code in usr.sbin/vmd, I can't figure out why calling
realpath could matter, but I might have missed something. I think
virtio_qcow2_get_base is only called by virtio_get_base, which is only
called by config_setvm, and I think the output is only passed into an
"open" system call. From then on, the function only keeps the file
descriptor from open. But there's a lot going on in config_setvm and I
haven't read every line.

> The following diff lets vmctl(8) use a slightly different approach
> (similar to what you were getting at hitting vmd/vioqcow2.c with a
> hammer in your other email).
> 
> - If the base image path looks like an absolute path, i.e. starts with
>   '/', just try using it.
> - Otherwise, try the simplistic use of making it relative to the input
>   image file using a call to dirname(3).
> 
> This continues to let vmctl(8) use unveil(2) without having to taint the
> vmd(8) codebase with conditional unveil logic.

Your patch works for me, except the below hunk was rejected for some
reason.

To test: I briefly tried vmd, and I tried out vmctl create commands
with a symlink involved.

> @@ -729,7 +734,7 @@ virtio_qcow2_create(const char *imgfile_
>       if (ftruncate(fd, (off_t)initsz + clustersz) == -1)
>               goto error;
> 
> -     /*
> +     /*
>        * Paranoia: if our disk image takes more than one cluster
>        * to refcount the initial image, fail.
>        */

-- 
James

Reply via email to