[Qemu-devel] [PATCH v2 0/2] virtio: cosmetic tweaks

[Qemu-devel] [PATCH v2 0/2] virtio: cosmetic tweaks

[Ladi Prosek]

Just a trivial couple of patches addressing minor style issues.

v1 -> v2:
  * rebased on top of current master

Ladi Prosek (2):
  virtio: rename virtqueue_discard to virtqueue_unpop
  virtio: make virtqueue_alloc_element static

[Qemu-devel] [PATCH v2 1/2] virtio: rename virtqueue_discard to virtqueue_unpop

[Ladi Prosek]

The function undoes the effect of virtqueue_pop and doesn't do anything
destructive or irreversible so virtqueue_unpop is a more fitting name.

Signed-off-by: Ladi Prosek <lprosek@redhat.com>
---
 hw/net/virtio-net.c        | 2 +-
 hw/virtio/virtio-balloon.c | 2 +-
 hw/virtio/virtio.c         | 8 ++++----
 include/hw/virtio/virtio.h | 4 ++--
 4 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index 06bfe4b..20aa63e 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -1177,7 +1177,7 @@ static ssize_t virtio_net_receive(NetClientState *nc, const uint8_t *buf, size_t
          * must have consumed the complete packet.
          * Otherwise, drop it. */
         if (!n->mergeable_rx_bufs && offset < size) {
-            virtqueue_discard(q->rx_vq, elem, total);
+            virtqueue_unpop(q->rx_vq, elem, total);
             g_free(elem);
             return size;
         }
diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c
index 1d77028..8e3b91c 100644
--- a/hw/virtio/virtio-balloon.c
+++ b/hw/virtio/virtio-balloon.c
@@ -456,7 +456,7 @@ static void virtio_balloon_device_reset(VirtIODevice *vdev)
     VirtIOBalloon *s = VIRTIO_BALLOON(vdev);
 
     if (s->stats_vq_elem != NULL) {
-        virtqueue_discard(s->svq, s->stats_vq_elem, 0);
+        virtqueue_unpop(s->svq, s->stats_vq_elem, 0);
         g_free(s->stats_vq_elem);
         s->stats_vq_elem = NULL;
     }
diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index d48d1a9..68971fd 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -280,7 +280,7 @@ void virtqueue_detach_element(VirtQueue *vq, const VirtQueueElement *elem,
     virtqueue_unmap_sg(vq, elem, len);
 }
 
-/* virtqueue_discard:
+/* virtqueue_unpop:
  * @vq: The #VirtQueue
  * @elem: The #VirtQueueElement
  * @len: number of bytes written
@@ -288,8 +288,8 @@ void virtqueue_detach_element(VirtQueue *vq, const VirtQueueElement *elem,
  * Pretend the most recent element wasn't popped from the virtqueue.  The next
  * call to virtqueue_pop() will refetch the element.
  */
-void virtqueue_discard(VirtQueue *vq, const VirtQueueElement *elem,
-                       unsigned int len)
+void virtqueue_unpop(VirtQueue *vq, const VirtQueueElement *elem,
+                     unsigned int len)
 {
     vq->last_avail_idx--;
     virtqueue_detach_element(vq, elem, len);
@@ -302,7 +302,7 @@ void virtqueue_discard(VirtQueue *vq, const VirtQueueElement *elem,
  * Pretend that elements weren't popped from the virtqueue.  The next
  * virtqueue_pop() will refetch the oldest element.
  *
- * Use virtqueue_discard() instead if you have a VirtQueueElement.
+ * Use virtqueue_unpop() instead if you have a VirtQueueElement.
  *
  * Returns: true on success, false if @num is greater than the number of in use
  * elements.
diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
index b913aac..95adaca 100644
--- a/include/hw/virtio/virtio.h
+++ b/include/hw/virtio/virtio.h
@@ -157,8 +157,8 @@ void virtqueue_push(VirtQueue *vq, const VirtQueueElement *elem,
 void virtqueue_flush(VirtQueue *vq, unsigned int count);
 void virtqueue_detach_element(VirtQueue *vq, const VirtQueueElement *elem,
                               unsigned int len);
-void virtqueue_discard(VirtQueue *vq, const VirtQueueElement *elem,
-                       unsigned int len);
+void virtqueue_unpop(VirtQueue *vq, const VirtQueueElement *elem,
+                     unsigned int len);
 bool virtqueue_rewind(VirtQueue *vq, unsigned int num);
 void virtqueue_fill(VirtQueue *vq, const VirtQueueElement *elem,
                     unsigned int len, unsigned int idx);
-- 
2.5.5

[Qemu-devel] [PATCH v2 2/2] virtio: make virtqueue_alloc_element static

[Ladi Prosek]

The function does not fully initialize the returned VirtQueueElement and should
be used only internally from the virtio module.

Signed-off-by: Ladi Prosek <lprosek@redhat.com>
---
 hw/virtio/virtio.c         | 2 +-
 include/hw/virtio/virtio.h | 1 -
 2 files changed, 1 insertion(+), 2 deletions(-)

diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index 68971fd..2471722 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -633,7 +633,7 @@ void virtqueue_map(VirtQueueElement *elem)
                         VIRTQUEUE_MAX_SIZE, 0);
 }
 
-void *virtqueue_alloc_element(size_t sz, unsigned out_num, unsigned in_num)
+static void *virtqueue_alloc_element(size_t sz, unsigned out_num, unsigned in_num)
 {
     VirtQueueElement *elem;
     size_t in_addr_ofs = QEMU_ALIGN_UP(sz, __alignof__(elem->in_addr[0]));
diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
index 95adaca..0deafb7 100644
--- a/include/hw/virtio/virtio.h
+++ b/include/hw/virtio/virtio.h
@@ -151,7 +151,6 @@ VirtQueue *virtio_add_queue_aio(VirtIODevice *vdev, int queue_size,
 
 void virtio_del_queue(VirtIODevice *vdev, int n);
 
-void *virtqueue_alloc_element(size_t sz, unsigned out_num, unsigned in_num);
 void virtqueue_push(VirtQueue *vq, const VirtQueueElement *elem,
                     unsigned int len);
 void virtqueue_flush(VirtQueue *vq, unsigned int count);
-- 
2.5.5

Re: [Qemu-devel] [PATCH v2 1/2] virtio: rename virtqueue_discard to virtqueue_unpop

[Alexey Kardashevskiy]

On 03/11/16 19:55, Ladi Prosek wrote:
> The function undoes the effect of virtqueue_pop and doesn't do anything
> destructive or irreversible so virtqueue_unpop is a more fitting name.

virtqueue_undo_pop() is even better, it was suggested by native english
speaker (i.e. not myself) :)



> 
> Signed-off-by: Ladi Prosek <lprosek@redhat.com>
> ---
>  hw/net/virtio-net.c        | 2 +-
>  hw/virtio/virtio-balloon.c | 2 +-
>  hw/virtio/virtio.c         | 8 ++++----
>  include/hw/virtio/virtio.h | 4 ++--
>  4 files changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
> index 06bfe4b..20aa63e 100644
> --- a/hw/net/virtio-net.c
> +++ b/hw/net/virtio-net.c
> @@ -1177,7 +1177,7 @@ static ssize_t virtio_net_receive(NetClientState *nc, const uint8_t *buf, size_t
>           * must have consumed the complete packet.
>           * Otherwise, drop it. */
>          if (!n->mergeable_rx_bufs && offset < size) {
> -            virtqueue_discard(q->rx_vq, elem, total);
> +            virtqueue_unpop(q->rx_vq, elem, total);
>              g_free(elem);
>              return size;
>          }
> diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c
> index 1d77028..8e3b91c 100644
> --- a/hw/virtio/virtio-balloon.c
> +++ b/hw/virtio/virtio-balloon.c
> @@ -456,7 +456,7 @@ static void virtio_balloon_device_reset(VirtIODevice *vdev)
>      VirtIOBalloon *s = VIRTIO_BALLOON(vdev);
>  
>      if (s->stats_vq_elem != NULL) {
> -        virtqueue_discard(s->svq, s->stats_vq_elem, 0);
> +        virtqueue_unpop(s->svq, s->stats_vq_elem, 0);
>          g_free(s->stats_vq_elem);
>          s->stats_vq_elem = NULL;
>      }
> diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
> index d48d1a9..68971fd 100644
> --- a/hw/virtio/virtio.c
> +++ b/hw/virtio/virtio.c
> @@ -280,7 +280,7 @@ void virtqueue_detach_element(VirtQueue *vq, const VirtQueueElement *elem,
>      virtqueue_unmap_sg(vq, elem, len);
>  }
>  
> -/* virtqueue_discard:
> +/* virtqueue_unpop:
>   * @vq: The #VirtQueue
>   * @elem: The #VirtQueueElement
>   * @len: number of bytes written
> @@ -288,8 +288,8 @@ void virtqueue_detach_element(VirtQueue *vq, const VirtQueueElement *elem,
>   * Pretend the most recent element wasn't popped from the virtqueue.  The next
>   * call to virtqueue_pop() will refetch the element.
>   */
> -void virtqueue_discard(VirtQueue *vq, const VirtQueueElement *elem,
> -                       unsigned int len)
> +void virtqueue_unpop(VirtQueue *vq, const VirtQueueElement *elem,
> +                     unsigned int len)
>  {
>      vq->last_avail_idx--;
>      virtqueue_detach_element(vq, elem, len);
> @@ -302,7 +302,7 @@ void virtqueue_discard(VirtQueue *vq, const VirtQueueElement *elem,
>   * Pretend that elements weren't popped from the virtqueue.  The next
>   * virtqueue_pop() will refetch the oldest element.
>   *
> - * Use virtqueue_discard() instead if you have a VirtQueueElement.
> + * Use virtqueue_unpop() instead if you have a VirtQueueElement.
>   *
>   * Returns: true on success, false if @num is greater than the number of in use
>   * elements.
> diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
> index b913aac..95adaca 100644
> --- a/include/hw/virtio/virtio.h
> +++ b/include/hw/virtio/virtio.h
> @@ -157,8 +157,8 @@ void virtqueue_push(VirtQueue *vq, const VirtQueueElement *elem,
>  void virtqueue_flush(VirtQueue *vq, unsigned int count);
>  void virtqueue_detach_element(VirtQueue *vq, const VirtQueueElement *elem,
>                                unsigned int len);
> -void virtqueue_discard(VirtQueue *vq, const VirtQueueElement *elem,
> -                       unsigned int len);
> +void virtqueue_unpop(VirtQueue *vq, const VirtQueueElement *elem,
> +                     unsigned int len);
>  bool virtqueue_rewind(VirtQueue *vq, unsigned int num);
>  void virtqueue_fill(VirtQueue *vq, const VirtQueueElement *elem,
>                      unsigned int len, unsigned int idx);
> 


-- 
Alexey

Re: [Qemu-devel] [PATCH v2 1/2] virtio: rename virtqueue_discard to virtqueue_unpop

[Ladi Prosek]

On Fri, Nov 4, 2016 at 3:42 AM, Alexey Kardashevskiy <aik@ozlabs.ru> wrote:
> On 03/11/16 19:55, Ladi Prosek wrote:
>> The function undoes the effect of virtqueue_pop and doesn't do anything
>> destructive or irreversible so virtqueue_unpop is a more fitting name.
>
> virtqueue_undo_pop() is even better, it was suggested by native english
> speaker (i.e. not myself) :)

virtqueue_undo_pop sounds good too, I have no preference between the
two really :) Thanks!

>>
>> Signed-off-by: Ladi Prosek <lprosek@redhat.com>
>> ---
>>  hw/net/virtio-net.c        | 2 +-
>>  hw/virtio/virtio-balloon.c | 2 +-
>>  hw/virtio/virtio.c         | 8 ++++----
>>  include/hw/virtio/virtio.h | 4 ++--
>>  4 files changed, 8 insertions(+), 8 deletions(-)
>>
>> diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
>> index 06bfe4b..20aa63e 100644
>> --- a/hw/net/virtio-net.c
>> +++ b/hw/net/virtio-net.c
>> @@ -1177,7 +1177,7 @@ static ssize_t virtio_net_receive(NetClientState *nc, const uint8_t *buf, size_t
>>           * must have consumed the complete packet.
>>           * Otherwise, drop it. */
>>          if (!n->mergeable_rx_bufs && offset < size) {
>> -            virtqueue_discard(q->rx_vq, elem, total);
>> +            virtqueue_unpop(q->rx_vq, elem, total);
>>              g_free(elem);
>>              return size;
>>          }
>> diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c
>> index 1d77028..8e3b91c 100644
>> --- a/hw/virtio/virtio-balloon.c
>> +++ b/hw/virtio/virtio-balloon.c
>> @@ -456,7 +456,7 @@ static void virtio_balloon_device_reset(VirtIODevice *vdev)
>>      VirtIOBalloon *s = VIRTIO_BALLOON(vdev);
>>
>>      if (s->stats_vq_elem != NULL) {
>> -        virtqueue_discard(s->svq, s->stats_vq_elem, 0);
>> +        virtqueue_unpop(s->svq, s->stats_vq_elem, 0);
>>          g_free(s->stats_vq_elem);
>>          s->stats_vq_elem = NULL;
>>      }
>> diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
>> index d48d1a9..68971fd 100644
>> --- a/hw/virtio/virtio.c
>> +++ b/hw/virtio/virtio.c
>> @@ -280,7 +280,7 @@ void virtqueue_detach_element(VirtQueue *vq, const VirtQueueElement *elem,
>>      virtqueue_unmap_sg(vq, elem, len);
>>  }
>>
>> -/* virtqueue_discard:
>> +/* virtqueue_unpop:
>>   * @vq: The #VirtQueue
>>   * @elem: The #VirtQueueElement
>>   * @len: number of bytes written
>> @@ -288,8 +288,8 @@ void virtqueue_detach_element(VirtQueue *vq, const VirtQueueElement *elem,
>>   * Pretend the most recent element wasn't popped from the virtqueue.  The next
>>   * call to virtqueue_pop() will refetch the element.
>>   */
>> -void virtqueue_discard(VirtQueue *vq, const VirtQueueElement *elem,
>> -                       unsigned int len)
>> +void virtqueue_unpop(VirtQueue *vq, const VirtQueueElement *elem,
>> +                     unsigned int len)
>>  {
>>      vq->last_avail_idx--;
>>      virtqueue_detach_element(vq, elem, len);
>> @@ -302,7 +302,7 @@ void virtqueue_discard(VirtQueue *vq, const VirtQueueElement *elem,
>>   * Pretend that elements weren't popped from the virtqueue.  The next
>>   * virtqueue_pop() will refetch the oldest element.
>>   *
>> - * Use virtqueue_discard() instead if you have a VirtQueueElement.
>> + * Use virtqueue_unpop() instead if you have a VirtQueueElement.
>>   *
>>   * Returns: true on success, false if @num is greater than the number of in use
>>   * elements.
>> diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
>> index b913aac..95adaca 100644
>> --- a/include/hw/virtio/virtio.h
>> +++ b/include/hw/virtio/virtio.h
>> @@ -157,8 +157,8 @@ void virtqueue_push(VirtQueue *vq, const VirtQueueElement *elem,
>>  void virtqueue_flush(VirtQueue *vq, unsigned int count);
>>  void virtqueue_detach_element(VirtQueue *vq, const VirtQueueElement *elem,
>>                                unsigned int len);
>> -void virtqueue_discard(VirtQueue *vq, const VirtQueueElement *elem,
>> -                       unsigned int len);
>> +void virtqueue_unpop(VirtQueue *vq, const VirtQueueElement *elem,
>> +                     unsigned int len);
>>  bool virtqueue_rewind(VirtQueue *vq, unsigned int num);
>>  void virtqueue_fill(VirtQueue *vq, const VirtQueueElement *elem,
>>                      unsigned int len, unsigned int idx);
>>
>
>
> --
> Alexey

Re: [Qemu-devel] [PATCH v2 1/2] virtio: rename virtqueue_discard to virtqueue_unpop

[Cornelia Huck]

On Fri, 4 Nov 2016 07:19:16 +0100
Ladi Prosek <lprosek@redhat.com> wrote:

> On Fri, Nov 4, 2016 at 3:42 AM, Alexey Kardashevskiy <aik@ozlabs.ru> wrote:
> > On 03/11/16 19:55, Ladi Prosek wrote:
> >> The function undoes the effect of virtqueue_pop and doesn't do anything
> >> destructive or irreversible so virtqueue_unpop is a more fitting name.
> >
> > virtqueue_undo_pop() is even better, it was suggested by native english
> > speaker (i.e. not myself) :)
> 
> virtqueue_undo_pop sounds good too, I have no preference between the
> two really :) Thanks!

If you need a tie breaker: I like virtqueue_undo_pop() better :)

Re: [Qemu-devel] [PATCH v2 2/2] virtio: make virtqueue_alloc_element static

[Cornelia Huck]

On Thu,  3 Nov 2016 09:55:50 +0100
Ladi Prosek <lprosek@redhat.com> wrote:

> The function does not fully initialize the returned VirtQueueElement and should
> be used only internally from the virtio module.
> 
> Signed-off-by: Ladi Prosek <lprosek@redhat.com>
> ---
>  hw/virtio/virtio.c         | 2 +-
>  include/hw/virtio/virtio.h | 1 -
>  2 files changed, 1 insertion(+), 2 deletions(-)

Reviewed-by: Cornelia Huck <cornelia.huck@de.ibm.com>

...although I'm wondering if the callers could not pass in the index
value as well? (Still keeping this internal to virtio.c makes sense.)