On 2018-02-01 14:34, Anton Nefedov wrote: > On 31/1/2018 8:31 PM, Max Reitz wrote: >> On 2018-01-30 13:34, Anton Nefedov wrote: >>> Offtop: does REQ_ZERO_WRITE override REQ_WRITE_COMPRESSED in this >>> function? at least with !REQ_MAY_UNMAP it looks wrong >> >> Looks like zero detection will indeed override compression. I think >> that was intended, but I don't even have an opinion either way. >> >> Of course, it wouldn't be so nice if you tried to compress something and >> then, because the zero write failed, you actually write uncompressed >> zeroes... But since zero detection is an optional feature, it might be >> your own fault if you enable it when you want compression anyway, and if >> you write to some format/protocol combination that doesn't allow zero >> writes. >> >> Max >> > > Is it detection only? I guess in case the caller sets > (REQ_ZERO_WRITE | REQ_COMPRESSED) it also fires here.
True, but that's the caller's "fault". One of those things has to take precedence. And I've just noticed that when bdrv_co_do_pwrite_zeroes() falls back to a bounce buffer, it passes the original flags (without BDRV_REQ_ZERO_WRITE) to bdrv_driver_pwritev(), so compression will take effect then. So the only result is that we first try a zero write, and if that fails, we do a compressed write. That sounds reasonable to me. (I mean, we could do it the other way around, but I firstly I don't think it matters much and secondly it's probably better this way because I'd suspect zero writes to be more efficient than compressing the bounce buffer; at least that's how it is for qcow2.) Max > Looks like noone does that though, but for example backup might; > it supports compression, and it also does a zero detection itself and > calls write_zeroes() when it can. > It sets REQ_MAY_UNMAP which should indeed override a compression, but > unmap might be not supported. > > /Anton
signature.asc
Description: OpenPGP digital signature
