From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:33558) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1buzI4-0003Xu-7b for qemu-devel@nongnu.org; Fri, 14 Oct 2016 06:00:40 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1buzI0-0000Jj-33 for qemu-devel@nongnu.org; Fri, 14 Oct 2016 06:00:35 -0400 References: <1476380062-18001-1-git-send-email-pbonzini@redhat.com> <1476380062-18001-4-git-send-email-pbonzini@redhat.com> <20161014094353.GB14830@lemon> From: Paolo Bonzini Message-ID: <6a6dfe03-0fbf-65d7-f0c3-029e15317899@redhat.com> Date: Fri, 14 Oct 2016 12:00:18 +0200 MIME-Version: 1.0 In-Reply-To: <20161014094353.GB14830@lemon> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 03/18] mirror: use bdrv_drained_begin/bdrv_drained_end List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Fam Zheng Cc: qemu-devel@nongnu.org, qemu-block@nongnu.org, kwolf@redhat.com, stefanha@redhat.com, qemu-stable@nongnu.org On 14/10/2016 11:43, Fam Zheng wrote: > On Thu, 10/13 19:34, Paolo Bonzini wrote: >> Ensure that there are no changes between the last check to >> bdrv_get_dirty_count and the switch to the target. >> >> There is already a bdrv_drained_end call, we only need to ensure >> that bdrv_drained_begin is not called twice. >> >> Cc: qemu-stable@nongnu.org > > Cc stable? I don't see an existing bug here, can you explain? Hmm, I was not sure that mirror was safe for dataplane devices without the drained section, but it looks like there is no "hole". So no need to Cc stable. >> @@ -802,9 +812,10 @@ immediate_exit: >> >> data = g_malloc(sizeof(*data)); >> data->ret = ret; >> - /* Before we switch to target in mirror_exit, make sure data doesn't >> - * change. */ >> - bdrv_drained_begin(bs); >> + >> + if (need_drain) { > > Not sure whether this if block is necessary (i.e. when !(cnt == 0 && > should_complete)), but it certainly doesn't hurt. Yes, the alternative is to have something similar, to skip the bdrv_drained_end, in mirror_exit. Paolo >> + bdrv_drained_begin(bs); >> + } >> block_job_defer_to_main_loop(&s->common, mirror_exit, data); >> } >> >> -- >> 2.7.4 >> >> > > Fam >