[Qemu-devel] [FIX V2] repair snapshots

[Qemu-devel] [FIX V2] repair snapshots

[Benoît Canet]

This patch fix the snapshoting code.

When initially testing block filter snapshots patches and quorum I used a wrong
quorum command line which containned an extra top level "file."

As a result quorum was encapsulated in a top level bs.

This caused me to write and test against the wrong configuration and
break snapshot.

This patch implement Kevin sugestion for the fix.

I tested

snapshot of a Blockbackend: ex virtio0 ok
snapshot of a quorum file instantiated in the command line. ok
snapshot of a quorum file instantiated via blockdev_add by references ok
snapshot of a quorum file instianciated via blockdev without references ok

A comment update for quorum will be needed to avoid user to use the wrong
command line.

Benoît Canet (1):
  block: Fix device snapshots broken by the block filter snapshots
    patchset.

 block.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

-- 
1.8.3.2

[Qemu-devel] [FIX V2] block: Fix device snapshots broken by the block filter snapshots patchset.

[Benoît Canet]

Signed-off-by: Benoit Canet <benoit@irqsave.net>
---
 block.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/block.c b/block.c
index 70d7f35..d9553ba 100644
--- a/block.c
+++ b/block.c
@@ -5419,11 +5419,7 @@ bool bdrv_is_first_non_filter(BlockDriverState *candidate)
     QTAILQ_FOREACH(bs, &bdrv_states, device_list) {
         bool perm;
 
-        if (!bs->file) {
-            continue;
-        }
-
-        perm = bdrv_recurse_is_first_non_filter(bs->file, candidate);
+        perm = bdrv_recurse_is_first_non_filter(bs, candidate);
 
         /* candidate is the first non filter */
         if (perm) {
-- 
1.8.3.2

Re: [Qemu-devel] [FIX V2] block: Fix device snapshots broken by the block filter snapshots patchset.

[Kevin Wolf]

Am 13.02.2014 um 02:51 hat Benoît Canet geschrieben:
> Signed-off-by: Benoit Canet <benoit@irqsave.net>
> ---
>  block.c | 6 +-----
>  1 file changed, 1 insertion(+), 5 deletions(-)

Should be [PATCH v3] in the subject line.

> diff --git a/block.c b/block.c
> index 70d7f35..d9553ba 100644
> --- a/block.c
> +++ b/block.c
> @@ -5419,11 +5419,7 @@ bool bdrv_is_first_non_filter(BlockDriverState *candidate)
>      QTAILQ_FOREACH(bs, &bdrv_states, device_list) {
>          bool perm;
>  
> -        if (!bs->file) {
> -            continue;
> -        }
> -
> -        perm = bdrv_recurse_is_first_non_filter(bs->file, candidate);
> +        perm = bdrv_recurse_is_first_non_filter(bs, candidate);
>  
>          /* candidate is the first non filter */
>          if (perm) {

So we're back to my patch, just with different author? I thought you
said something was broken with it?

Kevin

Re: [Qemu-devel] [FIX V2] block: Fix device snapshots broken by the block filter snapshots patchset.

[Benoît Canet]

The Thursday 13 Feb 2014 à 09:33:52 (+0100), Kevin Wolf wrote :
> Am 13.02.2014 um 02:51 hat Benoît Canet geschrieben:
> > Signed-off-by: Benoit Canet <benoit@irqsave.net>
> > ---
> >  block.c | 6 +-----
> >  1 file changed, 1 insertion(+), 5 deletions(-)
> 
> Should be [PATCH v3] in the subject line.
> 
> > diff --git a/block.c b/block.c
> > index 70d7f35..d9553ba 100644
> > --- a/block.c
> > +++ b/block.c
> > @@ -5419,11 +5419,7 @@ bool bdrv_is_first_non_filter(BlockDriverState *candidate)
> >      QTAILQ_FOREACH(bs, &bdrv_states, device_list) {
> >          bool perm;
> >  
> > -        if (!bs->file) {
> > -            continue;
> > -        }
> > -
> > -        perm = bdrv_recurse_is_first_non_filter(bs->file, candidate);
> > +        perm = bdrv_recurse_is_first_non_filter(bs, candidate);
> >  
> >          /* candidate is the first non filter */
> >          if (perm) {
> 
> So we're back to my patch, just with different author? I thought you
> said something was broken with it?

Yes it's your patch.

My quorum command line had extra "file." in front of the quorum parameters.
(the user error you mentioned)
That's the reason I initially broke snapshot's because I was testing against
something like bs->file thinking it was normal to get top bs in bs->file.
And that's also the reason why I though your patch was broken.

The quorum series also need a repost to get rid of the "file." of the commit
comment.

Best regards

Benoît

> 
> Kevin
>