* [Bluez PATCH v2 1/2] plugins/admin: add adapter_remove handler
@ 2021-09-06 6:03 Howard Chung
2021-09-06 6:03 ` [Bluez PATCH v2 2/2] plugins/admin: create admin_policy_settings if not exists Howard Chung
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: Howard Chung @ 2021-09-06 6:03 UTC (permalink / raw)
To: linux-bluetooth, luiz.dentz; +Cc: Yun-Hao Chung, Shyh-In Hwang, Miao-chen Chou
From: Yun-Hao Chung <howardchung@chromium.org>
Currently admin doesn't handle adapter removed callbacks, which causes
interfaces AdminPolicySet1 and AdminPolicyStatus1 not being
unregistered, which in turns causes these interfaces can not be
re-registered once adapter is back.
This adds handler for adapter_remove.
Reviewed-by: Shyh-In Hwang <josephsih@chromium.org>
Reviewed-by: Miao-chen Chou <mcchou@chromium.org>
---
tested with following steps
1. rmmod btusb
2. modprobe btusb
3. read allowlist via bluetoothctl
Changes in v2:
1. Fix make errors
plugins/admin.c | 35 ++++++++++++++++++++++++++++-------
1 file changed, 28 insertions(+), 7 deletions(-)
diff --git a/plugins/admin.c b/plugins/admin.c
index 02fec04568ba..82c00cabdb6b 100644
--- a/plugins/admin.c
+++ b/plugins/admin.c
@@ -85,6 +85,17 @@ static void admin_policy_free(void *data)
g_free(admin_policy);
}
+static void admin_policy_destroy(struct btd_admin_policy *admin_policy)
+{
+ const char *path = adapter_get_path(admin_policy->adapter);
+
+ g_dbus_unregister_interface(dbus_conn, path,
+ ADMIN_POLICY_SET_INTERFACE);
+ g_dbus_unregister_interface(dbus_conn, path,
+ ADMIN_POLICY_STATUS_INTERFACE);
+ admin_policy_free(admin_policy);
+}
+
static bool uuid_match(const void *data, const void *match_data)
{
const bt_uuid_t *uuid = data;
@@ -492,7 +503,7 @@ static int admin_policy_adapter_probe(struct btd_adapter *adapter)
if (!g_dbus_register_interface(dbus_conn, adapter_path,
ADMIN_POLICY_SET_INTERFACE,
admin_policy_adapter_methods, NULL,
- NULL, policy_data, admin_policy_free)) {
+ NULL, policy_data, NULL)) {
btd_error(policy_data->adapter_id,
"Admin Policy Set interface init failed on path %s",
adapter_path);
@@ -506,7 +517,7 @@ static int admin_policy_adapter_probe(struct btd_adapter *adapter)
ADMIN_POLICY_STATUS_INTERFACE,
NULL, NULL,
admin_policy_adapter_properties,
- policy_data, admin_policy_free)) {
+ policy_data, NULL)) {
btd_error(policy_data->adapter_id,
"Admin Policy Status interface init failed on path %s",
adapter_path);
@@ -574,10 +585,24 @@ static void admin_policy_device_removed(struct btd_adapter *adapter,
unregister_device_data(data, NULL);
}
+static void admin_policy_remove(struct btd_adapter *adapter)
+{
+ DBG("");
+
+ queue_foreach(devices, unregister_device_data, NULL);
+ queue_destroy(devices, g_free);
+
+ if (policy_data) {
+ admin_policy_destroy(policy_data);
+ policy_data = NULL;
+ }
+}
+
static struct btd_adapter_driver admin_policy_driver = {
.name = "admin_policy",
.probe = admin_policy_adapter_probe,
.resume = NULL,
+ .remove = admin_policy_remove,
.device_resolved = admin_policy_device_added,
.device_removed = admin_policy_device_removed
};
@@ -597,11 +622,7 @@ static void admin_exit(void)
DBG("");
btd_unregister_adapter_driver(&admin_policy_driver);
- queue_foreach(devices, unregister_device_data, NULL);
- queue_destroy(devices, g_free);
-
- if (policy_data)
- admin_policy_free(policy_data);
+ admin_policy_remove(NULL);
}
BLUETOOTH_PLUGIN_DEFINE(admin, VERSION,
--
2.33.0.153.gba50c8fa24-goog
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [Bluez PATCH v2 2/2] plugins/admin: create admin_policy_settings if not exists
2021-09-06 6:03 [Bluez PATCH v2 1/2] plugins/admin: add adapter_remove handler Howard Chung
@ 2021-09-06 6:03 ` Howard Chung
2021-09-06 6:40 ` [Bluez,v2,1/2] plugins/admin: add adapter_remove handler bluez.test.bot
2021-09-15 12:32 ` [Bluez PATCH v2 1/2] " Yun-hao Chung
2 siblings, 0 replies; 7+ messages in thread
From: Howard Chung @ 2021-09-06 6:03 UTC (permalink / raw)
To: linux-bluetooth, luiz.dentz; +Cc: Yun-Hao Chung, Shyh-In Hwang, Miao-chen Chou
From: Yun-Hao Chung <howardchung@chromium.org>
If admin_policy_settings is not found when loading, we should create one
instead of printing error.
Reviewed-by: Shyh-In Hwang <josephsih@chromium.org>
Reviewed-by: Miao-chen Chou <mcchou@chromium.org>
---
This patch has been tested with following steps:
rm /var/lib/bluetooth/admin_policy_settings and restart bluetoothd.
Check if the file is created.
(no changes since v1)
plugins/admin.c | 10 +++-------
1 file changed, 3 insertions(+), 7 deletions(-)
diff --git a/plugins/admin.c b/plugins/admin.c
index 82c00cabdb6b..8390f3c32bbd 100644
--- a/plugins/admin.c
+++ b/plugins/admin.c
@@ -67,7 +67,7 @@ static struct btd_admin_policy *admin_policy_new(struct btd_adapter *adapter)
admin_policy->adapter = adapter;
admin_policy->adapter_id = btd_adapter_get_index(adapter);
- admin_policy->service_allowlist = NULL;
+ admin_policy->service_allowlist = queue_new();
return admin_policy;
}
@@ -335,12 +335,8 @@ static void load_policy_settings(struct btd_admin_policy *admin_policy)
char *filename = ADMIN_POLICY_STORAGE;
struct stat st;
- if (stat(filename, &st) < 0) {
- btd_error(admin_policy->adapter_id,
- "Failed to get file %s information",
- filename);
- return;
- }
+ if (stat(filename, &st) < 0)
+ store_policy_settings(policy_data);
key_file = g_key_file_new();
--
2.33.0.153.gba50c8fa24-goog
^ permalink raw reply related [flat|nested] 7+ messages in thread
* RE: [Bluez,v2,1/2] plugins/admin: add adapter_remove handler
2021-09-06 6:03 [Bluez PATCH v2 1/2] plugins/admin: add adapter_remove handler Howard Chung
2021-09-06 6:03 ` [Bluez PATCH v2 2/2] plugins/admin: create admin_policy_settings if not exists Howard Chung
@ 2021-09-06 6:40 ` bluez.test.bot
2021-09-15 12:32 ` [Bluez PATCH v2 1/2] " Yun-hao Chung
2 siblings, 0 replies; 7+ messages in thread
From: bluez.test.bot @ 2021-09-06 6:40 UTC (permalink / raw)
To: linux-bluetooth, howardchung
[-- Attachment #1: Type: text/plain, Size: 1953 bytes --]
This is automated email and please do not reply to this email!
Dear submitter,
Thank you for submitting the patches to the linux bluetooth mailing list.
This is a CI test results with your patch series:
PW Link:https://patchwork.kernel.org/project/bluetooth/list/?series=542331
---Test result---
Test Summary:
CheckPatch PASS 0.46 seconds
GitLint PASS 0.20 seconds
Prep - Setup ELL PASS 40.61 seconds
Build - Prep PASS 0.09 seconds
Build - Configure PASS 7.15 seconds
Build - Make PASS 177.46 seconds
Make Check PASS 9.32 seconds
Make Distcheck PASS 214.73 seconds
Build w/ext ELL - Configure PASS 7.19 seconds
Build w/ext ELL - Make PASS 167.46 seconds
Details
##############################
Test: CheckPatch - PASS
Desc: Run checkpatch.pl script with rule in .checkpatch.conf
##############################
Test: GitLint - PASS
Desc: Run gitlint with rule in .gitlint
##############################
Test: Prep - Setup ELL - PASS
Desc: Clone, build, and install ELL
##############################
Test: Build - Prep - PASS
Desc: Prepare environment for build
##############################
Test: Build - Configure - PASS
Desc: Configure the BlueZ source tree
##############################
Test: Build - Make - PASS
Desc: Build the BlueZ source tree
##############################
Test: Make Check - PASS
Desc: Run 'make check'
##############################
Test: Make Distcheck - PASS
Desc: Run distcheck to check the distribution
##############################
Test: Build w/ext ELL - Configure - PASS
Desc: Configure BlueZ source with '--enable-external-ell' configuration
##############################
Test: Build w/ext ELL - Make - PASS
Desc: Build BlueZ source with '--enable-external-ell' configuration
---
Regards,
Linux Bluetooth
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Bluez PATCH v2 1/2] plugins/admin: add adapter_remove handler
2021-09-06 6:03 [Bluez PATCH v2 1/2] plugins/admin: add adapter_remove handler Howard Chung
2021-09-06 6:03 ` [Bluez PATCH v2 2/2] plugins/admin: create admin_policy_settings if not exists Howard Chung
2021-09-06 6:40 ` [Bluez,v2,1/2] plugins/admin: add adapter_remove handler bluez.test.bot
@ 2021-09-15 12:32 ` Yun-hao Chung
2021-09-15 23:40 ` Luiz Augusto von Dentz
2 siblings, 1 reply; 7+ messages in thread
From: Yun-hao Chung @ 2021-09-15 12:32 UTC (permalink / raw)
To: Bluez mailing list, Luiz Augusto von Dentz
Gentle ping. Thanks.
On Mon, Sep 6, 2021 at 2:03 PM Howard Chung <howardchung@google.com> wrote:
>
> From: Yun-Hao Chung <howardchung@chromium.org>
>
> Currently admin doesn't handle adapter removed callbacks, which causes
> interfaces AdminPolicySet1 and AdminPolicyStatus1 not being
> unregistered, which in turns causes these interfaces can not be
> re-registered once adapter is back.
>
> This adds handler for adapter_remove.
>
> Reviewed-by: Shyh-In Hwang <josephsih@chromium.org>
> Reviewed-by: Miao-chen Chou <mcchou@chromium.org>
> ---
> tested with following steps
> 1. rmmod btusb
> 2. modprobe btusb
> 3. read allowlist via bluetoothctl
>
> Changes in v2:
> 1. Fix make errors
>
> plugins/admin.c | 35 ++++++++++++++++++++++++++++-------
> 1 file changed, 28 insertions(+), 7 deletions(-)
>
> diff --git a/plugins/admin.c b/plugins/admin.c
> index 02fec04568ba..82c00cabdb6b 100644
> --- a/plugins/admin.c
> +++ b/plugins/admin.c
> @@ -85,6 +85,17 @@ static void admin_policy_free(void *data)
> g_free(admin_policy);
> }
>
> +static void admin_policy_destroy(struct btd_admin_policy *admin_policy)
> +{
> + const char *path = adapter_get_path(admin_policy->adapter);
> +
> + g_dbus_unregister_interface(dbus_conn, path,
> + ADMIN_POLICY_SET_INTERFACE);
> + g_dbus_unregister_interface(dbus_conn, path,
> + ADMIN_POLICY_STATUS_INTERFACE);
> + admin_policy_free(admin_policy);
> +}
> +
> static bool uuid_match(const void *data, const void *match_data)
> {
> const bt_uuid_t *uuid = data;
> @@ -492,7 +503,7 @@ static int admin_policy_adapter_probe(struct btd_adapter *adapter)
> if (!g_dbus_register_interface(dbus_conn, adapter_path,
> ADMIN_POLICY_SET_INTERFACE,
> admin_policy_adapter_methods, NULL,
> - NULL, policy_data, admin_policy_free)) {
> + NULL, policy_data, NULL)) {
> btd_error(policy_data->adapter_id,
> "Admin Policy Set interface init failed on path %s",
> adapter_path);
> @@ -506,7 +517,7 @@ static int admin_policy_adapter_probe(struct btd_adapter *adapter)
> ADMIN_POLICY_STATUS_INTERFACE,
> NULL, NULL,
> admin_policy_adapter_properties,
> - policy_data, admin_policy_free)) {
> + policy_data, NULL)) {
> btd_error(policy_data->adapter_id,
> "Admin Policy Status interface init failed on path %s",
> adapter_path);
> @@ -574,10 +585,24 @@ static void admin_policy_device_removed(struct btd_adapter *adapter,
> unregister_device_data(data, NULL);
> }
>
> +static void admin_policy_remove(struct btd_adapter *adapter)
> +{
> + DBG("");
> +
> + queue_foreach(devices, unregister_device_data, NULL);
> + queue_destroy(devices, g_free);
> +
> + if (policy_data) {
> + admin_policy_destroy(policy_data);
> + policy_data = NULL;
> + }
> +}
> +
> static struct btd_adapter_driver admin_policy_driver = {
> .name = "admin_policy",
> .probe = admin_policy_adapter_probe,
> .resume = NULL,
> + .remove = admin_policy_remove,
> .device_resolved = admin_policy_device_added,
> .device_removed = admin_policy_device_removed
> };
> @@ -597,11 +622,7 @@ static void admin_exit(void)
> DBG("");
>
> btd_unregister_adapter_driver(&admin_policy_driver);
> - queue_foreach(devices, unregister_device_data, NULL);
> - queue_destroy(devices, g_free);
> -
> - if (policy_data)
> - admin_policy_free(policy_data);
> + admin_policy_remove(NULL);
> }
>
> BLUETOOTH_PLUGIN_DEFINE(admin, VERSION,
> --
> 2.33.0.153.gba50c8fa24-goog
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Bluez PATCH v2 1/2] plugins/admin: add adapter_remove handler
2021-09-15 12:32 ` [Bluez PATCH v2 1/2] " Yun-hao Chung
@ 2021-09-15 23:40 ` Luiz Augusto von Dentz
2021-09-16 23:50 ` Luiz Augusto von Dentz
0 siblings, 1 reply; 7+ messages in thread
From: Luiz Augusto von Dentz @ 2021-09-15 23:40 UTC (permalink / raw)
To: Yun-hao Chung; +Cc: Bluez mailing list
Hi Howard,
On Wed, Sep 15, 2021 at 5:32 AM Yun-hao Chung <howardchung@google.com> wrote:
>
> Gentle ping. Thanks.
>
>
> On Mon, Sep 6, 2021 at 2:03 PM Howard Chung <howardchung@google.com> wrote:
> >
> > From: Yun-Hao Chung <howardchung@chromium.org>
> >
> > Currently admin doesn't handle adapter removed callbacks, which causes
> > interfaces AdminPolicySet1 and AdminPolicyStatus1 not being
> > unregistered, which in turns causes these interfaces can not be
> > re-registered once adapter is back.
> >
> > This adds handler for adapter_remove.
> >
> > Reviewed-by: Shyh-In Hwang <josephsih@chromium.org>
> > Reviewed-by: Miao-chen Chou <mcchou@chromium.org>
> > ---
> > tested with following steps
> > 1. rmmod btusb
> > 2. modprobe btusb
> > 3. read allowlist via bluetoothctl
> >
> > Changes in v2:
> > 1. Fix make errors
> >
> > plugins/admin.c | 35 ++++++++++++++++++++++++++++-------
> > 1 file changed, 28 insertions(+), 7 deletions(-)
> >
> > diff --git a/plugins/admin.c b/plugins/admin.c
> > index 02fec04568ba..82c00cabdb6b 100644
> > --- a/plugins/admin.c
> > +++ b/plugins/admin.c
> > @@ -85,6 +85,17 @@ static void admin_policy_free(void *data)
> > g_free(admin_policy);
> > }
> >
> > +static void admin_policy_destroy(struct btd_admin_policy *admin_policy)
> > +{
> > + const char *path = adapter_get_path(admin_policy->adapter);
> > +
> > + g_dbus_unregister_interface(dbus_conn, path,
> > + ADMIN_POLICY_SET_INTERFACE);
> > + g_dbus_unregister_interface(dbus_conn, path,
> > + ADMIN_POLICY_STATUS_INTERFACE);
> > + admin_policy_free(admin_policy);
> > +}
> > +
> > static bool uuid_match(const void *data, const void *match_data)
> > {
> > const bt_uuid_t *uuid = data;
> > @@ -492,7 +503,7 @@ static int admin_policy_adapter_probe(struct btd_adapter *adapter)
> > if (!g_dbus_register_interface(dbus_conn, adapter_path,
> > ADMIN_POLICY_SET_INTERFACE,
> > admin_policy_adapter_methods, NULL,
> > - NULL, policy_data, admin_policy_free)) {
> > + NULL, policy_data, NULL)) {
> > btd_error(policy_data->adapter_id,
> > "Admin Policy Set interface init failed on path %s",
> > adapter_path);
> > @@ -506,7 +517,7 @@ static int admin_policy_adapter_probe(struct btd_adapter *adapter)
> > ADMIN_POLICY_STATUS_INTERFACE,
> > NULL, NULL,
> > admin_policy_adapter_properties,
> > - policy_data, admin_policy_free)) {
> > + policy_data, NULL)) {
> > btd_error(policy_data->adapter_id,
> > "Admin Policy Status interface init failed on path %s",
> > adapter_path);
> > @@ -574,10 +585,24 @@ static void admin_policy_device_removed(struct btd_adapter *adapter,
> > unregister_device_data(data, NULL);
> > }
> >
> > +static void admin_policy_remove(struct btd_adapter *adapter)
> > +{
> > + DBG("");
> > +
> > + queue_foreach(devices, unregister_device_data, NULL);
> > + queue_destroy(devices, g_free);
> > +
> > + if (policy_data) {
> > + admin_policy_destroy(policy_data);
> > + policy_data = NULL;
> > + }
> > +}
> > +
> > static struct btd_adapter_driver admin_policy_driver = {
> > .name = "admin_policy",
> > .probe = admin_policy_adapter_probe,
> > .resume = NULL,
> > + .remove = admin_policy_remove,
> > .device_resolved = admin_policy_device_added,
> > .device_removed = admin_policy_device_removed
> > };
> > @@ -597,11 +622,7 @@ static void admin_exit(void)
> > DBG("");
> >
> > btd_unregister_adapter_driver(&admin_policy_driver);
> > - queue_foreach(devices, unregister_device_data, NULL);
> > - queue_destroy(devices, g_free);
> > -
> > - if (policy_data)
> > - admin_policy_free(policy_data);
> > + admin_policy_remove(NULL);
> > }
> >
> > BLUETOOTH_PLUGIN_DEFINE(admin, VERSION,
> > --
> > 2.33.0.153.gba50c8fa24-goog
> >
Applied, thanks.
--
Luiz Augusto von Dentz
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Bluez PATCH v2 1/2] plugins/admin: add adapter_remove handler
2021-09-15 23:40 ` Luiz Augusto von Dentz
@ 2021-09-16 23:50 ` Luiz Augusto von Dentz
2021-09-17 3:43 ` Yun-hao Chung
0 siblings, 1 reply; 7+ messages in thread
From: Luiz Augusto von Dentz @ 2021-09-16 23:50 UTC (permalink / raw)
To: Yun-hao Chung; +Cc: Bluez mailing list
Hi Howard,
On Wed, Sep 15, 2021 at 4:40 PM Luiz Augusto von Dentz
<luiz.dentz@gmail.com> wrote:
>
> Hi Howard,
>
> On Wed, Sep 15, 2021 at 5:32 AM Yun-hao Chung <howardchung@google.com> wrote:
> >
> > Gentle ping. Thanks.
> >
> >
> > On Mon, Sep 6, 2021 at 2:03 PM Howard Chung <howardchung@google.com> wrote:
> > >
> > > From: Yun-Hao Chung <howardchung@chromium.org>
> > >
> > > Currently admin doesn't handle adapter removed callbacks, which causes
> > > interfaces AdminPolicySet1 and AdminPolicyStatus1 not being
> > > unregistered, which in turns causes these interfaces can not be
> > > re-registered once adapter is back.
> > >
> > > This adds handler for adapter_remove.
> > >
> > > Reviewed-by: Shyh-In Hwang <josephsih@chromium.org>
> > > Reviewed-by: Miao-chen Chou <mcchou@chromium.org>
> > > ---
> > > tested with following steps
> > > 1. rmmod btusb
> > > 2. modprobe btusb
> > > 3. read allowlist via bluetoothctl
> > >
> > > Changes in v2:
> > > 1. Fix make errors
> > >
> > > plugins/admin.c | 35 ++++++++++++++++++++++++++++-------
> > > 1 file changed, 28 insertions(+), 7 deletions(-)
> > >
> > > diff --git a/plugins/admin.c b/plugins/admin.c
> > > index 02fec04568ba..82c00cabdb6b 100644
> > > --- a/plugins/admin.c
> > > +++ b/plugins/admin.c
> > > @@ -85,6 +85,17 @@ static void admin_policy_free(void *data)
> > > g_free(admin_policy);
> > > }
> > >
> > > +static void admin_policy_destroy(struct btd_admin_policy *admin_policy)
> > > +{
> > > + const char *path = adapter_get_path(admin_policy->adapter);
> > > +
> > > + g_dbus_unregister_interface(dbus_conn, path,
> > > + ADMIN_POLICY_SET_INTERFACE);
> > > + g_dbus_unregister_interface(dbus_conn, path,
> > > + ADMIN_POLICY_STATUS_INTERFACE);
> > > + admin_policy_free(admin_policy);
> > > +}
> > > +
> > > static bool uuid_match(const void *data, const void *match_data)
> > > {
> > > const bt_uuid_t *uuid = data;
> > > @@ -492,7 +503,7 @@ static int admin_policy_adapter_probe(struct btd_adapter *adapter)
> > > if (!g_dbus_register_interface(dbus_conn, adapter_path,
> > > ADMIN_POLICY_SET_INTERFACE,
> > > admin_policy_adapter_methods, NULL,
> > > - NULL, policy_data, admin_policy_free)) {
> > > + NULL, policy_data, NULL)) {
> > > btd_error(policy_data->adapter_id,
> > > "Admin Policy Set interface init failed on path %s",
> > > adapter_path);
> > > @@ -506,7 +517,7 @@ static int admin_policy_adapter_probe(struct btd_adapter *adapter)
> > > ADMIN_POLICY_STATUS_INTERFACE,
> > > NULL, NULL,
> > > admin_policy_adapter_properties,
> > > - policy_data, admin_policy_free)) {
> > > + policy_data, NULL)) {
> > > btd_error(policy_data->adapter_id,
> > > "Admin Policy Status interface init failed on path %s",
> > > adapter_path);
> > > @@ -574,10 +585,24 @@ static void admin_policy_device_removed(struct btd_adapter *adapter,
> > > unregister_device_data(data, NULL);
> > > }
> > >
> > > +static void admin_policy_remove(struct btd_adapter *adapter)
> > > +{
> > > + DBG("");
> > > +
> > > + queue_foreach(devices, unregister_device_data, NULL);
> > > + queue_destroy(devices, g_free);
> > > +
> > > + if (policy_data) {
> > > + admin_policy_destroy(policy_data);
> > > + policy_data = NULL;
> > > + }
> > > +}
> > > +
> > > static struct btd_adapter_driver admin_policy_driver = {
> > > .name = "admin_policy",
> > > .probe = admin_policy_adapter_probe,
> > > .resume = NULL,
> > > + .remove = admin_policy_remove,
> > > .device_resolved = admin_policy_device_added,
> > > .device_removed = admin_policy_device_removed
> > > };
> > > @@ -597,11 +622,7 @@ static void admin_exit(void)
> > > DBG("");
> > >
> > > btd_unregister_adapter_driver(&admin_policy_driver);
> > > - queue_foreach(devices, unregister_device_data, NULL);
> > > - queue_destroy(devices, g_free);
> > > -
> > > - if (policy_data)
> > > - admin_policy_free(policy_data);
> > > + admin_policy_remove(NULL);
> > > }
> > >
> > > BLUETOOTH_PLUGIN_DEFINE(admin, VERSION,
> > > --
> > > 2.33.0.153.gba50c8fa24-goog
> > >
>
> Applied, thanks.
>
>
> --
There are actually some problems with these admin plugin:
https://patchwork.kernel.org/project/bluetooth/patch/20210916223825.276530-1-luiz.dentz@gmail.com/
https://patchwork.kernel.org/project/bluetooth/patch/20210916223825.276530-2-luiz.dentz@gmail.com/
There is also some assumption that there could be only one
policy_data, when in fact there could be multiple btd_adapter in the
system so having admin plugin in a system with multiple adapters might
lead to various problems.
--
Luiz Augusto von Dentz
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Bluez PATCH v2 1/2] plugins/admin: add adapter_remove handler
2021-09-16 23:50 ` Luiz Augusto von Dentz
@ 2021-09-17 3:43 ` Yun-hao Chung
0 siblings, 0 replies; 7+ messages in thread
From: Yun-hao Chung @ 2021-09-17 3:43 UTC (permalink / raw)
To: Luiz Augusto von Dentz; +Cc: Bluez mailing list
Thanks for catching and delivering the fixes.
I didn't consider the multiple adapters case, and I will send out
another patch to fix it.
On Fri, Sep 17, 2021 at 7:50 AM Luiz Augusto von Dentz
<luiz.dentz@gmail.com> wrote:
>
> Hi Howard,
>
> On Wed, Sep 15, 2021 at 4:40 PM Luiz Augusto von Dentz
> <luiz.dentz@gmail.com> wrote:
> >
> > Hi Howard,
> >
> > On Wed, Sep 15, 2021 at 5:32 AM Yun-hao Chung <howardchung@google.com> wrote:
> > >
> > > Gentle ping. Thanks.
> > >
> > >
> > > On Mon, Sep 6, 2021 at 2:03 PM Howard Chung <howardchung@google.com> wrote:
> > > >
> > > > From: Yun-Hao Chung <howardchung@chromium.org>
> > > >
> > > > Currently admin doesn't handle adapter removed callbacks, which causes
> > > > interfaces AdminPolicySet1 and AdminPolicyStatus1 not being
> > > > unregistered, which in turns causes these interfaces can not be
> > > > re-registered once adapter is back.
> > > >
> > > > This adds handler for adapter_remove.
> > > >
> > > > Reviewed-by: Shyh-In Hwang <josephsih@chromium.org>
> > > > Reviewed-by: Miao-chen Chou <mcchou@chromium.org>
> > > > ---
> > > > tested with following steps
> > > > 1. rmmod btusb
> > > > 2. modprobe btusb
> > > > 3. read allowlist via bluetoothctl
> > > >
> > > > Changes in v2:
> > > > 1. Fix make errors
> > > >
> > > > plugins/admin.c | 35 ++++++++++++++++++++++++++++-------
> > > > 1 file changed, 28 insertions(+), 7 deletions(-)
> > > >
> > > > diff --git a/plugins/admin.c b/plugins/admin.c
> > > > index 02fec04568ba..82c00cabdb6b 100644
> > > > --- a/plugins/admin.c
> > > > +++ b/plugins/admin.c
> > > > @@ -85,6 +85,17 @@ static void admin_policy_free(void *data)
> > > > g_free(admin_policy);
> > > > }
> > > >
> > > > +static void admin_policy_destroy(struct btd_admin_policy *admin_policy)
> > > > +{
> > > > + const char *path = adapter_get_path(admin_policy->adapter);
> > > > +
> > > > + g_dbus_unregister_interface(dbus_conn, path,
> > > > + ADMIN_POLICY_SET_INTERFACE);
> > > > + g_dbus_unregister_interface(dbus_conn, path,
> > > > + ADMIN_POLICY_STATUS_INTERFACE);
> > > > + admin_policy_free(admin_policy);
> > > > +}
> > > > +
> > > > static bool uuid_match(const void *data, const void *match_data)
> > > > {
> > > > const bt_uuid_t *uuid = data;
> > > > @@ -492,7 +503,7 @@ static int admin_policy_adapter_probe(struct btd_adapter *adapter)
> > > > if (!g_dbus_register_interface(dbus_conn, adapter_path,
> > > > ADMIN_POLICY_SET_INTERFACE,
> > > > admin_policy_adapter_methods, NULL,
> > > > - NULL, policy_data, admin_policy_free)) {
> > > > + NULL, policy_data, NULL)) {
> > > > btd_error(policy_data->adapter_id,
> > > > "Admin Policy Set interface init failed on path %s",
> > > > adapter_path);
> > > > @@ -506,7 +517,7 @@ static int admin_policy_adapter_probe(struct btd_adapter *adapter)
> > > > ADMIN_POLICY_STATUS_INTERFACE,
> > > > NULL, NULL,
> > > > admin_policy_adapter_properties,
> > > > - policy_data, admin_policy_free)) {
> > > > + policy_data, NULL)) {
> > > > btd_error(policy_data->adapter_id,
> > > > "Admin Policy Status interface init failed on path %s",
> > > > adapter_path);
> > > > @@ -574,10 +585,24 @@ static void admin_policy_device_removed(struct btd_adapter *adapter,
> > > > unregister_device_data(data, NULL);
> > > > }
> > > >
> > > > +static void admin_policy_remove(struct btd_adapter *adapter)
> > > > +{
> > > > + DBG("");
> > > > +
> > > > + queue_foreach(devices, unregister_device_data, NULL);
> > > > + queue_destroy(devices, g_free);
> > > > +
> > > > + if (policy_data) {
> > > > + admin_policy_destroy(policy_data);
> > > > + policy_data = NULL;
> > > > + }
> > > > +}
> > > > +
> > > > static struct btd_adapter_driver admin_policy_driver = {
> > > > .name = "admin_policy",
> > > > .probe = admin_policy_adapter_probe,
> > > > .resume = NULL,
> > > > + .remove = admin_policy_remove,
> > > > .device_resolved = admin_policy_device_added,
> > > > .device_removed = admin_policy_device_removed
> > > > };
> > > > @@ -597,11 +622,7 @@ static void admin_exit(void)
> > > > DBG("");
> > > >
> > > > btd_unregister_adapter_driver(&admin_policy_driver);
> > > > - queue_foreach(devices, unregister_device_data, NULL);
> > > > - queue_destroy(devices, g_free);
> > > > -
> > > > - if (policy_data)
> > > > - admin_policy_free(policy_data);
> > > > + admin_policy_remove(NULL);
> > > > }
> > > >
> > > > BLUETOOTH_PLUGIN_DEFINE(admin, VERSION,
> > > > --
> > > > 2.33.0.153.gba50c8fa24-goog
> > > >
> >
> > Applied, thanks.
> >
> >
> > --
>
> There are actually some problems with these admin plugin:
>
> https://patchwork.kernel.org/project/bluetooth/patch/20210916223825.276530-1-luiz.dentz@gmail.com/
> https://patchwork.kernel.org/project/bluetooth/patch/20210916223825.276530-2-luiz.dentz@gmail.com/
>
> There is also some assumption that there could be only one
> policy_data, when in fact there could be multiple btd_adapter in the
> system so having admin plugin in a system with multiple adapters might
> lead to various problems.
>
>
> --
> Luiz Augusto von Dentz
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2021-09-17 3:43 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-06 6:03 [Bluez PATCH v2 1/2] plugins/admin: add adapter_remove handler Howard Chung
2021-09-06 6:03 ` [Bluez PATCH v2 2/2] plugins/admin: create admin_policy_settings if not exists Howard Chung
2021-09-06 6:40 ` [Bluez,v2,1/2] plugins/admin: add adapter_remove handler bluez.test.bot
2021-09-15 12:32 ` [Bluez PATCH v2 1/2] " Yun-hao Chung
2021-09-15 23:40 ` Luiz Augusto von Dentz
2021-09-16 23:50 ` Luiz Augusto von Dentz
2021-09-17 3:43 ` Yun-hao Chung
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.