[PATCH 1/1] diskpart: lock the device before writing the disklabel

[James Hilliard]

This should help make sure it is safe to modify the disk
before we modify the partition table.

Adapted from:
https://github.com/karelzak/util-linux/blob/v2.37/lib/blkdev.c#L365-L413

Signed-off-by: James Hilliard <james.h...@gmail.com>
---
handlers/diskpart_handler.c | 35 +++++++++++++++++++++++++++++++++--
1 file changed, 33 insertions(+), 2 deletions(-)

diff --git a/handlers/diskpart_handler.c b/handlers/diskpart_handler.c
index 5eba2bd..09832ec 100644
--- a/handlers/diskpart_handler.c
+++ b/handlers/diskpart_handler.c
@@ -13,6 +13,7 @@
#include <string.h>
#include <errno.h>
#include <ctype.h>
+#include <sys/file.h>
#include <sys/types.h>
#include <libfdisk/libfdisk.h>
#include "swupdate.h"
@@ -634,14 +635,44 @@ static int diskpart_compare_tables(struct diskpart_table *tb, struct diskpart_ta
return ret;
}

+static int diskpart_blkdev_lock(struct fdisk_context *cxt)
+{
+ int oper = LOCK_EX;
+ int ret = 0;
+
+ if (fdisk_device_is_used(cxt)) {
+ ERROR("%s: device is in use", fdisk_get_devname(cxt));
+ return -EBUSY;
+ }
+
+ if (!fdisk_is_readonly(cxt)) {
+ ret = flock(fdisk_get_devfd(cxt), oper);
+ if (ret) {
+ switch (errno) {
+ case EWOULDBLOCK:
+ ERROR("%s: device already locked", fdisk_get_devname(cxt));
+ break;
+ default:
+ ERROR("%s: failed to get lock", fdisk_get_devname(cxt));
+ }
+ return -EBUSY;
+ }
+ }
+ return ret;
+}
+
static int diskpart_write_table(struct fdisk_context *cxt, struct create_table *createtable)
{
int ret = 0;

- if (createtable->parent || createtable->child)
+ if (createtable->parent || createtable->child) {
TRACE("Partitions on disk differ, write to disk;");
- else
+ ret = diskpart_blkdev_lock(PARENT(cxt));
+ if (ret)
+ return ret;
+ } else {
TRACE("Same partition table on disk, do not touch partition table !");
+ }

if (createtable->child) {
if (!IS_HYBRID(cxt)) {
--
2.32.0

[James Hilliard]

On Sun, Jul 4, 2021 at 9:09 AM James Hilliard <james.h...@gmail.com> wrote:
>
> This should help make sure it is safe to modify the disk
> before we modify the partition table.

FYI this appears to be required for systemd-udevd interoperability, see:
https://systemd.io/BLOCK_DEVICE_LOCKING/

[Stefano Babic]

Hi James,

I am missing how the lock is release, that is, where the file descriptor
is implicitely closed by fdisk ? I will be sure that if I run SWUpdate
twice without rebooting still works.

Regards,
Stefano

--
=====================================================================
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
=====================================================================

[James Hilliard]

I think it's here:
https://github.com/karelzak/util-linux/blob/v2.37/libfdisk/src/context.c#L770

Which gets called from here:
https://github.com/sbabic/swupdate/blob/eb8657fa91754029e4cd16df3e959a3498779599/handlers/diskpart_handler.c#L896

[Stefano Babic]

Hi James,

Ah ok, thanks ! That's fine, I will just test on a device and apply the
patch.