Xen devel - May 2012 - qemu xen_disk mis-accounting?

ioreq_release() gets called from two places, yet only in the
blk_send_response_all() case does it appear appropriate to decrement
blkdev->requests_finished. In the error handling path of
blk_handle_requests() I would think it should decrement
blkdev->requests_inflight instead.

While blkdev->requests_finished isn''t being used anywhere,
blkdev->requests_inflight serves blk_handle_requests() to tell
whether to call qemu_bh_schedule(), so this isn''t a purely cosmetic
mistake afaics (and the error path in question is actually being
consistently exercised by our frontends probing for the packet
command extension - yes, it should have been implemented via
xenstore node presence, but the author failed to do so back then).

Jan

On Thu, 10 May 2012, Jan Beulich wrote:
> ioreq_release() gets called from two places, yet only in the
> blk_send_response_all() case does it appear appropriate to decrement
> blkdev->requests_finished. In the error handling path of
> blk_handle_requests() I would think it should decrement
> blkdev->requests_inflight instead.
>
> While blkdev->requests_finished isn''t being used anywhere,
> blkdev->requests_inflight serves blk_handle_requests() to tell
> whether to call qemu_bh_schedule(), so this isn''t a purely
cosmetic
> mistake afaics (and the error path in question is actually being
> consistently exercised by our frontends probing for the packet
> command extension - yes, it should have been implemented via
> xenstore node presence, but the author failed to do so back then).
The error handling path of blk_handle_requests() should definitely
decrement blkdev->requests_inflight for the reasons you mentioned, but
it should also call ioreq_release(), because otherwise
blk_send_response_all could call blk_send_response_one on the ioreq that
wasn''t parsed correctly.

Could you please submit a patch to add blkdev->requests_inflight-- in
the right place?