qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH 0/4] virtio: handle zoned backing devices


From: Kevin Wolf
Subject: Re: [Qemu-devel] [PATCH 0/4] virtio: handle zoned backing devices
Date: Wed, 10 Jul 2019 23:09:14 +0200
User-agent: Mutt/1.11.3 (2019-02-01)

Am 10.07.2019 um 13:33 hat Paolo Bonzini geschrieben:
> On 10/07/19 13:02, Kevin Wolf wrote:
> > Hm... Actually, file-posix implements .bdrv_check_perm and could just
> > refuse attaching a parent there if it doesn't request a specific
> > permission like BLK_PERM_SUPPORT_ZONED. That should give us the
> > whitelist semantics through existing infrastructure.
> 
> I'd like Dmitry to have something more precise to base his work on.  The
> permissions system is really complicated and I never really wrapped my
> head around it, so I need your help.
> 
> IIUC, blkconf_apply_backend_options would grow a new argument (like
> "resizable") and that argument would add BLK_PERM_SUPPORT_ZONED to the
> perm that blkconf_apply_backend_options passes to blk_set_perm.  On the
> other side raw_check_perm would say something like
> 
>     if (is_zoned(s) && !(perm & BLK_PERM_SUPPORT_ZONED)) {
>         error_setg(....);
>         return -ENOTSUP;
>     }
> 
> Is this correct?

Yes, I think this is how you'd best implement it.

> In addition, BLK_PERM_SUPPORT_ZONED would have to be a shared
> permission, since it's possible to assign the same block device to
> multiple scsi-block devices.  So BLK_PERM_SUPPORT_ZONED would be added
> unconditionally to shared_perm.

Right, this part shows that we're kind of abusing the permission system
here. I think unconditionally adding BLK_PERM_SUPPORT_ZONED to the set
of shared permissions could probably happen centrally in
bdrv_child_perm().

> ps: I have always thought that shared_perm is expressed the wrong way
> and should have been "denied_perm".  How hard would it be to change that
> now?

I'm not sure it would be better. It is shared_perm because that means
that the default is restrictive (error mode: operation refused, clearly
reported, easy to fix) rather than permissive (error mode: image
corruption, hard to figure out where we were too permissive). Basically,
whitelist instead of blacklist, once again.

But if we did indeed decide to change it, the only way to find out how
hard it is would be doing it. I suspect not too hard in principle, but
making sure that we converted all callers and don't introduce wrong
callers later through (silent) merge conflicts is the more concerning
part.

Kevin



reply via email to

[Prev in Thread] Current Thread [Next in Thread]