* [RFC] block/mirror: fix file-system went to read-only after block-mirror
@ 2021-06-24 12:06 Jinhua Cao
2021-06-24 13:30 ` no-reply
2021-07-05 11:36 ` Vladimir Sementsov-Ogievskiy
0 siblings, 2 replies; 3+ messages in thread
From: Jinhua Cao @ 2021-06-24 12:06 UTC (permalink / raw
To: qemu-devel, qemu-block; +Cc: kwolf, eric.fangyi, vsementsov, jsnow, mreitz
1) Configure the VM disk as prdm.
...
<disk type='block' device='lun'>
<driver name='qemu' type='raw' cache='none'/>
<source dev='/dev/disk/by-id/scsi-368886030000000ca50c1cd1563996917' index='1'/>
<backingStore/>
<target dev='sdb' bus='scsi'/>
<alias name='scsi0-0-0-1'/>
<address type='drive' controller='0' bus='0' target='0' unit='1'/>
</disk>
...
Mount the disk in guest and keep the disk writing data continuously during block-mirror,
the file-system went to read-only after block-mirror.
2) This commit 6cdbceb12cf[mirror: Add filter-node-name to blockdev-mirror] introduces
mirror_top_bs which does not set default function for mirror_top_bs->drv->bdrv_co_ioctl.
3) The function bdrv_co_ioctl in block/io.c will be called during block-mirror, in this
function, the judgment is as follow:
---
if (!drv || (!drv->bdrv_aio_ioctl && !drv->bdrv_co_ioctl)) {
co.ret = -ENOTSUP;
goto out;
}
---
The mirror_top_bs does not set drv->bdrv_aio_ioctl or drv->bdrv_co_ioctl which result this
return -ENOTSUP. So the file-system went to read-only after block-mirror.
4) This patch set a default function for mirror_top_bs->drv->bdrv_aio_ioctl, fix this problem.
---
block/mirror.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/block/mirror.c b/block/mirror.c
index 019f6deaa5..63b788ec39 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -1480,6 +1480,12 @@ static int coroutine_fn bdrv_mirror_top_flush(BlockDriverState *bs)
return bdrv_co_flush(bs->backing->bs);
}
+static int coroutine_fn bdrv_mirror_top_ioctl(BlockDriverState *bs,
+ unsigned long int req, void *buf)
+{
+ return 0;
+}
+
static int coroutine_fn bdrv_mirror_top_pwrite_zeroes(BlockDriverState *bs,
int64_t offset, int bytes, BdrvRequestFlags flags)
{
@@ -1555,6 +1561,7 @@ static BlockDriver bdrv_mirror_top = {
.bdrv_co_pwrite_zeroes = bdrv_mirror_top_pwrite_zeroes,
.bdrv_co_pdiscard = bdrv_mirror_top_pdiscard,
.bdrv_co_flush = bdrv_mirror_top_flush,
+ .bdrv_co_ioctl = bdrv_mirror_top_ioctl,
.bdrv_refresh_filename = bdrv_mirror_top_refresh_filename,
.bdrv_child_perm = bdrv_mirror_top_child_perm,
--
2.27.0
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [RFC] block/mirror: fix file-system went to read-only after block-mirror
2021-06-24 12:06 [RFC] block/mirror: fix file-system went to read-only after block-mirror Jinhua Cao
@ 2021-06-24 13:30 ` no-reply
2021-07-05 11:36 ` Vladimir Sementsov-Ogievskiy
1 sibling, 0 replies; 3+ messages in thread
From: no-reply @ 2021-06-24 13:30 UTC (permalink / raw
To: caojinhua1
Cc: kwolf, vsementsov, qemu-block, eric.fangyi, qemu-devel, mreitz,
jsnow
Patchew URL: https://patchew.org/QEMU/20210624120635.54573-1-caojinhua1@huawei.com/
Hi,
This series seems to have some coding style problems. See output below for
more information:
Type: series
Message-id: 20210624120635.54573-1-caojinhua1@huawei.com
Subject: [RFC] block/mirror: fix file-system went to read-only after block-mirror
=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===
Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
* [new tag] patchew/20210624120635.54573-1-caojinhua1@huawei.com -> patchew/20210624120635.54573-1-caojinhua1@huawei.com
Switched to a new branch 'test'
6e2ba54 block/mirror: fix file-system went to read-only after block-mirror
=== OUTPUT BEGIN ===
ERROR: Missing Signed-off-by: line(s)
total: 1 errors, 0 warnings, 19 lines checked
Commit 6e2ba547f042 (block/mirror: fix file-system went to read-only after block-mirror) has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
=== OUTPUT END ===
Test command exited with code: 1
The full log is available at
http://patchew.org/logs/20210624120635.54573-1-caojinhua1@huawei.com/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [RFC] block/mirror: fix file-system went to read-only after block-mirror
2021-06-24 12:06 [RFC] block/mirror: fix file-system went to read-only after block-mirror Jinhua Cao
2021-06-24 13:30 ` no-reply
@ 2021-07-05 11:36 ` Vladimir Sementsov-Ogievskiy
1 sibling, 0 replies; 3+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2021-07-05 11:36 UTC (permalink / raw
To: Jinhua Cao, qemu-devel, qemu-block; +Cc: jsnow, kwolf, mreitz, eric.fangyi
24.06.2021 15:06, Jinhua Cao wrote:
> 1) Configure the VM disk as prdm.
> ...
> <disk type='block' device='lun'>
> <driver name='qemu' type='raw' cache='none'/>
> <source dev='/dev/disk/by-id/scsi-368886030000000ca50c1cd1563996917' index='1'/>
> <backingStore/>
> <target dev='sdb' bus='scsi'/>
> <alias name='scsi0-0-0-1'/>
> <address type='drive' controller='0' bus='0' target='0' unit='1'/>
> </disk>
> ...
> Mount the disk in guest and keep the disk writing data continuously during block-mirror,
> the file-system went to read-only after block-mirror.
>
> 2) This commit 6cdbceb12cf[mirror: Add filter-node-name to blockdev-mirror] introduces
> mirror_top_bs which does not set default function for mirror_top_bs->drv->bdrv_co_ioctl.
>
> 3) The function bdrv_co_ioctl in block/io.c will be called during block-mirror, in this
> function, the judgment is as follow:
> ---
> if (!drv || (!drv->bdrv_aio_ioctl && !drv->bdrv_co_ioctl)) {
> co.ret = -ENOTSUP;
> goto out;
> }
> ---
> The mirror_top_bs does not set drv->bdrv_aio_ioctl or drv->bdrv_co_ioctl which result this
> return -ENOTSUP. So the file-system went to read-only after block-mirror.
>
> 4) This patch set a default function for mirror_top_bs->drv->bdrv_aio_ioctl, fix this problem.
> ---
> block/mirror.c | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/block/mirror.c b/block/mirror.c
> index 019f6deaa5..63b788ec39 100644
> --- a/block/mirror.c
> +++ b/block/mirror.c
> @@ -1480,6 +1480,12 @@ static int coroutine_fn bdrv_mirror_top_flush(BlockDriverState *bs)
> return bdrv_co_flush(bs->backing->bs);
> }
>
> +static int coroutine_fn bdrv_mirror_top_ioctl(BlockDriverState *bs,
> + unsigned long int req, void *buf)
> +{
> + return 0;
> +}
I'm not very familiar with .bdrv_co_ioctl kitchen in Qemu, but intuitively this seems wrong to me: you do nothing and report success in this handler.
So, probably more correct would be at least call bdrv_co_ioctl() on bs->backing->bs, which is filtered child of mirror_top.
This raise another question: what exactly this ioctl does? If it changes the device like write operation, we should somehow handle it to update dirty bitmap, otherwise mirror will not work correctly. In this way, it seems correctly to not implement .bdrv_co_ioctl in mirror_top, and keep it returning ENOTSUP, as we really can't support it..
> +
> static int coroutine_fn bdrv_mirror_top_pwrite_zeroes(BlockDriverState *bs,
> int64_t offset, int bytes, BdrvRequestFlags flags)
> {
> @@ -1555,6 +1561,7 @@ static BlockDriver bdrv_mirror_top = {
> .bdrv_co_pwrite_zeroes = bdrv_mirror_top_pwrite_zeroes,
> .bdrv_co_pdiscard = bdrv_mirror_top_pdiscard,
> .bdrv_co_flush = bdrv_mirror_top_flush,
> + .bdrv_co_ioctl = bdrv_mirror_top_ioctl,
> .bdrv_refresh_filename = bdrv_mirror_top_refresh_filename,
> .bdrv_child_perm = bdrv_mirror_top_child_perm,
>
>
--
Best regards,
Vladimir
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2021-07-05 11:37 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2021-06-24 12:06 [RFC] block/mirror: fix file-system went to read-only after block-mirror Jinhua Cao
2021-06-24 13:30 ` no-reply
2021-07-05 11:36 ` 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.