[libvirt] [tck PATCH v2 2/4] new NetworkHelper function get_network_ip()

Laine Stump posted 4 patches 7 years, 2 months ago
[libvirt] [tck PATCH v2 2/4] new NetworkHelper function get_network_ip()
Posted by Laine Stump 7 years, 2 months ago
This function gets the first IP address for the named virtual
network. It is returned as a Net::IP object, so that we will have info
about its netmask/prefix and can easily get it broadcast address and
perform arithmetic on the address.

Signed-off-by: Laine Stump <laine@laine.org>
---

Change from V1: return a NetAddr::IP object instead of a string.

 lib/Sys/Virt/TCK/NetworkHelpers.pm | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/lib/Sys/Virt/TCK/NetworkHelpers.pm b/lib/Sys/Virt/TCK/NetworkHelpers.pm
index 5f563e5..7bbce62 100644
--- a/lib/Sys/Virt/TCK/NetworkHelpers.pm
+++ b/lib/Sys/Virt/TCK/NetworkHelpers.pm
@@ -1,4 +1,5 @@
 use Sys::Virt::TCK qw(xpath);
+use NetAddr::IP qw(:lower);
 use strict;
 use utf8;
 
@@ -9,6 +10,27 @@ sub get_first_macaddress {
     return $mac;
 }
 
+sub get_network_ip {
+    my $conn = shift;
+    my $netname = shift;
+    diag "getting ip for network $netname";
+    my $net = $conn->get_network_by_name($netname);
+    my $net_ip = xpath($net, "string(/network/ip[1]/\@address");
+    my $net_mask = xpath($net, "string(/network/ip[1]/\@netmask");
+    my $net_prefix = xpath($net, "string(/network/ip[1]/\@prefix");
+    my $ip;
+
+    if ($net_mask) {
+        $ip = NetAddr::IP->new($net_ip, $net_mask);
+    } elsif ($net_prefix) {
+        $ip = NetAddr::IP->new("$net_ip/$net_mask");
+    } else {
+        $ip = NetAddr::IP->new("$net_ip");
+    }
+    return $ip;
+}
+
+
 sub get_ip_from_leases{
     my $conn = shift;
     my $netname = shift;
-- 
2.14.3

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [tck PATCH v2 2/4] new NetworkHelper function get_network_ip()
Posted by Daniel P. Berrangé 7 years, 2 months ago
On Thu, Mar 01, 2018 at 09:49:58PM -0500, Laine Stump wrote:
> This function gets the first IP address for the named virtual
> network. It is returned as a Net::IP object, so that we will have info
> about its netmask/prefix and can easily get it broadcast address and
> perform arithmetic on the address.
> 
> Signed-off-by: Laine Stump <laine@laine.org>
> ---
> 
> Change from V1: return a NetAddr::IP object instead of a string.
> 
>  lib/Sys/Virt/TCK/NetworkHelpers.pm | 22 ++++++++++++++++++++++
>  1 file changed, 22 insertions(+)
> 
> diff --git a/lib/Sys/Virt/TCK/NetworkHelpers.pm b/lib/Sys/Virt/TCK/NetworkHelpers.pm
> index 5f563e5..7bbce62 100644
> --- a/lib/Sys/Virt/TCK/NetworkHelpers.pm
> +++ b/lib/Sys/Virt/TCK/NetworkHelpers.pm
> @@ -1,4 +1,5 @@
>  use Sys::Virt::TCK qw(xpath);
> +use NetAddr::IP qw(:lower);

This isn't part of base perl, so you'll need to list it in Build.PL and
the RPM spec file.

>  use strict;
>  use utf8;
>  
> @@ -9,6 +10,27 @@ sub get_first_macaddress {
>      return $mac;
>  }
>  
> +sub get_network_ip {
> +    my $conn = shift;
> +    my $netname = shift;
> +    diag "getting ip for network $netname";
> +    my $net = $conn->get_network_by_name($netname);
> +    my $net_ip = xpath($net, "string(/network/ip[1]/\@address");
> +    my $net_mask = xpath($net, "string(/network/ip[1]/\@netmask");
> +    my $net_prefix = xpath($net, "string(/network/ip[1]/\@prefix");
> +    my $ip;
> +
> +    if ($net_mask) {
> +        $ip = NetAddr::IP->new($net_ip, $net_mask);
> +    } elsif ($net_prefix) {
> +        $ip = NetAddr::IP->new("$net_ip/$net_mask");
> +    } else {
> +        $ip = NetAddr::IP->new("$net_ip");
> +    }
> +    return $ip;
> +}
> +
> +
>  sub get_ip_from_leases{
>      my $conn = shift;
>      my $netname = shift;
> -- 
> 2.14.3
> 
> --
> libvir-list mailing list
> libvir-list@redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list

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
Re: [libvirt] [tck PATCH v2 2/4] new NetworkHelper function get_network_ip()
Posted by Laine Stump 7 years, 2 months ago
On 03/05/2018 04:31 AM, Daniel P. Berrangé wrote:
> On Thu, Mar 01, 2018 at 09:49:58PM -0500, Laine Stump wrote:
>> This function gets the first IP address for the named virtual
>> network. It is returned as a Net::IP object, so that we will have info
>> about its netmask/prefix and can easily get it broadcast address and
>> perform arithmetic on the address.
>>
>> Signed-off-by: Laine Stump <laine@laine.org>
>> ---
>>
>> Change from V1: return a NetAddr::IP object instead of a string.
>>
>>  lib/Sys/Virt/TCK/NetworkHelpers.pm | 22 ++++++++++++++++++++++
>>  1 file changed, 22 insertions(+)
>>
>> diff --git a/lib/Sys/Virt/TCK/NetworkHelpers.pm b/lib/Sys/Virt/TCK/NetworkHelpers.pm
>> index 5f563e5..7bbce62 100644
>> --- a/lib/Sys/Virt/TCK/NetworkHelpers.pm
>> +++ b/lib/Sys/Virt/TCK/NetworkHelpers.pm
>> @@ -1,4 +1,5 @@
>>  use Sys::Virt::TCK qw(xpath);
>> +use NetAddr::IP qw(:lower);
> This isn't part of base perl, so you'll need to list it in Build.PL and
> the RPM spec file.

I originally assumed that, but remembered seeing "something somewhere"
about implicit dependencies and decided to try it out by not listing it
in the specfile - on both Fedora and RHEL7 the dependency was properly
pulled in and it was installed.

This leads to one of three possibilities:

1) implicit dependencies are figured out properly by yum and dnf (at
least for RHEL7, don't know about RHEL6).

2) (1), but it's just coincidentally happening and not guaranteed.

3) I wasn't paying attention when I tested, and what I say isn't
actually true.


I don't have any problem putting in the explicit Requires though. Can I
assumed a Reviewed-by with that in place?

>
>>  use strict;
>>  use utf8;
>>  
>> @@ -9,6 +10,27 @@ sub get_first_macaddress {
>>      return $mac;
>>  }
>>  
>> +sub get_network_ip {
>> +    my $conn = shift;
>> +    my $netname = shift;
>> +    diag "getting ip for network $netname";
>> +    my $net = $conn->get_network_by_name($netname);
>> +    my $net_ip = xpath($net, "string(/network/ip[1]/\@address");
>> +    my $net_mask = xpath($net, "string(/network/ip[1]/\@netmask");
>> +    my $net_prefix = xpath($net, "string(/network/ip[1]/\@prefix");
>> +    my $ip;
>> +
>> +    if ($net_mask) {
>> +        $ip = NetAddr::IP->new($net_ip, $net_mask);
>> +    } elsif ($net_prefix) {
>> +        $ip = NetAddr::IP->new("$net_ip/$net_mask");
>> +    } else {
>> +        $ip = NetAddr::IP->new("$net_ip");
>> +    }
>> +    return $ip;
>> +}
>> +
>> +
>>  sub get_ip_from_leases{
>>      my $conn = shift;
>>      my $netname = shift;
>> -- 
>> 2.14.3
>>
>> --
>> libvir-list mailing list
>> libvir-list@redhat.com
>> https://www.redhat.com/mailman/listinfo/libvir-list
> Regards,
> Daniel


--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [tck PATCH v2 2/4] new NetworkHelper function get_network_ip()
Posted by Daniel P. Berrangé 7 years, 2 months ago
On Mon, Mar 05, 2018 at 10:10:36AM -0500, Laine Stump wrote:
> On 03/05/2018 04:31 AM, Daniel P. Berrangé wrote:
> > On Thu, Mar 01, 2018 at 09:49:58PM -0500, Laine Stump wrote:
> >> This function gets the first IP address for the named virtual
> >> network. It is returned as a Net::IP object, so that we will have info
> >> about its netmask/prefix and can easily get it broadcast address and
> >> perform arithmetic on the address.
> >>
> >> Signed-off-by: Laine Stump <laine@laine.org>
> >> ---
> >>
> >> Change from V1: return a NetAddr::IP object instead of a string.
> >>
> >>  lib/Sys/Virt/TCK/NetworkHelpers.pm | 22 ++++++++++++++++++++++
> >>  1 file changed, 22 insertions(+)
> >>
> >> diff --git a/lib/Sys/Virt/TCK/NetworkHelpers.pm b/lib/Sys/Virt/TCK/NetworkHelpers.pm
> >> index 5f563e5..7bbce62 100644
> >> --- a/lib/Sys/Virt/TCK/NetworkHelpers.pm
> >> +++ b/lib/Sys/Virt/TCK/NetworkHelpers.pm
> >> @@ -1,4 +1,5 @@
> >>  use Sys::Virt::TCK qw(xpath);
> >> +use NetAddr::IP qw(:lower);
> > This isn't part of base perl, so you'll need to list it in Build.PL and
> > the RPM spec file.
> 
> I originally assumed that, but remembered seeing "something somewhere"
> about implicit dependencies and decided to try it out by not listing it
> in the specfile - on both Fedora and RHEL7 the dependency was properly
> pulled in and it was installed.
> 
> This leads to one of three possibilities:
> 
> 1) implicit dependencies are figured out properly by yum and dnf (at
> least for RHEL7, don't know about RHEL6).
> 
> 2) (1), but it's just coincidentally happening and not guaranteed.
> 
> 3) I wasn't paying attention when I tested, and what I say isn't
> actually true.
>
> I don't have any problem putting in the explicit Requires though. Can I
> assumed a Reviewed-by with that in place?

We don't need to list it with a Requires:  tag because automatic
dependancies take care of that. We need it listed as BuildRequires
though, *if* the NetworkHelpers mod is pulled in by any of the unit
tests which I thought it was (but I could be wrong there).

Still need it in the Build.PL no matter what, as that's what CPAN
and other Perl tools use.

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
Re: [libvirt] [tck PATCH v2 2/4] new NetworkHelper function get_network_ip()
Posted by Laine Stump 7 years, 2 months ago
On 03/05/2018 10:15 AM, Daniel P. Berrangé wrote:
> On Mon, Mar 05, 2018 at 10:10:36AM -0500, Laine Stump wrote:
>> On 03/05/2018 04:31 AM, Daniel P. Berrangé wrote:
>>> On Thu, Mar 01, 2018 at 09:49:58PM -0500, Laine Stump wrote:
>>>> This function gets the first IP address for the named virtual
>>>> network. It is returned as a Net::IP object, so that we will have info
>>>> about its netmask/prefix and can easily get it broadcast address and
>>>> perform arithmetic on the address.
>>>>
>>>> Signed-off-by: Laine Stump <laine@laine.org>
>>>> ---
>>>>
>>>> Change from V1: return a NetAddr::IP object instead of a string.
>>>>
>>>>  lib/Sys/Virt/TCK/NetworkHelpers.pm | 22 ++++++++++++++++++++++
>>>>  1 file changed, 22 insertions(+)
>>>>
>>>> diff --git a/lib/Sys/Virt/TCK/NetworkHelpers.pm b/lib/Sys/Virt/TCK/NetworkHelpers.pm
>>>> index 5f563e5..7bbce62 100644
>>>> --- a/lib/Sys/Virt/TCK/NetworkHelpers.pm
>>>> +++ b/lib/Sys/Virt/TCK/NetworkHelpers.pm
>>>> @@ -1,4 +1,5 @@
>>>>  use Sys::Virt::TCK qw(xpath);
>>>> +use NetAddr::IP qw(:lower);
>>> This isn't part of base perl, so you'll need to list it in Build.PL and
>>> the RPM spec file.
>> I originally assumed that, but remembered seeing "something somewhere"
>> about implicit dependencies and decided to try it out by not listing it
>> in the specfile - on both Fedora and RHEL7 the dependency was properly
>> pulled in and it was installed.
>>
>> This leads to one of three possibilities:
>>
>> 1) implicit dependencies are figured out properly by yum and dnf (at
>> least for RHEL7, don't know about RHEL6).
>>
>> 2) (1), but it's just coincidentally happening and not guaranteed.
>>
>> 3) I wasn't paying attention when I tested, and what I say isn't
>> actually true.
>>
>> I don't have any problem putting in the explicit Requires though. Can I
>> assumed a Reviewed-by with that in place?
> We don't need to list it with a Requires:  tag because automatic
> dependancies take care of that. We need it listed as BuildRequires
> though, *if* the NetworkHelpers mod is pulled in by any of the unit
> tests which I thought it was (but I could be wrong there).

Well, being the perl-idiot that I am, I didn't realize that the unit
tests even *existed* until you said that and I investigated to see what
you were talking about :-)

So what you're saying is that if any function in NetworkHelpers is
either directly or indirectly used in the unit tests (which are in the
"t" subdirectory of the source tree, right?) then we need to list is in
BuildRequires, correct? I looked in the unit tests  and while
NetworkBuilders is pulled it, NetworkHelpers isn't (and NetworkBuilders
doesn't pull in NetworkHelpers), so I *think* we're safe, but I'll try
building on a system where I've removed the NetAddr::IP package to
verify that it's still successful..


>
> Still need it in the Build.PL no matter what, as that's what CPAN
> and other Perl tools use.

Ah, for non-rpm-based installations, right?

Okay, I'll add that and resend.

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [tck PATCH v2 2/4] new NetworkHelper function get_network_ip()
Posted by Daniel P. Berrangé 7 years, 2 months ago
On Mon, Mar 05, 2018 at 03:32:02PM -0500, Laine Stump wrote:
> On 03/05/2018 10:15 AM, Daniel P. Berrangé wrote:
> > On Mon, Mar 05, 2018 at 10:10:36AM -0500, Laine Stump wrote:
> >> On 03/05/2018 04:31 AM, Daniel P. Berrangé wrote:
> >>> On Thu, Mar 01, 2018 at 09:49:58PM -0500, Laine Stump wrote:
> >>>> This function gets the first IP address for the named virtual
> >>>> network. It is returned as a Net::IP object, so that we will have info
> >>>> about its netmask/prefix and can easily get it broadcast address and
> >>>> perform arithmetic on the address.
> >>>>
> >>>> Signed-off-by: Laine Stump <laine@laine.org>
> >>>> ---
> >>>>
> >>>> Change from V1: return a NetAddr::IP object instead of a string.
> >>>>
> >>>>  lib/Sys/Virt/TCK/NetworkHelpers.pm | 22 ++++++++++++++++++++++
> >>>>  1 file changed, 22 insertions(+)
> >>>>
> >>>> diff --git a/lib/Sys/Virt/TCK/NetworkHelpers.pm b/lib/Sys/Virt/TCK/NetworkHelpers.pm
> >>>> index 5f563e5..7bbce62 100644
> >>>> --- a/lib/Sys/Virt/TCK/NetworkHelpers.pm
> >>>> +++ b/lib/Sys/Virt/TCK/NetworkHelpers.pm
> >>>> @@ -1,4 +1,5 @@
> >>>>  use Sys::Virt::TCK qw(xpath);
> >>>> +use NetAddr::IP qw(:lower);
> >>> This isn't part of base perl, so you'll need to list it in Build.PL and
> >>> the RPM spec file.
> >> I originally assumed that, but remembered seeing "something somewhere"
> >> about implicit dependencies and decided to try it out by not listing it
> >> in the specfile - on both Fedora and RHEL7 the dependency was properly
> >> pulled in and it was installed.
> >>
> >> This leads to one of three possibilities:
> >>
> >> 1) implicit dependencies are figured out properly by yum and dnf (at
> >> least for RHEL7, don't know about RHEL6).
> >>
> >> 2) (1), but it's just coincidentally happening and not guaranteed.
> >>
> >> 3) I wasn't paying attention when I tested, and what I say isn't
> >> actually true.
> >>
> >> I don't have any problem putting in the explicit Requires though. Can I
> >> assumed a Reviewed-by with that in place?
> > We don't need to list it with a Requires:  tag because automatic
> > dependancies take care of that. We need it listed as BuildRequires
> > though, *if* the NetworkHelpers mod is pulled in by any of the unit
> > tests which I thought it was (but I could be wrong there).
> 
> Well, being the perl-idiot that I am, I didn't realize that the unit
> tests even *existed* until you said that and I investigated to see what
> you were talking about :-)
> 
> So what you're saying is that if any function in NetworkHelpers is
> either directly or indirectly used in the unit tests (which are in the
> "t" subdirectory of the source tree, right?) then we need to list is in
> BuildRequires, correct? I looked in the unit tests  and while
> NetworkBuilders is pulled it, NetworkHelpers isn't (and NetworkBuilders
> doesn't pull in NetworkHelpers), so I *think* we're safe, but I'll try
> building on a system where I've removed the NetAddr::IP package to
> verify that it's still successful..

Yep, that sounds fine then. 

> > Still need it in the Build.PL no matter what, as that's what CPAN
> > and other Perl tools use.
> 
> Ah, for non-rpm-based installations, right?

Yes, exactly - the cpan CLI uses the info there to figure out what
dependancies need installing.

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