diff mbox

[RFC,11/41] vvfat: Implement .bdrv_child_perm()

Message ID 1487006583-24350-12-git-send-email-kwolf@redhat.com
State New
Headers show

Commit Message

Kevin Wolf Feb. 13, 2017, 5:22 p.m. UTC
vvfat is the last remaining driver that can have children, but doesn't
implement .bdrv_child_perm() yet. The default handlers aren't suitable
here, so let's implement a very simple driver-specific one that protects
the internal child from being used by other users as good as our
permissions permit.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 block/vvfat.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

Comments

Max Reitz Feb. 15, 2017, 5:30 p.m. UTC | #1
On 13.02.2017 18:22, Kevin Wolf wrote:
> vvfat is the last remaining driver that can have children, but doesn't
> implement .bdrv_child_perm() yet. The default handlers aren't suitable
> here, so let's implement a very simple driver-specific one that protects
> the internal child from being used by other users as good as our
> permissions permit.
> 
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
>  block/vvfat.c | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> diff --git a/block/vvfat.c b/block/vvfat.c
> index c6bf67e..7246432 100644
> --- a/block/vvfat.c
> +++ b/block/vvfat.c
> @@ -3052,6 +3052,18 @@ err:
>      return ret;
>  }
>  
> +static void vvfat_child_perm(BlockDriverState *bs, BdrvChild *c,
> +                             const BdrvChildRole *role,
> +                             uint64_t perm, uint64_t shared,
> +                             uint64_t *nperm, uint64_t *nshared)
> +{
> +    assert(role == &child_vvfat_qcow);
> +
> +    /* This is a private node, nobody should try to attach to it */
> +    *nperm = BLK_PERM_WRITE;
> +    *nshared = 0;

0 for shared is probably enough to ward every other access off, but
maybe we should still pro forma request consistent read access...?

Max

> +}
> +
>  static void vvfat_close(BlockDriverState *bs)
>  {
>      BDRVVVFATState *s = bs->opaque;
> @@ -3077,6 +3089,7 @@ static BlockDriver bdrv_vvfat = {
>      .bdrv_file_open         = vvfat_open,
>      .bdrv_refresh_limits    = vvfat_refresh_limits,
>      .bdrv_close             = vvfat_close,
> +    .bdrv_child_perm        = vvfat_child_perm,
>  
>      .bdrv_co_preadv         = vvfat_co_preadv,
>      .bdrv_co_pwritev        = vvfat_co_pwritev,
>
Kevin Wolf Feb. 15, 2017, 5:42 p.m. UTC | #2
Am 15.02.2017 um 18:30 hat Max Reitz geschrieben:
> On 13.02.2017 18:22, Kevin Wolf wrote:
> > vvfat is the last remaining driver that can have children, but doesn't
> > implement .bdrv_child_perm() yet. The default handlers aren't suitable
> > here, so let's implement a very simple driver-specific one that protects
> > the internal child from being used by other users as good as our
> > permissions permit.
> > 
> > Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> > ---
> >  block/vvfat.c | 13 +++++++++++++
> >  1 file changed, 13 insertions(+)
> > 
> > diff --git a/block/vvfat.c b/block/vvfat.c
> > index c6bf67e..7246432 100644
> > --- a/block/vvfat.c
> > +++ b/block/vvfat.c
> > @@ -3052,6 +3052,18 @@ err:
> >      return ret;
> >  }
> >  
> > +static void vvfat_child_perm(BlockDriverState *bs, BdrvChild *c,
> > +                             const BdrvChildRole *role,
> > +                             uint64_t perm, uint64_t shared,
> > +                             uint64_t *nperm, uint64_t *nshared)
> > +{
> > +    assert(role == &child_vvfat_qcow);
> > +
> > +    /* This is a private node, nobody should try to attach to it */
> > +    *nperm = BLK_PERM_WRITE;
> > +    *nshared = 0;
> 
> 0 for shared is probably enough to ward every other access off, but
> maybe we should still pro forma request consistent read access...?

Makes sense, yes.

But you missed the real bug I hid there for you:

qemu-system-x86_64: block.c:1530: bdrv_check_update_perm: Assertion
`new_shared_perm & BLK_PERM_WRITE_UNCHANGED' failed.

Kevin

> Max
> 
> > +}
> > +
> >  static void vvfat_close(BlockDriverState *bs)
> >  {
> >      BDRVVVFATState *s = bs->opaque;
> > @@ -3077,6 +3089,7 @@ static BlockDriver bdrv_vvfat = {
> >      .bdrv_file_open         = vvfat_open,
> >      .bdrv_refresh_limits    = vvfat_refresh_limits,
> >      .bdrv_close             = vvfat_close,
> > +    .bdrv_child_perm        = vvfat_child_perm,
> >  
> >      .bdrv_co_preadv         = vvfat_co_preadv,
> >      .bdrv_co_pwritev        = vvfat_co_pwritev,
> > 
> 
>
diff mbox

Patch

diff --git a/block/vvfat.c b/block/vvfat.c
index c6bf67e..7246432 100644
--- a/block/vvfat.c
+++ b/block/vvfat.c
@@ -3052,6 +3052,18 @@  err:
     return ret;
 }
 
+static void vvfat_child_perm(BlockDriverState *bs, BdrvChild *c,
+                             const BdrvChildRole *role,
+                             uint64_t perm, uint64_t shared,
+                             uint64_t *nperm, uint64_t *nshared)
+{
+    assert(role == &child_vvfat_qcow);
+
+    /* This is a private node, nobody should try to attach to it */
+    *nperm = BLK_PERM_WRITE;
+    *nshared = 0;
+}
+
 static void vvfat_close(BlockDriverState *bs)
 {
     BDRVVVFATState *s = bs->opaque;
@@ -3077,6 +3089,7 @@  static BlockDriver bdrv_vvfat = {
     .bdrv_file_open         = vvfat_open,
     .bdrv_refresh_limits    = vvfat_refresh_limits,
     .bdrv_close             = vvfat_close,
+    .bdrv_child_perm        = vvfat_child_perm,
 
     .bdrv_co_preadv         = vvfat_co_preadv,
     .bdrv_co_pwritev        = vvfat_co_pwritev,