Re: [Qemu-devel] [PATCH 1/5] block: added lock image option and callback

[Kevin Wolf]

Am 12.01.2016 um 06:38 hat Denis V. Lunev geschrieben:
> On 01/11/2016 08:31 PM, Kevin Wolf wrote:
> >Am 23.12.2015 um 08:46 hat Denis V. Lunev geschrieben:
> >>From: Olga Krishtal <address@hidden>
> >>
> >>While opening the image we want to be sure that we are the
> >>one who works with image, anf if it is not true -
> >>opening the image for writing should fail.
> >>
> >>There are 2 ways at the moment: no lock at all and lock the file
> >>image.
> >>
> >>Signed-off-by: Olga Krishtal <address@hidden>
> >>Signed-off-by: Denis V. Lunev <address@hidden>
> >>CC: Kevin Wolf <address@hidden>
> >>CC: Max Reitz <address@hidden>
> >>CC: Eric Blake <address@hidden>
> >>CC: Fam Zheng <address@hidden>
> >As long as locking is disabled by default, it's useless and won't
> >prevent people from corrupting their images. These corruptions happen
> >exactly because people don't know how to use qemu properly. You can't
> >expect them to enable locking manually.
> You are right. Though this is not a big problem to enable locking.
> If there are several mechanics, we could save one into the
> qcow2 header.

The problem is that libvirt already takes a lock, as Dan mentioned in
another reply in this thread, so we can't enable locking in qemu by
default. It would always fail when run under libvirt.

Unless I'm seriously mistaken, this means that flock() inside qemu is
dead.

> >Also, you probably need to consider bdrv_reopen() and live migration.
> >I think live migration would be blocked if source and destination both
> >see the lock; which is admittedly less likely than with the qcow2 patch
> >(and generally a problem of this series), but with localhost migration
> >and potentially with some NFS setups it can be the case.
> >
> >Kevin
> for live migration the situation should be not that problematic.
> The disk is opened in RW mode only on one node. Am I right?
> The lock is taken when the image is opened in RW mode only.

You wish. :-)

That's exactly the reason why my patch series contains all these patches
with inactivate/invalidate_cache. These patches still don't avoid that
the file descriptor is opened r/w on both sides, but at least they
ensure that it's clearly defined who actually uses it to write.

Kevin