Change in vdsm[master]: netinfo: rework reporting of DHCPv4/6 on network devices

osvoboda at redhat.com osvoboda at redhat.com
Sun Oct 18 10:58:39 UTC 2015


Ondřej Svoboda has posted comments on this change.

Change subject: netinfo: rework reporting of DHCPv4/6 on network devices
......................................................................


Patch Set 9:

(1 comment)

https://gerrit.ovirt.org/#/c/46430/9/lib/vdsm/netinfo.py
File lib/vdsm/netinfo.py:

Line 582: 
Line 583:     return updated_networking
Line 584: 
Line 585: 
Line 586: def _update_nested_dict(original, replacement):
> I think this should be implemented a s a straight forward update of a dicti
Basically, we're dealing here with 4 subdictionaries (bondings={}, bridges={}, nics={}, vlans={}) of netinfo, and we may be updating any of it, depending on the nature of networks defined: some can be based on nics, others on bondings, others on VLANs, and the rest (should be the biggest part) are bridged. There can be a lot of networks based on any combination and amount of these devices.

This means that there would be a general function to update the given subdictionary, one of (bondings={}, bridges={}, nics={}, vlans={}), because all of these devices have the dhcpv4/6 and cfg subdictionaries.

This function would iterate over the individual devices and replace the dhcpv4/6 strings. Something like devices[device]['dhcpv4'] = replacement_devices[device]['dhcpv4']. It wouldn't be hard to replace the one field we know about, in cfg dictionary, but the line would be even longer.

Remember that a caller of this function (I think it would have to be netinfo.get) would have to invoke it four times for each device type, and we get this, and the inner iteration, for free if we use _update_nested_dict.

The solution I describe is specific to the topology of netinfo and (inherently?) a bit more complex... Both the current solution and this proposed one are okay to me, but I like the current one because I think it is just clear that _propose_updates_to_reported_dhcp knows how netinfo looks like (and this way, this knowledge is in one place only), and I trust _update_nested_dict to update netinfo, however its input changes (we will remove cfg one day, we may add something else). Imagine we would like to use _update_nested_dict in a different place...

If you still want to go for the specific function, I'll try to prepare it.
Line 587:     """
Line 588:     Use a replacement dict's fields in two ways: if a value is a dictionary,
Line 589:     update it recursively. Otherwise replace the value (e.g. a string).
Line 590:     """


-- 
To view, visit https://gerrit.ovirt.org/46430
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Iaffdc836f8f64ecdc0a7e37ef50c228032f99696
Gerrit-PatchSet: 9
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ondřej Svoboda <osvoboda at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Ido Barkan <ibarkan at redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Ondřej Svoboda <osvoboda at redhat.com>
Gerrit-Reviewer: Petr Horáček <phoracek at redhat.com>
Gerrit-Reviewer: automation at ovirt.org
Gerrit-HasComments: Yes


More information about the vdsm-patches mailing list