In case the D-Bus interfaces start dropping off (typically all off them go
one by one when the object is being deleted), don't reset all the properties.
In particular, keep most properties around, only tear down "o" and "ao",
so that the object dependencies get torn down, but we still get enough
properties around to identify what the dead object was its heyday.
One example of where this is not good is when the device-removed signal
is emmitted, the device no longer has the ifname:
$ nmcli monitor
<quit NetworkManager>
(null): device removed
(null): device removed
...
Currently we assert that properties are reset on client teardown. That
is not the right thing to do and we're not going to do that in future.
However, what is important to test is that the properties are reset when
the daemon goes away. Test that.
The part where a device was created and its cleanup on client
description was only run randomly.
This is silly and gave me hard time. No reason not to be always running
it.
Previously we'd note if NM is stopped, but not if it's running.
I suppose it's nice for the user to know that the monitor started
running, but, it's also important for the monitor to be testable (so
that we know that we are ready to start adding mock objects, etc.)
This also gets rids of some duplication at expense of a little less
nuanced message.
NMSettingWired does not reject invalid flags. Filter them out in wake_on_lan_enable().
In practice, it makes no difference, the unknown flags were ignored anyway.
If there were any pause options and any non-pause options,
the created setting was invalid.
I don't think it's reasonably possible to parse the broken settings.
So there is no workaround trying to read the existing broken settings
from disk. Luckily, the broken setting was just silently ignored by
the parser, so you simply could not persist certain settings.
https://bugzilla.redhat.com/show_bug.cgi?id=2134569
Fixes: 652ddca04c ('ethtool: Introducing PAUSE support')
It's possible that the modem is enabled outside of NM. If not
re-check, device could stay disabled throughout until something else
about the modem changes again.
If modem is not at least "registered", a connection is not happening, which
means IP settings change is probably not interesting. Avoid trying to
parse it, so that we don't trigger connection failure when there isn't
one.
Downstream patches for this does it through NMSettings plugin, however
settings plugin are hard to maintain and complicated architecture wise
as well.
So directly create a connection profiles in-memory from the
nm-modem-ofono side. Those profiles are created in /run, and are not
added as a persistent connection, because connection state quite depends
on the state of ofono
This also allows us to drop the hack where we are keeping track of
active context/APN through the connection name, i.e if connection name
was in /imsi/context1 format, it was used. Instead now, Connection name
is actual context name which is user friendly ("Vodafone Connect" e.g.
in my case), and details like IMSI and context are stored internally.
[ratchanan@ubports.com:
- forward-ported to main branch.
- fold "wwan/ofono: handle context removal" into this commit.
- track the "preferred"-ness of the context and react accordingly.
Creates proxies for all retrived contexts to listen to changes.
While at it, also track name and type.
- use, instead of ignore, internet APN. Also support internet+mms APN.
- correct priv->contexts' value destroy function.
- factor out UUID generation as a helper function.
- handle the case where context dictionary is missing required keys.
- simplify nm_ofono_connection_new's arguments and rename to
add_or_update_connection. Makes it handle the case where the
connection already exists.
- also simplify other functions' arguments.
- clean up code and comments. Fix memory problems. Get rid of warnings.
]
Co-authored-by: Ratchanan Srirattanamet <ratchanan@ubports.com>
This verifies that what's in our public headers has version nodes, and
that they match Since: tags.
Not pretty (because python) but discovered a *lot* of issues.
"gen-metadata-nm-settings-libnm-core.xml" now contains also the names of
the NMSetting types, like "NMSettingConnection". That can be useful
to create NMSetting instances generically (that is, without knowing
the C API that gets called).
So you might be tempted to run
#!/bin/python
import gi
gi.require_version("NM", "1.0")
from gi.repository import GObject, NM
connection = NM.SimpleConnection()
# NM.utils_ensure_gtypes()
gtype_name = "NMSetting6Lowpan"
gtype = GObject.type_from_name(gtype_name)
setting = GObject.new(gtype)
connection.add_setting(setting)
However, without NM.utils_ensure_gtypes() that would not work, because
the GType is not yet created. For a user who doesn't know a priory all
setting types, it's not entirely clear how to make this work. Well, a
GObject introspection user could iterate over al NM.Setting* names and
try to instantiate the classes. However, that is still cumbersome, and not
accessible to a C user (without GI) and the currently loaded libnm
library may be newer and have unknown setting types.
In particular plain C user would need to know to call all the right
nm_setting_*_get_type(), functions, so it needs to know all the existing
52 type getters (and cannot support those from a newer libnm version).
With nm_utils_ensure_gtypes(), the user can get the typename and create
instances generically only using g_type_from_name().
Possible alternatives:
- libnm also has _nm_utils_init() which runs as __attribute__((constructor)).
We could also always instantiate all GType there. However, I don't like running
non-trivial, absolutely necessary code before main().
- hook nm_setting_get_type() to create all GType for the NMSetting
subclasses too. The problem is, that it's not entirely trivial to
avoid deadlock.
- hook nm_connection_get_type() to create all NMSetting types. That
would not deadlock, but it still is questionable whether we should
automatically, at non-obvious times instantiate all GTypes.
The NMPObject is a tagged union. There is no need to initialize anything
after the size of the actually used union field. Change this, so maybe we
get a valgrind warning about uninitialized memory if we wrongly try to
access it.
On the other hand, the object really is supposed to be a full
NMPObject. Previously, we would get a valgrind warning, if we tried to
pass fewer data there.
It really doesn't matter much, but all other functions don't assume
that there is any important data after the size indicated by the class.
We will extend IPv4 routes with the list of next hops. This field will
be heap allocated and be part of the NMPObjectIP4Route object, while
also being part of the identity. To support the ID operator that checks
fields of the NMPObject, add a "for_id" argument to the hash/cmp hooks.
Also, a function that sets cmd_obj_{hash_update,cmp}() MUST not set
cmd_plobj_id_{hashupdate,cmp}(), as it would have overlapping
functionality. Therefore, the objects that define
cmd_obj_{hash_update,cmp}() need to fully implement the ID comparison.
A NMPClass that has data outside the plobj part, needs
to implement the cmd_obj_*() hooks, instead of cmd_plobj_*().
For those objects, reasoning only about the plobj part is not
sufficient. Implementing both hooks is also unnecessary and
confusing.
Ensure that if we have cmd_obj_*() hooks set, that the corresponding
cmd_plobj_*() hooks are unset.
The if-else-if was wrong. It meant that if an object did not implement
cmd_plobj_id_copy(), nothign was copied (for id-only).
I think this code path was not actually hit, because we never clone
an object only by ID.
Fixes: c91a4617a1 ('nmp-object: allow missing implementations for certain virtual functions')
Use of conditionals in makefiles needs to be kept to a necessary minimum
otherwise they get out of hand quickly. There's no indentation to aid
reading and conditional chunks longer than a screen and nested ones are
almost impossible to comprehend.
The "if HAVE_INTROSPECTION" part does both.
Let's make it a little less horrible. There's generally no point in
making unused targets or variable assignment unless they collide with
pre-built stuff or have multiple variants.
Turns out automake doesn't like either that we add dependencies to a
target conditionally. Not sure why, but it's a tasteless thing to do
nevertheless:
gtk-doc.make:174: warning: $(DOC_MAIN_SGML_FILE) was already defined in condition BUILD_DOCS, which is included in condition TRUE ...
docs/api/Makefile.am:112: 'gtk-doc.make' included from here
docs/api/Makefile.am:14: ... '$(DOC_MAIN_SGML_FILE)' previously defined here
In any case, the dependency is wrong in the first place -- the source
document doesn't depend on other source. The target (which is a stamp
file, since the xslt processor produces multiple files) does! Moreover,
there's a dependency of the stamp file on $(content_files) already, so
including GENERATED_FILES (like we already do) in content_files should
be sufficient.
While at that, deal with other not-so-nice things; don't extend "all"
target needlessly and don't define a target conditionally.
These are present in a public header yet are not properly commented,
versioned or exported.
Export them now. Another option would be to move them to a private
header; but I suspect someone has intended them to be exported at some
point.
Add them to @libnm_1_40_4 as opposed to @libnm_1_42_0 because we now know
this is going to be backported to 1.40.4 first.
Remove the functions from a public header. They were missing from
libnm.ver and thus never actually exported.
There's no point in salvaging them now as the whole NMVpnPluginOld has
been replaced NMVpnServicePlugin and new uses are discouraged.
Remove the function from a public header. It was missing from libnm.ver
and thus never actually exported.
There's no point in salvaging it now as it nm_device_get_hw_address()
exists as a better option.
