On 3/27/19 3:33 PM, John Snow wrote: > > > On 3/27/19 8:49 AM, Vladimir Sementsov-Ogievskiy wrote: >> bdrv_replace_child() calls bdrv_check_perm() with error_abort on >> loosening permissions. However file-locking operations may fail even >> in this case, for example on NFS. And this leads to Qemu crash. >> >> Let's ignore such errors, as we do already on permission update commit >> and abort. >> >> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsement...@virtuozzo.com> >> --- >> block/file-posix.c | 14 ++++++++++++++ >> 1 file changed, 14 insertions(+) >> >> diff --git a/block/file-posix.c b/block/file-posix.c >> index db4cccbe51..403e67fe90 100644 >> --- a/block/file-posix.c >> +++ b/block/file-posix.c >> @@ -815,6 +815,20 @@ static int raw_handle_perm_lock(BlockDriverState *bs, >> >> switch (op) { >> case RAW_PL_PREPARE: >> + if ((s->perm | new_perm) == s->perm &&
This says if new_perm does not add any bits beyond what s->perm had. >> + (~s->shared_perm | ~new_perm) == ~s->shared_perm) > > Little strange to read, but ultimately "If we aren't changing anything" > based on the call below. '(~a | ~b)' is equivalent to '~(a & b)'. '~(a & b) == ~a' is equivalent to '(a & b) == a' That expression is much easier to read, as new_perm does not remove any bits beyond what s->shared_perm already had. But rewriting it in an easier form would indeed make the patch easier to swallow. > >> + { >> + /* >> + * We are going to unlock bytes, it should not fail. If fail, >> + * just report it and ignore, like we do for ABORT and COMMIT >> + * anyway. >> + */ >> + ret = raw_check_lock_bytes(s->fd, new_perm, new_shared, >> &local_err); >> + if (local_err) { >> + error_report_err(local_err); >> + } >> + return 0; >> + } >> ret = raw_apply_lock_bytes(s, s->fd, s->perm | new_perm, >> ~s->shared_perm | ~new_shared, >> false, errp); >> > > I thiiiink this makes sense, but hopefully someone else can give it the > once-over too. > > Reviewed-by: John Snow <js...@redhat.com> > > -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
signature.asc
Description: OpenPGP digital signature