On 9/2/22 10:53, Alexander Ivanov wrote:
Repair an image at opening if the image is unclean or
out-of-image corruption was detected.
Signed-off-by: Alexander Ivanov <alexander.iva...@virtuozzo.com>
---
block/parallels.c | 95 ++++++++++++++++++++++++++++++++---------------
1 file changed, 65 insertions(+), 30 deletions(-)
diff --git a/block/parallels.c b/block/parallels.c
index 08526196da..a7c3af4ef2 100644
--- a/block/parallels.c
+++ b/block/parallels.c
@@ -735,6 +735,18 @@ static int coroutine_fn
parallels_co_check(BlockDriverState *bs,
return ret;
}
+typedef struct ParallelsOpenCheckCo {
+ BlockDriverState *bs;
+ BdrvCheckResult *res;
+ BdrvCheckMode fix;
+ int ret;
+} ParallelsOpenCheckCo;
+
+static void coroutine_fn parallels_co_open_check(void *opaque)
+{
+ ParallelsOpenCheckCo *poc = opaque;
+ poc->ret = parallels_co_check(poc->bs, poc->res, poc->fix);
+}
static int coroutine_fn parallels_co_create(BlockdevCreateOptions* opts,
Error **errp)
@@ -947,8 +959,8 @@ static int parallels_open(BlockDriverState *bs, QDict
*options, int flags,
{
BDRVParallelsState *s = bs->opaque;
ParallelsHeader ph;
- int ret, size, i;
- int64_t file_size;
+ int ret, size;
+ int64_t file_size, high_off;
QemuOpts *opts = NULL;
Error *local_err = NULL;
char *buf;
@@ -1027,34 +1039,6 @@ static int parallels_open(BlockDriverState *bs, QDict
*options, int flags,
}
s->bat_bitmap = (uint32_t *)(s->header + 1);
- for (i = 0; i < s->bat_size; i++) {
- int64_t off = bat2sect(s, i);
- if (off >= file_size) {
- if (flags & BDRV_O_CHECK) {
- continue;
- }
- error_setg(errp, "parallels: Offset %" PRIi64 " in BAT[%d] entry "
- "is larger than file size (%" PRIi64 ")",
- off, i, file_size);
- ret = -EINVAL;
- goto fail;
- }
- if (off >= s->data_end) {
- s->data_end = off + s->tracks;
- }
- }
-
- if (le32_to_cpu(ph.inuse) == HEADER_INUSE_MAGIC) {
- /* Image was not closed correctly. The check is mandatory */
- s->header_unclean = true;
- if ((flags & BDRV_O_RDWR) && !(flags & BDRV_O_CHECK)) {
- error_setg(errp, "parallels: Image was not closed correctly; "
- "cannot be opened read/write");
- ret = -EACCES;
- goto fail;
- }
- }
-
opts = qemu_opts_create(¶llels_runtime_opts, NULL, 0, errp);
if (!opts) {
goto fail_options;
@@ -1116,7 +1100,58 @@ static int parallels_open(BlockDriverState *bs, QDict
*options, int flags,
error_free(s->migration_blocker);
goto fail;
}
+
qemu_co_mutex_init(&s->lock);
+
+ if (le32_to_cpu(ph.inuse) == HEADER_INUSE_MAGIC) {
+ s->header_unclean = true;
+ }
+
+ high_off = highest_offset(s) >> BDRV_SECTOR_BITS;
+ if (high_off >= s->data_end) {
+ s->data_end = high_off + s->tracks;
+ }
+
+ /*
+ * We don't repair the image here if it is opened for checks.
+ * Also let to work with images in RO mode.
My silly $0.02.
Also let *us allow* to with in read-only more.
+ */
+ if ((flags & BDRV_O_CHECK) || !(flags & BDRV_O_RDWR)) {
+ return 0;
+ }
not enough, We are not allowed to make changes with O_INACTIVE.
The check in this case should be postponed till O_INACTIVE clearance.
Very specific note. header_unclean is allowed in O_INACTIVE.
The image could be opened at the moment on the other
host!
This should be clarified.
+
+ /*
+ * Repair the image if it's dirty or
+ * out-of-image corruption was detected.
+ */
+ if (s->data_end > file_size ||
+ le32_to_cpu(ph.inuse) == HEADER_INUSE_MAGIC) {
I dislike this. There are detection conditions above
and I think we should respect them adding the flag
'need_check' which should be kept in the BDS to
respect O_INACTIVE.
+ BdrvCheckResult res = {0};
such assignments are weird (not portable for older compilers).
+ Coroutine *co;
+ ParallelsOpenCheckCo poc = {
+ .bs = bs,
+ .res = &res,
This is strange, why not to put BdrvCheckResult as
a whole to the ParallelsOpenCheckCo
+ .fix = BDRV_FIX_ERRORS | BDRV_FIX_LEAKS,
+ .ret = -EINPROGRESS
pls add comma on the line above, This is common convention
as in the case of addition one more initializing field you will
not change that line.
+ };
+
+ if (qemu_in_coroutine()) {
+ /* From bdrv_co_create. */
+ parallels_co_open_check(&poc);
+ } else {
+ assert(qemu_get_current_aio_context() == qemu_get_aio_context());
+ co = qemu_coroutine_create(parallels_co_open_check, &poc);
+ qemu_coroutine_enter(co);
+ BDRV_POLL_WHILE(bs, poc.ret == -EINPROGRESS);
+ }
+
+ if (poc.ret < 0) {
+ error_setg_errno(errp, -poc.ret,
+ "Could not repair corrupted image");
+ goto fail;
+ }
+ }
+
return 0;
fail_format:
