On Tue, 21 Aug 2018 23:03:36 -0700, Carlos Cardenas <[email protected]>
wrote:
> On Sun, Aug 19, 2018 at 11:46:12PM -0700, Ori Bernstein wrote:
> > One more minor update to this patch:
> >
> > - Remove unused enum
> > - Remove unused function prototype
> > - Move some qcow2-specific headers into the qcow2 patch.
>
> Ori, it's nice seeing good progress on this. I have a couple of
> questions inline below.
>
> Why are we making sz represent the number of 512 byte sectors? This may
> be true for hd disks but it needs to be *bytes* for ISO/cdrom.
Oops, was focused on hard disks, and misread what CDs did. Moved this division
out to the hard disk initialization. Fixed in both this and the qcow2, but
will see if you have any more comments on the qcow2 patch before I spam the
list with yet another minor revision.
> This has to be the number of bytes shifted to get the number of 2K
> blocks. As the patch is, this yields the incorrect number of blocks an
> ISO has. This breaks ISO/cdrom support.
Solved, see above -- we can now boot from and mount CD correctly.
> [snip]
> > +struct virtio_backing {
> > + void *p;
> > + char *path;
> > + ssize_t (*pread)(void *p, char *buf, size_t len, off_t off);
> > + ssize_t (*pwrite)(void *p, char *buf, size_t len, off_t off);
> > + void (*close)(void *p);
> > +};
>
> What is path used for? I don't see it used anywhere in this patch or in
> the qcow2 patch.
The intent was to use it for better error messages, especially since with
qcow2 snapshots you can smear the disk image across multiple files. I never
hooked it into any errors, and it's probably not worth the trouble.
Updated patch inline below:
---
usr.sbin/vmd/Makefile | 2 +-
usr.sbin/vmd/vioraw.c | 73 ++++++++++++++++++++++++++++++++++++++++++++++++++
usr.sbin/vmd/vioscsi.c | 7 +++--
usr.sbin/vmd/virtio.c | 56 +++++++++++++++++++++-----------------
usr.sbin/vmd/virtio.h | 16 ++++++++---
5 files changed, 124 insertions(+), 30 deletions(-)
create mode 100644 usr.sbin/vmd/vioraw.c
diff --git usr.sbin/vmd/Makefile usr.sbin/vmd/Makefile
index 7ea090f8886..24c1d1b1d4a 100644
--- usr.sbin/vmd/Makefile
+++ usr.sbin/vmd/Makefile
@@ -6,7 +6,7 @@ PROG= vmd
SRCS= vmd.c control.c log.c priv.c proc.c config.c vmm.c
SRCS+= vm.c loadfile_elf.c pci.c virtio.c i8259.c mc146818.c
SRCS+= ns8250.c i8253.c vmboot.c ufs.c disklabel.c dhcp.c packet.c
-SRCS+= parse.y atomicio.c vioscsi.c
+SRCS+= parse.y atomicio.c vioscsi.c vioraw.c
CFLAGS+= -Wall -I${.CURDIR}
CFLAGS+= -Wstrict-prototypes -Wmissing-prototypes
diff --git usr.sbin/vmd/vioraw.c usr.sbin/vmd/vioraw.c
new file mode 100644
index 00000000000..0dcb8b3c9c2
--- /dev/null
+++ usr.sbin/vmd/vioraw.c
@@ -0,0 +1,73 @@
+/* $OpenBSD: $ */
+/*
+ * Copyright (c) 2018 Ori Bernstein <[email protected]>
+ *
+ * Permission to use, copy, modify, and distribute this software for any
+ * purpose with or without fee is hereby granted, provided that the above
+ * copyright notice and this permission notice appear in all copies.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
+ * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
+ * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
+ * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
+ * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
+ * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
+ * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
+ */
+
+#include <sys/types.h>
+
+#include <machine/vmmvar.h>
+#include <dev/pci/pcireg.h>
+
+#include <stdlib.h>
+#include <unistd.h>
+
+#include "vmd.h"
+#include "vmm.h"
+#include "virtio.h"
+
+static ssize_t
+raw_pread(void *file, char *buf, size_t len, off_t off)
+{
+ return pread(*(int *)file, buf, len, off);
+}
+
+static ssize_t
+raw_pwrite(void *file, char *buf, size_t len, off_t off)
+{
+ return pwrite(*(int *)file, buf, len, off);
+}
+
+static void
+raw_close(void *file)
+{
+ close(*(int *)file);
+ free(file);
+}
+
+/*
+ * Initializes a raw disk image backing file from an fd.
+ * Stores the number of 512 byte sectors in *szp,
+ * returning -1 for error, 0 for success.
+ */
+int
+virtio_init_raw(struct virtio_backing *file, off_t *szp, int fd)
+{
+ off_t sz;
+ int *fdp;
+
+ sz = lseek(fd, 0, SEEK_END);
+ if (sz == -1)
+ return -1;
+
+ fdp = malloc(sizeof(int));
+ *fdp = fd;
+ file->p = fdp;
+ file->pread = raw_pread;
+ file->pwrite = raw_pwrite;
+ file->close = raw_close;
+ *szp = sz;
+ return 0;
+}
+
diff --git usr.sbin/vmd/vioscsi.c usr.sbin/vmd/vioscsi.c
index 93867887598..af504f0550d 100644
--- usr.sbin/vmd/vioscsi.c
+++ usr.sbin/vmd/vioscsi.c
@@ -197,7 +197,7 @@ vioscsi_start_read(struct vioscsi_dev *dev, off_t block,
ssize_t n_blocks)
goto nomem;
info->len = n_blocks * VIOSCSI_BLOCK_SIZE_CDROM;
info->offset = block * VIOSCSI_BLOCK_SIZE_CDROM;
- info->fd = dev->fd;
+ info->file = &dev->file;
return info;
@@ -210,7 +210,10 @@ nomem:
static const uint8_t *
vioscsi_finish_read(struct ioinfo *info)
{
- if (pread(info->fd, info->buf, info->len, info->offset) != info->len) {
+ struct virtio_backing *f;
+
+ f = info->file;
+ if (f->pread(f->p, info->buf, info->len, info->offset) != info->len) {
info->error = errno;
log_warn("vioscsi read error");
return NULL;
diff --git usr.sbin/vmd/virtio.c usr.sbin/vmd/virtio.c
index 4622ef4943f..b3a3116b569 100644
--- usr.sbin/vmd/virtio.c
+++ usr.sbin/vmd/virtio.c
@@ -361,7 +361,7 @@ vioblk_start_read(struct vioblk_dev *dev, off_t sector,
ssize_t sz)
goto nomem;
info->len = sz;
info->offset = sector * VIRTIO_BLK_SECTOR_SIZE;
- info->fd = dev->fd;
+ info->file = &dev->file;
return info;
@@ -375,7 +375,10 @@ nomem:
static const uint8_t *
vioblk_finish_read(struct ioinfo *info)
{
- if (pread(info->fd, info->buf, info->len, info->offset) != info->len) {
+ struct virtio_backing *file;
+
+ file = info->file;
+ if (file->pread(file->p, info->buf, info->len, info->offset) !=
info->len) {
info->error = errno;
log_warn("vioblk read error");
return NULL;
@@ -398,7 +401,7 @@ vioblk_start_write(struct vioblk_dev *dev, off_t sector,
goto nomem;
info->len = len;
info->offset = sector * VIRTIO_BLK_SECTOR_SIZE;
- info->fd = dev->fd;
+ info->file = &dev->file;
if (read_mem(addr, info->buf, len)) {
vioblk_free_info(info);
@@ -416,7 +419,10 @@ nomem:
static int
vioblk_finish_write(struct ioinfo *info)
{
- if (pwrite(info->fd, info->buf, info->len, info->offset) != info->len) {
+ struct virtio_backing *file;
+
+ file = info->file;
+ if (file->pwrite(file->p, info->buf, info->len, info->offset) !=
info->len) {
log_warn("vioblk write error");
return EIO;
}
@@ -1739,6 +1745,16 @@ vmmci_io(int dir, uint16_t reg, uint32_t *data, uint8_t
*intr,
return (0);
}
+static int
+virtio_init_disk(struct virtio_backing *file, off_t *sz, int fd)
+{
+ /*
+ * This is where we slot in disk type selection.
+ * Right now, there's only raw.
+ */
+ return virtio_init_raw(file, sz, fd);
+}
+
void
virtio_init(struct vmd_vm *vm, int child_cdrom, int *child_disks,
int *child_taps)
@@ -1748,7 +1764,6 @@ virtio_init(struct vmd_vm *vm, int child_cdrom, int
*child_disks,
uint8_t id;
uint8_t i;
int ret;
- off_t sz;
/* Virtio entropy device */
if (pci_add_device(&id, PCI_VENDOR_QUMRANET,
@@ -1789,9 +1804,6 @@ virtio_init(struct vmd_vm *vm, int child_cdrom, int
*child_disks,
/* One virtio block device for each disk defined in vcp */
for (i = 0; i < vcp->vcp_ndisks; i++) {
- if ((sz = lseek(child_disks[i], 0, SEEK_END)) == -1)
- continue;
-
if (pci_add_device(&id, PCI_VENDOR_QUMRANET,
PCI_PRODUCT_QUMRANET_VIO_BLOCK,
PCI_CLASS_MASS_STORAGE,
@@ -1815,13 +1827,15 @@ virtio_init(struct vmd_vm *vm, int child_cdrom, int
*child_disks,
sizeof(struct vring_desc) * VIOBLK_QUEUE_SIZE
+ sizeof(uint16_t) * (2 + VIOBLK_QUEUE_SIZE));
vioblk[i].vq[0].last_avail = 0;
- vioblk[i].fd = child_disks[i];
- vioblk[i].sz = sz / 512;
vioblk[i].cfg.device_feature = VIRTIO_BLK_F_SIZE_MAX;
vioblk[i].max_xfer = 1048576;
vioblk[i].pci_id = id;
vioblk[i].vm_id = vcp->vcp_id;
vioblk[i].irq = pci_get_dev_irq(id);
+ if (virtio_init_disk(&vioblk[i].file, &vioblk[i].sz,
+ child_disks[i]) == -1)
+ continue;
+ vioblk[i].sz /= 512;
}
}
@@ -1944,12 +1958,12 @@ virtio_init(struct vmd_vm *vm, int child_cdrom, int
*child_disks,
+ sizeof(uint16_t) * (2 + VIOSCSI_QUEUE_SIZE));
vioscsi->vq[i].last_avail = 0;
}
- sz = lseek(child_cdrom, 0, SEEK_END);
- vioscsi->fd = child_cdrom;
+ if (virtio_init_disk(&vioscsi->file, &vioscsi->sz,
+ child_cdrom) == -1)
+ return;
vioscsi->locked = 0;
vioscsi->lba = 0;
- vioscsi->sz = sz;
- vioscsi->n_blocks = sz >> 11; /* num of 2048 blocks in file */
+ vioscsi->n_blocks = vioscsi->sz >> 11; /* num of 2048 blocks in
file */
vioscsi->max_xfer = VIOSCSI_BLOCK_SIZE_CDROM;
vioscsi->pci_id = id;
vioscsi->vm_id = vcp->vcp_id;
@@ -2087,7 +2101,6 @@ int
vioblk_restore(int fd, struct vm_create_params *vcp, int *child_disks)
{
uint8_t i;
- off_t sz;
nr_vioblk = vcp->vcp_ndisks;
vioblk = calloc(vcp->vcp_ndisks, sizeof(struct vioblk_dev));
@@ -2103,16 +2116,15 @@ vioblk_restore(int fd, struct vm_create_params *vcp,
int *child_disks)
return (-1);
}
for (i = 0; i < vcp->vcp_ndisks; i++) {
- if ((sz = lseek(child_disks[i], 0, SEEK_END)) == -1)
- continue;
-
if (pci_set_bar_fn(vioblk[i].pci_id, 0, virtio_blk_io,
&vioblk[i])) {
log_warnx("%s: can't set bar fn for virtio block "
"device", __progname);
return (-1);
}
- vioblk[i].fd = child_disks[i];
+ if (virtio_init_disk(&vioblk[i].file, &vioblk[i].sz,
+ child_disks[i]) == -1)
+ continue;
}
return (0);
}
@@ -2120,8 +2132,6 @@ vioblk_restore(int fd, struct vm_create_params *vcp, int
*child_disks)
int
vioscsi_restore(int fd, struct vm_create_params *vcp, int child_cdrom)
{
- off_t sz;
-
if (!strlen(vcp->vcp_cdrom))
return (0);
@@ -2139,15 +2149,13 @@ vioscsi_restore(int fd, struct vm_create_params *vcp,
int child_cdrom)
return (-1);
}
- sz = lseek(child_cdrom, 0, SEEK_END);
-
if (pci_set_bar_fn(vioscsi->pci_id, 0, vioscsi_io, vioscsi)) {
log_warnx("%s: can't set bar fn for vmm control device",
__progname);
return (-1);
}
- vioscsi->fd = child_cdrom;
+ virtio_init_disk(&vioscsi->file, &vioscsi->sz, child_cdrom);
return (0);
}
diff --git usr.sbin/vmd/virtio.h usr.sbin/vmd/virtio.h
index 20327f9915c..3d2c94de1ce 100644
--- usr.sbin/vmd/virtio.h
+++ usr.sbin/vmd/virtio.h
@@ -61,6 +61,13 @@ struct virtio_io_cfg {
uint8_t isr_status;
};
+struct virtio_backing {
+ void *p;
+ ssize_t (*pread)(void *p, char *buf, size_t len, off_t off);
+ ssize_t (*pwrite)(void *p, char *buf, size_t len, off_t off);
+ void (*close)(void *p);
+};
+
/*
* A virtio device can have several virtqs. For example, vionet has one virtq
* each for transmitting and receiving packets. This struct describes the state
@@ -147,8 +154,8 @@ struct vioblk_dev {
struct virtio_io_cfg cfg;
struct virtio_vq_info vq[VIRTIO_MAX_QUEUES];
+ struct virtio_backing file;
- int fd;
uint64_t sz;
uint32_t max_xfer;
@@ -168,9 +175,10 @@ struct vioscsi_dev {
struct virtio_vq_info vq[VIRTIO_MAX_QUEUES];
+ struct virtio_backing file;
+
/* is the device locked */
int locked;
- int fd;
/* size of iso file in bytes */
uint64_t sz;
/* last block address read */
@@ -241,10 +249,10 @@ struct vmmci_dev {
};
struct ioinfo {
+ struct virtio_backing *file;
uint8_t *buf;
ssize_t len;
off_t offset;
- int fd;
int error;
};
@@ -261,6 +269,8 @@ void viornd_update_qs(void);
void viornd_update_qa(void);
int viornd_notifyq(void);
+int virtio_init_raw(struct virtio_backing *dev, off_t *sz, int fd);
+
int virtio_blk_io(int, uint16_t, uint32_t *, uint8_t *, void *, uint8_t);
int vioblk_dump(int);
int vioblk_restore(int, struct vm_create_params *, int *);
--
Ori Bernstein
