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 <karolina.stolarek@oracle.com>
---
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,
-
- .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;
+
+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;
+
+ 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;
--
2.47.1
Karolina Stolarek <karolina.stolarek@oracle.com> 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 <karolina.stolarek@oracle.com>
> ---
> 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
On 08/12/2025 14:54, Alex Bennée wrote:
> Karolina Stolarek <karolina.stolarek@oracle.com> writes:
>
>> -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.
Ah, that is true, it escaped me.
>
>> +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.
I understand, that sounds like a better solution. It might take some
time to finalize this, but I'd like to try it for v2.
Many thanks for your review, really appreciate it.
All the best,
Karolina
>
>> +
>> + 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;
>
© 2016 - 2026 Red Hat, Inc.