This was initially inspired by https://issues.redhat.com/browse/RHEL-50968 which
does things behind our back.  However, I have found some other things when
digging into the aforemention bug.

I rebased, changed, rebased, refactored, and rebased again this branch so many
times there might be a bunch of weird stuff I forgot to remove before posting.
I hope I did not miss any, but one can never be sure ;)

Martin Kletzander (8):
  network: Do not update network ports for inactive networks
  network: Do not call virNetworkObjUnsetDefTransient on start cleanup
  network: Move port deletion into the shutdown function
  network: Don't check if network is active in networkShutdownNetwork
  network: Clean up after inactive objects during start
  network: Try to read dnsmasq PIDs for inactive networks too
  network: Separate cleanup from networkRemoveInactive
  network: Clean up after disappeared transient inactive networks

 src/network/bridge_driver.c | 62 ++++++++++++++++++++++++++++++-------
 1 file changed, 50 insertions(+), 12 deletions(-)

-- 
2.46.0

On 9/3/24 10:36 AM, Martin Kletzander wrote:
> This was initially inspired by https://issues.redhat.com/browse/RHEL-50968 which
> does things behind our back.  However, I have found some other things when
> digging into the aforemention bug.
> 
> I rebased, changed, rebased, refactored, and rebased again this branch so many
> times there might be a bunch of weird stuff I forgot to remove before posting.
> I hope I did not miss any, but one can never be sure ;)

Everything looks fine to me (I've sent Reviewed-by: for most of the 
patches). I had responded to one of the patches (that fortunately has 
become lost - I don't see it anywhere on the list archives or in my 
email client) commenting about going through all the cleanup even for 
networks that were already legitimately and completely down, but now 
that I've applied all the patches I see that isn't what happens anyway! 
(I have no idea what I was thinking)

Anyway

Reviewed-by: Laine Stump <laine@redhat.com>

for the entire series, and thanks for fixing a problem that has been 
popping up once every year or two for as long as I can remember :-)


> 
> Martin Kletzander (8):
>    network: Do not update network ports for inactive networks
>    network: Do not call virNetworkObjUnsetDefTransient on start cleanup
>    network: Move port deletion into the shutdown function
>    network: Don't check if network is active in networkShutdownNetwork
>    network: Clean up after inactive objects during start
>    network: Try to read dnsmasq PIDs for inactive networks too
>    network: Separate cleanup from networkRemoveInactive
>    network: Clean up after disappeared transient inactive networks
> 
>   src/network/bridge_driver.c | 62 ++++++++++++++++++++++++++++++-------
>   1 file changed, 50 insertions(+), 12 deletions(-)
>

On Mon, Sep 16, 2024 at 08:50:31PM -0400, Laine Stump wrote:
>On 9/3/24 10:36 AM, Martin Kletzander wrote:
>> This was initially inspired by https://issues.redhat.com/browse/RHEL-50968 which
>> does things behind our back.  However, I have found some other things when
>> digging into the aforemention bug.
>>
>> I rebased, changed, rebased, refactored, and rebased again this branch so many
>> times there might be a bunch of weird stuff I forgot to remove before posting.
>> I hope I did not miss any, but one can never be sure ;)
>
>Everything looks fine to me (I've sent Reviewed-by: for most of the
>patches). I had responded to one of the patches (that fortunately has
>become lost - I don't see it anywhere on the list archives or in my
>email client) commenting about going through all the cleanup even for
>networks that were already legitimately and completely down, but now
>that I've applied all the patches I see that isn't what happens anyway!
>(I have no idea what I was thinking)
>

I spent non-trivial amount of time trying to run this only on the
networks for which we loaded the state file and not the config one, but
kept running into more and more issues.  I don't even remember all of
them, but I think one of them was that during the clean-up we do not
know whether it is a transient network or not and that changes the
clean-up paths as well (e.g. removing the leases file etc.), so I went
with the "clean everything" approach since it was partially
pre-existing.

>Anyway
>
>Reviewed-by: Laine Stump <laine@redhat.com>
>
>for the entire series, and thanks for fixing a problem that has been
>popping up once every year or two for as long as I can remember :-)
>

Really?  Well, now I'm glad I spent so much time on that =D

Thanks for the review and sorry for pushing you to do that ;)

>
>>
>> Martin Kletzander (8):
>>    network: Do not update network ports for inactive networks
>>    network: Do not call virNetworkObjUnsetDefTransient on start cleanup
>>    network: Move port deletion into the shutdown function
>>    network: Don't check if network is active in networkShutdownNetwork
>>    network: Clean up after inactive objects during start
>>    network: Try to read dnsmasq PIDs for inactive networks too
>>    network: Separate cleanup from networkRemoveInactive
>>    network: Clean up after disappeared transient inactive networks
>>
>>   src/network/bridge_driver.c | 62 ++++++++++++++++++++++++++++++-------
>>   1 file changed, 50 insertions(+), 12 deletions(-)
>>
>

Polite ping

On Tue, Sep 03, 2024 at 04:36:19PM +0200, Martin Kletzander wrote:
>This was initially inspired by https://issues.redhat.com/browse/RHEL-50968 which
>does things behind our back.  However, I have found some other things when
>digging into the aforemention bug.
>
>I rebased, changed, rebased, refactored, and rebased again this branch so many
>times there might be a bunch of weird stuff I forgot to remove before posting.
>I hope I did not miss any, but one can never be sure ;)
>
>Martin Kletzander (8):
>  network: Do not update network ports for inactive networks
>  network: Do not call virNetworkObjUnsetDefTransient on start cleanup
>  network: Move port deletion into the shutdown function
>  network: Don't check if network is active in networkShutdownNetwork
>  network: Clean up after inactive objects during start
>  network: Try to read dnsmasq PIDs for inactive networks too
>  network: Separate cleanup from networkRemoveInactive
>  network: Clean up after disappeared transient inactive networks
>
> src/network/bridge_driver.c | 62 ++++++++++++++++++++++++++++++-------
> 1 file changed, 50 insertions(+), 12 deletions(-)
>
>-- 
>2.46.0
>