From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:33350) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YsYHc-0003dF-Bz for qemu-devel@nongnu.org; Wed, 13 May 2015 11:09:20 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YsYHW-0001Sh-MJ for qemu-devel@nongnu.org; Wed, 13 May 2015 11:09:16 -0400 Sender: Paolo Bonzini Message-ID: <55536907.5060303@redhat.com> Date: Wed, 13 May 2015 17:08:55 +0200 From: Paolo Bonzini MIME-Version: 1.0 References: <1431538099-3286-1-git-send-email-famz@redhat.com> <1431538099-3286-11-git-send-email-famz@redhat.com> <555341B8.3070604@redhat.com> In-Reply-To: <555341B8.3070604@redhat.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 8bit Subject: Re: [Qemu-devel] [PATCH v2 10/11] blockdev: Block device IO during blockdev-backup transaction List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Fam Zheng , qemu-devel@nongnu.org Cc: qemu-block@nongnu.org, armbru@redhat.com, Stefan Hajnoczi , mreitz@redhat.com On 13/05/2015 16:55, Fam Zheng wrote: > > So if we consider this idea worthwhile, and decide that pausing device > > I/O on the target should pause the block job, the backup job actually > > has to prevent *adding a DEVICE_IO blocker* on the target. > > When do we need to pause device IO on the target? For drive-backup the target > is anonymous and no device exists on it, so it's out of question. For > blockdev-backup, users can have a device attached, but I'm not sure we should > pause the job in this case. Well, it depends on the meaning you give to "pause"/"device_io blocker". For me it was "I want exclusive ownership of the disk, no one else should write". For blockdev-backup for example you could mirror the backup destination somewhere else, and mirror needs to drain the source. For that to make sense, the block job has to be paused. Of course this is contrived, but it's better to make the design generic. > > > This > > > "meta-block" is not possible in your design, which is a pity because on > > > the surface it looked nicer than mine. > > > > > > FWIW, my original idea was: > > > > > > - bdrv_pause checks if there is an operation blocker for PAUSE. if it > > > is there, it fails > > It's more complicated, because bdrv_drain_all can't fail now, if we want > bdrv_pause there, then it could fail, and will become much harder to use. I think we don't need bdrv_pause there. See my reply to patch 11---we want it in the callers. > In your idea, who will block PAUSE, and why? For example, blockdev-backup/drive-backup can block PAUSE on the target. But maybe they only need to add an op blocker notifier, and use it to block device I/O on the source and drain it. So your idea is safe. Good! Another idea was to block PAUSE in virtio-scsi until it's fixed. Paolo > > > - otherwise, bdrv_pause invokes a notifier list if this is the outermost > > > call. if not the outermost call, it does nothing > > > > > > - bdrv_resume does the same, but does not need a blocker > > > > > > - drive-backup should block PAUSE on its target