Various clean up items for disks.

- qcow2: general cleanup
- vioraw: check malloc
- virtio: add function to sync disks
- vm: call virtio_shutdown to sync disks when vm is finished executing

Thanks to Ori Bernstein.

Ok miko@

5 files changed, 62 insertions, 26 deletions

diff --git a/usr.sbin/vmd/vioqcow2.c b/usr.sbin/vmd/vioqcow2.c
index 7c7e4857695..ec2bbedd7df 100644
--- a/usr.sbin/vmd/vioqcow2.c
+++ b/usr.sbin/vmd/vioqcow2.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: vioqcow2.c,v 1.2 2018/09/11 04:06:32 ccardenas Exp $	*/
+/*	$OpenBSD: vioqcow2.c,v 1.3 2018/09/19 04:29:21 ccardenas Exp $	*/
 
 /*
  * Copyright (c) 2018 Ori Bernstein <ori@eigenstate.org>
@@ -77,7 +77,6 @@ struct qcdisk {
 
 	int       fd;
 	uint64_t *l1;
-	char     *scratch;
 	off_t     end;
 	uint32_t  clustersz;
 	off_t	  disksz; /* In bytes */
@@ -161,17 +160,19 @@ qc2_open(struct qcdisk *disk, int fd)
 	size_t i;
 	int version;
 
+	pthread_rwlock_init(&disk->lock, NULL);
+	disk->fd = fd;
+	disk->base = NULL;
+	disk->l1 = NULL;
+
 	if (pread(fd, &header, sizeof header, 0) != sizeof header) {
 		log_warn("%s: short read on header", __func__);
-		return -1;
+		goto error;
 	}
 	if (strncmp(header.magic, "QFI\xfb", 4) != 0) {
 		log_warn("%s: invalid magic numbers", __func__);
-		return -1;
+		goto error;
 	}
-	pthread_rwlock_init(&disk->lock, NULL);
-	disk->fd = fd;
-	disk->base = NULL;
 
 	disk->clustersz		= (1ull << be32toh(header.clustershift));
 	disk->disksz		= be64toh(header.disksz);
@@ -182,6 +183,7 @@ qc2_open(struct qcdisk *disk, int fd)
 	disk->refoff		= be64toh(header.refoff);
 	disk->nsnap		= be32toh(header.snapcount);
 	disk->snapoff		= be64toh(header.snapsz);
+
 	/*
 	 * The additional features here are defined as 0 in the v2 format,
 	 * so as long as we clear the buffer before parsing, we don't need
@@ -196,23 +198,25 @@ qc2_open(struct qcdisk *disk, int fd)
 	 * We only know about the dirty or corrupt bits here.
 	 */
 	if (disk->incompatfeatures & ~(QCOW2_DIRTY|QCOW2_CORRUPT)) {
-		log_warn("%s: unsupported features %llx", __func__,
+		log_warnx("%s: unsupported features %llx", __func__,
 		    disk->incompatfeatures & ~(QCOW2_DIRTY|QCOW2_CORRUPT));
-		return -1;
+		goto error;
 	}
 
 	disk->l1 = calloc(disk->l1sz, sizeof *disk->l1);
-	if (pread(disk->fd, (char*)disk->l1, 8*disk->l1sz, disk->l1off)
+	if (!disk->l1)
+		goto error;
+	if (pread(disk->fd, disk->l1, 8*disk->l1sz, disk->l1off)
 	    != 8*disk->l1sz) {
-		free(disk->l1);
-		return -1;
+		log_warn("%s: unable to read qcow2 L1 table", __func__);
+		goto error;
 	}
 	for (i = 0; i < disk->l1sz; i++)
 		disk->l1[i] = be64toh(disk->l1[i]);
 	version = be32toh(header.version);
 	if (version != 2 && version != 3) {
 		log_warn("%s: unknown qcow2 version %d", __func__, version);
-		return -1;
+		goto error;
 	}
 
 	backingoff = be64toh(header.backingoff);
@@ -223,34 +227,47 @@ qc2_open(struct qcdisk *disk, int fd)
 		 * otherwise we just crash with a pledge violation.
 		 */
 		log_warn("%s: unsupported external snapshot images", __func__);
-		return -1;
+		goto error;
 
 		if (backingsz >= sizeof basepath - 1) {
 			log_warn("%s: snapshot path too long", __func__);
-			return -1;
+			goto error;
 		}
 		if (pread(fd, basepath, backingsz, backingoff) != backingsz) {
 			log_warn("%s: could not read snapshot base name",
 			    __func__);
-			return -1;
+			goto error;
 		}
 		basepath[backingsz] = 0;
 
 		disk->base = calloc(1, sizeof(struct qcdisk));
+		if (!disk->base)
+			goto error;
 		if (qc2_openpath(disk->base, basepath, O_RDONLY) == -1) {
-			free(disk->base);
-			return -1;
+			log_warn("%s: could not open %s", basepath, __func__);
+			goto error;
 		}
 		if (disk->base->clustersz != disk->clustersz) {
 			log_warn("%s: all disks must share clustersize",
 			    __func__);
-			free(disk->base);
-			return -1;
+			goto error;
 		}
 	}
-	fstat(fd, &st);
+	if (fstat(fd, &st) == -1) {
+		log_warn("%s: unable to stat disk", __func__);
+		goto error;
+	}
+
 	disk->end = st.st_size;
+
+	log_debug("opened qcow2 disk version %d:", version);
+	log_debug("size:\t%lld", disk->disksz);
+	log_debug("end:\t%lld", disk->end);
+	log_debug("nsnap:\t%d", disk->nsnap);
 	return 0;
+error:
+	qc2_close(disk);
+	return -1;
 }
 
 static ssize_t
@@ -362,8 +379,10 @@ qc2_close(void *p)
 	struct qcdisk *disk;
 
 	disk = p;
-	pwrite(disk->fd, disk->l1, disk->l1sz, disk->l1off);
+	if (disk->base)
+		qc2_close(disk->base);
 	close(disk->fd);
+	free(disk->l1);
 	free(disk);
 }
 
diff --git a/usr.sbin/vmd/vioraw.c b/usr.sbin/vmd/vioraw.c
index 4b87c649407..621d7a6854d 100644
--- a/usr.sbin/vmd/vioraw.c
+++ b/usr.sbin/vmd/vioraw.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: vioraw.c,v 1.1 2018/08/25 04:16:09 ccardenas Exp $	*/
+/*	$OpenBSD: vioraw.c,v 1.2 2018/09/19 04:29:21 ccardenas Exp $	*/
 /*
  * Copyright (c) 2018 Ori Bernstein <ori@eigenstate.org>
  *
@@ -62,6 +62,8 @@ virtio_init_raw(struct virtio_backing *file, off_t *szp, int fd)
 		return -1;
 
 	fdp = malloc(sizeof(int));
+	if (!fdp)
+		return -1;
 	*fdp = fd;
 	file->p = fdp;
 	file->pread = raw_pread;
diff --git a/usr.sbin/vmd/virtio.c b/usr.sbin/vmd/virtio.c
index cdff71aed69..05734cfae34 100644
--- a/usr.sbin/vmd/virtio.c
+++ b/usr.sbin/vmd/virtio.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: virtio.c,v 1.68 2018/09/13 04:23:36 pd Exp $	*/
+/*	$OpenBSD: virtio.c,v 1.69 2018/09/19 04:29:21 ccardenas Exp $	*/
 
 /*
  * Copyright (c) 2015 Mike Larkin <mlarkin@openbsd.org>
@@ -2009,6 +2009,17 @@ virtio_init(struct vmd_vm *vm, int child_cdrom, int *child_disks,
 	evtimer_set(&vmmci.timeout, vmmci_timeout, NULL);
 }
 
+void
+virtio_shutdown(struct vmd_vm *vm)
+{
+	int i;
+
+	/* ensure that our disks are synced */
+	vioscsi->file.close(vioscsi->file.p);
+	for (i = 0; i < nr_vioblk; i++)
+		vioblk[i].file.close(vioblk[i].file.p);
+}
+
 int
 vmmci_restore(int fd, uint32_t vm_id)
 {
diff --git a/usr.sbin/vmd/virtio.h b/usr.sbin/vmd/virtio.h
index 86ee6d21f9f..2cbb0d627eb 100644
--- a/usr.sbin/vmd/virtio.h
+++ b/usr.sbin/vmd/virtio.h
@@ -1,4 +1,4 @@
-/*	$OpenBSD: virtio.h,v 1.28 2018/09/09 04:09:32 ccardenas Exp $	*/
+/*	$OpenBSD: virtio.h,v 1.29 2018/09/19 04:29:21 ccardenas Exp $	*/
 
 /*
  * Copyright (c) 2015 Mike Larkin <mlarkin@openbsd.org>
@@ -258,6 +258,7 @@ struct ioinfo {
 
 /* virtio.c */
 void virtio_init(struct vmd_vm *, int, int *, int *);
+void virtio_shutdown(struct vmd_vm *);
 int virtio_dump(int);
 int virtio_restore(int, struct vmd_vm *, int, int *, int *);
 uint32_t vring_size(uint32_t);
diff --git a/usr.sbin/vmd/vm.c b/usr.sbin/vmd/vm.c
index 046b2be8503..99acb3025f2 100644
--- a/usr.sbin/vmd/vm.c
+++ b/usr.sbin/vmd/vm.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: vm.c,v 1.38 2018/07/17 13:47:06 mlarkin Exp $	*/
+/*	$OpenBSD: vm.c,v 1.39 2018/09/19 04:29:21 ccardenas Exp $	*/
 
 /*
  * Copyright (c) 2015 Mike Larkin <mlarkin@openbsd.org>
@@ -371,6 +371,9 @@ start_vm(struct vmd_vm *vm, int fd)
 	/* Execute the vcpu run loop(s) for this VM */
 	ret = run_vm(vm->vm_cdrom, vm->vm_disks, nicfds, &vm->vm_params, &vrs);
 
+	/* Ensure that any in-flight data is written back */
+	virtio_shutdown(vm);
+
 	return (ret);
 }