* [PATCH] mkfs: Refuse to initialize a misaligned device if not forced using libblkid
@ 2011-12-02 12:14 Carlos Maiolino
2011-12-13 5:01 ` Eric Sandeen
0 siblings, 1 reply; 4+ messages in thread
From: Carlos Maiolino @ 2011-12-02 12:14 UTC (permalink / raw)
To: xfs; +Cc: Carlos Maiolino
The patch fix the problem about the usage of 4k sector devices when the
device is not properly aligned. It makes mkfs to refuse to initialize a
xfs filesystem if the -f option is not passed at the command line, and
forces a 512b sector size if the user chooses to force the device
initialization.
Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>
---
mkfs/xfs_mkfs.c | 35 +++++++++++++++++++++++++++++------
1 files changed, 29 insertions(+), 6 deletions(-)
diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c
index f527f3d..159bf60 100644
--- a/mkfs/xfs_mkfs.c
+++ b/mkfs/xfs_mkfs.c
@@ -369,8 +369,14 @@ out:
return ret;
}
-static void blkid_get_topology(const char *device, int *sunit, int *swidth, int *sectorsize)
+static void blkid_get_topology(
+ const char *device,
+ int *sunit,
+ int *swidth,
+ int *sectorsize,
+ int force_overwrite)
{
+
blkid_topology tp;
blkid_probe pr;
unsigned long val;
@@ -409,6 +415,15 @@ static void blkid_get_topology(const char *device, int *sunit, int *swidth, int
fprintf(stderr,
_("warning: device is not properly aligned %s\n"),
device);
+
+ if(!force_overwrite) {
+ fprintf(stderr,
+ _("Use -f to force usage of a misaligned device\n"));
+
+ exit(EXIT_FAILURE);
+ }
+ /* Force a 512b sector size if the device is misaligned*/
+ *sectorsize = BBSIZE;
}
blkid_free_probe(pr);
@@ -421,19 +436,23 @@ out_free_probe:
device);
}
-static void get_topology(libxfs_init_t *xi, struct fs_topology *ft)
+static void get_topology(
+ libxfs_init_t *xi,
+ struct fs_topology *ft,
+ int force_overwrite)
{
if (!xi->disfile) {
const char *dfile = xi->volname ? xi->volname : xi->dname;
blkid_get_topology(dfile, &ft->dsunit, &ft->dswidth,
- &ft->sectorsize);
+ &ft->sectorsize, force_overwrite);
}
if (xi->rtname && !xi->risfile) {
int dummy;
- blkid_get_topology(xi->rtname, &dummy, &ft->rtswidth, &dummy);
+ blkid_get_topology(xi->rtname, &dummy, &ft->rtswidth,
+ &dummy, force_overwrite);
}
}
#else /* ENABLE_BLKID */
@@ -460,8 +479,12 @@ check_overwrite(
return 0;
}
-static void get_topology(libxfs_init_t *xi, struct fs_topology *ft)
+static void get_topology(
+ libxfs_init_t *xi,
+ struct fs_topology *ft,
+ int force_overwrite)
{
+
char *dfile = xi->volname ? xi->volname : xi->dname;
int bsz = BBSIZE;
@@ -1625,7 +1648,7 @@ main(
}
memset(&ft, 0, sizeof(ft));
- get_topology(&xi, &ft);
+ get_topology(&xi, &ft, force_overwrite);
if (ft.sectoralign) {
/*
--
1.7.6.4
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH] mkfs: Refuse to initialize a misaligned device if not forced using libblkid
2011-12-02 12:14 [PATCH] mkfs: Refuse to initialize a misaligned device if not forced using libblkid Carlos Maiolino
@ 2011-12-13 5:01 ` Eric Sandeen
0 siblings, 0 replies; 4+ messages in thread
From: Eric Sandeen @ 2011-12-13 5:01 UTC (permalink / raw)
To: Carlos Maiolino; +Cc: xfs
On 12/2/11 6:14 AM, Carlos Maiolino wrote:
> The patch fix the problem about the usage of 4k sector devices when the
> device is not properly aligned. It makes mkfs to refuse to initialize a
> xfs filesystem if the -f option is not passed at the command line, and
> forces a 512b sector size if the user chooses to force the device
> initialization.
[editor]
In mkfs, ensure that the partition is aligned to the device's
physical sector size, so that we do not break atomic IO guarantees
due to read modify write for IOs that span physical blocks.
If an override is forced, set the sector size to the device's
logical sector size as a next-best backup plan.
[/editor]
I think that reads better? (And I think it is correct?)
> Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>
> ---
> mkfs/xfs_mkfs.c | 35 +++++++++++++++++++++++++++++------
> 1 files changed, 29 insertions(+), 6 deletions(-)
>
> diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c
> index f527f3d..159bf60 100644
> --- a/mkfs/xfs_mkfs.c
> +++ b/mkfs/xfs_mkfs.c
> @@ -369,8 +369,14 @@ out:
> return ret;
> }
>
> -static void blkid_get_topology(const char *device, int *sunit, int *swidth, int *sectorsize)
> +static void blkid_get_topology(
> + const char *device,
> + int *sunit,
> + int *swidth,
> + int *sectorsize,
> + int force_overwrite)
> {
> +
> blkid_topology tp;
> blkid_probe pr;
> unsigned long val;
> @@ -409,6 +415,15 @@ static void blkid_get_topology(const char *device, int *sunit, int *swidth, int
> fprintf(stderr,
> _("warning: device is not properly aligned %s\n"),
> device);
> +
> + if(!force_overwrite) {
> + fprintf(stderr,
> + _("Use -f to force usage of a misaligned device\n"));
> +
> + exit(EXIT_FAILURE);
> + }
I think it might be better to send in *offset rather than force_overwrite;
that way the _caller_ can decide what to do with a misaligned partition.
I suppose by that token maybe the warning should be moved out too.
> + /* Force a 512b sector size if the device is misaligned*/
space before "*/" - but perhaps the caller should do this.
This function should just get the topology, not handle commandline
switches, exits, etc IMHO.
> + *sectorsize = BBSIZE;
> }
>
> blkid_free_probe(pr);
> @@ -421,19 +436,23 @@ out_free_probe:
> device);
> }
>
> -static void get_topology(libxfs_init_t *xi, struct fs_topology *ft)
> +static void get_topology(
> + libxfs_init_t *xi,
> + struct fs_topology *ft,
> + int force_overwrite)
> {
> if (!xi->disfile) {
> const char *dfile = xi->volname ? xi->volname : xi->dname;
>
> blkid_get_topology(dfile, &ft->dsunit, &ft->dswidth,
> - &ft->sectorsize);
> + &ft->sectorsize, force_overwrite);
> }
>
> if (xi->rtname && !xi->risfile) {
> int dummy;
>
> - blkid_get_topology(xi->rtname, &dummy, &ft->rtswidth, &dummy);
> + blkid_get_topology(xi->rtname, &dummy, &ft->rtswidth,
> + &dummy, force_overwrite);
> }
so here I'd check alignment, and decide what to do, i.e. something like:
if (!force && (data_offset || rt_offset)) {
sprintf(" ... bad alignment!\n");
sprintf("use -f to force this geometry\n");
exit(EXIT_FAILURE);
}
-Eric
> }
> #else /* ENABLE_BLKID */
> @@ -460,8 +479,12 @@ check_overwrite(
> return 0;
> }
>
> -static void get_topology(libxfs_init_t *xi, struct fs_topology *ft)
> +static void get_topology(
> + libxfs_init_t *xi,
> + struct fs_topology *ft,
> + int force_overwrite)
> {
> +
> char *dfile = xi->volname ? xi->volname : xi->dname;
> int bsz = BBSIZE;
>
> @@ -1625,7 +1648,7 @@ main(
> }
>
> memset(&ft, 0, sizeof(ft));
> - get_topology(&xi, &ft);
> + get_topology(&xi, &ft, force_overwrite);
>
> if (ft.sectoralign) {
> /*
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH] mkfs: Refuse to initialize a misaligned device if not forced using libblkid
@ 2011-12-01 12:44 Carlos Maiolino
2011-12-02 11:27 ` Christoph Hellwig
0 siblings, 1 reply; 4+ messages in thread
From: Carlos Maiolino @ 2011-12-01 12:44 UTC (permalink / raw)
To: xfs; +Cc: Carlos Maiolino
This is the first patch proposal to fix the problem about the usage of
4k sector devices when the device is not properly aligned
The patch make mkfs to refuse to initialize a xfs filesystem if the -f
option is not passed at the command line, and forces a 512b sector size
if the user opt to force the device initialization
Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>
---
mkfs/xfs_mkfs.c | 24 ++++++++++++++++++------
1 files changed, 18 insertions(+), 6 deletions(-)
diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c
index f527f3d..495c70d 100644
--- a/mkfs/xfs_mkfs.c
+++ b/mkfs/xfs_mkfs.c
@@ -369,8 +369,14 @@ out:
return ret;
}
-static void blkid_get_topology(const char *device, int *sunit, int *swidth, int *sectorsize)
+static void blkid_get_topology(
+ const char *device,
+ int *sunit,
+ int *swidth,
+ int *sectorsize,
+ int force_overwrite)
{
+
blkid_topology tp;
blkid_probe pr;
unsigned long val;
@@ -409,6 +415,12 @@ static void blkid_get_topology(const char *device, int *sunit, int *swidth, int
fprintf(stderr,
_("warning: device is not properly aligned %s\n"),
device);
+
+ if(!force_overwrite){
+ fprintf(stderr, _("Use -f to force usage of a misaligned device\n"));
+ exit(EXIT_FAILURE);
+ }
+ *sectorsize = BBSIZE; /* Force a 512b sector size if the device is misaligned*/
}
blkid_free_probe(pr);
@@ -421,19 +433,19 @@ out_free_probe:
device);
}
-static void get_topology(libxfs_init_t *xi, struct fs_topology *ft)
+static void get_topology(libxfs_init_t *xi, struct fs_topology *ft, int force_overwrite)
{
if (!xi->disfile) {
const char *dfile = xi->volname ? xi->volname : xi->dname;
blkid_get_topology(dfile, &ft->dsunit, &ft->dswidth,
- &ft->sectorsize);
+ &ft->sectorsize, force_overwrite);
}
if (xi->rtname && !xi->risfile) {
int dummy;
- blkid_get_topology(xi->rtname, &dummy, &ft->rtswidth, &dummy);
+ blkid_get_topology(xi->rtname, &dummy, &ft->rtswidth, &dummy, force_overwrite);
}
}
#else /* ENABLE_BLKID */
@@ -460,7 +472,7 @@ check_overwrite(
return 0;
}
-static void get_topology(libxfs_init_t *xi, struct fs_topology *ft)
+static void get_topology(libxfs_init_t *xi, struct fs_topology *ft, int force_overwrite)
{
char *dfile = xi->volname ? xi->volname : xi->dname;
int bsz = BBSIZE;
@@ -1625,7 +1637,7 @@ main(
}
memset(&ft, 0, sizeof(ft));
- get_topology(&xi, &ft);
+ get_topology(&xi, &ft, force_overwrite);
if (ft.sectoralign) {
/*
--
1.7.6.4
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH] mkfs: Refuse to initialize a misaligned device if not forced using libblkid
2011-12-01 12:44 Carlos Maiolino
@ 2011-12-02 11:27 ` Christoph Hellwig
0 siblings, 0 replies; 4+ messages in thread
From: Christoph Hellwig @ 2011-12-02 11:27 UTC (permalink / raw)
To: Carlos Maiolino; +Cc: xfs
On Thu, Dec 01, 2011 at 10:44:25AM -0200, Carlos Maiolino wrote:
> This is the first patch proposal to fix the problem about the usage of
> 4k sector devices when the device is not properly aligned
>
> The patch make mkfs to refuse to initialize a xfs filesystem if the -f
> option is not passed at the command line, and forces a 512b sector size
> if the user opt to force the device initialization
>
> Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>
> ---
> mkfs/xfs_mkfs.c | 24 ++++++++++++++++++------
> 1 files changed, 18 insertions(+), 6 deletions(-)
>
> diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c
> index f527f3d..495c70d 100644
> --- a/mkfs/xfs_mkfs.c
> +++ b/mkfs/xfs_mkfs.c
> @@ -369,8 +369,14 @@ out:
> return ret;
> }
>
> -static void blkid_get_topology(const char *device, int *sunit, int *swidth, int *sectorsize)
> +static void blkid_get_topology(
> + const char *device,
> + int *sunit,
> + int *swidth,
> + int *sectorsize,
> + int force_overwrite)
> {
> +
> blkid_topology tp;
> blkid_probe pr;
> unsigned long val;
> @@ -409,6 +415,12 @@ static void blkid_get_topology(const char *device, int *sunit, int *swidth, int
> fprintf(stderr,
> _("warning: device is not properly aligned %s\n"),
> device);
> +
> + if(!force_overwrite){
> + fprintf(stderr, _("Use -f to force usage of a misaligned device\n"));
> + exit(EXIT_FAILURE);
> + }
> + *sectorsize = BBSIZE; /* Force a 512b sector size if the device is misaligned*/
> }
style issues:
- always use whitespaces beforew and after braces, except in
function calls e.g.
if (!force_overwrite) {
avoid 80 character lines if possible, e.g.
fprintf(stderr,
_("Use -f to force usage of a misaligned device\n"));
and
/* Force a 512b sector size if the device is misaligned */
*sectorsize = BBSIZE;
> - blkid_get_topology(xi->rtname, &dummy, &ft->rtswidth, &dummy);
> + blkid_get_topology(xi->rtname, &dummy, &ft->rtswidth, &dummy, force_overwrite);
needs linebreak, too.
> }
> }
> #else /* ENABLE_BLKID */
> @@ -460,7 +472,7 @@ check_overwrite(
> return 0;
> }
>
> -static void get_topology(libxfs_init_t *xi, struct fs_topology *ft)
> +static void get_topology(libxfs_init_t *xi, struct fs_topology *ft, int force_overwrite)
same here
otherwise this looks good to me.
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2011-12-13 5:01 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-12-02 12:14 [PATCH] mkfs: Refuse to initialize a misaligned device if not forced using libblkid Carlos Maiolino
2011-12-13 5:01 ` Eric Sandeen
-- strict thread matches above, loose matches on Subject: below --
2011-12-01 12:44 Carlos Maiolino
2011-12-02 11:27 ` Christoph Hellwig
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox