[Qemu-devel] [PATCH 0/3] 512-on-4k follow-up patches

[Qemu-devel] [PATCH 0/3] 512-on-4k follow-up patches

[Kevin Wolf]

Kevin Wolf (3):
  block: Fix memory leaks in bdrv_co_do_pwritev()
  block: bdrv_aligned_pwritev: Assert overlap range
  block: Don't call ROUND_UP with negative values

 block.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

-- 
1.8.1.4

[Qemu-devel] [PATCH 1/3] block: Fix memory leaks in bdrv_co_do_pwritev()

[Kevin Wolf]

The error path for a failure in one of the two bdrv_aligned_preadv()
calls leaked head_buf or tail_buf, respectively. This fixes the memory
leak.

Reported-by: Laszlo Ersek <lersek@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 block.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/block.c b/block.c
index ac0ccac..c1d1f74 100644
--- a/block.c
+++ b/block.c
@@ -3279,9 +3279,9 @@ fail:
 
     if (use_local_qiov) {
         qemu_iovec_destroy(&local_qiov);
-        qemu_vfree(head_buf);
-        qemu_vfree(tail_buf);
     }
+    qemu_vfree(head_buf);
+    qemu_vfree(tail_buf);
 
     return ret;
 }
-- 
1.8.1.4

[Qemu-devel] [PATCH 2/3] block: bdrv_aligned_pwritev: Assert overlap range

[Kevin Wolf]

This adds assertions that the request that we actually end up passing to
the block driver (which includes RMW data and has therefore potentially
been rounded to alignment boundaries) is fully covered by the
overlap_{offset,size} fields of the associated BdrvTrackedRequest.

Suggested-by: Laszlo Ersek <lersek@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 block.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/block.c b/block.c
index c1d1f74..a027823 100644
--- a/block.c
+++ b/block.c
@@ -3134,6 +3134,8 @@ static int coroutine_fn bdrv_aligned_pwritev(BlockDriverState *bs,
 
     waited = wait_serialising_requests(req);
     assert(!waited || !req->serialising);
+    assert(req->overlap_offset <= offset);
+    assert(offset + bytes <= req->overlap_offset + req->overlap_bytes);
 
     ret = notifier_with_return_list_notify(&bs->before_write_notifiers, req);
 
-- 
1.8.1.4

[Qemu-devel] [PATCH 3/3] block: Don't call ROUND_UP with negative values

[Kevin Wolf]

The behaviour of the ROUND_UP macro with negative numbers isn't obvious.
It happens to do the right thing in this please, but better avoid it.

Suggested-by: Laszlo Ersek <lersek@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 block.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/block.c b/block.c
index a027823..b0c5025 100644
--- a/block.c
+++ b/block.c
@@ -2915,8 +2915,8 @@ static int coroutine_fn bdrv_aligned_preadv(BlockDriverState *bs,
         }
 
         total_sectors = DIV_ROUND_UP(len, BDRV_SECTOR_SIZE);
-        max_nb_sectors = MAX(0, ROUND_UP(total_sectors - sector_num,
-                                         align >> BDRV_SECTOR_BITS));
+        max_nb_sectors = ROUND_UP(MAX(0, total_sectors - sector_num),
+                                  align >> BDRV_SECTOR_BITS);
         if (max_nb_sectors > 0) {
             ret = drv->bdrv_co_readv(bs, sector_num,
                                      MIN(nb_sectors, max_nb_sectors), qiov);
-- 
1.8.1.4

Re: [Qemu-devel] [PATCH 3/3] block: Don't call ROUND_UP with negative values

[Eric Blake]

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

On 02/07/2014 09:12 AM, Kevin Wolf wrote:
> The behaviour of the ROUND_UP macro with negative numbers isn't obvious.
> It happens to do the right thing in this please, but better avoid it.

s/please/place/

> 
> Suggested-by: Laszlo Ersek <lersek@redhat.com>
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
>  block.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)

Series: Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


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

Re: [Qemu-devel] [PATCH 3/3] block: Don't call ROUND_UP with negative values

[Kevin Wolf]

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

Am 07.02.2014 um 17:27 hat Eric Blake geschrieben:
> On 02/07/2014 09:12 AM, Kevin Wolf wrote:
> > The behaviour of the ROUND_UP macro with negative numbers isn't obvious.
> > It happens to do the right thing in this please, but better avoid it.
> 
> s/please/place/

Indeed... Thanks, fixed it locally.

> > Suggested-by: Laszlo Ersek <lersek@redhat.com>
> > Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> > ---
> >  block.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> Series: Reviewed-by: Eric Blake <eblake@redhat.com>

Kevin

[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]

Re: [Qemu-devel] [PATCH 3/3] block: Don't call ROUND_UP with negative values

[Laszlo Ersek]

On 02/07/14 22:45, Kevin Wolf wrote:
> Am 07.02.2014 um 17:27 hat Eric Blake geschrieben:
>> On 02/07/2014 09:12 AM, Kevin Wolf wrote:
>>> The behaviour of the ROUND_UP macro with negative numbers isn't obvious.
>>> It happens to do the right thing in this please, but better avoid it.
>>
>> s/please/place/
> 
> Indeed... Thanks, fixed it locally.
> 
>>> Suggested-by: Laszlo Ersek <lersek@redhat.com>
>>> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
>>> ---
>>>  block.c | 4 ++--
>>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> Series: Reviewed-by: Eric Blake <eblake@redhat.com>

Not necessary after Eric's review, but here it is anyway:

series
Reviewed-by: Laszlo Ersek <lersek@redhat.com>

Thanks!
Laszlo

Re: [Qemu-devel] [PATCH 3/3] block: Don't call ROUND_UP with negative values

[Kevin Wolf]

Am 08.02.2014 um 00:24 hat Laszlo Ersek geschrieben:
> On 02/07/14 22:45, Kevin Wolf wrote:
> > Am 07.02.2014 um 17:27 hat Eric Blake geschrieben:
> >> On 02/07/2014 09:12 AM, Kevin Wolf wrote:
> >>> The behaviour of the ROUND_UP macro with negative numbers isn't obvious.
> >>> It happens to do the right thing in this please, but better avoid it.
> >>
> >> s/please/place/
> > 
> > Indeed... Thanks, fixed it locally.
> > 
> >>> Suggested-by: Laszlo Ersek <lersek@redhat.com>
> >>> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> >>> ---
> >>>  block.c | 4 ++--
> >>>  1 file changed, 2 insertions(+), 2 deletions(-)
> >>
> >> Series: Reviewed-by: Eric Blake <eblake@redhat.com>
> 
> Not necessary after Eric's review, but here it is anyway:
> 
> series
> Reviewed-by: Laszlo Ersek <lersek@redhat.com>

Review is never unnecessary. Thanks, Laszlo.

Kevin