qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Fam Zheng <famz@redhat.com>
To: Kevin Wolf <kwolf@redhat.com>
Cc: Max Reitz <mreitz@redhat.com>,
	qemu-block@nongnu.org, Markus Armbruster <armbru@redhat.com>,
	Jeff Cody <jcody@redhat.com>,
	qemu-devel@nongnu.org, rjones@redhat.com, pbonzini@redhat.com,
	stefanha@redhat.com, den@openvz.org, John Snow <jsnow@redhat.com>
Subject: Re: [Qemu-devel] [PATCH v8 00/36] block: Image locking series
Date: Wed, 26 Oct 2016 19:01:28 +0800	[thread overview]
Message-ID: <20161026110128.GF14605@lemon> (raw)
In-Reply-To: <20161025145739.GL4695@noname.str.redhat.com>

On Tue, 10/25 16:57, Kevin Wolf wrote:
> Am 25.10.2016 um 15:30 hat Max Reitz geschrieben:
> > On 25.10.2016 10:24, Kevin Wolf wrote:
> > > Am 24.10.2016 um 20:03 hat Max Reitz geschrieben:
> > >> On 24.10.2016 12:11, Kevin Wolf wrote:
> > >>
> > >> [...]
> > >>
> > >>> Now, the big question is how to translate this into file locking. This
> > >>> could become a little tricky. I had a few thoughts involving another
> > >>> lock on byte 2, but none of them actually worked out so far, because
> > >>> what we want is essentially a lock that can be shared by readers, that
> > >>> can also be shared by writers, but not by readers and writers at the
> > >>> same time.
> > >>
> > >> You can also share it between readers and writers, as long as everyone
> > >> can cope with volatile data.
> > > 
> > > Sorry, that was ambiguous. I meant a file-level lock rather than the
> > > high-level one. If we had a lock that can be shared by one or the other,
> > > but not both, then two locks would be enough to build what we really
> > > want.
> > > 
> > >> I agree that it's very similar to the proposed op blocker style, but I
> > >> can't really come up with a meaningful translation either.
> > >>
> > >> Maybe something like this (?): All readers who do not want the file to
> > >> be modified grab a shared lock on byte 1. All writers who can deal with
> > >> volatile data grab a shared lock on byte 2. Exclusive writers grab an
> > >> exclusive lock on byte 1 and 2. Readers who can cope with volatile data
> > >> get no lock at all.
> > >>
> > >> When opening, the first and second group would always have to test
> > >> whether there is a lock on the other byte, respectively. E.g. sharing
> > >> writers would first grab an exclusive lock on byte 1, then the shared
> > >> lock on byte 2 and then release the exclusive lock again.
> > >>
> > >> Would that work?
> > > 
> > > I'm afraid it wouldn't. If you start the sharing writer first and then
> > > the writer-blocking reader, the writer doesn't hold a lock on byte 1 any
> > > more,
> > 
> > But it holds a lock on byte 2.
> > 
> > >       so the reader can start even though someone is writing to the
> > > image.
> > 
> > It can't because it would try to grab an exclusive lock on byte 2 before
> > grabbing the shared lock on byte 1.
> 
> Apparently I failed to understand the most important part of the
> proposal. :-)
> 
> So we have two locks. Both are only held for a longer time in shared
> mode. Exclusive mode is only used for testing whether the lock is being
> held and is immediately given up again.
> 
> The meaning of holding a shared lock is:
> 
>     byte 1: I can't allow other processes to write to the image
>     byte 2: I am writing to the image
> 
> The four cases that we have involve:
> 
> * shared writer: Take shared lock on byte 2. Test whether byte 1 is
>   locked using an exclusive lock, and fail if so.
> 
> * exclusive writer: Take shared lock on byte 2. Test whether byte 1 is
>   locked using an exclusive lock, and fail if so. Then take shared lock
>   on byte 1. I suppose this is racy, but we can probably tolerate that.
> 
> * reader that can tolerate writers: Don't do anything
> 
> * reader that can't tolerate writers: Take shared lock on byte 1. Test
>   whether byte 2 is locked, and fail if so.
> 
> Seems to work if I got that right.

Does this mean I should change ImageLockMode to:

 * exclusive
 * shared-write
 * shared-read
 * nolock
 * auto

Where "auto" maps to exclusive for O_RDWR and shared-read for O_RDONLY?

Fam

  reply	other threads:[~2016-10-26 11:01 UTC|newest]

Thread overview: 69+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-09-30 12:09 [Qemu-devel] [PATCH v8 00/36] block: Image locking series Fam Zheng
2016-09-30 12:09 ` [Qemu-devel] [PATCH v8 01/36] block: Add flag bits for image locking Fam Zheng
2016-09-30 12:09 ` [Qemu-devel] [PATCH v8 02/36] qapi: Add ImageLockMode Fam Zheng
2016-10-21 20:45   ` Max Reitz
2016-10-25  5:36     ` Fam Zheng
2016-10-25 13:20       ` Max Reitz
2016-10-25 13:34         ` Fam Zheng
2016-09-30 12:09 ` [Qemu-devel] [PATCH v8 03/36] block: Introduce image file locking Fam Zheng
2016-10-21 21:04   ` Max Reitz
2016-10-25  5:48     ` Fam Zheng
2016-10-25 13:21       ` Max Reitz
2016-09-30 12:09 ` [Qemu-devel] [PATCH v8 04/36] osdep: Add qemu_lock_fd and qemu_unlock_fd Fam Zheng
2016-10-21 21:15   ` Max Reitz
2016-09-30 12:09 ` [Qemu-devel] [PATCH v8 05/36] raw-posix: Add image locking support Fam Zheng
2016-10-21 23:40   ` Max Reitz
2016-10-25  6:31     ` Fam Zheng
2016-10-25 13:28       ` Max Reitz
2016-10-25 13:43         ` Fam Zheng
2016-10-26 14:56           ` Max Reitz
2016-09-30 12:09 ` [Qemu-devel] [PATCH v8 06/36] qemu-io: Add "-L" option for BDRV_O_NO_LOCK Fam Zheng
2016-09-30 12:09 ` [Qemu-devel] [PATCH v8 07/36] qemu-img: Add "-L" option to sub commands Fam Zheng
2016-09-30 12:09 ` [Qemu-devel] [PATCH v8 08/36] qemu-img: Update documentation of "-L" option Fam Zheng
2016-09-30 12:09 ` [Qemu-devel] [PATCH v8 09/36] qemu-nbd: Add "--no-lock/-L" option Fam Zheng
2016-09-30 12:09 ` [Qemu-devel] [PATCH v8 10/36] block: Don't lock drive-backup target image in none mode Fam Zheng
2016-09-30 12:09 ` [Qemu-devel] [PATCH v8 11/36] block: Add blk_lock_image Fam Zheng
2016-09-30 12:09 ` [Qemu-devel] [PATCH v8 12/36] virtio-blk: Apply lock-mode when realize Fam Zheng
2016-10-22  0:08   ` Max Reitz
2016-10-22  0:12     ` Max Reitz
2016-09-30 12:09 ` [Qemu-devel] [PATCH v8 13/36] scsi-disk: " Fam Zheng
2016-09-30 12:09 ` [Qemu-devel] [PATCH v8 14/36] scsi-generic: " Fam Zheng
2016-09-30 12:09 ` [Qemu-devel] [PATCH v8 15/36] qdev: Add "lock-mode" to block device options Fam Zheng
2016-10-22  0:11   ` Max Reitz
2016-10-25  5:58     ` Fam Zheng
2016-09-30 12:09 ` [Qemu-devel] [PATCH v8 16/36] ide: Apply lock-mode when initialize Fam Zheng
2016-09-30 12:09 ` [Qemu-devel] [PATCH v8 17/36] nvme: " Fam Zheng
2016-09-30 12:09 ` [Qemu-devel] [PATCH v8 18/36] usb-storage: Apply lock-mode when realize Fam Zheng
2016-09-30 12:09 ` [Qemu-devel] [PATCH v8 19/36] pflash: Add "lock-mode" property Fam Zheng
2016-09-30 12:09 ` [Qemu-devel] [PATCH v8 20/36] qemu-iotests: 046: Move version detection out from verify_io Fam Zheng
2016-09-30 12:09 ` [Qemu-devel] [PATCH v8 21/36] qemu-iotests: 091: Prepare for image lock Fam Zheng
2016-09-30 12:09 ` [Qemu-devel] [PATCH v8 22/36] qemu-iotests: 030: Disable image locking when checking test image Fam Zheng
2016-09-30 12:09 ` [Qemu-devel] [PATCH v8 23/36] iotests: 087: Disable image locking in cases where file is shared Fam Zheng
2016-09-30 12:09 ` [Qemu-devel] [PATCH v8 24/36] " Fam Zheng
2016-09-30 12:09 ` [Qemu-devel] [PATCH v8 25/36] iotests: 130: Check image info locklessly Fam Zheng
2016-09-30 12:09 ` [Qemu-devel] [PATCH v8 26/36] iotests: Disable image locking in 085 Fam Zheng
2016-09-30 12:09 ` [Qemu-devel] [PATCH v8 27/36] tests: Use null-co:// instead of /dev/null Fam Zheng
2016-09-30 12:09 ` [Qemu-devel] [PATCH v8 28/36] qemu-iotests: Add test case 153 for image locking Fam Zheng
2016-09-30 12:09 ` [Qemu-devel] [PATCH v8 29/36] ahci: Use shared lock for shared storage migration Fam Zheng
2016-09-30 12:10 ` [Qemu-devel] [PATCH v8 30/36] tests/postcopy: Use shared lock for images Fam Zheng
2016-09-30 12:10 ` [Qemu-devel] [PATCH v8 31/36] fdc: Add lock-mode qdev properties Fam Zheng
2016-09-30 12:10 ` [Qemu-devel] [PATCH v8 32/36] m25p80: Add 'lock-mode' property Fam Zheng
2016-09-30 12:10 ` [Qemu-devel] [PATCH v8 33/36] nand: " Fam Zheng
2016-09-30 12:10 ` [Qemu-devel] [PATCH v8 34/36] onenand: " Fam Zheng
2016-09-30 12:10 ` [Qemu-devel] [PATCH v8 35/36] spapr_nvram: " Fam Zheng
2016-09-30 12:10 ` [Qemu-devel] [PATCH v8 36/36] sd: " Fam Zheng
2016-10-22  1:00 ` [Qemu-devel] [PATCH v8 00/36] block: Image locking series Max Reitz
2016-10-24 10:11   ` Kevin Wolf
2016-10-24 18:03     ` Max Reitz
2016-10-25  8:24       ` Kevin Wolf
2016-10-25 13:30         ` Max Reitz
2016-10-25 14:57           ` Kevin Wolf
2016-10-26 11:01             ` Fam Zheng [this message]
2016-10-26 15:12               ` Max Reitz
2016-10-26 15:33                 ` Kevin Wolf
2016-10-26 15:34                   ` Max Reitz
2016-10-27  6:25                   ` Fam Zheng
2016-10-26 15:04             ` Max Reitz
2016-10-25  7:09     ` Fam Zheng
2016-10-25  8:06       ` Richard W.M. Jones
2016-10-25  9:19         ` Fam Zheng

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=20161026110128.GF14605@lemon \
    --to=famz@redhat.com \
    --cc=armbru@redhat.com \
    --cc=den@openvz.org \
    --cc=jcody@redhat.com \
    --cc=jsnow@redhat.com \
    --cc=kwolf@redhat.com \
    --cc=mreitz@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=qemu-block@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    --cc=rjones@redhat.com \
    --cc=stefanha@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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).