Re: [Qemu-block] [PATCH v4 03/15] block: introduce BDRV_REQ_ALLOCATE flag

On 08/01/2017 09:19 AM, Anton Nefedov wrote: > The flag is supposed to indicate that the region of the disk image has > to be sufficiently allocated so it reads as zeroes. The call with the flag > set has to return -ENOTSUP if allocation cannot be done efficiently > (i.e. without falling back to

Re: [Qemu-block] [Qemu-devel] [PATCH 3/3] parallels: drop check that bdrv_truncate() is working

On 08/04/2017 10:32 PM, Eric Blake wrote: > On 08/04/2017 10:10 AM, Denis V. Lunev wrote: >> This would be actually strange and error prone. If truncate() nowadays >> will fail, there is something fatally wrong. Let's check for that during >> the actual work. >> >> The only fallback case is when

Re: [Qemu-block] [Qemu-devel] [PATCH 3/3] parallels: drop check that bdrv_truncate() is working

On 08/04/2017 10:10 AM, Denis V. Lunev wrote: > This would be actually strange and error prone. If truncate() nowadays > will fail, there is something fatally wrong. Let's check for that during > the actual work. > > The only fallback case is when the file is not zero initialized. In this > case

Re: [Qemu-block] [Qemu-devel] [PATCH 1/3] block: respect error code from bdrv_getlength in handle_aiocb_write_zeroes

On 08/04/2017 10:10 AM, Denis V. Lunev wrote: > Original idea beyond the code in question was the following: we have failed > to write zeroes with fallocate(FALLOC_FL_ZERO_RANGE) as the simplest > approach and via fallocate(FALLOC_FL_PUNCH_HOLE)/fallocate(0). We have the > only chance now: if the

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

On 08/04/2017 10:26 AM, Paolo Bonzini wrote: > This is not used anymore since c01c214b69 ("block: remove all encryption > handling APIs", 2017-07-11). > > Signed-off-by: Paolo Bonzini > --- > include/block/block_int.h | 1 - > 1 file changed, 1 deletion(-) Reviewed-by:

Re: [Qemu-block] [Qemu-devel] [PATCH 05/17] block/nbd-client: get rid of ssize_t

On Fri, Aug 04, 2017 at 06:14:28PM +0300, Vladimir Sementsov-Ogievskiy wrote: > Use int variable for nbd_co_send_request return value (as > nbd_co_send_request returns int). Hmm, nbd_co_send_request() propagates return value of nbd_send_request, which returns ssize_t. IOW, I think we need to fix

Re: [Qemu-block] [Qemu-devel] [PATCH for-2.10] vmdk: Fix error handling/reporting of vmdk_check

On 08/04/2017 09:09 AM, Fam Zheng wrote: > Errors from the callees must be captured and propagated to our caller, > ensure this for both find_extent() and bdrv_getlength(). > > Reported-by: Markus Armbruster > Signed-off-by: Fam Zheng > --- > block/vmdk.c |

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

On Fri, Aug 04, 2017 at 10:41:54AM -0500, Eric Blake wrote: > On 08/04/2017 09:06 AM, Daniel P. Berrange wrote: > > On Fri, Aug 04, 2017 at 04:02:10PM +0200, Kevin Wolf wrote: > >> Am 04.08.2017 um 12:50 hat Daniel P. Berrange geschrieben: > >>> Signed-off-by: Daniel P. Berrange

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

On 08/04/2017 09:08 AM, Alberto Garcia wrote: > A bdrv_getlength() call can fail and return a negative value. This > is not being handled in quorum_co_flush(), which can result in a > QUORUM_REPORT_BAD event with an arbitrary value on the 'sectors-count' > field. > > Reported-by: Markus

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

On 08/04/2017 09:06 AM, Daniel P. Berrange wrote: > On Fri, Aug 04, 2017 at 04:02:10PM +0200, Kevin Wolf wrote: >> Am 04.08.2017 um 12:50 hat Daniel P. Berrange geschrieben: >>> Signed-off-by: Daniel P. Berrange >>> --- >> Really? We are asserting that they match in

Re: [Qemu-block] [Qemu-devel] Is blk_getlength() in find_image_format() and img_map() kosher?

On 08/04/2017 08:55 AM, Markus Armbruster wrote: > Have a look at find_image_format(): > > if (blk_is_sg(file) || !blk_is_inserted(file) || blk_getlength(file) == > 0) { > *pdrv = _raw; > return ret; > } > > blk_getlength() can fail. Shouldn't we error out then? > > We

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

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 another one in dump_refcounts(). > > While we're

Re: [Qemu-block] [PATCH v4 14/15] iotest 190: test BDRV_REQ_ALLOCATE

On 08/01/2017 09:19 AM, Anton Nefedov wrote: > Signed-off-by: Anton Nefedov > --- > tests/qemu-iotests/190 | 146 > + > tests/qemu-iotests/190.out | 50 > tests/qemu-iotests/group | 1 + > 3 files

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

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 support the flag; if either side can't

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

On Fri, Aug 04, 2017 at 09:28:54AM -0500, Eric Blake wrote: > On 08/04/2017 09:08 AM, Daniel P. Berrange wrote: > > Signed-off-by: Daniel P. Berrange > > --- > > > > - Clarify that @bytes matches @qiov total size (Kevin) > > > > include/block/block_int.h | 31

[Qemu-block] [PATCH 15/17] block/nbd-client: refactor reading reply

Read the whole reply in one place - in nbd_read_reply_entry. Signed-off-by: Vladimir Sementsov-Ogievskiy --- block/nbd-client.h | 1 + block/nbd-client.c | 27 +-- 2 files changed, 14 insertions(+), 14 deletions(-) diff --git

Re: [Qemu-block] [Qemu-devel] [PATCH v2 6/6] dev-storage: Fix the unusual function name

On 08/04/2017 07:26 AM, Mao Zhongyi wrote: The function name of usb_msd_{realize,unrealize}_*, usb_msd_class_initfn_* are unusual. Rename it to usb_msd_*_{realize,unrealize}, usb_msd_class_*_initfn. Cc: Gerd Hoffmann Signed-off-by: Mao Zhongyi

[Qemu-block] [PATCH 08/17] block/nbd-client: rename nbd_recv_coroutines_enter_all

Rename nbd_recv_coroutines_enter_all to nbd_recv_coroutines_wake_all, as it most probably just add all recv coroutines into co_queue_wakeup, not directly enter them. Signed-off-by: Vladimir Sementsov-Ogievskiy --- block/nbd-client.c | 4 ++-- 1 file changed, 2

[Qemu-block] [PATCH 14/17] block/nbd-client: exit reply-reading coroutine on incorrect handle

Check reply-handle == request-handle in the same place, where recv coroutine number is calculated from reply->handle and it's correctness checked - in nbd_read_reply_entry. Also finish nbd_read_reply_entry in case of reply-handle != request-handle in the same way as in case of incorrect

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

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 --- nbd/client.c | 2 -- 1 file changed, 2 deletions(-)

[Qemu-block] [PATCH 12/17] block/nbd-client: refactor nbd_co_request

Reduce nesting, get rid of extra variable. Signed-off-by: Vladimir Sementsov-Ogievskiy --- block/nbd-client.c | 25 - 1 file changed, 12 insertions(+), 13 deletions(-) diff --git a/block/nbd-client.c b/block/nbd-client.c index

[Qemu-block] [PATCH 03/17] nbd/client: refactor nbd_receive_reply

Refactor nbd_receive_reply 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 --- include/block/nbd.h | 2 +- nbd/client.c| 12

Re: [Qemu-block] [PATCH v2] qemu-img: Clarify about relative backing file options

On Fri, Aug 04, 2017 at 10:36:58PM +0800, Fam Zheng wrote: > It's not too surprising when a user specifies the backing file relative > to the current working directory instead of the top layer image. This > causes error when they differ. Though the error message has enough > information to infer

[Qemu-block] [PATCH 05/17] block/nbd-client: get rid of ssize_t

Use int variable for nbd_co_send_request return value (as nbd_co_send_request returns int). Signed-off-by: Vladimir Sementsov-Ogievskiy --- block/nbd-client.c | 10 +- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/block/nbd-client.c

[Qemu-block] [PATCH 16/17] block/nbd-client: drop reply field from NBDClientSession

Drop 'reply' from NBDClientSession. It's usage is not very transparent: 1. it is used to deliver error to receiving coroutine, and receiving coroutine must save or handle it somehow and then zero out it in NBDClientSession. 2. it is used to inform receiving coroutines that

[Qemu-block] [PATCH 00/17] nbd client refactoring and fixing

A bit more refactoring and fixing before BLOCK_STATUS series. I've tried to make individual patches simple enough, so there are a lot of them. Vladimir Sementsov-Ogievskiy (17): nbd/client: fix nbd_opt_go nbd/client: refactor nbd_read_eof nbd/client: refactor nbd_receive_reply nbd/client:

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

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(+) diff --git a/block/nbd-client.c b/block/nbd-client.c index

[Qemu-block] [PATCH 13/17] block/nbd-client: refactor NBDClientSession.recv_coroutine

Move from recv_coroutine[i] to requests[i].co. This is needed for further refactoring, new fields will be added to created structure. Signed-off-by: Vladimir Sementsov-Ogievskiy --- block/nbd-client.h | 4 +++- block/nbd-client.c | 20 ++-- 2 files

Re: [Qemu-block] [PATCH] block/null: Remove 'filename' option

On Fri, Aug 04, 2017 at 04:43:54PM +0200, Kevin Wolf wrote: > This option was only added to allow 'null-co://' and 'null-aio://' as > filenames, its value never served any actual purpose and was ignored. > Nevertheless it was accepted as '-drive driver=null,filename=foo'. > > The correct way to

[Qemu-block] [PATCH 0/3] respect bdrv_getlength() error code

These cases were reported by Markus Armbruster Patches add error checking of the bdrv_getlength() call or remove the call of that function. Signed-off-by: Denis V. Lunev CC: Markus Armbruster CC: Kevin Wolf CC: Max Reitz

[Qemu-block] [PATCH 2/3] parallels: respect error code of bdrv_getlength() in allocate_clusters()

If we can not get the file length, the state of BDS is broken completely. Return error to the caller. Signed-off-by: Denis V. Lunev CC: Markus Armbruster CC: Kevin Wolf CC: Max Reitz CC: Stefan Hajnoczi

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

This is not used anymore since c01c214b69 ("block: remove all encryption handling APIs", 2017-07-11). Signed-off-by: Paolo Bonzini --- include/block/block_int.h | 1 - 1 file changed, 1 deletion(-) diff --git a/include/block/block_int.h b/include/block/block_int.h index

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

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 Sementsov-Ogievskiy --- block/nbd-client.c | 6 +++---

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

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 --- nbd/nbd-internal.h | 34

[Qemu-block] [PATCH 07/17] block/nbd-client: refactor request send/receive

Move nbd_co_receive_reply and nbd_coroutine_end calls into nbd_co_send_request and rename the latter to just nbd_co_request. This removes code duplications in nbd_client_co_{pwrite,pread,...} functions. Also this is needed for further refactoring. Signed-off-by: Vladimir Sementsov-Ogievskiy

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

Fix nbd_send_request to return int, as it returns a return value of nbd_write (which is int), and the only user of nbd_send_request's return value (nbd_co_send_request) consider it as int too. Signed-off-by: Vladimir Sementsov-Ogievskiy --- include/block/nbd.h | 2 +-

Re: [Qemu-block] [PATCH] block/null: Remove 'filename' option

On 08/04/2017 09:43 AM, Kevin Wolf wrote: > This option was only added to allow 'null-co://' and 'null-aio://' as > filenames, its value never served any actual purpose and was ignored. > Nevertheless it was accepted as '-drive driver=null,filename=foo'. > > The correct way to enable the protocol

Re: [Qemu-block] [Qemu-devel] Is the use of bdrv_getlength() in handle_aiocb_write_zeroes() kosher?

"Denis V. Lunev" writes: > On 08/04/2017 03:16 PM, Markus Armbruster wrote: >> Denis, you added this in commit d50d822: >> >> #ifdef CONFIG_FALLOCATE >> if (s->has_fallocate && aiocb->aio_offset >= bdrv_getlength(aiocb->bs)) { >> int ret = do_fallocate(s->fd, 0,

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

Signed-off-by: Daniel P. Berrange --- Changed in v2: - Document 'flags' parameter too (Eric) include/block/block_int.h | 31 +++ 1 file changed, 31 insertions(+) diff --git a/include/block/block_int.h b/include/block/block_int.h index

Re: [Qemu-block] [PATCH v4 for-2.10 01/15] mirror: inherit supported write/zero flags

On 08/01/2017 09:18 AM, Anton Nefedov wrote: > Signed-off-by: Anton Nefedov > --- > block/mirror.c | 5 + > 1 file changed, 5 insertions(+) I think this one counts as a bug fix worthy of inclusion in 2.10. Reviewed-by: Eric Blake > > diff

[Qemu-block] [PATCH 3/3] parallels: drop check that bdrv_truncate() is working

This would be actually strange and error prone. If truncate() nowadays will fail, there is something fatally wrong. Let's check for that during the actual work. The only fallback case is when the file is not zero initialized. In this case we should switch to preallocation via fallocate().

[Qemu-block] [PATCH 1/3] block: respect error code from bdrv_getlength in handle_aiocb_write_zeroes

Original idea beyond the code in question was the following: we have failed to write zeroes with fallocate(FALLOC_FL_ZERO_RANGE) as the simplest approach and via fallocate(FALLOC_FL_PUNCH_HOLE)/fallocate(0). We have the only chance now: if the request comes beyond end of the file. Thus we should

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

On Fri, Aug 04, 2017 at 02:49:42PM +0200, Markus Armbruster wrote: > bdrv_getlength() can fail. There are several calls in vhdx*.c that > don't seem to check. Bug or not? Most definitely a bug. Shall I queue up some patches, or do you already have some?

[Qemu-block] [PATCH for-2.10] vmdk: Fix error handling/reporting of vmdk_check

Errors from the callees must be captured and propagated to our caller, ensure this for both find_extent() and bdrv_getlength(). Reported-by: Markus Armbruster Signed-off-by: Fam Zheng --- block/vmdk.c | 26 ++ 1 file changed, 18

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

Signed-off-by: Daniel P. Berrange --- - Clarify that @bytes matches @qiov total size (Kevin) include/block/block_int.h | 31 +++ 1 file changed, 31 insertions(+) diff --git a/include/block/block_int.h b/include/block/block_int.h index

Re: [Qemu-block] [PATCH for-2.10] block: move trace probes into bdrv_co_preadv|pwritev

On 08/04/2017 05:50 AM, Daniel P. Berrange wrote: > There are trace probes in bdrv_co_readv|writev, however, the > block drivers are being gradually moved over to using the > bdrv_co_preadv|pwritev functions instead. As a result some > block drivers miss the current probes. Move the probes > into

[Qemu-block] [PATCH] quorum: Handle bdrv_getlength() failures in quorum_co_flush()

A bdrv_getlength() call can fail and return a negative value. This is not being handled in quorum_co_flush(), which can result in a QUORUM_REPORT_BAD event with an arbitrary value on the 'sectors-count' field. Reported-by: Markus Armbruster Signed-off-by: Alberto Garcia

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

On Fri, Aug 04, 2017 at 04:02:10PM +0200, Kevin Wolf wrote: > Am 04.08.2017 um 12:50 hat Daniel P. Berrange geschrieben: > > Signed-off-by: Daniel P. Berrange > > --- > > include/block/block_int.h | 29 + > > 1 file changed, 29 insertions(+) > >

[Qemu-block] Unchecked blk_getlength() in device models and board code

blk_getlength() can fail. I figure the following need fixing: hw/arm/musicpal.c: musicpal_init() hw/block/nand.c: nand_realize() hw/block/virtio-blk.c: virtio_blk_update_config() hw/ppc/ppc405_boards.c: taihu_405ep_init()

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

On Fri 04 Aug 2017 02:48:03 PM CEST, Markus Armbruster wrote: > Have a look at quorum_co_flush(): > > quorum_report_bad(QUORUM_OP_TYPE_FLUSH, 0, > bdrv_getlength(s->children[i]->bs), > s->children[i]->bs->node_name, result); >

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

Am 04.08.2017 um 12:50 hat Daniel P. Berrange geschrieben: > Signed-off-by: Daniel P. Berrange > --- > include/block/block_int.h | 29 + > 1 file changed, 29 insertions(+) > > diff --git a/include/block/block_int.h b/include/block/block_int.h >

Re: [Qemu-block] [PATCH for 2.10] block: use 1 MB bounce buffers for crypto instead of 16KB

On Fri, Aug 04, 2017 at 07:08:54AM -0500, Eric Blake wrote: > On 08/04/2017 05:51 AM, Daniel P. Berrange wrote: > > Using 16KB bounce buffers creates a significant performance > > penalty for I/O to encrypted volumes on storage with high > > I/O latency (rotating rust & network drives), because it

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

bdrv_getlength() can fail. The uses in qcow.c don't check. Is that safe?

Re: [Qemu-block] [PATCH v3 3/7] block: tidy ThrottleGroupMember initializations

On Mon 31 Jul 2017 11:54:39 AM CEST, Manos Pitsidianakis wrote: > Move the CoMutex and CoQueue inits inside throttle_group_register_tgm() > which is called whenever a ThrottleGroupMember is initialized. There's > no need for them to be separate. > > Reviewed-by: Stefan Hajnoczi

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

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 another one in dump_refcounts(). While we're talking: the one in qcow2_measure() assigns to a variable of type

[Qemu-block] Is the use of bdrv_getlength() in vmdk_check() kosher?

Have a look at vmdk_check(): if (ret == VMDK_OK && cluster_offset >= bdrv_getlength(extent->file->bs)) bdrv_getlength() can fail. Does it do the right thing then? For what it's worth, the comparison of its value is unsigned.

[Qemu-block] Is the use of bdrv_getlength() in quorum_co_flush() kosher?

Have a look at quorum_co_flush(): quorum_report_bad(QUORUM_OP_TYPE_FLUSH, 0, bdrv_getlength(s->children[i]->bs), s->children[i]->bs->node_name, result); bdrv_getlength() can fail. Does it do the right thing then?

Re: [Qemu-block] [PATCH for 2.10] block: use 1 MB bounce buffers for crypto instead of 16KB

On Fri, Aug 04, 2017 at 11:51:36AM +0100, Daniel P. Berrange wrote: > Using 16KB bounce buffers creates a significant performance > penalty for I/O to encrypted volumes on storage with high > I/O latency (rotating rust & network drives), because it > triggers lots of fairly small I/O operations. >

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

On 08/04/2017 05:50 AM, Daniel P. Berrange wrote: > Signed-off-by: Daniel P. Berrange > --- > include/block/block_int.h | 29 + > 1 file changed, 29 insertions(+) > > diff --git a/include/block/block_int.h b/include/block/block_int.h > index

Re: [Qemu-block] [Qemu-devel] [PATCH for-2.10 2/5] block: Allow reopen rw without BDRV_O_ALLOW_RDWR

On 08/04/2017 05:20 AM, Kevin Wolf wrote: >>> >>> Hmm, I wonder. https://bugzilla.redhat.com/show_bug.cgi?id=1465320 >>> details a failure when starting qemu with a read-write NBD disk, then >>> taking several snapshots (nbd <- snap1 <- snap2 <- snap3), then where >>> intermediate commit (snap2

Re: [Qemu-block] [PATCH v2] qemu-img: Clarify about relative backing file options

On 08/04/2017 09:36 AM, Fam Zheng wrote: > It's not too surprising when a user specifies the backing file relative > to the current working directory instead of the top layer image. This > causes error when they differ. Though the error message has enough > information to infer the fact about the

[Qemu-block] [PATCH] block/null: Remove 'filename' option

This option was only added to allow 'null-co://' and 'null-aio://' as filenames, its value never served any actual purpose and was ignored. Nevertheless it was accepted as '-drive driver=null,filename=foo'. The correct way to enable the protocol prefixes (and that without adding a useless -drive

[Qemu-block] [PATCH v2] qemu-img: Clarify about relative backing file options

It's not too surprising when a user specifies the backing file relative to the current working directory instead of the top layer image. This causes error when they differ. Though the error message has enough information to infer the fact about the misunderstanding, it is better if we document

Re: [Qemu-block] [PATCH] qemu-img: Clarify about relative backing file options

On Fri, 08/04 09:23, Eric Blake wrote: > > +If a relative path name is given, the backing file is looked up against > > +@var{filename}, rather than the current working directory. > > Maybe better as: > the backing file is looked up relative to the directory containing > @var{filename} Sounds

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

On 08/04/2017 09:08 AM, Daniel P. Berrange wrote: > Signed-off-by: Daniel P. Berrange > --- > > - Clarify that @bytes matches @qiov total size (Kevin) > > include/block/block_int.h | 31 +++ > 1 file changed, 31 insertions(+) [looks like the

Re: [Qemu-block] [PATCH v3 1/7] block: move ThrottleGroup membership to ThrottleGroupMember

On Mon 31 Jul 2017 11:54:37 AM CEST, Manos Pitsidianakis wrote: > This commit eliminates the 1:1 relationship between BlockBackend and > throttle group state. Users will be able to create multiple throttle > nodes, each with its own throttle group state, in the future. The > throttle group state

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

Markus Armbruster writes: > bdrv_getlength() can fail. The uses in qcow.c don't check. Is that > safe? There's another one in vpc_open().

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

On Fri, Aug 04, 2017 at 11:50:59AM +0100, Daniel P. Berrange wrote: > Signed-off-by: Daniel P. Berrange > --- > include/block/block_int.h | 29 + > 1 file changed, 29 insertions(+) Reviewed-by: Stefan Hajnoczi

[Qemu-block] Is the use of bdrv_getlength() in vhdx*.c kosher?

bdrv_getlength() can fail. There are several calls in vhdx*.c that don't seem to check. Bug or not?

Re: [Qemu-block] [PATCH v2 5/6] hw/block: Use errp directly rather than local_err

On Fri, Aug 04, 2017 at 06:26:42PM +0800, Mao Zhongyi wrote: > Pass the error message to errp directly rather than the local > variable local_err and propagate it to errp via error_propagate(). > > Cc: John Snow > Cc: Kevin Wolf > Cc: Max Reitz

Re: [Qemu-block] [PATCH] block: move trace probes into bdrv_co_preadv|pwritev

On Fri, Aug 04, 2017 at 11:50:36AM +0100, Daniel P. Berrange wrote: > There are trace probes in bdrv_co_readv|writev, however, the > block drivers are being gradually moved over to using the > bdrv_co_preadv|pwritev functions instead. As a result some > block drivers miss the current probes. Move

Re: [Qemu-block] [PATCH v2 4/6] hw/block: Fix the return type

On Fri, Aug 04, 2017 at 06:26:41PM +0800, Mao Zhongyi wrote: > When the function no success value to transmit, it usually make the > function return void. It has turned out not to be a success, because > it means that the extra local_err variable and error_propagate() will > be needed. It leads to

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

On 08/04/2017 03:31 PM, Markus Armbruster wrote: > Same question for allocate_clusters() in parallels.c, commit 5a41e1f, > modified in commit ddd2ef2: > > if (s->data_end + space > bdrv_getlength(bs->file->bs) >> > BDRV_SECTOR_BITS) { > > bdrv_getlength() can fail. Does it do the right thing

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

On 08/04/2017 03:16 PM, Markus Armbruster wrote: > Denis, you added this in commit d50d822: > > #ifdef CONFIG_FALLOCATE > if (s->has_fallocate && aiocb->aio_offset >= bdrv_getlength(aiocb->bs)) { > int ret = do_fallocate(s->fd, 0, aiocb->aio_offset, > aiocb->aio_nbytes); > if

[Qemu-block] [PATCH] qemu-img: Clarify about relative backing file options

It's not too surprising when a user specifies the backing file relative to the current working directory instead of the top layer image. This causes error when they differ. Though the error message has enough information to infer the fact about the misunderstanding, it is better if we document

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

On Fri, 08/04 15:28, Markus Armbruster wrote: > Have a look at vmdk_check(): > > if (ret == VMDK_OK && > cluster_offset >= bdrv_getlength(extent->file->bs)) > > bdrv_getlength() can fail. Does it do the right thing then? For what > it's worth, the comparison of its value is

[Qemu-block] Is the use of bdrv_getlength() in handle_aiocb_write_zeroes() kosher?

Denis, you added this in commit d50d822: #ifdef CONFIG_FALLOCATE if (s->has_fallocate && aiocb->aio_offset >= bdrv_getlength(aiocb->bs)) { int ret = do_fallocate(s->fd, 0, aiocb->aio_offset, aiocb->aio_nbytes); if (ret == 0 || ret != -ENOTSUP) { return ret;

[Qemu-block] Is the use of bdrv_getlength() in parallels.c kosher? (was: Is the use of bdrv_getlength() in handle_aiocb_write_zeroes() kosher?)

Same question for allocate_clusters() in parallels.c, commit 5a41e1f, modified in commit ddd2ef2: if (s->data_end + space > bdrv_getlength(bs->file->bs) >> BDRV_SECTOR_BITS) { bdrv_getlength() can fail. Does it do the right thing then? For what it's worth, the comparison of its value is

[Qemu-block] Is blk_getlength() in find_image_format() and img_map() kosher?

Have a look at find_image_format(): if (blk_is_sg(file) || !blk_is_inserted(file) || blk_getlength(file) == 0) { *pdrv = _raw; return ret; } blk_getlength() can fail. Shouldn't we error out then? We pretty obviously should in img_map(): length = blk_getlength(blk);

Re: [Qemu-block] [PATCH for 2.10] block: use 1 MB bounce buffers for crypto instead of 16KB

On Fri, Aug 04, 2017 at 01:48:01PM +0100, Stefan Hajnoczi wrote: > On Fri, Aug 04, 2017 at 11:51:36AM +0100, Daniel P. Berrange wrote: > > Using 16KB bounce buffers creates a significant performance > > penalty for I/O to encrypted volumes on storage with high > > I/O latency (rotating rust &

[Qemu-block] [PATCH for 2.10] block: use 1 MB bounce buffers for crypto instead of 16KB

Using 16KB bounce buffers creates a significant performance penalty for I/O to encrypted volumes on storage with high I/O latency (rotating rust & network drives), because it triggers lots of fairly small I/O operations. On tests with rotating rust, and cache=none|directsync, write speed

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

Signed-off-by: Daniel P. Berrange --- include/block/block_int.h | 29 + 1 file changed, 29 insertions(+) diff --git a/include/block/block_int.h b/include/block/block_int.h index d4f4ea7584..deb81a58bd 100644 --- a/include/block/block_int.h +++

[Qemu-block] [PATCH] block: move trace probes into bdrv_co_preadv|pwritev

There are trace probes in bdrv_co_readv|writev, however, the block drivers are being gradually moved over to using the bdrv_co_preadv|pwritev functions instead. As a result some block drivers miss the current probes. Move the probes into bdrv_co_preadv|pwritev instead, so that they are triggered

[Qemu-block] [PATCH v2 5/6] hw/block: Use errp directly rather than local_err

Pass the error message to errp directly rather than the local variable local_err and propagate it to errp via error_propagate(). Cc: John Snow Cc: Kevin Wolf Cc: Max Reitz Cc: Keith Busch Cc: Stefan Hajnoczi

[Qemu-block] [PATCH v2 0/6] Convert to realize and improve error handling

This series mainly implements the conversions of ide, floppy and nvme device to realize. Add some error handling messages and remove the local variable local_err, use errp to propagate the error directly. Also fix the unusual function name. v2: -use bool as the return type instead of int.

[Qemu-block] [PATCH v2 6/6] dev-storage: Fix the unusual function name

The function name of usb_msd_{realize,unrealize}_*, usb_msd_class_initfn_* are unusual. Rename it to usb_msd_*_{realize,unrealize}, usb_msd_class_*_initfn. Cc: Gerd Hoffmann Signed-off-by: Mao Zhongyi --- hw/usb/dev-storage.c | 20

[Qemu-block] [PATCH v2 4/6] hw/block: Fix the return type

When the function no success value to transmit, it usually make the function return void. It has turned out not to be a success, because it means that the extra local_err variable and error_propagate() will be needed. It leads to cumbersome code, therefore, transmit success/ failure in the return

[Qemu-block] [PATCH v2 2/6] hw/block/fdc: Convert to realize

Convert floppy_drive_init() to realize and rename it to floppy_drive_realize(). Cc: John Snow Cc: Kevin Wolf Cc: Max Reitz Cc: Markus Armbruster Signed-off-by: Mao Zhongyi ---

Re: [Qemu-block] [Qemu-devel] [PATCH for-2.10 2/5] block: Allow reopen rw without BDRV_O_ALLOW_RDWR

Am 04.08.2017 um 03:49 hat Eric Blake geschrieben: > On 08/03/2017 10:21 AM, Eric Blake wrote: > > On 08/03/2017 10:02 AM, Kevin Wolf wrote: > >> BDRV_O_ALLOW_RDWR is a flag that tells whether qemu can internally > >> reopen a node read-write temporarily because the user requested > >> read-write