Signed-off-by: Denis Plotnikov <dplotnikov@virtuozzo.com>
---
hw/block/virtio-blk.c | 6 ++++--
hw/scsi/virtio-scsi.c | 5 +++--
include/hw/virtio/virtio.h | 1 +
3 files changed, 8 insertions(+), 4 deletions(-)
diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
index 09f46ed85f..72f935033f 100644
--- a/hw/block/virtio-blk.c
+++ b/hw/block/virtio-blk.c
@@ -914,7 +914,8 @@ static void virtio_blk_update_config(VirtIODevice *vdev, uint8_t *config)
memset(&blkcfg, 0, sizeof(blkcfg));
virtio_stq_p(vdev, &blkcfg.capacity, capacity);
virtio_stl_p(vdev, &blkcfg.seg_max,
- s->conf.seg_max_adjust ? s->conf.queue_size - 2 : 128 - 2);
+ s->conf.seg_max_adjust ? s->conf.queue_size - 2 :
+ VIRTQUEUE_DEFAULT_SIZE - 2);
virtio_stw_p(vdev, &blkcfg.geometry.cylinders, conf->cyls);
virtio_stl_p(vdev, &blkcfg.blk_size, blk_size);
virtio_stw_p(vdev, &blkcfg.min_io_size, conf->min_io_size / blk_size);
@@ -1272,7 +1273,8 @@ static Property virtio_blk_properties[] = {
DEFINE_PROP_BIT("request-merging", VirtIOBlock, conf.request_merging, 0,
true),
DEFINE_PROP_UINT16("num-queues", VirtIOBlock, conf.num_queues, 1),
- DEFINE_PROP_UINT16("queue-size", VirtIOBlock, conf.queue_size, 128),
+ DEFINE_PROP_UINT16("queue-size", VirtIOBlock, conf.queue_size,
+ VIRTQUEUE_DEFAULT_SIZE),
DEFINE_PROP_BOOL("seg-max-adjust", VirtIOBlock, conf.seg_max_adjust, true),
DEFINE_PROP_LINK("iothread", VirtIOBlock, conf.iothread, TYPE_IOTHREAD,
IOThread *),
diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c
index 3b61563609..36f66046ae 100644
--- a/hw/scsi/virtio-scsi.c
+++ b/hw/scsi/virtio-scsi.c
@@ -660,7 +660,8 @@ static void virtio_scsi_get_config(VirtIODevice *vdev,
virtio_stl_p(vdev, &scsiconf->num_queues, s->conf.num_queues);
virtio_stl_p(vdev, &scsiconf->seg_max,
- s->conf.seg_max_adjust ? s->conf.virtqueue_size - 2 : 128 - 2);
+ s->conf.seg_max_adjust ? s->conf.virtqueue_size - 2 :
+ VIRTQUEUE_DEFAULT_SIZE - 2);
virtio_stl_p(vdev, &scsiconf->max_sectors, s->conf.max_sectors);
virtio_stl_p(vdev, &scsiconf->cmd_per_lun, s->conf.cmd_per_lun);
virtio_stl_p(vdev, &scsiconf->event_info_size, sizeof(VirtIOSCSIEvent));
@@ -965,7 +966,7 @@ static void virtio_scsi_device_unrealize(DeviceState *dev, Error **errp)
static Property virtio_scsi_properties[] = {
DEFINE_PROP_UINT32("num_queues", VirtIOSCSI, parent_obj.conf.num_queues, 1),
DEFINE_PROP_UINT32("virtqueue_size", VirtIOSCSI,
- parent_obj.conf.virtqueue_size, 128),
+ parent_obj.conf.virtqueue_size, VIRTQUEUE_DEFAULT_SIZE),
DEFINE_PROP_BOOL("seg_max_adjust", VirtIOSCSI,
parent_obj.conf.seg_max_adjust, true),
DEFINE_PROP_UINT32("max_sectors", VirtIOSCSI, parent_obj.conf.max_sectors,
diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
index b69d517496..a66ea2368b 100644
--- a/include/hw/virtio/virtio.h
+++ b/include/hw/virtio/virtio.h
@@ -48,6 +48,7 @@ size_t virtio_feature_get_config_size(VirtIOFeature *features,
typedef struct VirtQueue VirtQueue;
#define VIRTQUEUE_MAX_SIZE 1024
+#define VIRTQUEUE_DEFAULT_SIZE 128
typedef struct VirtQueueElement
{
--
2.17.0
On 1/29/20 3:06 PM, Denis Plotnikov wrote:
> Signed-off-by: Denis Plotnikov <dplotnikov@virtuozzo.com>
typo VIRTQUEUE_DEFUALT_SIZE -> VIRTQUEUE_DEFAULT_SIZE in subject
With subject fixed:
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
> hw/block/virtio-blk.c | 6 ++++--
> hw/scsi/virtio-scsi.c | 5 +++--
> include/hw/virtio/virtio.h | 1 +
> 3 files changed, 8 insertions(+), 4 deletions(-)
>
> diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
> index 09f46ed85f..72f935033f 100644
> --- a/hw/block/virtio-blk.c
> +++ b/hw/block/virtio-blk.c
> @@ -914,7 +914,8 @@ static void virtio_blk_update_config(VirtIODevice *vdev, uint8_t *config)
> memset(&blkcfg, 0, sizeof(blkcfg));
> virtio_stq_p(vdev, &blkcfg.capacity, capacity);
> virtio_stl_p(vdev, &blkcfg.seg_max,
> - s->conf.seg_max_adjust ? s->conf.queue_size - 2 : 128 - 2);
> + s->conf.seg_max_adjust ? s->conf.queue_size - 2 :
> + VIRTQUEUE_DEFAULT_SIZE - 2);
> virtio_stw_p(vdev, &blkcfg.geometry.cylinders, conf->cyls);
> virtio_stl_p(vdev, &blkcfg.blk_size, blk_size);
> virtio_stw_p(vdev, &blkcfg.min_io_size, conf->min_io_size / blk_size);
> @@ -1272,7 +1273,8 @@ static Property virtio_blk_properties[] = {
> DEFINE_PROP_BIT("request-merging", VirtIOBlock, conf.request_merging, 0,
> true),
> DEFINE_PROP_UINT16("num-queues", VirtIOBlock, conf.num_queues, 1),
> - DEFINE_PROP_UINT16("queue-size", VirtIOBlock, conf.queue_size, 128),
> + DEFINE_PROP_UINT16("queue-size", VirtIOBlock, conf.queue_size,
> + VIRTQUEUE_DEFAULT_SIZE),
> DEFINE_PROP_BOOL("seg-max-adjust", VirtIOBlock, conf.seg_max_adjust, true),
> DEFINE_PROP_LINK("iothread", VirtIOBlock, conf.iothread, TYPE_IOTHREAD,
> IOThread *),
> diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c
> index 3b61563609..36f66046ae 100644
> --- a/hw/scsi/virtio-scsi.c
> +++ b/hw/scsi/virtio-scsi.c
> @@ -660,7 +660,8 @@ static void virtio_scsi_get_config(VirtIODevice *vdev,
>
> virtio_stl_p(vdev, &scsiconf->num_queues, s->conf.num_queues);
> virtio_stl_p(vdev, &scsiconf->seg_max,
> - s->conf.seg_max_adjust ? s->conf.virtqueue_size - 2 : 128 - 2);
> + s->conf.seg_max_adjust ? s->conf.virtqueue_size - 2 :
> + VIRTQUEUE_DEFAULT_SIZE - 2);
> virtio_stl_p(vdev, &scsiconf->max_sectors, s->conf.max_sectors);
> virtio_stl_p(vdev, &scsiconf->cmd_per_lun, s->conf.cmd_per_lun);
> virtio_stl_p(vdev, &scsiconf->event_info_size, sizeof(VirtIOSCSIEvent));
> @@ -965,7 +966,7 @@ static void virtio_scsi_device_unrealize(DeviceState *dev, Error **errp)
> static Property virtio_scsi_properties[] = {
> DEFINE_PROP_UINT32("num_queues", VirtIOSCSI, parent_obj.conf.num_queues, 1),
> DEFINE_PROP_UINT32("virtqueue_size", VirtIOSCSI,
> - parent_obj.conf.virtqueue_size, 128),
> + parent_obj.conf.virtqueue_size, VIRTQUEUE_DEFAULT_SIZE),
> DEFINE_PROP_BOOL("seg_max_adjust", VirtIOSCSI,
> parent_obj.conf.seg_max_adjust, true),
> DEFINE_PROP_UINT32("max_sectors", VirtIOSCSI, parent_obj.conf.max_sectors,
> diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
> index b69d517496..a66ea2368b 100644
> --- a/include/hw/virtio/virtio.h
> +++ b/include/hw/virtio/virtio.h
> @@ -48,6 +48,7 @@ size_t virtio_feature_get_config_size(VirtIOFeature *features,
> typedef struct VirtQueue VirtQueue;
>
> #define VIRTQUEUE_MAX_SIZE 1024
> +#define VIRTQUEUE_DEFAULT_SIZE 128
>
> typedef struct VirtQueueElement
> {
>
On Wed, 29 Jan 2020 17:06:59 +0300
Denis Plotnikov <dplotnikov@virtuozzo.com> wrote:
> Signed-off-by: Denis Plotnikov <dplotnikov@virtuozzo.com>
> ---
> hw/block/virtio-blk.c | 6 ++++--
> hw/scsi/virtio-scsi.c | 5 +++--
> include/hw/virtio/virtio.h | 1 +
> 3 files changed, 8 insertions(+), 4 deletions(-)
>
> diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
> index 09f46ed85f..72f935033f 100644
> --- a/hw/block/virtio-blk.c
> +++ b/hw/block/virtio-blk.c
> @@ -914,7 +914,8 @@ static void virtio_blk_update_config(VirtIODevice *vdev, uint8_t *config)
> memset(&blkcfg, 0, sizeof(blkcfg));
> virtio_stq_p(vdev, &blkcfg.capacity, capacity);
> virtio_stl_p(vdev, &blkcfg.seg_max,
> - s->conf.seg_max_adjust ? s->conf.queue_size - 2 : 128 - 2);
> + s->conf.seg_max_adjust ? s->conf.queue_size - 2 :
> + VIRTQUEUE_DEFAULT_SIZE - 2);
> virtio_stw_p(vdev, &blkcfg.geometry.cylinders, conf->cyls);
> virtio_stl_p(vdev, &blkcfg.blk_size, blk_size);
> virtio_stw_p(vdev, &blkcfg.min_io_size, conf->min_io_size / blk_size);
> @@ -1272,7 +1273,8 @@ static Property virtio_blk_properties[] = {
> DEFINE_PROP_BIT("request-merging", VirtIOBlock, conf.request_merging, 0,
> true),
> DEFINE_PROP_UINT16("num-queues", VirtIOBlock, conf.num_queues, 1),
> - DEFINE_PROP_UINT16("queue-size", VirtIOBlock, conf.queue_size, 128),
> + DEFINE_PROP_UINT16("queue-size", VirtIOBlock, conf.queue_size,
> + VIRTQUEUE_DEFAULT_SIZE),
> DEFINE_PROP_BOOL("seg-max-adjust", VirtIOBlock, conf.seg_max_adjust, true),
> DEFINE_PROP_LINK("iothread", VirtIOBlock, conf.iothread, TYPE_IOTHREAD,
> IOThread *),
> diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c
> index 3b61563609..36f66046ae 100644
> --- a/hw/scsi/virtio-scsi.c
> +++ b/hw/scsi/virtio-scsi.c
> @@ -660,7 +660,8 @@ static void virtio_scsi_get_config(VirtIODevice *vdev,
>
> virtio_stl_p(vdev, &scsiconf->num_queues, s->conf.num_queues);
> virtio_stl_p(vdev, &scsiconf->seg_max,
> - s->conf.seg_max_adjust ? s->conf.virtqueue_size - 2 : 128 - 2);
> + s->conf.seg_max_adjust ? s->conf.virtqueue_size - 2 :
> + VIRTQUEUE_DEFAULT_SIZE - 2);
> virtio_stl_p(vdev, &scsiconf->max_sectors, s->conf.max_sectors);
> virtio_stl_p(vdev, &scsiconf->cmd_per_lun, s->conf.cmd_per_lun);
> virtio_stl_p(vdev, &scsiconf->event_info_size, sizeof(VirtIOSCSIEvent));
> @@ -965,7 +966,7 @@ static void virtio_scsi_device_unrealize(DeviceState *dev, Error **errp)
> static Property virtio_scsi_properties[] = {
> DEFINE_PROP_UINT32("num_queues", VirtIOSCSI, parent_obj.conf.num_queues, 1),
> DEFINE_PROP_UINT32("virtqueue_size", VirtIOSCSI,
> - parent_obj.conf.virtqueue_size, 128),
> + parent_obj.conf.virtqueue_size, VIRTQUEUE_DEFAULT_SIZE),
> DEFINE_PROP_BOOL("seg_max_adjust", VirtIOSCSI,
> parent_obj.conf.seg_max_adjust, true),
> DEFINE_PROP_UINT32("max_sectors", VirtIOSCSI, parent_obj.conf.max_sectors,
> diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
> index b69d517496..a66ea2368b 100644
> --- a/include/hw/virtio/virtio.h
> +++ b/include/hw/virtio/virtio.h
> @@ -48,6 +48,7 @@ size_t virtio_feature_get_config_size(VirtIOFeature *features,
> typedef struct VirtQueue VirtQueue;
>
> #define VIRTQUEUE_MAX_SIZE 1024
> +#define VIRTQUEUE_DEFAULT_SIZE 128
Going from the header only, this looks like a value that is supposed to
be used for every virtqueue... but from the users, this is only for blk
and scsi.
I don't think adding a default for everything makes sense, even if the
same value makes sense for blk and scsi.
>
> typedef struct VirtQueueElement
> {
On Wed, Jan 29, 2020 at 06:55:18PM +0100, Cornelia Huck wrote:
> On Wed, 29 Jan 2020 17:06:59 +0300
> Denis Plotnikov <dplotnikov@virtuozzo.com> wrote:
>
> > Signed-off-by: Denis Plotnikov <dplotnikov@virtuozzo.com>
> > ---
> > hw/block/virtio-blk.c | 6 ++++--
> > hw/scsi/virtio-scsi.c | 5 +++--
> > include/hw/virtio/virtio.h | 1 +
> > 3 files changed, 8 insertions(+), 4 deletions(-)
> >
> > diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
> > index 09f46ed85f..72f935033f 100644
> > --- a/hw/block/virtio-blk.c
> > +++ b/hw/block/virtio-blk.c
> > @@ -914,7 +914,8 @@ static void virtio_blk_update_config(VirtIODevice *vdev, uint8_t *config)
> > memset(&blkcfg, 0, sizeof(blkcfg));
> > virtio_stq_p(vdev, &blkcfg.capacity, capacity);
> > virtio_stl_p(vdev, &blkcfg.seg_max,
> > - s->conf.seg_max_adjust ? s->conf.queue_size - 2 : 128 - 2);
> > + s->conf.seg_max_adjust ? s->conf.queue_size - 2 :
> > + VIRTQUEUE_DEFAULT_SIZE - 2);
> > virtio_stw_p(vdev, &blkcfg.geometry.cylinders, conf->cyls);
> > virtio_stl_p(vdev, &blkcfg.blk_size, blk_size);
> > virtio_stw_p(vdev, &blkcfg.min_io_size, conf->min_io_size / blk_size);
> > @@ -1272,7 +1273,8 @@ static Property virtio_blk_properties[] = {
> > DEFINE_PROP_BIT("request-merging", VirtIOBlock, conf.request_merging, 0,
> > true),
> > DEFINE_PROP_UINT16("num-queues", VirtIOBlock, conf.num_queues, 1),
> > - DEFINE_PROP_UINT16("queue-size", VirtIOBlock, conf.queue_size, 128),
> > + DEFINE_PROP_UINT16("queue-size", VirtIOBlock, conf.queue_size,
> > + VIRTQUEUE_DEFAULT_SIZE),
> > DEFINE_PROP_BOOL("seg-max-adjust", VirtIOBlock, conf.seg_max_adjust, true),
> > DEFINE_PROP_LINK("iothread", VirtIOBlock, conf.iothread, TYPE_IOTHREAD,
> > IOThread *),
> > diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c
> > index 3b61563609..36f66046ae 100644
> > --- a/hw/scsi/virtio-scsi.c
> > +++ b/hw/scsi/virtio-scsi.c
> > @@ -660,7 +660,8 @@ static void virtio_scsi_get_config(VirtIODevice *vdev,
> >
> > virtio_stl_p(vdev, &scsiconf->num_queues, s->conf.num_queues);
> > virtio_stl_p(vdev, &scsiconf->seg_max,
> > - s->conf.seg_max_adjust ? s->conf.virtqueue_size - 2 : 128 - 2);
> > + s->conf.seg_max_adjust ? s->conf.virtqueue_size - 2 :
> > + VIRTQUEUE_DEFAULT_SIZE - 2);
> > virtio_stl_p(vdev, &scsiconf->max_sectors, s->conf.max_sectors);
> > virtio_stl_p(vdev, &scsiconf->cmd_per_lun, s->conf.cmd_per_lun);
> > virtio_stl_p(vdev, &scsiconf->event_info_size, sizeof(VirtIOSCSIEvent));
> > @@ -965,7 +966,7 @@ static void virtio_scsi_device_unrealize(DeviceState *dev, Error **errp)
> > static Property virtio_scsi_properties[] = {
> > DEFINE_PROP_UINT32("num_queues", VirtIOSCSI, parent_obj.conf.num_queues, 1),
> > DEFINE_PROP_UINT32("virtqueue_size", VirtIOSCSI,
> > - parent_obj.conf.virtqueue_size, 128),
> > + parent_obj.conf.virtqueue_size, VIRTQUEUE_DEFAULT_SIZE),
> > DEFINE_PROP_BOOL("seg_max_adjust", VirtIOSCSI,
> > parent_obj.conf.seg_max_adjust, true),
> > DEFINE_PROP_UINT32("max_sectors", VirtIOSCSI, parent_obj.conf.max_sectors,
> > diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
> > index b69d517496..a66ea2368b 100644
> > --- a/include/hw/virtio/virtio.h
> > +++ b/include/hw/virtio/virtio.h
> > @@ -48,6 +48,7 @@ size_t virtio_feature_get_config_size(VirtIOFeature *features,
> > typedef struct VirtQueue VirtQueue;
> >
> > #define VIRTQUEUE_MAX_SIZE 1024
> > +#define VIRTQUEUE_DEFAULT_SIZE 128
>
> Going from the header only, this looks like a value that is supposed to
> be used for every virtqueue... but from the users, this is only for blk
> and scsi.
>
> I don't think adding a default for everything makes sense, even if the
> same value makes sense for blk and scsi.
Agreed, this value is too general. VIRTIO_BLK_VQ_DEFAULT_SIZE and
VIRTIO_SCSI_VQ_DEFAULT_SIZE would make sense to me.
Stefan
On 30.01.2020 17:56, Stefan Hajnoczi wrote:
> On Wed, Jan 29, 2020 at 06:55:18PM +0100, Cornelia Huck wrote:
>> On Wed, 29 Jan 2020 17:06:59 +0300
>> Denis Plotnikov <dplotnikov@virtuozzo.com> wrote:
>>
>>> Signed-off-by: Denis Plotnikov <dplotnikov@virtuozzo.com>
>>> ---
>>> hw/block/virtio-blk.c | 6 ++++--
>>> hw/scsi/virtio-scsi.c | 5 +++--
>>> include/hw/virtio/virtio.h | 1 +
>>> 3 files changed, 8 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
>>> index 09f46ed85f..72f935033f 100644
>>> --- a/hw/block/virtio-blk.c
>>> +++ b/hw/block/virtio-blk.c
>>> @@ -914,7 +914,8 @@ static void virtio_blk_update_config(VirtIODevice *vdev, uint8_t *config)
>>> memset(&blkcfg, 0, sizeof(blkcfg));
>>> virtio_stq_p(vdev, &blkcfg.capacity, capacity);
>>> virtio_stl_p(vdev, &blkcfg.seg_max,
>>> - s->conf.seg_max_adjust ? s->conf.queue_size - 2 : 128 - 2);
>>> + s->conf.seg_max_adjust ? s->conf.queue_size - 2 :
>>> + VIRTQUEUE_DEFAULT_SIZE - 2);
>>> virtio_stw_p(vdev, &blkcfg.geometry.cylinders, conf->cyls);
>>> virtio_stl_p(vdev, &blkcfg.blk_size, blk_size);
>>> virtio_stw_p(vdev, &blkcfg.min_io_size, conf->min_io_size / blk_size);
>>> @@ -1272,7 +1273,8 @@ static Property virtio_blk_properties[] = {
>>> DEFINE_PROP_BIT("request-merging", VirtIOBlock, conf.request_merging, 0,
>>> true),
>>> DEFINE_PROP_UINT16("num-queues", VirtIOBlock, conf.num_queues, 1),
>>> - DEFINE_PROP_UINT16("queue-size", VirtIOBlock, conf.queue_size, 128),
>>> + DEFINE_PROP_UINT16("queue-size", VirtIOBlock, conf.queue_size,
>>> + VIRTQUEUE_DEFAULT_SIZE),
>>> DEFINE_PROP_BOOL("seg-max-adjust", VirtIOBlock, conf.seg_max_adjust, true),
>>> DEFINE_PROP_LINK("iothread", VirtIOBlock, conf.iothread, TYPE_IOTHREAD,
>>> IOThread *),
>>> diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c
>>> index 3b61563609..36f66046ae 100644
>>> --- a/hw/scsi/virtio-scsi.c
>>> +++ b/hw/scsi/virtio-scsi.c
>>> @@ -660,7 +660,8 @@ static void virtio_scsi_get_config(VirtIODevice *vdev,
>>>
>>> virtio_stl_p(vdev, &scsiconf->num_queues, s->conf.num_queues);
>>> virtio_stl_p(vdev, &scsiconf->seg_max,
>>> - s->conf.seg_max_adjust ? s->conf.virtqueue_size - 2 : 128 - 2);
>>> + s->conf.seg_max_adjust ? s->conf.virtqueue_size - 2 :
>>> + VIRTQUEUE_DEFAULT_SIZE - 2);
>>> virtio_stl_p(vdev, &scsiconf->max_sectors, s->conf.max_sectors);
>>> virtio_stl_p(vdev, &scsiconf->cmd_per_lun, s->conf.cmd_per_lun);
>>> virtio_stl_p(vdev, &scsiconf->event_info_size, sizeof(VirtIOSCSIEvent));
>>> @@ -965,7 +966,7 @@ static void virtio_scsi_device_unrealize(DeviceState *dev, Error **errp)
>>> static Property virtio_scsi_properties[] = {
>>> DEFINE_PROP_UINT32("num_queues", VirtIOSCSI, parent_obj.conf.num_queues, 1),
>>> DEFINE_PROP_UINT32("virtqueue_size", VirtIOSCSI,
>>> - parent_obj.conf.virtqueue_size, 128),
>>> + parent_obj.conf.virtqueue_size, VIRTQUEUE_DEFAULT_SIZE),
>>> DEFINE_PROP_BOOL("seg_max_adjust", VirtIOSCSI,
>>> parent_obj.conf.seg_max_adjust, true),
>>> DEFINE_PROP_UINT32("max_sectors", VirtIOSCSI, parent_obj.conf.max_sectors,
>>> diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
>>> index b69d517496..a66ea2368b 100644
>>> --- a/include/hw/virtio/virtio.h
>>> +++ b/include/hw/virtio/virtio.h
>>> @@ -48,6 +48,7 @@ size_t virtio_feature_get_config_size(VirtIOFeature *features,
>>> typedef struct VirtQueue VirtQueue;
>>>
>>> #define VIRTQUEUE_MAX_SIZE 1024
>>> +#define VIRTQUEUE_DEFAULT_SIZE 128
>> Going from the header only, this looks like a value that is supposed to
>> be used for every virtqueue... but from the users, this is only for blk
>> and scsi.
>>
>> I don't think adding a default for everything makes sense, even if the
>> same value makes sense for blk and scsi.
> Agreed, this value is too general. VIRTIO_BLK_VQ_DEFAULT_SIZE and
> VIRTIO_SCSI_VQ_DEFAULT_SIZE would make sense to me.
>
> Stefan
agree, that would be better.
Will redo and resend the series.
Denis
On Wed, Jan 29, 2020 at 05:06:59PM +0300, Denis Plotnikov wrote:
> Signed-off-by: Denis Plotnikov <dplotnikov@virtuozzo.com>
I'm not sure what the point is. It's more or less an accident that
these two devices share the queue size, this constance
makes no sense to me.
> ---
> hw/block/virtio-blk.c | 6 ++++--
> hw/scsi/virtio-scsi.c | 5 +++--
> include/hw/virtio/virtio.h | 1 +
> 3 files changed, 8 insertions(+), 4 deletions(-)
>
> diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
> index 09f46ed85f..72f935033f 100644
> --- a/hw/block/virtio-blk.c
> +++ b/hw/block/virtio-blk.c
> @@ -914,7 +914,8 @@ static void virtio_blk_update_config(VirtIODevice *vdev, uint8_t *config)
> memset(&blkcfg, 0, sizeof(blkcfg));
> virtio_stq_p(vdev, &blkcfg.capacity, capacity);
> virtio_stl_p(vdev, &blkcfg.seg_max,
> - s->conf.seg_max_adjust ? s->conf.queue_size - 2 : 128 - 2);
> + s->conf.seg_max_adjust ? s->conf.queue_size - 2 :
> + VIRTQUEUE_DEFAULT_SIZE - 2);
> virtio_stw_p(vdev, &blkcfg.geometry.cylinders, conf->cyls);
> virtio_stl_p(vdev, &blkcfg.blk_size, blk_size);
> virtio_stw_p(vdev, &blkcfg.min_io_size, conf->min_io_size / blk_size);
> @@ -1272,7 +1273,8 @@ static Property virtio_blk_properties[] = {
> DEFINE_PROP_BIT("request-merging", VirtIOBlock, conf.request_merging, 0,
> true),
> DEFINE_PROP_UINT16("num-queues", VirtIOBlock, conf.num_queues, 1),
> - DEFINE_PROP_UINT16("queue-size", VirtIOBlock, conf.queue_size, 128),
> + DEFINE_PROP_UINT16("queue-size", VirtIOBlock, conf.queue_size,
> + VIRTQUEUE_DEFAULT_SIZE),
> DEFINE_PROP_BOOL("seg-max-adjust", VirtIOBlock, conf.seg_max_adjust, true),
> DEFINE_PROP_LINK("iothread", VirtIOBlock, conf.iothread, TYPE_IOTHREAD,
> IOThread *),
> diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c
> index 3b61563609..36f66046ae 100644
> --- a/hw/scsi/virtio-scsi.c
> +++ b/hw/scsi/virtio-scsi.c
> @@ -660,7 +660,8 @@ static void virtio_scsi_get_config(VirtIODevice *vdev,
>
> virtio_stl_p(vdev, &scsiconf->num_queues, s->conf.num_queues);
> virtio_stl_p(vdev, &scsiconf->seg_max,
> - s->conf.seg_max_adjust ? s->conf.virtqueue_size - 2 : 128 - 2);
> + s->conf.seg_max_adjust ? s->conf.virtqueue_size - 2 :
> + VIRTQUEUE_DEFAULT_SIZE - 2);
> virtio_stl_p(vdev, &scsiconf->max_sectors, s->conf.max_sectors);
> virtio_stl_p(vdev, &scsiconf->cmd_per_lun, s->conf.cmd_per_lun);
> virtio_stl_p(vdev, &scsiconf->event_info_size, sizeof(VirtIOSCSIEvent));
> @@ -965,7 +966,7 @@ static void virtio_scsi_device_unrealize(DeviceState *dev, Error **errp)
> static Property virtio_scsi_properties[] = {
> DEFINE_PROP_UINT32("num_queues", VirtIOSCSI, parent_obj.conf.num_queues, 1),
> DEFINE_PROP_UINT32("virtqueue_size", VirtIOSCSI,
> - parent_obj.conf.virtqueue_size, 128),
> + parent_obj.conf.virtqueue_size, VIRTQUEUE_DEFAULT_SIZE),
> DEFINE_PROP_BOOL("seg_max_adjust", VirtIOSCSI,
> parent_obj.conf.seg_max_adjust, true),
> DEFINE_PROP_UINT32("max_sectors", VirtIOSCSI, parent_obj.conf.max_sectors,
> diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
> index b69d517496..a66ea2368b 100644
> --- a/include/hw/virtio/virtio.h
> +++ b/include/hw/virtio/virtio.h
> @@ -48,6 +48,7 @@ size_t virtio_feature_get_config_size(VirtIOFeature *features,
> typedef struct VirtQueue VirtQueue;
>
> #define VIRTQUEUE_MAX_SIZE 1024
> +#define VIRTQUEUE_DEFAULT_SIZE 128
>
> typedef struct VirtQueueElement
> {
> --
> 2.17.0
On 30.01.2020 16:38, Michael S. Tsirkin wrote:
> On Wed, Jan 29, 2020 at 05:06:59PM +0300, Denis Plotnikov wrote:
>> Signed-off-by: Denis Plotnikov <dplotnikov@virtuozzo.com>
>
> I'm not sure what the point is. It's more or less an accident that
> these two devices share the queue size, this constance
> makes no sense to me.
Ok, then let's just make a separate queue length constant for each type.
(Will redo and send in the next series)
Thanks!
Denis
>
>> ---
>> hw/block/virtio-blk.c | 6 ++++--
>> hw/scsi/virtio-scsi.c | 5 +++--
>> include/hw/virtio/virtio.h | 1 +
>> 3 files changed, 8 insertions(+), 4 deletions(-)
>>
>> diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
>> index 09f46ed85f..72f935033f 100644
>> --- a/hw/block/virtio-blk.c
>> +++ b/hw/block/virtio-blk.c
>> @@ -914,7 +914,8 @@ static void virtio_blk_update_config(VirtIODevice *vdev, uint8_t *config)
>> memset(&blkcfg, 0, sizeof(blkcfg));
>> virtio_stq_p(vdev, &blkcfg.capacity, capacity);
>> virtio_stl_p(vdev, &blkcfg.seg_max,
>> - s->conf.seg_max_adjust ? s->conf.queue_size - 2 : 128 - 2);
>> + s->conf.seg_max_adjust ? s->conf.queue_size - 2 :
>> + VIRTQUEUE_DEFAULT_SIZE - 2);
>> virtio_stw_p(vdev, &blkcfg.geometry.cylinders, conf->cyls);
>> virtio_stl_p(vdev, &blkcfg.blk_size, blk_size);
>> virtio_stw_p(vdev, &blkcfg.min_io_size, conf->min_io_size / blk_size);
>> @@ -1272,7 +1273,8 @@ static Property virtio_blk_properties[] = {
>> DEFINE_PROP_BIT("request-merging", VirtIOBlock, conf.request_merging, 0,
>> true),
>> DEFINE_PROP_UINT16("num-queues", VirtIOBlock, conf.num_queues, 1),
>> - DEFINE_PROP_UINT16("queue-size", VirtIOBlock, conf.queue_size, 128),
>> + DEFINE_PROP_UINT16("queue-size", VirtIOBlock, conf.queue_size,
>> + VIRTQUEUE_DEFAULT_SIZE),
>> DEFINE_PROP_BOOL("seg-max-adjust", VirtIOBlock, conf.seg_max_adjust, true),
>> DEFINE_PROP_LINK("iothread", VirtIOBlock, conf.iothread, TYPE_IOTHREAD,
>> IOThread *),
>> diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c
>> index 3b61563609..36f66046ae 100644
>> --- a/hw/scsi/virtio-scsi.c
>> +++ b/hw/scsi/virtio-scsi.c
>> @@ -660,7 +660,8 @@ static void virtio_scsi_get_config(VirtIODevice *vdev,
>>
>> virtio_stl_p(vdev, &scsiconf->num_queues, s->conf.num_queues);
>> virtio_stl_p(vdev, &scsiconf->seg_max,
>> - s->conf.seg_max_adjust ? s->conf.virtqueue_size - 2 : 128 - 2);
>> + s->conf.seg_max_adjust ? s->conf.virtqueue_size - 2 :
>> + VIRTQUEUE_DEFAULT_SIZE - 2);
>> virtio_stl_p(vdev, &scsiconf->max_sectors, s->conf.max_sectors);
>> virtio_stl_p(vdev, &scsiconf->cmd_per_lun, s->conf.cmd_per_lun);
>> virtio_stl_p(vdev, &scsiconf->event_info_size, sizeof(VirtIOSCSIEvent));
>> @@ -965,7 +966,7 @@ static void virtio_scsi_device_unrealize(DeviceState *dev, Error **errp)
>> static Property virtio_scsi_properties[] = {
>> DEFINE_PROP_UINT32("num_queues", VirtIOSCSI, parent_obj.conf.num_queues, 1),
>> DEFINE_PROP_UINT32("virtqueue_size", VirtIOSCSI,
>> - parent_obj.conf.virtqueue_size, 128),
>> + parent_obj.conf.virtqueue_size, VIRTQUEUE_DEFAULT_SIZE),
>> DEFINE_PROP_BOOL("seg_max_adjust", VirtIOSCSI,
>> parent_obj.conf.seg_max_adjust, true),
>> DEFINE_PROP_UINT32("max_sectors", VirtIOSCSI, parent_obj.conf.max_sectors,
>> diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
>> index b69d517496..a66ea2368b 100644
>> --- a/include/hw/virtio/virtio.h
>> +++ b/include/hw/virtio/virtio.h
>> @@ -48,6 +48,7 @@ size_t virtio_feature_get_config_size(VirtIOFeature *features,
>> typedef struct VirtQueue VirtQueue;
>>
>> #define VIRTQUEUE_MAX_SIZE 1024
>> +#define VIRTQUEUE_DEFAULT_SIZE 128
>>
>> typedef struct VirtQueueElement
>> {
>> --
>> 2.17.0
On Mon, Feb 03, 2020 at 03:17:07PM +0300, Denis Plotnikov wrote:
>
>
> On 30.01.2020 16:38, Michael S. Tsirkin wrote:
> > On Wed, Jan 29, 2020 at 05:06:59PM +0300, Denis Plotnikov wrote:
> > > Signed-off-by: Denis Plotnikov <dplotnikov@virtuozzo.com>
> >
> > I'm not sure what the point is. It's more or less an accident that
> > these two devices share the queue size, this constance
> > makes no sense to me.
> Ok, then let's just make a separate queue length constant for each type.
it's just a number, I don't think we need a constant here.
If you feel it needs documentation, add a comment!
> (Will redo and send in the next series)
> Thanks!
>
> Denis
> >
> > > ---
> > > hw/block/virtio-blk.c | 6 ++++--
> > > hw/scsi/virtio-scsi.c | 5 +++--
> > > include/hw/virtio/virtio.h | 1 +
> > > 3 files changed, 8 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
> > > index 09f46ed85f..72f935033f 100644
> > > --- a/hw/block/virtio-blk.c
> > > +++ b/hw/block/virtio-blk.c
> > > @@ -914,7 +914,8 @@ static void virtio_blk_update_config(VirtIODevice *vdev, uint8_t *config)
> > > memset(&blkcfg, 0, sizeof(blkcfg));
> > > virtio_stq_p(vdev, &blkcfg.capacity, capacity);
> > > virtio_stl_p(vdev, &blkcfg.seg_max,
> > > - s->conf.seg_max_adjust ? s->conf.queue_size - 2 : 128 - 2);
> > > + s->conf.seg_max_adjust ? s->conf.queue_size - 2 :
> > > + VIRTQUEUE_DEFAULT_SIZE - 2);
> > > virtio_stw_p(vdev, &blkcfg.geometry.cylinders, conf->cyls);
> > > virtio_stl_p(vdev, &blkcfg.blk_size, blk_size);
> > > virtio_stw_p(vdev, &blkcfg.min_io_size, conf->min_io_size / blk_size);
> > > @@ -1272,7 +1273,8 @@ static Property virtio_blk_properties[] = {
> > > DEFINE_PROP_BIT("request-merging", VirtIOBlock, conf.request_merging, 0,
> > > true),
> > > DEFINE_PROP_UINT16("num-queues", VirtIOBlock, conf.num_queues, 1),
> > > - DEFINE_PROP_UINT16("queue-size", VirtIOBlock, conf.queue_size, 128),
> > > + DEFINE_PROP_UINT16("queue-size", VirtIOBlock, conf.queue_size,
> > > + VIRTQUEUE_DEFAULT_SIZE),
> > > DEFINE_PROP_BOOL("seg-max-adjust", VirtIOBlock, conf.seg_max_adjust, true),
> > > DEFINE_PROP_LINK("iothread", VirtIOBlock, conf.iothread, TYPE_IOTHREAD,
> > > IOThread *),
> > > diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c
> > > index 3b61563609..36f66046ae 100644
> > > --- a/hw/scsi/virtio-scsi.c
> > > +++ b/hw/scsi/virtio-scsi.c
> > > @@ -660,7 +660,8 @@ static void virtio_scsi_get_config(VirtIODevice *vdev,
> > > virtio_stl_p(vdev, &scsiconf->num_queues, s->conf.num_queues);
> > > virtio_stl_p(vdev, &scsiconf->seg_max,
> > > - s->conf.seg_max_adjust ? s->conf.virtqueue_size - 2 : 128 - 2);
> > > + s->conf.seg_max_adjust ? s->conf.virtqueue_size - 2 :
> > > + VIRTQUEUE_DEFAULT_SIZE - 2);
> > > virtio_stl_p(vdev, &scsiconf->max_sectors, s->conf.max_sectors);
> > > virtio_stl_p(vdev, &scsiconf->cmd_per_lun, s->conf.cmd_per_lun);
> > > virtio_stl_p(vdev, &scsiconf->event_info_size, sizeof(VirtIOSCSIEvent));
> > > @@ -965,7 +966,7 @@ static void virtio_scsi_device_unrealize(DeviceState *dev, Error **errp)
> > > static Property virtio_scsi_properties[] = {
> > > DEFINE_PROP_UINT32("num_queues", VirtIOSCSI, parent_obj.conf.num_queues, 1),
> > > DEFINE_PROP_UINT32("virtqueue_size", VirtIOSCSI,
> > > - parent_obj.conf.virtqueue_size, 128),
> > > + parent_obj.conf.virtqueue_size, VIRTQUEUE_DEFAULT_SIZE),
> > > DEFINE_PROP_BOOL("seg_max_adjust", VirtIOSCSI,
> > > parent_obj.conf.seg_max_adjust, true),
> > > DEFINE_PROP_UINT32("max_sectors", VirtIOSCSI, parent_obj.conf.max_sectors,
> > > diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
> > > index b69d517496..a66ea2368b 100644
> > > --- a/include/hw/virtio/virtio.h
> > > +++ b/include/hw/virtio/virtio.h
> > > @@ -48,6 +48,7 @@ size_t virtio_feature_get_config_size(VirtIOFeature *features,
> > > typedef struct VirtQueue VirtQueue;
> > > #define VIRTQUEUE_MAX_SIZE 1024
> > > +#define VIRTQUEUE_DEFAULT_SIZE 128
> > > typedef struct VirtQueueElement
> > > {
> > > --
> > > 2.17.0
On 03.02.2020 15:51, Michael S. Tsirkin wrote:
> On Mon, Feb 03, 2020 at 03:17:07PM +0300, Denis Plotnikov wrote:
>>
>> On 30.01.2020 16:38, Michael S. Tsirkin wrote:
>>> On Wed, Jan 29, 2020 at 05:06:59PM +0300, Denis Plotnikov wrote:
>>>> Signed-off-by: Denis Plotnikov <dplotnikov@virtuozzo.com>
>>> I'm not sure what the point is. It's more or less an accident that
>>> these two devices share the queue size, this constance
>>> makes no sense to me.
>> Ok, then let's just make a separate queue length constant for each type.
> it's just a number, I don't think we need a constant here.
> If you feel it needs documentation, add a comment!
I just thought that the meaningful name for the number would be better
for the code understanding.
Anyway, If doesn't improve anything I'll just change the number and add
a comment what it means.
Denis
>
>> (Will redo and send in the next series)
>> Thanks!
>>
>> Denis
>>>> ---
>>>> hw/block/virtio-blk.c | 6 ++++--
>>>> hw/scsi/virtio-scsi.c | 5 +++--
>>>> include/hw/virtio/virtio.h | 1 +
>>>> 3 files changed, 8 insertions(+), 4 deletions(-)
>>>>
>>>> diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
>>>> index 09f46ed85f..72f935033f 100644
>>>> --- a/hw/block/virtio-blk.c
>>>> +++ b/hw/block/virtio-blk.c
>>>> @@ -914,7 +914,8 @@ static void virtio_blk_update_config(VirtIODevice *vdev, uint8_t *config)
>>>> memset(&blkcfg, 0, sizeof(blkcfg));
>>>> virtio_stq_p(vdev, &blkcfg.capacity, capacity);
>>>> virtio_stl_p(vdev, &blkcfg.seg_max,
>>>> - s->conf.seg_max_adjust ? s->conf.queue_size - 2 : 128 - 2);
>>>> + s->conf.seg_max_adjust ? s->conf.queue_size - 2 :
>>>> + VIRTQUEUE_DEFAULT_SIZE - 2);
>>>> virtio_stw_p(vdev, &blkcfg.geometry.cylinders, conf->cyls);
>>>> virtio_stl_p(vdev, &blkcfg.blk_size, blk_size);
>>>> virtio_stw_p(vdev, &blkcfg.min_io_size, conf->min_io_size / blk_size);
>>>> @@ -1272,7 +1273,8 @@ static Property virtio_blk_properties[] = {
>>>> DEFINE_PROP_BIT("request-merging", VirtIOBlock, conf.request_merging, 0,
>>>> true),
>>>> DEFINE_PROP_UINT16("num-queues", VirtIOBlock, conf.num_queues, 1),
>>>> - DEFINE_PROP_UINT16("queue-size", VirtIOBlock, conf.queue_size, 128),
>>>> + DEFINE_PROP_UINT16("queue-size", VirtIOBlock, conf.queue_size,
>>>> + VIRTQUEUE_DEFAULT_SIZE),
>>>> DEFINE_PROP_BOOL("seg-max-adjust", VirtIOBlock, conf.seg_max_adjust, true),
>>>> DEFINE_PROP_LINK("iothread", VirtIOBlock, conf.iothread, TYPE_IOTHREAD,
>>>> IOThread *),
>>>> diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c
>>>> index 3b61563609..36f66046ae 100644
>>>> --- a/hw/scsi/virtio-scsi.c
>>>> +++ b/hw/scsi/virtio-scsi.c
>>>> @@ -660,7 +660,8 @@ static void virtio_scsi_get_config(VirtIODevice *vdev,
>>>> virtio_stl_p(vdev, &scsiconf->num_queues, s->conf.num_queues);
>>>> virtio_stl_p(vdev, &scsiconf->seg_max,
>>>> - s->conf.seg_max_adjust ? s->conf.virtqueue_size - 2 : 128 - 2);
>>>> + s->conf.seg_max_adjust ? s->conf.virtqueue_size - 2 :
>>>> + VIRTQUEUE_DEFAULT_SIZE - 2);
>>>> virtio_stl_p(vdev, &scsiconf->max_sectors, s->conf.max_sectors);
>>>> virtio_stl_p(vdev, &scsiconf->cmd_per_lun, s->conf.cmd_per_lun);
>>>> virtio_stl_p(vdev, &scsiconf->event_info_size, sizeof(VirtIOSCSIEvent));
>>>> @@ -965,7 +966,7 @@ static void virtio_scsi_device_unrealize(DeviceState *dev, Error **errp)
>>>> static Property virtio_scsi_properties[] = {
>>>> DEFINE_PROP_UINT32("num_queues", VirtIOSCSI, parent_obj.conf.num_queues, 1),
>>>> DEFINE_PROP_UINT32("virtqueue_size", VirtIOSCSI,
>>>> - parent_obj.conf.virtqueue_size, 128),
>>>> + parent_obj.conf.virtqueue_size, VIRTQUEUE_DEFAULT_SIZE),
>>>> DEFINE_PROP_BOOL("seg_max_adjust", VirtIOSCSI,
>>>> parent_obj.conf.seg_max_adjust, true),
>>>> DEFINE_PROP_UINT32("max_sectors", VirtIOSCSI, parent_obj.conf.max_sectors,
>>>> diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
>>>> index b69d517496..a66ea2368b 100644
>>>> --- a/include/hw/virtio/virtio.h
>>>> +++ b/include/hw/virtio/virtio.h
>>>> @@ -48,6 +48,7 @@ size_t virtio_feature_get_config_size(VirtIOFeature *features,
>>>> typedef struct VirtQueue VirtQueue;
>>>> #define VIRTQUEUE_MAX_SIZE 1024
>>>> +#define VIRTQUEUE_DEFAULT_SIZE 128
>>>> typedef struct VirtQueueElement
>>>> {
>>>> --
>>>> 2.17.0
© 2016 - 2025 Red Hat, Inc.