Change in osmo-mgw[master]: trunk: get rid of virt_trunk pointer

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.org
Thu Jun 18 09:16:58 UTC 2020


dexter 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>


More information about the gerrit-log mailing list