On 08/12/2025 14:54, Alex Bennée wrote:
Karolina Stolarek <[email protected]> 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;
