This macro checks whether given number is an integer power of
two.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
---
 src/internal.h           | 10 +++++++++
 src/qemu/qemu_validate.c | 46 +++++++++++++++++++++-------------------
 src/util/virrandom.c     |  2 +-
 3 files changed, 35 insertions(+), 23 deletions(-)

diff --git a/src/internal.h b/src/internal.h
index ff94e7e53e..0a03dfc46f 100644
--- a/src/internal.h
+++ b/src/internal.h
@@ -237,6 +237,16 @@
         (a) = (a) ^ (b); \
     } while (0)
 
+
+/**
+ * VIR_IS_POW2:
+ *
+ * Returns true if given number is a power of two
+ */
+#define VIR_IS_POW2(x) \
+    ((x) && !((x) & ((x) - 1)))
+
+
 /**
  * virCheckFlags:
  * @supported: an OR'ed set of supported flags
diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c
index e60d39a22f..6f4662b25a 100644
--- a/src/qemu/qemu_validate.c
+++ b/src/qemu/qemu_validate.c
@@ -1460,20 +1460,32 @@ qemuValidateDomainDeviceDefNetwork(const virDomainNetDef *net,
             return -1;
         }
 
-        if (net->driver.virtio.rx_queue_size &&
-            !virQEMUCapsGet(qemuCaps, QEMU_CAPS_VIRTIO_NET_RX_QUEUE_SIZE)) {
-            virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
-                           _("virtio rx_queue_size option is not supported "
-                             "with this QEMU binary"));
-            return -1;
+        if (net->driver.virtio.rx_queue_size) {
+            if (!VIR_IS_POW2(net->driver.virtio.rx_queue_size)) {
+                virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+                               _("rx_queue_size has to be a power of two"));
+                return -1;
+            }
+            if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_VIRTIO_NET_RX_QUEUE_SIZE)) {
+                virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+                               _("virtio rx_queue_size option is not supported "
+                                 "with this QEMU binary"));
+                return -1;
+            }
         }
 
-        if (net->driver.virtio.tx_queue_size &&
-            !virQEMUCapsGet(qemuCaps, QEMU_CAPS_VIRTIO_NET_TX_QUEUE_SIZE)) {
-            virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
-                           _("virtio tx_queue_size option is not supported "
-                             "with this QEMU binary"));
-            return -1;
+        if (net->driver.virtio.tx_queue_size) {
+            if (!VIR_IS_POW2(net->driver.virtio.tx_queue_size)) {
+                virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+                               _("tx_queue_size has to be a power of two"));
+                return -1;
+            }
+            if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_VIRTIO_NET_TX_QUEUE_SIZE)) {
+                virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+                               _("virtio tx_queue_size option is not supported "
+                                 "with this QEMU binary"));
+                return -1;
+            }
         }
 
         if (net->mtu &&
@@ -1484,16 +1496,6 @@ qemuValidateDomainDeviceDefNetwork(const virDomainNetDef *net,
             return -1;
         }
 
-        if (net->driver.virtio.rx_queue_size & (net->driver.virtio.rx_queue_size - 1)) {
-            virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
-                           _("rx_queue_size has to be a power of two"));
-            return -1;
-        }
-        if (net->driver.virtio.tx_queue_size & (net->driver.virtio.tx_queue_size - 1)) {
-            virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
-                           _("tx_queue_size has to be a power of two"));
-            return -1;
-        }
         if (qemuValidateDomainVirtioOptions(net->virtio, qemuCaps) < 0)
             return -1;
     }
diff --git a/src/util/virrandom.c b/src/util/virrandom.c
index 6417232e3a..3ae1297e6b 100644
--- a/src/util/virrandom.c
+++ b/src/util/virrandom.c
@@ -89,7 +89,7 @@ double virRandom(void)
  */
 uint32_t virRandomInt(uint32_t max)
 {
-    if ((max & (max - 1)) == 0)
+    if (VIR_IS_POW2(max))
         return virRandomBits(__builtin_ffs(max) - 1);
 
     return virRandom() * max;
-- 
2.26.2

On Thu, Dec 03, 2020 at 13:36:05 +0100, Michal Privoznik wrote:
> This macro checks whether given number is an integer power of
> two.

Mention that you are also refactoring code to use it.

> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
> ---
>  src/internal.h           | 10 +++++++++
>  src/qemu/qemu_validate.c | 46 +++++++++++++++++++++-------------------
>  src/util/virrandom.c     |  2 +-
>  3 files changed, 35 insertions(+), 23 deletions(-)

Reviewed-by: Peter Krempa <pkrempa@redhat.com>

On a Friday in 2020, Peter Krempa wrote:
>On Thu, Dec 03, 2020 at 13:36:05 +0100, Michal Privoznik wrote:
>> This macro checks whether given number is an integer power of
>> two.
>
>Mention that you are also refactoring code to use it.
>

IIUC those are new checks - not a refactor.

Jano

>> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
>> ---
>>  src/internal.h           | 10 +++++++++
>>  src/qemu/qemu_validate.c | 46 +++++++++++++++++++++-------------------
>>  src/util/virrandom.c     |  2 +-
>>  3 files changed, 35 insertions(+), 23 deletions(-)
>
>Reviewed-by: Peter Krempa <pkrempa@redhat.com>
>

On Mon, Dec 07, 2020 at 09:55:31 +0100, Ján Tomko wrote:
> On a Friday in 2020, Peter Krempa wrote:
> > On Thu, Dec 03, 2020 at 13:36:05 +0100, Michal Privoznik wrote:
> > > This macro checks whether given number is an integer power of
> > > two.
> > 
> > Mention that you are also refactoring code to use it.
> > 
> 
> IIUC those are new checks - not a refactor.

It is in fact a bit more involved refactor. The checks are removed in a
different hunk.

In fact this question shows that the patch split is wrong.

On 12/7/20 9:55 AM, Ján Tomko wrote:
> On a Friday in 2020, Peter Krempa wrote:
>> On Thu, Dec 03, 2020 at 13:36:05 +0100, Michal Privoznik wrote:
>>> This macro checks whether given number is an integer power of
>>> two.
>>
>> Mention that you are also refactoring code to use it.
>>
> 
> IIUC those are new checks - not a refactor.

What is new? These checks for pow2 were introduced in v2.3.0-rc1~186 and 
v3.7.0-rc1~236. I'm not sure such old checks can be viewed as new.

Michal

On a Monday in 2020, Michal Privoznik wrote:
>On 12/7/20 9:55 AM, Ján Tomko wrote:
>>On a Friday in 2020, Peter Krempa wrote:
>>>On Thu, Dec 03, 2020 at 13:36:05 +0100, Michal Privoznik wrote:
>>>>This macro checks whether given number is an integer power of
>>>>two.
>>>
>>>Mention that you are also refactoring code to use it.
>>>
>>
>>IIUC those are new checks - not a refactor.
>
>What is new? These checks for pow2 were introduced in v2.3.0-rc1~186 
>and v3.7.0-rc1~236. I'm not sure such old checks can be viewed as new.
>

Right, guess I shouldn't look at code so early in the morning.

Jano

>Michal
>