[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH V3 WIP 2/3] vhost-scsi: new device supporting th
From: |
Stefan Hajnoczi |
Subject: |
Re: [Qemu-devel] [PATCH V3 WIP 2/3] vhost-scsi: new device supporting the tcm_vhost Linux kernel module |
Date: |
Tue, 19 Mar 2013 09:40:16 +0100 |
User-agent: |
Mutt/1.5.21 (2010-09-15) |
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.
> 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.