* [PATCH 0/2] qcow2: add keep-dirty open option @ 2022-01-20 16:20 Vladimir Sementsov-Ogievskiy 2022-01-20 16:20 ` [PATCH 1/2] " Vladimir Sementsov-Ogievskiy 2022-01-20 16:20 ` [PATCH 2/2] iotests: add qcow2-keep-dirty Vladimir Sementsov-Ogievskiy 0 siblings, 2 replies; 5+ messages in thread From: Vladimir Sementsov-Ogievskiy @ 2022-01-20 16:20 UTC (permalink / raw) To: qemu-block Cc: qemu-devel, armbru, eblake, hreitz, kwolf, vsementsov, den, ktkhai, igor Hi all! Here is suggestion of a new option which we need for our developments in Virtuozzo. For details look at patch 01. Vladimir Sementsov-Ogievskiy (2): qcow2: add keep-dirty open option iotests: add qcow2-keep-dirty qapi/block-core.json | 5 + block/qcow2.h | 2 + block/qcow2.c | 66 +++++++++-- tests/qemu-iotests/tests/qcow2-keep-dirty | 104 ++++++++++++++++++ tests/qemu-iotests/tests/qcow2-keep-dirty.out | 34 ++++++ 5 files changed, 199 insertions(+), 12 deletions(-) create mode 100755 tests/qemu-iotests/tests/qcow2-keep-dirty create mode 100644 tests/qemu-iotests/tests/qcow2-keep-dirty.out -- 2.31.1 ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH 1/2] qcow2: add keep-dirty open option 2022-01-20 16:20 [PATCH 0/2] qcow2: add keep-dirty open option Vladimir Sementsov-Ogievskiy @ 2022-01-20 16:20 ` Vladimir Sementsov-Ogievskiy 2022-01-27 9:43 ` Kirill Tkhai 2022-01-20 16:20 ` [PATCH 2/2] iotests: add qcow2-keep-dirty Vladimir Sementsov-Ogievskiy 1 sibling, 1 reply; 5+ messages in thread From: Vladimir Sementsov-Ogievskiy @ 2022-01-20 16:20 UTC (permalink / raw) To: qemu-block Cc: qemu-devel, armbru, eblake, hreitz, kwolf, vsementsov, den, ktkhai, igor Consider the case: Thirdparty component works with qcow2 image, and dirty bit is set. Thirdparty component want to start qemu-img to do some manipulation. Ofcourse, third party component flushes refcounts and other metadata before starting QEMU. But the component don't want to clear dirty bit, as this breaks transactionability of the operation: we'll have to set it again but it may fail. Clearing the dirty bit is unrecoverable action and can't be transactional. That's a problem. The solution is a new qcow2 open option: keep-dirty. When set: 1. On qcow2 open, ignore dirty bit and don't do check: caller is responsible for refcounts being valid. 2. Never clear dirty bit during QEMU execution, including close. Details: 1. For simplicity let's just not allow keep-dirty without lazy refcounts. 2. Don't allow to open with keep-dirty when dirty bit is unset. This may mean some error in user logic. 3. For implementation do the following: dirty flag in s->incompatible_features behaves same way as without keep-dirty option: it actually designate status of refcounts dirtiness. But we never clear the flag in the image, and we remember that it is always set. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> --- qapi/block-core.json | 5 ++++ block/qcow2.h | 2 ++ block/qcow2.c | 66 ++++++++++++++++++++++++++++++++++++-------- 3 files changed, 61 insertions(+), 12 deletions(-) diff --git a/qapi/block-core.json b/qapi/block-core.json index 9a5a3641d0..3e35357182 100644 --- a/qapi/block-core.json +++ b/qapi/block-core.json @@ -3228,6 +3228,10 @@ # @lazy-refcounts: whether to enable the lazy refcounts # feature (default is taken from the image file) # +# @keep-dirty: whether to not touch dirty bit. When set, QEMU doesn't +# check refcounts on qcow2 open (ignoring dirty bit) and doesn't +# clear dirty bit on qcow2 close. (since 7.0) +# # @pass-discard-request: whether discard requests to the qcow2 # device should be forwarded to the data source # @@ -3276,6 +3280,7 @@ { 'struct': 'BlockdevOptionsQcow2', 'base': 'BlockdevOptionsGenericCOWFormat', 'data': { '*lazy-refcounts': 'bool', + '*keep-dirty': 'bool', '*pass-discard-request': 'bool', '*pass-discard-snapshot': 'bool', '*pass-discard-other': 'bool', diff --git a/block/qcow2.h b/block/qcow2.h index fd48a89d45..696e13377a 100644 --- a/block/qcow2.h +++ b/block/qcow2.h @@ -130,6 +130,7 @@ #define QCOW2_OPT_DATA_FILE "data-file" #define QCOW2_OPT_LAZY_REFCOUNTS "lazy-refcounts" +#define QCOW2_OPT_KEEP_DIRTY "keep-dirty" #define QCOW2_OPT_DISCARD_REQUEST "pass-discard-request" #define QCOW2_OPT_DISCARD_SNAPSHOT "pass-discard-snapshot" #define QCOW2_OPT_DISCARD_OTHER "pass-discard-other" @@ -376,6 +377,7 @@ typedef struct BDRVQcow2State { int flags; int qcow_version; bool use_lazy_refcounts; + bool keep_dirty; int refcount_order; int refcount_bits; uint64_t refcount_max; diff --git a/block/qcow2.c b/block/qcow2.c index d509016756..1c42103fb9 100644 --- a/block/qcow2.c +++ b/block/qcow2.c @@ -514,15 +514,17 @@ int qcow2_mark_dirty(BlockDriverState *bs) return 0; /* already dirty */ } - val = cpu_to_be64(s->incompatible_features | QCOW2_INCOMPAT_DIRTY); - ret = bdrv_pwrite(bs->file, offsetof(QCowHeader, incompatible_features), - &val, sizeof(val)); - if (ret < 0) { - return ret; - } - ret = bdrv_flush(bs->file->bs); - if (ret < 0) { - return ret; + if (!s->keep_dirty) { + val = cpu_to_be64(s->incompatible_features | QCOW2_INCOMPAT_DIRTY); + ret = bdrv_pwrite(bs->file, offsetof(QCowHeader, incompatible_features), + &val, sizeof(val)); + if (ret < 0) { + return ret; + } + ret = bdrv_flush(bs->file->bs); + if (ret < 0) { + return ret; + } } /* Only treat image as dirty if the header was updated successfully */ @@ -549,7 +551,13 @@ static int qcow2_mark_clean(BlockDriverState *bs) return ret; } - return qcow2_update_header(bs); + if (!s->keep_dirty) { + /* + * No reason to update the header if we don't want to clear dirty + * bit. + */ + return qcow2_update_header(bs); + } } return 0; } @@ -709,6 +717,11 @@ static QemuOptsList qcow2_runtime_opts = { .type = QEMU_OPT_BOOL, .help = "Postpone refcount updates", }, + { + .name = QCOW2_OPT_KEEP_DIRTY, + .type = QEMU_OPT_BOOL, + .help = "Keep dirty bit untouched", + }, { .name = QCOW2_OPT_DISCARD_REQUEST, .type = QEMU_OPT_BOOL, @@ -966,6 +979,7 @@ typedef struct Qcow2ReopenState { Qcow2Cache *refcount_block_cache; int l2_slice_size; /* Number of entries in a slice of the L2 table */ bool use_lazy_refcounts; + bool keep_dirty; int overlap_check; bool discard_passthrough[QCOW2_DISCARD_MAX]; uint64_t cache_clean_interval; @@ -1088,6 +1102,13 @@ static int qcow2_update_options_prepare(BlockDriverState *bs, } } + r->keep_dirty = qemu_opt_get_bool(opts, QCOW2_OPT_KEEP_DIRTY, false); + if (r->keep_dirty && !r->use_lazy_refcounts) { + error_setg(errp, "keep-dirty requires lazy refcounts"); + ret = -EINVAL; + goto fail; + } + /* Overlap check options */ opt_overlap_check = qemu_opt_get(opts, QCOW2_OPT_OVERLAP); opt_overlap_check_template = qemu_opt_get(opts, QCOW2_OPT_OVERLAP_TEMPLATE); @@ -1214,6 +1235,7 @@ static void qcow2_update_options_commit(BlockDriverState *bs, s->overlap_check = r->overlap_check; s->use_lazy_refcounts = r->use_lazy_refcounts; + s->keep_dirty = r->keep_dirty; for (i = 0; i < QCOW2_DISCARD_MAX; i++) { s->discard_passthrough[i] = r->discard_passthrough[i]; @@ -1810,7 +1832,7 @@ static int coroutine_fn qcow2_do_open(BlockDriverState *bs, QDict *options, bs->supported_truncate_flags = BDRV_REQ_ZERO_WRITE; /* Repair image if dirty */ - if (!(flags & BDRV_O_CHECK) && bdrv_is_writable(bs) && + if (!s->keep_dirty && !(flags & BDRV_O_CHECK) && bdrv_is_writable(bs) && (s->incompatible_features & QCOW2_INCOMPAT_DIRTY)) { BdrvCheckResult result = {0}; @@ -1825,6 +1847,20 @@ static int coroutine_fn qcow2_do_open(BlockDriverState *bs, QDict *options, } } + if (s->keep_dirty) { + if (!(s->incompatible_features & QCOW2_INCOMPAT_DIRTY)) { + error_setg(errp, "keep-dirty behaviour is requested but image " + "is not dirty"); + ret = -EINVAL; + goto fail; + } + /* + * User guarantees that refcounts are valid. So, consider them valid, + * keeping dirty bit set in the header. + */ + s->incompatible_features &= ~QCOW2_INCOMPAT_DIRTY; + } + #ifdef DEBUG_ALLOC { BdrvCheckResult result = {0}; @@ -2826,6 +2862,7 @@ int qcow2_update_header(BlockDriverState *bs) uint32_t refcount_table_clusters; size_t header_length; Qcow2UnknownHeaderExtension *uext; + uint64_t incompatible_features; buf = qemu_blockalign(bs, buflen); @@ -2846,6 +2883,11 @@ int qcow2_update_header(BlockDriverState *bs) goto fail; } + incompatible_features = s->incompatible_features; + if (s->keep_dirty) { + incompatible_features |= QCOW2_INCOMPAT_DIRTY; + } + *header = (QCowHeader) { /* Version 2 fields */ .magic = cpu_to_be32(QCOW_MAGIC), @@ -2863,7 +2905,7 @@ int qcow2_update_header(BlockDriverState *bs) .snapshots_offset = cpu_to_be64(s->snapshots_offset), /* Version 3 fields */ - .incompatible_features = cpu_to_be64(s->incompatible_features), + .incompatible_features = cpu_to_be64(incompatible_features), .compatible_features = cpu_to_be64(s->compatible_features), .autoclear_features = cpu_to_be64(s->autoclear_features), .refcount_order = cpu_to_be32(s->refcount_order), -- 2.31.1 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 1/2] qcow2: add keep-dirty open option 2022-01-20 16:20 ` [PATCH 1/2] " Vladimir Sementsov-Ogievskiy @ 2022-01-27 9:43 ` Kirill Tkhai 2022-01-27 9:52 ` Vladimir Sementsov-Ogievskiy 0 siblings, 1 reply; 5+ messages in thread From: Kirill Tkhai @ 2022-01-27 9:43 UTC (permalink / raw) To: Vladimir Sementsov-Ogievskiy, qemu-block Cc: qemu-devel, armbru, eblake, hreitz, kwolf, den, igor On 20.01.2022 19:20, Vladimir Sementsov-Ogievskiy wrote: > Consider the case: > > Thirdparty component works with qcow2 image, and dirty bit is set. > > Thirdparty component want to start qemu-img to do some manipulation. > Ofcourse, third party component flushes refcounts and other metadata > before starting QEMU. > > But the component don't want to clear dirty bit, as this breaks > transactionability of the operation: we'll have to set it again but it > may fail. Clearing the dirty bit is unrecoverable action and can't be > transactional. That's a problem. > > The solution is a new qcow2 open option: keep-dirty. When set: > 1. On qcow2 open, ignore dirty bit and don't do check: caller is > responsible for refcounts being valid. > 2. Never clear dirty bit during QEMU execution, including close. > > Details: > > 1. For simplicity let's just not allow keep-dirty without lazy > refcounts. > > 2. Don't allow to open with keep-dirty when dirty bit is unset. This > may mean some error in user logic. > > 3. For implementation do the following: dirty flag > in s->incompatible_features behaves same way as without keep-dirty > option: it actually designate status of refcounts dirtiness. But we > never clear the flag in the image, and we remember that it is always > set. > > Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> > --- > qapi/block-core.json | 5 ++++ > block/qcow2.h | 2 ++ > block/qcow2.c | 66 ++++++++++++++++++++++++++++++++++++-------- > 3 files changed, 61 insertions(+), 12 deletions(-) > > diff --git a/qapi/block-core.json b/qapi/block-core.json > index 9a5a3641d0..3e35357182 100644 > --- a/qapi/block-core.json > +++ b/qapi/block-core.json > @@ -3228,6 +3228,10 @@ > # @lazy-refcounts: whether to enable the lazy refcounts > # feature (default is taken from the image file) > # > +# @keep-dirty: whether to not touch dirty bit. When set, QEMU doesn't > +# check refcounts on qcow2 open (ignoring dirty bit) and doesn't > +# clear dirty bit on qcow2 close. (since 7.0) > +# > # @pass-discard-request: whether discard requests to the qcow2 > # device should be forwarded to the data source > # > @@ -3276,6 +3280,7 @@ > { 'struct': 'BlockdevOptionsQcow2', > 'base': 'BlockdevOptionsGenericCOWFormat', > 'data': { '*lazy-refcounts': 'bool', > + '*keep-dirty': 'bool', > '*pass-discard-request': 'bool', > '*pass-discard-snapshot': 'bool', > '*pass-discard-other': 'bool', > diff --git a/block/qcow2.h b/block/qcow2.h > index fd48a89d45..696e13377a 100644 > --- a/block/qcow2.h > +++ b/block/qcow2.h > @@ -130,6 +130,7 @@ > > #define QCOW2_OPT_DATA_FILE "data-file" > #define QCOW2_OPT_LAZY_REFCOUNTS "lazy-refcounts" > +#define QCOW2_OPT_KEEP_DIRTY "keep-dirty" > #define QCOW2_OPT_DISCARD_REQUEST "pass-discard-request" > #define QCOW2_OPT_DISCARD_SNAPSHOT "pass-discard-snapshot" > #define QCOW2_OPT_DISCARD_OTHER "pass-discard-other" > @@ -376,6 +377,7 @@ typedef struct BDRVQcow2State { > int flags; > int qcow_version; > bool use_lazy_refcounts; > + bool keep_dirty; > int refcount_order; > int refcount_bits; > uint64_t refcount_max; > diff --git a/block/qcow2.c b/block/qcow2.c > index d509016756..1c42103fb9 100644 > --- a/block/qcow2.c > +++ b/block/qcow2.c > @@ -514,15 +514,17 @@ int qcow2_mark_dirty(BlockDriverState *bs) > return 0; /* already dirty */ > } > > - val = cpu_to_be64(s->incompatible_features | QCOW2_INCOMPAT_DIRTY); > - ret = bdrv_pwrite(bs->file, offsetof(QCowHeader, incompatible_features), > - &val, sizeof(val)); > - if (ret < 0) { > - return ret; > - } > - ret = bdrv_flush(bs->file->bs); > - if (ret < 0) { > - return ret; > + if (!s->keep_dirty) { > + val = cpu_to_be64(s->incompatible_features | QCOW2_INCOMPAT_DIRTY); > + ret = bdrv_pwrite(bs->file, offsetof(QCowHeader, incompatible_features), > + &val, sizeof(val)); > + if (ret < 0) { > + return ret; > + } > + ret = bdrv_flush(bs->file->bs); > + if (ret < 0) { > + return ret; > + } > } > > /* Only treat image as dirty if the header was updated successfully */ > @@ -549,7 +551,13 @@ static int qcow2_mark_clean(BlockDriverState *bs) > return ret; > } > > - return qcow2_update_header(bs); > + if (!s->keep_dirty) { > + /* > + * No reason to update the header if we don't want to clear dirty > + * bit. > + */ > + return qcow2_update_header(bs); > + } > } > return 0; > } > @@ -709,6 +717,11 @@ static QemuOptsList qcow2_runtime_opts = { > .type = QEMU_OPT_BOOL, > .help = "Postpone refcount updates", > }, > + { > + .name = QCOW2_OPT_KEEP_DIRTY, > + .type = QEMU_OPT_BOOL, > + .help = "Keep dirty bit untouched", > + }, > { > .name = QCOW2_OPT_DISCARD_REQUEST, > .type = QEMU_OPT_BOOL, > @@ -966,6 +979,7 @@ typedef struct Qcow2ReopenState { > Qcow2Cache *refcount_block_cache; > int l2_slice_size; /* Number of entries in a slice of the L2 table */ > bool use_lazy_refcounts; > + bool keep_dirty; > int overlap_check; > bool discard_passthrough[QCOW2_DISCARD_MAX]; > uint64_t cache_clean_interval; > @@ -1088,6 +1102,13 @@ static int qcow2_update_options_prepare(BlockDriverState *bs, > } > } > > + r->keep_dirty = qemu_opt_get_bool(opts, QCOW2_OPT_KEEP_DIRTY, false); > + if (r->keep_dirty && !r->use_lazy_refcounts) { > + error_setg(errp, "keep-dirty requires lazy refcounts"); > + ret = -EINVAL; > + goto fail; Vladimir, according to documentation dirty bit and lazy refcounts are independent: incompatible_features: Bit 0: Dirty bit. If this bit is set then refcounts may be inconsistent, make sure to scan L1/L2 tables to repair refcounts before accessing the image. compatible_features: Bit 0: Lazy refcounts bit. If this bit is set then lazy refcount updates can be used. This means marking the image file dirty and postponing refcount metadata updates. while this patch assumes existence of some dependence between them. Is there a real reason to introduce such the connection? [snip] Kirill ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 1/2] qcow2: add keep-dirty open option 2022-01-27 9:43 ` Kirill Tkhai @ 2022-01-27 9:52 ` Vladimir Sementsov-Ogievskiy 0 siblings, 0 replies; 5+ messages in thread From: Vladimir Sementsov-Ogievskiy @ 2022-01-27 9:52 UTC (permalink / raw) To: Kirill Tkhai, qemu-block Cc: qemu-devel, armbru, eblake, hreitz, kwolf, den, igor 27.01.2022 12:43, Kirill Tkhai wrote: > On 20.01.2022 19:20, Vladimir Sementsov-Ogievskiy wrote: >> Consider the case: >> >> Thirdparty component works with qcow2 image, and dirty bit is set. >> >> Thirdparty component want to start qemu-img to do some manipulation. >> Ofcourse, third party component flushes refcounts and other metadata >> before starting QEMU. >> >> But the component don't want to clear dirty bit, as this breaks >> transactionability of the operation: we'll have to set it again but it >> may fail. Clearing the dirty bit is unrecoverable action and can't be >> transactional. That's a problem. >> >> The solution is a new qcow2 open option: keep-dirty. When set: >> 1. On qcow2 open, ignore dirty bit and don't do check: caller is >> responsible for refcounts being valid. >> 2. Never clear dirty bit during QEMU execution, including close. >> >> Details: >> >> 1. For simplicity let's just not allow keep-dirty without lazy >> refcounts. >> >> 2. Don't allow to open with keep-dirty when dirty bit is unset. This >> may mean some error in user logic. >> >> 3. For implementation do the following: dirty flag >> in s->incompatible_features behaves same way as without keep-dirty >> option: it actually designate status of refcounts dirtiness. But we >> never clear the flag in the image, and we remember that it is always >> set. >> >> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> >> --- >> qapi/block-core.json | 5 ++++ >> block/qcow2.h | 2 ++ >> block/qcow2.c | 66 ++++++++++++++++++++++++++++++++++++-------- >> 3 files changed, 61 insertions(+), 12 deletions(-) >> >> diff --git a/qapi/block-core.json b/qapi/block-core.json >> index 9a5a3641d0..3e35357182 100644 >> --- a/qapi/block-core.json >> +++ b/qapi/block-core.json >> @@ -3228,6 +3228,10 @@ >> # @lazy-refcounts: whether to enable the lazy refcounts >> # feature (default is taken from the image file) >> # >> +# @keep-dirty: whether to not touch dirty bit. When set, QEMU doesn't >> +# check refcounts on qcow2 open (ignoring dirty bit) and doesn't >> +# clear dirty bit on qcow2 close. (since 7.0) >> +# >> # @pass-discard-request: whether discard requests to the qcow2 >> # device should be forwarded to the data source >> # >> @@ -3276,6 +3280,7 @@ >> { 'struct': 'BlockdevOptionsQcow2', >> 'base': 'BlockdevOptionsGenericCOWFormat', >> 'data': { '*lazy-refcounts': 'bool', >> + '*keep-dirty': 'bool', >> '*pass-discard-request': 'bool', >> '*pass-discard-snapshot': 'bool', >> '*pass-discard-other': 'bool', >> diff --git a/block/qcow2.h b/block/qcow2.h >> index fd48a89d45..696e13377a 100644 >> --- a/block/qcow2.h >> +++ b/block/qcow2.h >> @@ -130,6 +130,7 @@ >> >> #define QCOW2_OPT_DATA_FILE "data-file" >> #define QCOW2_OPT_LAZY_REFCOUNTS "lazy-refcounts" >> +#define QCOW2_OPT_KEEP_DIRTY "keep-dirty" >> #define QCOW2_OPT_DISCARD_REQUEST "pass-discard-request" >> #define QCOW2_OPT_DISCARD_SNAPSHOT "pass-discard-snapshot" >> #define QCOW2_OPT_DISCARD_OTHER "pass-discard-other" >> @@ -376,6 +377,7 @@ typedef struct BDRVQcow2State { >> int flags; >> int qcow_version; >> bool use_lazy_refcounts; >> + bool keep_dirty; >> int refcount_order; >> int refcount_bits; >> uint64_t refcount_max; >> diff --git a/block/qcow2.c b/block/qcow2.c >> index d509016756..1c42103fb9 100644 >> --- a/block/qcow2.c >> +++ b/block/qcow2.c >> @@ -514,15 +514,17 @@ int qcow2_mark_dirty(BlockDriverState *bs) >> return 0; /* already dirty */ >> } >> >> - val = cpu_to_be64(s->incompatible_features | QCOW2_INCOMPAT_DIRTY); >> - ret = bdrv_pwrite(bs->file, offsetof(QCowHeader, incompatible_features), >> - &val, sizeof(val)); >> - if (ret < 0) { >> - return ret; >> - } >> - ret = bdrv_flush(bs->file->bs); >> - if (ret < 0) { >> - return ret; >> + if (!s->keep_dirty) { >> + val = cpu_to_be64(s->incompatible_features | QCOW2_INCOMPAT_DIRTY); >> + ret = bdrv_pwrite(bs->file, offsetof(QCowHeader, incompatible_features), >> + &val, sizeof(val)); >> + if (ret < 0) { >> + return ret; >> + } >> + ret = bdrv_flush(bs->file->bs); >> + if (ret < 0) { >> + return ret; >> + } >> } >> >> /* Only treat image as dirty if the header was updated successfully */ >> @@ -549,7 +551,13 @@ static int qcow2_mark_clean(BlockDriverState *bs) >> return ret; >> } >> >> - return qcow2_update_header(bs); >> + if (!s->keep_dirty) { >> + /* >> + * No reason to update the header if we don't want to clear dirty >> + * bit. >> + */ >> + return qcow2_update_header(bs); >> + } >> } >> return 0; >> } >> @@ -709,6 +717,11 @@ static QemuOptsList qcow2_runtime_opts = { >> .type = QEMU_OPT_BOOL, >> .help = "Postpone refcount updates", >> }, >> + { >> + .name = QCOW2_OPT_KEEP_DIRTY, >> + .type = QEMU_OPT_BOOL, >> + .help = "Keep dirty bit untouched", >> + }, >> { >> .name = QCOW2_OPT_DISCARD_REQUEST, >> .type = QEMU_OPT_BOOL, >> @@ -966,6 +979,7 @@ typedef struct Qcow2ReopenState { >> Qcow2Cache *refcount_block_cache; >> int l2_slice_size; /* Number of entries in a slice of the L2 table */ >> bool use_lazy_refcounts; >> + bool keep_dirty; >> int overlap_check; >> bool discard_passthrough[QCOW2_DISCARD_MAX]; >> uint64_t cache_clean_interval; >> @@ -1088,6 +1102,13 @@ static int qcow2_update_options_prepare(BlockDriverState *bs, >> } >> } >> >> + r->keep_dirty = qemu_opt_get_bool(opts, QCOW2_OPT_KEEP_DIRTY, false); >> + if (r->keep_dirty && !r->use_lazy_refcounts) { >> + error_setg(errp, "keep-dirty requires lazy refcounts"); >> + ret = -EINVAL; >> + goto fail; > > Vladimir, according to documentation dirty bit and lazy refcounts are independent: > > incompatible_features: > Bit 0: Dirty bit. If this bit is set then refcounts > may be inconsistent, make sure to scan L1/L2 > tables to repair refcounts before accessing the > image. > > compatible_features: > Bit 0: Lazy refcounts bit. If this bit is set then > lazy refcount updates can be used. This means > marking the image file dirty and postponing > refcount metadata updates. > > while this patch assumes existence of some dependence between them. > Is there a real reason to introduce such the connection? > Hmm, somehow I thought that there was some documented dependency.. But seems you are right. Honestly, I don't know real scenarios where non-lazy refcounts make sense. In Virtuozzo we always use lazy refcounts turned on by bit in the image. OK, I'll drop a dependency for new option. -- Best regards, Vladimir ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH 2/2] iotests: add qcow2-keep-dirty 2022-01-20 16:20 [PATCH 0/2] qcow2: add keep-dirty open option Vladimir Sementsov-Ogievskiy 2022-01-20 16:20 ` [PATCH 1/2] " Vladimir Sementsov-Ogievskiy @ 2022-01-20 16:20 ` Vladimir Sementsov-Ogievskiy 1 sibling, 0 replies; 5+ messages in thread From: Vladimir Sementsov-Ogievskiy @ 2022-01-20 16:20 UTC (permalink / raw) To: qemu-block Cc: qemu-devel, armbru, eblake, hreitz, kwolf, vsementsov, den, ktkhai, igor Test new qcow2 open option: keep-dirty. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> --- tests/qemu-iotests/tests/qcow2-keep-dirty | 104 ++++++++++++++++++ tests/qemu-iotests/tests/qcow2-keep-dirty.out | 34 ++++++ 2 files changed, 138 insertions(+) create mode 100755 tests/qemu-iotests/tests/qcow2-keep-dirty create mode 100644 tests/qemu-iotests/tests/qcow2-keep-dirty.out diff --git a/tests/qemu-iotests/tests/qcow2-keep-dirty b/tests/qemu-iotests/tests/qcow2-keep-dirty new file mode 100755 index 0000000000..101a82bd28 --- /dev/null +++ b/tests/qemu-iotests/tests/qcow2-keep-dirty @@ -0,0 +1,104 @@ +#!/usr/bin/env bash +# group: rw quick +# +# Test qcow2 keep-dirty option +# +# Copyright (c) 2022 Virtuozzo International GmbH. +# +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 2 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see <http://www.gnu.org/licenses/>. +# + +# creator +owner=vsementsov@virtuozzo.com + +seq=`basename $0` +echo "QA output created by $seq" + +status=1 # failure is the default! + +_cleanup() +{ + _cleanup_test_img +} +trap "_cleanup; exit \$status" 0 1 2 3 15 + +# get standard environment, filters and checks +. ../common.rc +. ../common.filter +. ../common.qemu + +_supported_fmt qcow2 +_supported_proto file +_supported_os Linux +# This test does not make much sense with external data files +_unsupported_imgopts data_file + +check_dirty_bit() { + $QEMU_IMG info --output=json "$TEST_IMG" | grep 'dirty-flag' +} + +set_dirty_bit() { + echo Set dirty bit + $PYTHON ../qcow2.py "$TEST_IMG" set-feature-bit incompatible 0 + check_dirty_bit +} + +QEMU_IO_OPTIONS=$QEMU_IO_OPTIONS_NO_FMT + +keep_dirty_opts="driver=qcow2,keep-dirty=true,file.filename=$TEST_IMG" + +size=10M + +_make_test_img $size + +echo Check that keep-dirty not allowed without lazy refcounts +$QEMU_IO -c 'quit' --image-opts "$keep_dirty_opts" + +_make_test_img -o "lazy_refcounts=on" $size + +echo +echo Check that keep-dirty not allowed without dirty bit +$QEMU_IO -c 'quit' --image-opts "$keep_dirty_opts" + +echo +echo Check that usual access clears dirty bit +set_dirty_bit +$QEMU_IO -c 'quit' "$TEST_IMG" +check_dirty_bit + +echo +echo Check keep-dirty +set_dirty_bit +$QEMU_IO -c 'quit' --image-opts "$keep_dirty_opts" +check_dirty_bit + +echo +echo Check that usual qemu-img check clears dirty bit +set_dirty_bit +$QEMU_IMG check -r all "$TEST_IMG" +check_dirty_bit + +echo +echo Test qemu-img check with keep-dirty +set_dirty_bit +# also set corrupt bit +$PYTHON ../qcow2.py "$TEST_IMG" set-feature-bit incompatible 1 +$PYTHON ../qcow2.py "$TEST_IMG" dump-header | grep incompatible +$QEMU_IMG check -r all --image-opts "$keep_dirty_opts" +$PYTHON ../qcow2.py "$TEST_IMG" dump-header | grep incompatible + +# success, all done +echo "*** done" +rm -f $seq.full +status=0 diff --git a/tests/qemu-iotests/tests/qcow2-keep-dirty.out b/tests/qemu-iotests/tests/qcow2-keep-dirty.out new file mode 100644 index 0000000000..4d2bf40521 --- /dev/null +++ b/tests/qemu-iotests/tests/qcow2-keep-dirty.out @@ -0,0 +1,34 @@ +QA output created by qcow2-keep-dirty +Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=10485760 +Check that keep-dirty not allowed without lazy refcounts +qemu-io: can't open: keep-dirty requires lazy refcounts +Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=10485760 + +Check that keep-dirty not allowed without dirty bit +qemu-io: can't open: keep-dirty behaviour is requested but image is not dirty + +Check that usual access clears dirty bit +Set dirty bit + "dirty-flag": true + "dirty-flag": false + +Check keep-dirty +Set dirty bit + "dirty-flag": true + "dirty-flag": true + +Check that usual qemu-img check clears dirty bit +Set dirty bit + "dirty-flag": true +No errors were found on the image. +Image end offset: 262144 + "dirty-flag": false + +Test qemu-img check with keep-dirty +Set dirty bit + "dirty-flag": true +incompatible_features [0, 1] +No errors were found on the image. +Image end offset: 262144 +incompatible_features [0] +*** done -- 2.31.1 ^ permalink raw reply related [flat|nested] 5+ messages in thread
end of thread, other threads:[~2022-01-27 10:18 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2022-01-20 16:20 [PATCH 0/2] qcow2: add keep-dirty open option Vladimir Sementsov-Ogievskiy 2022-01-20 16:20 ` [PATCH 1/2] " Vladimir Sementsov-Ogievskiy 2022-01-27 9:43 ` Kirill Tkhai 2022-01-27 9:52 ` Vladimir Sementsov-Ogievskiy 2022-01-20 16:20 ` [PATCH 2/2] iotests: add qcow2-keep-dirty Vladimir Sementsov-Ogievskiy
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.