All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Daniel P. Berrange" <berrange@redhat.com>
To: Jamie Lokier <jamie@shareable.org>
Cc: "Richard W.M. Jones" <rjones@redhat.com>,
	qemu-devel@nongnu.org, Avi Kivity <avi@redhat.com>
Subject: Re: [Qemu-devel] [PATCH] Disk image shared and exclusive locks.
Date: Mon, 7 Dec 2009 10:45:17 +0000	[thread overview]
Message-ID: <20091207104517.GJ24530@redhat.com> (raw)
In-Reply-To: <20091207103128.GA26970@shareable.org>

On Mon, Dec 07, 2009 at 10:31:28AM +0000, Jamie Lokier wrote:
> Anthony Liguori wrote:
> > I'm not sure whether it's best to enable it by default because, as I 
> > said earlier, I'm not comfortable with the lack of correctness wrt 
> > advisory vs. mandatory locking.
> 
> In my experience, disk images are accessed in one of five ways:
> 
>     qemu-img
>     qemu
>     qemu-nbd
>     mount -o loop
>     cp/rsync
> 
> If all but the last implement qemu's advisory locking, that's comforting.
> 
> > Only qemu can implement this level of locking though so it's
> > definitely something we ought to support.
> 
> That's not quite true.  I have management scripts which call qemu-img
> to determine the chain of backing images, and then can lock the chain.
> (They determine the chain anyway, to provide reliable behaviour with
> image names containing unusual characters and the old monitor, by
> creating links with sane names in /tmp to the real files.)
> 
> But it's a lot nicer if qemu does it.
> 
> > However, from a UI and implementation perspective, I think we need 
> > significantly different semantics.  You either want locking or you 
> > don't.  I don't think the selection of none|shared|exclusive really 
> > makes sense.
> 
> Sometimes shared access to a raw image (partitioned or whole disk
> filesystem) is ok, and sometimes it is not ok.  Only the user knows
> the difference, because only the user knows if the guests they are
> running use distinct partitions in the same raw image, or cooperative
> access to a shard image.
> 
> But does it make sense to request a shared lock in that case?  Not
> really.  If you have a group of guests correctly sharing an image, you
> still want to prevent running the same group a second time - and a
> shared lock wouldn't do that, because each group would be requesting
> shared locks.
> 
> So the distinction read/write makes more sense.  Can anyone think of a
> situation where a shared lock on an image opened for writing is useful?

Isn't this what Richard has already done ? The patch implements 'shared'
as a 'F_RDLCK' lock and 'exclusive' as 'F_WRLCK':

+        if (bdrv_flags & BDRV_O_LOCK_SHARED)
+            lk.l_type = F_RDLCK;
+        else /* bdrv_flags & BDRV_O_LOCK_EXCLUSIVE */
+            lk.l_type = F_WRLCK;

It seems we're just debating different terminology for the same thing.
Indeed the fcntl()  man page uses read/write and shared/exclusive
interchangably too

       The  l_type  field  can  be  used  to  place  a  read
       (F_RDLCK) or a write (F_WRLCK) lock on a  file.   Any
       number  of  processes  may  hold  a read lock (shared
       lock) on a file region, but only one process may hold
       a  write  lock  (exclusive  lock).  An exclusive lock
       excludes all other locks, both shared and  exclusive.

Regards,
Daniel
-- 
|: Red Hat, Engineering, London   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

  parent reply	other threads:[~2009-12-07 10:45 UTC|newest]

Thread overview: 52+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-12-04 16:53 [Qemu-devel] [PATCH] Disk image shared and exclusive locks Richard W.M. Jones
2009-12-04 17:15 ` Anthony Liguori
2009-12-04 21:57   ` Richard W.M. Jones
2009-12-04 22:29     ` Anthony Liguori
2009-12-05 17:31       ` Avi Kivity
2009-12-05 17:47         ` Anthony Liguori
2009-12-05 17:55           ` Avi Kivity
2009-12-05 17:59             ` Anthony Liguori
2009-12-07 10:31               ` Jamie Lokier
2009-12-07 10:42                 ` Kevin Wolf
2009-12-07 10:48                   ` Avi Kivity
2009-12-07 10:56                     ` Kevin Wolf
2009-12-07 11:28                   ` Jamie Lokier
2009-12-07 11:51                     ` Kevin Wolf
2009-12-07 12:06                     ` Daniel P. Berrange
2009-12-07 10:45                 ` Daniel P. Berrange [this message]
2009-12-07 11:19                   ` Jamie Lokier
2009-12-07 11:30                     ` Daniel P. Berrange
2009-12-07 11:31                       ` Richard W.M. Jones
2009-12-07 11:38                         ` Jamie Lokier
2009-12-07 11:49                         ` Daniel P. Berrange
2009-12-07 11:59                           ` Richard W.M. Jones
2009-12-07 14:35                           ` [Qemu-devel] " Paolo Bonzini
2009-12-07 13:43                       ` [Qemu-devel] " Anthony Liguori
2009-12-07 14:01                         ` Daniel P. Berrange
2009-12-07 14:15                           ` Anthony Liguori
2009-12-07 14:28                             ` Daniel P. Berrange
2009-12-07 14:53                               ` Anthony Liguori
2009-12-08  9:40                                 ` Kevin Wolf
2009-12-07 11:04                 ` Richard W.M. Jones
2009-12-07 10:58           ` Richard W.M. Jones
2009-12-07 11:35             ` Jamie Lokier
2009-12-07 13:39             ` Anthony Liguori
2009-12-07 14:08               ` Richard W.M. Jones
2009-12-07 14:22                 ` Anthony Liguori
2009-12-07 14:31                   ` Richard W.M. Jones
2009-12-07 14:55                     ` Anthony Liguori
2009-12-08  9:48                     ` Kevin Wolf
2009-12-08 10:16                       ` Richard W.M. Jones
2009-12-07 14:38               ` [Qemu-devel] " Paolo Bonzini
2009-12-07  9:38   ` [Qemu-devel] " Daniel P. Berrange
2009-12-07 10:39 ` Chris Webb
2009-12-07 13:32   ` Anthony Liguori
2009-12-07 13:38     ` Chris Webb
2009-12-07 13:47       ` Anthony Liguori
2009-12-07 14:25     ` Daniel P. Berrange
2009-12-07 14:58       ` Chris Webb
2009-12-07 14:16 ` [Qemu-devel] [PATCH VERSION 2] " Richard W.M. Jones
2009-12-07 15:06   ` Anthony Liguori
2009-12-08  8:48     ` [Qemu-devel] " Paolo Bonzini
2009-12-08 10:00   ` [Qemu-devel] " Kevin Wolf
2009-12-08 10:25     ` Richard W.M. Jones

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20091207104517.GJ24530@redhat.com \
    --to=berrange@redhat.com \
    --cc=avi@redhat.com \
    --cc=jamie@shareable.org \
    --cc=qemu-devel@nongnu.org \
    --cc=rjones@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.