Re: Host folder sharing via USB issue

[Vladimir Sementsov-Ogievskiy]

14.07.2021 00:04, Programmingkid wrote:
> Hi I have noticed that host folder sharing via USB has recently stopped 
> working. After doing some git bisecting I found this as the patch that seems to 
> be the issue:
>
> 25f78d9e2de528473d52acfcf7acdfb64e3453d4 is the first bad commit
> commit 25f78d9e2de528473d52acfcf7acdfb64e3453d4
> Author: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> Date:   Thu Jun 10 15:05:34 2021 +0300
>
>      block: move supports_backing check to bdrv_set_file_or_backing_noperm()
>      
>      Move supports_backing check of bdrv_reopen_parse_backing to called
>      (through bdrv_set_backing_noperm()) bdrv_set_file_or_backing_noperm()
>      function. The check applies to general case, so it's appropriate for
>      bdrv_set_file_or_backing_noperm().
>      
>      We have to declare backing support for two test drivers, otherwise new
>      check fails.
>      
>      Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
>      Message-Id: <20210610120537.196183-7-vsementsov@virtuozzo.com>
>      Signed-off-by: Kevin Wolf <kwolf@redhat.com>
>
>   block.c                          | 29 +++++++++++++++--------------
>   tests/unit/test-bdrv-drain.c     |  1 +
>   tests/unit/test-bdrv-graph-mod.c |  1 +
>   3 files changed, 17 insertions(+), 14 deletions(-)
>
> To reproduce this issue run this command:
> qemu-system-i386 -usb -device usb-storage,drive=fat16 -drive 
> file=fat:rw:fat-type=16:"<path to host folder>",id=fat16,format=raw,if=none
>
> Results:
> Unexpected error in bdrv_set_file_or_backing_noperm() at ../block.c:3159:
> qemu-system-i386: -drive file=fat:rw:fat-type=16:<host folder 
> path>,id=fat16,format=raw,if=none: Driver 'vvfat' of node '#block057' does not 
> support backing files
> Abort trap: 6
>
> Expected results:
> QEMU start running normally.
>
> Please let me know if you need more information.
>
> Thank you.

Hi!

Look at bt:

#0  0x00007fd34f6939d5 in raise () at /lib64/libc.so.6
#1  0x00007fd34f67c8a4 in abort () at /lib64/libc.so.6
#2  0x000055e446d967aa in error_handle_fatal (errp=0x55e447652680 
<error_abort>, err=0x55e448d17a20) at ../util/error.c:40
#3  0x000055e446d968da in error_setv
    (errp=0x55e447652680 <error_abort>, src=0x55e446f8755b "../block.c", line=3158, func=0x55e446f89c20 
<__func__.64> "bdrv_set_file_or_backing_noperm", err_class=ERROR_CLASS_GENERIC_ERROR, fmt=0x55e446f88458 
"Driver '%s' of node '%s' does not support backing files", ap=0x7ffc31aba090, suffix=0x0) at ../util/error.c:73
#4  0x000055e446d96ab8 in error_setg_internal
    (errp=0x55e447652680 <error_abort>, src=0x55e446f8755b "../block.c", line=3158, func=0x55e446f89c20 
<__func__.64> "bdrv_set_file_or_backing_noperm", fmt=0x55e446f88458 "Driver '%s' of node '%s' does 
not support backing files") at ../util/error.c:97
#5  0x000055e446c411cf in bdrv_set_file_or_backing_noperm (parent_bs=0x55e448ceebe0, 
child_bs=0x55e448d21e40, is_backing=true, tran=0x55e448d16c20, errp=0x55e447652680 
<error_abort>) at ../block.c:3158
#6  0x000055e446c41377 in bdrv_set_backing_noperm (bs=0x55e448ceebe0, 
backing_hd=0x55e448d21e40, tran=0x55e448d16c20, errp=0x55e447652680 
<error_abort>) at ../block.c:3218
#7  0x000055e446c413ae in bdrv_set_backing_hd (bs=0x55e448ceebe0, 
backing_hd=0x55e448d21e40, errp=0x55e447652680 <error_abort>) at ../block.c:3227
#8  0x000055e446c1bd37 in enable_write_target (bs=0x55e448ceebe0, 
errp=0x7ffc31aba360) at ../block/vvfat.c:3191
#9  0x000055e446c16fe8 in vvfat_open (bs=0x55e448ceebe0, 
options=0x55e448cf4330, flags=155650, errp=0x7ffc31aba360) at 
../block/vvfat.c:1236
#10 0x000055e446c3df37 in bdrv_open_driver (bs=0x55e448ceebe0, drv=0x55e4475e9760 
<bdrv_vvfat>, node_name=0x0, options=0x55e448cf4330, open_flags=155650, 
errp=0x7ffc31aba470) at ../block.c:1550
#11 0x000055e446c3e8ee in bdrv_open_common (bs=0x55e448ceebe0, file=0x0, 
options=0x55e448cf4330, errp=0x7ffc31aba470) at ../block.c:1827
#12 0x000055e446c427b6 in bdrv_open_inherit
    (filename=0x55e448ce4300 "fat:rw:fat-type=16:/tmp", reference=0x0, 
options=0x55e448cf4330, flags=40962, parent=0x55e448ce75a0, child_class=0x55e4475099c0 
<child_of_bds>, child_role=19, errp=0x7ffc31aba670)
    at ../block.c:3747
#13 0x000055e446c419f5 in bdrv_open_child_bs
    (filename=0x55e448ce4300 "fat:rw:fat-type=16:/tmp", options=0x55e448cec9f0, 
bdref_key=0x55e446f884d0 "file", parent=0x55e448ce75a0, child_class=0x55e4475099c0 
<child_of_bds>, child_role=19, allow_none=true, errp=0x7ffc31aba670) at ../block.c:3387
#14 0x000055e446c42568 in bdrv_open_inherit
    (filename=0x55e448ce4300 "fat:rw:fat-type=16:/tmp", reference=0x0, 
options=0x55e448cec9f0, flags=8194, parent=0x0, child_class=0x0, child_role=0, 
errp=0x55e447652688 <error_fatal>) at ../block.c:3694
#15 0x000055e446c42cf6 in bdrv_open (filename=0x55e448ce4300 
"fat:rw:fat-type=16:/tmp", reference=0x0, options=0x55e448ce4f00, flags=0, 
errp=0x55e447652688 <error_fatal>) at ../block.c:3840
#16 0x000055e446c5fcaf in blk_new_open (filename=0x55e448ce4300 
"fat:rw:fat-type=16:/tmp", reference=0x0, options=0x55e448ce4f00, flags=0, 
errp=0x55e447652688 <error_fatal>) at ../block/block-backend.c:435
#17 0x000055e446beca1d in blockdev_init (file=0x55e448ce4300 
"fat:rw:fat-type=16:/tmp", bs_opts=0x55e448ce4f00, errp=0x55e447652688 
<error_fatal>) at ../blockdev.c:609
#18 0x000055e446bed900 in drive_new (all_opts=0x55e448ac4850, 
block_default_type=IF_IDE, errp=0x55e447652688 <error_fatal>) at 
../blockdev.c:993
#19 0x000055e446abd69e in drive_init_func (opaque=0x55e448bd4d40, 
opts=0x55e448ac4850, errp=0x55e447652688 <error_fatal>) at ../softmmu/vl.c:613
#20 0x000055e446da26d9 in qemu_opts_foreach (list=0x55e4475e8960 <qemu_drive_opts>, 
func=0x55e446abd66a <drive_init_func>, opaque=0x55e448bd4d40, errp=0x55e447652688 
<error_fatal>) at ../util/qemu-option.c:1137
#21 0x000055e446abd8e7 in configure_blockdev (bdo_queue=0x55e44757a2a0 
<bdo_queue>, machine_class=0x55e448bd4c90, snapshot=0) at ../softmmu/vl.c:672
#22 0x000055e446ac1b75 in qemu_create_early_backends () at ../softmmu/vl.c:1925
#23 0x000055e446ac5c1f in qemu_init (argc=6, argv=0x7ffc31abae58, 
envp=0x7ffc31abae90) at ../softmmu/vl.c:3636
#24 0x000055e4466b3c71 in main (argc=6, argv=0x7ffc31abae58, 
envp=0x7ffc31abae90) at ../softmmu/main.c:49
(gdb) fr 5
#5  0x000055e446c411cf in bdrv_set_file_or_backing_noperm (parent_bs=0x55e448ceebe0, 
child_bs=0x55e448d21e40, is_backing=true, tran=0x55e448d16c20, errp=0x55e447652680 
<error_abort>) at ../block.c:3158
3158            error_setg(errp, "Driver '%s' of node '%s' does not support backing 
"


(gdb) fr 7
#7  0x000055e446c413ae in bdrv_set_backing_hd (bs=0x55e448ceebe0, 
backing_hd=0x55e448d21e40, errp=0x55e447652680 <error_abort>) at ../block.c:3227
3227        ret = bdrv_set_backing_noperm(bs, backing_hd, tran, errp);
(gdb) p bs->drv
$1 = (BlockDriver *) 0x55e4475e9760 <bdrv_vvfat>


Hmm. Really vvfat doesn't seem to support backing files. But it does create a 
node with vvfat_write_target driver and set it as backing of itself (of vvfat 
node I mean).. And I don't see, where is this backing used.


Looking at git history, I see commit a8a4d15c1c34d of 2017, which describes 
that this fake backing file doesn't work anyway.

So, if just remove this backing file, bug doesn't reproduce. But I think better 
fix is to deprecate vvfat (recommend use virtio-fs instead for sharing) and 
drop it after deprecation period.



Use it with no guarantee:) :

diff --git a/block/vvfat.c b/block/vvfat.c
index ae9d387da7..34bf1e3a86 100644
--- a/block/vvfat.c
+++ b/block/vvfat.c
@@ -3098,26 +3098,6 @@ static int coroutine_fn 
vvfat_co_block_status(BlockDriverState *bs,
     return BDRV_BLOCK_DATA;
 }
 
-static int coroutine_fn
-write_target_commit(BlockDriverState *bs, uint64_t offset, uint64_t bytes,
-                    QEMUIOVector *qiov, int flags)
-{
-    int ret;
-
-    BDRVVVFATState* s = *((BDRVVVFATState**) bs->opaque);
-    qemu_co_mutex_lock(&s->lock);
-    ret = try_commit(s);
-    qemu_co_mutex_unlock(&s->lock);
-
-    return ret;
-}
-
-static BlockDriver vvfat_write_target = {
-    .format_name        = "vvfat_write_target",
-    .instance_size      = sizeof(void*),
-    .bdrv_co_pwritev    = write_target_commit,
-};
-
 static void vvfat_qcow_options(BdrvChildRole role, bool parent_is_format,
                                int *child_flags, QDict *child_options,
                                int parent_flags, QDict *parent_options)
@@ -3133,7 +3113,6 @@ static int enable_write_target(BlockDriverState *bs, 
Error **errp)
 {
     BDRVVVFATState *s = bs->opaque;
     BlockDriver *bdrv_qcow = NULL;
-    BlockDriverState *backing;
     QemuOpts *opts = NULL;
     int ret;
     int size = sector2cluster(s, s->sector_count);
@@ -3184,13 +3163,6 @@ static int enable_write_target(BlockDriverState *bs, 
Error **errp)
     unlink(s->qcow_filename);
 #endif
 
-    backing = bdrv_new_open_driver(&vvfat_write_target, NULL, BDRV_O_ALLOW_RDWR,
-                                   &error_abort);
-    *(void**) backing->opaque = s;
-
-    bdrv_set_backing_hd(s->bs, backing, &error_abort);
-    bdrv_unref(backing);
-
     return 0;
 
 err:
@@ -3205,17 +3177,10 @@ static void vvfat_child_perm(BlockDriverState *bs, 
BdrvChild *c,
                              uint64_t perm, uint64_t shared,
                              uint64_t *nperm, uint64_t *nshared)
 {
-    if (role & BDRV_CHILD_DATA) {
-        /* This is a private node, nobody should try to attach to it */
-        *nperm = BLK_PERM_CONSISTENT_READ | BLK_PERM_WRITE;
-        *nshared = BLK_PERM_WRITE_UNCHANGED;
-    } else {
-        assert(role & BDRV_CHILD_COW);
-        /* The backing file is there so 'commit' can use it. vvfat doesn't
-         * access it in any way. */
-        *nperm = 0;
-        *nshared = BLK_PERM_ALL;
-    }
+    assert(role & BDRV_CHILD_DATA);
+    /* This is a private node, nobody should try to attach to it */
+    *nperm = BLK_PERM_CONSISTENT_READ | BLK_PERM_WRITE;
+    *nshared = BLK_PERM_WRITE_UNCHANGED;
 }
 
 static void vvfat_close(BlockDriverState *bs)





--
Best regards,
Vladimir