Patch Set 1: (4 comments)
https://gerrit.osmocom.org/#/c/3731/2/src/osmo_gsm_tester/bts_sysmo.py File src/osmo_gsm_tester/bts_sysmo.py: Line 117: self.band_arfcn = band_arfcn > self.band_arfcn should have a default value up in __init__() or above that. Good point. I didn't do it probably because it is always set when requesting the object in suite.py:bts_obj(), but it still may make sense to have a default None there. Line 120: return self.band_arfcn.get('band') > can self.band_arfcn be unset? Then this would raise a NoneType exception. It should always be set by suite.py just after creating it in suite.py:bts_obj, so I don't expect it to be used without it being set. So no problem here. Line 123: return int(self.band_arfcn.get('arfcn')) > same same too. https://gerrit.osmocom.org/#/c/3731/1/src/osmo_gsm_tester/suite.py File src/osmo_gsm_tester/suite.py: Line 484: arfcn = suite_run._try_reserve_arfcn(supported_bands) > I am fairly strongly against reserving more resources after the test starte If the suite cannot reserve a resource, it can wait for it. The only issue I see here is actually about possible deadlocks as I explained above. On the other hand, during last days I though about the current proposal in this patchset, and I think it still fails to resolve the issue because it only provides automatic matching between free ARFCNs and BTS, but not with modems. Which means if we have modems not supporting all bands, we could end up in an scenario in which a BTS supporting 1800 is configured for that one, but obtain a modem which only supports 900, unless we really specify a scenario for all to use the same band. And I'd definitely like to avoid this. So, I think the only safe bet here, would be to add specific parser logic while holding the resource lock and resolving resources, to check that ARFCN we select is in a band which is supported by both modem and bts (which can be checked by looking at modem and bts "bands" resource properties.) What do you think? Let's talk about this in the office, I agree it's going to be more useful. -- To view, visit https://gerrit.osmocom.org/3731 To unsubscribe, visit https://gerrit.osmocom.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I6fb5d95bed1fa50c3deaf62a7a6df3cb276bc3c9 Gerrit-PatchSet: 1 Gerrit-Project: osmo-gsm-tester Gerrit-Branch: master Gerrit-Owner: Pau Espin Pedrol <[email protected]> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: Neels Hofmeyr <[email protected]> Gerrit-Reviewer: Pau Espin Pedrol <[email protected]> Gerrit-Reviewer: neels <[email protected]> Gerrit-HasComments: Yes
