[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: Host folder sharing via USB issue
|
From: |
Programmingkid |
|
Subject: |
Re: Host folder sharing via USB issue |
|
Date: |
Wed, 14 Jul 2021 17:12:27 -0400 |
> On Jul 14, 2021, at 6:35 AM, Vladimir Sementsov-Ogievskiy
> <vsementsov@virtuozzo.com> wrote:
>
> 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)
>
I could not apply the patch directly so I had to apply it by hand. This is what
I used:
---
block/vvfat.c | 42 ++++--------------------------------------
1 file changed, 4 insertions(+), 38 deletions(-)
diff --git a/block/vvfat.c b/block/vvfat.c
index ae9d387da7..430dcc3ce8 100644
--- a/block/vvfat.c
+++ b/block/vvfat.c
@@ -3098,25 +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,
@@ -3133,7 +3114,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 +3164,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 +3178,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)
--
2.24.3 (Apple Git-128)
This patch works. It was tested with Windows 2000, Mac OS 10.4, and Mac OS 9.2.
I do agree with Balaton about the virtio-fs suggestion. It is pretty much only
for Linux. It would take a tremendous amount of work to make virtio-fs to work
on Windows and Mac OS. The vvfat option uses two technologies that pretty much
everyone can use: USB and the FAT file system. Virtio-fs would only be a good
solution for Linux guests.
Thank you.