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 update:

[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 bdrv_child_cb_up

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, &top->parents, next_parent, next) { /* ... */ +QLIST_FOREACH_SAFE(c, &base->parents, next_parent, next) { I also wonder, is top->parents and base-

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, &top->parents, next_parent, next) { /* ... */ > +QLIST_FOREACH_SAFE(c, &base->parents, next_parent, next) { I also wonder, is top->parents and base->parents guaranteed to be the same list in

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, &top->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 besides

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: bdrv_check

[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 bdrv_child_cb_up