Am 20.04.2017 um 09:52 hat Fam Zheng geschrieben: > virtlockd in libvirt locks the first byte, so we start looking at the > file bytes from 0x10. > > The complication is in the transactional interface. To make the reopen > logic managable, and allow better reuse, the code is internally > organized with a table from old mode to the new one. > > Signed-off-by: Fam Zheng <f...@redhat.com>
Looking at the very early patches in this series, I think it quickly becomes obvious that we need to discuss one thing first: > +static int raw_check_perm(BlockDriverState *bs, uint64_t perm, uint64_t > shared, > + Error **errp) > +{ > + bool is_shared; > + BDRVRawState *s = bs->opaque; > + > + if (!RAW_LOCK_SUPPORTED) { > + return 0; > + } > + if (s->lock_update) { > + /* Override the previously stashed update. */ > + g_free(s->lock_update); > + s->lock_update = NULL; > + } > + is_shared = !(perm & BLK_PERM_CONSISTENT_READ) && (shared & > BLK_PERM_WRITE); Why do you check BLK_PERM_CONSISTENT_READ? The locks that we said we would take on the image file represent BLK_PERM_WRITE, both in perm and in shared, so this is what they should be checked against. Opening the image in another process is fine if BLK_PERM_WRITE is set in shared, even if BLK_PERM_CONSISTENT_READ is also set in perm. BLK_PERM_CONSISTENT_READ is for cases where the contents of an image is inherently invalid, not just because of a concurrent writer that we might not be aware of, but because the image just doesn't makes sense on it own. It may make sense as part of larger backing chain, though (the only place where we clear the flag is for intermediate nodes in the commit job). This semantics is more or less separate from what we want to achieve here. Of course, if we wanted, I guess we could individually map all 64 bits of each perm and shared to bytes to be locked in the file, so that all permissions would be shared between qemu instances. That's probably not worth the effort though. And even if we did that, most likely you still wouldn't need any special exceptions for BLK_PERM_CONSISTENT_READ, because it is always shared except when one process wants to run a commit job - and for a commit job, making sure that nobody else uses the image would probably be right. So if we really treat the file system level locks just as a mapping of BLK_PERM_WRITE, things should become a bit easier in the early patches of this series. Kevin