[sheepdog] [PATCH] journal: refactor the journaling subsystem

[Kai Zhang]

Hitoshi Mitake <mitake.hitoshi at lab.ntt.co.jp> writes:

> Current journaling subsystem has some dirty parts because of its old
> journal descriptor types (epoch and config). This patch cleans them.
>
> Signed-off-by: Hitoshi Mitake <mitake.hitoshi at lab.ntt.co.jp>
> ---
>  sheep/journal.c |   31 +++++++++++++++----------------
>  1 files changed, 15 insertions(+), 16 deletions(-)
>
> diff --git a/sheep/journal.c b/sheep/journal.c
> index 50783f2..f1f94da 100644
> --- a/sheep/journal.c
> +++ b/sheep/journal.c
> @@ -137,21 +137,18 @@ static bool journal_entry_full_write(struct journal_descriptor *jd)
>
>  static void journal_get_path(struct journal_descriptor *jd, char *path)
>  {
> -	switch (jd->flag) {
> -	case JF_STORE:
> -	case JF_REMOVE_OBJ:
> -		snprintf(path, PATH_MAX, "%s/%016"PRIx64,
> -			 md_get_object_path(jd->oid), jd->oid);
> -		if (jd->flag == JF_STORE)
> -			sd_iprintf("%s, size %"PRIu64", off %"PRIu64", %d",
> -				path, jd->size, jd->offset, jd->create);
> -		else		/* JF_REMOVE_OBJ */
> -			sd_iprintf("%s (remove)", path);
> -		break;
> -	default:
> -		panic("unknown type of journal flag: %d", jd->flag);
> -		break;
> -	}
> +	snprintf(path, PATH_MAX, "%s/%016"PRIx64,
> +		md_get_object_path(jd->oid), jd->oid);
> +
> +	if (jd->flag == JF_STORE)
> +		sd_iprintf("%s, size %"PRIu64", off %"PRIu64", %d",
> +			path, jd->size, jd->offset, jd->create);
> +	else if (jd->flag == JF_REMOVE_OBJ)
> +		sd_iprintf("%s (remove)", path);
> +	else
> +		panic("unknown type of journal flag,"
> +			" the journal flag seems to be corrupted: %d",
> +			jd->flag);
>  }
>
>  static int replay_journal_entry(struct journal_descriptor *jd)
> @@ -168,6 +165,8 @@ static int replay_journal_entry(struct journal_descriptor *jd)
>		return 0;
>	}
>
> +	assert(jd->flag == JF_STORE);
> +

Because journal_get_path(jd, patch) is needed whenever jd->flag is
JF_REMOVE_OBJ or JF_STORE, I think we can put it to the top to clean the code.
And because the value of jd->flag will be checked by both journal_get_path(),
and this assert, so it would be better to check it in one place.
The modifide code would be like:

journal_get_path(jd, path); // no check
if (jd->flag == JF_REMOVE_OBJ) {
   unlink(path);
   return 0;
} else if (jd->flag == JF_RESTORE) {
   /* restore logic here */
} else {
   panic(...);
}

A switch case is also a good choice.


>	if (jd->create)
>		flags |= O_CREAT;
>
> @@ -178,7 +177,7 @@ static int replay_journal_entry(struct journal_descriptor *jd)
>		return -1;
>	}
>
> -	if (jd->create && jd->flag == JF_STORE) {
> +	if (jd->create) {
>		ret = prealloc(fd, get_objsize(jd->oid));
>		if (ret < 0)
>			goto out;
> --
> 1.7.2.5