Re: [Qemu-devel] [RFC PATCH 19/41] hw/block: Request permissions

[Max Reitz]

On 13.02.2017 18:22, Kevin Wolf wrote:
> This makes all device emulations with a qdev drive property request
> permissions on their BlockBackend. We don't block anything yet.
> 
> Signed-off-by: Kevin Wolf <address@hidden>
> ---
>  hw/block/block.c                 | 19 ++++++++++++++++++-
>  hw/block/fdc.c                   | 25 +++++++++++++++++++++++--
>  hw/block/m25p80.c                |  8 ++++++++
>  hw/block/nand.c                  |  7 +++++++
>  hw/block/nvme.c                  |  8 +++++++-
>  hw/block/onenand.c               |  7 +++++++
>  hw/block/pflash_cfi01.c          | 18 ++++++++++++------
>  hw/block/pflash_cfi02.c          | 19 +++++++++++++------
>  hw/block/virtio-blk.c            |  8 +++++++-
>  hw/core/qdev-properties-system.c |  1 -
>  hw/ide/qdev.c                    |  7 +++++--
>  hw/nvram/spapr_nvram.c           |  8 ++++++++
>  hw/scsi/scsi-disk.c              |  8 ++++++--

Since I have no idea how scsi-generic and all of that works, just an
innocent question: Do we need to set permissions there, too?

>  hw/sd/sd.c                       |  6 ++++++
>  hw/usb/dev-storage.c             |  6 +++++-
>  include/hw/block/block.h         |  3 ++-
>  tests/qemu-iotests/051.pc.out    |  6 +++---
>  17 files changed, 137 insertions(+), 27 deletions(-)

[...]

> diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c
> index 2d6eb46..190573c 100644
> --- a/hw/block/m25p80.c
> +++ b/hw/block/m25p80.c
> @@ -1215,6 +1215,7 @@ static void m25p80_realize(SSISlave *ss, Error **errp)
>  {
>      Flash *s = M25P80(ss);
>      M25P80Class *mc = M25P80_GET_CLASS(s);
> +    int ret;
>  
>      s->pi = mc->pi;
>  
> @@ -1222,6 +1223,13 @@ static void m25p80_realize(SSISlave *ss, Error **errp)
>      s->dirty_page = -1;
>  
>      if (s->blk) {
> +        uint64_t perm = BLK_PERM_CONSISTENT_READ |
> +                        (blk_is_read_only(s->blk) ? 0 : BLK_PERM_WRITE);
> +        ret = blk_set_perm(s->blk, perm, BLK_PERM_ALL, errp);

Not sure whether it should be changed in this patch, but I don't know
whether BLK_PERM_ALL is right here; and from a quick glance it doesn't
look like any of the following patches change it.

(Same for all of the block device emulation code that invokes
blk_set_perm() directly.)

Max

> +        if (ret < 0) {
> +            return;
> +        }
> +
>          DB_PRINT_L(0, "Binding to IF_MTD drive\n");
>          s->storage = blk_blockalign(s->blk, s->size);
>  

[...]

signature.asc
Description: OpenPGP digital signature