| Message ID | 20210728140238.8708-1-roland.gaudig-oss@weidmueller.com |
|---|---|
| Headers | show |
| Series | handlers: diskpart: Add check for existing filesystem | expand |
Hi Roland, On 28.07.21 16:02, Roland Gaudig wrote: > From: Roland Gaudig <roland.gaudig@weidmueller.com> > > > This patch series adds two changes. > > The first changes allows switching off the check if partition table has > been altered and not creating a file system if not, > wich was introduces with commit 3c70984c6880 > "diskpart: format filesystem only if partition table is changed". > > The reason for this change is, there are situations when there is an > unused and unformatted partition, which shall be made usable by a later > update. In this situation it is necessary to have the possebility to > format that partition even though the partition table remains unchanged. > Ok, I agree this is a use case. > It is controlled with the config DISKFORMAT_IGNORE_PT_CHANGE option. > I disagree with the implementation. This means you know just at compile time if a filesystem should be formatted or not, and it is nonsense. We should use a property for the diskpart handler. Something like "force-fs" or whatever. > The second patch introduces a check whether already a file system > exists on a partition before creating a new file system. > And this flag can is also well suitable to be set via a property instead of a CONFIG_. > Before creating a file system it is checked if the requested file system > already exists on that partition. > > In case the requested file system is of different type than the existing > file system, the existing file system will be overwritten. > > The reason for this change is, there are situations when there was > an unformated partition in the past which shall be used after an update. > Therefore the sw-description contains a fstype option. Now it can happen > such an update is run more than once. In such case already existing data > on that partition would be erased. > > This option is controlled with config DISKFORMAT_CHECK_OVERWRITE > > > > Roland Gaudig (2): > diskpart: format partition anyway > diskpart: format only if filesystem not exists > > Makefile.flags | 6 +++- > handlers/Config.in | 21 ++++++++++++++ > handlers/diskpart_handler.c | 58 ++++++++++++++++++++++++++++++++++++- > 3 files changed, 83 insertions(+), 2 deletions(-) > Best regards, Stefano Babic
Hi Stefano, On 28.07.21 14:26, Stefano Babic wrote: > Hi Roland, > > On 28.07.21 16:02, Roland Gaudig wrote: >> From: Roland Gaudig <roland.gaudig@weidmueller.com> >> >> >> This patch series adds two changes. >> >> The first changes allows switching off the check if partition table has >> been altered and not creating a file system if not, >> wich was introduces with commit 3c70984c6880 >> "diskpart: format filesystem only if partition table is changed". >> >> The reason for this change is, there are situations when there is an >> unused and unformatted partition, which shall be made usable by a later >> update. In this situation it is necessary to have the possebility to >> format that partition even though the partition table remains unchanged. >> > > Ok, I agree this is a use case. > >> It is controlled with the config DISKFORMAT_IGNORE_PT_CHANGE option. >> > > I disagree with the implementation. This means you know just at compile > time if a filesystem should be formatted or not, and it is nonsense. > > We should use a property for the diskpart handler. Something like > "force-fs" or whatever. OK, that is possible how should the default behaviour look like, when this property is not given.? >> The second patch introduces a check whether already a file system >> exists on a partition before creating a new file system. >> > > And this flag can is also well suitable to be set via a property instead > of a CONFIG_. Yes, what should be the default behaviour if this property is not given? I am asking because in our use case we need the file system to be created even if the partition table was not changed. And we also need the check before writing a file system if it already exists. In worst case our sw-description would contain both properties. On the other hand when the check if the partition table has been altered is enabled, further checking if the file system exists is nonsense. Therefore, Stefan Herbrechtsmeier suggests to add a single "fs-ensure" flag which disables the check if the partition table has changed and enables checking if a file system exists before writing. >> Before creating a file system it is checked if the requested file system >> already exists on that partition. >> >> In case the requested file system is of different type than the existing >> file system, the existing file system will be overwritten. >> >> The reason for this change is, there are situations when there was >> an unformated partition in the past which shall be used after an update. >> Therefore the sw-description contains a fstype option. Now it can happen >> such an update is run more than once. In such case already existing data >> on that partition would be erased. > >> This option is controlled with config DISKFORMAT_CHECK_OVERWRITE >> >> >> >> Roland Gaudig (2): >> diskpart: format partition anyway >> diskpart: format only if filesystem not exists >> >> Makefile.flags | 6 +++- >> handlers/Config.in | 21 ++++++++++++++ >> handlers/diskpart_handler.c | 58 ++++++++++++++++++++++++++++++++++++- >> 3 files changed, 83 insertions(+), 2 deletions(-) >> > > Best regards, > Stefano Babic > Best regards, Roland Gaudig
Hi Roland, On 28.07.21 16:55, Roland Gaudig wrote: > Hi Stefano, > > On 28.07.21 14:26, Stefano Babic wrote: >> Hi Roland, >> >> On 28.07.21 16:02, Roland Gaudig wrote: >>> From: Roland Gaudig <roland.gaudig@weidmueller.com> >>> >>> >>> This patch series adds two changes. >>> >>> The first changes allows switching off the check if partition table has >>> been altered and not creating a file system if not, >>> wich was introduces with commit 3c70984c6880 >>> "diskpart: format filesystem only if partition table is changed". >>> >>> The reason for this change is, there are situations when there is an >>> unused and unformatted partition, which shall be made usable by a later >>> update. In this situation it is necessary to have the possebility to >>> format that partition even though the partition table remains unchanged. >>> >> >> Ok, I agree this is a use case. >> >>> It is controlled with the config DISKFORMAT_IGNORE_PT_CHANGE option. >>> >> >> I disagree with the implementation. This means you know just at compile >> time if a filesystem should be formatted or not, and it is nonsense. >> >> We should use a property for the diskpart handler. Something like >> "force-fs" or whatever. > > OK, that is possible how should the default behaviour look like, when > this property is not given.? If we agree this is called like "force-fs", if not set the filesystem is created only if partition table is altered, like today. > >>> The second patch introduces a check whether already a file system >>> exists on a partition before creating a new file system. >>> >> >> And this flag can is also well suitable to be set via a property instead >> of a CONFIG_. > > Yes, what should be the default behaviour if this property is not given? IMO no check if the flag is not set. > > I am asking because in our use case we need the file system to be > created even if the partition table was not changed. And we also need > the check before writing a file system if it already exists. > > In worst case our sw-description would contain both properties. > Right. > On the other hand when the check if the partition table has been altered > is enabled, further checking if the file system exists is nonsense. Right, fully agree. > Therefore, Stefan Herbrechtsmeier suggests to add a single "fs-ensure" > flag which disables the check if the partition table has changed and > enables checking if a file system exists before writing. If partition table was changed, the two flags should be ignored and filesystem is always created. However, you can have also the case, where the user requires always to create the filesystem, independently if the filesystem is ok and can be mounted. In case partition table was not changed, such as use case require first flag to be set, second flag to be off. I can imagine this is a use case, too. > >>> Before creating a file system it is checked if the requested file system >>> already exists on that partition. >>> >>> In case the requested file system is of different type than the existing >>> file system, the existing file system will be overwritten. >>> >>> The reason for this change is, there are situations when there was >>> an unformated partition in the past which shall be used after an update. >>> Therefore the sw-description contains a fstype option. Now it can happen >>> such an update is run more than once. In such case already existing data >>> on that partition would be erased. > >>> This option is controlled with config DISKFORMAT_CHECK_OVERWRITE >>> >>> >>> >>> Roland Gaudig (2): >>> diskpart: format partition anyway >>> diskpart: format only if filesystem not exists >>> >>> Makefile.flags | 6 +++- >>> handlers/Config.in | 21 ++++++++++++++ >>> handlers/diskpart_handler.c | 58 ++++++++++++++++++++++++++++++++++++- >>> 3 files changed, 83 insertions(+), 2 deletions(-) >>> >> Best regards, Stefano Babic
On Wed, Jul 28, 2021 at 9:40 AM Stefano Babic <sbabic@denx.de> wrote: > > Hi Roland, > > On 28.07.21 16:55, Roland Gaudig wrote: > > Hi Stefano, > > > > On 28.07.21 14:26, Stefano Babic wrote: > >> Hi Roland, > >> > >> On 28.07.21 16:02, Roland Gaudig wrote: > >>> From: Roland Gaudig <roland.gaudig@weidmueller.com> > >>> > >>> > >>> This patch series adds two changes. > >>> > >>> The first changes allows switching off the check if partition table has > >>> been altered and not creating a file system if not, > >>> wich was introduces with commit 3c70984c6880 > >>> "diskpart: format filesystem only if partition table is changed". > >>> > >>> The reason for this change is, there are situations when there is an > >>> unused and unformatted partition, which shall be made usable by a later > >>> update. In this situation it is necessary to have the possebility to > >>> format that partition even though the partition table remains unchanged. > >>> > >> > >> Ok, I agree this is a use case. > >> > >>> It is controlled with the config DISKFORMAT_IGNORE_PT_CHANGE option. > >>> > >> > >> I disagree with the implementation. This means you know just at compile > >> time if a filesystem should be formatted or not, and it is nonsense. > >> > >> We should use a property for the diskpart handler. Something like > >> "force-fs" or whatever. > > > > OK, that is possible how should the default behaviour look like, when > > this property is not given.? > > If we agree this is called like "force-fs", if not set the filesystem is > created only if partition table is altered, like today. > > > > >>> The second patch introduces a check whether already a file system > >>> exists on a partition before creating a new file system. > >>> > >> > >> And this flag can is also well suitable to be set via a property instead > >> of a CONFIG_. > > > > Yes, what should be the default behaviour if this property is not given? > > IMO no check if the flag is not set. > > > > > I am asking because in our use case we need the file system to be > > created even if the partition table was not changed. And we also need > > the check before writing a file system if it already exists. > > > > In worst case our sw-description would contain both properties. > > > > Right. > > > On the other hand when the check if the partition table has been altered > > is enabled, further checking if the file system exists is nonsense. > > Right, fully agree. > > > Therefore, Stefan Herbrechtsmeier suggests to add a single "fs-ensure" > > flag which disables the check if the partition table has changed and > > enables checking if a file system exists before writing. > > If partition table was changed, the two flags should be ignored and > filesystem is always created. > > However, you can have also the case, where the user requires always to > create the filesystem, independently if the filesystem is ok and can be > mounted. In case partition table was not changed, such as use case > require first flag to be set, second flag to be off. I can imagine this > is a use case, too. Yeah, this would be useful for me as well I think, currently when doing A/B rotation with tar.xz rootfs extraction I have a lua script format the inactive partition, would be better if diskpart would be able to handle that natively instead. > > > > >>> Before creating a file system it is checked if the requested file system > >>> already exists on that partition. > >>> > >>> In case the requested file system is of different type than the existing > >>> file system, the existing file system will be overwritten. > >>> > >>> The reason for this change is, there are situations when there was > >>> an unformated partition in the past which shall be used after an update. > >>> Therefore the sw-description contains a fstype option. Now it can happen > >>> such an update is run more than once. In such case already existing data > >>> on that partition would be erased. > > >>> This option is controlled with config DISKFORMAT_CHECK_OVERWRITE > >>> > >>> > >>> > >>> Roland Gaudig (2): > >>> diskpart: format partition anyway > >>> diskpart: format only if filesystem not exists > >>> > >>> Makefile.flags | 6 +++- > >>> handlers/Config.in | 21 ++++++++++++++ > >>> handlers/diskpart_handler.c | 58 ++++++++++++++++++++++++++++++++++++- > >>> 3 files changed, 83 insertions(+), 2 deletions(-) > >>> > >> > > 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: sbabic@denx.de > ===================================================================== > > -- > 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+unsubscribe@googlegroups.com. > To view this discussion on the web visit https://groups.google.com/d/msgid/swupdate/13efe517-2485-7b66-aab3-fbdb0e45fef1%40denx.de.
Hello, the documentation recommends to prepare patches with the patman tool. Now I am confused with patman as it runs checkpatch.pl, which complains about the SPDX license identifier being in the wrong line. It insists, the SPDX license identifier should be in the first line, while most of the files inside the swupdate project put it at lower lines. The swupdate documentation recommends using the SPDX license identifier, but does not tell which line it should be. https://sbabic.github.io/swupdate/licensing.html Is there any recommendation, where should I put the SPDX license identifier? Best Regards, Roland Gaudig
Hi Roland, On 02.08.21 10:48, Roland Gaudig wrote: > Hello, > > the documentation recommends to prepare patches with the patman tool. > Well, this is maybe too polarised because I am U-Boot maintainer, too, and I am using for that project. > Now I am confused with patman as it runs checkpatch.pl, which complains > about the SPDX license identifier being in the wrong line. Just ignore them: U-Boot still uses old SPDX identifier, just sees GPL-2.0+ , and check https://spdx.org/licenses/. I have already upgraded SWUpdate to new identifier, also to be at the end compliant with REUSE. > It insists, the SPDX license identifier should be in the first line, > while most of the files inside the swupdate project put it at lower lines. I guess this was introduced later by kernel. In kernel, SPDX is set at first line, and checkpatch was adjusted to reflect this change. However, in SPDX documentation I have not seen a rule where to put SPDX, just that the line must exists. We ran the REUSE tool without complains about this. > > The swupdate documentation recommends using the SPDX license identifier, > but does not tell which line it should be. > > https://sbabic.github.io/swupdate/licensing.html Right - so in my exes it is ok if it is not on the first line. On the kernel, there was this announcement: https://lwn.net/Articles/739183/ "For kernel source files, the decision was made that the SPDX tag should appear as the first line in the file (or the second line for scripts where the first line must be the #! string). For normal C source files, the string will be a comment using the "//" syntax;" It looks to me a strict rule applied in the kernel development, not required to be compliant to SPDX. > > Is there any recommendation, where should I put the SPDX license identifier? There is no one as in kernel, it is fine for me if you put it as done in the rest of project in lower lines. If it will be introduced a stricter rule to be SPDX compliant, change will be done at once with a single (big) patch. Regards, Stefano
Hi, On Mon, Aug 2, 2021 at 11:09 AM Stefano Babic <sbabic@denx.de> wrote: > Hi Roland, > > On 02.08.21 10:48, Roland Gaudig wrote: > > Hello, > > > > the documentation recommends to prepare patches with the patman tool. > > > > Well, this is maybe too polarised because I am U-Boot maintainer, too, > and I am using for that project. > > > Now I am confused with patman as it runs checkpatch.pl, which complains > > about the SPDX license identifier being in the wrong line. > > Just ignore them: U-Boot still uses old SPDX identifier, just sees > GPL-2.0+ , and check https://spdx.org/licenses/. > > I have already upgraded SWUpdate to new identifier, also to be at the > end compliant with REUSE. > > > It insists, the SPDX license identifier should be in the first line, > > while most of the files inside the swupdate project put it at lower > lines. > > I guess this was introduced later by kernel. In kernel, SPDX is set at > first line, and checkpatch was adjusted to reflect this change. > > However, in SPDX documentation I have not seen a rule where to put SPDX, > just that the line must exists. > > We ran the REUSE tool without complains about this. > REUSE only requires that - SPDX identifier tags must be at the top of the file (not necessarily at line 1) - SPDX-License-Identifier tag must be followed by a valid SPDX License Expression https://reuse.software/spec/#comment-headers > > > > The swupdate documentation recommends using the SPDX license identifier, > > but does not tell which line it should be. > > > > https://sbabic.github.io/swupdate/licensing.html > > Right - so in my exes it is ok if it is not on the first line. > > On the kernel, there was this announcement: > > https://lwn.net/Articles/739183/ > > "For kernel source files, the decision was made that the SPDX tag should > appear as the first line in the file (or the second line for scripts > where the first line must be the #! string). For normal C source files, > the string will be a comment using the "//" syntax;" > > It looks to me a strict rule applied in the kernel development, not > required to be compliant to SPDX. > > > > > Is there any recommendation, where should I put the SPDX license > identifier? > > There is no one as in kernel, it is fine for me if you put it as done in > the rest of project in lower lines. If it will be introduced a stricter > rule to be SPDX compliant, change will be done at once with a single > (big) patch. > > Regards, > Stefano > Cheers, Mark
From: Roland Gaudig <roland.gaudig@weidmueller.com> This patch series adds two changes. The first changes allows switching off the check if partition table has been altered and not creating a file system if not, wich was introduces with commit 3c70984c6880 "diskpart: format filesystem only if partition table is changed". The reason for this change is, there are situations when there is an unused and unformatted partition, which shall be made usable by a later update. In this situation it is necessary to have the possebility to format that partition even though the partition table remains unchanged. It is controlled with the config DISKFORMAT_IGNORE_PT_CHANGE option. The second patch introduces a check whether already a file system exists on a partition before creating a new file system. Before creating a file system it is checked if the requested file system already exists on that partition. In case the requested file system is of different type than the existing file system, the existing file system will be overwritten. The reason for this change is, there are situations when there was an unformated partition in the past which shall be used after an update. Therefore the sw-description contains a fstype option. Now it can happen such an update is run more than once. In such case already existing data on that partition would be erased. This option is controlled with config DISKFORMAT_CHECK_OVERWRITE Roland Gaudig (2): diskpart: format partition anyway diskpart: format only if filesystem not exists Makefile.flags | 6 +++- handlers/Config.in | 21 ++++++++++++++ handlers/diskpart_handler.c | 58 ++++++++++++++++++++++++++++++++++++- 3 files changed, 83 insertions(+), 2 deletions(-)