On 18/12/19 17:37, Kevin Wolf wrote:
>> * passthrough flags. */
>> - assert(!(flags & ~(BDRV_REQ_NO_SERIALISING | BDRV_REQ_COPY_ON_READ |
>> - BDRV_REQ_PREFETCH)));
>> + assert(!(flags & ~(BDRV_REQ_COPY_ON_READ | BDRV_REQ_PREFETCH)));
>>
>> /* Handle Copy on Read and associated serialisation */
>> if (flags & BDRV_REQ_COPY_ON_READ) {
>> @@ -1458,12 +1457,7 @@ static int coroutine_fn bdrv_aligned_preadv(BdrvChild
>> *child,
>> bdrv_mark_request_serialising(req, bdrv_get_cluster_size(bs));
>> }
>>
>> - /* BDRV_REQ_SERIALISING is only for write operation */
>> - assert(!(flags & BDRV_REQ_SERIALISING));
> I think we shoud still keep this assertion as long as read requests
> don't mark themselves as serialising when BDRV_REQ_SERIALISING is given.
> Otherwise, someone might add the flag to a read request and will later
> be surprised that it didn't work.
I'm removing it because it's anyway tested by the earlier
assert(!(flags & ~(BDRV_REQ_COPY_ON_READ | BDRV_REQ_PREFETCH)));
>> @@ -3222,9 +3214,7 @@ static int coroutine_fn bdrv_co_copy_range_internal(
>>
>> /* BDRV_REQ_SERIALISING is only for write operation */
>> assert(!(read_flags & BDRV_REQ_SERIALISING));
> Here you kept the assertion, so apart from making sense anyway, it would
> also be more consistent to keep it above, too. :-)
... which is not present here.
Paolo
