block/mirror.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-)
If a blockcommit is aborted the base image remains in RW mode, that leads
to a fail of subsequent live migration.
How to reproduce:
$ virsh snapshot-create-as vm snp1 --disk-only
*** write something to the disk inside the guest ***
$ virsh blockcommit vm vda --active --shallow && virsh blockjob vm vda --abort
$ lsof /vzt/vm.qcow2
COMMAND PID USER FD TYPE DEVICE SIZE/OFF NODE NAME
qemu-syst 433203 root 45u REG 253,0 1724776448 133 /vzt/vm.qcow2
$ cat /proc/433203/fdinfo/45
pos: 0
flags: 02140002 <==== The last 2 means RW mode
If the base image is in RW mode at the end of blockcommit and was in RO
mode before blockcommit, reopen the base BDS in RO.
Signed-off-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com>
---
block/mirror.c | 11 +++++++++--
1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/block/mirror.c b/block/mirror.c
index 1bdce3b657..d23be57255 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -93,6 +93,7 @@ typedef struct MirrorBlockJob {
int64_t active_write_bytes_in_flight;
bool prepared;
bool in_drain;
+ bool base_ro;
} MirrorBlockJob;
typedef struct MirrorBDSOpaque {
@@ -797,6 +798,10 @@ static int mirror_exit_common(Job *job)
bdrv_drained_end(target_bs);
bdrv_unref(target_bs);
+ if (abort && s->base_ro && !bdrv_is_read_only(target_bs)) {
+ bdrv_reopen_set_read_only(target_bs, true, NULL);
+ }
+
bs_opaque->job = NULL;
bdrv_drained_end(src);
@@ -1717,6 +1722,7 @@ static BlockJob *mirror_start_job(
bool is_none_mode, BlockDriverState *base,
bool auto_complete, const char *filter_node_name,
bool is_mirror, MirrorCopyMode copy_mode,
+ bool base_ro,
Error **errp)
{
MirrorBlockJob *s;
@@ -1800,6 +1806,7 @@ static BlockJob *mirror_start_job(
bdrv_unref(mirror_top_bs);
s->mirror_top_bs = mirror_top_bs;
+ s->base_ro = base_ro;
/* No resize for the target either; while the mirror is still running, a
* consistent read isn't necessarily possible. We could possibly allow
@@ -2029,7 +2036,7 @@ void mirror_start(const char *job_id, BlockDriverState *bs,
speed, granularity, buf_size, backing_mode, zero_target,
on_source_error, on_target_error, unmap, NULL, NULL,
&mirror_job_driver, is_none_mode, base, false,
- filter_node_name, true, copy_mode, errp);
+ filter_node_name, true, copy_mode, false, errp);
}
BlockJob *commit_active_start(const char *job_id, BlockDriverState *bs,
@@ -2058,7 +2065,7 @@ BlockJob *commit_active_start(const char *job_id, BlockDriverState *bs,
on_error, on_error, true, cb, opaque,
&commit_active_job_driver, false, base, auto_complete,
filter_node_name, false, MIRROR_COPY_MODE_BACKGROUND,
- errp);
+ base_read_only, errp);
if (!job) {
goto error_restore_flags;
}
--
2.40.1On 28.03.24 12:16, Alexander Ivanov wrote:
> If a blockcommit is aborted the base image remains in RW mode, that leads
> to a fail of subsequent live migration.
>
> How to reproduce:
> $ virsh snapshot-create-as vm snp1 --disk-only
>
> *** write something to the disk inside the guest ***
>
> $ virsh blockcommit vm vda --active --shallow && virsh blockjob vm vda --abort
> $ lsof /vzt/vm.qcow2
> COMMAND PID USER FD TYPE DEVICE SIZE/OFF NODE NAME
> qemu-syst 433203 root 45u REG 253,0 1724776448 133 /vzt/vm.qcow2
> $ cat /proc/433203/fdinfo/45
> pos: 0
> flags: 02140002 <==== The last 2 means RW mode
>
> If the base image is in RW mode at the end of blockcommit and was in RO
> mode before blockcommit, reopen the base BDS in RO.
>
> Signed-off-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com>
> ---
> block/mirror.c | 11 +++++++++--
> 1 file changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/block/mirror.c b/block/mirror.c
> index 1bdce3b657..d23be57255 100644
> --- a/block/mirror.c
> +++ b/block/mirror.c
> @@ -93,6 +93,7 @@ typedef struct MirrorBlockJob {
> int64_t active_write_bytes_in_flight;
> bool prepared;
> bool in_drain;
> + bool base_ro;
> } MirrorBlockJob;
>
> typedef struct MirrorBDSOpaque {
> @@ -797,6 +798,10 @@ static int mirror_exit_common(Job *job)
> bdrv_drained_end(target_bs);
> bdrv_unref(target_bs);
>
> + if (abort && s->base_ro && !bdrv_is_read_only(target_bs)) {
> + bdrv_reopen_set_read_only(target_bs, true, NULL);
> + }
> +
All looks good to me except this: seems it is safer to place this "if" block before "bdrv_drained_end(); bdrv_unref();" above. With it moved:
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
> bs_opaque->job = NULL;
>
> bdrv_drained_end(src);
> @@ -1717,6 +1722,7 @@ static BlockJob *mirror_start_job(
> bool is_none_mode, BlockDriverState *base,
> bool auto_complete, const char *filter_node_name,
> bool is_mirror, MirrorCopyMode copy_mode,
> + bool base_ro,
> Error **errp)
> {
> MirrorBlockJob *s;
> @@ -1800,6 +1806,7 @@ static BlockJob *mirror_start_job(
> bdrv_unref(mirror_top_bs);
>
> s->mirror_top_bs = mirror_top_bs;
> + s->base_ro = base_ro;
>
> /* No resize for the target either; while the mirror is still running, a
> * consistent read isn't necessarily possible. We could possibly allow
> @@ -2029,7 +2036,7 @@ void mirror_start(const char *job_id, BlockDriverState *bs,
> speed, granularity, buf_size, backing_mode, zero_target,
> on_source_error, on_target_error, unmap, NULL, NULL,
> &mirror_job_driver, is_none_mode, base, false,
> - filter_node_name, true, copy_mode, errp);
> + filter_node_name, true, copy_mode, false, errp);
> }
>
> BlockJob *commit_active_start(const char *job_id, BlockDriverState *bs,
> @@ -2058,7 +2065,7 @@ BlockJob *commit_active_start(const char *job_id, BlockDriverState *bs,
> on_error, on_error, true, cb, opaque,
> &commit_active_job_driver, false, base, auto_complete,
> filter_node_name, false, MIRROR_COPY_MODE_BACKGROUND,
> - errp);
> + base_read_only, errp);
> if (!job) {
> goto error_restore_flags;
> }
--
Best regards,
Vladimir
© 2016 - 2026 Red Hat, Inc.