Am 20.02.2017 um 13:25 hat Max Reitz geschrieben: > 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 <[email protected]> > > --- > > 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?
I couldn't see any use for it. With an SG BDS, you can't really do
anything anyway except directly attach it to scsi-generic. And the only
thing that scsi-generic does is invoking ioctls, which the block layer
doesn't understand but just pass though.
So I didn't feel that op blockers could provide anything here.
> > 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.)
Yeah, I'm not completely sure about the real requirements here either.
What I do know is that currently nothing is blocked (so doing the same
in the future won't make things worse at least), and that I don't want
to break exotic devices that I can't really test. So for these devices I
tended to be more permissive in case of doubt.
Kevin
pgp5XD08vgXG2.pgp
Description: PGP signature
