Fix some random problems

[libvirt] [PATCH 2/2] viriscsi: Request more random bits for interface name

Posted by Michal Privoznik 6 years, 11 months ago

In virStorageBackendCreateIfaceIQN() the virRandomBits() is
called in order to use random bits to generate random name for
new interface. However, virAsprintf() is expecting 32 bits and we
are requesting only 30.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
---
 src/util/viriscsi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/util/viriscsi.c b/src/util/viriscsi.c
index 653b4fd932..f00aeb53a7 100644
--- a/src/util/viriscsi.c
+++ b/src/util/viriscsi.c
@@ -221,7 +221,7 @@ virStorageBackendCreateIfaceIQN(const char *initiatoriqn,
 
     if (virAsprintf(&temp_ifacename,
                     "libvirt-iface-%08llx",
-                    (unsigned long long)virRandomBits(30)) < 0)
+                    (unsigned long long)virRandomBits(32)) < 0)
         return -1;
 
     VIR_DEBUG("Attempting to create interface '%s' with IQN '%s'",
-- 
2.16.4

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 2/2] viriscsi: Request more random bits for interface name

Posted by Eric Blake 6 years, 11 months ago

On 08/01/2018 06:44 AM, Michal Privoznik wrote:
> In virStorageBackendCreateIfaceIQN() the virRandomBits() is
> called in order to use random bits to generate random name for
> new interface. However, virAsprintf() is expecting 32 bits and we
> are requesting only 30.
> 
> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
> ---
>   src/util/viriscsi.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/src/util/viriscsi.c b/src/util/viriscsi.c
> index 653b4fd932..f00aeb53a7 100644
> --- a/src/util/viriscsi.c
> +++ b/src/util/viriscsi.c
> @@ -221,7 +221,7 @@ virStorageBackendCreateIfaceIQN(const char *initiatoriqn,
>   
>       if (virAsprintf(&temp_ifacename,
>                       "libvirt-iface-%08llx",
> -                    (unsigned long long)virRandomBits(30)) < 0)
> +                    (unsigned long long)virRandomBits(32)) < 0)

Are we sure that this wasn't intentional that the 2 most significant 
bits are supposed to be zero (to avoid broadcast interface addresses, ro 
example)?  (If so, then keep things at 30 but add a comment; if not, 
then this change seems fine).

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

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 2/2] viriscsi: Request more random bits for interface name

Posted by Michal Privoznik 6 years, 11 months ago

On 08/01/2018 04:48 PM, Eric Blake wrote:
> On 08/01/2018 06:44 AM, Michal Privoznik wrote:
>> In virStorageBackendCreateIfaceIQN() the virRandomBits() is
>> called in order to use random bits to generate random name for
>> new interface. However, virAsprintf() is expecting 32 bits and we
>> are requesting only 30.
>>
>> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
>> ---
>>   src/util/viriscsi.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/src/util/viriscsi.c b/src/util/viriscsi.c
>> index 653b4fd932..f00aeb53a7 100644
>> --- a/src/util/viriscsi.c
>> +++ b/src/util/viriscsi.c
>> @@ -221,7 +221,7 @@ virStorageBackendCreateIfaceIQN(const char
>> *initiatoriqn,
>>         if (virAsprintf(&temp_ifacename,
>>                       "libvirt-iface-%08llx",
>> -                    (unsigned long long)virRandomBits(30)) < 0)
>> +                    (unsigned long long)virRandomBits(32)) < 0)
> 
> Are we sure that this wasn't intentional that the 2 most significant
> bits are supposed to be zero (to avoid broadcast interface addresses, ro
> example)?  

Yes we are. The random bits are used only for generating a name, not an
address. The iSCSI interfaces we use here are in fact just a separate
iSCSI connection, not real NICs. It's just bad naming on iSCSI side, but
we are already used to that (initiator/target vs client/server), aren't we?

Michal

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 2/2] viriscsi: Request more random bits for interface name

Posted by Daniel P. Berrangé 6 years, 11 months ago

On Wed, Aug 01, 2018 at 01:44:33PM +0200, Michal Privoznik wrote:
> In virStorageBackendCreateIfaceIQN() the virRandomBits() is
> called in order to use random bits to generate random name for
> new interface. However, virAsprintf() is expecting 32 bits and we
> are requesting only 30.
> 
> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
> ---
>  src/util/viriscsi.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/src/util/viriscsi.c b/src/util/viriscsi.c
> index 653b4fd932..f00aeb53a7 100644
> --- a/src/util/viriscsi.c
> +++ b/src/util/viriscsi.c
> @@ -221,7 +221,7 @@ virStorageBackendCreateIfaceIQN(const char *initiatoriqn,
>  
>      if (virAsprintf(&temp_ifacename,
>                      "libvirt-iface-%08llx",
> -                    (unsigned long long)virRandomBits(30)) < 0)
> +                    (unsigned long long)virRandomBits(32)) < 0)
>          return -1;
>  
>      VIR_DEBUG("Attempting to create interface '%s' with IQN '%s'",

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list