Re: [PATCH 2/5] virtio-blk: clarify that there is at least 1 virtqueue

[Hanna Czenczek]

On 05.02.24 18:26, Stefan Hajnoczi wrote:

It is not possible to instantiate a virtio-blk device with 0 virtqueues.
The following check is located in ->realize():

   if (!conf->num_queues) {
   error_setg(errp, "num-queues property must be larger than 0");
   return;
   }

Later on we access s->vq_aio_context[0] under the assumption that there
is as least one virtqueue. Hanna Czenczek  noted that
it would help to show that the array index is already valid.

Add an assertion to document that s->vq_aio_context[0] is always
safe...and catch future code changes that break this assumption.

Suggested-by: Hanna Czenczek
Signed-off-by: Stefan Hajnoczi
---
  hw/block/virtio-blk.c | 1 +
  1 file changed, 1 insertion(+)


Reviewed-by: Hanna Czenczek 

Re: [PATCH 2/5] virtio-blk: clarify that there is at least 1 virtqueue

[Manos Pitsidianakis]

On Mon, 05 Feb 2024 19:26, Stefan Hajnoczi  wrote:

It is not possible to instantiate a virtio-blk device with 0 virtqueues.
The following check is located in ->realize():

 if (!conf->num_queues) {
 error_setg(errp, "num-queues property must be larger than 0");
 return;
 }

Later on we access s->vq_aio_context[0] under the assumption that there
is as least one virtqueue. Hanna Czenczek  noted that
it would help to show that the array index is already valid.

Add an assertion to document that s->vq_aio_context[0] is always
safe...and catch future code changes that break this assumption.

Suggested-by: Hanna Czenczek 
Signed-off-by: Stefan Hajnoczi 
---
hw/block/virtio-blk.c | 1 +
1 file changed, 1 insertion(+)

diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
index e8b37fd5f4..a0735a9bca 100644
--- a/hw/block/virtio-blk.c
+++ b/hw/block/virtio-blk.c
@@ -1825,6 +1825,7 @@ static int virtio_blk_start_ioeventfd(VirtIODevice *vdev)
 * Try to change the AioContext so that block jobs and other operations can
 * co-locate their activity in the same AioContext. If it fails, nevermind.
 */
+assert(nvqs > 0); /* enforced during ->realize() */
r = blk_set_aio_context(s->conf.conf.blk, s->vq_aio_context[0],
_err);
if (r < 0) {
--
2.43.0



Reviewed-by: Manos Pitsidianakis 

[PATCH 2/5] virtio-blk: clarify that there is at least 1 virtqueue

[Stefan Hajnoczi]

It is not possible to instantiate a virtio-blk device with 0 virtqueues.
The following check is located in ->realize():

  if (!conf->num_queues) {
  error_setg(errp, "num-queues property must be larger than 0");
  return;
  }

Later on we access s->vq_aio_context[0] under the assumption that there
is as least one virtqueue. Hanna Czenczek  noted that
it would help to show that the array index is already valid.

Add an assertion to document that s->vq_aio_context[0] is always
safe...and catch future code changes that break this assumption.

Suggested-by: Hanna Czenczek 
Signed-off-by: Stefan Hajnoczi 
---
 hw/block/virtio-blk.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
index e8b37fd5f4..a0735a9bca 100644
--- a/hw/block/virtio-blk.c
+++ b/hw/block/virtio-blk.c
@@ -1825,6 +1825,7 @@ static int virtio_blk_start_ioeventfd(VirtIODevice *vdev)
  * Try to change the AioContext so that block jobs and other operations can
  * co-locate their activity in the same AioContext. If it fails, nevermind.
  */
+assert(nvqs > 0); /* enforced during ->realize() */
 r = blk_set_aio_context(s->conf.conf.blk, s->vq_aio_context[0],
 _err);
 if (r < 0) {
-- 
2.43.0