[PATCH v2 0/1] libfdisk: write mbr only when useful

[PATCH v2 0/1] libfdisk: write mbr only when useful

[Philippe Reynes]

The MBR has no backup, so any failure when writing it
may leads to critical issue. But when only using gpt
partition, the protective MBR is always the same. It is
useless to write it each time the gpt partition table is
updated.

Usually, the gpt is not written in the life of a product.
But in case a product uses a dual bank scheme to update
a board, the new version of the firmware will be written
in backup partitions, and then swap the name of primary
partitions and backup partitions. This leads to update
the gpt table at each update.

A failure while writing a gpt table may be recoved at
next boot, but a failure when writing the protective
MBR could break the board. So this patch removes useless
update of the MBR, enhancing the reliability of the gpt
table update.

Changelog:
v2:
- fix memleak (thanks Karel)

Philippe Reynes (1):
  libfdisk: write mbr only when useful

 libfdisk/src/gpt.c | 33 +++++++++++++++++++++++++++++++++
 1 file changed, 33 insertions(+)

-- 
2.25.1

[PATCH v2 1/1] libfdisk: write mbr only when useful

[Philippe Reynes]

The MBR is critical as it doesn't have
a backup, so we only write it when it
is really useful (ie: the current MBR
is different than the new one).

Signed-off-by: Philippe Reynes <philippe.reynes@softathome.com>
---
 libfdisk/src/gpt.c | 33 +++++++++++++++++++++++++++++++++
 1 file changed, 33 insertions(+)

diff --git a/libfdisk/src/gpt.c b/libfdisk/src/gpt.c
index 72370a19a..c6f482fd8 100644
--- a/libfdisk/src/gpt.c
+++ b/libfdisk/src/gpt.c
@@ -2024,6 +2024,19 @@ static int gpt_set_partition(struct fdisk_context *cxt, size_t n,
 	return rc;
 }
 
+static int gpt_read(struct fdisk_context *cxt, off_t offset, void *buf, size_t count)
+{
+	if (offset != lseek(cxt->dev_fd, offset, SEEK_SET))
+		return -errno;
+
+	if (read_all(cxt->dev_fd, buf, count))
+		return -errno;
+
+	DBG(GPT, ul_debug("  read OK [offset=%zu, size=%zu]",
+				(size_t) offset, count));
+	return 0;
+}
+
 static int gpt_write(struct fdisk_context *cxt, off_t offset, void *buf, size_t count)
 {
 	if (offset != lseek(cxt->dev_fd, offset, SEEK_SET))
@@ -2079,6 +2092,8 @@ static int gpt_write_header(struct fdisk_context *cxt,
 static int gpt_write_pmbr(struct fdisk_context *cxt)
 {
 	struct gpt_legacy_mbr *pmbr;
+	struct gpt_legacy_mbr *current;
+	int rc;
 
 	assert(cxt);
 	assert(cxt->firstsector);
@@ -2107,6 +2122,24 @@ static int gpt_write_pmbr(struct fdisk_context *cxt)
 		pmbr->partition_record[0].size_in_lba =
 			cpu_to_le32((uint32_t) (cxt->total_sectors - 1ULL));
 
+	current = malloc(sizeof(*current));
+	if (!current)
+		goto do_write;
+
+	rc = gpt_read(cxt, GPT_PMBR_LBA * cxt->sector_size,
+		      current, cxt->sector_size);
+
+	if (!rc)
+		rc = memcmp(pmbr, current, sizeof(*current));
+
+	free(current);
+
+	if (!rc) {
+		DBG(GPT, ul_debug("Same MBR on disk => don't write it"));
+		return 0;
+	}
+
+ do_write:
 	/* pMBR covers the first sector (LBA) of the disk */
 	return gpt_write(cxt, GPT_PMBR_LBA * cxt->sector_size,
 			 pmbr, cxt->sector_size);
-- 
2.25.1

Re: [PATCH v2 0/1] libfdisk: write mbr only when useful

[Karel Zak]

On Mon, Apr 11, 2022 at 02:42:28PM +0200, Philippe Reynes wrote:
>  libfdisk/src/gpt.c | 33 +++++++++++++++++++++++++++++++++
>  1 file changed, 33 insertions(+)

Applied, thanks.

-- 
 Karel Zak  <kzak@redhat.com>
 http://karelzak.blogspot.com