Re: [Qemu-devel] [PATCH v2] block: don't set the same context

[Eric Blake]

On 2/15/19 7:03 AM, Denis Plotnikov wrote:
> Adds a fast path on aio context setting preventing
> unnecessary context setting routine.
> Also, it prevents issues with cyclic walk of child
> bds-es appeared because of registering aio walking
> notifiers:

> Signed-off-by: Denis Plotnikov <address@hidden>
> ---

Right here, after the ---, is a good place to list how v2 differs from
v1. It looks to me like the only change was a typo fix in the commit
message?  But that still doesn't address the concern on whether this
approach is right in the face of nesting (if attach/detach is always
paired, then attach-attach-detach-detach will cause the inner detach to
pair to the outer attach, any actions between the inner and outer detach
will not be against an attached context, and the outer detach needs to
be safe as a no-op).  The patch may still be right, but I'm not enough
of an expert in how aio attach/detach are supposed to work to know for
sure.  Or, we may need some form of reference counting, so that the
inner detach is a no-op if the inner attach was short-circuited, and the
outer detach then resumes being the proper pair against the outer
attach.  If you have checked why the patch works, then amending the
commit message to address my question will also help future readers that
might otherwise ask the same question. Posting a rapid-turnaround v2 for
just a typo fix, while not addressing the larger question raised as to
whether the patch is correct, is just churn.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org

signature.asc
Description: OpenPGP digital signature