From 3bdc9b93adb0550271ebaed3852dd25720b71b8d Mon Sep 17 00:00:00 2001 From: Andrea Mazzoleni Date: Wed, 28 Sep 2016 20:32:50 +0200 Subject: [PATCH] Add additional checks on parity size Ensures that it's always aligned at the block size. Also adds more comments about resizing process. --- cmdline/check.c | 4 +-- cmdline/dry.c | 2 +- cmdline/parity.c | 76 +++++++++++++++++++++++++++++++++--------------- cmdline/parity.h | 8 ++--- cmdline/scrub.c | 2 +- cmdline/sync.c | 2 +- 6 files changed, 61 insertions(+), 33 deletions(-) diff --git a/cmdline/check.c b/cmdline/check.c index f9d5c732..293139b7 100644 --- a/cmdline/check.c +++ b/cmdline/check.c @@ -1947,7 +1947,7 @@ int state_check(struct snapraid_state* state, int fix, block_off_t blockstart, b } parity_ptr[l] = &parity[l]; - ret = parity_create(parity_ptr[l], &state->parity[l], l, state->file_mode, state->opt.parity_limit_size); + ret = parity_create(parity_ptr[l], &state->parity[l], l, state->file_mode, state->block_size, state->opt.parity_limit_size); if (ret == -1) { /* LCOV_EXCL_START */ log_fatal("WARNING! Without an accessible %s file, it isn't possible to fix any error.\n", lev_name(l)); @@ -1968,7 +1968,7 @@ int state_check(struct snapraid_state* state, int fix, block_off_t blockstart, b /* it may fail if the file doesn't exist, in this case we continue to check the files */ for (l = 0; l < state->level; ++l) { parity_ptr[l] = &parity[l]; - ret = parity_open(parity_ptr[l], &state->parity[l], l, state->file_mode, state->opt.parity_limit_size); + ret = parity_open(parity_ptr[l], &state->parity[l], l, state->file_mode, state->block_size, state->opt.parity_limit_size); if (ret == -1) { msg_status("No accessible %s file, only files will be checked.\n", lev_name(l)); /* continue anyway */ diff --git a/cmdline/dry.c b/cmdline/dry.c index b3551ce3..57e642b8 100644 --- a/cmdline/dry.c +++ b/cmdline/dry.c @@ -417,7 +417,7 @@ void state_dry(struct snapraid_state* state, block_off_t blockstart, block_off_t /* open the file for reading */ /* it may fail if the file doesn't exist, in this case we continue to dry the files */ for (l = 0; l < state->level; ++l) { - ret = parity_open(&parity_handle[l], &state->parity[l], l, state->file_mode, state->opt.parity_limit_size); + ret = parity_open(&parity_handle[l], &state->parity[l], l, state->file_mode, state->block_size, state->opt.parity_limit_size); if (ret == -1) { /* LCOV_EXCL_START */ log_fatal("WARNING! Without an accessible %s file, it isn't possible to dry.\n", lev_name(l)); diff --git a/cmdline/parity.c b/cmdline/parity.c index 8181dab6..9ca6c48a 100644 --- a/cmdline/parity.c +++ b/cmdline/parity.c @@ -161,32 +161,25 @@ void parity_size(struct snapraid_parity_handle* handle, data_off_t* out_size) unsigned s; data_off_t size; - /* now compute the size */ + /* now compute the size summing all the parity splits */ size = 0; + for (s = 0;s < handle->split_mac;++s) { struct snapraid_split_handle* split = &handle->split_map[s]; - /* check for validity */ - if (split->st.st_size < 0) - break; - - if (split->st.st_size < split->size) { - /* this is a size error, so report only up to this size */ - /* but don't fail the function */ - size += split->st.st_size; - break; - } - - /* ignore instead size bigger than expected */ size += split->size; } *out_size = size; } -int parity_create(struct snapraid_parity_handle* handle, const struct snapraid_parity* parity, unsigned level, int mode, data_off_t limit_size) +int parity_create(struct snapraid_parity_handle* handle, const struct snapraid_parity* parity, unsigned level, int mode, uint32_t block_size, data_off_t limit_size) { unsigned s; + data_off_t block_mask; + + /* mask of bits used by the block size */ + block_mask = ((data_off_t)block_size) - 1; handle->level = level; handle->split_mac = 0; @@ -225,11 +218,21 @@ int parity_create(struct snapraid_parity_handle* handle, const struct snapraid_p /** * If the parity size is not yet set, set it now. - * This happens when expanding the number of parities. + * This happens when expanding the number of parities, + * or when upgrading from a content file that has not split->size data. */ - if (split->size == PARITY_SIZE_INVALID) + if (split->size == PARITY_SIZE_INVALID) { split->size = split->st.st_size; + /* ensure that the resulting size if block aligned */ + if ((split->size & block_mask) != 0) { + /* LCOV_EXCL_START */ + log_fatal("Error in preallocated size of parity file '%s' with size %llu and block %u .\n", split->path, split->size, block_size); + goto bail; + /* LCOV_EXCL_STOP */ + } + } + ret = advise_open(&split->advise, split->f); if (ret != 0) { /* LCOV_EXCL_START */ @@ -530,21 +533,26 @@ int parity_chsize(struct snapraid_parity_handle* handle, struct snapraid_parity* data_off_t run; if (is_fixed) { + /* if the required size is smaller, we have to reduce also the file */ + /* ignoring the previous size */ if (size <= split->size) { - is_fixed = 0; /* not fixed anymore */ - run = size; + /* mark it as not fixed anymore for the later check */ + is_fixed = 0; + + run = size; /* allocate only the needed size */ } else { + /* if the size cannot be changed, use the fixed one */ run = split->size; - /* in the < 11.0-38 BETA it was possible to get wrong split size */ if ((run & block_mask) != 0) { /* LCOV_EXCL_START */ - log_fatal("Invalid split '%s' size with extra '%llu' bytes.\n", split->path, run & block_mask); + log_fatal("Internal inconsistency in split '%s' size with extra '%llu' bytes.\n", split->path, run & block_mask); return -1; /* LCOV_EXCL_STOP */ } } } else { + /* otherwise tries to allocate all the needed remaining size */ run = size; } @@ -563,6 +571,7 @@ int parity_chsize(struct snapraid_parity_handle* handle, struct snapraid_parity* return -1; /* LCOV_EXCL_STOP */ } else { + /* here it's possible to get less than the requested size */ run = split->st.st_size; if ((run & block_mask) != 0) { @@ -572,8 +581,11 @@ int parity_chsize(struct snapraid_parity_handle* handle, struct snapraid_parity* /* LCOV_EXCL_STOP */ } - size -= run; + /* store what we have allocated */ split->size = run; + + /* decrease the remaining size */ + size -= run; } } @@ -600,13 +612,17 @@ int parity_chsize(struct snapraid_parity_handle* handle, struct snapraid_parity* return 0; } -int parity_open(struct snapraid_parity_handle* handle, const struct snapraid_parity* parity, unsigned level, int mode, data_off_t limit_size) +int parity_open(struct snapraid_parity_handle* handle, const struct snapraid_parity* parity, unsigned level, int mode, uint32_t block_size, data_off_t limit_size) { unsigned s; + data_off_t block_mask; handle->level = level; handle->split_mac = 0; + /* mask of bits used by the block size */ + block_mask = ((data_off_t)block_size) - 1; + for (s = 0;s < parity->split_mac;++s) { struct snapraid_split_handle* split = &handle->split_map[s]; int ret; @@ -623,8 +639,10 @@ int parity_open(struct snapraid_parity_handle* handle, const struct snapraid_par split->f = open_noatime(split->path, flags); if (split->f == -1) { + /* LCOV_EXCL_START */ log_fatal("Error opening parity file '%s'. %s.\n", split->path, strerror(errno)); goto bail; + /* LCOV_EXCL_STOP */ } /* we have a valid file handle */ @@ -641,11 +659,21 @@ int parity_open(struct snapraid_parity_handle* handle, const struct snapraid_par /** * If the parity size is not yet set, set it now. - * This happens when expanding the number of parities. + * This happens when expanding the number of parities, + * or when upgrading from a content file that has not split->size data. */ - if (split->size == PARITY_SIZE_INVALID) + if (split->size == PARITY_SIZE_INVALID) { split->size = split->st.st_size; + /* ensure that the resulting size if block aligned */ + if ((split->size & block_mask) != 0) { + /* LCOV_EXCL_START */ + log_fatal("Error in preallocated size of parity file '%s' with size %llu and block %u .\n", split->path, split->size, block_size); + goto bail; + /* LCOV_EXCL_STOP */ + } + } + ret = advise_open(&split->advise, split->f); if (ret != 0) { /* LCOV_EXCL_START */ diff --git a/cmdline/parity.h b/cmdline/parity.h index d39f1473..6099416e 100644 --- a/cmdline/parity.h +++ b/cmdline/parity.h @@ -88,7 +88,7 @@ void parity_overflow(struct snapraid_state* state, data_off_t size); * Create the parity file. * \param out_size Return the size of the parity file. */ -int parity_create(struct snapraid_parity_handle* handle, const struct snapraid_parity* parity, unsigned level, int mode, data_off_t limit_size); +int parity_create(struct snapraid_parity_handle* handle, const struct snapraid_parity* parity, unsigned level, int mode, uint32_t block_size, data_off_t limit_size); /** * Change the parity size. @@ -97,16 +97,16 @@ int parity_create(struct snapraid_parity_handle* handle, const struct snapraid_p int parity_chsize(struct snapraid_parity_handle* handle, struct snapraid_parity* parity, int* is_modified, data_off_t size, uint32_t block_size, int skip_fallocate); /** - * Get the size of opened parity file. + * Get the size of the parity. * - * This function never fails as it returns the cached size. + * This returns the cached/expected version of the split sizes, and not the real file size. */ void parity_size(struct snapraid_parity_handle* handle, data_off_t* out_size); /** * Open an already existing parity file. */ -int parity_open(struct snapraid_parity_handle* handle, const struct snapraid_parity* parity, unsigned level, int mode, data_off_t limit_size); +int parity_open(struct snapraid_parity_handle* handle, const struct snapraid_parity* parity, unsigned level, int mode, uint32_t block_size, data_off_t limit_size); /** * Flush the parity file in the disk. diff --git a/cmdline/scrub.c b/cmdline/scrub.c index aa83c757..1936a845 100644 --- a/cmdline/scrub.c +++ b/cmdline/scrub.c @@ -854,7 +854,7 @@ int state_scrub(struct snapraid_state* state, int plan, int olderthan) /* open the file for reading */ for (l = 0; l < state->level; ++l) { - ret = parity_open(&parity_handle[l], &state->parity[l], l, state->file_mode, state->opt.parity_limit_size); + ret = parity_open(&parity_handle[l], &state->parity[l], l, state->file_mode, state->block_size, state->opt.parity_limit_size); if (ret == -1) { /* LCOV_EXCL_START */ log_fatal("WARNING! Without an accessible %s file, it isn't possible to scrub.\n", lev_name(l)); diff --git a/cmdline/sync.c b/cmdline/sync.c index 6db07311..6f00d7ab 100644 --- a/cmdline/sync.c +++ b/cmdline/sync.c @@ -1446,7 +1446,7 @@ int state_sync(struct snapraid_state* state, block_off_t blockstart, block_off_t block_off_t parityblocks; /* create the file and open for writing */ - ret = parity_create(&parity_handle[l], &state->parity[l], l, state->file_mode, state->opt.parity_limit_size); + ret = parity_create(&parity_handle[l], &state->parity[l], l, state->file_mode, state->block_size, state->opt.parity_limit_size); if (ret == -1) { /* LCOV_EXCL_START */ log_fatal("WARNING! Without an accessible %s file, it isn't possible to sync.\n", lev_name(l));