[Qemu-devel] [PATCH 0/4] block: Trivial fixes in offloading code

[Qemu-devel] [PATCH 0/4] block: Trivial fixes in offloading code

[Fam Zheng]

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

[Qemu-devel] [PATCH 1/4] qcow2: Drop unused cluster_data

[Fam Zheng]

Reported-by: Max Reitz <mreitz@redhat.com>
Signed-off-by: Fam Zheng <famz@redhat.com>
---
 block/qcow2.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/block/qcow2.c b/block/qcow2.c
index 2f9e58e0c4..3177c72de0 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -3338,7 +3338,6 @@ qcow2_co_copy_range_to(BlockDriverState *bs,
     int ret;
     unsigned int cur_bytes; /* number of sectors in current iteration */
     uint64_t cluster_offset;
-    uint8_t *cluster_data = NULL;
     QCowL2Meta *l2meta = NULL;
 
     assert(!bs->encrypted);
@@ -3397,7 +3396,6 @@ fail:
 
     qemu_co_mutex_unlock(&s->lock);
 
-    qemu_vfree(cluster_data);
     trace_qcow2_writev_done_req(qemu_coroutine_self(), ret);
 
     return ret;
-- 
2.17.1

[Qemu-devel] [PATCH 2/4] file-posix: Fix fd_open check in raw_co_copy_range_to

[Fam Zheng]

One of them is a typo. But update both to be more readable.

Reported-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Fam Zheng <famz@redhat.com>
---
 block/file-posix.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/block/file-posix.c b/block/file-posix.c
index 829ee538d8..e04e464e72 100644
--- a/block/file-posix.c
+++ b/block/file-posix.c
@@ -2600,7 +2600,7 @@ static int coroutine_fn raw_co_copy_range_to(BlockDriverState *bs,
     }
 
     src_s = src->bs->opaque;
-    if (fd_open(bs) < 0 || fd_open(bs) < 0) {
+    if (fd_open(src->bs) < 0 || fd_open(dst->bs) < 0) {
         return -EIO;
     }
     return paio_submit_co_full(bs, src_s->fd, src_offset, s->fd, dst_offset,
-- 
2.17.1

[Qemu-devel] [PATCH 3/4] qcow2: Drop unreachable break

[Fam Zheng]

Reported-by: Max Reitz <mreitz@redhat.com>
Signed-off-by: Fam Zheng <famz@redhat.com>
---
 block/qcow2.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/block/qcow2.c b/block/qcow2.c
index 3177c72de0..1a2e498a3c 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -3292,7 +3292,6 @@ qcow2_co_copy_range_from(BlockDriverState *bs,
         case QCOW2_CLUSTER_COMPRESSED:
             ret = -ENOTSUP;
             goto out;
-            break;
 
         case QCOW2_CLUSTER_NORMAL:
             child = bs->file;
-- 
2.17.1

[Qemu-devel] [PATCH 4/4] raw: Drop superfluous semicolon

[Fam Zheng]

Reported-by: Max Reitz <mreitz@redhat.com>
Signed-off-by: Fam Zheng <famz@redhat.com>
---
 block/raw-format.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/block/raw-format.c b/block/raw-format.c
index b78da564d4..8e648a5666 100644
--- a/block/raw-format.c
+++ b/block/raw-format.c
@@ -177,7 +177,7 @@ static inline int raw_adjust_offset(BlockDriverState *bs, uint64_t *offset,
         /* There's not enough space for the write, or the read request is
          * out-of-range. Don't read/write anything to prevent leaking out of
          * the size specified in options. */
-        return is_write ? -ENOSPC : -EINVAL;;
+        return is_write ? -ENOSPC : -EINVAL;
     }
 
     if (*offset > INT64_MAX - s->offset) {
-- 
2.17.1

Re: [Qemu-devel] [PATCH 0/4] block: Trivial fixes in offloading code

[Max Reitz]

[-- Attachment #1: Type: text/plain, Size: 1008 bytes --]

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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [Qemu-devel] [Qemu-block] [PATCH 0/4] block: Trivial fixes in offloading code

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 552 bytes --]

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>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]

Re: [Qemu-devel] [PATCH 0/4] block: Trivial fixes in offloading code

[Fam Zheng]

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

Re: [Qemu-devel] [PATCH 0/4] block: Trivial fixes in offloading code

[Max Reitz]

[-- Attachment #1: Type: text/plain, Size: 1453 bytes --]

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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]