[Qemu-devel] [PATCH v1] virtio-net: code cleanup

[Qemu-devel] [PATCH v1] virtio-net: code cleanup

[Wei Wang]

Use is_power_of_2(), and remove trailing "." from error_setg().

Signed-off-by: Wei Wang <wei.w.wang@intel.com>
---
 hw/net/virtio-net.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index 91eddaf..144169c 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -1918,9 +1918,9 @@ static void virtio_net_device_realize(DeviceState *dev, Error **errp)
      */
     if (n->net_conf.rx_queue_size < VIRTIO_NET_RX_QUEUE_MIN_SIZE ||
         n->net_conf.rx_queue_size > VIRTQUEUE_MAX_SIZE ||
-        (n->net_conf.rx_queue_size & (n->net_conf.rx_queue_size - 1))) {
+        !is_power_of_2(n->net_conf.rx_queue_size)) {
         error_setg(errp, "Invalid rx_queue_size (= %" PRIu16 "), "
-                   "must be a power of 2 between %d and %d.",
+                   "must be a power of 2 between %d and %d",
                    n->net_conf.rx_queue_size, VIRTIO_NET_RX_QUEUE_MIN_SIZE,
                    VIRTQUEUE_MAX_SIZE);
         virtio_cleanup(vdev);
@@ -1930,7 +1930,7 @@ static void virtio_net_device_realize(DeviceState *dev, Error **errp)
     n->max_queues = MAX(n->nic_conf.peers.queues, 1);
     if (n->max_queues * 2 + 1 > VIRTIO_QUEUE_MAX) {
         error_setg(errp, "Invalid number of queues (= %" PRIu32 "), "
-                   "must be a positive integer less than %d.",
+                   "must be a positive integer less than %d",
                    n->max_queues, (VIRTIO_QUEUE_MAX - 1) / 2);
         virtio_cleanup(vdev);
         return;
-- 
2.7.4

Re: [Qemu-devel] [PATCH v1] virtio-net: code cleanup

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 435 bytes --]

On 06/28/2017 12:53 AM, Wei Wang wrote:
> Use is_power_of_2(), and remove trailing "." from error_setg().
> 
> Signed-off-by: Wei Wang <wei.w.wang@intel.com>
> ---
>  hw/net/virtio-net.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

Re: [Qemu-devel] [PATCH v1] virtio-net: code cleanup

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 1525 bytes --]

[meta-comment, replacing virtio-dev with virtio-dev-owner in cc]

On 06/28/2017 07:22 AM, Eric Blake wrote:
> On 06/28/2017 12:53 AM, Wei Wang wrote:
>> Use is_power_of_2(), and remove trailing "." from error_setg().
>>
>> Signed-off-by: Wei Wang <wei.w.wang@intel.com>
>> ---
>>  hw/net/virtio-net.c | 6 +++---
>>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> Reviewed-by: Eric Blake <eblake@redhat.com>
> 

I got this rejection letter:

> 
>                    The mail system
> 
> <virtio-dev@lists.oasis-open.org>: permission denied. Command output: Sorry,
>     only subscribers may post. If you are a subscriber, please forward this
>     message to virtio-dev-owner@lists.oasis-open.org to get your new address
>     included (#5.7.2) ERROR: postqmail-local Error #77

It's generally considered poor netiquette to cc both an open list and a
closed list that rejects emails from non-subscribers (note that you do
NOT have to subscribe to qemu-devel to post to it, although a moderation
delay may be involved).  It's not necessarily my place to tell the
virtio-dev list what to do (since I don't subscribe), but it may be
worth considereing relaxing the list posting policies there to allow
moderated posts from non-subscribers, especially if it is going to be
common-place that people want to cc virtio-dev at the same time as
another open list.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

Re: [Qemu-devel] [PATCH v1] virtio-net: code cleanup

[Michael S. Tsirkin]

On Wed, Jun 28, 2017 at 08:02:06AM -0500, Eric Blake wrote:
> [meta-comment, replacing virtio-dev with virtio-dev-owner in cc]
> 
> On 06/28/2017 07:22 AM, Eric Blake wrote:
> > On 06/28/2017 12:53 AM, Wei Wang wrote:
> >> Use is_power_of_2(), and remove trailing "." from error_setg().
> >>
> >> Signed-off-by: Wei Wang <wei.w.wang@intel.com>
> >> ---
> >>  hw/net/virtio-net.c | 6 +++---
> >>  1 file changed, 3 insertions(+), 3 deletions(-)
> > 
> > Reviewed-by: Eric Blake <eblake@redhat.com>
> > 
> 
> I got this rejection letter:
> 
> > 
> >                    The mail system
> > 
> > <virtio-dev@lists.oasis-open.org>: permission denied. Command output: Sorry,
> >     only subscribers may post. If you are a subscriber, please forward this
> >     message to virtio-dev-owner@lists.oasis-open.org to get your new address
> >     included (#5.7.2) ERROR: postqmail-local Error #77
> 
> It's generally considered poor netiquette to cc both an open list and a
> closed list that rejects emails from non-subscribers (note that you do
> NOT have to subscribe to qemu-devel to post to it, although a moderation
> delay may be involved).  It's not necessarily my place to tell the
> virtio-dev list what to do (since I don't subscribe), but it may be
> worth considereing relaxing the list posting policies there to allow
> moderated posts from non-subscribers, especially if it is going to be
> common-place that people want to cc virtio-dev at the same time as
> another open list.

Unfortunately OASIS policies require you to subscribe to post.  I wish
there was a way to request post rights without actually subscribing. It
doesn't look like OASIS mailing list software supports that.  In this
case I'm guessing you can just ignore this, it should not matter that
this specific email did not make it to virtio-dev.


> -- 
> Eric Blake, Principal Software Engineer
> Red Hat, Inc.           +1-919-301-3266
> Virtualization:  qemu.org | libvirt.org
> 

Re: [Qemu-devel] [PATCH v1] virtio-net: code cleanup

[Michael S. Tsirkin]

On Wed, Jun 28, 2017 at 01:53:20PM +0800, Wei Wang wrote:
> Use is_power_of_2(), and remove trailing "." from error_setg().
> 
> Signed-off-by: Wei Wang <wei.w.wang@intel.com>

Reviewed-by: Michael S. Tsirkin <mst@redhat.com>

> ---
>  hw/net/virtio-net.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
> index 91eddaf..144169c 100644
> --- a/hw/net/virtio-net.c
> +++ b/hw/net/virtio-net.c
> @@ -1918,9 +1918,9 @@ static void virtio_net_device_realize(DeviceState *dev, Error **errp)
>       */
>      if (n->net_conf.rx_queue_size < VIRTIO_NET_RX_QUEUE_MIN_SIZE ||
>          n->net_conf.rx_queue_size > VIRTQUEUE_MAX_SIZE ||
> -        (n->net_conf.rx_queue_size & (n->net_conf.rx_queue_size - 1))) {
> +        !is_power_of_2(n->net_conf.rx_queue_size)) {
>          error_setg(errp, "Invalid rx_queue_size (= %" PRIu16 "), "
> -                   "must be a power of 2 between %d and %d.",
> +                   "must be a power of 2 between %d and %d",
>                     n->net_conf.rx_queue_size, VIRTIO_NET_RX_QUEUE_MIN_SIZE,
>                     VIRTQUEUE_MAX_SIZE);
>          virtio_cleanup(vdev);
> @@ -1930,7 +1930,7 @@ static void virtio_net_device_realize(DeviceState *dev, Error **errp)
>      n->max_queues = MAX(n->nic_conf.peers.queues, 1);
>      if (n->max_queues * 2 + 1 > VIRTIO_QUEUE_MAX) {
>          error_setg(errp, "Invalid number of queues (= %" PRIu32 "), "
> -                   "must be a positive integer less than %d.",
> +                   "must be a positive integer less than %d",
>                     n->max_queues, (VIRTIO_QUEUE_MAX - 1) / 2);
>          virtio_cleanup(vdev);
>          return;
> -- 
> 2.7.4