Commit eeb1650c authored by Dariusz Stojaczyk's avatar Dariusz Stojaczyk Committed by Jim Harris
Browse files

virtio: propagate error codes 



Many functions were inconsistent in regard of error codes
they returned. In a couple of places we could print "Operation
not permitted" via strerror() from functions that were
supposed to return negative return codes, but returned
`-1` instead. (-EPERM == -1)

Change-Id: I7c36e54d449352dbd8036746e4f44a65c9b1d0b3
Signed-off-by: default avatarDariusz Stojaczyk <dariuszx.stojaczyk@intel.com>
Reviewed-on: https://review.gerrithub.io/419007


Tested-by: default avatarSPDK CI Jenkins <sys_sgci@intel.com>
Chandler-Test-Pool: SPDK Automated Test System <sys_sgsw@intel.com>
Reviewed-by: default avatarBen Walker <benjamin.walker@intel.com>
Reviewed-by: default avatarJim Harris <james.r.harris@intel.com>
parent 813ed709

lib/virtio/virtio.c

+8 −6
Original line number Diff line number Diff line
@@ -118,6 +118,7 @@ virtio_init_queue(struct virtio_dev *dev, uint16_t vtpci_queue_idx) 
{

	unsigned int vq_size, size;

	struct virtqueue *vq;

	int rc;



	SPDK_DEBUGLOG(SPDK_LOG_VIRTIO_DEV, "setting up queue: %"PRIu16"\n", vtpci_queue_idx);
@@ -163,11 +164,12 @@ virtio_init_queue(struct virtio_dev *dev, uint16_t vtpci_queue_idx) 


	vq->owner_thread = NULL;



	if (virtio_dev_backend_ops(dev)->setup_queue(dev, vq) < 0) {

	rc = virtio_dev_backend_ops(dev)->setup_queue(dev, vq);

	if (rc < 0) {

		SPDK_ERRLOG("setup_queue failed\n");

		spdk_dma_free(vq);

		dev->vqs[vtpci_queue_idx] = NULL;

		return -EINVAL;

		return rc;

	}



	SPDK_DEBUGLOG(SPDK_LOG_VIRTIO_DEV, "vq->vq_ring_mem:      0x%" PRIx64 "\n",
@@ -255,7 +257,7 @@ virtio_negotiate_features(struct virtio_dev *dev, uint64_t req_features) 
	rc = virtio_dev_backend_ops(dev)->set_features(dev, req_features & host_features);

	if (rc != 0) {

		SPDK_ERRLOG("failed to negotiate device features.\n");

		return -1;

		return rc;

	}



	SPDK_DEBUGLOG(SPDK_LOG_VIRTIO_DEV, "negotiated features = %" PRIx64 "\n",
@@ -264,7 +266,7 @@ virtio_negotiate_features(struct virtio_dev *dev, uint64_t req_features) 
	virtio_dev_set_status(dev, VIRTIO_CONFIG_S_FEATURES_OK);

	if (!(virtio_dev_get_status(dev) & VIRTIO_CONFIG_S_FEATURES_OK)) {

		SPDK_ERRLOG("failed to set FEATURES_OK status!\n");

		return -1;

		return -EINVAL;

	}



	return 0;
@@ -303,13 +305,13 @@ virtio_dev_reset(struct virtio_dev *dev, uint64_t req_features) 
	virtio_dev_set_status(dev, VIRTIO_CONFIG_S_ACKNOWLEDGE);

	if (!(virtio_dev_get_status(dev) & VIRTIO_CONFIG_S_ACKNOWLEDGE)) {

		SPDK_ERRLOG("Failed to set VIRTIO_CONFIG_S_ACKNOWLEDGE status.\n");

		return -1;

		return -EIO;

	}



	virtio_dev_set_status(dev, VIRTIO_CONFIG_S_DRIVER);

	if (!(virtio_dev_get_status(dev) & VIRTIO_CONFIG_S_DRIVER)) {

		SPDK_ERRLOG("Failed to set VIRTIO_CONFIG_S_DRIVER status.\n");

		return -1;

		return -EIO;

	}



	return virtio_negotiate_features(dev, req_features);

lib/virtio/virtio_pci.c

+9 −5
Original line number Diff line number Diff line
@@ -206,7 +206,7 @@ modern_set_features(struct virtio_dev *dev, uint64_t features) 


	if ((features & (1ULL << VIRTIO_F_VERSION_1)) == 0) {

		SPDK_ERRLOG("VIRTIO_F_VERSION_1 feature is not enabled.\n");

		return -1;

		return -EINVAL;

	}



	spdk_mmio_write_4(&hw->common_cfg->guest_feature_select, 0);
@@ -282,7 +282,7 @@ modern_setup_queue(struct virtio_dev *dev, struct virtqueue *vq) 


	if (!check_vq_phys_addr_ok(vq)) {

		spdk_dma_free(queue_mem);

		return -1;

		return -ENOMEM;

	}



	desc_addr = vq->vq_ring_mem;
@@ -398,7 +398,7 @@ virtio_read_caps(struct virtio_hw *hw) 
	ret = spdk_pci_device_cfg_read(hw->pci_dev, &pos, 1, PCI_CAPABILITY_LIST);

	if (ret < 0) {

		SPDK_DEBUGLOG(SPDK_LOG_VIRTIO_PCI, "failed to read pci capability list\n");

		return -1;

		return ret;

	}



	while (pos) {
@@ -447,7 +447,11 @@ next: 
	if (hw->common_cfg == NULL || hw->notify_base == NULL ||

	    hw->dev_cfg == NULL    || hw->isr == NULL) {

		SPDK_DEBUGLOG(SPDK_LOG_VIRTIO_PCI, "no modern virtio pci device found.\n");

		return -1;

		if (ret < 0) {

			return ret;

		} else {

			return -EINVAL;

		}

	}



	SPDK_DEBUGLOG(SPDK_LOG_VIRTIO_PCI, "found modern virtio pci device.\n");
@@ -574,7 +578,7 @@ virtio_pci_dev_init(struct virtio_dev *vdev, const char *name, 


	rc = virtio_dev_construct(vdev, name, &modern_ops, pci_ctx);

	if (rc != 0) {

		return -1;

		return rc;

	}



	vdev->is_hw = 1;

lib/virtio/virtio_user.c

+51 −42
Original line number Diff line number Diff line
@@ -128,11 +128,13 @@ virtio_user_queue_setup(struct virtio_dev *vdev, 
			int (*fn)(struct virtio_dev *, uint32_t))

{

	uint32_t i;

	int rc;



	for (i = 0; i < vdev->max_queues; ++i) {

		if (fn(vdev, i) < 0) {

		rc = fn(vdev, i);

		if (rc < 0) {

			SPDK_ERRLOG("setup tx vq fails: %"PRIu32".\n", i);

			return -1;

			return rc;

		}

	}
@@ -158,7 +160,7 @@ virtio_user_start_device(struct virtio_dev *vdev) 
	/* negotiate the number of I/O queues. */

	ret = dev->ops->send_request(dev, VHOST_USER_GET_QUEUE_NUM, &host_max_queues);

	if (ret < 0) {

		return -1;

		return ret;

	}



	if (vdev->max_queues > host_max_queues + vdev->fixed_queues_num) {
@@ -170,19 +172,21 @@ virtio_user_start_device(struct virtio_dev *vdev) 
	}



	/* tell vhost to create queues */

	if (virtio_user_queue_setup(vdev, virtio_user_create_queue) < 0) {

		return -1;

	ret = virtio_user_queue_setup(vdev, virtio_user_create_queue);

	if (ret < 0) {

		return ret;

	}



	/* share memory regions */

	ret = dev->ops->send_request(dev, VHOST_USER_SET_MEM_TABLE, NULL);

	if (ret < 0) {

		return -1;

		return ret;

	}



	/* kick queues */

	if (virtio_user_queue_setup(vdev, virtio_user_kick_queue) < 0) {

		return -1;

	ret = virtio_user_queue_setup(vdev, virtio_user_kick_queue);

	if (ret < 0) {

		return ret;

	}



	return 0;
@@ -208,11 +212,7 @@ virtio_user_dev_setup(struct virtio_dev *vdev) 


	dev->ops = &ops_user;



	if (dev->ops->setup(dev) < 0) {

		return -1;

	}



	return 0;

	return dev->ops->setup(dev);

}



static int
@@ -221,6 +221,7 @@ virtio_user_read_dev_config(struct virtio_dev *vdev, size_t offset, 
{

	struct virtio_user_dev *dev = vdev->ctx;

	struct vhost_user_config cfg = {0};

	int rc;



	if ((dev->protocol_features & (1ULL << VHOST_USER_PROTOCOL_F_CONFIG)) == 0) {

		return -ENOTSUP;
@@ -229,9 +230,10 @@ virtio_user_read_dev_config(struct virtio_dev *vdev, size_t offset, 
	cfg.offset = 0;

	cfg.size = VHOST_USER_MAX_CONFIG_SIZE;



	if (dev->ops->send_request(dev, VHOST_USER_GET_CONFIG, &cfg) < 0) {

		SPDK_ERRLOG("get_config failed: %s\n", spdk_strerror(errno));

		return -errno;

	rc = dev->ops->send_request(dev, VHOST_USER_GET_CONFIG, &cfg);

	if (rc < 0) {

		SPDK_ERRLOG("get_config failed: %s\n", spdk_strerror(-rc));

		return rc;

	}



	memcpy(dst, cfg.region + offset, length);
@@ -244,6 +246,7 @@ virtio_user_write_dev_config(struct virtio_dev *vdev, size_t offset, 
{

	struct virtio_user_dev *dev = vdev->ctx;

	struct vhost_user_config cfg = {0};

	int rc;



	if ((dev->protocol_features & (1ULL << VHOST_USER_PROTOCOL_F_CONFIG)) == 0) {

		return -ENOTSUP;
@@ -253,9 +256,10 @@ virtio_user_write_dev_config(struct virtio_dev *vdev, size_t offset, 
	cfg.size = length;

	memcpy(cfg.region, src, length);



	if (dev->ops->send_request(dev, VHOST_USER_SET_CONFIG, &cfg) < 0) {

		SPDK_ERRLOG("set_config failed: %s\n", spdk_strerror(errno));

		return -errno;

	rc = dev->ops->send_request(dev, VHOST_USER_SET_CONFIG, &cfg);

	if (rc < 0) {

		SPDK_ERRLOG("set_config failed: %s\n", spdk_strerror(-rc));

		return rc;

	}



	return 0;
@@ -297,9 +301,11 @@ virtio_user_get_features(struct virtio_dev *vdev) 
{

	struct virtio_user_dev *dev = vdev->ctx;

	uint64_t features;

	int rc;



	if (dev->ops->send_request(dev, VHOST_USER_GET_FEATURES, &features) < 0) {

		SPDK_ERRLOG("get_features failed: %s\n", spdk_strerror(errno));

	rc = dev->ops->send_request(dev, VHOST_USER_GET_FEATURES, &features);

	if (rc < 0) {

		SPDK_ERRLOG("get_features failed: %s\n", spdk_strerror(-rc));

		return 0;

	}
@@ -315,7 +321,7 @@ virtio_user_set_features(struct virtio_dev *vdev, uint64_t features) 


	ret = dev->ops->send_request(dev, VHOST_USER_SET_FEATURES, &features);

	if (ret < 0) {

		return -1;

		return ret;

	}



	vdev->negotiated_features = features;
@@ -328,13 +334,13 @@ virtio_user_set_features(struct virtio_dev *vdev, uint64_t features) 


	ret = dev->ops->send_request(dev, VHOST_USER_GET_PROTOCOL_FEATURES, &protocol_features);

	if (ret < 0) {

		return -1;

		return ret;

	}



	protocol_features &= VIRTIO_USER_SUPPORTED_PROTOCOL_FEATURES;

	ret = dev->ops->send_request(dev, VHOST_USER_SET_PROTOCOL_FEATURES, &protocol_features);

	if (ret < 0) {

		return -1;

		return ret;

	}



	dev->protocol_features = protocol_features;
@@ -358,12 +364,11 @@ virtio_user_setup_queue(struct virtio_dev *vdev, struct virtqueue *vq) 
	uint16_t queue_idx = vq->vq_queue_index;

	void *queue_mem;

	uint64_t desc_addr, avail_addr, used_addr;

	int callfd;

	int kickfd;

	int callfd, kickfd, rc;



	if (dev->callfds[queue_idx] != -1 || dev->kickfds[queue_idx] != -1) {

		SPDK_ERRLOG("queue %"PRIu16" already exists\n", queue_idx);

		return -1;

		return -EEXIST;

	}



	/* May use invalid flag, but some backend uses kickfd and
@@ -373,14 +378,14 @@ virtio_user_setup_queue(struct virtio_dev *vdev, struct virtqueue *vq) 
	callfd = eventfd(0, EFD_CLOEXEC | EFD_NONBLOCK);

	if (callfd < 0) {

		SPDK_ERRLOG("callfd error, %s\n", spdk_strerror(errno));

		return -1;

		return -errno;

	}



	kickfd = eventfd(0, EFD_CLOEXEC | EFD_NONBLOCK);

	if (kickfd < 0) {

		SPDK_ERRLOG("kickfd error, %s\n", spdk_strerror(errno));

		close(callfd);

		return -1;

		return -errno;

	}



	queue_mem = spdk_dma_zmalloc(vq->vq_ring_size, VIRTIO_PCI_VRING_ALIGN, NULL);
@@ -396,12 +401,14 @@ virtio_user_setup_queue(struct virtio_dev *vdev, struct virtqueue *vq) 
	state.index = vq->vq_queue_index;

	state.num = 0;



	if (virtio_dev_has_feature(vdev, VHOST_USER_F_PROTOCOL_FEATURES) &&

	    dev->ops->send_request(dev, VHOST_USER_SET_VRING_ENABLE, &state) < 0) {

	if (virtio_dev_has_feature(vdev, VHOST_USER_F_PROTOCOL_FEATURES)) {

		rc = dev->ops->send_request(dev, VHOST_USER_SET_VRING_ENABLE, &state);

		if (rc < 0) {

			SPDK_ERRLOG("failed to send VHOST_USER_SET_VRING_ENABLE: %s\n",

			    spdk_strerror(errno));

				    spdk_strerror(-rc));

			spdk_dma_free(queue_mem);

		return -1;

			return -rc;

		}

	}



	dev->callfds[queue_idx] = callfd;
@@ -511,19 +518,19 @@ virtio_user_dev_init(struct virtio_dev *vdev, const char *name, const char *path 


	if (name == NULL) {

		SPDK_ERRLOG("No name gived for controller: %s\n", path);

		return -1;

		return -EINVAL;

	}



	dev = calloc(1, sizeof(*dev));

	if (dev == NULL) {

		return -1;

		return -ENOMEM;

	}



	rc = virtio_dev_construct(vdev, name, &virtio_user_ops, dev);

	if (rc != 0) {

		SPDK_ERRLOG("Failed to init device: %s\n", path);

		free(dev);

		return -1;

		return rc;

	}



	vdev->is_hw = 0;
@@ -531,13 +538,15 @@ virtio_user_dev_init(struct virtio_dev *vdev, const char *name, const char *path 
	snprintf(dev->path, PATH_MAX, "%s", path);

	dev->queue_size = queue_size;



	if (virtio_user_dev_setup(vdev) < 0) {

	rc = virtio_user_dev_setup(vdev);

	if (rc < 0) {

		SPDK_ERRLOG("backend set up fails\n");

		goto err;

	}



	if (dev->ops->send_request(dev, VHOST_USER_SET_OWNER, NULL) < 0) {

		SPDK_ERRLOG("set_owner fails: %s\n", spdk_strerror(errno));

	rc = dev->ops->send_request(dev, VHOST_USER_SET_OWNER, NULL);

	if (rc < 0) {

		SPDK_ERRLOG("set_owner fails: %s\n", spdk_strerror(-rc));

		goto err;

	}
@@ -545,5 +554,5 @@ virtio_user_dev_init(struct virtio_dev *vdev, const char *name, const char *path 


err:

	virtio_dev_destruct(vdev);

	return -1;

	return rc;

}

lib/virtio/virtio_user/vhost_user.c

+55 −37
Original line number Diff line number Diff line
@@ -108,7 +108,11 @@ vhost_user_write(int fd, void *buf, int len, int *fds, int fd_num) 
		r = sendmsg(fd, &msgh, 0);

	} while (r < 0 && errno == EINTR);



	return r;

	if (r == -1) {

		return -errno;

	}



	return 0;

}



static int
@@ -122,14 +126,18 @@ vhost_user_read(int fd, struct vhost_user_msg *msg) 
	if ((size_t)ret != sz_hdr) {

		SPDK_WARNLOG("Failed to recv msg hdr: %zd instead of %zu.\n",

			     ret, sz_hdr);

		goto fail;

		if (ret == -1) {

			return -errno;

		} else {

			return -EBUSY;

		}

	}



	/* validate msg flags */

	if (msg->flags != (valid_flags)) {

		SPDK_WARNLOG("Failed to recv msg: flags %"PRIx32" instead of %"PRIx32".\n",

			     msg->flags, valid_flags);

		goto fail;

		return -EIO;

	}



	sz_payload = msg->size;
@@ -137,7 +145,7 @@ vhost_user_read(int fd, struct vhost_user_msg *msg) 
	if (sizeof(*msg) - sz_hdr < sz_payload) {

		SPDK_WARNLOG("Received oversized msg: payload size %zu > available space %zu\n",

			     sz_payload, sizeof(*msg) - sz_hdr);

		goto fail;

		return -EIO;

	}



	if (sz_payload) {
@@ -145,14 +153,15 @@ vhost_user_read(int fd, struct vhost_user_msg *msg) 
		if ((size_t)ret != sz_payload) {

			SPDK_WARNLOG("Failed to recv msg payload: %zd instead of %"PRIu32".\n",

				     ret, msg->size);

			goto fail;

			if (ret == -1) {

				return -errno;

			} else {

				return -EBUSY;

			}

		}

	}



	return 0;



fail:

	return -1;

}



struct hugepage_file_info {
@@ -172,7 +181,7 @@ struct hugepage_file_info { 
static int

get_hugepage_file_info(struct hugepage_file_info huges[], int max)

{

	int idx;

	int idx, rc;

	FILE *f;

	char buf[BUFSIZ], *tmp, *tail;

	char *str_underline, *str_start;
@@ -182,14 +191,17 @@ get_hugepage_file_info(struct hugepage_file_info huges[], int max) 
	f = fopen("/proc/self/maps", "r");

	if (!f) {

		SPDK_ERRLOG("cannot open /proc/self/maps\n");

		return -1;

		rc = -errno;

		assert(rc < 0); /* scan-build hack */

		return rc;

	}



	idx = 0;

	while (fgets(buf, sizeof(buf), f) != NULL) {

		if (sscanf(buf, "%" PRIx64 "-%" PRIx64, &v_start, &v_end) < 2) {

			SPDK_ERRLOG("Failed to parse address\n");

			goto error;

			rc = -EIO;

			goto out;

		}



		tmp = strchr(buf, ' ') + 1; /** skip address */
@@ -224,7 +236,8 @@ get_hugepage_file_info(struct hugepage_file_info huges[], int max) 


		if (idx >= max) {

			SPDK_ERRLOG("Exceed maximum of %d\n", max);

			goto error;

			rc = -ENOSPC;

			goto out;

		}



		if (idx > 0 &&
@@ -240,12 +253,10 @@ get_hugepage_file_info(struct hugepage_file_info huges[], int max) 
		idx++;

	}



	rc = idx;

out:

	fclose(f);

	return idx;



error:

	fclose(f);

	return -1;

	return rc;

}



static int
@@ -257,7 +268,7 @@ prepare_vhost_memory_user(struct vhost_user_msg *msg, int fds[]) 
	num = get_hugepage_file_info(huges, VHOST_MEMORY_MAX_NREGIONS);

	if (num < 0) {

		SPDK_ERRLOG("Failed to prepare memory for vhost-user\n");

		return -1;

		return num;

	}



	for (i = 0; i < num; ++i) {
@@ -303,7 +314,7 @@ vhost_user_sock(struct virtio_user_dev *dev, 
	int need_reply = 0;

	int fds[VHOST_MEMORY_MAX_NREGIONS];

	int fd_num = 0;

	int i, len;

	int i, len, rc;

	int vhostfd = dev->vhostfd;



	SPDK_DEBUGLOG(SPDK_LOG_VIRTIO_USER, "sent message %d = %s\n", req, vhost_msg_strings[req]);
@@ -331,8 +342,9 @@ vhost_user_sock(struct virtio_user_dev *dev, 
		break;



	case VHOST_USER_SET_MEM_TABLE:

		if (prepare_vhost_memory_user(&msg, fds) < 0) {

			return -1;

		rc = prepare_vhost_memory_user(&msg, fds);

		if (rc < 0) {

			return rc;

		}

		fd_num = msg.payload.memory.nregions;

		msg.size = sizeof(msg.payload.memory.nregions);
@@ -387,15 +399,16 @@ vhost_user_sock(struct virtio_user_dev *dev, 
		break;



	default:

		SPDK_ERRLOG("trying to send unhandled msg type\n");

		return -1;

		SPDK_ERRLOG("trying to send unknown msg\n");

		return -EINVAL;

	}



	len = VHOST_USER_HDR_SIZE + msg.size;

	if (vhost_user_write(vhostfd, &msg, len, fds, fd_num) < 0) {

	rc = vhost_user_write(vhostfd, &msg, len, fds, fd_num);

	if (rc < 0) {

		SPDK_ERRLOG("%s failed: %s\n",

			    vhost_msg_strings[req], spdk_strerror(errno));

		return -1;

			    vhost_msg_strings[req], spdk_strerror(-rc));

		return rc;

	}



	if (req == VHOST_USER_SET_MEM_TABLE)
@@ -404,14 +417,15 @@ vhost_user_sock(struct virtio_user_dev *dev, 
		}



	if (need_reply) {

		if (vhost_user_read(vhostfd, &msg) < 0) {

			SPDK_WARNLOG("Received msg failed: %s\n", spdk_strerror(errno));

			return -1;

		rc = vhost_user_read(vhostfd, &msg);

		if (rc < 0) {

			SPDK_WARNLOG("Received msg failed: %s\n", spdk_strerror(-rc));

			return rc;

		}



		if (req != msg.request) {

			SPDK_WARNLOG("Received unexpected msg type\n");

			return -1;

			return -EIO;

		}



		switch (req) {
@@ -420,14 +434,14 @@ vhost_user_sock(struct virtio_user_dev *dev, 
		case VHOST_USER_GET_QUEUE_NUM:

			if (msg.size != sizeof(msg.payload.u64)) {

				SPDK_WARNLOG("Received bad msg size\n");

				return -1;

				return -EIO;

			}

			*((__u64 *)arg) = msg.payload.u64;

			break;

		case VHOST_USER_GET_VRING_BASE:

			if (msg.size != sizeof(msg.payload.state)) {

				SPDK_WARNLOG("Received bad msg size\n");

				return -1;

				return -EIO;

			}

			memcpy(arg, &msg.payload.state,

			       sizeof(struct vhost_vring_state));
@@ -435,13 +449,13 @@ vhost_user_sock(struct virtio_user_dev *dev, 
		case VHOST_USER_GET_CONFIG:

			if (msg.size != sizeof(msg.payload.cfg)) {

				SPDK_WARNLOG("Received bad msg size\n");

				return -1;

				return -EIO;

			}

			memcpy(arg, &msg.payload.cfg, sizeof(msg.payload.cfg));

			break;

		default:

			SPDK_WARNLOG("Received unexpected msg type\n");

			return -1;

			return -EBADMSG;

		}

	}
@@ -466,7 +480,7 @@ vhost_user_setup(struct virtio_user_dev *dev) 
	fd = socket(AF_UNIX, SOCK_STREAM, 0);

	if (fd < 0) {

		SPDK_ERRLOG("socket() error, %s\n", spdk_strerror(errno));

		return -1;

		return -errno;

	}



	flag = fcntl(fd, F_GETFD);
@@ -480,12 +494,16 @@ vhost_user_setup(struct virtio_user_dev *dev) 
	if (rc < 0 || (size_t)rc >= sizeof(un.sun_path)) {

		SPDK_ERRLOG("socket path too long\n");

		close(fd);

		return -1;

		if (rc < 0) {

			return -errno;

		} else {

			return -EINVAL;

		}

	}

	if (connect(fd, (struct sockaddr *)&un, sizeof(un)) < 0) {

		SPDK_ERRLOG("connect error, %s\n", spdk_strerror(errno));

		close(fd);

		return -1;

		return -errno;

	}



	dev->vhostfd = fd;