block.c | 6 ++++++ 1 file changed, 6 insertions(+)
QEMU exits in error when passing a vfat shared folder in read-only mode.
To fix this issue, this patch removes any potential write permission
from cumulative_perms, when a read-only block device is in use.
Buglink: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=918950
Signed-off-by: Guillaume Roche <groche@genymobile.com>
---
This is an attempt to fix this behavior, but it feels a bit hacky to me
since this patch checks for the vvfat format in a generic function.
However, I'd be glad to have some advice to make it better. Anyway, I
ran the block tests to ensure this does not introduce any regression.
To add some context: I know that this can be worked around by setting
the shared folder in rw mode. But our use-case requires using both
shared and VM snapshots, and QEMU prevents using snapshot with a rw
shared folder.
block.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/block.c b/block.c
index e97ce0b1c8..3f59e3843f 100644
--- a/block.c
+++ b/block.c
@@ -2383,6 +2383,12 @@ void bdrv_get_cumulative_perm(BlockDriverState *bs, uint64_t *perm,
cumulative_shared_perms &= c->shared_perm;
}
+ /* Discard write permission if vvfat block device is read-only */
+ const char *format = bdrv_get_format_name(bs);
+ if (format != NULL && strncmp(format, "vvfat", 5) == 0 && bdrv_is_read_only(bs)) {
+ cumulative_perms &= ~BLK_PERM_WRITE;
+ }
+
*perm = cumulative_perms;
*shared_perm = cumulative_shared_perms;
}
--
2.31.1
Hi Guillaume, On 8/31/21 4:17 PM, Guillaume Roche wrote: > QEMU exits in error when passing a vfat shared folder in read-only mode. > > To fix this issue, this patch removes any potential write permission > from cumulative_perms, when a read-only block device is in use. > > Buglink: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=918950 > > Signed-off-by: Guillaume Roche <groche@genymobile.com> > --- > This is an attempt to fix this behavior, but it feels a bit hacky to me > since this patch checks for the vvfat format in a generic function. What about implementing bdrv_vvfat::bdrv_check_perm()? > However, I'd be glad to have some advice to make it better. Anyway, I > ran the block tests to ensure this does not introduce any regression. > > To add some context: I know that this can be worked around by setting > the shared folder in rw mode. But our use-case requires using both > shared and VM snapshots, and QEMU prevents using snapshot with a rw > shared folder. > > block.c | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/block.c b/block.c > index e97ce0b1c8..3f59e3843f 100644 > --- a/block.c > +++ b/block.c > @@ -2383,6 +2383,12 @@ void bdrv_get_cumulative_perm(BlockDriverState *bs, uint64_t *perm, > cumulative_shared_perms &= c->shared_perm; > } > > + /* Discard write permission if vvfat block device is read-only */ > + const char *format = bdrv_get_format_name(bs); > + if (format != NULL && strncmp(format, "vvfat", 5) == 0 && bdrv_is_read_only(bs)) { > + cumulative_perms &= ~BLK_PERM_WRITE; > + } > + > *perm = cumulative_perms; > *shared_perm = cumulative_shared_perms; > } >
Hi Philippe, Le mar. 31 août 2021 à 18:06, Philippe Mathieu-Daudé <philmd@redhat.com> a écrit : > > Hi Guillaume, > > On 8/31/21 4:17 PM, Guillaume Roche wrote: > > QEMU exits in error when passing a vfat shared folder in read-only mode. > > > > To fix this issue, this patch removes any potential write permission > > from cumulative_perms, when a read-only block device is in use. > > > > Buglink: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=918950 > > > > Signed-off-by: Guillaume Roche <groche@genymobile.com> > > --- > > This is an attempt to fix this behavior, but it feels a bit hacky to me > > since this patch checks for the vvfat format in a generic function. > > What about implementing bdrv_vvfat::bdrv_check_perm()? Thanks for this feedback. I had a look at your suggestion, but I'm a bit confused. As I understand it, bdrv_node_refresh_perm() calls bdrv_get_cumulative_perm() (which I patched to remove the write permission) then it checks the permissions. Afterwards, it calls bdrv_drv_set_perm() that in turn calls bs->drv->bdrv_check_perm(). So even if I implement bdrv_vvfat::bdrv_check_perm(), I will get the permission error before it is called. Could you elaborate a bit on what you have in mind please? Regards, Guillaume > > > However, I'd be glad to have some advice to make it better. Anyway, I > > ran the block tests to ensure this does not introduce any regression. > > > > To add some context: I know that this can be worked around by setting > > the shared folder in rw mode. But our use-case requires using both > > shared and VM snapshots, and QEMU prevents using snapshot with a rw > > shared folder. > > > > block.c | 6 ++++++ > > 1 file changed, 6 insertions(+) > > > > diff --git a/block.c b/block.c > > index e97ce0b1c8..3f59e3843f 100644 > > --- a/block.c > > +++ b/block.c > > @@ -2383,6 +2383,12 @@ void bdrv_get_cumulative_perm(BlockDriverState *bs, uint64_t *perm, > > cumulative_shared_perms &= c->shared_perm; > > } > > > > + /* Discard write permission if vvfat block device is read-only */ > > + const char *format = bdrv_get_format_name(bs); > > + if (format != NULL && strncmp(format, "vvfat", 5) == 0 && bdrv_is_read_only(bs)) { > > + cumulative_perms &= ~BLK_PERM_WRITE; > > + } > > + > > *perm = cumulative_perms; > > *shared_perm = cumulative_shared_perms; > > } > > >
Am 31.08.2021 um 16:17 hat Guillaume Roche geschrieben: > QEMU exits in error when passing a vfat shared folder in read-only mode. > > To fix this issue, this patch removes any potential write permission > from cumulative_perms, when a read-only block device is in use. > > Buglink: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=918950 > > Signed-off-by: Guillaume Roche <groche@genymobile.com> > --- > This is an attempt to fix this behavior, but it feels a bit hacky to me > since this patch checks for the vvfat format in a generic function. > > However, I'd be glad to have some advice to make it better. Anyway, I > ran the block tests to ensure this does not introduce any regression. > > To add some context: I know that this can be worked around by setting > the shared folder in rw mode. But our use-case requires using both > shared and VM snapshots, and QEMU prevents using snapshot with a rw > shared folder. I don't think the behaviour is actually a bug: ide-hd requires a writable backend, so attaching a read-only vvfat node is a real error. You can either specify -drive read-only=on and use a device that can accept read-only backends (such as virtio-blk or ide-cd), or add a temporary writable qcow2 overlay with -snapshot or -drive snapshot=on so that the ide-hd device actually does get the writable backend it needs. Kevin
© 2016 - 2024 Red Hat, Inc.