[PATCH 1/3] [meta-swupdate] [v1,1/3] swupdate-usb: update service to use mount points dynamically

[Muhammad Hamza]

swupdate-usb service uses a fixed mount point which caters
block devices with one partition only. If a device has multiple
partitions swupdate-usb service launches for every partition and
tries to mount it at same mount point which is not a clean
way. This also results in failures during un-mounting even if
the update is performed successfully.
This is due to the fact that when multiple partitons are mounted
at same mount point, it acts like a stack and with each un-mount
the latest mounted partiton is removed. An example case is if a
USB with 3 partitions is plugged in it launches services for all,
say all partitons are mounted in sequence as sda1, sda2 and sda3.
However if only one partiton say sda2 contains the update image
it will start update from it and services for sda1 and sda3 will
exit as no image is present in them. This will cause services for
sda1 and sda3 to run umount twice on /mnt/ but second partition in
mounting sequence was sda2 which is busy and it would fail to
un-mount. Now when swupdate-usb@sda2 will exit after update it
will run umount once and one partition will still be left mounted
as corresponding umount for that service failed.

This commit introduces a change that for every partiton
a unique mount point is created, and when done, that mount
point is removed, this ensures no such conficts occur and every
service uses a mount point unique to the partiton against which
service was launched

Signed-off-by: Muhammad Hamza <muhamma...@mentor.com>
---
recipes-support/swupdate/swupdate/swupdate-usb@.service | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/recipes-support/swupdate/swupdate/swupdate-usb@.service b/recipes-support/swupdate/swupdate/swupdate-usb@.service
index eda9d15..752253e 100644
--- a/recipes-support/swupdate/swupdate/swupdate-usb@.service
+++ b/recipes-support/swupdate/swupdate/swupdate-usb@.service
@@ -3,6 +3,8 @@ Description=usb media swupdate service
Requires=swupdate-progress.service

[Service]
-ExecStartPre=/bin/mount /dev/%I /mnt
-ExecStart=/bin/sh -c "swupdate-client -v /mnt/*.swu"
-ExecStopPost=/bin/umount /mnt
+ExecStartPre=/bin/mkdir -p /mnt/%I
+ExecStartPre=/bin/mount /dev/%I /mnt/%I
+ExecStart=/bin/sh -c "swupdate-client -v /mnt/%I/*.swu"
+ExecStopPost=/bin/umount /mnt/%I
+ExecStopPost=/bin/rmdir /mnt/%I
--
2.25.1

[Muhammad Hamza]

There is a true dependecy of swupdate-usb service on swupdate
service which is not present in a proper way. If swupdate is
not running for some reason, the swupdate-usb service will fail
as swupdate-client requires the host socket to be present which
is provided by host.
In current implementation swupdate-usb has a dependedency on
swupdate-progress which does not have a "Requires" dependency
on swupdate. This results in a failure if swupdate service is not
running and swupdate-usb tries to invoke swupdate-client.
The swupdate-sb must have a direct dependency on swupdate service
because we might have systems where one does not want to monitor
progress. Therefore any changes in current dependency chain will
result in failure for swupdate.
Therefore a "Require" dependency on swupdate service must be
present in swupdate-usb. swsupdate-usb does not require
swupdate-progress service to be present or running for a successful
update but it does need swupdate.service to be up and running to
run. This is makes it a true dependency.
This commit changes swupdate-usb to require swsupdate service to
have a proper and right dedpendency.

Signed-off-by: Muhammad Hamza <muhamma...@mentor.com>
---

recipes-support/swupdate/swupdate/swupdate-usb@.service | 1 +
1 file changed, 1 insertion(+)

diff --git a/recipes-support/swupdate/swupdate/swupdate-usb@.service b/recipes-support/swupdate/swupdate/swupdate-usb@.service
index 752253e..cf5dcaf 100644
--- a/recipes-support/swupdate/swupdate/swupdate-usb@.service
+++ b/recipes-support/swupdate/swupdate/swupdate-usb@.service
@@ -1,5 +1,6 @@
[Unit]
Description=usb media swupdate service
+Requires=swupdate.service
Requires=swupdate-progress.service

[Service]
--
2.25.1

[Muhammad Hamza]

swupdate service launches a shell script which is responsible for
launcing swupdate core. Default service type considers that unit
is started right after main process is forked, and dependent services
can be launched right away, however it take some time for swupdate
core to initialize. This causes swupdate-client, launched by
swupdate-usb service, to end up in a race condition if swupdate-usb
service is launched right after swupdate service as service manager
considers that swupdate unit is fully running, whereas swupdate core
has not completely initialized yet. This results in immature launcing of
swupdate-client and it fails with "swupdate_async_start returns -1"
because host socket are not initialized yet in swupdate core for
the client to connect to.
When service type is set to "exec" service manager considers that a
service unit is started after the main process binary is executed
making sure that dependent services are not launched till core is
initialized. swupdate.sh uses exec to launch swupdate core this makes
sure that swupdate.sh is considered executed only once swupdate core
is running.
This commits sets service type to "exec" and makes use of the way
swupdate.sh executes swupdate core using exec command to make sure
that swupdate-client and core do not end up in race condition.

Signed-off-by: Muhammad Hamza <muhamma...@mentor.com>
---

recipes-support/swupdate/swupdate/swupdate.service | 1 +

1 file changed, 1 insertion(+)

diff --git a/recipes-support/swupdate/swupdate/swupdate.service b/recipes-support/swupdate/swupdate/swupdate.service
index 7f36619..c0253aa 100644
--- a/recipes-support/swupdate/swupdate/swupdate.service
+++ b/recipes-support/swupdate/swupdate/swupdate.service
@@ -4,6 +4,7 @@ Documentation=https://github.com/sbabic/swupdate
Documentation=https://sbabic.github.io/swupdate

[Service]
+Type=exec
ExecStart=@LIBDIR@/swupdate/swupdate.sh
KillMode=mixed

--
2.25.1

[Stefano Babic]

Hi Muhammad,

sorry for late review, this series was quite lost:

On 28.04.22 14:10, Muhammad Hamza wrote:
> swupdate-usb service uses a fixed mount point which caters
> block devices with one partition only. If a device has multiple
> partitions swupdate-usb service launches for every partition and
> tries to mount it at same mount point which is not a clean
> way. This also results in failures during un-mounting even if
> the update is performed successfully.
> This is due to the fact that when multiple partitons are mounted
> at same mount point, it acts like a stack and with each un-mount
> the latest mounted partiton is removed. An example case is if a
> USB with 3 partitions is plugged in it launches services for all,
> say all partitons are mounted in sequence as sda1, sda2 and sda3.
> However if only one partiton say sda2 contains the update image
> it will start update from it and services for sda1 and sda3 will
> exit as no image is present in them. This will cause services for
> sda1 and sda3 to run umount twice on /mnt/ but second partition in
> mounting sequence was sda2 which is busy and it would fail to
> un-mount. Now when swupdate-usb@sda2 will exit after update it
> will run umount once and one partition will still be left mounted
> as corresponding umount for that service failed.
>

Behavior is clear.

> This commit introduces a change that for every partiton
> a unique mount point is created, and when done, that mount
> point is removed, this ensures no such conficts occur and every
> service uses a mount point unique to the partiton against which
> service was launched

Theoretical a good way, but...what about if the rootfs is read-only ?

>
> Signed-off-by: Muhammad Hamza <muhamma...@mentor.com>
> ---
> recipes-support/swupdate/swupdate/swupdate-usb@.service | 8 +++++---
> 1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/recipes-support/swupdate/swupdate/swupdate-usb@.service b/recipes-support/swupdate/swupdate/swupdate-usb@.service
> index eda9d15..752253e 100644
> --- a/recipes-support/swupdate/swupdate/swupdate-usb@.service
> +++ b/recipes-support/swupdate/swupdate/swupdate-usb@.service
> @@ -3,6 +3,8 @@ Description=usb media swupdate service
> Requires=swupdate-progress.service
>
> [Service]
> -ExecStartPre=/bin/mount /dev/%I /mnt
> -ExecStart=/bin/sh -c "swupdate-client -v /mnt/*.swu"
> -ExecStopPost=/bin/umount /mnt
> +ExecStartPre=/bin/mkdir -p /mnt/%I
> +ExecStartPre=/bin/mount /dev/%I /mnt/%I

It is not really important where it is mounted. We could maybe move it
to /tmp instead of /mnt, and tmp is guaranteed to be writable.

> +ExecStart=/bin/sh -c "swupdate-client -v /mnt/%I/*.swu"
> +ExecStopPost=/bin/umount /mnt/%I
> +ExecStopPost=/bin/rmdir /mnt/%I

Best regards,
Stefano Babic

--
=====================================================================
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sba...@denx.de
=====================================================================

[Stefano Babic]

On 28.04.22 14:10, Muhammad Hamza wrote:

Acked-by: Stefano Babic <sba...@denx.de>

[Stefano Babic]

On 28.04.22 14:10, Muhammad Hamza wrote:

[James Hilliard]

Should actually be this I think:
Type=notify

Since we implement notify support here:
https://github.com/sbabic/swupdate/blob/a10214e88eba43ae0382c84d9f0d10cf5d623e2a/core/swupdate.c#L927
> ExecStart=@LIBDIR@/swupdate/swupdate.sh
> KillMode=mixed
>
> --
> 2.25.1
>
> --
> You received this message because you are subscribed to the Google Groups "swupdate" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to swupdate+u...@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/swupdate/20220428121033.1609620-3-muhammad_hamza%40mentor.com.

[Stefano Babic]

Hi James,

You're right - notify is the right type here. @Muhammad ?

Best regards,
Stefano

[Hamza, Muhammad]

Hello,

Yes, you're right, I have been away for few days, will send updated patch soon.

Regards,
Hamza

[Hamza, Muhammad]

swupdate-usb service uses a fixed mount point which caters block
devices with one partition only. If a device has multiple partitions
swupdate-usb service launches for every partition and tries to mount
it at same mount point which is not a clean way. This also results in
failures during un-mounting even if the update is performed
successfully.
This is due to the fact that when multiple partitons are mounted at
same mount point, it acts like a stack and with each un-mount the

latest mounted partition is removed. An example case is if a USB with 3
partitions are plugged in it launches services for all, say all
partitions are mounted in sequence as sda1, sda2 and sda3.
However, if only one partiton say sda2 contains the update image it

will start update from it and services for sda1 and sda3 will exit as
no image is present in them. This will cause services for
sda1 and sda3 to run umount twice on /mnt/ but second partition in

mounting sequence was sda2 which is busy, and it would fail to

un-mount. Now when swupdate-usb@sda2 will exit after update it will
run umount once and one partition will still be left mounted as
corresponding umount for that service failed.

This commit introduces a change that for every partiton a unique mount
point is created, and when done, that mount point is removed, this
ensures no such conficts occur and every service uses a mount point

unique to the partiton against which service was launched. This also
replaces mount point 'mnt' with 'tmp' to guarantee writable mount-point
in-case the rootfs is read-only.

Signed-off-by: Muhammad Hamza <muhamma...@mentor.com>
---

recipes-support/swupdate/swupdate/swupdate-usb@.service | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/recipes-support/swupdate/swupdate/swupdate-usb@.service b/recipes-support/swupdate/swupdate/swupdate-usb@.service
index df1a408..cf36b2c 100644
--- a/recipes-support/swupdate/swupdate/swupdate-usb@.service
+++ b/recipes-support/swupdate/swupdate/swupdate-usb@.service
@@ -4,6 +4,8 @@ Requires=swupdate.service

Requires=swupdate-progress.service

[Service]
-ExecStartPre=/bin/mount /dev/%I /mnt
-ExecStart=/bin/sh -c "swupdate-client -v /mnt/*.swu"
-ExecStopPost=/bin/umount /mnt

+ExecStartPre=/bin/mkdir -p /tmp/%I
+ExecStartPre=/bin/mount /dev/%I /tmp/%I
+ExecStart=/bin/sh -c "swupdate-client -v /tmp/%I/*.swu"
+ExecStopPost=/bin/umount /tmp/%I
+ExecStopPost=/bin/rmdir /tmp/%I

[Hamza, Muhammad]

+ExecStartPre=/bin/mount /dev/%I /tmp/%I ExecStart=/bin/sh -c

+"swupdate-client -v /tmp/%I/*.swu"

+ExecStopPost=/bin/umount /tmp/%I

+ExecStopPost=/bin/rmdir /tmp/%I

--
2.25.1

[Hamza, Muhammad]

Please ignore last mail as it was re-sent by mistake.

[Hamza, Muhammad]

[Hamza, Muhammad]

[Hamza, Muhammad]

[Stefano Babic]

Hi Muhammad,

you are sending multiple times the same patch with the same version
number, it is difficult to track. I have already applied 2/3 and 3/3 and
the follow-up to 3/3, so this patch has nothing to do anymore with the
series.

On 17.06.22 09:19, Hamza, Muhammad wrote:
> swupdate-usb service uses a fixed mount point which caters block devices
> with one partition only. If a device has multiple partitions
> swupdate-usb service launches for every partition and tries to mount it
> at same mount point which is not a clean way. This also results in
> failures during un-mounting even if the update is performed successfully.
>
> This is due to the fact that when multiple partitons are mounted at same
> mount point, it acts like a stack and with each un-mount the latest
> mounted partition is removed. An example case is if a USB with 3
> partitions are plugged in it launches services for all, say all
> partitions are mounted in sequence as sda1, sda2 and sda3.
>
> However, if only one partiton say sda2 contains the update image it will
> start update from it and services for sda1 and sda3 will exit as no
> image is present in them. This will cause services for
>

The use case for this is for end users, generally not aware of Linux,
that just load the SWU and copy it on a fresh bought USP pen. USB pen
are sold for Windows, they have no partition table, they are pre
formatted with VFAT, and Linux find a single /dev/%I (mostly sda).

In case of multiple partitions, it raises the question what should be
done: search for any SWU ? Provide a menu to the customer to select ? Do
nothing because it cannot decide, if a HMI is availbvale ? These
questions are very specific to each project, and this service is mostly
overridden in customer's project.

I am not sure what should be done with multiple SWUs. Should be tried to
run an update for all of them ? And then, which order ?

Anyway, I can merge this - it remains an example (maybe it should be
written somewhere). The integrator should check it and verify with the
own project.

Which is the difference with previous versions, if any ?

> sda1 and sda3 to run umount twice on /mnt/ but second partition in
> mounting sequence was sda2 which is busy, and it would fail to un-mount.
> Now when swupdate-usb@sda2 will exit after update it will run umount
> once and one partition will still be left mounted as corresponding
> umount for that service failed.
>
> This commit introduces a change that for every partiton a unique mount
> point is created, and when done, that mount point is removed, this
> ensures no such conficts occur and every service uses a mount point
> unique to the partiton against which service was launched. This also
> replaces mount point 'mnt' with 'tmp' to guarantee writable mount-point
> in-case the rootfs is read-only.
>

Not clear what happens when multiple SUs are stored, IMO is becoming
messy. I do not know if a general solution can be found, maybe executing
a script before swupdate-client that will decide if the update should be
performed or not.

> Signed-off-by: Muhammad Hamza <muhamma...@mentor.com
> <mailto:muhamma...@mentor.com>>
>
> ---
>
> recipes-support/swupdate/swupdate/swupdate-usb@.service
> <mailto:recipes-support/swupdate/swupdate/swupdate-usb@.service> | 8

> +++++---
>
> 1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/recipes-support/swupdate/swupdate/swupdate-usb@.service

> <mailto:a/recipes-support/swupdate/swupdate/swupdate-usb@.service>
> b/recipes-support/swupdate/swupdate/swupdate-usb@.service
> <mailto:b/recipes-support/swupdate/swupdate/swupdate-usb@.service>

>
> index df1a408..cf36b2c 100644
>
> --- a/recipes-support/swupdate/swupdate/swupdate-usb@.service

> <mailto:a/recipes-support/swupdate/swupdate/swupdate-usb@.service>
>
> +++ b/recipes-support/swupdate/swupdate/swupdate-usb@.service
> <mailto:b/recipes-support/swupdate/swupdate/swupdate-usb@.service>

>
> @@ -4,6 +4,8 @@ Requires=swupdate.service
> Requires=swupdate-progress.service
>
> [Service]
>
> -ExecStartPre=/bin/mount /dev/%I /mnt
>
> -ExecStart=/bin/sh -c "swupdate-client -v /mnt/*.swu"
>
> -ExecStopPost=/bin/umount /mnt
>
> +ExecStartPre=/bin/mkdir -p /tmp/%I
>
> +ExecStartPre=/bin/mount /dev/%I /tmp/%I ExecStart=/bin/sh -c
>
> +"swupdate-client -v /tmp/%I/*.swu"
>
> +ExecStopPost=/bin/umount /tmp/%I
>
> +ExecStopPost=/bin/rmdir /tmp/%I
>
> --
>
> 2.25.1
>

Best regards,
Stefano Babic

--
=====================================================================
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk

HRB 165235 Munich, Office: Kirchenstr.5, 82194 Groebenzell, Germany

[Hamza, Muhammad]

Hello Stefano,

Apologies for multiple patches, I was trying to send in with different patch number, the patch 1/3 so it comes up as a new message, the change from initial version was using 'tmp' as mount point as pointed out in initial message that if the rootfs is read only, but somehow these are ending up as follow ups on same thread. Apologies again for the confusion due to multiple patches on same thread.

Regards,
Hamza

-----Original Message-----
From: Stefano Babic <sba...@denx.de>

[Hamza, Muhammad]

Swupdate by default launches client for all partitions detected in USB pen drive, and this causes failures in unmounting as swupdate mounts these all partitions on same mount point, this patch will resolve this issue at swupdate's end rest as you mentioned, I agree, that every project needs to consider its implementation.

-----Original Message-----
From: Stefano Babic <sba...@denx.de>
Sent: Friday, June 17, 2022 2:21 PM
To: Hamza, Muhammad <Muhamma...@mentor.com>; Stefano Babic <sba...@denx.de>; swup...@googlegroups.com
Subject: Re: [swupdate] [PATCH 1/3] [meta-swupdate] [v3,1/3] swupdate-usb: update service to use mount points dynamically

[Hamza, Muhammad]

Hi Stefano,

I was seeing that this patch has not been applied yet, patch 2/3 and 3/3 were applied, I have explained in my previous message that why it is needed
in this series.

Regards,
hamza

[Stefano Babic]

Hi Muhammad,

sorry for very late answer:

On 25.08.22 12:35, Hamza, Muhammad wrote:
> Hi Stefano,
>
> I was seeing that this patch has not been applied yet, patch 2/3 and 3/3 were applied, I have explained in my previous message that why it is needed
> in this series.
>

The patch was malformed and couldn't be applied - there were line
breaks. I have checked this and fixed, and applied to all supported
meta-swupdate branches.

Applied to -dunfell, -master, -langdale, -kirkstone, thanks !

Best regards,
Stefano Babic

DENX Software Engineering GmbH, Managing Director: Erika Unter

[Muhammad.Hamza Ishfaq]

Hello Ali,

Looking at the logs, issue is likely present in your swupdate.sh. Can you share content of your script?

Also it is better to start a new thread as this issue doesn't look relevant to original thread.

Regards,
Hamza

[Muhammad Hamza]

As discussed, configuration of swupdate service should be correctly modified for OTA updated eg. if using hawkbit server, proper IP should be configured for OTAs else it should be disabled if only USB updates are required.