diff mbox series

[v1,1/2] handlers: add diskformat handler

Message ID 20210802144326.28642-2-roland.gaudig-oss@weidmueller.com
State Changes Requested
Headers show
Series [v1,1/2] handlers: add diskformat handler | expand

Commit Message

Roland Gaudig Aug. 2, 2021, 2:43 p.m. UTC
From: Roland Gaudig <roland.gaudig@weidmueller.com>

Currently the discpart handler only creates a new file system when the
partition table has changed.

In cases where there exists an unused and unformatted spare partion which
shall be activated by a later update, it is necessary to format such a
partition even if the partition table has not been altered.

Also there are cases when creating a file system on a single partition is
desired, but there is no partitioning scheme specified inside the
sw-description.

Therefore, this commit introduces a new diskformat handler which allows
creating a file system independently from the partitioning process.
Before creating a new file system it checks whether the desired file system
already exists and only writes if not.

In case it is desired to create a new file system anyway, this can be
achieved by setting the property "force" to "true".

	partitions: (
	{
		type = "diskformat";
		device = "/dev/mmcblk0p1";

		properties: {
			fstype = "vfat";
			force = "true";
		}
	})

The diskformat handler will be active when the configuration option
CONFIG_DISKFORMAT_HANDLER is selected.

Signed-off-by: Roland Gaudig <roland.gaudig@weidmueller.com>
---

 Makefile.flags                |   4 ++
 handlers/Config.in            |  18 +++---
 handlers/Makefile             |   2 +
 handlers/diskformat.h         |  13 +++++
 handlers/diskformat_common.c  |  68 +++++++++++++++++++++++
 handlers/diskformat_handler.c | 100 ++++++++++++++++++++++++++++++++++
 handlers/diskpart_handler.c   |  42 ++------------
 7 files changed, 201 insertions(+), 46 deletions(-)
 create mode 100644 handlers/diskformat.h
 create mode 100644 handlers/diskformat_common.c
 create mode 100644 handlers/diskformat_handler.c

Comments

Stefano Babic Aug. 6, 2021, 10:37 a.m. UTC | #1
Hi Roland,

On 02.08.21 16:43, Roland Gaudig wrote:
> From: Roland Gaudig <roland.gaudig@weidmueller.com>
> 
> Currently the discpart handler only creates a new file system when the
> partition table has changed.
> 

It is fine with me to have a separate handler. Anyway, I see a couple of 
things:

- applying patches, I get several warnings. Please fix them.

In file included from handlers/diskformat_common.c:10:
include/handler.h:36:31: warning: ‘struct img_type’ declared inside 
parameter list will not be visible outside of this definition or declaration
    36 | typedef int (*handler)(struct img_type *img, void *data);
       |                               ^~~~~~~~
include/handler.h:47:47: warning: ‘struct img_type’ declared inside 
parameter list will not be visible outside of this definition or declaration
    47 | struct installer_handler *find_handler(struct img_type *img);
       |                                               ^~~~~~~~
include/handler.h:50:38: warning: ‘struct img_type’ declared inside 
parameter list will not be visible outside of this definition or declaration
    50 | unsigned int get_handler_mask(struct img_type *img);
       |                                      ^~~~~~~~
   CC      handlers/diskformat_handler.o
In file included from handlers/diskformat_handler.c:10:
include/handler.h:36:31: warning: ‘struct img_type’ declared inside 
parameter list will not be visible outside of this definition or declaration
    36 | typedef int (*handler)(struct img_type *img, void *data);
       |                               ^~~~~~~~
include/handler.h:47:47: warning: ‘struct img_type’ declared inside 
parameter list will not be visible outside of this definition or declaration
    47 | struct installer_handler *find_handler(struct img_type *img);
       |                                               ^~~~~~~~
include/handler.h:50:38: warning: ‘struct img_type’ declared inside 
parameter list will not be visible outside of this definition or declaration
    50 | unsigned int get_handler_mask(struct img_type *img);
       |                                      ^~~~~~~~
handlers/diskformat_handler.c: In function ‘diskformat_handler’:
handlers/diskformat_handler.c:99:33: warning: passing argument 2 of 
‘register_handler’ from incompatible pointer type 
[-Wincompatible-pointer-types]
    99 |  register_handler("diskformat", diskformat,
       |                                 ^~~~~~~~~~
       |                                 |
       |                                 int (*)(struct img_type *, void *)



- the common code (diskformat.c, mainly) should be better put in a 
central place, what about fs/ ?

Best regards,
Stefano

> In cases where there exists an unused and unformatted spare partion which
> shall be activated by a later update, it is necessary to format such a
> partition even if the partition table has not been altered.
> 
> Also there are cases when creating a file system on a single partition is
> desired, but there is no partitioning scheme specified inside the
> sw-description.
> 
> Therefore, this commit introduces a new diskformat handler which allows
> creating a file system independently from the partitioning process.
> Before creating a new file system it checks whether the desired file system
> already exists and only writes if not.
> 
> In case it is desired to create a new file system anyway, this can be
> achieved by setting the property "force" to "true".
> 
> 	partitions: (
> 	{
> 		type = "diskformat";
> 		device = "/dev/mmcblk0p1";
> 
> 		properties: {
> 			fstype = "vfat";
> 			force = "true";
> 		}
> 	})
> 
> The diskformat handler will be active when the configuration option
> CONFIG_DISKFORMAT_HANDLER is selected.
> 
> Signed-off-by: Roland Gaudig <roland.gaudig@weidmueller.com>
> ---
> 
>   Makefile.flags                |   4 ++
>   handlers/Config.in            |  18 +++---
>   handlers/Makefile             |   2 +
>   handlers/diskformat.h         |  13 +++++
>   handlers/diskformat_common.c  |  68 +++++++++++++++++++++++
>   handlers/diskformat_handler.c | 100 ++++++++++++++++++++++++++++++++++
>   handlers/diskpart_handler.c   |  42 ++------------
>   7 files changed, 201 insertions(+), 46 deletions(-)
>   create mode 100644 handlers/diskformat.h
>   create mode 100644 handlers/diskformat_common.c
>   create mode 100644 handlers/diskformat_handler.c
> 
> diff --git a/Makefile.flags b/Makefile.flags
> index d3ca49d..e549b46 100644
> --- a/Makefile.flags
> +++ b/Makefile.flags
> @@ -185,6 +185,10 @@ ifeq ($(CONFIG_DISKPART),y)
>   LDLIBS += fdisk
>   endif
>   
> +ifeq ($(CONFIG_DISKFORMAT_HANDLER),y)
> +LDLIBS += blkid
> +endif
> +
>   ifeq ($(CONFIG_EXT_FILESYSTEM),y)
>   LDLIBS += ext2fs uuid blkid
>   endif
> diff --git a/handlers/Config.in b/handlers/Config.in
> index a0cc5a3..36f3ca4 100644
> --- a/handlers/Config.in
> +++ b/handlers/Config.in
> @@ -106,15 +106,19 @@ config DISKPART
>   comment "diskpart support needs libfdisk"
>   	depends on !HAVE_LIBFDISK
>   
> -if DISKPART
> -
> -menuconfig DISKFORMAT
> -        bool "diskpart extension for creating file systems"
> +config DISKFORMAT
> +	bool "diskpart extension for creating file systems"
>   	depends on DISKPART
> +	help
> +	  This extension allows formatting newly created partitions.
> +
> +config DISKFORMAT_HANDLER
> +	bool "diskformat handler for creating file systems"
> +	select DISKFORMAT
>   	default n
>   	help
> -	  This extension of the diskpart handler allows creating filesystems
> -	  on empty partitions.
> +	  The diskformat handler allows creating filesystems on empty
> +	  partitions.
>   
>   if DISKFORMAT
>   
> @@ -122,8 +126,6 @@ source fs/Config.in
>   
>   endif
>   
> -endif
> -
>   config UNIQUEUUID
>   	bool "uniqueuuid"
>   	depends on HAVE_LIBBLKID
> diff --git a/handlers/Makefile b/handlers/Makefile
> index 1f61f60..a37ce78 100644
> --- a/handlers/Makefile
> +++ b/handlers/Makefile
> @@ -11,6 +11,8 @@ obj-y	+= dummy_handler.o
>   obj-$(CONFIG_ARCHIVE) += archive_handler.o
>   obj-$(CONFIG_BOOTLOADERHANDLER) += boot_handler.o
>   obj-$(CONFIG_CFI)	+= flash_handler.o
> +obj-$(CONFIG_DISKFORMAT)	+= diskformat_common.o
> +obj-$(CONFIG_DISKFORMAT_HANDLER)	+= diskformat_handler.o
>   obj-$(CONFIG_DISKPART)	+= diskpart_handler.o
>   obj-$(CONFIG_UNIQUEUUID)	+= uniqueuuid_handler.o
>   obj-$(CONFIG_CFIHAMMING1)+= flash_hamming1_handler.o
> diff --git a/handlers/diskformat.h b/handlers/diskformat.h
> new file mode 100644
> index 0000000..441d291
> --- /dev/null
> +++ b/handlers/diskformat.h
> @@ -0,0 +1,13 @@
> +/*
> + * Copyright (C) 2021 Weidmueller Interface GmbH & Co. KG
> + * Roland Gaudig <roland.gaudig@weidmueller.com>
> + *
> + * SPDX-License-Identifier:     GPL-2.0-only
> + */
> +
> +#ifndef _DISKFORMAT_H
> +#define _DISKFORMAT_H
> +
> +int diskformat_mkfs(char *device, char *fstype);
> +
> +#endif
> diff --git a/handlers/diskformat_common.c b/handlers/diskformat_common.c
> new file mode 100644
> index 0000000..7f21c90
> --- /dev/null
> +++ b/handlers/diskformat_common.c
> @@ -0,0 +1,68 @@
> +/*
> + * Copyright (C) 2021 Weidmueller Interface GmbH & Co. KG
> + * Roland Gaudig <roland.gaudig@weidmueller.com>
> + *
> + * SPDX-License-Identifier:     GPL-2.0-only
> + */
> +
> +#include <errno.h>
> +#include <stdio.h>
> +#include <handler.h>
> +#include <util.h>
> +#include <blkid/blkid.h>
> +#include <fs_interface.h>
> +#include "diskformat.h"
> +
> +#if defined(CONFIG_EXT_FILESYSTEM)
> +static inline int ext_mkfs_short(const char *device_name, const char *fstype)
> +{
> +	return ext_mkfs(device_name, fstype, 0, NULL);
> +}
> +#endif
> +
> +struct supported_filesystems {
> +	const char *fstype;
> +	int (*mkfs)(const char *device_name, const char *fstype);
> +};
> +
> +static struct supported_filesystems fs[] = {
> +#if defined(CONFIG_FAT_FILESYSTEM)
> +	{"vfat", fat_mkfs},
> +#endif
> +#if defined(CONFIG_EXT_FILESYSTEM)
> +	{"ext2", ext_mkfs_short},
> +	{"ext3", ext_mkfs_short},
> +	{"ext4", ext_mkfs_short},
> +#endif
> +};
> +
> +int diskformat_mkfs(char *device, char *fstype)
> +{
> +	int index;
> +	int ret = 0;
> +
> +	if (!device || !fstype) {
> +		ERROR("Uninitialized pointer as device/fstype argument");
> +		return -EINVAL;
> +	}
> +
> +	for (index = 0; index < ARRAY_SIZE(fs); index++) {
> +		if (!strcmp(fs[index].fstype, fstype))
> +			break;
> +	}
> +	if (index >= ARRAY_SIZE(fs)) {
> +		ERROR("%s file system type not supported.", fstype);
> +		return -EINVAL;
> +	}
> +
> +	TRACE("Creating %s file system on %s", fstype, device);
> +	ret = fs[index].mkfs(device, fstype);
> +
> +	if (ret) {
> +		ERROR("creating %s file system on %s failed. %d",
> +		      fstype, device, ret);
> +		return -EFAULT;
> +	}
> +
> +	return ret;
> +}
> diff --git a/handlers/diskformat_handler.c b/handlers/diskformat_handler.c
> new file mode 100644
> index 0000000..a4449cf
> --- /dev/null
> +++ b/handlers/diskformat_handler.c
> @@ -0,0 +1,100 @@
> +/*
> + * Copyright (C) 2021 Weidmueller Interface GmbH & Co. KG
> + * Roland Gaudig <roland.gaudig@weidmueller.com>
> + *
> + * SPDX-License-Identifier:     GPL-2.0-only
> + */
> +
> +#include <errno.h>
> +#include <stdio.h>
> +#include <handler.h>
> +#include <util.h>
> +#include <blkid/blkid.h>
> +#include <fs_interface.h>
> +#include "diskformat.h"
> +
> +void diskformat_handler(void);
> +
> +/*
> + * Checks if file system fstype already exists on device.
> + * return 0 if not exists, 1 if exists, negative values on failure
> + */
> +static int fs_exists(char *device, char *fstype)
> +{
> +	char buf[10];
> +	const char *value = buf;
> +	size_t len;
> +	blkid_probe pr;
> +	int ret = 0;
> +
> +	pr = blkid_new_probe_from_filename(device);
> +
> +	if (!pr) {
> +		ERROR("%s: failed to create libblkid probe",
> +		      device);
> +		return -EFAULT;
> +	}
> +
> +	while (blkid_do_probe(pr) == 0) {
> +		if (blkid_probe_lookup_value(pr, "TYPE", &value, &len)) {
> +			ERROR("blkid_probe_lookup_value failed");
> +			ret = -EFAULT;
> +			break;
> +		}
> +
> +		if (!strncmp(value, fstype, sizeof(buf))) {
> +			ret = 1;
> +			break;
> +		}
> +	}
> +	blkid_free_probe(pr);
> +
> +	return ret;
> +}
> +
> +static int diskformat(struct img_type *img,
> +		      void __attribute__ ((__unused__)) *data)
> +{
> +	int ret = 0;
> +
> +	if (!strlen(img->device)) {
> +		ERROR("diskpart handler requires setting \"device\" attribute");
> +		return -EINVAL;
> +	}
> +
> +	char *fstype = dict_get_value(&img->properties, "fstype");
> +
> +	if (!fstype) {
> +		ERROR("diskpart handler requires setting \"fstype\" attribute");
> +		return -EINVAL;
> +	}
> +
> +	char *force = dict_get_value(&img->properties, "force");
> +
> +	if (force != NULL && strcmp(force, "true") == 0) {
> +		; /* Skip file system exists check */
> +	} else {
> +		/* Check if file system exists */
> +		ret = fs_exists(img->device, fstype);
> +
> +		if (ret < 0)
> +			return ret;
> +
> +		if (ret) {
> +			TRACE("Found %s file system on %s, skip mkfs",
> +			      fstype, img->device);
> +			return 0;
> +		}
> +	}
> +
> +	/* File system does not exist, create new file system */
> +	ret = diskformat_mkfs(img->device, fstype);
> +	return ret;
> +}
> +
> +__attribute__((constructor))
> +void diskformat_handler(void)
> +{
> +	register_handler("diskformat", diskformat,
> +			 PARTITION_HANDLER | NO_DATA_HANDLER, NULL);
> +}
> diff --git a/handlers/diskpart_handler.c b/handlers/diskpart_handler.c
> index b5c9a42..272d0b8 100644
> --- a/handlers/diskpart_handler.c
> +++ b/handlers/diskpart_handler.c
> @@ -19,7 +19,7 @@
>   #include "swupdate.h"
>   #include "handler.h"
>   #include "util.h"
> -#include "fs_interface.h"
> +#include "diskformat.h"
>   
>   void diskpart_handler(void);
>   
> @@ -31,12 +31,6 @@ void diskpart_handler(void);
>   /* Linux native partition type */
>    #define GPT_DEFAULT_ENTRY_TYPE "0FC63DAF-8483-4772-8E79-3D69D8477DE4"
>   
> -#if defined (CONFIG_EXT_FILESYSTEM)
> -static inline int ext_mkfs_short(const char *device_name, const char *fstype) {
> -	return ext_mkfs(device_name,fstype, 0, NULL);
> -}
> -#endif
> -
>   /*
>    * We will only have a parent in hybrid mode.
>    */
> @@ -47,22 +41,6 @@ static inline int ext_mkfs_short(const char *device_name, const char *fstype) {
>    */
>   #define PARENT(cxt) fdisk_get_parent(cxt) ? fdisk_get_parent(cxt) : cxt
>   
> -struct supported_filesystems {
> -	const char *fstype;
> -	int	(*mkfs) (const char *device_name, const char *fstype);
> -};
> -
> -static struct supported_filesystems fs[] = {
> -#if defined(CONFIG_FAT_FILESYSTEM)
> -	{"vfat", fat_mkfs},
> -#endif
> -#if defined (CONFIG_EXT_FILESYSTEM)
> -	{"ext2", ext_mkfs_short},
> -	{"ext3", ext_mkfs_short},
> -	{"ext4", ext_mkfs_short},
> -#endif
> -};
> -
>   /**
>    * Keys for the properties field in sw-description
>    */
> @@ -942,7 +920,6 @@ handler_release:
>   	/* Create filesystems */
>   	if (!ret && createtable->parent) {
>   		LIST_FOREACH(part, &priv.listparts, next) {
> -			int index;
>   			/*
>   			 * priv.listparts counts partitions starting with 0,
>   			 * but fdisk_partname expects the first partition having
> @@ -952,25 +929,14 @@ handler_release:
>   
>   			if (!strlen(part->fstype))
>   				continue;  /* Don't touch partitions without fstype */
> -			for (index = 0; index < ARRAY_SIZE(fs); index++) {
> -				if (!strcmp(fs[index].fstype, part->fstype))
> -					break;
> -			}
> -			if (index >= ARRAY_SIZE(fs)) {
> -				ERROR("partition-%lu %s filesystem type not supported.", partno, part->fstype);
> -				ret = -EINVAL;
> -				break;
> -			}
>   
>   			char *device = NULL;
> +
>   			device = fdisk_partname(img->device, partno);
> -			TRACE("Creating %s file system on partition-%lu, device %s", part->fstype, partno, device);
> -			ret = fs[index].mkfs(device, part->fstype);
> +			ret = diskformat_mkfs(device, part->fstype);
>   			free(device);
> -			if (ret) {
> -				ERROR("creating %s file system failed. %d", part->fstype, ret);
> +			if (ret)
>   				break;
> -			}
>   		}
>   	}
>   #endif
>
Roland Gaudig Aug. 6, 2021, 11:57 a.m. UTC | #2
Hi Stefano,

On 06.08.21 10:37, Stefano Babic wrote:
> Hi Roland,
> 
> On 02.08.21 16:43, Roland Gaudig wrote:
>> From: Roland Gaudig <roland.gaudig@weidmueller.com>
>>
>> Currently the discpart handler only creates a new file system when the
>> partition table has changed.
>>
> 
> It is fine with me to have a separate handler. Anyway, I see a couple of
> things:
> 
> - applying patches, I get several warnings. Please fix them.
> 
> In file included from handlers/diskformat_common.c:10:
> include/handler.h:36:31: warning: ‘struct img_type’ declared inside
> parameter list will not be visible outside of this definition or
> declaration
>    36 | typedef int (*handler)(struct img_type *img, void *data);
>       |                               ^~~~~~~~
> include/handler.h:47:47: warning: ‘struct img_type’ declared inside
> parameter list will not be visible outside of this definition or
> declaration
>    47 | struct installer_handler *find_handler(struct img_type *img);
>       |                                               ^~~~~~~~
> include/handler.h:50:38: warning: ‘struct img_type’ declared inside
> parameter list will not be visible outside of this definition or
> declaration
>    50 | unsigned int get_handler_mask(struct img_type *img);
>       |                                      ^~~~~~~~
>   CC      handlers/diskformat_handler.o
> In file included from handlers/diskformat_handler.c:10:
> include/handler.h:36:31: warning: ‘struct img_type’ declared inside
> parameter list will not be visible outside of this definition or
> declaration
>    36 | typedef int (*handler)(struct img_type *img, void *data);
>       |                               ^~~~~~~~
> include/handler.h:47:47: warning: ‘struct img_type’ declared inside
> parameter list will not be visible outside of this definition or
> declaration
>    47 | struct installer_handler *find_handler(struct img_type *img);
>       |                                               ^~~~~~~~
> include/handler.h:50:38: warning: ‘struct img_type’ declared inside
> parameter list will not be visible outside of this definition or
> declaration
>    50 | unsigned int get_handler_mask(struct img_type *img);
>       |                                      ^~~~~~~~
> handlers/diskformat_handler.c: In function ‘diskformat_handler’:
> handlers/diskformat_handler.c:99:33: warning: passing argument 2 of
> ‘register_handler’ from incompatible pointer type
> [-Wincompatible-pointer-types]
>    99 |  register_handler("diskformat", diskformat,
>       |                                 ^~~~~~~~~~
>       |                                 |
>       |                                 int (*)(struct img_type *, void *)

Oh, you are rigt. I will correct them.

> - the common code (diskformat.c, mainly) should be better put in a
> central place, what about fs/ ?

That sounds like a good idea. I will move handlers/diskformat_common.c
to fs/diskformat.c

> Best regards,
> Stefano
> 
>> In cases where there exists an unused and unformatted spare partion which
>> shall be activated by a later update, it is necessary to format such a
>> partition even if the partition table has not been altered.
>>
>> Also there are cases when creating a file system on a single partition is
>> desired, but there is no partitioning scheme specified inside the
>> sw-description.
>>
>> Therefore, this commit introduces a new diskformat handler which allows
>> creating a file system independently from the partitioning process.
>> Before creating a new file system it checks whether the desired file
>> system
>> already exists and only writes if not.
>>
>> In case it is desired to create a new file system anyway, this can be
>> achieved by setting the property "force" to "true".
>>
>>     partitions: (
>>     {
>>         type = "diskformat";
>>         device = "/dev/mmcblk0p1";
>>
>>         properties: {
>>             fstype = "vfat";
>>             force = "true";
>>         }
>>     })
>>
>> The diskformat handler will be active when the configuration option
>> CONFIG_DISKFORMAT_HANDLER is selected.
>>
>> Signed-off-by: Roland Gaudig <roland.gaudig@weidmueller.com>
>> ---
>>
>>   Makefile.flags                |   4 ++
>>   handlers/Config.in            |  18 +++---
>>   handlers/Makefile             |   2 +
>>   handlers/diskformat.h         |  13 +++++
>>   handlers/diskformat_common.c  |  68 +++++++++++++++++++++++
>>   handlers/diskformat_handler.c | 100 ++++++++++++++++++++++++++++++++++
>>   handlers/diskpart_handler.c   |  42 ++------------
>>   7 files changed, 201 insertions(+), 46 deletions(-)
>>   create mode 100644 handlers/diskformat.h
>>   create mode 100644 handlers/diskformat_common.c
>>   create mode 100644 handlers/diskformat_handler.c
>>
>> diff --git a/Makefile.flags b/Makefile.flags
>> index d3ca49d..e549b46 100644
>> --- a/Makefile.flags
>> +++ b/Makefile.flags
>> @@ -185,6 +185,10 @@ ifeq ($(CONFIG_DISKPART),y)
>>   LDLIBS += fdisk
>>   endif
>>   +ifeq ($(CONFIG_DISKFORMAT_HANDLER),y)
>> +LDLIBS += blkid
>> +endif
>> +
>>   ifeq ($(CONFIG_EXT_FILESYSTEM),y)
>>   LDLIBS += ext2fs uuid blkid
>>   endif
>> diff --git a/handlers/Config.in b/handlers/Config.in
>> index a0cc5a3..36f3ca4 100644
>> --- a/handlers/Config.in
>> +++ b/handlers/Config.in
>> @@ -106,15 +106,19 @@ config DISKPART
>>   comment "diskpart support needs libfdisk"
>>       depends on !HAVE_LIBFDISK
>>   -if DISKPART
>> -
>> -menuconfig DISKFORMAT
>> -        bool "diskpart extension for creating file systems"
>> +config DISKFORMAT
>> +    bool "diskpart extension for creating file systems"
>>       depends on DISKPART
>> +    help
>> +      This extension allows formatting newly created partitions.
>> +
>> +config DISKFORMAT_HANDLER
>> +    bool "diskformat handler for creating file systems"
>> +    select DISKFORMAT
>>       default n
>>       help
>> -      This extension of the diskpart handler allows creating filesystems
>> -      on empty partitions.
>> +      The diskformat handler allows creating filesystems on empty
>> +      partitions.
>>     if DISKFORMAT
>>   @@ -122,8 +126,6 @@ source fs/Config.in
>>     endif
>>   -endif
>> -
>>   config UNIQUEUUID
>>       bool "uniqueuuid"
>>       depends on HAVE_LIBBLKID
>> diff --git a/handlers/Makefile b/handlers/Makefile
>> index 1f61f60..a37ce78 100644
>> --- a/handlers/Makefile
>> +++ b/handlers/Makefile
>> @@ -11,6 +11,8 @@ obj-y    += dummy_handler.o
>>   obj-$(CONFIG_ARCHIVE) += archive_handler.o
>>   obj-$(CONFIG_BOOTLOADERHANDLER) += boot_handler.o
>>   obj-$(CONFIG_CFI)    += flash_handler.o
>> +obj-$(CONFIG_DISKFORMAT)    += diskformat_common.o
>> +obj-$(CONFIG_DISKFORMAT_HANDLER)    += diskformat_handler.o
>>   obj-$(CONFIG_DISKPART)    += diskpart_handler.o
>>   obj-$(CONFIG_UNIQUEUUID)    += uniqueuuid_handler.o
>>   obj-$(CONFIG_CFIHAMMING1)+= flash_hamming1_handler.o
>> diff --git a/handlers/diskformat.h b/handlers/diskformat.h
>> new file mode 100644
>> index 0000000..441d291
>> --- /dev/null
>> +++ b/handlers/diskformat.h
>> @@ -0,0 +1,13 @@
>> +/*
>> + * Copyright (C) 2021 Weidmueller Interface GmbH & Co. KG
>> + * Roland Gaudig <roland.gaudig@weidmueller.com>
>> + *
>> + * SPDX-License-Identifier:     GPL-2.0-only
>> + */
>> +
>> +#ifndef _DISKFORMAT_H
>> +#define _DISKFORMAT_H
>> +
>> +int diskformat_mkfs(char *device, char *fstype);
>> +
>> +#endif
>> diff --git a/handlers/diskformat_common.c b/handlers/diskformat_common.c
>> new file mode 100644
>> index 0000000..7f21c90
>> --- /dev/null
>> +++ b/handlers/diskformat_common.c
>> @@ -0,0 +1,68 @@
>> +/*
>> + * Copyright (C) 2021 Weidmueller Interface GmbH & Co. KG
>> + * Roland Gaudig <roland.gaudig@weidmueller.com>
>> + *
>> + * SPDX-License-Identifier:     GPL-2.0-only
>> + */
>> +
>> +#include <errno.h>
>> +#include <stdio.h>
>> +#include <handler.h>
>> +#include <util.h>
>> +#include <blkid/blkid.h>
>> +#include <fs_interface.h>
>> +#include "diskformat.h"
>> +
>> +#if defined(CONFIG_EXT_FILESYSTEM)
>> +static inline int ext_mkfs_short(const char *device_name, const char
>> *fstype)
>> +{
>> +    return ext_mkfs(device_name, fstype, 0, NULL);
>> +}
>> +#endif
>> +
>> +struct supported_filesystems {
>> +    const char *fstype;
>> +    int (*mkfs)(const char *device_name, const char *fstype);
>> +};
>> +
>> +static struct supported_filesystems fs[] = {
>> +#if defined(CONFIG_FAT_FILESYSTEM)
>> +    {"vfat", fat_mkfs},
>> +#endif
>> +#if defined(CONFIG_EXT_FILESYSTEM)
>> +    {"ext2", ext_mkfs_short},
>> +    {"ext3", ext_mkfs_short},
>> +    {"ext4", ext_mkfs_short},
>> +#endif
>> +};
>> +
>> +int diskformat_mkfs(char *device, char *fstype)
>> +{
>> +    int index;
>> +    int ret = 0;
>> +
>> +    if (!device || !fstype) {
>> +        ERROR("Uninitialized pointer as device/fstype argument");
>> +        return -EINVAL;
>> +    }
>> +
>> +    for (index = 0; index < ARRAY_SIZE(fs); index++) {
>> +        if (!strcmp(fs[index].fstype, fstype))
>> +            break;
>> +    }
>> +    if (index >= ARRAY_SIZE(fs)) {
>> +        ERROR("%s file system type not supported.", fstype);
>> +        return -EINVAL;
>> +    }
>> +
>> +    TRACE("Creating %s file system on %s", fstype, device);
>> +    ret = fs[index].mkfs(device, fstype);
>> +
>> +    if (ret) {
>> +        ERROR("creating %s file system on %s failed. %d",
>> +              fstype, device, ret);
>> +        return -EFAULT;
>> +    }
>> +
>> +    return ret;
>> +}
>> diff --git a/handlers/diskformat_handler.c
>> b/handlers/diskformat_handler.c
>> new file mode 100644
>> index 0000000..a4449cf
>> --- /dev/null
>> +++ b/handlers/diskformat_handler.c
>> @@ -0,0 +1,100 @@
>> +/*
>> + * Copyright (C) 2021 Weidmueller Interface GmbH & Co. KG
>> + * Roland Gaudig <roland.gaudig@weidmueller.com>
>> + *
>> + * SPDX-License-Identifier:     GPL-2.0-only
>> + */
>> +
>> +#include <errno.h>
>> +#include <stdio.h>
>> +#include <handler.h>
>> +#include <util.h>
>> +#include <blkid/blkid.h>
>> +#include <fs_interface.h>
>> +#include "diskformat.h"
>> +
>> +void diskformat_handler(void);
>> +
>> +/*
>> + * Checks if file system fstype already exists on device.
>> + * return 0 if not exists, 1 if exists, negative values on failure
>> + */
>> +static int fs_exists(char *device, char *fstype)
>> +{
>> +    char buf[10];
>> +    const char *value = buf;
>> +    size_t len;
>> +    blkid_probe pr;
>> +    int ret = 0;
>> +
>> +    pr = blkid_new_probe_from_filename(device);
>> +
>> +    if (!pr) {
>> +        ERROR("%s: failed to create libblkid probe",
>> +              device);
>> +        return -EFAULT;
>> +    }
>> +
>> +    while (blkid_do_probe(pr) == 0) {
>> +        if (blkid_probe_lookup_value(pr, "TYPE", &value, &len)) {
>> +            ERROR("blkid_probe_lookup_value failed");
>> +            ret = -EFAULT;
>> +            break;
>> +        }
>> +
>> +        if (!strncmp(value, fstype, sizeof(buf))) {
>> +            ret = 1;
>> +            break;
>> +        }
>> +    }
>> +    blkid_free_probe(pr);
>> +
>> +    return ret;
>> +}
>> +
>> +static int diskformat(struct img_type *img,
>> +              void __attribute__ ((__unused__)) *data)
>> +{
>> +    int ret = 0;
>> +
>> +    if (!strlen(img->device)) {
>> +        ERROR("diskpart handler requires setting \"device\" attribute");
>> +        return -EINVAL;
>> +    }
>> +
>> +    char *fstype = dict_get_value(&img->properties, "fstype");
>> +
>> +    if (!fstype) {
>> +        ERROR("diskpart handler requires setting \"fstype\" attribute");
>> +        return -EINVAL;
>> +    }
>> +
>> +    char *force = dict_get_value(&img->properties, "force");
>> +
>> +    if (force != NULL && strcmp(force, "true") == 0) {
>> +        ; /* Skip file system exists check */
>> +    } else {
>> +        /* Check if file system exists */
>> +        ret = fs_exists(img->device, fstype);
>> +
>> +        if (ret < 0)
>> +            return ret;
>> +
>> +        if (ret) {
>> +            TRACE("Found %s file system on %s, skip mkfs",
>> +                  fstype, img->device);
>> +            return 0;
>> +        }
>> +    }
>> +
>> +    /* File system does not exist, create new file system */
>> +    ret = diskformat_mkfs(img->device, fstype);
>> +    return ret;
>> +}
>> +
>> +__attribute__((constructor))
>> +void diskformat_handler(void)
>> +{
>> +    register_handler("diskformat", diskformat,
>> +             PARTITION_HANDLER | NO_DATA_HANDLER, NULL);
>> +}
>> diff --git a/handlers/diskpart_handler.c b/handlers/diskpart_handler.c
>> index b5c9a42..272d0b8 100644
>> --- a/handlers/diskpart_handler.c
>> +++ b/handlers/diskpart_handler.c
>> @@ -19,7 +19,7 @@
>>   #include "swupdate.h"
>>   #include "handler.h"
>>   #include "util.h"
>> -#include "fs_interface.h"
>> +#include "diskformat.h"
>>     void diskpart_handler(void);
>>   @@ -31,12 +31,6 @@ void diskpart_handler(void);
>>   /* Linux native partition type */
>>    #define GPT_DEFAULT_ENTRY_TYPE "0FC63DAF-8483-4772-8E79-3D69D8477DE4"
>>   -#if defined (CONFIG_EXT_FILESYSTEM)
>> -static inline int ext_mkfs_short(const char *device_name, const char
>> *fstype) {
>> -    return ext_mkfs(device_name,fstype, 0, NULL);
>> -}
>> -#endif
>> -
>>   /*
>>    * We will only have a parent in hybrid mode.
>>    */
>> @@ -47,22 +41,6 @@ static inline int ext_mkfs_short(const char
>> *device_name, const char *fstype) {
>>    */
>>   #define PARENT(cxt) fdisk_get_parent(cxt) ? fdisk_get_parent(cxt) : cxt
>>   -struct supported_filesystems {
>> -    const char *fstype;
>> -    int    (*mkfs) (const char *device_name, const char *fstype);
>> -};
>> -
>> -static struct supported_filesystems fs[] = {
>> -#if defined(CONFIG_FAT_FILESYSTEM)
>> -    {"vfat", fat_mkfs},
>> -#endif
>> -#if defined (CONFIG_EXT_FILESYSTEM)
>> -    {"ext2", ext_mkfs_short},
>> -    {"ext3", ext_mkfs_short},
>> -    {"ext4", ext_mkfs_short},
>> -#endif
>> -};
>> -
>>   /**
>>    * Keys for the properties field in sw-description
>>    */
>> @@ -942,7 +920,6 @@ handler_release:
>>       /* Create filesystems */
>>       if (!ret && createtable->parent) {
>>           LIST_FOREACH(part, &priv.listparts, next) {
>> -            int index;
>>               /*
>>                * priv.listparts counts partitions starting with 0,
>>                * but fdisk_partname expects the first partition having
>> @@ -952,25 +929,14 @@ handler_release:
>>                 if (!strlen(part->fstype))
>>                   continue;  /* Don't touch partitions without fstype */
>> -            for (index = 0; index < ARRAY_SIZE(fs); index++) {
>> -                if (!strcmp(fs[index].fstype, part->fstype))
>> -                    break;
>> -            }
>> -            if (index >= ARRAY_SIZE(fs)) {
>> -                ERROR("partition-%lu %s filesystem type not
>> supported.", partno, part->fstype);
>> -                ret = -EINVAL;
>> -                break;
>> -            }
>>                 char *device = NULL;
>> +
>>               device = fdisk_partname(img->device, partno);
>> -            TRACE("Creating %s file system on partition-%lu, device
>> %s", part->fstype, partno, device);
>> -            ret = fs[index].mkfs(device, part->fstype);
>> +            ret = diskformat_mkfs(device, part->fstype);
>>               free(device);
>> -            if (ret) {
>> -                ERROR("creating %s file system failed. %d",
>> part->fstype, ret);
>> +            if (ret)
>>                   break;
>> -            }
>>           }
>>       }
>>   #endif
>>
> 
>
Stefano Babic Aug. 6, 2021, 12:07 p.m. UTC | #3
On 06.08.21 13:57, Roland Gaudig wrote:
> Hi Stefano,
> 
> On 06.08.21 10:37, Stefano Babic wrote:
>> Hi Roland,
>>
>> On 02.08.21 16:43, Roland Gaudig wrote:
>>> From: Roland Gaudig <roland.gaudig@weidmueller.com>
>>>
>>> Currently the discpart handler only creates a new file system when the
>>> partition table has changed.
>>>
>>
>> It is fine with me to have a separate handler. Anyway, I see a couple of
>> things:
>>
>> - applying patches, I get several warnings. Please fix them.
>>
>> In file included from handlers/diskformat_common.c:10:
>> include/handler.h:36:31: warning: ‘struct img_type’ declared inside
>> parameter list will not be visible outside of this definition or
>> declaration
>>     36 | typedef int (*handler)(struct img_type *img, void *data);
>>        |                               ^~~~~~~~
>> include/handler.h:47:47: warning: ‘struct img_type’ declared inside
>> parameter list will not be visible outside of this definition or
>> declaration
>>     47 | struct installer_handler *find_handler(struct img_type *img);
>>        |                                               ^~~~~~~~
>> include/handler.h:50:38: warning: ‘struct img_type’ declared inside
>> parameter list will not be visible outside of this definition or
>> declaration
>>     50 | unsigned int get_handler_mask(struct img_type *img);
>>        |                                      ^~~~~~~~
>>    CC      handlers/diskformat_handler.o
>> In file included from handlers/diskformat_handler.c:10:
>> include/handler.h:36:31: warning: ‘struct img_type’ declared inside
>> parameter list will not be visible outside of this definition or
>> declaration
>>     36 | typedef int (*handler)(struct img_type *img, void *data);
>>        |                               ^~~~~~~~
>> include/handler.h:47:47: warning: ‘struct img_type’ declared inside
>> parameter list will not be visible outside of this definition or
>> declaration
>>     47 | struct installer_handler *find_handler(struct img_type *img);
>>        |                                               ^~~~~~~~
>> include/handler.h:50:38: warning: ‘struct img_type’ declared inside
>> parameter list will not be visible outside of this definition or
>> declaration
>>     50 | unsigned int get_handler_mask(struct img_type *img);
>>        |                                      ^~~~~~~~
>> handlers/diskformat_handler.c: In function ‘diskformat_handler’:
>> handlers/diskformat_handler.c:99:33: warning: passing argument 2 of
>> ‘register_handler’ from incompatible pointer type
>> [-Wincompatible-pointer-types]
>>     99 |  register_handler("diskformat", diskformat,
>>        |                                 ^~~~~~~~~~
>>        |                                 |
>>        |                                 int (*)(struct img_type *, void *)
> 
> Oh, you are rigt. I will correct them.
> 
>> - the common code (diskformat.c, mainly) should be better put in a
>> central place, what about fs/ ?
> 
> That sounds like a good idea. I will move handlers/diskformat_common.c
> to fs/diskformat.c

Fine. Do not forget to change all_handlers_defconfig, too, so that your 
handlers is automatically checked by CI.

Regards,
Stefano

> 
>> Best regards,
>> Stefano
>>
>>> In cases where there exists an unused and unformatted spare partion which
>>> shall be activated by a later update, it is necessary to format such a
>>> partition even if the partition table has not been altered.
>>>
>>> Also there are cases when creating a file system on a single partition is
>>> desired, but there is no partitioning scheme specified inside the
>>> sw-description.
>>>
>>> Therefore, this commit introduces a new diskformat handler which allows
>>> creating a file system independently from the partitioning process.
>>> Before creating a new file system it checks whether the desired file
>>> system
>>> already exists and only writes if not.
>>>
>>> In case it is desired to create a new file system anyway, this can be
>>> achieved by setting the property "force" to "true".
>>>
>>>      partitions: (
>>>      {
>>>          type = "diskformat";
>>>          device = "/dev/mmcblk0p1";
>>>
>>>          properties: {
>>>              fstype = "vfat";
>>>              force = "true";
>>>          }
>>>      })
>>>
>>> The diskformat handler will be active when the configuration option
>>> CONFIG_DISKFORMAT_HANDLER is selected.
>>>
>>> Signed-off-by: Roland Gaudig <roland.gaudig@weidmueller.com>
>>> ---
>>>
>>>    Makefile.flags                |   4 ++
>>>    handlers/Config.in            |  18 +++---
>>>    handlers/Makefile             |   2 +
>>>    handlers/diskformat.h         |  13 +++++
>>>    handlers/diskformat_common.c  |  68 +++++++++++++++++++++++
>>>    handlers/diskformat_handler.c | 100 ++++++++++++++++++++++++++++++++++
>>>    handlers/diskpart_handler.c   |  42 ++------------
>>>    7 files changed, 201 insertions(+), 46 deletions(-)
>>>    create mode 100644 handlers/diskformat.h
>>>    create mode 100644 handlers/diskformat_common.c
>>>    create mode 100644 handlers/diskformat_handler.c
>>>
>>> diff --git a/Makefile.flags b/Makefile.flags
>>> index d3ca49d..e549b46 100644
>>> --- a/Makefile.flags
>>> +++ b/Makefile.flags
>>> @@ -185,6 +185,10 @@ ifeq ($(CONFIG_DISKPART),y)
>>>    LDLIBS += fdisk
>>>    endif
>>>    +ifeq ($(CONFIG_DISKFORMAT_HANDLER),y)
>>> +LDLIBS += blkid
>>> +endif
>>> +
>>>    ifeq ($(CONFIG_EXT_FILESYSTEM),y)
>>>    LDLIBS += ext2fs uuid blkid
>>>    endif
>>> diff --git a/handlers/Config.in b/handlers/Config.in
>>> index a0cc5a3..36f3ca4 100644
>>> --- a/handlers/Config.in
>>> +++ b/handlers/Config.in
>>> @@ -106,15 +106,19 @@ config DISKPART
>>>    comment "diskpart support needs libfdisk"
>>>        depends on !HAVE_LIBFDISK
>>>    -if DISKPART
>>> -
>>> -menuconfig DISKFORMAT
>>> -        bool "diskpart extension for creating file systems"
>>> +config DISKFORMAT
>>> +    bool "diskpart extension for creating file systems"
>>>        depends on DISKPART
>>> +    help
>>> +      This extension allows formatting newly created partitions.
>>> +
>>> +config DISKFORMAT_HANDLER
>>> +    bool "diskformat handler for creating file systems"
>>> +    select DISKFORMAT
>>>        default n
>>>        help
>>> -      This extension of the diskpart handler allows creating filesystems
>>> -      on empty partitions.
>>> +      The diskformat handler allows creating filesystems on empty
>>> +      partitions.
>>>      if DISKFORMAT
>>>    @@ -122,8 +126,6 @@ source fs/Config.in
>>>      endif
>>>    -endif
>>> -
>>>    config UNIQUEUUID
>>>        bool "uniqueuuid"
>>>        depends on HAVE_LIBBLKID
>>> diff --git a/handlers/Makefile b/handlers/Makefile
>>> index 1f61f60..a37ce78 100644
>>> --- a/handlers/Makefile
>>> +++ b/handlers/Makefile
>>> @@ -11,6 +11,8 @@ obj-y    += dummy_handler.o
>>>    obj-$(CONFIG_ARCHIVE) += archive_handler.o
>>>    obj-$(CONFIG_BOOTLOADERHANDLER) += boot_handler.o
>>>    obj-$(CONFIG_CFI)    += flash_handler.o
>>> +obj-$(CONFIG_DISKFORMAT)    += diskformat_common.o
>>> +obj-$(CONFIG_DISKFORMAT_HANDLER)    += diskformat_handler.o
>>>    obj-$(CONFIG_DISKPART)    += diskpart_handler.o
>>>    obj-$(CONFIG_UNIQUEUUID)    += uniqueuuid_handler.o
>>>    obj-$(CONFIG_CFIHAMMING1)+= flash_hamming1_handler.o
>>> diff --git a/handlers/diskformat.h b/handlers/diskformat.h
>>> new file mode 100644
>>> index 0000000..441d291
>>> --- /dev/null
>>> +++ b/handlers/diskformat.h
>>> @@ -0,0 +1,13 @@
>>> +/*
>>> + * Copyright (C) 2021 Weidmueller Interface GmbH & Co. KG
>>> + * Roland Gaudig <roland.gaudig@weidmueller.com>
>>> + *
>>> + * SPDX-License-Identifier:     GPL-2.0-only
>>> + */
>>> +
>>> +#ifndef _DISKFORMAT_H
>>> +#define _DISKFORMAT_H
>>> +
>>> +int diskformat_mkfs(char *device, char *fstype);
>>> +
>>> +#endif
>>> diff --git a/handlers/diskformat_common.c b/handlers/diskformat_common.c
>>> new file mode 100644
>>> index 0000000..7f21c90
>>> --- /dev/null
>>> +++ b/handlers/diskformat_common.c
>>> @@ -0,0 +1,68 @@
>>> +/*
>>> + * Copyright (C) 2021 Weidmueller Interface GmbH & Co. KG
>>> + * Roland Gaudig <roland.gaudig@weidmueller.com>
>>> + *
>>> + * SPDX-License-Identifier:     GPL-2.0-only
>>> + */
>>> +
>>> +#include <errno.h>
>>> +#include <stdio.h>
>>> +#include <handler.h>
>>> +#include <util.h>
>>> +#include <blkid/blkid.h>
>>> +#include <fs_interface.h>
>>> +#include "diskformat.h"
>>> +
>>> +#if defined(CONFIG_EXT_FILESYSTEM)
>>> +static inline int ext_mkfs_short(const char *device_name, const char
>>> *fstype)
>>> +{
>>> +    return ext_mkfs(device_name, fstype, 0, NULL);
>>> +}
>>> +#endif
>>> +
>>> +struct supported_filesystems {
>>> +    const char *fstype;
>>> +    int (*mkfs)(const char *device_name, const char *fstype);
>>> +};
>>> +
>>> +static struct supported_filesystems fs[] = {
>>> +#if defined(CONFIG_FAT_FILESYSTEM)
>>> +    {"vfat", fat_mkfs},
>>> +#endif
>>> +#if defined(CONFIG_EXT_FILESYSTEM)
>>> +    {"ext2", ext_mkfs_short},
>>> +    {"ext3", ext_mkfs_short},
>>> +    {"ext4", ext_mkfs_short},
>>> +#endif
>>> +};
>>> +
>>> +int diskformat_mkfs(char *device, char *fstype)
>>> +{
>>> +    int index;
>>> +    int ret = 0;
>>> +
>>> +    if (!device || !fstype) {
>>> +        ERROR("Uninitialized pointer as device/fstype argument");
>>> +        return -EINVAL;
>>> +    }
>>> +
>>> +    for (index = 0; index < ARRAY_SIZE(fs); index++) {
>>> +        if (!strcmp(fs[index].fstype, fstype))
>>> +            break;
>>> +    }
>>> +    if (index >= ARRAY_SIZE(fs)) {
>>> +        ERROR("%s file system type not supported.", fstype);
>>> +        return -EINVAL;
>>> +    }
>>> +
>>> +    TRACE("Creating %s file system on %s", fstype, device);
>>> +    ret = fs[index].mkfs(device, fstype);
>>> +
>>> +    if (ret) {
>>> +        ERROR("creating %s file system on %s failed. %d",
>>> +              fstype, device, ret);
>>> +        return -EFAULT;
>>> +    }
>>> +
>>> +    return ret;
>>> +}
>>> diff --git a/handlers/diskformat_handler.c
>>> b/handlers/diskformat_handler.c
>>> new file mode 100644
>>> index 0000000..a4449cf
>>> --- /dev/null
>>> +++ b/handlers/diskformat_handler.c
>>> @@ -0,0 +1,100 @@
>>> +/*
>>> + * Copyright (C) 2021 Weidmueller Interface GmbH & Co. KG
>>> + * Roland Gaudig <roland.gaudig@weidmueller.com>
>>> + *
>>> + * SPDX-License-Identifier:     GPL-2.0-only
>>> + */
>>> +
>>> +#include <errno.h>
>>> +#include <stdio.h>
>>> +#include <handler.h>
>>> +#include <util.h>
>>> +#include <blkid/blkid.h>
>>> +#include <fs_interface.h>
>>> +#include "diskformat.h"
>>> +
>>> +void diskformat_handler(void);
>>> +
>>> +/*
>>> + * Checks if file system fstype already exists on device.
>>> + * return 0 if not exists, 1 if exists, negative values on failure
>>> + */
>>> +static int fs_exists(char *device, char *fstype)
>>> +{
>>> +    char buf[10];
>>> +    const char *value = buf;
>>> +    size_t len;
>>> +    blkid_probe pr;
>>> +    int ret = 0;
>>> +
>>> +    pr = blkid_new_probe_from_filename(device);
>>> +
>>> +    if (!pr) {
>>> +        ERROR("%s: failed to create libblkid probe",
>>> +              device);
>>> +        return -EFAULT;
>>> +    }
>>> +
>>> +    while (blkid_do_probe(pr) == 0) {
>>> +        if (blkid_probe_lookup_value(pr, "TYPE", &value, &len)) {
>>> +            ERROR("blkid_probe_lookup_value failed");
>>> +            ret = -EFAULT;
>>> +            break;
>>> +        }
>>> +
>>> +        if (!strncmp(value, fstype, sizeof(buf))) {
>>> +            ret = 1;
>>> +            break;
>>> +        }
>>> +    }
>>> +    blkid_free_probe(pr);
>>> +
>>> +    return ret;
>>> +}
>>> +
>>> +static int diskformat(struct img_type *img,
>>> +              void __attribute__ ((__unused__)) *data)
>>> +{
>>> +    int ret = 0;
>>> +
>>> +    if (!strlen(img->device)) {
>>> +        ERROR("diskpart handler requires setting \"device\" attribute");
>>> +        return -EINVAL;
>>> +    }
>>> +
>>> +    char *fstype = dict_get_value(&img->properties, "fstype");
>>> +
>>> +    if (!fstype) {
>>> +        ERROR("diskpart handler requires setting \"fstype\" attribute");
>>> +        return -EINVAL;
>>> +    }
>>> +
>>> +    char *force = dict_get_value(&img->properties, "force");
>>> +
>>> +    if (force != NULL && strcmp(force, "true") == 0) {
>>> +        ; /* Skip file system exists check */
>>> +    } else {
>>> +        /* Check if file system exists */
>>> +        ret = fs_exists(img->device, fstype);
>>> +
>>> +        if (ret < 0)
>>> +            return ret;
>>> +
>>> +        if (ret) {
>>> +            TRACE("Found %s file system on %s, skip mkfs",
>>> +                  fstype, img->device);
>>> +            return 0;
>>> +        }
>>> +    }
>>> +
>>> +    /* File system does not exist, create new file system */
>>> +    ret = diskformat_mkfs(img->device, fstype);
>>> +    return ret;
>>> +}
>>> +
>>> +__attribute__((constructor))
>>> +void diskformat_handler(void)
>>> +{
>>> +    register_handler("diskformat", diskformat,
>>> +             PARTITION_HANDLER | NO_DATA_HANDLER, NULL);
>>> +}
>>> diff --git a/handlers/diskpart_handler.c b/handlers/diskpart_handler.c
>>> index b5c9a42..272d0b8 100644
>>> --- a/handlers/diskpart_handler.c
>>> +++ b/handlers/diskpart_handler.c
>>> @@ -19,7 +19,7 @@
>>>    #include "swupdate.h"
>>>    #include "handler.h"
>>>    #include "util.h"
>>> -#include "fs_interface.h"
>>> +#include "diskformat.h"
>>>      void diskpart_handler(void);
>>>    @@ -31,12 +31,6 @@ void diskpart_handler(void);
>>>    /* Linux native partition type */
>>>     #define GPT_DEFAULT_ENTRY_TYPE "0FC63DAF-8483-4772-8E79-3D69D8477DE4"
>>>    -#if defined (CONFIG_EXT_FILESYSTEM)
>>> -static inline int ext_mkfs_short(const char *device_name, const char
>>> *fstype) {
>>> -    return ext_mkfs(device_name,fstype, 0, NULL);
>>> -}
>>> -#endif
>>> -
>>>    /*
>>>     * We will only have a parent in hybrid mode.
>>>     */
>>> @@ -47,22 +41,6 @@ static inline int ext_mkfs_short(const char
>>> *device_name, const char *fstype) {
>>>     */
>>>    #define PARENT(cxt) fdisk_get_parent(cxt) ? fdisk_get_parent(cxt) : cxt
>>>    -struct supported_filesystems {
>>> -    const char *fstype;
>>> -    int    (*mkfs) (const char *device_name, const char *fstype);
>>> -};
>>> -
>>> -static struct supported_filesystems fs[] = {
>>> -#if defined(CONFIG_FAT_FILESYSTEM)
>>> -    {"vfat", fat_mkfs},
>>> -#endif
>>> -#if defined (CONFIG_EXT_FILESYSTEM)
>>> -    {"ext2", ext_mkfs_short},
>>> -    {"ext3", ext_mkfs_short},
>>> -    {"ext4", ext_mkfs_short},
>>> -#endif
>>> -};
>>> -
>>>    /**
>>>     * Keys for the properties field in sw-description
>>>     */
>>> @@ -942,7 +920,6 @@ handler_release:
>>>        /* Create filesystems */
>>>        if (!ret && createtable->parent) {
>>>            LIST_FOREACH(part, &priv.listparts, next) {
>>> -            int index;
>>>                /*
>>>                 * priv.listparts counts partitions starting with 0,
>>>                 * but fdisk_partname expects the first partition having
>>> @@ -952,25 +929,14 @@ handler_release:
>>>                  if (!strlen(part->fstype))
>>>                    continue;  /* Don't touch partitions without fstype */
>>> -            for (index = 0; index < ARRAY_SIZE(fs); index++) {
>>> -                if (!strcmp(fs[index].fstype, part->fstype))
>>> -                    break;
>>> -            }
>>> -            if (index >= ARRAY_SIZE(fs)) {
>>> -                ERROR("partition-%lu %s filesystem type not
>>> supported.", partno, part->fstype);
>>> -                ret = -EINVAL;
>>> -                break;
>>> -            }
>>>                  char *device = NULL;
>>> +
>>>                device = fdisk_partname(img->device, partno);
>>> -            TRACE("Creating %s file system on partition-%lu, device
>>> %s", part->fstype, partno, device);
>>> -            ret = fs[index].mkfs(device, part->fstype);
>>> +            ret = diskformat_mkfs(device, part->fstype);
>>>                free(device);
>>> -            if (ret) {
>>> -                ERROR("creating %s file system failed. %d",
>>> part->fstype, ret);
>>> +            if (ret)
>>>                    break;
>>> -            }
>>>            }
>>>        }
>>>    #endif
>>>
>>
>>
diff mbox series

Patch

diff --git a/Makefile.flags b/Makefile.flags
index d3ca49d..e549b46 100644
--- a/Makefile.flags
+++ b/Makefile.flags
@@ -185,6 +185,10 @@  ifeq ($(CONFIG_DISKPART),y)
 LDLIBS += fdisk
 endif
 
+ifeq ($(CONFIG_DISKFORMAT_HANDLER),y)
+LDLIBS += blkid
+endif
+
 ifeq ($(CONFIG_EXT_FILESYSTEM),y)
 LDLIBS += ext2fs uuid blkid
 endif
diff --git a/handlers/Config.in b/handlers/Config.in
index a0cc5a3..36f3ca4 100644
--- a/handlers/Config.in
+++ b/handlers/Config.in
@@ -106,15 +106,19 @@  config DISKPART
 comment "diskpart support needs libfdisk"
 	depends on !HAVE_LIBFDISK
 
-if DISKPART
-
-menuconfig DISKFORMAT
-        bool "diskpart extension for creating file systems"
+config DISKFORMAT
+	bool "diskpart extension for creating file systems"
 	depends on DISKPART
+	help
+	  This extension allows formatting newly created partitions.
+
+config DISKFORMAT_HANDLER
+	bool "diskformat handler for creating file systems"
+	select DISKFORMAT
 	default n
 	help
-	  This extension of the diskpart handler allows creating filesystems
-	  on empty partitions.
+	  The diskformat handler allows creating filesystems on empty
+	  partitions.
 
 if DISKFORMAT
 
@@ -122,8 +126,6 @@  source fs/Config.in
 
 endif
 
-endif
-
 config UNIQUEUUID
 	bool "uniqueuuid"
 	depends on HAVE_LIBBLKID
diff --git a/handlers/Makefile b/handlers/Makefile
index 1f61f60..a37ce78 100644
--- a/handlers/Makefile
+++ b/handlers/Makefile
@@ -11,6 +11,8 @@  obj-y	+= dummy_handler.o
 obj-$(CONFIG_ARCHIVE) += archive_handler.o
 obj-$(CONFIG_BOOTLOADERHANDLER) += boot_handler.o
 obj-$(CONFIG_CFI)	+= flash_handler.o
+obj-$(CONFIG_DISKFORMAT)	+= diskformat_common.o
+obj-$(CONFIG_DISKFORMAT_HANDLER)	+= diskformat_handler.o
 obj-$(CONFIG_DISKPART)	+= diskpart_handler.o
 obj-$(CONFIG_UNIQUEUUID)	+= uniqueuuid_handler.o
 obj-$(CONFIG_CFIHAMMING1)+= flash_hamming1_handler.o
diff --git a/handlers/diskformat.h b/handlers/diskformat.h
new file mode 100644
index 0000000..441d291
--- /dev/null
+++ b/handlers/diskformat.h
@@ -0,0 +1,13 @@ 
+/*
+ * Copyright (C) 2021 Weidmueller Interface GmbH & Co. KG
+ * Roland Gaudig <roland.gaudig@weidmueller.com>
+ *
+ * SPDX-License-Identifier:     GPL-2.0-only
+ */
+
+#ifndef _DISKFORMAT_H
+#define _DISKFORMAT_H
+
+int diskformat_mkfs(char *device, char *fstype);
+
+#endif
diff --git a/handlers/diskformat_common.c b/handlers/diskformat_common.c
new file mode 100644
index 0000000..7f21c90
--- /dev/null
+++ b/handlers/diskformat_common.c
@@ -0,0 +1,68 @@ 
+/*
+ * Copyright (C) 2021 Weidmueller Interface GmbH & Co. KG
+ * Roland Gaudig <roland.gaudig@weidmueller.com>
+ *
+ * SPDX-License-Identifier:     GPL-2.0-only
+ */
+
+#include <errno.h>
+#include <stdio.h>
+#include <handler.h>
+#include <util.h>
+#include <blkid/blkid.h>
+#include <fs_interface.h>
+#include "diskformat.h"
+
+#if defined(CONFIG_EXT_FILESYSTEM)
+static inline int ext_mkfs_short(const char *device_name, const char *fstype)
+{
+	return ext_mkfs(device_name, fstype, 0, NULL);
+}
+#endif
+
+struct supported_filesystems {
+	const char *fstype;
+	int (*mkfs)(const char *device_name, const char *fstype);
+};
+
+static struct supported_filesystems fs[] = {
+#if defined(CONFIG_FAT_FILESYSTEM)
+	{"vfat", fat_mkfs},
+#endif
+#if defined(CONFIG_EXT_FILESYSTEM)
+	{"ext2", ext_mkfs_short},
+	{"ext3", ext_mkfs_short},
+	{"ext4", ext_mkfs_short},
+#endif
+};
+
+int diskformat_mkfs(char *device, char *fstype)
+{
+	int index;
+	int ret = 0;
+
+	if (!device || !fstype) {
+		ERROR("Uninitialized pointer as device/fstype argument");
+		return -EINVAL;
+	}
+
+	for (index = 0; index < ARRAY_SIZE(fs); index++) {
+		if (!strcmp(fs[index].fstype, fstype))
+			break;
+	}
+	if (index >= ARRAY_SIZE(fs)) {
+		ERROR("%s file system type not supported.", fstype);
+		return -EINVAL;
+	}
+
+	TRACE("Creating %s file system on %s", fstype, device);
+	ret = fs[index].mkfs(device, fstype);
+
+	if (ret) {
+		ERROR("creating %s file system on %s failed. %d",
+		      fstype, device, ret);
+		return -EFAULT;
+	}
+
+	return ret;
+}
diff --git a/handlers/diskformat_handler.c b/handlers/diskformat_handler.c
new file mode 100644
index 0000000..a4449cf
--- /dev/null
+++ b/handlers/diskformat_handler.c
@@ -0,0 +1,100 @@ 
+/*
+ * Copyright (C) 2021 Weidmueller Interface GmbH & Co. KG
+ * Roland Gaudig <roland.gaudig@weidmueller.com>
+ *
+ * SPDX-License-Identifier:     GPL-2.0-only
+ */
+
+#include <errno.h>
+#include <stdio.h>
+#include <handler.h>
+#include <util.h>
+#include <blkid/blkid.h>
+#include <fs_interface.h>
+#include "diskformat.h"
+
+void diskformat_handler(void);
+
+/*
+ * Checks if file system fstype already exists on device.
+ * return 0 if not exists, 1 if exists, negative values on failure
+ */
+static int fs_exists(char *device, char *fstype)
+{
+	char buf[10];
+	const char *value = buf;
+	size_t len;
+	blkid_probe pr;
+	int ret = 0;
+
+	pr = blkid_new_probe_from_filename(device);
+
+	if (!pr) {
+		ERROR("%s: failed to create libblkid probe",
+		      device);
+		return -EFAULT;
+	}
+
+	while (blkid_do_probe(pr) == 0) {
+		if (blkid_probe_lookup_value(pr, "TYPE", &value, &len)) {
+			ERROR("blkid_probe_lookup_value failed");
+			ret = -EFAULT;
+			break;
+		}
+
+		if (!strncmp(value, fstype, sizeof(buf))) {
+			ret = 1;
+			break;
+		}
+	}
+	blkid_free_probe(pr);
+
+	return ret;
+}
+
+static int diskformat(struct img_type *img,
+		      void __attribute__ ((__unused__)) *data)
+{
+	int ret = 0;
+
+	if (!strlen(img->device)) {
+		ERROR("diskpart handler requires setting \"device\" attribute");
+		return -EINVAL;
+	}
+
+	char *fstype = dict_get_value(&img->properties, "fstype");
+
+	if (!fstype) {
+		ERROR("diskpart handler requires setting \"fstype\" attribute");
+		return -EINVAL;
+	}
+
+	char *force = dict_get_value(&img->properties, "force");
+
+	if (force != NULL && strcmp(force, "true") == 0) {
+		; /* Skip file system exists check */
+	} else {
+		/* Check if file system exists */
+		ret = fs_exists(img->device, fstype);
+
+		if (ret < 0)
+			return ret;
+
+		if (ret) {
+			TRACE("Found %s file system on %s, skip mkfs",
+			      fstype, img->device);
+			return 0;
+		}
+	}
+
+	/* File system does not exist, create new file system */
+	ret = diskformat_mkfs(img->device, fstype);
+	return ret;
+}
+
+__attribute__((constructor))
+void diskformat_handler(void)
+{
+	register_handler("diskformat", diskformat,
+			 PARTITION_HANDLER | NO_DATA_HANDLER, NULL);
+}
diff --git a/handlers/diskpart_handler.c b/handlers/diskpart_handler.c
index b5c9a42..272d0b8 100644
--- a/handlers/diskpart_handler.c
+++ b/handlers/diskpart_handler.c
@@ -19,7 +19,7 @@ 
 #include "swupdate.h"
 #include "handler.h"
 #include "util.h"
-#include "fs_interface.h"
+#include "diskformat.h"
 
 void diskpart_handler(void);
 
@@ -31,12 +31,6 @@  void diskpart_handler(void);
 /* Linux native partition type */
  #define GPT_DEFAULT_ENTRY_TYPE "0FC63DAF-8483-4772-8E79-3D69D8477DE4"
 
-#if defined (CONFIG_EXT_FILESYSTEM)
-static inline int ext_mkfs_short(const char *device_name, const char *fstype) {
-	return ext_mkfs(device_name,fstype, 0, NULL);
-}
-#endif
-
 /*
  * We will only have a parent in hybrid mode.
  */
@@ -47,22 +41,6 @@  static inline int ext_mkfs_short(const char *device_name, const char *fstype) {
  */
 #define PARENT(cxt) fdisk_get_parent(cxt) ? fdisk_get_parent(cxt) : cxt
 
-struct supported_filesystems {
-	const char *fstype;
-	int	(*mkfs) (const char *device_name, const char *fstype);
-};
-
-static struct supported_filesystems fs[] = {
-#if defined(CONFIG_FAT_FILESYSTEM)
-	{"vfat", fat_mkfs},
-#endif
-#if defined (CONFIG_EXT_FILESYSTEM)
-	{"ext2", ext_mkfs_short},
-	{"ext3", ext_mkfs_short},
-	{"ext4", ext_mkfs_short},
-#endif
-};
-
 /**
  * Keys for the properties field in sw-description
  */
@@ -942,7 +920,6 @@  handler_release:
 	/* Create filesystems */
 	if (!ret && createtable->parent) {
 		LIST_FOREACH(part, &priv.listparts, next) {
-			int index;
 			/*
 			 * priv.listparts counts partitions starting with 0,
 			 * but fdisk_partname expects the first partition having
@@ -952,25 +929,14 @@  handler_release:
 
 			if (!strlen(part->fstype))
 				continue;  /* Don't touch partitions without fstype */
-			for (index = 0; index < ARRAY_SIZE(fs); index++) {
-				if (!strcmp(fs[index].fstype, part->fstype))
-					break;
-			}
-			if (index >= ARRAY_SIZE(fs)) {
-				ERROR("partition-%lu %s filesystem type not supported.", partno, part->fstype);
-				ret = -EINVAL;
-				break;
-			}
 
 			char *device = NULL;
+
 			device = fdisk_partname(img->device, partno);
-			TRACE("Creating %s file system on partition-%lu, device %s", part->fstype, partno, device);
-			ret = fs[index].mkfs(device, part->fstype);
+			ret = diskformat_mkfs(device, part->fstype);
 			free(device);
-			if (ret) {
-				ERROR("creating %s file system failed. %d", part->fstype, ret);
+			if (ret)
 				break;
-			}
 		}
 	}
 #endif