Change in vdsm[master]: models, configurators: replace the internal 'async' flag wit...
automat...@ovirt.org has posted comments on this change. Change subject: models, configurators: replace the internal 'async' flag with blockingdhcp .. Patch Set 8: * Update tracker::IGNORE, no Bug-Url found * Set MODIFIED::IGNORE, no Bug-Url found. -- To view, visit https://gerrit.ovirt.org/40456 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I0e60040320d65ddbc4146a31c02c388ddad13ea9 Gerrit-PatchSet: 8 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Ondřej Svoboda Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Ido Barkan Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Ondřej Svoboda Gerrit-Reviewer: Petr Horáček Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: models, configurators: replace the internal 'async' flag wit...
Dan Kenigsberg has submitted this change and it was merged. Change subject: models, configurators: replace the internal 'async' flag with blockingdhcp .. models, configurators: replace the internal 'async' flag with blockingdhcp DHCP is queried asynchronously in usual situations so it is better to use 'blockingdhcp' which is a flag that signals some exceptional flow (network restoration, for example). blockingdhcp is also a part of the API so now there is direct correspondence between network configurators' code and the API. Change-Id: I0e60040320d65ddbc4146a31c02c388ddad13ea9 Signed-off-by: Ondřej Svoboda Reviewed-on: https://gerrit.ovirt.org/40456 Continuous-Integration: Jenkins CI Reviewed-by: Ido Barkan Reviewed-by: Dan Kenigsberg --- M vdsm/network/configurators/__init__.py M vdsm/network/configurators/dhclient.py M vdsm/network/configurators/ifcfg.py M vdsm/network/models.py 4 files changed, 16 insertions(+), 20 deletions(-) Approvals: Ido Barkan: Looks good to me, but someone else must approve Ondřej Svoboda: Verified Jenkins CI: Passed CI tests Dan Kenigsberg: Looks good to me, approved -- To view, visit https://gerrit.ovirt.org/40456 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: merged Gerrit-Change-Id: I0e60040320d65ddbc4146a31c02c388ddad13ea9 Gerrit-PatchSet: 8 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Ondřej Svoboda Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Ido Barkan Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Ondřej Svoboda Gerrit-Reviewer: Petr Horáček Gerrit-Reviewer: automat...@ovirt.org ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: models, configurators: replace the internal 'async' flag wit...
Ondřej Svoboda has posted comments on this change. Change subject: models, configurators: replace the internal 'async' flag with blockingdhcp .. Patch Set 7: Verified+1 Verified together with https://gerrit.ovirt.org/#/c/40478/4 -- To view, visit https://gerrit.ovirt.org/40456 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I0e60040320d65ddbc4146a31c02c388ddad13ea9 Gerrit-PatchSet: 7 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Ondřej Svoboda Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Ido Barkan Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Ondřej Svoboda Gerrit-Reviewer: Petr Horáček Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: models, configurators: replace the internal 'async' flag wit...
Dan Kenigsberg has posted comments on this change. Change subject: models, configurators: replace the internal 'async' flag with blockingdhcp .. Patch Set 7: Code-Review+2 -- To view, visit https://gerrit.ovirt.org/40456 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I0e60040320d65ddbc4146a31c02c388ddad13ea9 Gerrit-PatchSet: 7 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Ondřej Svoboda Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Ido Barkan Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Ondřej Svoboda Gerrit-Reviewer: Petr Horáček Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: models, configurators: replace the internal 'async' flag wit...
Ido Barkan has posted comments on this change. Change subject: models, configurators: replace the internal 'async' flag with blockingdhcp .. Patch Set 7: Code-Review+1 -- To view, visit https://gerrit.ovirt.org/40456 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I0e60040320d65ddbc4146a31c02c388ddad13ea9 Gerrit-PatchSet: 7 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Ondřej Svoboda Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Ido Barkan Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Ondřej Svoboda Gerrit-Reviewer: Petr Horáček Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: models, configurators: replace the internal 'async' flag wit...
automat...@ovirt.org has posted comments on this change. Change subject: models, configurators: replace the internal 'async' flag with blockingdhcp .. Patch Set 7: * Update tracker::IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3']) -- To view, visit https://gerrit.ovirt.org/40456 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I0e60040320d65ddbc4146a31c02c388ddad13ea9 Gerrit-PatchSet: 7 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Ondřej Svoboda Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Ido Barkan Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Ondřej Svoboda Gerrit-Reviewer: Petr Horáček Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: models, configurators: replace the internal 'async' flag wit...
Ido Barkan has posted comments on this change. Change subject: models, configurators: replace the internal 'async' flag with blockingdhcp .. Patch Set 6: Code-Review+1 -- To view, visit https://gerrit.ovirt.org/40456 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I0e60040320d65ddbc4146a31c02c388ddad13ea9 Gerrit-PatchSet: 6 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Ondřej Svoboda Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Ido Barkan Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Ondřej Svoboda Gerrit-Reviewer: Petr Horáček Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: models, configurators: replace the internal 'async' flag wit...
Petr Horáček has posted comments on this change. Change subject: models, configurators: replace the internal 'async' flag with blockingdhcp .. Patch Set 6: Code-Review+1 -- To view, visit https://gerrit.ovirt.org/40456 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I0e60040320d65ddbc4146a31c02c388ddad13ea9 Gerrit-PatchSet: 6 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Ondřej Svoboda Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Ido Barkan Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Ondřej Svoboda Gerrit-Reviewer: Petr Horáček Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: models, configurators: replace the internal 'async' flag wit...
automat...@ovirt.org has posted comments on this change. Change subject: models, configurators: replace the internal 'async' flag with blockingdhcp .. Patch Set 6: * Update tracker::IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3']) -- To view, visit https://gerrit.ovirt.org/40456 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I0e60040320d65ddbc4146a31c02c388ddad13ea9 Gerrit-PatchSet: 6 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Ondřej Svoboda Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Ido Barkan Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Ondřej Svoboda Gerrit-Reviewer: Petr Horáček Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: models, configurators: replace the internal 'async' flag wit...
Ondřej Svoboda has posted comments on this change. Change subject: models, configurators: replace the internal 'async' flag with blockingdhcp .. Patch Set 5: Code-Review-1 (1 comment) https://gerrit.ovirt.org/#/c/40456/5/vdsm/network/configurators/ifcfg.py File vdsm/network/configurators/ifcfg.py: Line 774: raise ConfigNetworkError(ERR_FAILED_IFUP, out[-1] if out else '') Line 775: Line 776: Line 777: def _ifup_nonblocking(iface): Line 778: if iface and not iface.blockingdhcp and (iface.ipv4.bootproto == 'dhcp' or > why 'if iface' ? A remnant of the old approach, thanks for pointing it out :-) Line 779: iface.ipv6.dhcpv6): Line 780: # wait for dhcp in another thread, so vdsm won't get stuck (BZ#498940) Line 781: t = threading.Thread(target=_ifup, name='ifup-waiting-on-dhcp', Line 782: args=(iface.name,)) -- To view, visit https://gerrit.ovirt.org/40456 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I0e60040320d65ddbc4146a31c02c388ddad13ea9 Gerrit-PatchSet: 5 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Ondřej Svoboda Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Ido Barkan Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Ondřej Svoboda Gerrit-Reviewer: Petr Horáček Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: models, configurators: replace the internal 'async' flag wit...
Petr Horáček has posted comments on this change. Change subject: models, configurators: replace the internal 'async' flag with blockingdhcp .. Patch Set 5: (1 comment) https://gerrit.ovirt.org/#/c/40456/5/vdsm/network/configurators/ifcfg.py File vdsm/network/configurators/ifcfg.py: Line 774: raise ConfigNetworkError(ERR_FAILED_IFUP, out[-1] if out else '') Line 775: Line 776: Line 777: def _ifup_nonblocking(iface): Line 778: if iface and not iface.blockingdhcp and (iface.ipv4.bootproto == 'dhcp' or why 'if iface' ? Line 779: iface.ipv6.dhcpv6): Line 780: # wait for dhcp in another thread, so vdsm won't get stuck (BZ#498940) Line 781: t = threading.Thread(target=_ifup, name='ifup-waiting-on-dhcp', Line 782: args=(iface.name,)) -- To view, visit https://gerrit.ovirt.org/40456 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I0e60040320d65ddbc4146a31c02c388ddad13ea9 Gerrit-PatchSet: 5 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Ondřej Svoboda Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Ido Barkan Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Ondřej Svoboda Gerrit-Reviewer: Petr Horáček Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: models, configurators: replace the internal 'async' flag wit...
automat...@ovirt.org has posted comments on this change. Change subject: models, configurators: replace the internal 'async' flag with blockingdhcp .. Patch Set 5: * Update tracker::IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3']) -- To view, visit https://gerrit.ovirt.org/40456 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I0e60040320d65ddbc4146a31c02c388ddad13ea9 Gerrit-PatchSet: 5 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Ondřej Svoboda Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Ido Barkan Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Ondřej Svoboda Gerrit-Reviewer: Petr Horáček Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: models, configurators: replace the internal 'async' flag wit...
Ido Barkan has posted comments on this change. Change subject: models, configurators: replace the internal 'async' flag with blockingdhcp .. Patch Set 4: Code-Review+1 -- To view, visit https://gerrit.ovirt.org/40456 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I0e60040320d65ddbc4146a31c02c388ddad13ea9 Gerrit-PatchSet: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Ondřej Svoboda Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Ido Barkan Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Ondřej Svoboda Gerrit-Reviewer: Petr Horáček Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: models, configurators: replace the internal 'async' flag wit...
automat...@ovirt.org has posted comments on this change. Change subject: models, configurators: replace the internal 'async' flag with blockingdhcp .. Patch Set 4: * Update tracker::IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3']) -- To view, visit https://gerrit.ovirt.org/40456 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I0e60040320d65ddbc4146a31c02c388ddad13ea9 Gerrit-PatchSet: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Ondřej Svoboda Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Ido Barkan Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Ondřej Svoboda Gerrit-Reviewer: Petr Horáček Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: models, configurators: replace the internal 'async' flag wit...
Dan Kenigsberg has posted comments on this change. Change subject: models, configurators: replace the internal 'async' flag with blockingdhcp .. Patch Set 3: Code-Review-1 (1 comment) https://gerrit.ovirt.org/#/c/40456/3/vdsm/network/configurators/ifcfg.py File vdsm/network/configurators/ifcfg.py: Line 763: rc, _, _ = utils.execCmd([constants.EXT_IFDOWN, iface], raw=True) Line 764: return rc Line 765: Line 766: Line 767: def ifup(iface=None, iface_name=None): > And yes, as I state in the previous commit, since 'async' was not used by c Sorry - I cannot accept the polymorphism of this function. It would have to verify that only one of its args is non-None, and has a different functionality for each. In my book this is a not a good spec for a function. Please introduce the asych->blocking rename in one patch. Then, add a new module-level function that accepts only a NetDevice and extracts (iface_name, blocking) from it. Line 768: "Bring up an interface" Line 769: def _ifup(netIf): Line 770: rc, out, err = utils.execCmd([constants.EXT_IFUP, netIf], raw=False) Line 771: -- To view, visit https://gerrit.ovirt.org/40456 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I0e60040320d65ddbc4146a31c02c388ddad13ea9 Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Ondřej Svoboda Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Ido Barkan Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Ondřej Svoboda Gerrit-Reviewer: Petr Horáček Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: models, configurators: replace the internal 'async' flag wit...
Ondřej Svoboda has posted comments on this change. Change subject: models, configurators: replace the internal 'async' flag with blockingdhcp .. Patch Set 3: (1 comment) https://gerrit.ovirt.org/#/c/40456/3/vdsm/network/configurators/ifcfg.py File vdsm/network/configurators/ifcfg.py: Line 763: rc, _, _ = utils.execCmd([constants.EXT_IFDOWN, iface], raw=True) Line 764: return rc Line 765: Line 766: Line 767: def ifup(iface=None, iface_name=None): > I also thought of renaming but in this patch it is unrelated work :-) ifdow And yes, as I state in the previous commit, since 'async' was not used by callers that only knew an iface name I now make it explicit that async is only extracted from a NetDevice instance. If we want different behaviour for these callers (enable them to ask for nonblocking DHCP) we have to be explicit again, which I think is good strategy. "ifup calls that did not use 'async' now pass an interface name through 'iface_name' parameter so there will be no change in behaviour when the 'async' flag is removed and determined inside the function." Line 768: "Bring up an interface" Line 769: def _ifup(netIf): Line 770: rc, out, err = utils.execCmd([constants.EXT_IFUP, netIf], raw=False) Line 771: -- To view, visit https://gerrit.ovirt.org/40456 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I0e60040320d65ddbc4146a31c02c388ddad13ea9 Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Ondřej Svoboda Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Ido Barkan Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Ondřej Svoboda Gerrit-Reviewer: Petr Horáček Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: models, configurators: replace the internal 'async' flag wit...
Ondřej Svoboda has posted comments on this change. Change subject: models, configurators: replace the internal 'async' flag with blockingdhcp .. Patch Set 3: (1 comment) https://gerrit.ovirt.org/#/c/40456/3/vdsm/network/configurators/ifcfg.py File vdsm/network/configurators/ifcfg.py: Line 763: rc, _, _ = utils.execCmd([constants.EXT_IFDOWN, iface], raw=True) Line 764: return rc Line 765: Line 766: Line 767: def ifup(iface=None, iface_name=None): > okay. so now cannot use if with a name and make it async for dhcp. This is I also thought of renaming but in this patch it is unrelated work :-) ifdown would have to be renamed, too, and maybe even other functions. Line 768: "Bring up an interface" Line 769: def _ifup(netIf): Line 770: rc, out, err = utils.execCmd([constants.EXT_IFUP, netIf], raw=False) Line 771: -- To view, visit https://gerrit.ovirt.org/40456 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I0e60040320d65ddbc4146a31c02c388ddad13ea9 Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Ondřej Svoboda Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Ido Barkan Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Ondřej Svoboda Gerrit-Reviewer: Petr Horáček Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: models, configurators: replace the internal 'async' flag wit...
Ido Barkan has posted comments on this change. Change subject: models, configurators: replace the internal 'async' flag with blockingdhcp .. Patch Set 3: (1 comment) https://gerrit.ovirt.org/#/c/40456/3/vdsm/network/configurators/ifcfg.py File vdsm/network/configurators/ifcfg.py: Line 763: rc, _, _ = utils.execCmd([constants.EXT_IFDOWN, iface], raw=True) Line 764: return rc Line 765: Line 766: Line 767: def ifup(iface=None, iface_name=None): okay. so now cannot use if with a name and make it async for dhcp. This is fine, since this function is only used by ifcfg internally and is expecting an ifcfg iface instance to behave properly. I think we can make it 'module private' so no one will use it. Line 768: "Bring up an interface" Line 769: def _ifup(netIf): Line 770: rc, out, err = utils.execCmd([constants.EXT_IFUP, netIf], raw=False) Line 771: -- To view, visit https://gerrit.ovirt.org/40456 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I0e60040320d65ddbc4146a31c02c388ddad13ea9 Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Ondřej Svoboda Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Ido Barkan Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Ondřej Svoboda Gerrit-Reviewer: Petr Horáček Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: models, configurators: replace the internal 'async' flag wit...
Petr Horáček has posted comments on this change. Change subject: models, configurators: replace the internal 'async' flag with blockingdhcp .. Patch Set 3: Code-Review+1 -- To view, visit https://gerrit.ovirt.org/40456 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I0e60040320d65ddbc4146a31c02c388ddad13ea9 Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Ondřej Svoboda Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Ido Barkan Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Ondřej Svoboda Gerrit-Reviewer: Petr Horáček Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: models, configurators: replace the internal 'async' flag wit...
Ondřej Svoboda has posted comments on this change. Change subject: models, configurators: replace the internal 'async' flag with blockingdhcp .. Patch Set 3: Verified+1 Tested with #40478 (on which this patch depends) on EL7.1 and there was no regression in functional network tests. -- To view, visit https://gerrit.ovirt.org/40456 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I0e60040320d65ddbc4146a31c02c388ddad13ea9 Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Ondřej Svoboda Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Ondřej Svoboda Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: models, configurators: replace the internal 'async' flag wit...
automat...@ovirt.org has posted comments on this change. Change subject: models, configurators: replace the internal 'async' flag with blockingdhcp .. Patch Set 3: * Update tracker::IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3']) -- To view, visit https://gerrit.ovirt.org/40456 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I0e60040320d65ddbc4146a31c02c388ddad13ea9 Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Ondřej Svoboda Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: models, configurators: replace the internal 'async' flag wit...
automat...@ovirt.org has posted comments on this change. Change subject: models, configurators: replace the internal 'async' flag with blockingdhcp .. Patch Set 2: * Update tracker::IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3']) -- To view, visit https://gerrit.ovirt.org/40456 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I0e60040320d65ddbc4146a31c02c388ddad13ea9 Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Ondřej Svoboda Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: models, configurators: replace the internal 'async' flag wit...
Ondřej Svoboda has uploaded a new change for review.
Change subject: models, configurators: replace the internal 'async' flag with
blockingdhcp
..
models, configurators: replace the internal 'async' flag with blockingdhcp
DHCP is queried asynchronously in usual situations so it is better to use
a flag that signals some exceptional flow (network restoration, for example).
blockingdhcp is also a part of the API so now there is direct correspondence
between network configurators' code and the API.
Change-Id: I0e60040320d65ddbc4146a31c02c388ddad13ea9
Signed-off-by: Ondřej Svoboda
---
M vdsm/network/configurators/__init__.py
M vdsm/network/configurators/dhclient.py
M vdsm/network/configurators/ifcfg.py
M vdsm/network/models.py
4 files changed, 14 insertions(+), 18 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/56/40456/1
diff --git a/vdsm/network/configurators/__init__.py
b/vdsm/network/configurators/__init__.py
index 0df5731..39679c5 100644
--- a/vdsm/network/configurators/__init__.py
+++ b/vdsm/network/configurators/__init__.py
@@ -177,7 +177,7 @@
def runDhclient(iface, family=4):
dhclient = DhcpClient(iface.name, family)
-rc = dhclient.start(iface.asynchronous_dhcp)
-if not iface.asynchronous_dhcp and rc:
+rc = dhclient.start(iface.blockingdhcp)
+if iface.blockingdhcp and rc:
raise ConfigNetworkError(ERR_FAILED_IFUP, 'dhclient%s failed',
family)
diff --git a/vdsm/network/configurators/dhclient.py
b/vdsm/network/configurators/dhclient.py
index f6db491..8b78cc7 100644
--- a/vdsm/network/configurators/dhclient.py
+++ b/vdsm/network/configurators/dhclient.py
@@ -55,15 +55,15 @@
'-lf', self.leaseFile, self.iface])
return rc, out, err
-def start(self, async):
-if async:
+def start(self, blocking):
+if blocking:
+rc, _, _ = self._dhclient()
+return rc
+else:
t = threading.Thread(target=self._dhclient, name='vdsm-dhclient-%s'
% self.iface)
t.daemon = True
t.start()
-else:
-rc, _, _ = self._dhclient()
-return rc
def shutdown(self):
try:
diff --git a/vdsm/network/configurators/ifcfg.py
b/vdsm/network/configurators/ifcfg.py
index 6d35c0a..2a5faba 100644
--- a/vdsm/network/configurators/ifcfg.py
+++ b/vdsm/network/configurators/ifcfg.py
@@ -90,13 +90,13 @@
if bridge.port:
bridge.port.configure(**opts)
self._addSourceRoute(bridge)
-ifup(bridge.name, bridge.asynchronous_dhcp)
+ifup(bridge.name)
def configureVlan(self, vlan, **opts):
self.configApplier.addVlan(vlan, **opts)
vlan.device.configure(**opts)
self._addSourceRoute(vlan)
-ifup(vlan.name, vlan.asynchronous_dhcp)
+ifup(vlan.name)
def configureBond(self, bond, **opts):
self.configApplier.addBonding(bond, **opts)
@@ -106,7 +106,7 @@
for slave in bond.slaves:
slave.configure(**opts)
self._addSourceRoute(bond)
-ifup(bond.name, bond.asynchronous_dhcp)
+ifup(bond.name)
if self.unifiedPersistence:
self.runningConfig.setBonding(
bond.name, {'options': bond.options,
@@ -159,7 +159,7 @@
if nic.bond is None:
if not netinfo.isVlanned(nic.name):
ifdown(nic.name)
-ifup(nic.name, nic.asynchronous_dhcp)
+ifup(nic.name)
def removeBridge(self, bridge):
DynamicSourceRoute.addInterfaceTracking(bridge)
@@ -764,7 +764,7 @@
return rc
-def ifup(iface, async=False):
+def ifup(iface):
"Bring up an interface"
def _ifup(netIf):
rc, out, err = utils.execCmd([constants.EXT_IFUP, netIf], raw=False)
@@ -775,7 +775,8 @@
raise ConfigNetworkError(ERR_FAILED_IFUP, out[-1] if out else '')
return rc, out, err
-if async:
+if not iface.blockingdhcp and (iface.ipv4.bootproto == 'dhcp' or
+ iface.ipv6.dhcpv6):
# wait for dhcp in another thread,
# so vdsm won't get stuck (BZ#498940)
t = threading.Thread(target=_ifup, name='ifup-waiting-on-dhcp',
diff --git a/vdsm/network/models.py b/vdsm/network/models.py
index 708bad8..5365797 100644
--- a/vdsm/network/models.py
+++ b/vdsm/network/models.py
@@ -80,11 +80,6 @@
def backing_device(self):
return False
-@property
-def asynchronous_dhcp(self):
-return ((self.ipv4.bootproto == 'dhcp' or self.ipv6.dhcpv6) and
-not self.blockingdhcp)
-
class Nic(NetDevice):
def __init__(self, name, configurator, ipv4=None, ipv6=None,
--
To view, visit https://gerrit.ovirt.org/40456
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-Mes
Change in vdsm[master]: models, configurators: replace the internal 'async' flag wit...
automat...@ovirt.org has posted comments on this change. Change subject: models, configurators: replace the internal 'async' flag with blockingdhcp .. Patch Set 1: * Update tracker::IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3']) -- To view, visit https://gerrit.ovirt.org/40456 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I0e60040320d65ddbc4146a31c02c388ddad13ea9 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Ondřej Svoboda Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
