qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Kevin Wolf <kwolf@redhat.com>
To: "Daniel P. Berrange" <berrange@redhat.com>
Cc: Fam Zheng <famz@redhat.com>,
	qemu-devel@nongnu.org, qemu-block@nongnu.org, mreitz@redhat.com
Subject: Re: [Qemu-devel] [PATCH 00/10] qcow2: Implement image locking
Date: Mon, 11 Jan 2016 18:14:15 +0100	[thread overview]
Message-ID: <20160111171415.GK9454@noname.redhat.com> (raw)
In-Reply-To: <20151223104722.GB20028@redhat.com>

Am 23.12.2015 um 11:47 hat Daniel P. Berrange geschrieben:
> On Wed, Dec 23, 2015 at 11:14:12AM +0800, Fam Zheng wrote:
> > On Tue, 12/22 17:46, Kevin Wolf wrote:
> > > Enough innocent images have died because users called 'qemu-img snapshot' while
> > > the VM was still running. Educating the users doesn't seem to be a working
> > > strategy, so this series adds locking to qcow2 that refuses to access the image
> > > read-write from two processes.
> > > 
> > > Eric, this will require a libvirt update to deal with qemu crashes which leave
> > > locked images behind. The simplest thinkable way would be to unconditionally
> > > override the lock in libvirt whenever the option is present. In that case,
> > > libvirt VMs would be protected against concurrent non-libvirt accesses, but not
> > > the other way round. If you want more than that, libvirt would have to check
> > > somehow if it was its own VM that used the image and left the lock behind. I
> > > imagine that can't be too hard either.
> > 
> > The motivation is great, but I'm not sure I like the side-effect that an
> > unclean shutdown will require a "forced" open, because it makes using qcow2 in
> > development cumbersome, and like you said, management/user also needs to handle
> > this explicitly. This is a bit of a personal preference, but it's strong enough
> > that I want to speak up.
> 
> Yeah, I am also not really a big fan of locking mechanisms which are not
> automatically cleaned up on process exit. On the other hand you could
> say that people who choose to run qemu-img manually are already taking
> fate into their own hands, and ending up with a dirty image on unclean
> exit is still miles better than loosing all your data.
> 
> > As an alternative, can we introduce .bdrv_flock() in protocol drivers, with
> > similar semantics to flock(2) or lockf(3)? That way all formats can benefit,
> > and a program crash will automatically drop the lock.
> 
> FWIW, the libvirt locking daemon (virtlockd) will already attempt to take
> out locks using fcntl()/lockf() on all disk images associated with a VM.

Does this actually mean that if QEMU did try to use flock(), it would
fail because libvirt is already holding the lock?

I considered adding both locking schemes (the qcow2 flag for qcow2 on
any backend; flock() for anything else on local files), but if this is
true, that's game over for any flock() based patches.

Kevin

  parent reply	other threads:[~2016-01-11 17:14 UTC|newest]

Thread overview: 99+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-12-22 16:46 [Qemu-devel] [PATCH 00/10] qcow2: Implement image locking Kevin Wolf
2015-12-22 16:46 ` [Qemu-devel] [PATCH 01/10] qcow2: Write feature table only for v3 images Kevin Wolf
2015-12-22 20:20   ` Eric Blake
2016-01-11 15:20     ` Kevin Wolf
2015-12-22 16:46 ` [Qemu-devel] [PATCH 02/10] qcow2: Write full header on image creation Kevin Wolf
2015-12-22 20:25   ` Eric Blake
2015-12-22 16:46 ` [Qemu-devel] [PATCH 03/10] block: Assert no write requests under BDRV_O_INCOMING Kevin Wolf
2015-12-22 20:27   ` Eric Blake
2015-12-22 16:46 ` [Qemu-devel] [PATCH 04/10] block: Fix error path in bdrv_invalidate_cache() Kevin Wolf
2015-12-22 20:31   ` Eric Blake
2015-12-22 16:46 ` [Qemu-devel] [PATCH 05/10] block: Inactivate BDS when migration completes Kevin Wolf
2015-12-22 20:43   ` Eric Blake
2016-01-05 20:21     ` [Qemu-devel] [Qemu-block] " John Snow
2016-01-13 14:25       ` Kevin Wolf
2016-01-13 16:35         ` Eric Blake
2015-12-22 16:46 ` [Qemu-devel] [PATCH 06/10] qemu-img: Prepare for locked images Kevin Wolf
2015-12-22 16:57   ` Daniel P. Berrange
2015-12-22 17:00     ` Kevin Wolf
2015-12-22 21:06   ` Eric Blake
2016-01-11 15:49     ` Markus Armbruster
2016-01-11 16:05       ` Kevin Wolf
2016-01-12 15:20         ` Markus Armbruster
2016-01-12 17:36           ` Kevin Wolf
2016-01-13  8:44             ` Markus Armbruster
2016-01-13 14:19               ` Kevin Wolf
2016-01-14 13:07                 ` Markus Armbruster
2016-01-14 14:19                   ` Kevin Wolf
2016-01-11 16:22     ` Kevin Wolf
2015-12-22 21:41   ` Eric Blake
2015-12-22 16:46 ` [Qemu-devel] [PATCH 07/10] qcow2: Implement .bdrv_inactivate Kevin Wolf
2015-12-22 21:17   ` Eric Blake
2016-01-11 15:34     ` Kevin Wolf
2015-12-22 16:46 ` [Qemu-devel] [PATCH 08/10] qcow2: Fix BDRV_O_INCOMING handling in qcow2_invalidate_cache() Kevin Wolf
2015-12-22 21:22   ` Eric Blake
2015-12-22 16:46 ` [Qemu-devel] [PATCH 09/10] qcow2: Make image inaccessible after failed qcow2_invalidate_cache() Kevin Wolf
2015-12-22 21:24   ` Eric Blake
2015-12-22 16:46 ` [Qemu-devel] [PATCH 10/10] qcow2: Add image locking Kevin Wolf
2015-12-22 22:04   ` Eric Blake
2015-12-23  3:14 ` [Qemu-devel] [PATCH 00/10] qcow2: Implement " Fam Zheng
2015-12-23  7:35   ` [Qemu-devel] [Qemu-block] " Denis V. Lunev
2015-12-23  7:46     ` [Qemu-devel] [PATCH RFC 0/5] generic image locking and crash recovery Denis V. Lunev
2015-12-23  7:46       ` [Qemu-devel] [PATCH 1/5] block: added lock image option and callback Denis V. Lunev
2015-12-23 23:48         ` Eric Blake
2016-01-11 17:31         ` Kevin Wolf
2016-01-11 17:58           ` Daniel P. Berrange
2016-01-11 18:35             ` Kevin Wolf
2016-01-13  8:52               ` Markus Armbruster
2016-01-13  9:12                 ` Denis V. Lunev
2016-01-13  9:50                   ` Daniel P. Berrange
2016-01-13  9:51               ` Daniel P. Berrange
2016-01-12  5:38           ` Denis V. Lunev
2016-01-12 10:10             ` Kevin Wolf
2016-01-12 11:33               ` Fam Zheng
2016-01-12 12:24                 ` Denis V. Lunev
2016-01-12 12:28                 ` Kevin Wolf
2016-01-12 13:17                   ` Fam Zheng
2016-01-12 13:24                     ` Daniel P. Berrange
2016-01-13  0:08                       ` Fam Zheng
2016-01-12 15:59                 ` Denis V. Lunev
2016-01-13  0:10                   ` Fam Zheng
2016-01-13 16:44                     ` Eric Blake
2016-01-14  7:23                       ` Denis V. Lunev
2015-12-23  7:46       ` [Qemu-devel] [PATCH 2/5] block: implemented bdrv_lock_image for raw file Denis V. Lunev
2015-12-23 12:40         ` Daniel P. Berrange
2015-12-23  7:46       ` [Qemu-devel] [PATCH 3/5] block: added check image option and callback bdrv_is_opened_unclean Denis V. Lunev
2015-12-23  9:09         ` Fam Zheng
2015-12-23  9:14           ` Denis V. Lunev
2015-12-23  7:46       ` [Qemu-devel] [PATCH 4/5] qcow2: implemented bdrv_is_opened_unclean Denis V. Lunev
2016-01-11 17:37         ` Kevin Wolf
2015-12-23  7:46       ` [Qemu-devel] [PATCH 5/5] block/paralels: added paralles implementation for bdrv_is_opened_unclean Denis V. Lunev
2015-12-23  8:09       ` [Qemu-devel] [PATCH RFC 0/5] generic image locking and crash recovery Fam Zheng
2015-12-23  8:36         ` Denis V. Lunev
2015-12-23 10:47   ` [Qemu-devel] [PATCH 00/10] qcow2: Implement image locking Daniel P. Berrange
2015-12-23 12:15     ` [Qemu-devel] [Qemu-block] " Roman Kagan
2015-12-23 12:29       ` Daniel P. Berrange
2015-12-23 12:41         ` Denis V. Lunev
2015-12-23 12:46           ` Daniel P. Berrange
2015-12-23 12:34       ` Daniel P. Berrange
2015-12-23 12:47         ` Denis V. Lunev
2015-12-23 12:56           ` Daniel P. Berrange
2016-01-11 17:14     ` Kevin Wolf [this message]
2016-01-11 17:54       ` [Qemu-devel] " Daniel P. Berrange
2016-01-13  8:56       ` Markus Armbruster
2016-01-13  9:11         ` [Qemu-devel] [Qemu-block] " Denis V. Lunev
2015-12-23 23:19   ` [Qemu-devel] " Max Reitz
2015-12-24  5:41     ` [Qemu-devel] [Qemu-block] " Denis V. Lunev
2015-12-24  5:42       ` Denis V. Lunev
2016-01-04 17:02       ` Max Reitz
2016-01-11 16:47       ` Kevin Wolf
2016-01-11 17:56         ` Daniel P. Berrange
2015-12-23 14:57 ` [Qemu-devel] " Vasiliy Tolstov
2015-12-23 15:08   ` [Qemu-devel] [Qemu-block] " Denis V. Lunev
2015-12-23 15:11     ` Vasiliy Tolstov
2016-01-11 16:25       ` Kevin Wolf
2015-12-23 15:09   ` Denis V. Lunev
2015-12-24  5:43 ` Denis V. Lunev
2016-01-11 16:33   ` Kevin Wolf
2016-01-11 16:38     ` Denis V. Lunev
2016-01-14 14:01 ` Max Reitz

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=20160111171415.GK9454@noname.redhat.com \
    --to=kwolf@redhat.com \
    --cc=berrange@redhat.com \
    --cc=famz@redhat.com \
    --cc=mreitz@redhat.com \
    --cc=qemu-block@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    /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).