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:
+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.
+ }
+ }
+ 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.
+ }
+}
+
+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.