[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-block] [Qemu-devel] [PATCH v2 resend] block/mirror: change the
Re: [Qemu-block] [Qemu-devel] [PATCH v2 resend] block/mirror: change the semantic of 'force' of block-job-cancel
Tue, 13 Mar 2018 02:41:13 -0400
Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.6.0
On 03/12/2018 10:13 PM, Jeff Cody wrote:
> On Fri, Mar 02, 2018 at 11:20:55AM -0500, John Snow wrote:
>> On 03/02/2018 10:39 AM, Eric Blake wrote:
>>> On 02/26/2018 08:05 PM, Liang Li wrote:
>>>> When doing drive mirror to a low speed shared storage, if there was heavy
>>>> BLK IO write workload in VM after the 'ready' event, drive mirror
>>>> block job
>>>> can't be canceled immediately, it would keep running until the heavy
>>>> BLK IO
>>>> workload stopped in the VM.
>>>> Libvirt depends on the current block-job-cancel semantics, which is that
>>>> when used without a flag after the 'ready' event, the command blocks
>>>> until data is in sync. However, these semantics are awkward in other
>>>> situations, for example, people may use drive mirror for realtime
>>>> backups while still wanting to use block live migration. Libvirt cannot
>>>> start a block live migration while another drive mirror is in progress,
>>>> but the user would rather abandon the backup attempt as broken and
>>>> proceed with the live migration than be stuck waiting for the current
>>>> drive mirror backup to finish.
>>>> The drive-mirror command already includes a 'force' flag, which libvirt
>>>> does not use, although it documented the flag as only being useful to
>>>> quit a job which is paused. However, since quitting a paused job has
>>>> the same effect as abandoning a backup in a non-paused job (namely, the
>>>> destination file is not in sync, and the command completes immediately),
>>>> we can just improve the documentation to make the force flag obviously
>>> How does this interact with John's ongoing work to redo block job
>>>> Cc: Paolo Bonzini <address@hidden>
>>>> Cc: Jeff Cody <address@hidden>
>>>> Cc: Kevin Wolf <address@hidden>
>>>> Cc: Max Reitz <address@hidden>
>>>> Cc: Eric Blake <address@hidden>
>>>> Cc: John Snow <address@hidden>
>>>> Reported-by: Huaitong Han <address@hidden>
>>>> Signed-off-by: Huaitong Han <address@hidden>
>>>> Signed-off-by: Liang Li <address@hidden>
>>> But in isolation, the patch looks reasonable to me.
>>> Reviewed-by: Eric Blake <address@hidden>
>> In fairness, this patch hit the list before mine did, so...
>> I think it'll be OK to accommodate -- I think. It just changes the
>> nature of how fast the cancellation happens in mirror, which shouldn't
>> muck around with the general flow of job management in general. I think.
>> Famous last words.
> I think Kevin's pulled your series in through his branch, and this patch
> conflicts with it. Do we want to try to rebase this patch on top of your
Try the "review-reap" branch on my github.
Should just be a matter of extending the "force" flag to the user_cancel
interface and passing the bools through.