After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 700996 - Lots of cleanup for realmd code
Lots of cleanup for realmd code
Status: RESOLVED FIXED
Product: gnome-initial-setup
Classification: Applications
Component: general
unspecified
Other Linux
: Normal normal
: ---
Assigned To: GNOME Initial Setup maintainer(s)
GNOME Initial Setup maintainer(s)
Depends on:
Blocks: 701045
 
 
Reported: 2013-05-25 11:34 UTC by Stef Walter
Modified: 2013-05-27 05:40 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
account: Add debug output while joining a domain (6.51 KB, patch)
2013-05-25 11:34 UTC, Stef Walter
committed Details | Review
account: Correctly handle domains which are not joinable (3.32 KB, patch)
2013-05-25 11:34 UTC, Stef Walter
none Details | Review
account: Connect to realmd asynchronously (9.18 KB, patch)
2013-05-25 11:34 UTC, Stef Walter
committed Details | Review
account: Fix memory leak of discovered realms (981 bytes, patch)
2013-05-25 11:34 UTC, Stef Walter
committed Details | Review
accounts: Complete realmd dicsovery on failure (973 bytes, patch)
2013-05-25 11:34 UTC, Stef Walter
committed Details | Review
user-accounts: Tell realmd not to setup system management (1.63 KB, patch)
2013-05-25 11:35 UTC, Stef Walter
committed Details | Review
accounts: When enterprise account disabled, join as admin (4.50 KB, patch)
2013-05-25 11:35 UTC, Stef Walter
committed Details | Review
accounts: Don't have domains twice in the drop down (2.80 KB, patch)
2013-05-25 11:35 UTC, Stef Walter
needs-work Details | Review
account: Set timeout properly on all realm interface proxies (1.28 KB, patch)
2013-05-26 18:59 UTC, Stef Walter
committed Details | Review
account: Correctly handle domains which are not joinable (3.04 KB, patch)
2013-05-26 19:00 UTC, Stef Walter
committed Details | Review
accounts: Don't have domains twice in the drop down (2.78 KB, patch)
2013-05-26 19:02 UTC, Stef Walter
needs-work Details | Review
accounts: Don't have domains twice in the drop down (3.61 KB, patch)
2013-05-26 20:24 UTC, Stef Walter
committed Details | Review

Description Stef Walter 2013-05-25 11:34:00 UTC
Attaching several patches for cleanup of realmd code.
Comment 1 Stef Walter 2013-05-25 11:34:32 UTC
Created attachment 245290 [details] [review]
account: Add debug output while joining a domain
Comment 2 Stef Walter 2013-05-25 11:34:39 UTC
Created attachment 245291 [details] [review]
account: Correctly handle domains which are not joinable
Comment 3 Stef Walter 2013-05-25 11:34:45 UTC
Created attachment 245292 [details] [review]
account: Connect to realmd asynchronously

Even though realmd is pretty responsive for these calls, these are out
of process calls so do them async
Comment 4 Stef Walter 2013-05-25 11:34:52 UTC
Created attachment 245293 [details] [review]
account: Fix memory leak of discovered realms
Comment 5 Stef Walter 2013-05-25 11:34:59 UTC
Created attachment 245294 [details] [review]
accounts: Complete realmd dicsovery on failure
Comment 6 Stef Walter 2013-05-25 11:35:08 UTC
Created attachment 245295 [details] [review]
user-accounts: Tell realmd not to setup system management

The Enterprise Login feature is meant to setup a individiual
domain accounts for use with GNOME, and not manage the system.
realmd now configures more than just logins, so tell realmd
to hold back and not configure a system managed by the domain.

https://bugzilla.gnome.org/show_bug.cgi?id=697910
Comment 7 Stef Walter 2013-05-25 11:35:16 UTC
Created attachment 245296 [details] [review]
accounts: When enterprise account disabled, join as admin

When an enterprise login user account is disabled, or needs to have
its password change, just prompt for the administrator to do the join
to the domain. This is exactly the same fall through as if the user
does not have permission to join a machine to a domain.

https://bugzilla.gnome.org/show_bug.cgi?id=699293
Comment 8 Stef Walter 2013-05-25 11:35:24 UTC
Created attachment 245297 [details] [review]
accounts: Don't have domains twice in the drop down

If we discover domains again, don't add them twice to the drop
down and confuse the user. This is especially important if we
receive two realms from realmd for the same domain for use with
different clients (ie: sssd/winbind). We only want to offer the
first choice

https://bugzilla.gnome.org/show_bug.cgi?id=686397
Comment 9 Jasper St. Pierre (not reading bugmail) 2013-05-25 14:35:02 UTC
Review of attachment 245290 [details] [review]:

OK.
Comment 10 Jasper St. Pierre (not reading bugmail) 2013-05-25 14:35:55 UTC
Review of attachment 245291 [details] [review]:

::: gnome-initial-setup/pages/account/um-realm-manager.c
@@ +104,3 @@
+        for (l = interfaces; l != NULL; l = g_list_next (l))
+                on_interface_added (manager, object, l->data);
+        g_list_free_full (interfaces, g_object_unref);

This seems unrelated?
Comment 11 Jasper St. Pierre (not reading bugmail) 2013-05-25 14:36:56 UTC
Review of attachment 245292 [details] [review]:

OK.
Comment 12 Jasper St. Pierre (not reading bugmail) 2013-05-25 14:37:06 UTC
Review of attachment 245293 [details] [review]:

OK.
Comment 13 Jasper St. Pierre (not reading bugmail) 2013-05-25 14:37:50 UTC
Review of attachment 245294 [details] [review]:

OK.
Comment 14 Jasper St. Pierre (not reading bugmail) 2013-05-25 14:38:39 UTC
Review of attachment 245295 [details] [review]:

OK.
Comment 15 Jasper St. Pierre (not reading bugmail) 2013-05-25 14:39:32 UTC
Review of attachment 245296 [details] [review]:

OK.
Comment 16 Jasper St. Pierre (not reading bugmail) 2013-05-25 14:40:49 UTC
Review of attachment 245297 [details] [review]:

::: gnome-initial-setup/pages/account/gis-account-page.c
@@ +799,3 @@
+  /*
+   * Don't add a second realm if we already have one with this name.
+   * Sometimes realmd returns to realms for the same name, if it has

"two"

@@ +810,3 @@
+    g_free (name);
+    if (match) {
+      g_debug ("ignoring duplicate realm: %s", realm_name);

Isn't this leaking "common"?

You should probably split this out into a separate helper method.
Comment 17 Stef Walter 2013-05-26 18:39:44 UTC
Attachment 245290 [details] pushed as d10d16e - account: Add debug output while joining a domain
Attachment 245292 [details] pushed as 14d6abb - account: Connect to realmd asynchronously
Attachment 245293 [details] pushed as ce39c2a - account: Fix memory leak of discovered realms
Attachment 245294 [details] pushed as 5383989 - accounts: Complete realmd dicsovery on failure
Attachment 245295 [details] pushed as 6ea3daa - user-accounts: Tell realmd not to setup system management
Attachment 245296 [details] pushed as ed7af42 - accounts: When enterprise account disabled, join as admin
Comment 18 Stef Walter 2013-05-26 18:59:08 UTC
Created attachment 245349 [details] [review]
account: Set timeout properly on all realm interface proxies
Comment 19 Stef Walter 2013-05-26 19:00:02 UTC
Created attachment 245350 [details] [review]
account: Correctly handle domains which are not joinable

Updated. Split into two patches.
Comment 20 Stef Walter 2013-05-26 19:02:56 UTC
Created attachment 245351 [details] [review]
accounts: Don't have domains twice in the drop down

Updated, fixed typo and leak. Didn't add helper method since
we only do um_realm_object_get_common() + um_realm_common_get_name()
combo once.
Comment 21 Jasper St. Pierre (not reading bugmail) 2013-05-26 19:16:22 UTC
Review of attachment 245351 [details] [review]:

::: gnome-initial-setup/pages/account/gis-account-page.c
@@ +811,3 @@
+    if (match) {
+      g_debug ("ignoring duplicate realm: %s", realm_name);
+      g_object_unref (common);

Yeah, this is still a bit error prone. Either use a "goto out;" or split this out into a separate "model_contains_realm"
Comment 22 Jasper St. Pierre (not reading bugmail) 2013-05-26 19:17:06 UTC
Review of attachment 245349 [details] [review]:

OK.
Comment 23 Jasper St. Pierre (not reading bugmail) 2013-05-26 19:17:15 UTC
Review of attachment 245350 [details] [review]:

OK.
Comment 24 Stef Walter 2013-05-26 20:13:17 UTC
Attachment 245349 [details] pushed as 3a77768 - account: Set timeout properly on all realm interface proxies
Attachment 245350 [details] pushed as ca14cf2 - account: Correctly handle domains which are not joinable
Comment 25 Stef Walter 2013-05-26 20:24:58 UTC
Created attachment 245358 [details] [review]
accounts: Don't have domains twice in the drop down

Okay, changed to use helper functions. Hopefully that addresses the issue you raised.
Comment 26 Jasper St. Pierre (not reading bugmail) 2013-05-26 21:58:07 UTC
Review of attachment 245358 [details] [review]:

OK.
Comment 27 Stef Walter 2013-05-27 05:39:51 UTC
Attachment 245358 [details] pushed as 939eaa5 - accounts: Don't have domains twice in the drop down