mbox series

[0/4] block: Trivial fixes in offloading code

Message ID 20180702025836.20957-1-famz@redhat.com
Headers show
Series block: Trivial fixes in offloading code | expand

Message

Fam Zheng July 2, 2018, 2:58 a.m. UTC 
These are the low priority ones spotted by Kevin and Max last week.

Fam Zheng (4):
  qcow2: Drop unused cluster_data
  file-posix: Fix fd_open check in raw_co_copy_range_to
  qcow2: Drop unreachable break
  raw: Drop superfluous semicolon

 block/file-posix.c | 2 +-
 block/qcow2.c      | 3 ---
 block/raw-format.c | 2 +-
 3 files changed, 2 insertions(+), 5 deletions(-)

Comments

Max Reitz July 2, 2018, 12:35 p.m. UTC | #1 
On 2018-07-02 04:58, Fam Zheng wrote:
> These are the low priority ones spotted by Kevin and Max last week.
> 
> Fam Zheng (4):
>   qcow2: Drop unused cluster_data
>   file-posix: Fix fd_open check in raw_co_copy_range_to
>   qcow2: Drop unreachable break
>   raw: Drop superfluous semicolon
> 
>  block/file-posix.c | 2 +-
>  block/qcow2.c      | 3 ---
>  block/raw-format.c | 2 +-
>  3 files changed, 2 insertions(+), 5 deletions(-)

Thanks, applied to my block branch:

https://git.xanclic.moe/XanClic/qemu/commits/branch/block


Do you want to make qcow2_co_copy_range_to() do something special on
BDRV_REQ_ZERO_WRITE?  To me, it seems natural, but on the other hand
maybe it wouldn't bring anything.  If the protocol layer supports copy
offloading, then it'll probably do that zero write efficiently anyway.
If it doesn't, qemu-img convert will just fall back to the usual
implementation which involves writing zeroes when zeroes are read, so...
 What's your opinion?

Max
Stefan Hajnoczi July 2, 2018, 3:19 p.m. UTC | #2 
On Mon, Jul 02, 2018 at 10:58:32AM +0800, Fam Zheng wrote:
> These are the low priority ones spotted by Kevin and Max last week.
> 
> Fam Zheng (4):
>   qcow2: Drop unused cluster_data
>   file-posix: Fix fd_open check in raw_co_copy_range_to
>   qcow2: Drop unreachable break
>   raw: Drop superfluous semicolon
> 
>  block/file-posix.c | 2 +-
>  block/qcow2.c      | 3 ---
>  block/raw-format.c | 2 +-
>  3 files changed, 2 insertions(+), 5 deletions(-)
> 
> -- 
> 2.17.1
> 
> 

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Fam Zheng July 3, 2018, 1:26 a.m. UTC | #3 
On Mon, 07/02 14:35, Max Reitz wrote:
> On 2018-07-02 04:58, Fam Zheng wrote:
> > These are the low priority ones spotted by Kevin and Max last week.
> > 
> > Fam Zheng (4):
> >   qcow2: Drop unused cluster_data
> >   file-posix: Fix fd_open check in raw_co_copy_range_to
> >   qcow2: Drop unreachable break
> >   raw: Drop superfluous semicolon
> > 
> >  block/file-posix.c | 2 +-
> >  block/qcow2.c      | 3 ---
> >  block/raw-format.c | 2 +-
> >  3 files changed, 2 insertions(+), 5 deletions(-)
> 
> Thanks, applied to my block branch:
> 
> https://git.xanclic.moe/XanClic/qemu/commits/branch/block
> 
> 
> Do you want to make qcow2_co_copy_range_to() do something special on
> BDRV_REQ_ZERO_WRITE?  To me, it seems natural, but on the other hand
> maybe it wouldn't bring anything.  If the protocol layer supports copy
> offloading, then it'll probably do that zero write efficiently anyway.
> If it doesn't, qemu-img convert will just fall back to the usual
> implementation which involves writing zeroes when zeroes are read, so...
>  What's your opinion?

In bdrv_co_copy_range_internal() there is

    if (flags & BDRV_REQ_ZERO_WRITE) {
        return bdrv_co_pwrite_zeroes(dst, dst_offset, bytes, flags);
    }

before calling driver .bdrv_co_copy_range_to() callback. I think this is enough?

Fam
Max Reitz July 4, 2018, 2:09 p.m. UTC | #4 
On 2018-07-03 03:26, Fam Zheng wrote:
> On Mon, 07/02 14:35, Max Reitz wrote:
>> On 2018-07-02 04:58, Fam Zheng wrote:
>>> These are the low priority ones spotted by Kevin and Max last week.
>>>
>>> Fam Zheng (4):
>>>   qcow2: Drop unused cluster_data
>>>   file-posix: Fix fd_open check in raw_co_copy_range_to
>>>   qcow2: Drop unreachable break
>>>   raw: Drop superfluous semicolon
>>>
>>>  block/file-posix.c | 2 +-
>>>  block/qcow2.c      | 3 ---
>>>  block/raw-format.c | 2 +-
>>>  3 files changed, 2 insertions(+), 5 deletions(-)
>>
>> Thanks, applied to my block branch:
>>
>> https://git.xanclic.moe/XanClic/qemu/commits/branch/block
>>
>>
>> Do you want to make qcow2_co_copy_range_to() do something special on
>> BDRV_REQ_ZERO_WRITE?  To me, it seems natural, but on the other hand
>> maybe it wouldn't bring anything.  If the protocol layer supports copy
>> offloading, then it'll probably do that zero write efficiently anyway.
>> If it doesn't, qemu-img convert will just fall back to the usual
>> implementation which involves writing zeroes when zeroes are read, so...
>>  What's your opinion?
> 
> In bdrv_co_copy_range_internal() there is
> 
>     if (flags & BDRV_REQ_ZERO_WRITE) {
>         return bdrv_co_pwrite_zeroes(dst, dst_offset, bytes, flags);
>     }
> 
> before calling driver .bdrv_co_copy_range_to() callback. I think this is enough?

Ah, right...  Yep, that's enough. :-)

Max