We all know how the thing works and operates it does t take a genius to
know what's working and what's not.. I mean. I.notsaying take out tge
things that made us like to use it.. but I mean command should be
understood logic should be what we use as a am
Standard.. and trust me depending on where your at with structure and
scripts the logic can even be ...codeing..

On Thu, Jan 9, 2025, 4:55 AM Daniel Wielandt <[email protected]> wrote:

> I feel like your too focused on new problems when you  gave up repairing
> to old os.. be ause at one time it was real slick.. now.theres parts that
> are problems that can be eliminated easy and provide  a less bloated
> function with simple code deletion.. fix the original is down its it's
> primary parts
> .then u can add your fancy automation and setting get real com0lucated
>
> On Thu, Jan 9, 2025, 4:52 AM Daniel Wielandt <[email protected]>
> wrote:
>
>> Suggestions hold pattern.. programmers obviously understand control
>> logic. Systems built with operational flaws. Will freeze up when u start
>> repairing it while it's running.
>>
>> On Thu, Jan 9, 2025, 4:48 AM Kevin Wolf <[email protected]> wrote:
>>
>>> Am 08.01.2025 um 13:46 hat Fiona Ebner geschrieben:
>>> > Setting blk->root is a graph change operation and thus needs to be
>>> > protected by the block graph write lock in blk_remove_bs(). The
>>> > assignment to blk->root in blk_insert_bs() is already protected by
>>> > the block graph write lock.
>>>
>>> Hm, if that's the case, then we should also enforce this in the
>>> declaration of BlockBackend:
>>>
>>>     BdrvChild * GRAPH_RDLOCK_PTR root;
>>>
>>> However, this results in more compiler failures that we need to fix. You
>>> caught the only remaining writer, but the lock is only fully effective
>>> if all readers take it, too.
>>>
>>> > In particular, the graph read lock in blk_co_do_flush() could
>>> > previously not ensure that blk_bs(blk) would always return the same
>>> > value during the locked section, which could lead to a segfault [0] in
>>> > combination with migration [1].
>>> >
>>> > From the user-provided backtraces in the forum thread [1], it seems
>>> > like blk_co_do_flush() managed to get past the
>>> > blk_co_is_available(blk) check, meaning that blk_bs(blk) returned a
>>> > non-NULL value during the check, but then, when calling
>>> > bdrv_co_flush(), blk_bs(blk) returned NULL.
>>> >
>>> > [0]:
>>> >
>>> > > 0  bdrv_primary_child (bs=bs@entry=0x0) at ../block.c:8287
>>> > > 1  bdrv_co_flush (bs=0x0) at ../block/io.c:2948
>>> > > 2  bdrv_co_flush_entry (opaque=0x7a610affae90) at
>>> block/block-gen.c:901
>>> >
>>> > [1]: https://forum.proxmox.com/threads/158072
>>> >
>>> > Cc: [email protected]
>>> > Signed-off-by: Fiona Ebner <[email protected]>
>>> > ---
>>> >  block/block-backend.c | 2 +-
>>> >  1 file changed, 1 insertion(+), 1 deletion(-)
>>> >
>>> > diff --git a/block/block-backend.c b/block/block-backend.c
>>> > index c93a7525ad..9678615318 100644
>>> > --- a/block/block-backend.c
>>> > +++ b/block/block-backend.c
>>> > @@ -887,9 +887,9 @@ void blk_remove_bs(BlockBackend *blk)
>>> >       */
>>> >      blk_drain(blk);
>>> >      root = blk->root;
>>> > -    blk->root = NULL;
>>> >
>>> >      bdrv_graph_wrlock();
>>> > +    blk->root = NULL;
>>> >      bdrv_root_unref_child(root);
>>> >      bdrv_graph_wrunlock();
>>> >  }
>>>
>>> I think the 'root = blk->root' needs to be inside the locked section,
>>> too. Otherwise blk->root could change during bdrv_graph_wrlock() (which
>>> has a nested event loop) and root would be stale. I assume clang would
>>> complain about this with the added GRAPH_RDLOCK_PTR.
>>>
>>> Kevin
>>>
>>>
>>>

Reply via email to