Re: [PATCH V3 WIP 2/3] vhost-scsi: new device supporting the tcm_vhost Linux kernel module
From: Stefan Hajnoczi <hidden>
Date: 2013-03-19 08:45:22
Also in:
kvm, qemu-devel
On Tue, Mar 19, 2013 at 08:34:44AM +0800, Asias He wrote:
quoted hunk
+static void vhost_scsi_stop(VHostSCSI *vs, VirtIODevice *vdev) +{ + int ret = 0; + + if (!vdev->binding->set_guest_notifiers) { + ret = vdev->binding->set_guest_notifiers(vdev->binding_opaque, + vs->dev.nvqs, false); + if (ret < 0) { + error_report("vhost guest notifier cleanup failed: %d\n", ret);
Indentation. scripts/checkpatch.pl should catch this.
quoted hunk
+ } + } + assert(ret >= 0); + + vhost_scsi_clear_endpoint(vdev); + vhost_dev_stop(&vs->dev, vdev); + vhost_dev_disable_notifiers(&vs->dev, vdev); +} + +static void vhost_scsi_set_config(VirtIODevice *vdev, + const uint8_t *config) +{ + VirtIOSCSIConfig *scsiconf = (VirtIOSCSIConfig *)config; + VHostSCSI *vs = (VHostSCSI *)vdev; + + if ((uint32_t) ldl_raw(&scsiconf->sense_size) != vs->vs.sense_size || + (uint32_t) ldl_raw(&scsiconf->cdb_size) != vs->vs.cdb_size) { + error_report("vhost-scsi does not support changing the sense data and CDB sizes"); + exit(1);
Guest-triggerable exits can be used as a denial of service - especially under nested virtualization where killing the L1 hypervisor would kill all L2 guests! I would just log a warning here.
quoted hunk
+ } +} + +static void vhost_scsi_set_status(VirtIODevice *vdev, uint8_t val) +{ + VHostSCSI *vs = (VHostSCSI *)vdev; + bool start = (val & VIRTIO_CONFIG_S_DRIVER_OK); + + if (vs->dev.started == start) { + return; + } + + if (start) { + int ret; + + ret = vhost_scsi_start(vs, vdev); + if (ret < 0) { + error_report("virtio-scsi: unable to start vhost: %s\n", + strerror(-ret)); + + /* There is no userspace virtio-scsi fallback so exit */ + exit(1);
It's questionable whether to kill the guest or simply disable this virtio-scsi-pci adapter. Fine for now but we may want to allow a policy here in the future.
quoted hunk
diff --git a/hw/virtio-pci.c b/hw/virtio-pci.c index 39c1966..281a7e2 100644 --- a/hw/virtio-pci.c +++ b/hw/virtio-pci.c@@ -22,6 +22,7 @@ #include "hw/virtio-net.h" #include "hw/virtio-serial.h" #include "hw/virtio-scsi.h" +#include "hw/vhost-scsi.h"
Can this header be included unconditionally? It uses _IOW() which may not be available on all host platforms.