Re: [Qemu-devel] [Qemu-block] [PATCH 1/5] block: Add blklogwrites

[Ari Sundholm <ari@tuxera.com>]

On 06/07/2018 03:30 PM, Stefan Hajnoczi wrote:
> On Mon, Jun 04, 2018 at 03:10:47PM +0300, Ari Sundholm wrote:
>> On 06/04/2018 12:51 PM, Stefan Hajnoczi wrote:
>>> On Fri, Jun 01, 2018 at 05:24:53PM +0300, Ari Sundholm wrote:
>>>> On 06/01/2018 04:32 PM, Stefan Hajnoczi wrote:
>>>>> On Fri, Jun 01, 2018 at 12:17:19AM +0300, Ari Sundholm wrote:
>>>>>> From: Aapo Vienamo <aapo@tuxera.com>
>>>>>
>>>>> Thanks for the patch!
>>>>>
>>>>>> Implements a block device write logging system, similar to Linux kernel
>>>>>> device mapper dm-log-writes. The write operations that are performed
>>>>>> on a block device are logged to a file or another block device. The
>>>>>> write log format is identical to the dm-log-writes format. Currently,
>>>>>> log markers are not supported.
>>>>>>
>>>>>> This functionality can be used for fail-safe and fs consistency
>>>>>> testing. By implementing it in qemu, tests utilizing write logs can be
>>>>>> be used to test non-Linux drivers and older kernels.
>>>>>
>>>>> This patch doesn't implement the same semantics as dm-log-writes, where
>>>>> only completed writes are logged to make fs consistency testing easier.
>>>>> If you intend to use it for this purpose, shouldn't it act the same way
>>>>> as dm-log-writes?
>>>>>
>>>>
>>>> I am not quite sure what you mean. I am not the original author of this
>>>> proposed feature, but to me (admittedly with little experience of qemu
>>>> internals), it looks like the driver accurately logs the writes and flushes
>>>> performed on the guest block device. It intentionally does not concern
>>>> itself with when the write actually hits the physical host block device or
>>>> file, as we're interested in the direct interactions between a filesystem
>>>> driver and the guest block device. The write hitting the various levels of
>>>> the host-side caches and devices is left up to the caching mode. But perhaps
>>>> there's something obvious I'm not seeing?
>>>
>>> Linux dm-log-writes is specific about when logging happens:
>>>
>>>    * We log writes only after they have been flushed, this makes the log describe
>>>    * close to the order in which the data hits the actual disk, not its cache.  So
>>>    * for example the following sequence (W means write, C means complete)
>>>    *
>>>    * Wa,Wb,Wc,Cc,Ca,FLUSH,FUAd,Cb,CFLUSH,CFUAd
>>>    *
>>>    * Would result in the log looking like this:
>>>    *
>>>    * c,a,flush,fuad,b,<other writes>,<next flush>
>>>    *
>>>    * This is meant to help expose problems where file systems do not properly wait
>>>    * on data being written before invoking a FLUSH.  FUA bypasses cache so once it
>>>    * completes it is added to the log as it should be on disk.
>>>
>>> This patch implements the same on-disk format but the semantics are
>>> different since it doesn't wait for a flush.
>>>
>>> If I use dm-log-writes on a linear device-mapper target inside the guest
>>> or on the host, then I would have expected the same output as QEMU's
>>> dm-log-writes, but I think this is not the case.
>>>
>>> It's worth at least documenting this quirk.
>>
>> Oh, now I see what you mean.
>>
>> I was under the impression that when we do the logging at the level it is
>> done now, we see the actual writes to the guest block device to completion,
>> as far as the guest is concerned (being safely in the host's page cache is
>> enough for this). Is this understanding incorrect?
> 
> .bdrv_co_pwritev is called on request submission.
> blk_log_writes_co_log() spawns a coroutine for
> blk_log_writes_co_do_log(), which appends a struct log_write_entry to
> the log file independently of the other coroutine that is doing the
> actual write to the underlying file.
> 
> This means the entry could be added to the log file before the write
> request completes.
> 
> If you want to log on request completion then you cannot spawn the log
> coroutine in parallel with the write coroutine.  Instead you would have
> to complete the write and then update the log.
> 

Thank you. I can't believe I missed that...

Will be fixed in next version by making things sequential.

>> Regarding the issue of waiting for flushes, as it is now, the driver only
>> updates the superblock of the log on flush, and everything in the log
>> file/device beyond the limits set by the values in the superblock is
>> typically ignored as garbage by tools handling dm-log-writes logs. (I wonder
>> if dm-log-writes actually works similarly by exploiting this fact?)
> 
> This does work in the current patch because the log is updated before
> the write has completed.
> 
> Stefan
> 

Best regards,
Ari Sundholm
ari@tuxera.com