On Tue, May 10, 2016 at 11:14:22AM +0200, Kevin Wolf wrote: > Am 10.05.2016 um 10:50 hat Daniel P. Berrange geschrieben: > > On Tue, May 10, 2016 at 09:43:04AM +0100, Richard W.M. Jones wrote: > > > On Tue, May 10, 2016 at 09:14:26AM +0100, Richard W.M. Jones wrote: > > > > However I didn't test the write-shareable case (the libvirt > > > > <shareable/> flag which should map to a shared lock -- is that right > > > > Dan?). > > > > > > To Dan (mainly): I think setting the <shareable/> flag in libvirt only > > > sets cache=unsafe on the qemu drive (it may have other effects for > > > virtlockd). Should there be an extra qemu drive flag to communicate > > > that the drive is write-shareable? > > > > Sure, if QEMU had a way to indicate that the disk was used in a > > write-sharable mode, libvirt would use it. > > > > I agree with your general point that we should do no locking for > > read-only access, F_RDLCK for shared-write and F_WRLCK for > > exclusive-write access. I think I made that point similarly against > > an earlier version of this patchset > > Why should we do no locking for read-only access by default? If an image > is written to, read-only accesses are potentially inconsistent and we > should protect users against it. > > The only argument I can see in the old versions of this series is > "libguestfs does it and usually it gets away with it". For me, that's > not an argument for generally allowing this, but at most for providing a > switch to bypass the locking.
Most of the interesting stats derived from disks come from the superblock (eg. virt-df information) or parts of the filesystem that rarely change (for example contents of /usr). I don't claim that this is always safe, but I do claim that it works almost all the time, and when it doesn't, it doesn't especially matter because you're collecting time series data where one missed stat is irrelevant. > Because let's be clear about this: If someone lost data because they > took an inconsistent backup this way and comes to us qemu developers, > all we can tell them is "sorry, what you did is not supported". And > that's a pretty strong sign that locking should prevent it by default. Mostly that would happen because someone copies the disk image (eg. rsync /var/lib/libvirt/images/*.qcow2 /backups). Nothing in this patch or anywhere else will protect against that. Libguestfs has prominent warnings in every manual page about both writing to live VMs and reading live VMs, eg: http://libguestfs.org/guestfish.1.html#warning Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-df lists disk usage of guests without needing to install any software inside the virtual machine. Supports Linux and Windows. http://people.redhat.com/~rjones/virt-df/