[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 4/4] qemu-img: conditionally discard target on c
From: |
Peter Lieven |
Subject: |
Re: [Qemu-devel] [PATCH 4/4] qemu-img: conditionally discard target on convert |
Date: |
Wed, 17 Jul 2013 16:19:19 +0200 |
Am 17.07.2013 um 12:47 schrieb Paolo Bonzini <address@hidden>:
> Il 17/07/2013 12:27, Kevin Wolf ha scritto:
>> Am 17.07.2013 um 12:21 hat Peter Lieven geschrieben:
>>>
>>> Am 17.07.2013 um 11:58 schrieb Paolo Bonzini <address@hidden>:
>>>
>>>> Il 17/07/2013 10:46, Kevin Wolf ha scritto:
>>>>> Am 15.07.2013 um 12:49 hat Peter Lieven geschrieben:
>>>>>> if a destination has has_zero_init = 0, but it supports
>>>>>> discard zeroes use discard to convert the target
>>>>>> into an all zero device.
>>>>>>
>>>>>> Signed-off-by: Peter Lieven <address@hidden>
>>>>>
>>>>> Wouldn't it be better to use bdrv_write_zeroes() and extend the
>>>>> implementation of that to use discard internally in those block drivers
>>>>> where it makes sense?
>>>>>
>>>>> Because here you're not really discarding (i.e. don't care about the
>>>>> sectors any more), but you want them to be zeroed.
>>>>
>>>> I thought the same yesterday when reviewing the series, but I'm not
>>>> convinced.
>>>>
>>>> Discarding is not always the right way to write zeroes, because it can
>>>> disrupt performance. It may be fine when you are already going to write
>>>> a sparse image (as is the case for qemu-img convert), but not in
>>>> general. So if you just used write_zeroes, it would have to fall under
>>>> yet another -drive option (or an extension to "-drive discard"). I
>>>> think what Peter did is a good compromise in the end.
>>>>
>>>> BTW, Peter and Ronnie: we were assuming that UNMAP with LBPRZ=1 always
>>>> zeroes blocks, but is that true for unaligned operations?
>>>
>>> Good question, I will pass it to ronnie. My guess is that the command will
>>> fail with
>>> a check condition if it failed to unmap the data. From what Ronnie sent
>>> earlier
>>> it should be guaranteed that the blocks are at least zero after the unmap
>>> command.
>>>
>>> As for the qemu-img patch this shouldn't matter. It uses always blocks of
>>> bdi->max_unmap
>>> which should be a multiple of the alignment. It also checks if sectors are
>>> deallocated
>>> after the unmap afterwards. If the unmap fails it falls back to
>>> has_zero_init =1.
>>
>> Well, you use bdrv_discard(), and ignoring discards is valid. Just
>> another reason to use bdrv_write_zeroes() instead.
>
> He's only using it if discard_zeroes is true in the new BlockDriverInfo.
> We can define the semantics of that bit, and I think defining it as
> "ignored discards will still write zeroes" is a good thing (same as what
> SCSI targets do if you use WRITE SAME to do the discard operation).
Good idea
>
> Paolo
- [Qemu-devel] [PATCH 1/4] block: add discard_zeroes and max_unmap to BlockDriverInfo, (continued)
- [Qemu-devel] [PATCH 1/4] block: add discard_zeroes and max_unmap to BlockDriverInfo, Peter Lieven, 2013/07/15
- [Qemu-devel] [PATCH 3/4] block/raw: add .bdrv_get_info, Peter Lieven, 2013/07/15
- [Qemu-devel] [PATCH 2/4] iscsi: add .bdrv_get_info, Peter Lieven, 2013/07/15
- [Qemu-devel] [PATCH 4/4] qemu-img: conditionally discard target on convert, Peter Lieven, 2013/07/15
- Re: [Qemu-devel] [PATCH 4/4] qemu-img: conditionally discard target on convert, Kevin Wolf, 2013/07/17
- Re: [Qemu-devel] [PATCH 4/4] qemu-img: conditionally discard target on convert, Paolo Bonzini, 2013/07/17
- Re: [Qemu-devel] [PATCH 4/4] qemu-img: conditionally discard target on convert, Peter Lieven, 2013/07/17
- Re: [Qemu-devel] [PATCH 4/4] qemu-img: conditionally discard target on convert, Kevin Wolf, 2013/07/17
- Re: [Qemu-devel] [PATCH 4/4] qemu-img: conditionally discard target on convert, Peter Lieven, 2013/07/17
- Re: [Qemu-devel] [PATCH 4/4] qemu-img: conditionally discard target on convert, Paolo Bonzini, 2013/07/17
- Re: [Qemu-devel] [PATCH 4/4] qemu-img: conditionally discard target on convert,
Peter Lieven <=
- Re: [Qemu-devel] [PATCH 4/4] qemu-img: conditionally discard target on convert, Paolo Bonzini, 2013/07/17
- Re: [Qemu-devel] [PATCH 4/4] qemu-img: conditionally discard target on convert, Peter Lieven, 2013/07/17
- Re: [Qemu-devel] [PATCH 4/4] qemu-img: conditionally discard target on convert, Paolo Bonzini, 2013/07/17
- Re: [Qemu-devel] [PATCH 4/4] qemu-img: conditionally discard target on convert, Kevin Wolf, 2013/07/17
- Re: [Qemu-devel] [PATCH 4/4] qemu-img: conditionally discard target on convert, ronnie sahlberg, 2013/07/17
- Re: [Qemu-devel] [PATCH 4/4] qemu-img: conditionally discard target on convert, Paolo Bonzini, 2013/07/17
- Re: [Qemu-devel] [PATCH 4/4] qemu-img: conditionally discard target on convert, ronnie sahlberg, 2013/07/17
- Re: [Qemu-devel] [PATCH 4/4] qemu-img: conditionally discard target on convert, Peter Lieven, 2013/07/17
- Re: [Qemu-devel] [PATCH 4/4] qemu-img: conditionally discard target on convert, Paolo Bonzini, 2013/07/17
- Re: [Qemu-devel] [PATCH 4/4] qemu-img: conditionally discard target on convert, Peter Lieven, 2013/07/17