Re: [PATCH 1/2] block: make bdrv_drop_intermediate() less wrong

2020-11-24 Thread Vladimir Sementsov-Ogievskiy
23.11.2020 23:12, Vladimir Sementsov-Ogievskiy wrote: First, permission update loop tries to do iterations transactionally, but the whole update is not transactional: nobody roll-back successful loop iterations when some iteration fails. Second, in the iteration we have nested permission

[PATCH 1/2] block: make bdrv_drop_intermediate() less wrong

2020-11-23 Thread Vladimir Sementsov-Ogievskiy
First, permission update loop tries to do iterations transactionally, but the whole update is not transactional: nobody roll-back successful loop iterations when some iteration fails. Second, in the iteration we have nested permission update: c->klass->update_filename may point to

Re: [PATCH 1/2] block: make bdrv_drop_intermediate() less wrong

2020-11-06 Thread Vladimir Sementsov-Ogievskiy
05.11.2020 18:14, Alberto Garcia wrote: On Sat 31 Oct 2020 01:35:01 PM CET, Vladimir Sementsov-Ogievskiy wrote: -QLIST_FOREACH_SAFE(c, >parents, next_parent, next) { /* ... */ +QLIST_FOREACH_SAFE(c, >parents, next_parent, next) { I also wonder, is top->parents and base->parents

Re: [PATCH 1/2] block: make bdrv_drop_intermediate() less wrong

2020-11-05 Thread Alberto Garcia
On Sat 31 Oct 2020 01:35:01 PM CET, Vladimir Sementsov-Ogievskiy wrote: > -QLIST_FOREACH_SAFE(c, >parents, next_parent, next) { /* ... */ > +QLIST_FOREACH_SAFE(c, >parents, next_parent, next) { I also wonder, is top->parents and base->parents guaranteed to be the same list in this case?

Re: [PATCH 1/2] block: make bdrv_drop_intermediate() less wrong

2020-11-05 Thread Alberto Garcia
On Sat 31 Oct 2020 01:35:01 PM CET, Vladimir Sementsov-Ogievskiy wrote: > @@ -4958,36 +4958,30 @@ int bdrv_drop_intermediate(BlockDriverState *top, > BlockDriverState *base, > backing_file_str = base->filename; > } > > -QLIST_FOREACH_SAFE(c, >parents, next_parent, next) { > -

Re: [PATCH 1/2] block: make bdrv_drop_intermediate() less wrong

2020-11-05 Thread Max Reitz
On 05.11.20 14:22, Max Reitz wrote: On 31.10.20 13:35, Vladimir Sementsov-Ogievskiy wrote: First, permission update loop tries to do iterations transactionally, but the whole update is not transactional: nobody roll-back successful loop iterations when some iteration fails. [...] And

Re: [PATCH 1/2] block: make bdrv_drop_intermediate() less wrong

2020-11-05 Thread Max Reitz
On 31.10.20 13:35, Vladimir Sementsov-Ogievskiy wrote: First, permission update loop tries to do iterations transactionally, but the whole update is not transactional: nobody roll-back successful loop iterations when some iteration fails. Indeed. Another thing that should be noted:

[PATCH 1/2] block: make bdrv_drop_intermediate() less wrong

2020-10-31 Thread Vladimir Sementsov-Ogievskiy
First, permission update loop tries to do iterations transactionally, but the whole update is not transactional: nobody roll-back successful loop iterations when some iteration fails. Second, in the iteration we have nested permission update: c->klass->update_filename may point to