On Fri, Feb 09, 2024 at 10:02:29AM -0500, Dave Voutila wrote:
> Try this diff. There was an issue in the order of closing disk fds. I
> also noticed we're not closing the sockets when closing the data fds, so
> that's added into virtio_dev_closefds().
>
> With this i can boot a guest that uses a network interface and a qcow2
> disk image with a base image.
This fixes my reproducer and real vm.conf with a derived .qcow2 image.
Good catch, I forgot to mention that.
> diff refs/heads/master refs/heads/vmd-fd-fix
> commit - 06bc238730aac28903aeab0d96b2427760b0110a
> commit + 8e46c12aa617cf136fdb3557f0177d41adb4d9d9
> blob - afe3dd8f7a48cde226a4438567a8a3eb9dac2dce
> blob + ce052097a463bed0e75775d7acb2f036ca111572
> --- usr.sbin/vmd/virtio.c
> +++ usr.sbin/vmd/virtio.c
> @@ -1301,8 +1301,8 @@ virtio_dev_launch(struct vmd_vm *vm, struct virtio_dev
> {
> char *nargv[12], num[32], vmm_fd[32], vm_name[VM_NAME_MAX], t[2];
> pid_t dev_pid;
> - int data_fds[VM_MAX_BASE_PER_DISK], sync_fds[2], async_fds[2], ret = 0;
> - size_t i, data_fds_sz, sz = 0;
> + int sync_fds[2], async_fds[2], ret = 0;
> + size_t sz = 0;
> struct viodev_msg msg;
> struct virtio_dev *dev_entry;
> struct imsg imsg;
> @@ -1310,14 +1310,10 @@ virtio_dev_launch(struct vmd_vm *vm, struct virtio_dev
>
> switch (dev->dev_type) {
> case VMD_DEVTYPE_NET:
> - data_fds[0] = dev->vionet.data_fd;
> - data_fds_sz = 1;
> log_debug("%s: launching vionet%d",
> vm->vm_params.vmc_params.vcp_name, dev->vionet.idx);
> break;
> case VMD_DEVTYPE_DISK:
> - memcpy(&data_fds, dev->vioblk.disk_fd, sizeof(data_fds));
> - data_fds_sz = dev->vioblk.ndisk_fd;
> log_debug("%s: launching vioblk%d",
> vm->vm_params.vmc_params.vcp_name, dev->vioblk.idx);
> break;
> @@ -1359,10 +1355,6 @@ virtio_dev_launch(struct vmd_vm *vm, struct virtio_dev
> dev->sync_fd = sync_fds[1];
> dev->async_fd = async_fds[1];
>
> - /* Close data fds. Only the child device needs them now. */
> - for (i = 0; i < data_fds_sz; i++)
> - close_fd(data_fds[i]);
> -
> /* 1. Send over our configured device. */
> log_debug("%s: sending '%c' type device struct", __func__,
> dev->dev_type);
> @@ -1373,6 +1365,13 @@ virtio_dev_launch(struct vmd_vm *vm, struct virtio_dev
> goto err;
> }
>
> + /* Close data fds. Only the child device needs them now. */
> + if (virtio_dev_closefds(dev) == -1) {
> + log_warnx("%s: failed to close device data fds",
> + __func__);
> + goto err;
> + }
> +
> /* 2. Send over details on the VM (including memory fds). */
> log_debug("%s: sending vm message for '%s'", __func__,
> vm->vm_params.vmc_params.vcp_name);
> @@ -1775,5 +1774,10 @@ virtio_dev_closefds(struct virtio_dev *dev)
> return (-1);
> }
>
> + close_fd(dev->async_fd);
> + dev->async_fd = -1;
> + close_fd(dev->sync_fd);
> + dev->sync_fd = -1;
> +
> return (0);
> }
