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 <kw...@redhat.com> > > --- > > 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
pgpwGUFlWRVpe.pgp
Description: PGP signature