If a device-factory wouldn't support any link-type or setting-type,
we would not take an additional reference to the @factory instance
(because, the factory is not added to one of the static hash tables).
As such, we would invoke the callback with a factory instance, which
is about to be destroyed immediately afterwards. That would be unusual
for device-plugins, because usually a device-plugin is never destroyed
and essentially leaked at exit.
Just don't get into that situation. All device plugins are internal API,
and they are known to support at least something. Assert for that.
(cherry picked from commit 94200b03fe)
Actually, we anyway leak them, because they are added to static hash tables
which are never released. Anyway, get the ref-count right.
(cherry picked from commit 4c43d7cad3)
Internal device plugins are compiled-in. In fact, none of the
internal device plugins can currently be disabled via compile
time options. The user would have to patch the sources to
not include a particular device plugin.
Hence, the available device plugins depends exclusively on the
build itself. That is not worth <info> level logging. Especially,
as it was quite verbose, logging 13 lines.
(cherry picked from commit dff157b867)
In some cases we might want to load device plugins from multiple
directories. A special case that I have in mind is to load plugins from
build directory subdirectories in order to run NetworkManager from the
build directory.
[thaller@redhat.com: modify original patch]
We also do this for libnm, where it causes visible changes
in behavior. But if somebody would rely on the hashing implementation
for hash tables, it would be seriously flawed.
GHashTable optimizes a NULL equality function to use direct pointer
comparison. That saves the overhead of calling g_direct_equal().
This is also documented behavior for g_hash_table_new().
While at it, also don't pass g_direct_hash() but use the default
of %NULL. The behavior is the same, but consistently don't use
g_direct_hash().
2. NetworkManager-1.9.2/src/devices/nm-device-factory.c:312:
returned_pointer: Assigning value from "g_slist_append(list,
g_object_ref(factory))" to "list" here, but that stored value is
overwritten before it can be used.
Fixes: 98afc76184
Fixes: 449940af1d
2. NetworkManager-1.9.2/src/devices/nm-device-factory.c:312:
returned_pointer: Assigning value from "g_slist_append(list,
g_object_ref(factory))" to "list" here, but that stored value is
overwritten before it can be used.
Fixes: 98afc76184
2. NetworkManager-1.9.2/src/devices/nm-device-factory.c:312:
returned_pointer: Assigning value from "g_slist_append(list,
g_object_ref(factory))" to "list" here, but that stored value is
overwritten before it can be used.
Replace the usage of g_str_hash() with our own nm_str_hash().
GLib's g_str_hash() uses djb2 hashing function, just like we
do at the moment. The only difference is, that we use a diffrent
seed value.
Note, that we initialize the hash seed with random data (by calling
getrandom() or reading /dev/urandom). That is a change compared to
before.
This change of the hashing function and accessing the random pool
might be undesired for libnm/libnm-core. Hence, the change is not
done there as it possibly changes behavior for public API. Maybe
we should do that later though.
At this point, there isn't much of a change. This patch becomes
interesting, if we decide to use a different hashing algorithm.
Make it possible to register different factories for the same setting
type, and add a match_connection() method to let each factory decide
if it's capable of handling a connection.
This will be used to decide whether a PPPoE connection must be handled
through the legacy Ethernet factory or through the new PPP factory.
The new device type represents a PPP interface, and will implement the
activation of new-style PPPoE connections, i.e. the ones that don't
claim the parent device.
Branch f9b1bc16e9 added bluetooth NAP
support. A NAP connection is of connection.type "bluetooth", but it
also has a "bridge" setting. Also, it is primarily handled by NMDeviceBridge
and NMBridgeDeviceFactory (with help from NMBluezManager).
However, don't let nm_connection_get_connection_type() and
nm_connnection_is_type() lie about what the connection.type is.
The type is "bluetooth" for most purposes -- at least, as far as
the client is concerned (and the public API of libnm). This restores
previous API behavior, where nm_connection_get_connection_type()
and nm_connection_is_type() would be simple accessors to the
"connection.type" property.
Only a few places care about the bridge aspect, and those places need special
treatment. For example NMDeviceBridge needs to be fully aware that it can
handle bluetooth NAP connection. That is nothing new: if you handle a
connection of any type, you must know which fields matter and what they
mean. It's not enough that nm_connection_get_connection_type() for bluetooth
NAP connectins is claiming to be a bridge.
Counter examples, where the original behavior is right:
src/nm-manager.c- g_set_error (error,
src/nm-manager.c- NM_MANAGER_ERROR,
src/nm-manager.c- NM_MANAGER_ERROR_FAILED,
src/nm-manager.c- "NetworkManager plugin for '%s' unavailable",
src/nm-manager.c: nm_connection_get_connection_type (connection));
the correct message is: "no bluetooth plugin available", not "bridge".
src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-writer.c: if ( ( nm_connection_is_type (connection, NM_SETTING_WIRED_SETTING_NAME)
src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-writer.c: && !nm_connection_get_setting_pppoe (connection))
src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-writer.c: || nm_connection_is_type (connection, NM_SETTING_VLAN_SETTING_NAME)
src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-writer.c: || nm_connection_is_type (connection, NM_SETTING_WIRELESS_SETTING_NAME)
src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-writer.c: || nm_connection_is_type (connection, NM_SETTING_INFINIBAND_SETTING_NAME)
src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-writer.c: || nm_connection_is_type (connection, NM_SETTING_BOND_SETTING_NAME)
src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-writer.c: || nm_connection_is_type (connection, NM_SETTING_TEAM_SETTING_NAME)
src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-writer.c: || nm_connection_is_type (connection, NM_SETTING_BRIDGE_SETTING_NAME))
src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-writer.c- return TRUE;
the correct behavior is for ifcfg-rh plugin to reject bluetooth NAP
connections, not proceed and store it.
The duplicate check is based only on the link-types and setting-types.
However, that doesn't really cut it because in case of bluetooth
NAP connections, NMBridgeDeviceFactory is responsible for the connection,
although connection.type is "bluetooth".
This is only here to catch loading invalid plugins. But at the point
where we load an invalid plugin, the process is hosed. This is at best
an assertion, but rather pointless really.
The simple link-types/setting-types mechanism doesn't really cut it
because for the bluetooth NAP connection.type is "bluetooth", but
it shall be primarily handled by the bridge factory.
It's internal API that allows for a basic matching of the factory.
It is however not sophisticated enough for the full complexity.
Make it as internal API only.
We'll use this to let the devices know they can retry autoactivation
because some component became available without actually having any
data that would be useful for that device.
Adjust the comment.
Add support for creating dummy devices. This commit adds a D-Bus
interface 'org.freedesktop.NetworkManager.Device.Dummy' which is used
primarily for determining the device type but does not carry any
properties.
Keep the include paths clean and separate. We use directories to group source
files together. That makes sense (I guess), but then we should use this
grouping also when including files. Thus require to #include files with their
path relative to "src/".
Also, we build various artifacts from the "src/" tree. Instead of having
individual CFLAGS for each artifact in Makefile.am, the CFLAGS should be
unified. Previously, the CFLAGS for each artifact differ and are inconsistent
in which paths they add to the search path. Fix the inconsistency by just
don't add the paths at all.
On some architectures, it seems we don't properly expose
the symbol of these static variables from NetworkManager
binary.
Just avoid that and don't instead use a static array
inside the device plugin itself.
While at it, make the arrays all const, which possibly allows
the linker to put those symbols in the read-only section.
Internal device types are a static thing. Let's not do a
constructor function to register the device factory.
This gets rid of all attribute((constructor)) functions inside
NetworkManager core. That is desired, because we don't want to
run code before main(). For example, at that point logging is
not yet initialized, but with code that runs before main() it
is hard to ensure that we don't log anything yet.
An interface would make sense to allow the actual device-factory to inherit
from another type.
However, glib interfaces make code much harder to follow and less
efficient. The device factory shall be a very simple type with meta data
about supported device types and the ability to create device instances.
There is no need to make this an interface implementation, instead just
let the factories inherit from NM_TYPE_DEVICE_FACTORY directly.
If the connection specifies an interface name, it should never attach to
a device of a different name even if the factory thinks the connection
is compatible with the device.
This fixes an issue that caused the inifniband connections to attach to
different devices or partitions.
Functions that take a GError** MUST fill it in on error. There is no
need to check whether error is NULL if the function it was passed to
had a failing return value.
Likewise, a proper GError must have a non-NULL message, so there's no
need to double-check that either.
Based-on-patch-by: Dan Winship <danw@gnome.org>
- All internal source files (except "examples", which are not internal)
should include "config.h" first. As also all internal source
files should include "nm-default.h", let "config.h" be included
by "nm-default.h" and include "nm-default.h" as first in every
source file.
We already wanted to include "nm-default.h" before other headers
because it might contains some fixes (like "nm-glib.h" compatibility)
that is required first.
- After including "nm-default.h", we optinally allow for including the
corresponding header file for the source file at hand. The idea
is to ensure that each header file is self contained.
- Don't include "config.h" or "nm-default.h" in any header file
(except "nm-sd-adapt.h"). Public headers anyway must not include
these headers, and internal headers are never included after
"nm-default.h", as of the first previous point.
- Include all internal headers with quotes instead of angle brackets.
In practice it doesn't matter, because in our public headers we must
include other headers with angle brackets. As we use our public
headers also to compile our interal source files, effectively the
result must be the same. Still do it for consistency.
- Except for <config.h> itself. Include it with angle brackets as suggested by
https://www.gnu.org/software/autoconf/manual/autoconf.html#Configuration-Headers
Factories that overwrite this function are not supposed to chain
up the parent implementation. Thus there is no reason to have
a default implementation and it's clearer to inline it.
A GObject interface, like a class, has two different C types
associated with it; the type of the "class" struct (eg, GObjectClass,
GFileIface), and the type of instances of that class/interface (eg,
GObject, GFile).
NetworkManager was doing this wrong though, and using the same C type
to point to both the interface's class struct and to instances of the
interface. This ends up not actually breaking anything, since for
interface types, the instance type is a non-dereferenceable dummy type
anyway. But it's wrong, since if, eg, NMDeviceFactory is a struct type
containing members "start", "device_added", etc, then you should not
be using an NMDeviceFactory* to point to an object that does not
contain those members.
Fix this by splitting NMDeviceFactory into NMDeviceFactoryInterface
and NMDeviceFactory; by splitting NMConnectionProvider into
NMConnectionProviderInterface and NMConnectionProvider; and by
splitting NMSettingsPlugin into NMSettingsPluginInterface and
NMSettingsPlugin; and then use the right types in the right places.
As a bonus, this also lets us now use G_DEFINE_INTERFACE.
We can dispose the manager before loading the factories:
NetworkManager[20214]: <error> [1439391976.738579] [nm-bus-manager.c:764] nm_bus_manager_start_service(): Could not acquire the NetworkManager service as it is already taken.
NetworkManager[20214]: <error> [1439391976.738617] [main.c:424] main(): failed to start the dbus service.
NetworkManager[20214]: nm_device_factory_manager_for_each_factory: assertion 'factories_by_link' failed
Program received signal SIGTRAP, Trace/breakpoint trap.
g_logv (log_domain=0x5a5028 "NetworkManager", log_level=G_LOG_LEVEL_CRITICAL, format=<optimized out>, args=args@entry=0x7fffffffd750) at gmessages.c:1046
1046 g_private_set (&g_log_depth, GUINT_TO_POINTER (depth));
(gdb) bt
#0 0x00007ffff4a738c3 in g_logv (log_domain=0x5a5028 "NetworkManager", log_level=G_LOG_LEVEL_CRITICAL, format=<optimized out>, args=args@entry=0x7fffffffd750) at gmessages.c:1046
#1 0x00007ffff4a73a3f in g_log (log_domain=log_domain@entry=0x5a5028 "NetworkManager", log_level=log_level@entry=G_LOG_LEVEL_CRITICAL, format=format@entry=0x7ffff4ae173d "%s: assertion '%s' failed")
at gmessages.c:1079
#2 0x00007ffff4a73a79 in g_return_if_fail_warning (log_domain=log_domain@entry=0x5a5028 "NetworkManager", pretty_function=pretty_function@entry=0x5a5420 <__FUNCTION__.26921> "nm_device_factory_manager_for_each_factory", expression=expression@entry=0x5a4f25 "factories_by_link") at gmessages.c:1088
#3 0x00000000004704e4 in nm_device_factory_manager_for_each_factory (callback=callback@entry=0x4d29a0 <_deinit_device_factory>, user_data=user_data@entry=0x8802b0) at devices/nm-device-factory.c:306
#4 0x00000000004d2927 in dispose (object=0x8802b0 [NMManager]) at nm-manager.c:5086
#5 0x00007ffff4d6ea82 in g_object_unref (_object=0x8802b0) at gobject.c:3133
#6 0x00000000004459f6 in main (argc=1, argv=0x7fffffffda88) at main.c:468
(gdb)
Future patches will create devices long before they are backed by
kernel resources, so we need to split NMDevice object creation from
actual setup based on the backing resources.
This patch combines the NMDeviceFactory's new_link() and
create_virtual_device_for_connection() class methods into a single
create_device() method that simply creates an unrealized NMDevice
object; this method is not expected to fail unless the device is
supposed to be ignored. This also means that the NMDevice
'platform-device' property is removed, because a platform link
object may not be available at NMDevice object creation time.
After the device is created, it is then "realized" at some later
time from a platform link (for existing/hardware devices via the
realize() method) or from an NMConnection (for newly created software
devices via the create_and_realize() NMDeviceClass methods).
https://bugzilla.gnome.org/show_bug.cgi?id=737458
nm_device_factory_manager_find_factory_for_link_type()
easily can see a link-type NM_LINK_TYPE_UNKNOWN because
there are many link types that NetworkManager cannot detect.
Just return NULL early.
Fixes: 71bde20c30
Instead of hacky stuff in the Manager, let plugins themselves indicate
which links should be ignored (because they are really child links that
are controlled by a different device that the plugin handles).
Instead of having a bunch of logic in the Manager for determining the
VLAN and Infiniband virtual interface names, move the type-specific
logic into the plugins themselves.
Instead of looping over all plugins and asking each plugin whether it
can handle a link or a connection, have them advertise the link and
connection types they support, and use that when creating new devices.
config.h should be included from every .c file, and it should be
included before any other include. Fix that.
(As a side effect of how I did this, this also changes us to
consistently use "config.h" rather than <config.h>. To the extent that
it matters [which is not much], quotes are more correct anyway, since
we're talking about a file in our own build tree, not a system
include.)
