This is merely a historical archive of years 2008-2021, before the migration to mailman3.
A maintained and still updated list archive can be found at https://lists.osmocom.org/hyperkitty/list/gerrit-log@lists.osmocom.org/.
dexter gerrit-no-reply at lists.osmocom.orgdexter has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-mgw/+/18590 )
Change subject: trunk: get rid of virt_trunk pointer
......................................................................
Patch Set 12:
(16 comments)
Apparently gerrit does not work the way it should. Apparantly pending comments are not sent, even after pushing the changes. Please do not merge this or comment any further until I get back to you again.
https://gerrit.osmocom.org/c/osmo-mgw/+/18590/4/src/libosmo-mgcp/mgcp_endp.c
File src/libosmo-mgcp/mgcp_endp.c:
https://gerrit.osmocom.org/c/osmo-mgw/+/18590/4/src/libosmo-mgcp/mgcp_endp.c@a100
PS4, Line 100: if (strlen(epname) <= prefix_len)
> unrelated?
Done
https://gerrit.osmocom.org/c/osmo-mgw/+/18590/4/src/libosmo-mgcp/mgcp_osmux.c
File src/libosmo-mgcp/mgcp_osmux.c:
https://gerrit.osmocom.org/c/osmo-mgw/+/18590/4/src/libosmo-mgcp/mgcp_osmux.c@207
PS4, Line 207: for (i=0; i<trunk->number_endpoints; i++) {
> since you are changing the line you could fix the missing spacing too.
Done
https://gerrit.osmocom.org/c/osmo-mgw/+/18590/4/src/libosmo-mgcp/mgcp_trunk.c
File src/libosmo-mgcp/mgcp_trunk.c:
https://gerrit.osmocom.org/c/osmo-mgw/+/18590/4/src/libosmo-mgcp/mgcp_trunk.c@34
PS4, Line 34: int mgcp_trunk_alloc(struct mgcp_config *cfg, enum mgcp_trunk_type ttype, int nr)
> alloc returning something than a pointer is confusing. Better return the pointer or NULL.
Done
https://gerrit.osmocom.org/c/osmo-mgw/+/18590/4/src/libosmo-mgcp/mgcp_vty.c
File src/libosmo-mgcp/mgcp_vty.c:
https://gerrit.osmocom.org/c/osmo-mgw/+/18590/4/src/libosmo-mgcp/mgcp_vty.c@852
PS4, Line 852: * parameters, so wie restrict the access to trunks with id numbers
> typo we
Done
https://gerrit.osmocom.org/c/osmo-mgw/+/18590/10/src/libosmo-mgcp/mgcp_vty.c
File src/libosmo-mgcp/mgcp_vty.c:
https://gerrit.osmocom.org/c/osmo-mgw/+/18590/10/src/libosmo-mgcp/mgcp_vty.c@557
PS10, Line 557: struct mgcp_trunk *trunk = mgcp_trunk_by_num(g_cfg, MGCP_VIRT_TRUNK_ID);
> OSMO_ASSERT(trunk)?
Done
https://gerrit.osmocom.org/c/osmo-mgw/+/18590/10/src/libosmo-mgcp/mgcp_vty.c@572
PS10, Line 572: struct mgcp_trunk *trunk = mgcp_trunk_by_num(g_cfg, MGCP_VIRT_TRUNK_ID);
> OSMO_ASSERT(trunk)?
Done
https://gerrit.osmocom.org/c/osmo-mgw/+/18590/10/src/libosmo-mgcp/mgcp_vty.c@582
PS10, Line 582: struct mgcp_trunk *trunk = mgcp_trunk_by_num(g_cfg, MGCP_VIRT_TRUNK_ID);
> OSMO_ASSERT(trunk)?
Done
https://gerrit.osmocom.org/c/osmo-mgw/+/18590/10/src/libosmo-mgcp/mgcp_vty.c@620
PS10, Line 620: struct mgcp_trunk *trunk = mgcp_trunk_by_num(g_cfg, MGCP_VIRT_TRUNK_ID);
> OSMO_ASSERT(trunk)?
Done
https://gerrit.osmocom.org/c/osmo-mgw/+/18590/10/src/libosmo-mgcp/mgcp_vty.c@631
PS10, Line 631: struct mgcp_trunk *trunk = mgcp_trunk_by_num(g_cfg, MGCP_VIRT_TRUNK_ID);
> OSMO_ASSERT(trunk)?
Done
https://gerrit.osmocom.org/c/osmo-mgw/+/18590/10/src/libosmo-mgcp/mgcp_vty.c@642
PS10, Line 642: struct mgcp_trunk *trunk = mgcp_trunk_by_num(g_cfg, MGCP_VIRT_TRUNK_ID);
> OSMO_ASSERT(trunk)?
Done
https://gerrit.osmocom.org/c/osmo-mgw/+/18590/10/src/libosmo-mgcp/mgcp_vty.c@653
PS10, Line 653: struct mgcp_trunk *trunk = mgcp_trunk_by_num(g_cfg, MGCP_VIRT_TRUNK_ID);
> ...
Done
https://gerrit.osmocom.org/c/osmo-mgw/+/18590/10/src/libosmo-mgcp/mgcp_vty.c@880
PS10, Line 880: continue;
> so, where/how is the virtual trunk written to config?
The config for the virtual trunk is written in config_write_mgcp().
https://gerrit.osmocom.org/c/osmo-mgw/+/18590/10/src/libosmo-mgcp/mgcp_vty.c@891
PS10, Line 891: vty_out(vty, " rtp keep-alive %d%s",
> ... […]
There are vty commands with the same name, but on different nodes. See mgcp_vty_init(). This is very confusing, I got confused too. I have removed those fixme notes now.
https://gerrit.osmocom.org/c/osmo-mgw/+/18590/11/src/libosmo-mgcp/mgcp_vty.c
File src/libosmo-mgcp/mgcp_vty.c:
https://gerrit.osmocom.org/c/osmo-mgw/+/18590/11/src/libosmo-mgcp/mgcp_vty.c@751
PS11, Line 751: OSMO_ASSERT(trunk);
> thanks for applying here...
Done
https://gerrit.osmocom.org/c/osmo-mgw/+/18590/11/src/libosmo-mgcp/mgcp_vty.c@760
PS11, Line 760: struct mgcp_trunk *trunk = mgcp_trunk_by_num(g_cfg, MGCP_VIRT_TRUNK_ID);
> ...and there are still many more of these below […]
Thanks for catching this. I overlooked the remaining ones, probably my search string was too distinctive.
https://gerrit.osmocom.org/c/osmo-mgw/+/18590/11/src/libosmo-mgcp/mgcp_vty.c@882
PS11, Line 882: continue;
> please still explain, as in previous comment: […]
I would love to eliminate config_write_mgcp() completely, but I think this changes the syntax of the configuration file too much. Also I think the time is not right for that yet. When the E1 support is done we have two different trunk sets, and then we can see much better how we can find a more meaningful configuration scheme. I think it would also be very nice to be able to create multiple virtual trunks. Think of one MGW that handles BSC and MSC on two different virtual trunks.
--
To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/18590
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-mgw
Gerrit-Branch: master
Gerrit-Change-Id: I81934fbd211b225ab7920e78510729c8e22607b3
Gerrit-Change-Number: 18590
Gerrit-PatchSet: 12
Gerrit-Owner: dexter <pmaier at sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: dexter <pmaier at sysmocom.de>
Gerrit-Reviewer: laforge <laforge at osmocom.org>
Gerrit-Reviewer: neels <nhofmeyr at sysmocom.de>
Gerrit-Reviewer: pespin <pespin at sysmocom.de>
Gerrit-Comment-Date: Thu, 18 Jun 2020 09:16:58 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: pespin <pespin at sysmocom.de>
Comment-In-Reply-To: neels <nhofmeyr at sysmocom.de>
Comment-In-Reply-To: laforge <laforge at osmocom.org>
Gerrit-MessageType: comment
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osmocom.org/pipermail/gerrit-log/attachments/20200618/45cdead3/attachment.htm>