Re: [Qemu-block] [Qemu-devel] [RFC PATCH 27/56] block/dirty-bitmap: Clean up signed vs. unsigned dirty counts

2017-08-07 Thread John Snow
On 08/07/2017 10:45 AM, Markus Armbruster wrote: > hbitmap_count() returns uint64_t. > > Clean up test-hbitmap.c to check its value with g_assert_cmpuint() > instead of g_assert_cmpint(). > > bdrv_get_dirty_count() and bdrv_get_meta_dirty_count() return its > value converted to int64_t. Clean

Re: [Qemu-block] [Qemu-devel] [RFC PATCH 29/56] block: Make BlockDirtyInfo byte count unsigned in QAPI/QMP

2017-08-07 Thread John Snow
On 08/07/2017 10:45 AM, Markus Armbruster wrote: > Byte counts should use QAPI type 'size' (uint64_t). BlockDirtyInfo > member @count is 'int' (int64_t). bdrv_query_dirty_bitmaps() computes > @count from bdrv_get_dirty_count() in uint64_t, then implicitly > converts to int64_t. Before the

Re: [Qemu-block] [PATCH] block: document semanatics of bdrv_co_preadv|pwritev

2017-08-07 Thread Fam Zheng
On Fri, 08/04 16:49, Daniel P. Berrange wrote: > This is odd. In the bdrv_aligned_readv() it looks very much like > we'll reference qiov->niov, if bytes != 0, so if qiov was NULL we > would crash. It doesn't make sense if read doesn't have an iov, where should the data be placed? :) > > In

Re: [Qemu-block] [Qemu-devel] [RFC PATCH 28/56] block: Widen dirty bitmap granularity to uint64_t for safety

2017-08-07 Thread John Snow
On 08/07/2017 10:45 AM, Markus Armbruster wrote: > Block dirty bitmaps represent granularity in bytes as uint32_t. It > must be a power of two and a multiple of BDRV_SECTOR_SIZE. > > The trouble with uint32_t is computations like this one in > mirror_do_read(): > > uint64_t max_bytes; >

Re: [Qemu-block] Is the use of bdrv_getlength() in qcow.c kosher?

2017-08-07 Thread Eric Blake
On 08/04/2017 10:32 AM, Eric Blake wrote: > On 08/04/2017 07:45 AM, Markus Armbruster wrote: >> Markus Armbruster writes: >> >>> bdrv_getlength() can fail. The uses in qcow.c don't check. Is that >>> safe? >> >> There's another one in qcow2_co_pwritev_compressed(). >> >> Yet

Re: [Qemu-block] [Qemu-devel] [PATCH 1/4] vpc: Check failure of bdrv_getlength()

2017-08-07 Thread Philippe Mathieu-Daudé
On 08/07/2017 05:30 PM, Eric Blake wrote: vpc_open() was checking for bdrv_getlength() failure in one, but not the other, location. Reported-by: Markus Armbruster Signed-off-by: Eric Blake Reviewed-by: Philippe Mathieu-Daudé ---

Re: [Qemu-block] [Qemu-devel] [PATCH 2/4] qcow: Check failure of bdrv_getlength() and bdrv_truncate()

2017-08-07 Thread Philippe Mathieu-Daudé
On 08/07/2017 05:30 PM, Eric Blake wrote: This also requires changing the return type of get_cluster_offset() and adjusting all callers. Use osdep.h macros instead of open-coded rounding while in the area. Reported-by: Markus Armbruster Signed-off-by: Eric Blake

Re: [Qemu-block] [Qemu-devel] [PATCH for-2.10] throttle: Make LeakyBucket.avg and LeakyBucket.max integer types

2017-08-07 Thread Eric Blake
On 08/07/2017 11:15 AM, Alberto Garcia wrote: > Both the throttling limits set with the throttling.iops-* and > throttling.bps-* options and their QMP equivalents defined in the > BlockIOThrottle struct are integer values. > > Those limits are also reported in the BlockDeviceInfo struct and they

Re: [Qemu-block] [PATCH 1/4] vpc: Check failure of bdrv_getlength()

2017-08-07 Thread Jeff Cody
On Mon, Aug 07, 2017 at 03:30:04PM -0500, Eric Blake wrote: > vpc_open() was checking for bdrv_getlength() failure in one, but > not the other, location. > > Reported-by: Markus Armbruster > Signed-off-by: Eric Blake > --- > block/vpc.c | 9 - > 1

Re: [Qemu-block] [PATCH 2/4] qcow: Check failure of bdrv_getlength() and bdrv_truncate()

2017-08-07 Thread Jeff Cody
On Mon, Aug 07, 2017 at 03:30:05PM -0500, Eric Blake wrote: > This also requires changing the return type of get_cluster_offset() > and adjusting all callers. > > Use osdep.h macros instead of open-coded rounding while in the > area. > > Reported-by: Markus Armbruster >

Re: [Qemu-block] [Qemu-devel] [PATCH 3/4] qcow2: Drop debugging dump_refcounts()

2017-08-07 Thread Philippe Mathieu-Daudé
On 08/07/2017 05:30 PM, Eric Blake wrote: It's been #if 0'd since its introduction in 2006, commit 585f8587. We can revive dead code if we need it, but in the meantime, it has bit-rotted (for example, not checking for failure in bdrv_getlength()). Signed-off-by: Eric Blake

[Qemu-block] [PATCH for-2.10] block/nfs: fix mutex assertion in nfs_file_close()

2017-08-07 Thread Jeff Cody
Commit c096358e747e88fc7364e40e3c354ee0bb683960 introduced assertion checks for when qemu_mutex() functions are called without the corresponding qemu_mutex_init() having initialized the mutex. This uncovered a latent bug in qemu's nfs driver - in nfs_client_close(), the NFSClient structure is

[Qemu-block] [PATCH 4/4] qcow2: Check failure of bdrv_getlength()

2017-08-07 Thread Eric Blake
qcow2_co_pwritev_compressed() should not call bdrv_truncate() if determining the size failed. Reported-by: Markus Armbruster Signed-off-by: Eric Blake --- block/qcow2.c | 5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/block/qcow2.c

[Qemu-block] [PATCH 2/4] qcow: Check failure of bdrv_getlength() and bdrv_truncate()

2017-08-07 Thread Eric Blake
This also requires changing the return type of get_cluster_offset() and adjusting all callers. Use osdep.h macros instead of open-coded rounding while in the area. Reported-by: Markus Armbruster Signed-off-by: Eric Blake --- block/qcow.c | 64

[Qemu-block] [PATCH 3/4] qcow2: Drop debugging dump_refcounts()

2017-08-07 Thread Eric Blake
It's been #if 0'd since its introduction in 2006, commit 585f8587. We can revive dead code if we need it, but in the meantime, it has bit-rotted (for example, not checking for failure in bdrv_getlength()). Signed-off-by: Eric Blake --- block/qcow2.c | 21 -

[Qemu-block] [PATCH for-2.10 0/4] More blk_getlength() fixes

2017-08-07 Thread Eric Blake
Thanks again to Markus for catching these spots. Eric Blake (4): vpc: Check failure of bdrv_getlength() qcow: Check failure of bdrv_getlength() and bdrv_truncate() qcow2: Drop debugging dump_refcounts() qcow2: Check failure of bdrv_getlength() block/qcow.c | 64

[Qemu-block] [PATCH 1/4] vpc: Check failure of bdrv_getlength()

2017-08-07 Thread Eric Blake
vpc_open() was checking for bdrv_getlength() failure in one, but not the other, location. Reported-by: Markus Armbruster Signed-off-by: Eric Blake --- block/vpc.c | 9 - 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/block/vpc.c

Re: [Qemu-block] [PATCH for-2.10] block/nfs: fix mutex assertion in nfs_file_close()

2017-08-07 Thread John Snow
On 08/07/2017 06:29 PM, Jeff Cody wrote: > Commit c096358e747e88fc7364e40e3c354ee0bb683960 introduced assertion > checks for when qemu_mutex() functions are called without the > corresponding qemu_mutex_init() having initialized the mutex. > > This uncovered a latent bug in qemu's nfs driver -

Re: [Qemu-block] [Qemu-devel] [PATCH 4/4] qcow2: Check failure of bdrv_getlength()

2017-08-07 Thread Philippe Mathieu-Daudé
On 08/07/2017 05:30 PM, Eric Blake wrote: qcow2_co_pwritev_compressed() should not call bdrv_truncate() if determining the size failed. Reported-by: Markus Armbruster Signed-off-by: Eric Blake Reviewed-by: Philippe Mathieu-Daudé ---

Re: [Qemu-block] [PATCH 4/4] qcow2: Check failure of bdrv_getlength()

2017-08-07 Thread Jeff Cody
On Mon, Aug 07, 2017 at 03:30:07PM -0500, Eric Blake wrote: > qcow2_co_pwritev_compressed() should not call bdrv_truncate() > if determining the size failed. > > Reported-by: Markus Armbruster > Signed-off-by: Eric Blake Reviewed-by: Jeff Cody

Re: [Qemu-block] [PATCH for-2.10 0/5] block: bdrv_reopen() fixes

2017-08-07 Thread John Snow
On 08/03/2017 11:02 AM, Kevin Wolf wrote: > This is the first part of some fixes to bdrv_reopen(), which seems > reasonable enough to merge for 2.10. > > There is much more wrong with bdrv_reopen() currently, especially with > respect to op blocker permissions (basically the required

Re: [Qemu-block] [PATCH for-2.10 0/4] More blk_getlength() fixes

2017-08-07 Thread John Snow
On 08/07/2017 04:30 PM, Eric Blake wrote: > Thanks again to Markus for catching these spots. > > Eric Blake (4): > vpc: Check failure of bdrv_getlength() > qcow: Check failure of bdrv_getlength() and bdrv_truncate() > qcow2: Drop debugging dump_refcounts() > qcow2: Check failure of

Re: [Qemu-block] [PATCH 2/4] qcow: Check failure of bdrv_getlength() and bdrv_truncate()

2017-08-07 Thread Jeff Cody
On Mon, Aug 07, 2017 at 03:30:05PM -0500, Eric Blake wrote: > This also requires changing the return type of get_cluster_offset() > and adjusting all callers. > > Use osdep.h macros instead of open-coded rounding while in the > area. > > Reported-by: Markus Armbruster >

Re: [Qemu-block] [PATCH 3/4] qcow2: Drop debugging dump_refcounts()

2017-08-07 Thread Jeff Cody
On Mon, Aug 07, 2017 at 03:30:06PM -0500, Eric Blake wrote: > It's been #if 0'd since its introduction in 2006, commit 585f8587. > We can revive dead code if we need it, but in the meantime, it has > bit-rotted (for example, not checking for failure in bdrv_getlength()). > > Signed-off-by: Eric

Re: [Qemu-block] [Qemu-devel] [PATCH 1/2] block/vhdx: check error return of bdrv_getlength()

2017-08-07 Thread Eric Blake
On 08/06/2017 10:08 PM, Jeff Cody wrote: > Calls to bdrv_getlength() were not checking for error. In vhdx.c, this > can lead to truncating an image file, so it is a definite bug. In > vhdx-log.c, the path for improper behavior is less clear, but it is best > to check in any case. > >

Re: [Qemu-block] [Qemu-devel] [PATCH for-2.10] quorum: Handle bdrv_getlength() failures in quorum_co_flush()

2017-08-07 Thread Eric Blake
On 08/07/2017 03:43 AM, Alberto Garcia wrote: > On Fri 04 Aug 2017 05:44:00 PM CEST, Eric Blake wrote: >>> --- a/block/quorum.c >>> +++ b/block/quorum.c >>> @@ -785,8 +785,9 @@ static coroutine_fn int >>> quorum_co_flush(BlockDriverState *bs) >>> for (i = 0; i < s->num_children; i++) { >>>

Re: [Qemu-block] [PATCH 02/17] nbd/client: refactor nbd_read_eof

2017-08-07 Thread Vladimir Sementsov-Ogievskiy
07.08.2017 14:42, Eric Blake wrote: On 08/04/2017 10:14 AM, Vladimir Sementsov-Ogievskiy wrote: Refactor nbd_read_eof to return 1 on success, 0 on eof, when no data was read and <0 for other cases, because returned size of read data is not actually used. Signed-off-by: Vladimir

Re: [Qemu-block] [PATCH v4 08/15] qcow2: set inactive flag

2017-08-07 Thread Anton Nefedov
On 08/04/2017 11:00 PM, Eric Blake wrote: On 08/01/2017 09:19 AM, Anton Nefedov wrote: Qcow2State and BlockDriverState flags have to be in sync Signed-off-by: Anton Nefedov --- block/qcow2.c | 1 + 1 file changed, 1 insertion(+) Is this a bug fix needed for

[Qemu-block] [PATCH for-2.10] quorum: Set sectors-count to 0 when reporting a flush error

2017-08-07 Thread Alberto Garcia
The QUORUM_REPORT_BAD event has fields to report the sector in which the error was detected and the number of affected sectors starting from that one. This is important for read and write errors, but not for flush errors. For flush errors the current code reports the total size of the disk image.

[Qemu-block] [PATCH v2 for-2.10 4/4] block/vhdx: check error return of bdrv_truncate()

2017-08-07 Thread Jeff Cody
Signed-off-by: Jeff Cody --- block/vhdx-log.c | 6 +- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/block/vhdx-log.c b/block/vhdx-log.c index a27dc05..14b724e 100644 --- a/block/vhdx-log.c +++ b/block/vhdx-log.c @@ -558,7 +558,11 @@ static int

Re: [Qemu-block] [PATCH for-2.10] quorum: Set sectors-count to 0 when reporting a flush error

2017-08-07 Thread Eric Blake
On 08/07/2017 07:36 AM, Alberto Garcia wrote: > The QUORUM_REPORT_BAD event has fields to report the sector in which > the error was detected and the number of affected sectors starting > from that one. This is important for read and write errors, but not > for flush errors. > > For flush errors

Re: [Qemu-block] [PATCH 01/17] nbd/client: fix nbd_opt_go

2017-08-07 Thread Eric Blake
On 08/04/2017 10:14 AM, Vladimir Sementsov-Ogievskiy wrote: > Do not send NBD_OPT_ABORT to the broken server. After sending > NBD_REP_ACK on NBD_OPT_GO server is most probably in transmission > phase, when option sending is finished. > > Signed-off-by: Vladimir Sementsov-Ogievskiy

Re: [Qemu-block] [PATCH 02/17] nbd/client: refactor nbd_read_eof

2017-08-07 Thread Eric Blake
On 08/04/2017 10:14 AM, Vladimir Sementsov-Ogievskiy wrote: > Refactor nbd_read_eof to return 1 on success, 0 on eof, when no > data was read and <0 for other cases, because returned size of > read data is not actually used. > > Signed-off-by: Vladimir Sementsov-Ogievskiy

Re: [Qemu-block] [Qemu-devel] [PATCH 2/2] block/vhdx: check for offset overflow to bdrv_truncate()

2017-08-07 Thread Jeff Cody
On Mon, Aug 07, 2017 at 06:24:30AM -0500, Eric Blake wrote: > On 08/06/2017 10:08 PM, Jeff Cody wrote: > > VHDX uses uint64_t types for most offsets, following the VHDX spec. > > However, bdrv_truncate() takes an int64_t value for the truncating > > offset. Check for overflow before calling

Re: [Qemu-block] [PATCH 1/2] block/vhdx: check error return of bdrv_getlength()

2017-08-07 Thread Jeff Cody
On Mon, Aug 07, 2017 at 12:46:30PM +0200, Kevin Wolf wrote: > Am 07.08.2017 um 05:08 hat Jeff Cody geschrieben: > > Calls to bdrv_getlength() were not checking for error. In vhdx.c, this > > can lead to truncating an image file, so it is a definite bug. In > > vhdx-log.c, the path for improper

[Qemu-block] [PATCH v2 for-2.10 0/4] VHDX bugfixes

2017-08-07 Thread Jeff Cody
Some VHDX bug fixes, including: 1. Checking bdrv_getlength(), bdrv_flush(), and bdrv_truncate() return values Changes from v1->v2: git-backport-diff -r qemu/master.. -u vhdx-fixes-v1 Key: [] : patches are identical [] : number of functional differences between upstream/downstream patch

Re: [Qemu-block] [Qemu-devel] [PATCH 2/2] block/vhdx: check for offset overflow to bdrv_truncate()

2017-08-07 Thread Eric Blake
On 08/06/2017 10:08 PM, Jeff Cody wrote: > VHDX uses uint64_t types for most offsets, following the VHDX spec. > However, bdrv_truncate() takes an int64_t value for the truncating > offset. Check for overflow before calling bdrv_truncate(). > > N.B.: For a compliant image this is not an issue,

Re: [Qemu-block] [Qemu-devel] [PATCH 04/17] nbd/client: fix nbd_send_request to return int

2017-08-07 Thread Eric Blake
On 08/07/2017 03:57 AM, Vladimir Sementsov-Ogievskiy wrote: > 07.08.2017 11:23, Daniel P. Berrange wrote: >> On Fri, Aug 04, 2017 at 06:14:27PM +0300, Vladimir Sementsov-Ogievskiy >> wrote: >>> Fix nbd_send_request to return int, as it returns a return value >>> of nbd_write (which is int), and

Re: [Qemu-block] [PATCH 06/17] block/nbd-client: fix nbd_read_reply_entry

2017-08-07 Thread Eric Blake
On 08/04/2017 10:14 AM, Vladimir Sementsov-Ogievskiy wrote: > Set reply.handle to 0 on error path to prevent normal path of > nbd_co_receive_reply. > > Signed-off-by: Vladimir Sementsov-Ogievskiy > --- > block/nbd-client.c | 1 + > 1 file changed, 1 insertion(+) Can

Re: [Qemu-block] [PATCH 11/17] block/nbd-client: fix nbd_co_request: set s->reply.handle to 0 on error

2017-08-07 Thread Eric Blake
On 08/04/2017 10:14 AM, Vladimir Sementsov-Ogievskiy wrote: > We set s->reply.handle to 0 on one error path and don't set on another. > For consistancy and to avoid assert in nbd_read_reply_entry let's > set s->reply.handle to 0 in case of wrong handle too. > > Signed-off-by: Vladimir

Re: [Qemu-block] [Qemu-devel] [PATCH for-2.10] quorum: Handle bdrv_getlength() failures in quorum_co_flush()

2017-08-07 Thread Alberto Garcia
On Mon 07 Aug 2017 01:29:09 PM CEST, Eric Blake wrote: > On 08/07/2017 03:43 AM, Alberto Garcia wrote: >> On Fri 04 Aug 2017 05:44:00 PM CEST, Eric Blake wrote: --- a/block/quorum.c +++ b/block/quorum.c @@ -785,8 +785,9 @@ static coroutine_fn int

Re: [Qemu-block] [PATCH v4 02/15] blkverify: set supported write/zero flags

2017-08-07 Thread Anton Nefedov
On 08/04/2017 06:23 PM, Eric Blake wrote: On 08/01/2017 09:18 AM, Anton Nefedov wrote: Signed-off-by: Anton Nefedov --- block/blkverify.c | 9 + 1 file changed, 9 insertions(+) Basically, blkverify supports a flag if BOTH of its underlying files also

Re: [Qemu-block] [PATCH V8 2/6] qmp: Create IOThrottle structure

2017-08-07 Thread Eric Blake
On 08/07/2017 07:37 AM, Pradeep Jagadeesh wrote: > This patch enables qmp interfaces for the fsdev > devices. This provides two interfaces one > for querying info of all the fsdev devices. The second one > to set the IO limits for the required fsdev device. > > Signed-off-by: Pradeep Jagadeesh

[Qemu-block] [PATCH v2 for-2.10 2/4] block/vhdx: check for offset overflow to bdrv_truncate()

2017-08-07 Thread Jeff Cody
VHDX uses uint64_t types for most offsets, following the VHDX spec. However, bdrv_truncate() takes an int64_t value for the truncating offset. Check for overflow before calling bdrv_truncate(). While we are here, replace the bit shifting with QEMU_ALIGN_UP as well. N.B.: For a compliant image

Re: [Qemu-block] [Qemu-devel] [PATCH for-2.10] iotests: fix 185

2017-08-07 Thread Eric Blake
On 08/07/2017 09:16 AM, Vladimir Sementsov-Ogievskiy wrote: > 185 iotest is broken. > > How to test: >> i=0; while ./check -qcow2 -nocache 185; do ((i+=1)); echo N = $i; \ > done; echo N = $i > > finished for me like this: > > 185 2s ... - output mismatch (see 185.out.bad) > ---

[Qemu-block] [RFC PATCH 02/56] qdict: New helpers to put and get unsigned integers

2017-08-07 Thread Markus Armbruster
Signed-off-by: Markus Armbruster --- include/qapi/qmp/qdict.h | 5 + qobject/qdict.c | 43 --- 2 files changed, 41 insertions(+), 7 deletions(-) diff --git a/include/qapi/qmp/qdict.h b/include/qapi/qmp/qdict.h index

[Qemu-block] [RFC PATCH 04/56] char: Make ringbuf-read size unsigned in QAPI/QMP

2017-08-07 Thread Markus Armbruster
Sizes should use QAPI type 'size' (uint64_t). ringbuf-read parameter @size is 'int' (int64_t). qmp_ringbuf_read() rejects negative values, then implicitly converts to size_t. Change the parameter to 'size' and drop the check for negative values. ringbuf-read now accepts size values between

[Qemu-block] [RFC PATCH 01/56] qobject: Touch up comments to say @param instead of 'param'

2017-08-07 Thread Markus Armbruster
Signed-off-by: Markus Armbruster --- qobject/qdict.c | 68 - qobject/qlist.c | 2 +- 2 files changed, 35 insertions(+), 35 deletions(-) diff --git a/qobject/qdict.c b/qobject/qdict.c index 576018e..d795079 100644 ---

[Qemu-block] [RFC PATCH 03/56] monitor: Rewrite comment describing HMP .args_type

2017-08-07 Thread Markus Armbruster
Signed-off-by: Markus Armbruster --- monitor.c | 75 +++ 1 file changed, 47 insertions(+), 28 deletions(-) diff --git a/monitor.c b/monitor.c index e0f8801..8b54ba1 100644 --- a/monitor.c +++ b/monitor.c @@ -85,37

[Qemu-block] [RFC PATCH 16/56] migration: Make XBZRLE transferred size unsigned in QAPI/QMP

2017-08-07 Thread Markus Armbruster
Sizes should use QAPI type 'size' (uint64_t). XBZRLECacheStats member @bytes is 'int' (int64_t). save_xbzrle_page() computes the byte count increment in size_t, implicitly converts it to int, then adds that to @bytes. Change the XBZRLECacheStats member to 'size' and clean up save_xbzrle_page().

[Qemu-block] [RFC PATCH 22/56] block: Mix up signed and unsigned less in bdrv_img_create()

2017-08-07 Thread Markus Armbruster
@size is declared int64_t. It's set in two places. The second one assigns the (signed) value of bdrv_getlength(), then errors out if its negative. The first one assigns qemu_opt_get_size(opts, BLOCK_OPT_SIZE, 0), i.e. an uint64_t value. What if it exceeds INT64_MAX? Is that even possible?

[Qemu-block] [RFC PATCH 07/56] cpus: Make memsave, pmemsave sizes, addresses unsigned in QAPI/QMP

2017-08-07 Thread Markus Armbruster
Sizes, virtual and physical addresses should use QAPI type 'size' (uint64_t). memsave, pmemsave parameters @val, @size are 'int' (int64_t). qmp_memsave() and qmp_pmemsave() implicitly convert to target_ulong or hwaddr. Change the parameters to 'size'. Both commands now accept size and address

[Qemu-block] [RFC PATCH 11/56] monitor: Drop unused HMP .args_type 'M'

2017-08-07 Thread Markus Armbruster
The previous commit switched balloon from 'M' to 'o', rendering 'M' unused. It was never used for anything else. Drop it. Signed-off-by: Markus Armbruster --- monitor.c | 11 +-- 1 file changed, 1 insertion(+), 10 deletions(-) diff --git a/monitor.c b/monitor.c

[Qemu-block] [RFC PATCH 00/56] qapi: Use 'size' for byte counts & offsets

2017-08-07 Thread Markus Armbruster
Byte sizes, offsets and the like should use QAPI type 'size' (uint64_t). This rule is more honored in the breach than in the observance. Fix the obvious offenders. The series is RFC for at least two reasons: 1. It's only lightly tested. Commit message claims like "FOO now works" haven't

[Qemu-block] [RFC PATCH 05/56] char: Make ringbuf size unsigned in QAPI

2017-08-07 Thread Markus Armbruster
Sizes should use QAPI type 'size' (uint64_t). ChardevRingbuf member @size is 'int' (int64_t). Doesn't really matter, as its users chardev-add and chardev-change manually reject sizes that aren't powers of two. Change the ChardevRingbuf member to 'size' anyway. Signed-off-by: Markus Armbruster

[Qemu-block] [RFC PATCH 17/56] migration: Make MigrationStats sizes unsigned in QAPI/QMP

2017-08-07 Thread Markus Armbruster
Sizes should use QAPI type 'size' (uint64_t). MigrationStats members @transferred, @remaining, @total, @normal-bytes, @page-size are 'int' (int64_t). populate_ram_info(), populate_disk_info() and and many places that update them in global variable @ram_counters implicitly convert from unsigned

[Qemu-block] [RFC PATCH 35/56] blockjob: Lift speed sign conversion into block_job_set_speed()

2017-08-07 Thread Markus Armbruster
The BlockJob abstraction takes int64_t speed. The underlying RateLimit abstraction takes uint64_t. We convert from int64_t to uint64_t in the BlockJobDriver speed_set() methods. They all reject negative speed. Lift this check and conversion up into the method's caller block_job_set_speed().

[Qemu-block] [RFC PATCH 06/56] char: Don't truncate -chardev and HMP chardev-add ringbuf size

2017-08-07 Thread Markus Armbruster
qemu_chr_parse_ringbuf() initializes the new ChardevRingbuf's @size to the value of qemu_opt_get_size(). Except it first truncates the value from uint64_t to int. Fix that, so you can waste your RAM on multi-gigabyte ring buffers. Signed-off-by: Markus Armbruster ---

[Qemu-block] [RFC PATCH 39/56] blockjob: Lift speed sign conversion out of block_job_create()

2017-08-07 Thread Markus Armbruster
block_job_create() takes int64_t speed. The underlying RateLimit abstraction takes uint64_t. block_job_create() converts from int64_t to uint64_t, rejecting negative speed. Lift this check and conversion out of block_job_create() into its callers. I'm going to lift it further until it falls

[Qemu-block] [RFC PATCH 24/56] block/qcow2: Change align_offset() to operate on uint64_t

2017-08-07 Thread Markus Armbruster
align_offset() mixes different widths, and its callers pass both signed and unsigned values. It's best to stick to unsigned when twiddling bits. Signed-off-by: Markus Armbruster --- block/qcow2.h | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git

[Qemu-block] [RFC PATCH 28/56] block: Widen dirty bitmap granularity to uint64_t for safety

2017-08-07 Thread Markus Armbruster
Block dirty bitmaps represent granularity in bytes as uint32_t. It must be a power of two and a multiple of BDRV_SECTOR_SIZE. The trouble with uint32_t is computations like this one in mirror_do_read(): uint64_t max_bytes; max_bytes = s->granularity * s->max_iov; The operands of * are

[Qemu-block] [RFC PATCH 12/56] pc-dimm: Make size and address unsigned in QAPI/QMP

2017-08-07 Thread Markus Armbruster
Sizes and addresses should use QAPI type 'size' (uint64_t). PCDIMMDeviceInfo members @addr and @size are 'int' (int64_t). qmp_pc_dimm_device_list() implicitly converts from uint64_t. Change these PCDIMMDeviceInfo members to 'size'. query-memory-devices now reports sizes and addresses above

[Qemu-block] [RFC PATCH 43/56] blockjob: Lift speed sign conversion out of mirror_start()

2017-08-07 Thread Markus Armbruster
mirror_start() takes int64_t speed. The underlying BlockJob abstraction takes uint64_t. mirror_start() converts from int64_t to uint64_t, rejecting negative speed. Lift this check and conversion out of mirror_start() into its caller. I'm going to lift it further until it falls off the top.

[Qemu-block] [RFC PATCH 19/56] block: Make snapshot VM state size unsigned in QAPI/QMP

2017-08-07 Thread Markus Armbruster
Sizes should use QAPI type 'size' (uint64_t). SnapshotInfo member @vm-state-size is 'int' (int64_t). QEMUSnapshotInfo member @vm_state_size is uint64_t. bdrv_query_snapshot_info_list(), bdrv_image_info_dump(), qmp_blockdev_snapshot_delete_internal_sync() convert implicitly between the two.

[Qemu-block] [RFC PATCH 27/56] block/dirty-bitmap: Clean up signed vs. unsigned dirty counts

2017-08-07 Thread Markus Armbruster
hbitmap_count() returns uint64_t. Clean up test-hbitmap.c to check its value with g_assert_cmpuint() instead of g_assert_cmpint(). bdrv_get_dirty_count() and bdrv_get_meta_dirty_count() return its value converted to int64_t. Clean them up to return it unadulterated. This moves the implicit

[Qemu-block] [RFC PATCH 10/56] hmp: Make balloon's argument unsigned

2017-08-07 Thread Markus Armbruster
The previous commit made it unsigned in QMP. Switch HMP's args_type from 'M' to 'o'. Loses support for expressions (QEMU pocket calculator), gains support for units other than mebibytes. Negative values are no longer accepted and interpreted modulo 2^64. Instead, values between 2^63 and 2^64-1

[Qemu-block] [RFC PATCH 08/56] dump: Make sizes and addresses unsigned in QAPI/QMP

2017-08-07 Thread Markus Armbruster
Sizes, virtual and physical addresses should use QAPI type 'size' (uint64_t). dump-guest-memory parameters @begin, @length are 'int' (int64_t). They get implicitly converted to unsigned types somewhere down in the bowels of the dump machinery. DumpQueryResult members @completed and @total are

[Qemu-block] [RFC PATCH 18/56] migration: Make parameter max-bandwidth unsigned in QAPI/QMP

2017-08-07 Thread Markus Armbruster
Byte rates should use QAPI type 'size' (uint64_t). migrate_set_speed's parameter @value and member @max-bandwidth of MigrationParameters and MigrateSetParameters are 'int' (int64_t). Change them all to 'size'. migrate_set_speed and migrate-set-parameters now accept bandwidth values between 2^63

[Qemu-block] [RFC PATCH 15/56] migration: Make XBZRLE cache size unsigned in QAPI/QMP

2017-08-07 Thread Markus Armbruster
Sizes should use QAPI type 'size' (uint64_t). migrate-set-cache-size parameter @value is 'int' (int64_t). qmp_migrate_set_cache_size() ensures it fits into size_t. page_cache.c implicitly converts the signed size to unsigned types (it can't quite decide whether to use uint64_t or size_t for

[Qemu-block] [RFC PATCH 29/56] block: Make BlockDirtyInfo byte count unsigned in QAPI/QMP

2017-08-07 Thread Markus Armbruster
Byte counts should use QAPI type 'size' (uint64_t). BlockDirtyInfo member @count is 'int' (int64_t). bdrv_query_dirty_bitmaps() computes @count from bdrv_get_dirty_count() in uint64_t, then implicitly converts to int64_t. Before the commit before previous, the conversion was in

[Qemu-block] [RFC PATCH 26/56] block: Make BlockMeasureInfo sizes unsigned in QAPI

2017-08-07 Thread Markus Armbruster
Sizes should use QAPI type 'size' (uint64_t). BlockMeasureInfo members @required and @fully-allocated are 'int' (int64_t). qcow2_measure() computes their values from qcow2_calc_prealloc_size(), @virtual_size and @required, all uint64_t (the former only since the previous commit). raw_measure()

[Qemu-block] [RFC PATCH 37/56] blockjob: Make BlockJobInfo and event speed unsigned in QAPI/QMP

2017-08-07 Thread Markus Armbruster
Byte rates should use QAPI type 'size' (uint64_t). BlockJobInfo member @speed and parameter @speed of events BLOCK_JOB_COMPLETED, BLOCK_JOB_CANCELLED, BLOCK_JOB_READY are 'int' (int64_t). block_job_query(), block_job_event_completed(), block_job_event_cancelled(), block_job_event_ready() all get

[Qemu-block] [RFC PATCH 30/56] block: Make write thresholds unsigned in QAPI/QMP

2017-08-07 Thread Markus Armbruster
File offsets should use QAPI type 'size' (uint64_t). block-set-write-threshold parameter @write-threshold is 'int' (int64_t). qmp_block_set_write_threshold() passes it on to bdrv_write_threshold_set(), implicitly converting to uint64_t. BLOCK_WRITE_THRESHOLD parameters @write-threshold,

[Qemu-block] [RFC PATCH 23/56] option: Fix type of qemu_opt_set_number() parameter @val

2017-08-07 Thread Markus Armbruster
Parameter @val is int64_t. It's assigned to opt->value.uint, which is uint64_t, because that's what QemuOpts integers are. Screwed up when the function was added in commit b83c18e. Change @val to uint64_t. Signed-off-by: Markus Armbruster --- include/qemu/option.h | 2 +-

[Qemu-block] [RFC PATCH 50/56] block: Make BLOCK_IMAGE_CORRUPTED offset, size unsigned in QAPI/QMP

2017-08-07 Thread Markus Armbruster
File offsets should use QAPI type 'size' (uint64_t). BLOCK_IMAGE_CORRUPTED parameters @offset and @size are 'int' (int64_t). qcow2_signal_corruption() passes non-negative int64_t values. Change the event parameters to 'size', for QAPI/QMP consistency. Signed-off-by: Markus Armbruster

[Qemu-block] [RFC PATCH 21/56] block: Clean up get_human_readable_size()

2017-08-07 Thread Markus Armbruster
get_human_readable_size() formats all negative numbers as if they were small. The previous two commits changed all callers to pass unsigned arguments. Change the parameter type to from int64_t to uint64_t. Also change the buffer size parameter from int (ahem!) to size_t. Signed-off-by: Markus

[Qemu-block] [RFC PATCH 36/56] blockjob: Drop unused parameter @errp of method set_speed()

2017-08-07 Thread Markus Armbruster
Signed-off-by: Markus Armbruster --- block/backup.c | 2 +- block/commit.c | 2 +- block/mirror.c | 2 +- block/stream.c | 2 +- blockjob.c | 9 + include/block/blockjob_int.h | 2 +- 6 files

[Qemu-block] [RFC PATCH 40/56] blockjob: Lift speed sign conversion out of backup_job_create()

2017-08-07 Thread Markus Armbruster
backup_job_create() takes int64_t speed. The underlying BlockJob abstraction takes uint64_t. backup_job_create() converts from int64_t to uint64_t, rejecting negative speed. Lift this check and conversion out of backup_job_create() into its callers. I'm going to lift it further until it falls

[Qemu-block] [RFC PATCH 47/56] blockjob: Make BlockJobInfo and event offsets unsigned in QAPI/QMP

2017-08-07 Thread Markus Armbruster
File offsets should use QAPI type 'size' (uint64_t). BlockJobInfo members @len, offset and parameters @len, @offset of events BLOCK_JOB_COMPLETED, BLOCK_JOB_CANCELLED, BLOCK_JOB_READY are 'int' (int64_t). block_job_query(), block_job_event_completed(), block_job_event_cancelled(),

[Qemu-block] [RFC PATCH 31/56] block: Make throttle byte rates and sizes unsigned in QAPI/QMP

2017-08-07 Thread Markus Armbruster
Sizes and byte rates should use QAPI type 'size' (uint64_t). BlockIOThrottle and BlockDeviceInfo members @bps, @bps_rd, @bps_wr, @bps_max, @bps_rd_max, @bps_wr_max, @iops_size are 'int' (int64_t). qmp_block_set_io_throttle() and bdrv_block_device_info() copy @bps, @bps_rd, @bps_wr to / from

[Qemu-block] [RFC PATCH 34/56] block: Make BlockDeviceStats sizes, offsets unsigned in QAPI/QMP

2017-08-07 Thread Markus Armbruster
Byte counts and file offsets should use QAPI type 'size' (uint64_t). BlockDeviceStats members @rd_bytes, @wr_bytes and @wr_highest_offset are 'int' (int64_t). bdrv_query_blk_stats() gets them from BlockAcctStats member nr_bytes[] and stat64_get(), implicitly converting from uint64_t. Change all

[Qemu-block] [RFC PATCH 46/56] blockjob: Make job commands' speed parameter unsigned in QAPI/QMP

2017-08-07 Thread Markus Armbruster
Byte rates should use QAPI type 'size' (uint64_t). drive-backup, blockdev-backup, block-commit, drive-mirror, blockdev-mirror, block-stream and block-job-set-speed parameter @size is 'int' (int64_t). Their QMP command handlers all ensure it's non-negative before they pass it on to the next lower

[Qemu-block] [RFC PATCH 33/56] block: Make block_resize size unsigned in QAPI/QMP

2017-08-07 Thread Markus Armbruster
Sizes should use QAPI type 'size' (uint64_t). block_resize parameter @size is 'int' (int64_t). qmp_block_resize() ensures it's non-negative before it passes it on to blk_truncate(). Change parameter @size to 'size', and update the range check accordingly. Just cleanup; block_resize accepts the

[Qemu-block] [RFC PATCH 48/56] block: Make mirror buffer size unsigned in QAPI/QMP

2017-08-07 Thread Markus Armbruster
Byte counts should use QAPI type 'size' (uint64_t). Parameter @buf-size of drive-mirror and blockdev-mirror is 'int' (int64_t). The underlying MirrorBlockJob abstraction takes size_t. mirror_start_job() converts from int64_t to size_t, rejecting negative sizes (but not values exceeding

[Qemu-block] [RFC PATCH 32/56] hmp: Make block_set_io_throttle's arguments unsigned

2017-08-07 Thread Markus Armbruster
The previous commit made them unsigned in QMP. Switch HMP's args_type from 'l' to 'o'. Loses support for expressions (QEMU pocket calculator), gains support for unit suffixes. Negative values are no longer accepted and interpreted modulo 2^64. Instead, values between 2^63 and 2^64-1 are now

[Qemu-block] [RFC PATCH 54/56] qemu-img: blk_getlength() can fail, fix img_map() to check

2017-08-07 Thread Markus Armbruster
Signed-off-by: Markus Armbruster --- qemu-img.c | 4 1 file changed, 4 insertions(+) diff --git a/qemu-img.c b/qemu-img.c index 3ae5fe3..cf3ef3e 100644 --- a/qemu-img.c +++ b/qemu-img.c @@ -2838,6 +2838,10 @@ static int img_map(int argc, char **argv) } length

[Qemu-block] [RFC PATCH 44/56] blockjob: Lift speed sign conversion out of blockdev_mirror_common()

2017-08-07 Thread Markus Armbruster
blockdev_mirror_common() takes int64_t speed. The underlying BlockJob abstraction takes uint64_t. blockdev_mirror_common() converts from int64_t to uint64_t, rejecting negative speed. Lift this check and conversion out of blockdev_mirror_common() into its callers. I'm going to lift it further

[Qemu-block] [RFC PATCH 41/56] blockjob: Lift speed sign conversion out of mirror_start_job()

2017-08-07 Thread Markus Armbruster
mirror_start_job() takes int64_t speed. The underlying BlockJob abstraction takes uint64_t. mirror_start_job() converts from int64_t to uint64_t, rejecting negative speed. Lift this check and conversion out of mirror_start_job() into its callers. I'm going to lift it further until it falls off

Re: [Qemu-block] [Qemu-devel] [PATCH for-2.10] quorum: Handle bdrv_getlength() failures in quorum_co_flush()

2017-08-07 Thread Alberto Garcia
On Fri 04 Aug 2017 05:44:00 PM CEST, Eric Blake wrote: >> --- a/block/quorum.c >> +++ b/block/quorum.c >> @@ -785,8 +785,9 @@ static coroutine_fn int quorum_co_flush(BlockDriverState >> *bs) >> for (i = 0; i < s->num_children; i++) { >> result = bdrv_co_flush(s->children[i]->bs); >>

Re: [Qemu-block] [PATCH v2 for-2.10 0/4] VHDX bugfixes

2017-08-07 Thread Kevin Wolf
Am 07.08.2017 um 14:38 hat Jeff Cody geschrieben: > > Some VHDX bug fixes, including: > > 1. Checking bdrv_getlength(), bdrv_flush(), and bdrv_truncate() return values Thanks, applied to the block branch. Kevin

Re: [Qemu-block] [PATCH] block: drop bdrv_set_key from BlockDriver

2017-08-07 Thread Kevin Wolf
Am 04.08.2017 um 17:26 hat Paolo Bonzini geschrieben: > This is not used anymore since c01c214b69 ("block: remove all encryption > handling APIs", 2017-07-11). > > Signed-off-by: Paolo Bonzini Thanks, applied to the block branch. Kevin

Re: [Qemu-block] [Qemu-devel] [PATCH v2 for-2.10 2/4] block/vhdx: check for offset overflow to bdrv_truncate()

2017-08-07 Thread Philippe Mathieu-Daudé
On Mon, Aug 7, 2017 at 9:38 AM, Jeff Cody wrote: > VHDX uses uint64_t types for most offsets, following the VHDX spec. > However, bdrv_truncate() takes an int64_t value for the truncating > offset. Check for overflow before calling bdrv_truncate(). > > While we are here,

[Qemu-block] [PATCH] iotests: fix 185

2017-08-07 Thread Vladimir Sementsov-Ogievskiy
185 iotest is broken. How to test: > i=0; while ./check -qcow2 -nocache 185; do ((i+=1)); echo N = $i; \ done; echo N = $i finished for me like this: 185 2s ... - output mismatch (see 185.out.bad) --- /work/src/qemu/master/tests/qemu-iotests/185.out2017-07-14 \ 15:14:29.520343805

Re: [Qemu-block] [PATCH v2 for-2.10 3/4] block/vhdx: check error return of bdrv_flush()

2017-08-07 Thread Eric Blake
On 08/07/2017 07:38 AM, Jeff Cody wrote: > Reported-by: Kevin Wolf > Signed-off-by: Jeff Cody > --- > block/vhdx-log.c | 17 ++--- > 1 file changed, 14 insertions(+), 3 deletions(-) > Reviewed-by: Eric Blake -- Eric Blake,

Re: [Qemu-block] [PATCH 11/17] block/nbd-client: fix nbd_co_request: set s->reply.handle to 0 on error

2017-08-07 Thread Vladimir Sementsov-Ogievskiy
07.08.2017 14:55, Eric Blake wrote: On 08/04/2017 10:14 AM, Vladimir Sementsov-Ogievskiy wrote: We set s->reply.handle to 0 on one error path and don't set on another. For consistancy and to avoid assert in nbd_read_reply_entry let's set s->reply.handle to 0 in case of wrong handle too.

Re: [Qemu-block] [PATCH 06/17] block/nbd-client: fix nbd_read_reply_entry

2017-08-07 Thread Vladimir Sementsov-Ogievskiy
07.08.2017 14:52, Eric Blake wrote: On 08/04/2017 10:14 AM, Vladimir Sementsov-Ogievskiy wrote: Set reply.handle to 0 on error path to prevent normal path of nbd_co_receive_reply. Signed-off-by: Vladimir Sementsov-Ogievskiy --- block/nbd-client.c | 1 + 1 file

Re: [Qemu-block] [PATCH v2 for-2.10 4/4] block/vhdx: check error return of bdrv_truncate()

2017-08-07 Thread Eric Blake
On 08/07/2017 07:38 AM, Jeff Cody wrote: > Signed-off-by: Jeff Cody > --- > block/vhdx-log.c | 6 +- > 1 file changed, 5 insertions(+), 1 deletion(-) > Reviewed-by: Eric Blake > diff --git a/block/vhdx-log.c b/block/vhdx-log.c > index a27dc05..14b724e

Re: [Qemu-block] [PATCH for-2.10] quorum: Set sectors-count to 0 when reporting a flush error

2017-08-07 Thread Kevin Wolf
Am 07.08.2017 um 14:36 hat Alberto Garcia geschrieben: > The QUORUM_REPORT_BAD event has fields to report the sector in which > the error was detected and the number of affected sectors starting > from that one. This is important for read and write errors, but not > for flush errors. > > For

[Qemu-block] [RFC PATCH 14/56] migration: Fix migrate-set-cache-size error reporting

2017-08-07 Thread Markus Armbruster
qmp_migrate_set_cache_size() calls xbzrle_cache_resize() to do the actual work, which in turn calls cache_init() to resize the cache. If cache_init() fails, xbzrle_cache_resize() reports that error with error_report() and fails. qmp_migrate_set_cache_size() detects the failure and reports

[Qemu-block] [RFC PATCH 49/56] block: Make ImageCheck file offset unsigned in QAPI

2017-08-07 Thread Markus Armbruster
File offsets should use QAPI type 'size' (uint64_t). ImageCheck member @image-end-offset is 'int' (int64_t). collect_image_check() gets it from BdrvCheckResult member @image_end_offset (also int64_t, should never be negative). Change the ImageCheck member to 'size', for QAPI/QMP consistency.

  1   2   >