[Cluster-devel] [PATCH 1/2] libgfs2: Add support for dirent.de_rahead

[Cluster-devel] [PATCH 1/2] libgfs2: Add support for dirent.de_rahead

[Andreas Gruenbacher]

This field indicates how many blocks of metadata can be read ahead when reading
in the inode a directory entry points at.

Signed-off-by: Andreas Gruenbacher <andreas.gruenbacher@gmail.com>
---
 gfs2/libgfs2/meta.c   | 1 +
 gfs2/libgfs2/ondisk.c | 2 ++
 2 files changed, 3 insertions(+)

diff --git a/gfs2/libgfs2/meta.c b/gfs2/libgfs2/meta.c
index 79de64d..70daec4 100644
--- a/gfs2/libgfs2/meta.c
+++ b/gfs2/libgfs2/meta.c
@@ -428,6 +428,7 @@ F(de_hash, .flags = LGFS2_MFF_CHECK)
 F(de_rec_len, .flags = LGFS2_MFF_BYTES)
 F(de_name_len, .flags = LGFS2_MFF_BYTES)
 F(de_type)
+F(de_rahead)
 RF(__pad)
 };
 
diff --git a/gfs2/libgfs2/ondisk.c b/gfs2/libgfs2/ondisk.c
index 4744337..7f34bd2 100644
--- a/gfs2/libgfs2/ondisk.c
+++ b/gfs2/libgfs2/ondisk.c
@@ -415,6 +415,7 @@ void gfs2_dirent_in(struct gfs2_dirent *de, char *buf)
 	CPIN_16(de, str, de_rec_len);
 	CPIN_16(de, str, de_name_len);
 	CPIN_16(de, str, de_type);
+	CPIN_16(de, str, de_rahead);
 }
 
 void gfs2_dirent_out(struct gfs2_dirent *de, char *buf)
@@ -427,6 +428,7 @@ void gfs2_dirent_out(struct gfs2_dirent *de, char *buf)
 	CPOUT_16(de, str, de_name_len);
 	CPOUT_16(de, str, de_type);
 	memset(str->__pad, 0, sizeof(str->__pad));
+	CPOUT_16(de, str, de_rahead);
 }
 
 void gfs2_leaf_in(struct gfs2_leaf *lf, struct gfs2_buffer_head *bh)
-- 
2.4.3

[Cluster-devel] [PATCH 2/2] gfs2_edit: Include dirent.de_rahead in directory listings

[Andreas Gruenbacher]

When dumping a directory, for directory each entry, also print how many blocks
of metadata can be read ahead when reading in the inode the entry points at.

Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com>
---
 gfs2/edit/extended.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/gfs2/edit/extended.c b/gfs2/edit/extended.c
index e5cb12c..7a219ee 100644
--- a/gfs2/edit/extended.c
+++ b/gfs2/edit/extended.c
@@ -342,13 +342,14 @@ static int display_leaf(struct iinfo *ind)
 					strcpy(edit_fmt, "%llx");
 				}
 			}
-			print_gfs2("%d/%d [%08x] %lld/%lld (0x%llx/0x%llx): ",
+			print_gfs2("%d/%d [%08x] %lld/%lld (0x%llx/0x%llx) +%d: ",
 				   total_dirents, d + 1,
 				   ind->ii[0].dirent[d].dirent.de_hash,
 				   ind->ii[0].dirent[d].dirent.de_inum.no_formal_ino,
 				   ind->ii[0].dirent[d].block,
 				   ind->ii[0].dirent[d].dirent.de_inum.no_formal_ino,
-				   ind->ii[0].dirent[d].block);
+				   ind->ii[0].dirent[d].block,
+				   (unsigned int)ind->ii[0].dirent[d].dirent.de_rahead);
 		}
 		print_inode_type(ind->ii[0].dirent[d].dirent.de_type);
 		print_gfs2(" %s", ind->ii[0].dirent[d].filename);
-- 
2.4.3

[Cluster-devel] [PATCH 2/2] gfs2_edit: Include dirent.de_rahead in directory listings

[Andrew Price]

On 09/09/15 16:55, Andreas Gruenbacher wrote:
> When dumping a directory, for directory each entry, also print how many blocks
> of metadata can be read ahead when reading in the inode the entry points at.
>
> Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com>
> ---
>   gfs2/edit/extended.c | 5 +++--
>   1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/gfs2/edit/extended.c b/gfs2/edit/extended.c
> index e5cb12c..7a219ee 100644
> --- a/gfs2/edit/extended.c
> +++ b/gfs2/edit/extended.c
> @@ -342,13 +342,14 @@ static int display_leaf(struct iinfo *ind)
>   					strcpy(edit_fmt, "%llx");
>   				}
>   			}
> -			print_gfs2("%d/%d [%08x] %lld/%lld (0x%llx/0x%llx): ",
> +			print_gfs2("%d/%d [%08x] %lld/%lld (0x%llx/0x%llx) +%d: ",
>   				   total_dirents, d + 1,
>   				   ind->ii[0].dirent[d].dirent.de_hash,
>   				   ind->ii[0].dirent[d].dirent.de_inum.no_formal_ino,
>   				   ind->ii[0].dirent[d].block,
>   				   ind->ii[0].dirent[d].dirent.de_inum.no_formal_ino,
> -				   ind->ii[0].dirent[d].block);
> +				   ind->ii[0].dirent[d].block,
> +				   (unsigned int)ind->ii[0].dirent[d].dirent.de_rahead);

It would be better to use %u for an unsigned int. We don't have 
__attribute__((format(printf...))) on print_gfs2 yet but it would catch 
that.

Besides that, both patches look good.

Andy

>   		}
>   		print_inode_type(ind->ii[0].dirent[d].dirent.de_type);
>   		print_gfs2(" %s", ind->ii[0].dirent[d].filename);
>

[Cluster-devel] [PATCH 2/2] gfs2_edit: Include dirent.de_rahead in directory listings

[Andrew Price]

On 09/09/15 17:27, Andrew Price wrote:
> On 09/09/15 16:55, Andreas Gruenbacher wrote:
>> When dumping a directory, for directory each entry, also print how
>> many blocks
>> of metadata can be read ahead when reading in the inode the entry
>> points at.
>>
>> Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com>
>> ---
>>   gfs2/edit/extended.c | 5 +++--
>>   1 file changed, 3 insertions(+), 2 deletions(-)
>>
>> diff --git a/gfs2/edit/extended.c b/gfs2/edit/extended.c
>> index e5cb12c..7a219ee 100644
>> --- a/gfs2/edit/extended.c
>> +++ b/gfs2/edit/extended.c
>> @@ -342,13 +342,14 @@ static int display_leaf(struct iinfo *ind)
>>                       strcpy(edit_fmt, "%llx");
>>                   }
>>               }
>> -            print_gfs2("%d/%d [%08x] %lld/%lld (0x%llx/0x%llx): ",
>> +            print_gfs2("%d/%d [%08x] %lld/%lld (0x%llx/0x%llx) +%d: ",
>>                      total_dirents, d + 1,
>>                      ind->ii[0].dirent[d].dirent.de_hash,
>>                      ind->ii[0].dirent[d].dirent.de_inum.no_formal_ino,
>>                      ind->ii[0].dirent[d].block,
>>                      ind->ii[0].dirent[d].dirent.de_inum.no_formal_ino,
>> -                   ind->ii[0].dirent[d].block);
>> +                   ind->ii[0].dirent[d].block,
>> +                   (unsigned int)ind->ii[0].dirent[d].dirent.de_rahead);
>
> It would be better to use %u for an unsigned int. We don't have
> __attribute__((format(printf...))) on print_gfs2 yet but it would catch
> that.

I've pushed both patches, including the above tweak, and added another 
patch to support older versions which don't have the de_rahead field (< 
Linux 3.15).

Thanks,
Andy

[Cluster-devel] [PATCH 2/2] gfs2_edit: Include dirent.de_rahead in directory listings

[Andreas Grünbacher]

Thanks!

Andreas

[Cluster-devel] [PATCH 2/2] gfs2_edit: Include dirent.de_rahead in directory listings

[Andreas Gruenbacher]

2015-09-09 18:27 GMT+02:00 Andrew Price <anprice@redhat.com>:
> It would be better to use %u for an unsigned int. We don't have
> __attribute__((format(printf...))) on print_gfs2 yet but it would catch
> that.

I've tried adding the printf format attribute; it's quite a mess. A
common pattern is that values are not printed with the proper
signedness. The fact that the code is messing around with __be64
variables which have been converted to cpu endianness, and that __be64
and uint64_t need different printf formats ("%llu" and "%"PRIu64) to
pacify gcc doesn't help.

Andreas

[Cluster-devel] [PATCH 1/2] gfs2: Fix printf format errors on x86

[Andreas Gruenbacher]

Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com>
---
 gfs2/fsck/metawalk.c  | 2 +-
 gfs2/mkfs/main_grow.c | 5 +++--
 gfs2/mkfs/main_mkfs.c | 2 +-
 3 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/gfs2/fsck/metawalk.c b/gfs2/fsck/metawalk.c
index 333bec6..9923d94 100644
--- a/gfs2/fsck/metawalk.c
+++ b/gfs2/fsck/metawalk.c
@@ -2080,7 +2080,7 @@ int write_new_leaf(struct gfs2_inode *dip, int start_lindex, int num_copies,
 		  "(0x%llx) at index: 0x%x for 0x%lx pointers.\n"),
 		(unsigned long long)dip->i_di.di_num.no_addr,
 		(unsigned long long)dip->i_di.di_num.no_addr,
-		start_lindex, pad_size / sizeof(uint64_t));
+		start_lindex, (unsigned long)pad_size / sizeof(uint64_t));
 	if (dip->i_sbd->gfs1)
 		count = gfs1_writei(dip, padbuf, start_lindex *
 				    sizeof(uint64_t), pad_size);
diff --git a/gfs2/mkfs/main_grow.c b/gfs2/mkfs/main_grow.c
index 4757ac7..860c319 100644
--- a/gfs2/mkfs/main_grow.c
+++ b/gfs2/mkfs/main_grow.c
@@ -269,7 +269,8 @@ static void fix_rindex(int rindex_fd, lgfs2_rgrps_t rgs, unsigned old_rg_count,
 		off_t rindex_size = lseek(rindex_fd, 0, SEEK_END);
 		if (rindex_size != old_rg_count * entrysize) {
 			log_crit(_("Incorrect rindex size. Want %ld (%d resource groups), have %ld\n"),
-				 (old_rg_count * entrysize), old_rg_count, rindex_size);
+				 (long)(old_rg_count * entrysize), old_rg_count,
+				 (long)rindex_size);
 			goto out;
 		}
 		/* Write the first entry separately to ensure there's enough
@@ -300,7 +301,7 @@ out:
 trunc:
 	count = (count / sizeof(struct gfs2_rindex)) + old_rg_count;
 	log_crit(_("truncating rindex to %ld entries\n"),
-		 (off_t)count * sizeof(struct gfs2_rindex));
+		 (long)count * sizeof(struct gfs2_rindex));
 	ftruncate(rindex_fd, (off_t)count * sizeof(struct gfs2_rindex));
 	free(buf);
 }
diff --git a/gfs2/mkfs/main_mkfs.c b/gfs2/mkfs/main_mkfs.c
index 3fab08c..d3d8edf 100644
--- a/gfs2/mkfs/main_mkfs.c
+++ b/gfs2/mkfs/main_mkfs.c
@@ -599,7 +599,7 @@ static lgfs2_rgrps_t rgs_init(struct mkfs_opts *opts, struct gfs2_sbd *sdp)
 	if (opts->debug) {
 		printf("  rgrp align = ");
 		if (opts->align)
-			printf("%lu+%lu blocks\n", al_base, al_off);
+			printf("%"PRIu64"+%"PRIu64" blocks\n", al_base, al_off);
 		else
 			printf("(disabled)\n");
 	}
-- 
2.4.3

[Cluster-devel] [PATCH 2/2] gfs2_edit: Add attribute printf for print_gfs2

[Andreas Gruenbacher]

Add the appropriate function attribute for the printf-style function print_gfs2
and clean up the resulting fallout.

Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com>
---
 gfs2/edit/extended.c | 22 +++++++++++-----------
 gfs2/edit/gfs2hex.c  |  2 +-
 gfs2/edit/gfs2hex.h  |  2 +-
 gfs2/edit/hexedit.c  | 20 ++++++++++----------
 gfs2/edit/journal.c  | 14 +++++++-------
 gfs2/edit/savemeta.c |  2 +-
 6 files changed, 31 insertions(+), 31 deletions(-)

diff --git a/gfs2/edit/extended.c b/gfs2/edit/extended.c
index a13827e..c01427f 100644
--- a/gfs2/edit/extended.c
+++ b/gfs2/edit/extended.c
@@ -178,7 +178,7 @@ static int display_indirect(struct iinfo *ind, int indblocks, int level,
 		print_gfs2("%d => ", pndx);
 		if (termlines)
 			move(line,9);
-		print_gfs2("0x%llx / %lld", ind->ii[pndx].block,
+		print_gfs2("0x%"PRIx64" / %"PRId64, ind->ii[pndx].block,
 			   ind->ii[pndx].block);
 		if (termlines) {
 			if (edit_row[dmode] >= 0 &&
@@ -206,7 +206,7 @@ static int display_indirect(struct iinfo *ind, int indblocks, int level,
 			if (human_off > 1024.0) { h = 'T'; human_off /= 1024.0; }
 			if (human_off > 1024.0) { h = 'P'; human_off /= 1024.0; }
 			if (human_off > 1024.0) { h = 'E'; human_off /= 1024.0; }
-			print_gfs2("(data offset 0x%llx / %lld / %6.2f%c)",
+			print_gfs2("(data offset 0x%"PRIx64" / %"PRId64" / %6.2f%c)",
 				   file_offset, file_offset, human_off, h);
 			print_gfs2("   ");
 		}
@@ -297,8 +297,8 @@ static void print_inode_type(__be16 de_type)
 }
 
 #ifdef GFS2_HAS_LEAF_HINTS
-#define LEAF_HINT_FMTS "lf_inode: 0x%"PRIx64", lf_dist: %"PRIu32", " \
-                       "lf_nsec: %"PRIu32", lf_sec: %"PRIu64", "
+#define LEAF_HINT_FMTS "lf_inode: 0x%llx, lf_dist: %u, " \
+                       "lf_nsec: %u, lf_sec: %llu, "
 #define LEAF_HINT_FIELDS(lp) lp->lf_inode, lp->lf_dist, lp->lf_nsec, lp->lf_sec,
 #else
 #define LEAF_HINT_FMTS
@@ -342,7 +342,7 @@ static int display_leaf(struct iinfo *ind)
 					strcpy(edit_fmt, "%llx");
 				}
 			}
-			print_gfs2("%d/%d [%08x] %lld/%lld (0x%llx/0x%llx) +%u: ",
+			print_gfs2("%d/%d [%08x] %lld/%"PRId64" (0x%llx/0x%"PRIx64") +%u: ",
 				   total_dirents, d + 1,
 				   ind->ii[0].dirent[d].dirent.de_hash,
 				   ind->ii[0].dirent[d].dirent.de_inum.no_formal_ino,
@@ -429,7 +429,7 @@ static void print_block_details(struct iinfo *ind, int level, int cur_height,
 			eol(0);
 			if (termlines)
 				move(line,9);
-			print_gfs2("Continuation block 0x%llx / %lld",
+			print_gfs2("Continuation block 0x%"PRIx64" / %"PRId64,
 				   thisblk, thisblk);
 		}
 	}
@@ -451,7 +451,7 @@ static int print_gfs_jindex(struct gfs2_inode *dij)
 	char jbuf[sizeof(struct gfs_jindex)];
 
 	start_line = line;
-	print_gfs2("Journal index entries found: %d.",
+	print_gfs2("Journal index entries found: %lld.",
 		   dij->i_di.di_size / sizeof(struct gfs_jindex));
 	eol(0);
 	lines_per_row[dmode] = 4;
@@ -493,7 +493,7 @@ static int print_gfs2_jindex(void)
 		if (strncmp(indirect->ii[0].dirent[d].filename, "journal", 7))
 			continue;
 		ip = lgfs2_inode_read(&sbd, indirect->ii[0].dirent[d].block);
-		print_gfs2("%s: 0x%-5x %dMB ",
+		print_gfs2("%s: 0x%-5"PRIx64" %lldMB ",
 			   indirect->ii[0].dirent[d].filename,
 			   indirect->ii[0].dirent[d].block,
 			   ip->i_di.di_size / 1048576);
@@ -520,7 +520,7 @@ static int parse_rindex(struct gfs2_inode *dip, int print_rindex)
 	char highlighted_addr[32];
 
 	start_line = line;
-	print_gfs2("RG index entries found: %d.", dip->i_di.di_size /
+	print_gfs2("RG index entries found: %lld.", dip->i_di.di_size /
 		   sizeof(struct gfs2_rindex));
 	eol(0);
 	lines_per_row[dmode] = 6;
@@ -593,7 +593,7 @@ static int print_inum(struct gfs2_inode *dii)
 		return -1;
 	}
 	inodenum = be64_to_cpu(inum);
-	print_gfs2("Next inode num = %lld (0x%llx)", inodenum, inodenum);
+	print_gfs2("Next inode num = %"PRId64" (0x%"PRIx64")", inodenum, inodenum);
 	eol(0);
 	return 0;
 }
@@ -628,7 +628,7 @@ static int print_quota(struct gfs2_inode *diq)
 	
 	print_gfs2("quota file contents:");
 	eol(0);
-	print_gfs2("quota entries found: %d.", diq->i_di.di_size / sizeof(q));
+	print_gfs2("quota entries found: %lld.", diq->i_di.di_size / sizeof(q));
 	eol(0);
 	for (i=0; ; i++) {
 		error = gfs2_readi(diq, (void *)&qbuf, i * sizeof(q), sizeof(qbuf));
diff --git a/gfs2/edit/gfs2hex.c b/gfs2/edit/gfs2hex.c
index dc6bb8f..101a063 100644
--- a/gfs2/edit/gfs2hex.c
+++ b/gfs2/edit/gfs2hex.c
@@ -98,7 +98,7 @@ void eol(int col) /* end of line */
 	}
 }
 
-void __attribute__((format (printf, 1, 2))) print_gfs2(const char *fmt, ...)
+void print_gfs2(const char *fmt, ...)
 {
 	va_list args;
 	char string[PATH_MAX];
diff --git a/gfs2/edit/gfs2hex.h b/gfs2/edit/gfs2hex.h
index 3c08a19..1bd83c3 100644
--- a/gfs2/edit/gfs2hex.h
+++ b/gfs2/edit/gfs2hex.h
@@ -7,7 +7,7 @@ extern int display_gfs2(void);
 extern int edit_gfs2(void);
 extern void do_dinode_extended(struct gfs2_dinode *di,
 			       struct gfs2_buffer_head *lbh);
-extern void print_gfs2(const char *fmt, ...);
+extern void print_gfs2(const char *fmt, ...) __attribute__((format (printf, 1, 2)));
 extern uint64_t do_leaf_extended(char *dlebuf, struct iinfo *indir);
 extern void eol(int col);
 
diff --git a/gfs2/edit/hexedit.c b/gfs2/edit/hexedit.c
index 4df1b9f..70406df 100644
--- a/gfs2/edit/hexedit.c
+++ b/gfs2/edit/hexedit.c
@@ -332,14 +332,14 @@ int display_block_type(int from_restore)
 	else if (block == JOURNALS_DUMMY_BLOCK)
 		print_gfs2("Journal Status:      ");
 	else
-		print_gfs2("%lld    (0x%llx)", block, block);
+		print_gfs2("%"PRId64"    (0x%"PRIx64")", block, block);
 	if (termlines) {
 		if (edit_row[dmode] == -1)
 			COLORS_NORMAL;
 	}
 	print_gfs2(" ");
 	if (!from_restore)
-		print_gfs2("of %llu (0x%llx) ", max_block, max_block);
+		print_gfs2("of %"PRIu64" (0x%"PRIx64") ", max_block, max_block);
 	if (block == RGLIST_DUMMY_BLOCK) {
 		ret_type = GFS2_METATYPE_RG;
 		struct_len = sbd.gfs1 ? sizeof(struct gfs_rgrp) :
@@ -580,9 +580,9 @@ static int hexdump(uint64_t startaddr, int len, int trunc_zeros,
 			COLORS_OFFSETS; /* cyan for offsets */
 		}
 		if (startaddr < 0xffffffff)
-			print_gfs2("%.8llx", startaddr + l);
+			print_gfs2("%.8"PRIx64, startaddr + l);
 		else
-			print_gfs2("%.16llx", startaddr + l);
+			print_gfs2("%.16"PRIx64, startaddr + l);
 		if (termlines) {
 			if (l < struct_len)
 				COLORS_NORMAL; /* normal part of structure */
@@ -670,7 +670,7 @@ static int hexdump(uint64_t startaddr, int len, int trunc_zeros,
 					f = &m->fields[n];
 					if (print_field >= f->offset &&
 					    print_field < (f->offset + f->length)) {
-						print_gfs2(m->fields[n].name);
+						print_gfs2("%s", m->fields[n].name);
 						break;
 					}
 				}
@@ -699,12 +699,12 @@ static int hexdump(uint64_t startaddr, int len, int trunc_zeros,
 		if (line - 3 > last_entry_onscreen[dmode])
 			last_entry_onscreen[dmode] = line - 3;
 		if (flagref && be64_to_cpu(*ref) == flagref)
-			print_gfs2("<------------------------- ref in 0x%llx "
-				   "to 0x%llx", ref_blk, flagref);
+			print_gfs2("<------------------------- ref in 0x%"PRIx64" "
+				   "to 0x%"PRIx64, ref_blk, flagref);
 		ref++;
 		if (flagref && be64_to_cpu(*ref) == flagref)
-			print_gfs2("<------------------------- ref in 0x%llx "
-				   "to 0x%llx", ref_blk, flagref);
+			print_gfs2("<------------------------- ref in 0x%"PRIx64" "
+				   "to 0x%"PRIx64, ref_blk, flagref);
 		ref++;
 		eol(0);
 		l += 16;
@@ -849,7 +849,7 @@ static void set_rgrp_flags(int rgnum, uint32_t new_flags, int modify, int full)
 	} else {
 		if (full) {
 			print_gfs2("RG #%d", rgnum);
-			print_gfs2(" located at: %llu (0x%llx)", rgblk, rgblk);
+			print_gfs2(" located at: %"PRIu64" (0x%"PRIx64")", rgblk, rgblk);
                         eol(0);
 			if (sbd.gfs1)
 				gfs_rgrp_print(&rg.rg1);
diff --git a/gfs2/edit/journal.c b/gfs2/edit/journal.c
index a2aeb2c..15e02de 100644
--- a/gfs2/edit/journal.c
+++ b/gfs2/edit/journal.c
@@ -392,7 +392,7 @@ static int process_ld(uint64_t abs_block, uint64_t wrappt, uint64_t j_size,
 			       bitblk);
 
 	if (*prnt) {
-		print_gfs2("0x%llx (j+%4llx): Log descriptor, ",
+		print_gfs2("0x%"PRIx64" (j+%4"PRIx64"): Log descriptor, ",
 			   abs_block, ((jb + wrappt) % j_size) / sbd.bsize);
 		print_gfs2("type %d ", ld.ld_type);
 
@@ -586,9 +586,9 @@ void dump_journal(const char *journal, int tblk)
 				gfs_log_header_in(&lh1, &dummy_bh);
 				check_journal_wrap(lh1.lh_sequence,
 						   &highest_seq);
-				print_gfs2("0x%llx (j+%4llx): Log header: "
-					   "Flags:%x, Seq: 0x%x, 1st: 0x%x, "
-					   "tail: 0x%x, last: 0x%x",
+				print_gfs2("0x%"PRIx64" (j+%4"PRIx64"): Log header: "
+					   "Flags:%x, Seq: 0x%llx, 1st: 0x%llx, "
+					   "tail: 0x%llx, last: 0x%llx",
 					   abs_block, jb + wrappt,
 					   lh1.lh_flags, lh1.lh_sequence,
 					   lh1.lh_first, lh1.lh_tail,
@@ -597,8 +597,8 @@ void dump_journal(const char *journal, int tblk)
 				gfs2_log_header_in(&lh, &dummy_bh);
 				check_journal_wrap(lh.lh_sequence,
 						   &highest_seq);
-				print_gfs2("0x%llx (j+%4llx): Log header: Seq"
-					   ": 0x%x, tail: 0x%x, blk: 0x%x%s",
+				print_gfs2("0x%"PRIx64" (j+%4"PRIx64"): Log header: Seq"
+					   ": 0x%llx, tail: 0x%x, blk: 0x%x%s",
 					   abs_block, ((jb + wrappt) % j_size)
 					   / sbd.bsize, lh.lh_sequence,
 					   lh.lh_tail, lh.lh_blkno,
@@ -609,7 +609,7 @@ void dump_journal(const char *journal, int tblk)
 			eol(0);
 		} else if ((ld_blocks > 0) &&
 			   (sbd.gfs1 || block_type == GFS2_METATYPE_LB)) {
-			print_gfs2("0x%llx (j+%4llx): Log descriptor"
+			print_gfs2("0x%"PRIx64" (j+%4"PRIx64"): Log descriptor"
 				   " continuation block", abs_block, jb);
 			eol(0);
 			print_gfs2("                    ");
diff --git a/gfs2/edit/savemeta.c b/gfs2/edit/savemeta.c
index b68b0ff..bd34fb1 100644
--- a/gfs2/edit/savemeta.c
+++ b/gfs2/edit/savemeta.c
@@ -1012,7 +1012,7 @@ static int restore_data(int fd, gzFile gzin_fd, off_t pos, int printonly)
 				bh = NULL;
 				break;
 			} else if (printonly == 1) {
-				print_gfs2("%d (l=0x%x): ", blks_saved, savedata->siglen);
+				print_gfs2("%"PRId64" (l=0x%x): ", blks_saved, savedata->siglen);
 				display_block_type(TRUE);
 			}
 			bh = NULL;
-- 
2.4.3

[Cluster-devel] [PATCH 2/2] gfs2_edit: Add attribute printf for print_gfs2

[Andrew Price]

On 10/09/15 23:05, Andreas Gruenbacher wrote:
> Add the appropriate function attribute for the printf-style function print_gfs2
> and clean up the resulting fallout.

Both patches look good to me, thanks. I've pushed them both along with 
the savemeta speedup patch I posted last week.

Andy

> Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com>
> ---
>   gfs2/edit/extended.c | 22 +++++++++++-----------
>   gfs2/edit/gfs2hex.c  |  2 +-
>   gfs2/edit/gfs2hex.h  |  2 +-
>   gfs2/edit/hexedit.c  | 20 ++++++++++----------
>   gfs2/edit/journal.c  | 14 +++++++-------
>   gfs2/edit/savemeta.c |  2 +-
>   6 files changed, 31 insertions(+), 31 deletions(-)
>
> diff --git a/gfs2/edit/extended.c b/gfs2/edit/extended.c
> index a13827e..c01427f 100644
> --- a/gfs2/edit/extended.c
> +++ b/gfs2/edit/extended.c
> @@ -178,7 +178,7 @@ static int display_indirect(struct iinfo *ind, int indblocks, int level,
>   		print_gfs2("%d => ", pndx);
>   		if (termlines)
>   			move(line,9);
> -		print_gfs2("0x%llx / %lld", ind->ii[pndx].block,
> +		print_gfs2("0x%"PRIx64" / %"PRId64, ind->ii[pndx].block,
>   			   ind->ii[pndx].block);
>   		if (termlines) {
>   			if (edit_row[dmode] >= 0 &&
> @@ -206,7 +206,7 @@ static int display_indirect(struct iinfo *ind, int indblocks, int level,
>   			if (human_off > 1024.0) { h = 'T'; human_off /= 1024.0; }
>   			if (human_off > 1024.0) { h = 'P'; human_off /= 1024.0; }
>   			if (human_off > 1024.0) { h = 'E'; human_off /= 1024.0; }
> -			print_gfs2("(data offset 0x%llx / %lld / %6.2f%c)",
> +			print_gfs2("(data offset 0x%"PRIx64" / %"PRId64" / %6.2f%c)",
>   				   file_offset, file_offset, human_off, h);
>   			print_gfs2("   ");
>   		}
> @@ -297,8 +297,8 @@ static void print_inode_type(__be16 de_type)
>   }
>
>   #ifdef GFS2_HAS_LEAF_HINTS
> -#define LEAF_HINT_FMTS "lf_inode: 0x%"PRIx64", lf_dist: %"PRIu32", " \
> -                       "lf_nsec: %"PRIu32", lf_sec: %"PRIu64", "
> +#define LEAF_HINT_FMTS "lf_inode: 0x%llx, lf_dist: %u, " \
> +                       "lf_nsec: %u, lf_sec: %llu, "
>   #define LEAF_HINT_FIELDS(lp) lp->lf_inode, lp->lf_dist, lp->lf_nsec, lp->lf_sec,
>   #else
>   #define LEAF_HINT_FMTS
> @@ -342,7 +342,7 @@ static int display_leaf(struct iinfo *ind)
>   					strcpy(edit_fmt, "%llx");
>   				}
>   			}
> -			print_gfs2("%d/%d [%08x] %lld/%lld (0x%llx/0x%llx) +%u: ",
> +			print_gfs2("%d/%d [%08x] %lld/%"PRId64" (0x%llx/0x%"PRIx64") +%u: ",
>   				   total_dirents, d + 1,
>   				   ind->ii[0].dirent[d].dirent.de_hash,
>   				   ind->ii[0].dirent[d].dirent.de_inum.no_formal_ino,
> @@ -429,7 +429,7 @@ static void print_block_details(struct iinfo *ind, int level, int cur_height,
>   			eol(0);
>   			if (termlines)
>   				move(line,9);
> -			print_gfs2("Continuation block 0x%llx / %lld",
> +			print_gfs2("Continuation block 0x%"PRIx64" / %"PRId64,
>   				   thisblk, thisblk);
>   		}
>   	}
> @@ -451,7 +451,7 @@ static int print_gfs_jindex(struct gfs2_inode *dij)
>   	char jbuf[sizeof(struct gfs_jindex)];
>
>   	start_line = line;
> -	print_gfs2("Journal index entries found: %d.",
> +	print_gfs2("Journal index entries found: %lld.",
>   		   dij->i_di.di_size / sizeof(struct gfs_jindex));
>   	eol(0);
>   	lines_per_row[dmode] = 4;
> @@ -493,7 +493,7 @@ static int print_gfs2_jindex(void)
>   		if (strncmp(indirect->ii[0].dirent[d].filename, "journal", 7))
>   			continue;
>   		ip = lgfs2_inode_read(&sbd, indirect->ii[0].dirent[d].block);
> -		print_gfs2("%s: 0x%-5x %dMB ",
> +		print_gfs2("%s: 0x%-5"PRIx64" %lldMB ",
>   			   indirect->ii[0].dirent[d].filename,
>   			   indirect->ii[0].dirent[d].block,
>   			   ip->i_di.di_size / 1048576);
> @@ -520,7 +520,7 @@ static int parse_rindex(struct gfs2_inode *dip, int print_rindex)
>   	char highlighted_addr[32];
>
>   	start_line = line;
> -	print_gfs2("RG index entries found: %d.", dip->i_di.di_size /
> +	print_gfs2("RG index entries found: %lld.", dip->i_di.di_size /
>   		   sizeof(struct gfs2_rindex));
>   	eol(0);
>   	lines_per_row[dmode] = 6;
> @@ -593,7 +593,7 @@ static int print_inum(struct gfs2_inode *dii)
>   		return -1;
>   	}
>   	inodenum = be64_to_cpu(inum);
> -	print_gfs2("Next inode num = %lld (0x%llx)", inodenum, inodenum);
> +	print_gfs2("Next inode num = %"PRId64" (0x%"PRIx64")", inodenum, inodenum);
>   	eol(0);
>   	return 0;
>   }
> @@ -628,7 +628,7 @@ static int print_quota(struct gfs2_inode *diq)
>   	
>   	print_gfs2("quota file contents:");
>   	eol(0);
> -	print_gfs2("quota entries found: %d.", diq->i_di.di_size / sizeof(q));
> +	print_gfs2("quota entries found: %lld.", diq->i_di.di_size / sizeof(q));
>   	eol(0);
>   	for (i=0; ; i++) {
>   		error = gfs2_readi(diq, (void *)&qbuf, i * sizeof(q), sizeof(qbuf));
> diff --git a/gfs2/edit/gfs2hex.c b/gfs2/edit/gfs2hex.c
> index dc6bb8f..101a063 100644
> --- a/gfs2/edit/gfs2hex.c
> +++ b/gfs2/edit/gfs2hex.c
> @@ -98,7 +98,7 @@ void eol(int col) /* end of line */
>   	}
>   }
>
> -void __attribute__((format (printf, 1, 2))) print_gfs2(const char *fmt, ...)
> +void print_gfs2(const char *fmt, ...)
>   {
>   	va_list args;
>   	char string[PATH_MAX];
> diff --git a/gfs2/edit/gfs2hex.h b/gfs2/edit/gfs2hex.h
> index 3c08a19..1bd83c3 100644
> --- a/gfs2/edit/gfs2hex.h
> +++ b/gfs2/edit/gfs2hex.h
> @@ -7,7 +7,7 @@ extern int display_gfs2(void);
>   extern int edit_gfs2(void);
>   extern void do_dinode_extended(struct gfs2_dinode *di,
>   			       struct gfs2_buffer_head *lbh);
> -extern void print_gfs2(const char *fmt, ...);
> +extern void print_gfs2(const char *fmt, ...) __attribute__((format (printf, 1, 2)));
>   extern uint64_t do_leaf_extended(char *dlebuf, struct iinfo *indir);
>   extern void eol(int col);
>
> diff --git a/gfs2/edit/hexedit.c b/gfs2/edit/hexedit.c
> index 4df1b9f..70406df 100644
> --- a/gfs2/edit/hexedit.c
> +++ b/gfs2/edit/hexedit.c
> @@ -332,14 +332,14 @@ int display_block_type(int from_restore)
>   	else if (block == JOURNALS_DUMMY_BLOCK)
>   		print_gfs2("Journal Status:      ");
>   	else
> -		print_gfs2("%lld    (0x%llx)", block, block);
> +		print_gfs2("%"PRId64"    (0x%"PRIx64")", block, block);
>   	if (termlines) {
>   		if (edit_row[dmode] == -1)
>   			COLORS_NORMAL;
>   	}
>   	print_gfs2(" ");
>   	if (!from_restore)
> -		print_gfs2("of %llu (0x%llx) ", max_block, max_block);
> +		print_gfs2("of %"PRIu64" (0x%"PRIx64") ", max_block, max_block);
>   	if (block == RGLIST_DUMMY_BLOCK) {
>   		ret_type = GFS2_METATYPE_RG;
>   		struct_len = sbd.gfs1 ? sizeof(struct gfs_rgrp) :
> @@ -580,9 +580,9 @@ static int hexdump(uint64_t startaddr, int len, int trunc_zeros,
>   			COLORS_OFFSETS; /* cyan for offsets */
>   		}
>   		if (startaddr < 0xffffffff)
> -			print_gfs2("%.8llx", startaddr + l);
> +			print_gfs2("%.8"PRIx64, startaddr + l);
>   		else
> -			print_gfs2("%.16llx", startaddr + l);
> +			print_gfs2("%.16"PRIx64, startaddr + l);
>   		if (termlines) {
>   			if (l < struct_len)
>   				COLORS_NORMAL; /* normal part of structure */
> @@ -670,7 +670,7 @@ static int hexdump(uint64_t startaddr, int len, int trunc_zeros,
>   					f = &m->fields[n];
>   					if (print_field >= f->offset &&
>   					    print_field < (f->offset + f->length)) {
> -						print_gfs2(m->fields[n].name);
> +						print_gfs2("%s", m->fields[n].name);
>   						break;
>   					}
>   				}
> @@ -699,12 +699,12 @@ static int hexdump(uint64_t startaddr, int len, int trunc_zeros,
>   		if (line - 3 > last_entry_onscreen[dmode])
>   			last_entry_onscreen[dmode] = line - 3;
>   		if (flagref && be64_to_cpu(*ref) == flagref)
> -			print_gfs2("<------------------------- ref in 0x%llx "
> -				   "to 0x%llx", ref_blk, flagref);
> +			print_gfs2("<------------------------- ref in 0x%"PRIx64" "
> +				   "to 0x%"PRIx64, ref_blk, flagref);
>   		ref++;
>   		if (flagref && be64_to_cpu(*ref) == flagref)
> -			print_gfs2("<------------------------- ref in 0x%llx "
> -				   "to 0x%llx", ref_blk, flagref);
> +			print_gfs2("<------------------------- ref in 0x%"PRIx64" "
> +				   "to 0x%"PRIx64, ref_blk, flagref);
>   		ref++;
>   		eol(0);
>   		l += 16;
> @@ -849,7 +849,7 @@ static void set_rgrp_flags(int rgnum, uint32_t new_flags, int modify, int full)
>   	} else {
>   		if (full) {
>   			print_gfs2("RG #%d", rgnum);
> -			print_gfs2(" located at: %llu (0x%llx)", rgblk, rgblk);
> +			print_gfs2(" located at: %"PRIu64" (0x%"PRIx64")", rgblk, rgblk);
>                           eol(0);
>   			if (sbd.gfs1)
>   				gfs_rgrp_print(&rg.rg1);
> diff --git a/gfs2/edit/journal.c b/gfs2/edit/journal.c
> index a2aeb2c..15e02de 100644
> --- a/gfs2/edit/journal.c
> +++ b/gfs2/edit/journal.c
> @@ -392,7 +392,7 @@ static int process_ld(uint64_t abs_block, uint64_t wrappt, uint64_t j_size,
>   			       bitblk);
>
>   	if (*prnt) {
> -		print_gfs2("0x%llx (j+%4llx): Log descriptor, ",
> +		print_gfs2("0x%"PRIx64" (j+%4"PRIx64"): Log descriptor, ",
>   			   abs_block, ((jb + wrappt) % j_size) / sbd.bsize);
>   		print_gfs2("type %d ", ld.ld_type);
>
> @@ -586,9 +586,9 @@ void dump_journal(const char *journal, int tblk)
>   				gfs_log_header_in(&lh1, &dummy_bh);
>   				check_journal_wrap(lh1.lh_sequence,
>   						   &highest_seq);
> -				print_gfs2("0x%llx (j+%4llx): Log header: "
> -					   "Flags:%x, Seq: 0x%x, 1st: 0x%x, "
> -					   "tail: 0x%x, last: 0x%x",
> +				print_gfs2("0x%"PRIx64" (j+%4"PRIx64"): Log header: "
> +					   "Flags:%x, Seq: 0x%llx, 1st: 0x%llx, "
> +					   "tail: 0x%llx, last: 0x%llx",
>   					   abs_block, jb + wrappt,
>   					   lh1.lh_flags, lh1.lh_sequence,
>   					   lh1.lh_first, lh1.lh_tail,
> @@ -597,8 +597,8 @@ void dump_journal(const char *journal, int tblk)
>   				gfs2_log_header_in(&lh, &dummy_bh);
>   				check_journal_wrap(lh.lh_sequence,
>   						   &highest_seq);
> -				print_gfs2("0x%llx (j+%4llx): Log header: Seq"
> -					   ": 0x%x, tail: 0x%x, blk: 0x%x%s",
> +				print_gfs2("0x%"PRIx64" (j+%4"PRIx64"): Log header: Seq"
> +					   ": 0x%llx, tail: 0x%x, blk: 0x%x%s",
>   					   abs_block, ((jb + wrappt) % j_size)
>   					   / sbd.bsize, lh.lh_sequence,
>   					   lh.lh_tail, lh.lh_blkno,
> @@ -609,7 +609,7 @@ void dump_journal(const char *journal, int tblk)
>   			eol(0);
>   		} else if ((ld_blocks > 0) &&
>   			   (sbd.gfs1 || block_type == GFS2_METATYPE_LB)) {
> -			print_gfs2("0x%llx (j+%4llx): Log descriptor"
> +			print_gfs2("0x%"PRIx64" (j+%4"PRIx64"): Log descriptor"
>   				   " continuation block", abs_block, jb);
>   			eol(0);
>   			print_gfs2("                    ");
> diff --git a/gfs2/edit/savemeta.c b/gfs2/edit/savemeta.c
> index b68b0ff..bd34fb1 100644
> --- a/gfs2/edit/savemeta.c
> +++ b/gfs2/edit/savemeta.c
> @@ -1012,7 +1012,7 @@ static int restore_data(int fd, gzFile gzin_fd, off_t pos, int printonly)
>   				bh = NULL;
>   				break;
>   			} else if (printonly == 1) {
> -				print_gfs2("%d (l=0x%x): ", blks_saved, savedata->siglen);
> +				print_gfs2("%"PRId64" (l=0x%x): ", blks_saved, savedata->siglen);
>   				display_block_type(TRUE);
>   			}
>   			bh = NULL;
>

[Cluster-devel] [PATCH 2/2] gfs2_edit: Include dirent.de_rahead in directory listings

[Andrew Price]

On 10/09/15 23:04, Andreas Gruenbacher wrote:
> 2015-09-09 18:27 GMT+02:00 Andrew Price <anprice@redhat.com>:
>> It would be better to use %u for an unsigned int. We don't have
>> __attribute__((format(printf...))) on print_gfs2 yet but it would catch
>> that.
>
> I've tried adding the printf format attribute; it's quite a mess. A
> common pattern is that values are not printed with the proper
> signedness. The fact that the code is messing around with __be64
> variables which have been converted to cpu endianness, and that __be64
> and uint64_t need different printf formats ("%llu" and "%"PRIu64) to
> pacify gcc doesn't help.

Yes, I think ideally we should have a set of userspace structures to 
keep things separate. None of the applications should need to use the 
__be* kernel types and libgfs2 should only use them minimally, if at 
all. I'm not sure whether it's best to do that automatically using 
preprocessor magic on the kernel headers, or just by adding new types to 
gfs2-utils with the intention of keeping them in sync manually, or some 
other way.

Andy

[Cluster-devel] [PATCH 2/2] gfs2_edit: Include dirent.de_rahead in directory listings

[Andreas Gruenbacher]

2015-09-11 12:45 GMT+02:00 Andrew Price <anprice@redhat.com>:
> Yes, I think ideally we should have a set of userspace structures to keep
> things separate.

Building gfs2-utils currently also depends on
/usr/include/linux/gfs2_ondisk.h, so when building on an old system,
some features will be disabled. It would be more useful to keep an
up-to-date copy of that header in gfs2-utils: we have all the code to
support all features in there anyway, and gfs2-utils needs to remain
backwards compatible anyway.

Andreas

[Cluster-devel] gfs2 on-disk headers in user space

[Andrew Price]

On 11/09/15 12:02, Andreas Gruenbacher wrote:
> 2015-09-11 12:45 GMT+02:00 Andrew Price <anprice@redhat.com>:
>> Yes, I think ideally we should have a set of userspace structures to keep
>> things separate.
>
> Building gfs2-utils currently also depends on
> /usr/include/linux/gfs2_ondisk.h, so when building on an old system,
> some features will be disabled. It would be more useful to keep an
> up-to-date copy of that header in gfs2-utils: we have all the code to
> support all features in there anyway, and gfs2-utils needs to remain
> backwards compatible anyway.

That makes sense to me. I assume we would remove the #include 
<linux/types.h> from gfs2_ondisk.h and replace it with our own user 
space types header. I'm worried about clashes using the kernel type 
names though, so we might need to process the header somehow to change them.

Andy

[Cluster-devel] gfs2 on-disk headers in user space

[Andreas Grünbacher]

2015-09-11 13:28 GMT+02:00 Andrew Price <anprice@redhat.com>:
> That makes sense to me. I assume we would remove the #include
> <linux/types.h> from gfs2_ondisk.h and replace it with our own user space
> types header. I'm worried about clashes using the kernel type names though,
> so we might need to process the header somehow to change them.

We could substitute the __beX types with uintX_t on import; also we
could warn if the system header exists but in a different version than
what's "cached" in gfs2-utils.

Andreas

[Cluster-devel] gfs2 on-disk headers in user space

[Andrew Price]

On 11/09/15 12:40, Andreas Gr?nbacher wrote:
> 2015-09-11 13:28 GMT+02:00 Andrew Price <anprice@redhat.com>:
>> That makes sense to me. I assume we would remove the #include
>> <linux/types.h> from gfs2_ondisk.h and replace it with our own user space
>> types header. I'm worried about clashes using the kernel type names though,
>> so we might need to process the header somehow to change them.
>
> We could substitute the __beX types with uintX_t on import; also we
> could warn if the system header exists but in a different version than
> what's "cached" in gfs2-utils.

That sounds like a good direction to go, but it would be good to keep 
big-endian types separate from native-endian types with __bitwise so 
that mixing endianness will be picked up by a static checker. I believe 
that was Steve's main concern last time we discussed this.

Andy