[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v10 12/21] commit and mirror: create new nodes using bdrv_get
From: |
Emanuele Giuseppe Esposito |
Subject: |
Re: [PATCH v10 12/21] commit and mirror: create new nodes using bdrv_get_aio_context, and not the job aiocontext |
Date: |
Tue, 16 Aug 2022 16:57:18 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.2.0 |
Am 05/08/2022 um 10:14 schrieb Kevin Wolf:
> Am 25.07.2022 um 09:38 hat Emanuele Giuseppe Esposito geschrieben:
>> We are always using the given bs AioContext, so there is no need
>> to take the job ones (which is identical anyways).
>> This also reduces the point we need to check when protecting
>> job.aio_context field.
>>
>> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
>> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
>> Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
>
> I'm not sure against which scenario this is actually protecting. The
> only reason for not using job.aio_context seems to be if someone else
> can change the job AioContext in parallel. However, if that is the case
> (I don't think it is, but for the hypothetical case this patch seems to
> address), the AioContext is not identical any more and the change is
> wrong because we actually want things to run in the job AioContext -
> otherwise accessing the BlockBackend from the job coroutine wouldn't be
> possible.
>
> So I believe the current code expresses how we actually want to use the
> BlockBackend and the change doesn't only protect nothing, but is even
> misleading because it implies that the job can work with any AioContext,
> which is not true.
>
> Kevin
>
Ok, dropped