Re: Patch "block: drop ctx argument from bdrv_root_attach_child" causing crash

[Vladimir Sementsov-Ogievskiy]

24.05.2021 11:25, Vladimir Sementsov-Ogievskiy wrote:
> 24.05.2021 02:36, Programmingkid wrote:
> > After doing some git bisecting I found out a problem I was having
> > with sharing a host folder as an USB flash drive on the guest was
> > caused by this patch:
> >
> > 228ca37e12f97788e05bd0c92f89b3e5e4019607 is the first bad commit 
> > commit 228ca37e12f97788e05bd0c92f89b3e5e4019607 Author: Vladimir
> > Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Date:   Wed Apr 28
> > 18:17:34 2021 +0300
> >
> > block: drop ctx argument from bdrv_root_attach_child Passing parent
> > aio context is redundant, as child_class and parent opaque pointer
> > are enough to retrieve it. Drop the argument and use new 
> > bdrv_child_get_parent_aio_context() interface. Signed-off-by:
> > Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> 
> > Reviewed-by: Alberto Garcia <berto@igalia.com> Reviewed-by: Kevin
> > Wolf <kwolf@redhat.com> Message-Id:
> > <20210428151804.439460-7-vsementsov@virtuozzo.com> Signed-off-by:
> > Kevin Wolf <kwolf@redhat.com>
> >
> > block.c                   | 8 +++++--- block/block-backend.c     |
> > 4 ++-- blockjob.c                | 3 +-- include/block/block_int.h
> > | 1 - 4 files changed, 8 insertions(+), 8 deletions(-)
> >
> >
> > With this patch committed the crash can be reproduced like this:
> >
> > qemu-system-ppc -usb -device usb-storage,drive=fat16 -drive
> > file=fat:rw:fat-type=16:"<path of a host
> > folder>",id=fat16,format=raw,if=none
> >
> > On my system is causes a segment fault.
> >
> > Thank you.
>
> Oops, my fault, thanks for reporting. The problem is that previous
> commit
>
>
> commit 3ca1f3225727419ba573673b744edac10904276f Author: Vladimir
> Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Date:   Wed Apr 28
> 18:17:33 2021 +0300
>
> block: BdrvChildClass: add .get_parent_aio_context handler Add new
> handler to get aio context and implement it in all child classes. Add
> corresponding public interface to be used soon.
>
>
> didn't add .get_parent_aio_context to child_vvfat_qcow
>
>
> I'll send a patch


Looking at

const BdrvChildClass child_of_bds = {
    .parent_is_bds   = true,
    .get_parent_desc = bdrv_child_get_parent_desc,
    .inherit_options = bdrv_inherited_options,
    .drained_begin   = bdrv_child_cb_drained_begin,
    .drained_poll    = bdrv_child_cb_drained_poll,
    .drained_end     = bdrv_child_cb_drained_end,
    .attach          = bdrv_child_cb_attach,
    .detach          = bdrv_child_cb_detach,
    .inactivate      = bdrv_child_cb_inactivate,
    .can_set_aio_ctx = bdrv_child_cb_can_set_aio_ctx,
    .set_aio_ctx     = bdrv_child_cb_set_aio_ctx,
    .update_filename = bdrv_child_cb_update_filename,
    .get_parent_aio_context = bdrv_child_cb_get_parent_aio_context,
};


and at

static const BdrvChildClass child_vvfat_qcow = {
    .parent_is_bds      = true,
    .inherit_options    = vvfat_qcow_options,
};


I can assume, that there are also other scenarios, where vvfat will not work 
properly...

--
Best regards,
Vladimir