From: Lin Ma <lma@suse.de>

It doesn't make sense querying dhcp leases for interfaces against an inactive
network, This patch adds a check to see if the network is active.

Signed-off-by: Lin Ma <lma@suse.de>
---
 src/network/bridge_driver.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index 87d7acab06..1dffc2309f 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -4068,6 +4068,13 @@ networkGetDHCPLeases(virNetworkPtr net,
     if (virNetworkGetDHCPLeasesEnsureACL(net->conn, def) < 0)
         goto cleanup;
 
+    if (!virNetworkObjIsActive(obj)) {
+        virReportError(VIR_ERR_OPERATION_INVALID,
+                       _("network '%s' is not active"),
+                       def->name);
+        goto error;
+    }
+
     /* Retrieve custom leases file location */
     custom_lease_file = networkDnsmasqLeaseFileNameCustom(driver, def->bridge);
 
-- 
2.26.0

On 9/16/20 9:17 AM, morecache@gmail.com wrote:
> From: Lin Ma <lma@suse.de>
> 
> It doesn't make sense querying dhcp leases for interfaces against an inactive
> network, This patch adds a check to see if the network is active.
> 
> Signed-off-by: Lin Ma <lma@suse.de>
> ---
>   src/network/bridge_driver.c | 7 +++++++
>   1 file changed, 7 insertions(+)
> 
> diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
> index 87d7acab06..1dffc2309f 100644
> --- a/src/network/bridge_driver.c
> +++ b/src/network/bridge_driver.c
> @@ -4068,6 +4068,13 @@ networkGetDHCPLeases(virNetworkPtr net,
>       if (virNetworkGetDHCPLeasesEnsureACL(net->conn, def) < 0)
>           goto cleanup;
>   
> +    if (!virNetworkObjIsActive(obj)) {
> +        virReportError(VIR_ERR_OPERATION_INVALID,
> +                       _("network '%s' is not active"),
> +                       def->name);
> +        goto error;

No need to jump all the way to 'error' when 'cleanup' is just fine. 
@leases_ret wasn't touched until now and hence is still NULL, this 
'error' is the same as 'cleanup'. But you can keep it, if you want, 
we're jumping "randomly" on error and cleanup.

Michal

On a Wednesday in 2020, Michal Privoznik wrote:
>On 9/16/20 9:17 AM, morecache@gmail.com wrote:
>>From: Lin Ma <lma@suse.de>
>>
>>It doesn't make sense querying dhcp leases for interfaces against an inactive
>>network, This patch adds a check to see if the network is active.
>>

NACK, see discussion on v1.

https://www.redhat.com/archives/libvir-list/2020-September/msg00673.html

Jano

>>Signed-off-by: Lin Ma <lma@suse.de>
>>---
>>  src/network/bridge_driver.c | 7 +++++++
>>  1 file changed, 7 insertions(+)
>>
>>diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
>>index 87d7acab06..1dffc2309f 100644
>>--- a/src/network/bridge_driver.c
>>+++ b/src/network/bridge_driver.c
>>@@ -4068,6 +4068,13 @@ networkGetDHCPLeases(virNetworkPtr net,
>>      if (virNetworkGetDHCPLeasesEnsureACL(net->conn, def) < 0)
>>          goto cleanup;
>>+    if (!virNetworkObjIsActive(obj)) {
>>+        virReportError(VIR_ERR_OPERATION_INVALID,
>>+                       _("network '%s' is not active"),
>>+                       def->name);
>>+        goto error;
>
>No need to jump all the way to 'error' when 'cleanup' is just fine. 
>@leases_ret wasn't touched until now and hence is still NULL, this 
>'error' is the same as 'cleanup'. But you can keep it, if you want, 
>we're jumping "randomly" on error and cleanup.
>
>Michal
>

On 9/16/20 2:18 PM, Ján Tomko wrote:
> On a Wednesday in 2020, Michal Privoznik wrote:
>> On 9/16/20 9:17 AM, morecache@gmail.com wrote:
>>> From: Lin Ma <lma@suse.de>
>>>
>>> It doesn't make sense querying dhcp leases for interfaces against an 
>>> inactive
>>> network, This patch adds a check to see if the network is active.
>>>
> 
> NACK, see discussion on v1.
> 
> https://www.redhat.com/archives/libvir-list/2020-September/msg00673.html
> 

So what if I set up a static lease in network XM, start the network, 
then shut it down and change the XML? Wouldn't I get a different lease 
until the network is started again?

The NSS plugin suffers from the same problem, btw. But it can't know 
whether the network is still running or not (if we don't want it to do 
some hacks).

Michal