Karolina Stolarek <[email protected]> writes:
> In virtiscsi_probe(), virtio-scsi enumerates all possible
> targets, up to VIRTIO_SCSI_MAX_TARGET, to see which of them
> are available. This means that during that scan, the initiator
> queues up INQUIRY commands to the targets that do not exist.
> Such inquires fail, returning a BAD_TARGET response.
>
> Currently, there is no way to limit the number of possible
> targets or to finish the scan earlier. Add "max_target"
> option for virtio-scsi and vhost-scsi devices to provide
> a hint on the number of targets available for scanning.
>
> Signed-off-by: Karolina Stolarek <[email protected]>
> ---
> hw/scsi/vhost-scsi.c | 2 ++
> hw/scsi/virtio-scsi.c | 42 +++++++++++++++++++--------------
> include/hw/virtio/virtio-scsi.h | 1 +
> 3 files changed, 27 insertions(+), 18 deletions(-)
>
> diff --git a/hw/scsi/vhost-scsi.c b/hw/scsi/vhost-scsi.c
> index cdf405b0f8..1a4860a72f 100644
> --- a/hw/scsi/vhost-scsi.c
> +++ b/hw/scsi/vhost-scsi.c
> @@ -353,6 +353,8 @@ static const Property vhost_scsi_properties[] = {
> 128),
> DEFINE_PROP_BOOL("seg_max_adjust", VirtIOSCSICommon, conf.seg_max_adjust,
> true),
> + DEFINE_PROP_UINT16("max_target", VirtIOSCSICommon, conf.max_target,
> + VIRTIO_SCSI_MAX_TARGET),
> DEFINE_PROP_UINT32("max_sectors", VirtIOSCSICommon, conf.max_sectors,
> 0xFFFF),
> DEFINE_PROP_UINT32("cmd_per_lun", VirtIOSCSICommon, conf.cmd_per_lun,
> 128),
> diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c
> index 93e87c459c..091f68090e 100644
> --- a/hw/scsi/virtio-scsi.c
> +++ b/hw/scsi/virtio-scsi.c
> @@ -971,7 +971,7 @@ static void virtio_scsi_get_config(VirtIODevice *vdev,
> virtio_stl_p(vdev, &scsiconf->sense_size, s->sense_size);
> virtio_stl_p(vdev, &scsiconf->cdb_size, s->cdb_size);
> virtio_stw_p(vdev, &scsiconf->max_channel, VIRTIO_SCSI_MAX_CHANNEL);
> - virtio_stw_p(vdev, &scsiconf->max_target, VIRTIO_SCSI_MAX_TARGET);
> + virtio_stw_p(vdev, &scsiconf->max_target, s->conf.max_target);
> virtio_stl_p(vdev, &scsiconf->max_lun, VIRTIO_SCSI_MAX_LUN);
> }
>
> @@ -1260,23 +1260,26 @@ static void virtio_scsi_drained_end(SCSIBus *bus)
> }
> }
>
> -static struct SCSIBusInfo virtio_scsi_scsi_info = {
> - .tcq = true,
> - .max_channel = VIRTIO_SCSI_MAX_CHANNEL,
> - .max_target = VIRTIO_SCSI_MAX_TARGET,
> - .max_lun = VIRTIO_SCSI_MAX_LUN,
See bellow.
> -
> - .complete = virtio_scsi_command_complete,
> - .fail = virtio_scsi_command_failed,
> - .cancel = virtio_scsi_request_cancelled,
> - .change = virtio_scsi_change,
> - .parse_cdb = virtio_scsi_parse_cdb,
> - .get_sg_list = virtio_scsi_get_sg_list,
> - .save_request = virtio_scsi_save_request,
> - .load_request = virtio_scsi_load_request,
> - .drained_begin = virtio_scsi_drained_begin,
> - .drained_end = virtio_scsi_drained_end,
> -};
> +static struct SCSIBusInfo virtio_scsi_scsi_info;
> +
We can't have a static structure here otherwise all the VirtIO scsi
devices will share the same configuration.
> +static void virtio_scsi_init_scsi_info(struct VirtIOSCSIConf *conf)
> +{
> + virtio_scsi_scsi_info.tcq = true;
> + virtio_scsi_scsi_info.max_channel = VIRTIO_SCSI_MAX_CHANNEL;
> + virtio_scsi_scsi_info.max_lun = VIRTIO_SCSI_MAX_LUN;
> + virtio_scsi_scsi_info.max_target = conf->max_target;
I think we need to allocate a dynamic configuration block for this
stuff. Although given we have ops and config mixed in this structure I
suspect the cleaner option is to re-factor SCSIBusInfo into
SCSIBusConfig and SCSIBusOps so you can keep the static around. But this
would touch quite a bit of code.
> +
> + virtio_scsi_scsi_info.complete = virtio_scsi_command_complete;
> + virtio_scsi_scsi_info.fail = virtio_scsi_command_failed;
> + virtio_scsi_scsi_info.cancel = virtio_scsi_request_cancelled;
> + virtio_scsi_scsi_info.change = virtio_scsi_change;
> + virtio_scsi_scsi_info.parse_cdb = virtio_scsi_parse_cdb;
> + virtio_scsi_scsi_info.get_sg_list = virtio_scsi_get_sg_list;
> + virtio_scsi_scsi_info.save_request = virtio_scsi_save_request;
> + virtio_scsi_scsi_info.load_request = virtio_scsi_load_request;
> + virtio_scsi_scsi_info.drained_begin = virtio_scsi_drained_begin;
> + virtio_scsi_scsi_info.drained_end = virtio_scsi_drained_end;
> +}
>
> void virtio_scsi_common_realize(DeviceState *dev,
> VirtIOHandleOutput ctrl,
> @@ -1289,6 +1292,7 @@ void virtio_scsi_common_realize(DeviceState *dev,
> int i;
>
> virtio_init(vdev, VIRTIO_ID_SCSI, sizeof(VirtIOSCSIConfig));
> + virtio_scsi_init_scsi_info(&s->conf);
>
> if (s->conf.num_queues == VIRTIO_SCSI_AUTO_NUM_QUEUES) {
> s->conf.num_queues = 1;
> @@ -1379,6 +1383,8 @@ static const Property virtio_scsi_properties[] = {
> parent_obj.conf.virtqueue_size,
> 256),
> DEFINE_PROP_BOOL("seg_max_adjust", VirtIOSCSI,
> parent_obj.conf.seg_max_adjust, true),
> + DEFINE_PROP_UINT16("max_target", VirtIOSCSICommon, conf.max_target,
> + VIRTIO_SCSI_MAX_TARGET),
> DEFINE_PROP_UINT32("max_sectors", VirtIOSCSI,
> parent_obj.conf.max_sectors,
> 0xFFFF),
> DEFINE_PROP_UINT32("cmd_per_lun", VirtIOSCSI,
> parent_obj.conf.cmd_per_lun,
> diff --git a/include/hw/virtio/virtio-scsi.h b/include/hw/virtio/virtio-scsi.h
> index b6028bb5cd..3998b241f6 100644
> --- a/include/hw/virtio/virtio-scsi.h
> +++ b/include/hw/virtio/virtio-scsi.h
> @@ -54,6 +54,7 @@ struct VirtIOSCSIConf {
> uint32_t virtqueue_size;
> bool worker_per_virtqueue;
> bool seg_max_adjust;
> + uint16_t max_target;
> uint32_t max_sectors;
> uint32_t cmd_per_lun;
> char *vhostfd;
--
Alex Bennée
Virtualisation Tech Lead @ Linaro
