On Wed, 20 May 2015 11:20:36 +0800 Guoqing Jiang <gqjiang@xxxxxxxx> wrote:
> This adds capability of exmining bitmaps corresponding to all
> nodes/slots on the device.
>
> Signed-off-by: Goldwyn Rodrigues <rgoldwyn@xxxxxxxx>
> Signed-off-by: Guoqing Jiang <gqjiang@xxxxxxxx>
> ---
> bitmap.c | 72 ++++++++++++++++++++++++++++++++++++++++++++++++----------------
> 1 file changed, 54 insertions(+), 18 deletions(-)
>
> diff --git a/bitmap.c b/bitmap.c
> index 920033a..bccc67c 100644
> --- a/bitmap.c
> +++ b/bitmap.c
> @@ -260,7 +260,7 @@ int ExamineBitmap(char *filename, int brief, struct supertype *st)
> int rv = 1;
> char buf[64];
> int swap;
> - int fd;
> + int fd, i;
> __u32 uuid32[4];
>
> fd = bitmap_file_open(filename, &st);
> @@ -317,23 +317,59 @@ int ExamineBitmap(char *filename, int brief, struct supertype *st)
> uuid32[2],
> uuid32[3]);
>
> - printf(" Events : %llu\n", (unsigned long long)sb->events);
> - printf(" Events Cleared : %llu\n", (unsigned long long)sb->events_cleared);
> - printf(" State : %s\n", bitmap_state(sb->state));
> - printf(" Chunksize : %s\n", human_chunksize(sb->chunksize));
> - printf(" Daemon : %ds flush period\n", sb->daemon_sleep);
> - if (sb->write_behind)
> - sprintf(buf, "Allow write behind, max %d", sb->write_behind);
> - else
> - sprintf(buf, "Normal");
> - printf(" Write Mode : %s\n", buf);
> - printf(" Sync Size : %llu%s\n", (unsigned long long)sb->sync_size/2,
> - human_size(sb->sync_size * 512));
> - if (brief)
> - goto free_info;
> - printf(" Bitmap : %llu bits (chunks), %llu dirty (%2.1f%%)\n",
> - info->total_bits, info->dirty_bits,
> - 100.0 * info->dirty_bits / (info->total_bits?:1));
> + if (sb->nodes == 0) {
> + printf(" Events : %llu\n", (unsigned long long)sb->events);
> + printf(" Events Cleared : %llu\n", (unsigned long long)sb->events_cleared);
> + printf(" State : %s\n", bitmap_state(sb->state));
> + printf(" Chunksize : %s\n", human_chunksize(sb->chunksize));
> + printf(" Daemon : %ds flush period\n", sb->daemon_sleep);
> + if (sb->write_behind)
> + sprintf(buf, "Allow write behind, max %d", sb->write_behind);
> + else
> + sprintf(buf, "Normal");
> + printf(" Write Mode : %s\n", buf);
> + printf(" Sync Size : %llu%s\n", (unsigned long long)sb->sync_size/2,
> + human_size(sb->sync_size * 512));
> + if (brief)
> + goto free_info;
> + printf(" Bitmap : %llu bits (chunks), %llu dirty (%2.1f%%)\n",
> + info->total_bits, info->dirty_bits,
> + 100.0 * info->dirty_bits / (info->total_bits?:1));
> + } else {
> + printf(" Chunksize : %s\n", human_chunksize(sb->chunksize));
> + printf(" Daemon : %ds flush period\n", sb->daemon_sleep);
> + if (sb->write_behind)
> + sprintf(buf, "Allow write behind, max %d", sb->write_behind);
> + else
> + sprintf(buf, "Normal");
> + printf(" Write Mode : %s\n", buf);
> + printf(" Sync Size : %llu%s\n", (unsigned long long)sb->sync_size/2,
> + human_size(sb->sync_size * 512));
This is too much duplication. Duplicate code leads to maintenance errors.
if (sb->nodes == 0)
print Events/ Events cleared / state
print chunksuze, daemon flush period, write mode sync size
if (sb->nodes > 0) {
print Events/ Events cleared etc for each bitmap
}
> + printf(" Cluster nodes : %d\n", sb->nodes);
> + printf(" Cluster name : %s\n", sb->cluster_name);
sb->cluster_name is not nul terminated if it is exactly 64 bytes long.
You need "%64s" ... and you need to test that case.
> + i = 0;
> + do {
Please make this a regular
for(i = 0; i < nodes; i++)
loop. do/while should only be used when some non-trivial computation is
needed before the condition can be tested.
> + if (i) {
> + free(info);
> + info = bitmap_fd_read(fd, brief);
> + sb = &info->sb;
> + }
> + if (sb->magic != BITMAP_MAGIC)
> + pr_err("invalid bitmap magic 0x%x, the bitmap file appears to be corrupted\n", sb->magic);
> +
> + printf(" Node Slot : %d\n", i);
> + printf(" Events : %llu\n", (unsigned long long)sb->events);
> + printf(" Events Cleared : %llu\n", (unsigned long long)sb->events_cleared);
> + printf(" State : %s\n", bitmap_state(sb->state));
> + if (brief)
> + continue;
> + printf(" Bitmap : %llu bits (chunks), %llu dirty (%2.1f%%)\n",
> + info->total_bits, info->dirty_bits,
> + 100.0 * info->dirty_bits / (info->total_bits?:1));
> +
> + } while (++i < (int)sb->nodes);
> + }
> +
> free_info:
> free(info);
> return rv;
Thanks,
NeilBrown
Attachment:
pgp7GeUjXpJUm.pgp
Description: OpenPGP digital signature
