Re: [OpenWrt-Devel] [PATCH fstools] blockd: don't unmount device when removing it from the list
On 2018-12-06 08:24, John Crispin wrote: On 05/12/2018 13:39, Rafał Miłecki wrote: On 2018-12-05 13:31, Paul Oranje wrote: Op 4 dec. 2018, om 12:32 heeft Rafał Miłecki het volgende geschreven: From: Rafał Miłecki Device gets removed from the list (vlist_delete()) when block calls "hotplug" method of blockd using ubus. Right after that block unmounts that device on its own. blockd shouldn't care about unmounting on its own for following reasons: 1) To avoid code/behavior duplication with blockThe chicken or the eggThe chicken or the egg 2) To keep behavior consistent with mounting (blockd doesn't mount) 3) To allow implementing more features in block (e.g. hotplug.d events) The design should be to: 1) Have block handle (un)mounting 2) Use blockd for providing devices/mounts state (using ubus) 3) Have blockd handle autofs and call block when needed Can this cause a transition into a state where a device is (still) mounted but removed from the list, and if so, is that a valid, an admissible state ? Shouldn't block be required to first unmount before calling blockd's hotplug entry ? The chicken or the egg? ;) We don't have any mutex/semaphore mechanism in place right now. So unless that gets implemented, we have to chooce what's better. I believe the correct order would be to: 1) Stop reporting mounted device 2) Notify all users about unmounting (hotplug.d event I work on) 3) Unmount That's the safest way that will stop applications from trying to access device due to blockd incorrectly reporting it as mounted. please update the description before pushing Acked-by: John Crispin Pushed with updated commit message https://git.openwrt.org/?p=project/fstools.git;a=commit;h=f6a96865da9162f45ea1d482c794fe14b26ecfe1 ___ openwrt-devel mailing list openwrt-devel@lists.openwrt.org https://lists.openwrt.org/mailman/listinfo/openwrt-devel
Re: [OpenWrt-Devel] [PATCH fstools] blockd: don't unmount device when removing it from the list
On 05/12/2018 13:39, Rafał Miłecki wrote: On 2018-12-05 13:31, Paul Oranje wrote: Op 4 dec. 2018, om 12:32 heeft Rafał Miłecki het volgende geschreven: From: Rafał Miłecki Device gets removed from the list (vlist_delete()) when block calls "hotplug" method of blockd using ubus. Right after that block unmounts that device on its own. blockd shouldn't care about unmounting on its own for following reasons: 1) To avoid code/behavior duplication with blockThe chicken or the eggThe chicken or the egg 2) To keep behavior consistent with mounting (blockd doesn't mount) 3) To allow implementing more features in block (e.g. hotplug.d events) The design should be to: 1) Have block handle (un)mounting 2) Use blockd for providing devices/mounts state (using ubus) 3) Have blockd handle autofs and call block when needed Can this cause a transition into a state where a device is (still) mounted but removed from the list, and if so, is that a valid, an admissible state ? Shouldn't block be required to first unmount before calling blockd's hotplug entry ? The chicken or the egg? ;) We don't have any mutex/semaphore mechanism in place right now. So unless that gets implemented, we have to chooce what's better. I believe the correct order would be to: 1) Stop reporting mounted device 2) Notify all users about unmounting (hotplug.d event I work on) 3) Unmount That's the safest way that will stop applications from trying to access device due to blockd incorrectly reporting it as mounted. please update the description before pushing Acked-by: John Crispin ___ openwrt-devel mailing list openwrt-devel@lists.openwrt.org https://lists.openwrt.org/mailman/listinfo/openwrt-devel
Re: [OpenWrt-Devel] [PATCH fstools] blockd: don't unmount device when removing it from the list
On 2018-12-05 13:31, Paul Oranje wrote: Op 4 dec. 2018, om 12:32 heeft Rafał Miłecki het volgende geschreven: From: Rafał Miłecki Device gets removed from the list (vlist_delete()) when block calls "hotplug" method of blockd using ubus. Right after that block unmounts that device on its own. blockd shouldn't care about unmounting on its own for following reasons: 1) To avoid code/behavior duplication with blockThe chicken or the eggThe chicken or the egg 2) To keep behavior consistent with mounting (blockd doesn't mount) 3) To allow implementing more features in block (e.g. hotplug.d events) The design should be to: 1) Have block handle (un)mounting 2) Use blockd for providing devices/mounts state (using ubus) 3) Have blockd handle autofs and call block when needed Can this cause a transition into a state where a device is (still) mounted but removed from the list, and if so, is that a valid, an admissible state ? Shouldn't block be required to first unmount before calling blockd's hotplug entry ? The chicken or the egg? ;) We don't have any mutex/semaphore mechanism in place right now. So unless that gets implemented, we have to chooce what's better. I believe the correct order would be to: 1) Stop reporting mounted device 2) Notify all users about unmounting (hotplug.d event I work on) 3) Unmount That's the safest way that will stop applications from trying to access device due to blockd incorrectly reporting it as mounted. ___ openwrt-devel mailing list openwrt-devel@lists.openwrt.org https://lists.openwrt.org/mailman/listinfo/openwrt-devel
Re: [OpenWrt-Devel] [PATCH fstools] blockd: don't unmount device when removing it from the list
> Op 4 dec. 2018, om 12:32 heeft Rafał Miłecki het volgende
> geschreven:
>
> From: Rafał Miłecki
>
> Device gets removed from the list (vlist_delete()) when block calls
> "hotplug" method of blockd using ubus. Right after that block unmounts
> that device on its own.
>
> blockd shouldn't care about unmounting on its own for following reasons:
> 1) To avoid code/behavior duplication with block
> 2) To keep behavior consistent with mounting (blockd doesn't mount)
> 3) To allow implementing more features in block (e.g. hotplug.d events)
>
> The design should be to:
> 1) Have block handle (un)mounting
> 2) Use blockd for providing devices/mounts state (using ubus)
> 3) Have blockd handle autofs and call block when needed
Can this cause a transition into a state where a device is (still) mounted but
removed from the list, and if so, is that a valid, an admissible state ?
Shouldn't block be required to first unmount before calling blockd's hotplug
entry ?
>
> Signed-off-by: Rafał Miłecki
> ---
> blockd.c | 26 ++
> 1 file changed, 2 insertions(+), 24 deletions(-)
>
> diff --git a/blockd.c b/blockd.c
> index a5da32c..1379635 100644
> --- a/blockd.c
> +++ b/blockd.c
> @@ -112,34 +112,12 @@ static void
> device_free(struct device *device)
> {
> struct blob_attr *data[__MOUNT_MAX];
> - char *target = NULL;
> - char *path = NULL, _path[64], *mp;
>
> blobmsg_parse(mount_policy, __MOUNT_MAX, data,
> blob_data(device->msg), blob_len(device->msg));
>
> - if (data[MOUNT_AUTOFS]) {
> - target = device->target;
> - snprintf(_path, sizeof(_path), "/tmp/run/blockd/%s",
> - blobmsg_get_string(data[MOUNT_DEVICE]));
> - path = _path;
> - } else {
> - path = target = device->target;
> - }
> -
> - mp = _find_mount_point(device->name);
> - if (path && mp)
> - if (umount2(path, MNT_DETACH))
> - ULOG_ERR("failed to unmount %s\n", path);
> - free(mp);
> -
> - if (!target)
> - return;
> -
> - if (data[MOUNT_AUTOFS])
> - unlink(target);
> - else
> - rmdir(target);
> + if (data[MOUNT_AUTOFS] && device->target)
> + unlink(device->target);
> }
>
> static void
> --
> 2.13.7
>
>
> ___
> openwrt-devel mailing list
> openwrt-devel@lists.openwrt.org
> https://lists.openwrt.org/mailman/listinfo/openwrt-devel
--
Paul Oranje
M +31 6 2127 8389
T +31 20 494 1306
Achterlaan 20, 1027 AK Zunderdorp
___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel
[OpenWrt-Devel] [PATCH fstools] blockd: don't unmount device when removing it from the list
From: Rafał Miłecki
Device gets removed from the list (vlist_delete()) when block calls
"hotplug" method of blockd using ubus. Right after that block unmounts
that device on its own.
blockd shouldn't care about unmounting on its own for following reasons:
1) To avoid code/behavior duplication with block
2) To keep behavior consistent with mounting (blockd doesn't mount)
3) To allow implementing more features in block (e.g. hotplug.d events)
The design should be to:
1) Have block handle (un)mounting
2) Use blockd for providing devices/mounts state (using ubus)
3) Have blockd handle autofs and call block when needed
Signed-off-by: Rafał Miłecki
---
blockd.c | 26 ++
1 file changed, 2 insertions(+), 24 deletions(-)
diff --git a/blockd.c b/blockd.c
index a5da32c..1379635 100644
--- a/blockd.c
+++ b/blockd.c
@@ -112,34 +112,12 @@ static void
device_free(struct device *device)
{
struct blob_attr *data[__MOUNT_MAX];
- char *target = NULL;
- char *path = NULL, _path[64], *mp;
blobmsg_parse(mount_policy, __MOUNT_MAX, data,
blob_data(device->msg), blob_len(device->msg));
- if (data[MOUNT_AUTOFS]) {
- target = device->target;
- snprintf(_path, sizeof(_path), "/tmp/run/blockd/%s",
-blobmsg_get_string(data[MOUNT_DEVICE]));
- path = _path;
- } else {
- path = target = device->target;
- }
-
- mp = _find_mount_point(device->name);
- if (path && mp)
- if (umount2(path, MNT_DETACH))
- ULOG_ERR("failed to unmount %s\n", path);
- free(mp);
-
- if (!target)
- return;
-
- if (data[MOUNT_AUTOFS])
- unlink(target);
- else
- rmdir(target);
+ if (data[MOUNT_AUTOFS] && device->target)
+ unlink(device->target);
}
static void
--
2.13.7
___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel
