* [Qemu-devel] [PATCH 0/2] Clean up the remainders of bdrv_swap
@ 2015-12-17 5:09 Fam Zheng
2015-12-17 5:09 ` [Qemu-devel] [PATCH 1/2] block: Remove prototype of bdrv_swap from header Fam Zheng
2015-12-17 5:09 ` [Qemu-devel] [PATCH 2/2] iotests: Don't mention bdrv_swap in comments Fam Zheng
0 siblings, 2 replies; 7+ messages in thread
From: Fam Zheng @ 2015-12-17 5:09 UTC (permalink / raw)
To: qemu-devel; +Cc: Kevin Wolf, qemu-block
Fam Zheng (2):
block: Remove prototype of bdrv_swap from header
iotests: Don't mention bdrv_swap in comments
include/block/block.h | 1 -
tests/qemu-iotests/094 | 4 +---
2 files changed, 1 insertion(+), 4 deletions(-)
--
2.4.3
^ permalink raw reply [flat|nested] 7+ messages in thread
* [Qemu-devel] [PATCH 1/2] block: Remove prototype of bdrv_swap from header
2015-12-17 5:09 [Qemu-devel] [PATCH 0/2] Clean up the remainders of bdrv_swap Fam Zheng
@ 2015-12-17 5:09 ` Fam Zheng
2015-12-17 5:09 ` [Qemu-devel] [PATCH 2/2] iotests: Don't mention bdrv_swap in comments Fam Zheng
1 sibling, 0 replies; 7+ messages in thread
From: Fam Zheng @ 2015-12-17 5:09 UTC (permalink / raw)
To: qemu-devel; +Cc: Kevin Wolf, qemu-block
The function has gone.
Signed-off-by: Fam Zheng <famz@redhat.com>
---
include/block/block.h | 1 -
1 file changed, 1 deletion(-)
diff --git a/include/block/block.h b/include/block/block.h
index 3477328..028f2fb 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -197,7 +197,6 @@ int bdrv_create_file(const char *filename, QemuOpts *opts, Error **errp);
BlockDriverState *bdrv_new_root(void);
BlockDriverState *bdrv_new(void);
void bdrv_make_anon(BlockDriverState *bs);
-void bdrv_swap(BlockDriverState *bs_new, BlockDriverState *bs_old);
void bdrv_append(BlockDriverState *bs_new, BlockDriverState *bs_top);
void bdrv_replace_in_backing_chain(BlockDriverState *old,
BlockDriverState *new);
--
2.4.3
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [Qemu-devel] [PATCH 2/2] iotests: Don't mention bdrv_swap in comments
2015-12-17 5:09 [Qemu-devel] [PATCH 0/2] Clean up the remainders of bdrv_swap Fam Zheng
2015-12-17 5:09 ` [Qemu-devel] [PATCH 1/2] block: Remove prototype of bdrv_swap from header Fam Zheng
@ 2015-12-17 5:09 ` Fam Zheng
2015-12-17 8:21 ` Kevin Wolf
1 sibling, 1 reply; 7+ messages in thread
From: Fam Zheng @ 2015-12-17 5:09 UTC (permalink / raw)
To: qemu-devel; +Cc: Kevin Wolf, qemu-block
Signed-off-by: Fam Zheng <famz@redhat.com>
---
tests/qemu-iotests/094 | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/tests/qemu-iotests/094 b/tests/qemu-iotests/094
index 27a2be2..d30c78d 100755
--- a/tests/qemu-iotests/094
+++ b/tests/qemu-iotests/094
@@ -1,6 +1,6 @@
#!/bin/bash
#
-# Test case for drive-mirror to NBD (especially bdrv_swap() on NBD BDS)
+# Test case for drive-mirror to NBD
#
# Copyright (C) 2015 Red Hat, Inc.
#
@@ -50,8 +50,6 @@ _send_qemu_cmd $QEMU_HANDLE \
"{'execute': 'qmp_capabilities'}" \
'return'
-# 'format': 'nbd' is not actually "correct", but this is probably the only way
-# to test bdrv_swap() on an NBD BDS
_send_qemu_cmd $QEMU_HANDLE \
"{'execute': 'drive-mirror',
'arguments': {'device': 'src',
--
2.4.3
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH 2/2] iotests: Don't mention bdrv_swap in comments
2015-12-17 5:09 ` [Qemu-devel] [PATCH 2/2] iotests: Don't mention bdrv_swap in comments Fam Zheng
@ 2015-12-17 8:21 ` Kevin Wolf
2015-12-17 12:44 ` Fam Zheng
2015-12-17 19:07 ` Paolo Bonzini
0 siblings, 2 replies; 7+ messages in thread
From: Kevin Wolf @ 2015-12-17 8:21 UTC (permalink / raw)
To: Fam Zheng; +Cc: qemu-devel, qemu-block
Am 17.12.2015 um 06:09 hat Fam Zheng geschrieben:
> Signed-off-by: Fam Zheng <famz@redhat.com>
> ---
> tests/qemu-iotests/094 | 4 +---
> 1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/tests/qemu-iotests/094 b/tests/qemu-iotests/094
> index 27a2be2..d30c78d 100755
> --- a/tests/qemu-iotests/094
> +++ b/tests/qemu-iotests/094
> @@ -1,6 +1,6 @@
> #!/bin/bash
> #
> -# Test case for drive-mirror to NBD (especially bdrv_swap() on NBD BDS)
> +# Test case for drive-mirror to NBD
> #
> # Copyright (C) 2015 Red Hat, Inc.
> #
> @@ -50,8 +50,6 @@ _send_qemu_cmd $QEMU_HANDLE \
> "{'execute': 'qmp_capabilities'}" \
> 'return'
>
> -# 'format': 'nbd' is not actually "correct", but this is probably the only way
> -# to test bdrv_swap() on an NBD BDS
> _send_qemu_cmd $QEMU_HANDLE \
> "{'execute': 'drive-mirror',
> 'arguments': {'device': 'src',
Just completely removing the comment doesn't seem right to me if we
leave the "bad" option around.
The test seems to be a regression test for what was fixed in commit
f53a829, i.e. a direct effect of bdrv_swap(). This effect can't exist
any more, so we would keep the test just for some additional coverage
for NBD. Do we still need 'format': 'nbd' (if so, with a comment why we
do that) or should we make it 'raw' now?
Kevin
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH 2/2] iotests: Don't mention bdrv_swap in comments
2015-12-17 8:21 ` Kevin Wolf
@ 2015-12-17 12:44 ` Fam Zheng
2015-12-17 12:48 ` Kevin Wolf
2015-12-17 19:07 ` Paolo Bonzini
1 sibling, 1 reply; 7+ messages in thread
From: Fam Zheng @ 2015-12-17 12:44 UTC (permalink / raw)
To: Kevin Wolf; +Cc: qemu-devel, qemu-block
On Thu, 12/17 09:21, Kevin Wolf wrote:
> Am 17.12.2015 um 06:09 hat Fam Zheng geschrieben:
> > Signed-off-by: Fam Zheng <famz@redhat.com>
> > ---
> > tests/qemu-iotests/094 | 4 +---
> > 1 file changed, 1 insertion(+), 3 deletions(-)
> >
> > diff --git a/tests/qemu-iotests/094 b/tests/qemu-iotests/094
> > index 27a2be2..d30c78d 100755
> > --- a/tests/qemu-iotests/094
> > +++ b/tests/qemu-iotests/094
> > @@ -1,6 +1,6 @@
> > #!/bin/bash
> > #
> > -# Test case for drive-mirror to NBD (especially bdrv_swap() on NBD BDS)
> > +# Test case for drive-mirror to NBD
> > #
> > # Copyright (C) 2015 Red Hat, Inc.
> > #
> > @@ -50,8 +50,6 @@ _send_qemu_cmd $QEMU_HANDLE \
> > "{'execute': 'qmp_capabilities'}" \
> > 'return'
> >
> > -# 'format': 'nbd' is not actually "correct", but this is probably the only way
> > -# to test bdrv_swap() on an NBD BDS
> > _send_qemu_cmd $QEMU_HANDLE \
> > "{'execute': 'drive-mirror',
> > 'arguments': {'device': 'src',
>
> Just completely removing the comment doesn't seem right to me if we
> leave the "bad" option around.
>
> The test seems to be a regression test for what was fixed in commit
> f53a829, i.e. a direct effect of bdrv_swap(). This effect can't exist
> any more, so we would keep the test just for some additional coverage
> for NBD. Do we still need 'format': 'nbd' (if so, with a comment why we
> do that) or should we make it 'raw' now?
>
I prefer keeping it as is, because making it raw would spoil the purpose as a
regression test (the fix was on nbd).
Can we re-comment it as below?
# 'format': 'nbd' is not actually "correct", but this was the only way to
# test the bug fixed in commit f53a829. Though the bug's related code
# bdrv_swap() was replaced later, let's make sure we don't fall in the same
# pit again.
Fam
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH 2/2] iotests: Don't mention bdrv_swap in comments
2015-12-17 12:44 ` Fam Zheng
@ 2015-12-17 12:48 ` Kevin Wolf
0 siblings, 0 replies; 7+ messages in thread
From: Kevin Wolf @ 2015-12-17 12:48 UTC (permalink / raw)
To: Fam Zheng; +Cc: qemu-devel, qemu-block
Am 17.12.2015 um 13:44 hat Fam Zheng geschrieben:
> On Thu, 12/17 09:21, Kevin Wolf wrote:
> > Am 17.12.2015 um 06:09 hat Fam Zheng geschrieben:
> > > Signed-off-by: Fam Zheng <famz@redhat.com>
> > > ---
> > > tests/qemu-iotests/094 | 4 +---
> > > 1 file changed, 1 insertion(+), 3 deletions(-)
> > >
> > > diff --git a/tests/qemu-iotests/094 b/tests/qemu-iotests/094
> > > index 27a2be2..d30c78d 100755
> > > --- a/tests/qemu-iotests/094
> > > +++ b/tests/qemu-iotests/094
> > > @@ -1,6 +1,6 @@
> > > #!/bin/bash
> > > #
> > > -# Test case for drive-mirror to NBD (especially bdrv_swap() on NBD BDS)
> > > +# Test case for drive-mirror to NBD
> > > #
> > > # Copyright (C) 2015 Red Hat, Inc.
> > > #
> > > @@ -50,8 +50,6 @@ _send_qemu_cmd $QEMU_HANDLE \
> > > "{'execute': 'qmp_capabilities'}" \
> > > 'return'
> > >
> > > -# 'format': 'nbd' is not actually "correct", but this is probably the only way
> > > -# to test bdrv_swap() on an NBD BDS
> > > _send_qemu_cmd $QEMU_HANDLE \
> > > "{'execute': 'drive-mirror',
> > > 'arguments': {'device': 'src',
> >
> > Just completely removing the comment doesn't seem right to me if we
> > leave the "bad" option around.
> >
> > The test seems to be a regression test for what was fixed in commit
> > f53a829, i.e. a direct effect of bdrv_swap(). This effect can't exist
> > any more, so we would keep the test just for some additional coverage
> > for NBD. Do we still need 'format': 'nbd' (if so, with a comment why we
> > do that) or should we make it 'raw' now?
> >
>
> I prefer keeping it as is, because making it raw would spoil the purpose as a
> regression test (the fix was on nbd).
>
> Can we re-comment it as below?
>
> # 'format': 'nbd' is not actually "correct", but this was the only way to
> # test the bug fixed in commit f53a829. Though the bug's related code
> # bdrv_swap() was replaced later, let's make sure we don't fall in the same
> # pit again.
That's fine with me.
Kevin
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH 2/2] iotests: Don't mention bdrv_swap in comments
2015-12-17 8:21 ` Kevin Wolf
2015-12-17 12:44 ` Fam Zheng
@ 2015-12-17 19:07 ` Paolo Bonzini
1 sibling, 0 replies; 7+ messages in thread
From: Paolo Bonzini @ 2015-12-17 19:07 UTC (permalink / raw)
To: Kevin Wolf, Fam Zheng; +Cc: qemu-devel, qemu-block
On 17/12/2015 09:21, Kevin Wolf wrote:
>> > -# 'format': 'nbd' is not actually "correct", but this is probably the only way
>> > -# to test bdrv_swap() on an NBD BDS
>> > _send_qemu_cmd $QEMU_HANDLE \
>> > "{'execute': 'drive-mirror',
>> > 'arguments': {'device': 'src',
> Just completely removing the comment doesn't seem right to me if we
> leave the "bad" option around.
>
> The test seems to be a regression test for what was fixed in commit
> f53a829, i.e. a direct effect of bdrv_swap(). This effect can't exist
> any more, so we would keep the test just for some additional coverage
> for NBD. Do we still need 'format': 'nbd' (if so, with a comment why we
> do that) or should we make it 'raw' now?
Some coverage of no-format BDSes is nice to have, since raw is sometimes
measurably slower than no format at all.
Paolo
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2015-12-17 19:07 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-12-17 5:09 [Qemu-devel] [PATCH 0/2] Clean up the remainders of bdrv_swap Fam Zheng
2015-12-17 5:09 ` [Qemu-devel] [PATCH 1/2] block: Remove prototype of bdrv_swap from header Fam Zheng
2015-12-17 5:09 ` [Qemu-devel] [PATCH 2/2] iotests: Don't mention bdrv_swap in comments Fam Zheng
2015-12-17 8:21 ` Kevin Wolf
2015-12-17 12:44 ` Fam Zheng
2015-12-17 12:48 ` Kevin Wolf
2015-12-17 19:07 ` Paolo Bonzini
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.