summaryrefslogtreecommitdiff
path: root/block
AgeCommit message (Collapse)AuthorFilesLines
2016-11-08block: Don't mark node clean after failed flushKevin Wolf1-1/+3
Commit 3ff2f67a changed bdrv_co_flush() so that no flush is issues if the image hasn't been dirtied since the last flush. This is not quite correct: The condition should be that the image hasn't been dirtied since the last _successful_ flush. This patch changes the logic accordingly. Without this fix, subsequent bdrv_co_flush() calls would return success without actually doing anything even though the image is still dirty. The difference is visible in some blkdebug test cases where error messages incorrectly disappeared after commit 3ff2f67a. Cc: qemu-stable@nongnu.org Signed-off-by: Kevin Wolf <kwolf@redhat.com> Reviewed-by: Denis V. Lunev <den@openvz.org> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Reviewed-by: John Snow <jsnow@redhat.com> Message-id: 1478300595-10090-1-git-send-email-kwolf@redhat.com Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
2016-11-03Merge remote-tracking branch 'remotes/bonzini/tags/for-upstream' into stagingStefan Hajnoczi3-43/+81
* NBD bugfix (Changlong) * NBD write zeroes support (Eric) * Memory backend fixes (Haozhong) * Atomics fix (Alex) * New AVX512 features (Luwei) * "make check" logging fix (Paolo) * Chardev refactoring fallout (Paolo) * Small checkpatch improvements (Paolo, Jeff) # gpg: Signature made Wed 02 Nov 2016 08:31:11 AM GMT # gpg: using RSA key 0xBFFBD25F78C7AE83 # gpg: Good signature from "Paolo Bonzini <bonzini@gnu.org>" # gpg: aka "Paolo Bonzini <pbonzini@redhat.com>" # Primary key fingerprint: 46F5 9FBD 57D6 12E7 BFD4 E2F7 7E15 100C CD36 69B1 # Subkey fingerprint: F133 3857 4B66 2389 866C 7682 BFFB D25F 78C7 AE83 * remotes/bonzini/tags/for-upstream: (30 commits) main-loop: Suppress I/O thread warning under qtest docs/rcu.txt: Fix minor typo vl: exit qemu on guest panic if -no-shutdown is not set checkpatch: allow spaces before parenthesis for 'coroutine_fn' x86: add AVX512_4VNNIW and AVX512_4FMAPS features slirp: fix CharDriver breakage qemu-char: do not forward events through the mux until QEMU has started nbd: Implement NBD_CMD_WRITE_ZEROES on client nbd: Implement NBD_CMD_WRITE_ZEROES on server nbd: Improve server handling of shutdown requests nbd: Refactor conversion to errno to silence checkpatch nbd: Support shorter handshake nbd: Less allocation during NBD_OPT_LIST nbd: Let client skip portions of server reply nbd: Let server know when client gives up negotiation nbd: Share common option-sending code in client nbd: Send message along with server NBD_REP_ERR errors nbd: Share common reply-sending code in server nbd: Rename struct nbd_request and nbd_reply nbd: Rename NbdClientSession to NBDClientSession ... Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
2016-11-02nbd: Implement NBD_CMD_WRITE_ZEROES on clientEric Blake3-0/+41
Upstream NBD protocol recently added the ability to efficiently write zeroes without having to send the zeroes over the wire, along with a flag to control whether the client wants a hole. The generic block code takes care of falling back to the obvious write of lots of zeroes if we return -ENOTSUP because the server does not have WRITE_ZEROES. Ideally, since NBD_CMD_WRITE_ZEROES does not involve any data over the wire, we want to support transactions that are much larger than the normal 32M limit imposed on NBD_CMD_WRITE. But the server may still have a limit smaller than UINT_MAX, so until experimental NBD protocol additions for advertising various command sizes is finalized (see [1], [2]), for now we just stick to the same limits as normal writes. [1] https://github.com/yoe/nbd/blob/extension-info/doc/proto.md [2] https://sourceforge.net/p/nbd/mailman/message/35081223/ Signed-off-by: Eric Blake <eblake@redhat.com> Message-Id: <1476469998-28592-17-git-send-email-eblake@redhat.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
2016-11-02nbd: Rename struct nbd_request and nbd_replyEric Blake2-15/+15
Our coding convention prefers CamelCase names, and we already have other existing structs with NBDFoo naming. Let's be consistent, before later patches add even more structs. Signed-off-by: Eric Blake <eblake@redhat.com> Message-Id: <1476469998-28592-6-git-send-email-eblake@redhat.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
2016-11-02nbd: Rename NbdClientSession to NBDClientSessionEric Blake3-18/+18
It's better to use consistent capitalization of the namespace used for NBD functions; we have more instances of NBD* than Nbd*. Signed-off-by: Eric Blake <eblake@redhat.com> Message-Id: <1476469998-28592-5-git-send-email-eblake@redhat.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
2016-11-02nbd: Treat flags vs. command type as separate fieldsEric Blake1-6/+3
Current upstream NBD documents that requests have a 16-bit flags, followed by a 16-bit type integer; although older versions mentioned only a 32-bit field with masking to find flags. Since the protocol is in network order (big-endian over the wire), the ABI is unchanged; but dealing with the flags as a separate field rather than masking will make it easier to add support for upcoming NBD extensions that increase the number of both flags and commands. Improve some comments in nbd.h based on the current upstream NBD protocol (https://github.com/yoe/nbd/blob/master/doc/proto.md), and touch some nearby code to keep checkpatch.pl happy. Signed-off-by: Eric Blake <eblake@redhat.com> Message-Id: <1476469998-28592-3-git-send-email-eblake@redhat.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
2016-11-01nbd: Use CoQueue for free_sema instead of CoMutexChanglong Xie2-5/+5
NBD is using the CoMutex in a way that wasn't anticipated. For example, if there are N(N=26, MAX_NBD_REQUESTS=16) nbd write requests, so we will invoke nbd_client_co_pwritev N times. ---------------------------------------------------------------------------------------- time request Actions 1 1 in_flight=1, Coroutine=C1 2 2 in_flight=2, Coroutine=C2 ... 15 15 in_flight=15, Coroutine=C15 16 16 in_flight=16, Coroutine=C16, free_sema->holder=C16, mutex->locked=true 17 17 in_flight=16, Coroutine=C17, queue C17 into free_sema->queue 18 18 in_flight=16, Coroutine=C18, queue C18 into free_sema->queue ... 26 N in_flight=16, Coroutine=C26, queue C26 into free_sema->queue ---------------------------------------------------------------------------------------- Once nbd client recieves request No.16' reply, we will re-enter C16. It's ok, because it's equal to 'free_sema->holder'. ---------------------------------------------------------------------------------------- time request Actions 27 16 in_flight=15, Coroutine=C16, free_sema->holder=C16, mutex->locked=false ---------------------------------------------------------------------------------------- Then nbd_coroutine_end invokes qemu_co_mutex_unlock what will pop coroutines from free_sema->queue's head and enter C17. More free_sema->holder is C17 now. ---------------------------------------------------------------------------------------- time request Actions 28 17 in_flight=16, Coroutine=C17, free_sema->holder=C17, mutex->locked=true ---------------------------------------------------------------------------------------- In above scenario, we only recieves request No.16' reply. As time goes by, nbd client will almostly recieves replies from requests 1 to 15 rather than request 17 who owns C17. In this case, we will encounter assert "mutex->holder == self" failed since Kevin's commit 0e438cdc "coroutine: Let CoMutex remember who holds it". For example, if nbd client recieves request No.15' reply, qemu will stop unexpectedly: ---------------------------------------------------------------------------------------- time request Actions 29 15(most case) in_flight=15, Coroutine=C15, free_sema->holder=C17, mutex->locked=false ---------------------------------------------------------------------------------------- Per Paolo's suggestion "The simplest fix is to change it to CoQueue, which is like a condition variable", this patch replaces CoMutex with CoQueue. Cc: Wen Congyang <wency@cn.fujitsu.com> Reported-by: zhanghailiang <zhang.zhanghailiang@huawei.com> Suggested-by: Paolo Bonzini <pbonzini@redhat.com> Signed-off-by: Changlong Xie <xiecl.fnst@cn.fujitsu.com> Message-Id: <1476267508-19499-1-git-send-email-xiecl.fnst@cn.fujitsu.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
2016-11-01blockjobs: split interface into public/private, Part 1John Snow4-4/+4
To make it a little more obvious which functions are intended to be public interface and which are intended to be for use only by jobs themselves, split the interface into "public" and "private" files. Convert blockjobs (e.g. block/backup) to using the private interface. Leave blockdev and others on the public interface. There are remaining uses of private state by qemu-img, and several cases in blockdev.c and block/io.c where we grab job->blk for the purposes of acquiring an AIOContext. These will be corrected in future patches. Signed-off-by: John Snow <jsnow@redhat.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com> Reviewed-by: Jeff Cody <jcody@redhat.com> Message-id: 1477584421-1399-7-git-send-email-jsnow@redhat.com Signed-off-by: Jeff Cody <jcody@redhat.com>
2016-11-01blockjob: centralize QMP event emissionsJohn Snow4-15/+11
There's no reason to leave this to blockdev; we can do it in blockjobs directly and get rid of an extra callback for most users. All non-internal events, even those created outside of QMP, will consistently emit events. Signed-off-by: John Snow <jsnow@redhat.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com> Reviewed-by: Jeff Cody <jcody@redhat.com> Message-id: 1477584421-1399-5-git-send-email-jsnow@redhat.com Signed-off-by: Jeff Cody <jcody@redhat.com>
2016-11-01Replication/Blockjobs: Create replication jobs as internalJohn Snow3-19/+19
Bubble up the internal interface to commit and backup jobs, then switch replication tasks over to using this methodology. Signed-off-by: John Snow <jsnow@redhat.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com> Reviewed-by: Jeff Cody <jcody@redhat.com> Message-id: 1477584421-1399-4-git-send-email-jsnow@redhat.com Signed-off-by: Jeff Cody <jcody@redhat.com>
2016-11-01blockjobs: Allow creating internal jobsJohn Snow4-4/+5
Add the ability to create jobs without an ID. Signed-off-by: John Snow <jsnow@redhat.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com> Reviewed-by: Jeff Cody <jcody@redhat.com> Message-id: 1477584421-1399-3-git-send-email-jsnow@redhat.com Signed-off-by: Jeff Cody <jcody@redhat.com>
2016-11-01block/gluster: fix port type in the QAPI options listPrasanna Kumar Kalever1-1/+1
After introduction of qapi schema in gluster block driver code, the port type is now string as per InetSocketAddress { 'struct': 'InetSocketAddress', 'data': { 'host': 'str', 'port': 'str', '*to': 'uint16', '*ipv4': 'bool', '*ipv6': 'bool' } } but the current code still treats it as QEMU_OPT_NUMBER, hence fixing port to accept QEMU_OPT_STRING. Suggested-by: Markus Armbruster <armbru@redhat.com> Signed-off-by: Prasanna Kumar Kalever <prasanna.kalever@redhat.com> Reviewed-by: Jeff Cody <jcody@redhat.com> Signed-off-by: Jeff Cody <jcody@redhat.com>
2016-11-01block/gluster: improve defense over string to int conversionPrasanna Kumar Kalever1-1/+10
using atoi() for converting string to int may be error prone in case if string supplied in the argument is not a fold of numerical number, This is not a bug because in the existing code, static QemuOptsList runtime_tcp_opts = { .name = "gluster_tcp", .head = QTAILQ_HEAD_INITIALIZER(runtime_tcp_opts.head), .desc = { ... { .name = GLUSTER_OPT_PORT, .type = QEMU_OPT_NUMBER, .help = "port number ...", }, ... }; port type is QEMU_OPT_NUMBER, before we actually reaches atoi() port is already defended by parse_option_number() However It is a good practice to use function like parse_uint_full() over atoi() to keep port self defended Note: As now the port string to int conversion has its defence code set, and also we understand that port argument is actually a string type, in the follow up patch let's move port type from QEMU_OPT_NUMBER to QEMU_OPT_STRING [Jeff Cody: removed spurious parenthesis] Signed-off-by: Prasanna Kumar Kalever <prasanna.kalever@redhat.com> Reviewed-by: Markus Armbruster <armbru@redhat.com> Signed-off-by: Jeff Cody <jcody@redhat.com>
2016-11-01block: Turn on "unmap" in active commitFam Zheng1-1/+1
We already specified BDRV_O_UNMAP when opening images in 'qemu-img commit', but didn't turn on the "unmap" in the active commit job. This patch fixes that so that zeroed clusters in top image can be discarded which is desired in the virt-sparsify use case, where a temporary overlay is created and fstrim'ed before commiting back, to free space in the original image. This also enables it for block-commit. Signed-off-by: Fam Zheng <famz@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Message-id: 1474974892-5031-1-git-send-email-famz@redhat.com Signed-off-by: Jeff Cody <jcody@redhat.com>
2016-11-01block/gluster: memory usage: use one glfs instance per volumePrasanna Kumar Kalever1-14/+80
Currently, for every drive accessed via gfapi we create a new glfs instance (call glfs_new() followed by glfs_init()) which could consume memory in few 100 MB's, from the table below it looks like for each instance ~300 MB VSZ was consumed Before: ------- Disks VSZ RSS 1 1098728 187756 2 1430808 198656 3 1764932 199704 4 2084728 202684 This patch maintains a list of pre-opened glfs objects. On adding a new drive belonging to the same gluster volume, we just reuse the existing glfs object by updating its refcount. With this approch we shrink up the unwanted memory consumption and glfs_new/glfs_init calls for accessing a disk (file) if belongs to same volume. From below table notice that the memory usage after adding a disk (which will reuse the existing glfs object hence) is in negligible compared to before. After: ------ Disks VSZ RSS 1 1101964 185768 2 1109604 194920 3 1114012 196036 4 1114496 199868 Disks: number of -drive VSZ: virtual memory size of the process in KiB RSS: resident set size, the non-swapped physical memory (in kiloBytes) VSZ and RSS are analyzed using 'ps aux' utility. Signed-off-by: Prasanna Kumar Kalever <prasanna.kalever@redhat.com> Reviewed-by: Jeff Cody <jcody@redhat.com> Message-id: 1477581890-4811-1-git-send-email-prasanna.kalever@redhat.com Signed-off-by: Jeff Cody <jcody@redhat.com>
2016-11-01block: add gluster ifdef guard checks for SEEK_DATA/SEEK_HOLE supportJeff Cody1-3/+14
Add checks to see if the system compiling QEMU has support for SEEK_HOLE/SEEK_DATA. If the system does not, we will flag that seek data is unsupported in gluster. Note: this is not a check on whether the gluster server itself supports SEEK_DATA (that is already done during runtime), but rather if the compilation environment supports SEEK_DATA. Reviewed-by: Eric Blake <eblake@redhat.com> Signed-off-by: Jeff Cody <jcody@redhat.com> Tested-by: Eric Blake <eblake@redhat.com> Message-id: 00370bce5c98140d6c56ad5145635ec6551265cc.1475876377.git.jcody@redhat.com Signed-off-by: Jeff Cody <jcody@redhat.com>
2016-11-01rbd: make the code more readableXiubo Li1-13/+12
Make it a bit clearer and more readable. Signed-off-by: Xiubo Li <lixiubo@cmss.chinamobile.com> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Reviewed-by: John Snow <jsnow@redhat.com> Reviewed-by: Jeff Cody <jcody@redhat.com> Message-id: 1476519973-6436-1-git-send-email-lixiubo@cmss.chinamobile.com CC: John Snow <jsnow@redhat.com> Signed-off-by: Jeff Cody <jcody@redhat.com>
2016-10-31block/nfs: Introduce runtime_opts in NFSAshijeet Acharya1-93/+349
Make NFS block driver use various fine grained runtime_opts. Set .bdrv_parse_filename() to nfs_parse_filename() and introduce two new functions nfs_parse_filename() and nfs_parse_uri() to help parsing the URI. Add a new option "server" which then accepts a new struct NFSServer. Signed-off-by: Ashijeet Acharya <ashijeetacharya@gmail.com> [ kwolf: Fixed client->path ] Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2016-10-31raw_bsd: add offset and size optionsTomáš Golembiovský1-3/+214
Added two new options 'offset' and 'size'. This makes it possible to use only part of the file as a device. This can be used e.g. to limit the access only to single partition in a disk image or use a disk inside a tar archive (like OVA). When 'size' is specified we do our best to honour it. Signed-off-by: Tomáš Golembiovský <tgolembi@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2016-10-31block: Support streaming to an intermediate layerAlberto Garcia1-0/+24
This makes sure that the image we are streaming into is open in read-write mode during the operation. Operation blockers are also set in all intermediate nodes, since they will be removed from the chain afterwards. Finally, this also unblocks the stream operation in backing files. Signed-off-by: Alberto Garcia <berto@igalia.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2016-10-31block: Block all intermediate nodes in commit_active_start()Alberto Garcia1-0/+8
When block-commit is launched without the top parameter, it uses internally a mirror block job. In that case all intermediate nodes between the active and base nodes must be blocked as well. Signed-off-by: Alberto Garcia <berto@igalia.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2016-10-31block: Block all nodes involved in the block-commit operationAlberto Garcia1-0/+14
After a successful block-commit operation all nodes between top and base are removed from the backing chain, and top's overlay needs to be updated to point to base. Because of that we should prevent other block jobs from messing with them. This patch blocks all operations in these nodes in commit_start(). Signed-off-by: Alberto Garcia <berto@igalia.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2016-10-31block: Use block_job_add_bdrv() in backup_start()Alberto Garcia1-4/+1
Use block_job_add_bdrv() instead of blocking all operations in backup_start() and unblocking them in backup_run(). Signed-off-by: Alberto Garcia <berto@igalia.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2016-10-31block: Use block_job_add_bdrv() in mirror_start_job()Alberto Garcia1-2/+1
Use block_job_add_bdrv() instead of blocking all operations in mirror_start_job() and unblocking them in mirror_exit(). Signed-off-by: Alberto Garcia <berto@igalia.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2016-10-31block: Add bdrv_drain_all_{begin,end}()Alberto Garcia1-3/+24
bdrv_drain_all() doesn't allow the caller to do anything after all pending requests have been completed but before block jobs are resumed. This patch splits bdrv_drain_all() into _begin() and _end() for that purpose. It also adds aio_{disable,enable}_external() calls to disable external clients in the meantime. An important restriction of this split is that no new block jobs or BlockDriverStates can be created between the bdrv_drain_all_begin() and bdrv_drain_all_end() calls. This is not a concern now because we'll only be using this in bdrv_reopen_multiple(), but it must be dealt with if we ever have other uses cases in the future. Signed-off-by: Alberto Garcia <berto@igalia.com> Cc: Paolo Bonzini <pbonzini@redhat.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2016-10-31block/ssh: Use InetSocketAddress optionsAshijeet Acharya1-4/+5
Drop the use of legacy options in favour of the InetSocketAddress options. Signed-off-by: Ashijeet Acharya <ashijeetacharya@gmail.com> Reviewed-by: Max Reitz <mreitz@redhat.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2016-10-31block/ssh: Add InetSocketAddress and accept itAshijeet Acharya1-16/+82
Add InetSocketAddress compatibility to SSH driver. Add a new option "server" to the SSH block driver which then accepts a InetSocketAddress. "host" and "port" are supported as legacy options and are mapped to their InetSocketAddress representation. Signed-off-by: Ashijeet Acharya <ashijeetacharya@gmail.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2016-10-31block/ssh: Add ssh_has_filename_options_conflict()Ashijeet Acharya1-6/+21
We have 5 options plus ("server") option which is added in the next patch that conflict with specifying a SSH filename. We need to iterate over all the options to check whether its key has an "server." prefix. This iteration will help us adding the new option "server" easily. Signed-off-by: Ashijeet Acharya <ashijeetacharya@gmail.com> Reviewed-by: Max Reitz <mreitz@redhat.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2016-10-31Merge remote-tracking branch 'remotes/famz/tags/for-upstream' into stagingPeter Maydell10-166/+271
# gpg: Signature made Fri 28 Oct 2016 15:47:39 BST # gpg: using RSA key 0xCA35624C6A9171C6 # gpg: Good signature from "Fam Zheng <famz@redhat.com>" # gpg: WARNING: This key is not certified with a trusted signature! # gpg: There is no indication that the signature belongs to the owner. # Primary key fingerprint: 5003 7CB7 9706 0F76 F021 AD56 CA35 624C 6A91 71C6 * remotes/famz/tags/for-upstream: aio: convert from RFifoLock to QemuRecMutex qemu-thread: introduce QemuRecMutex iothread: release AioContext around aio_poll block: only call aio_poll on the current thread's AioContext qemu-img: call aio_context_acquire/release around block job qemu-io: acquire AioContext block: prepare bdrv_reopen_multiple to release AioContext replication: pass BlockDriverState to reopen_backing_file iothread: detach all block devices before stopping them aio: introduce qemu_get_current_aio_context sheepdog: use BDRV_POLL_WHILE nfs: use BDRV_POLL_WHILE nfs: move nfs_set_events out of the while loops block: introduce BDRV_POLL_WHILE qed: Implement .bdrv_drain block: change drain to look only at one child at a time block: add BDS field to count in-flight requests mirror: use bdrv_drained_begin/bdrv_drained_end blockjob: introduce .drain callback for jobs replication: interrupt failover if the main device is closed Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
2016-10-28Merge remote-tracking branch 'remotes/berrange/tags/pull-qio-2016-10-27-1' ↵Peter Maydell1-0/+1
into staging Merge qio 2016/10/27 v1 # gpg: Signature made Thu 27 Oct 2016 13:54:03 BST # gpg: using RSA key 0xBE86EBB415104FDF # gpg: Good signature from "Daniel P. Berrange <dan@berrange.com>" # gpg: aka "Daniel P. Berrange <berrange@redhat.com>" # Primary key fingerprint: DAF3 A6FD B26B 6291 2D0E 8E3F BE86 EBB4 1510 4FDF * remotes/berrange/tags/pull-qio-2016-10-27-1: main: set names for main loop sources created vnc: set name for all I/O channels created migration: set name for all I/O channels created char: set name for all I/O channels created nbd: set name for all I/O channels created io: add ability to set a name for IO channels io: Add a QIOChannelSocket cleanup test io: set LISTEN flag explicitly for listen sockets io: Introduce a qio_channel_set_feature() helper io: Use qio_channel_has_feature() where applicable io: Fix double shift usages on QIOChannel features Conflicts: qemu-char.c Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
2016-10-28block: only call aio_poll on the current thread's AioContextPaolo Bonzini3-0/+16
aio_poll is not thread safe; for example bdrv_drain can hang if the last in-flight I/O operation is completed in the I/O thread after the main thread has checked bs->in_flight. The bug remains latent as long as all of it is called within aio_context_acquire/aio_context_release, but this will change soon. To fix this, if bdrv_drain is called from outside the I/O thread, signal the main AioContext through a dummy bottom half. The event loop then only runs in the I/O thread. Reviewed-by: Fam Zheng <famz@redhat.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> Message-Id: <1477565348-5458-18-git-send-email-pbonzini@redhat.com> Signed-off-by: Fam Zheng <famz@redhat.com>
2016-10-28block: prepare bdrv_reopen_multiple to release AioContextPaolo Bonzini2-2/+3
After the next patch bdrv_drain_all will have to be called without holding any AioContext. Prepare to do this by adding an AioContext argument to bdrv_reopen_multiple. Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Reviewed-by: Fam Zheng <famz@redhat.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> Message-Id: <1477565348-5458-15-git-send-email-pbonzini@redhat.com> Signed-off-by: Fam Zheng <famz@redhat.com>
2016-10-28replication: pass BlockDriverState to reopen_backing_filePaolo Bonzini1-9/+12
This will be needed in the next patch to retrieve the AioContext. Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Reviewed-by: Fam Zheng <famz@redhat.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> Message-Id: <1477565348-5458-14-git-send-email-pbonzini@redhat.com> Signed-off-by: Fam Zheng <famz@redhat.com>
2016-10-28sheepdog: use BDRV_POLL_WHILEPaolo Bonzini1-29/+35
This is important when the sheepdog driver works on a BlockDriverState that is attached to an I/O thread other than the main thread. Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> Reviewed-by: Fam Zheng <famz@redhat.com> Message-Id: <1477565348-5458-11-git-send-email-pbonzini@redhat.com> Signed-off-by: Fam Zheng <famz@redhat.com>
2016-10-28nfs: use BDRV_POLL_WHILEPaolo Bonzini1-18/+28
This will make it possible to use nfs_get_allocated_file_size on a file that is not in the main AioContext. Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Reviewed-by: Fam Zheng <famz@redhat.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> Message-Id: <1477565348-5458-10-git-send-email-pbonzini@redhat.com> Signed-off-by: Fam Zheng <famz@redhat.com>
2016-10-28nfs: move nfs_set_events out of the while loopsPaolo Bonzini1-4/+4
nfs_set_events only needs to be called once before entering the while loop; afterwards, nfs_process_read and nfs_process_write take care of it. Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Reviewed-by: Fam Zheng <famz@redhat.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> Message-Id: <1477565348-5458-9-git-send-email-pbonzini@redhat.com> Signed-off-by: Fam Zheng <famz@redhat.com>
2016-10-28block: introduce BDRV_POLL_WHILEPaolo Bonzini3-50/+10
We want the BDS event loop to run exclusively in the iothread that owns the BDS's AioContext. This macro will provide the synchronization between the two event loops; for now it just wraps the common idiom of a while loop around aio_poll. Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> Reviewed-by: Fam Zheng <famz@redhat.com> Message-Id: <1477565348-5458-8-git-send-email-pbonzini@redhat.com> Signed-off-by: Fam Zheng <famz@redhat.com>
2016-10-28qed: Implement .bdrv_drainFam Zheng1-1/+15
The "need_check_timer" is used to clear the "NEED_CHECK" flag in the image header after a grace period once metadata update has finished. To comply with the bdrv_drain semantics, we should make sure it remains deleted once .bdrv_drain is called. The change to qed_need_check_timer_cb is needed because bdrv_qed_drain is called after s->bs has been drained, and should not operate on it; instead it should operate on the BdrvChild-ren exclusively. Doing so is easy because QED does not have a bdrv_co_flush_to_os callback, hence all that is needed to flush it is to ensure writes have reached the disk. Based on commit df9a681dc9a (which however included some unrelated hunks, possibly due to a merge failure or an overlooked squash). The patch was reverted because at the time bdrv_qed_drain could call qed_plug_allocating_write_reqs while an allocating write was queued. This however is not possible anymore after the previous patch; .bdrv_drain is only called after all writes have completed at the QED level, and its purpose is to trigger metadata writes in bs->file. Signed-off-by: Fam Zheng <famz@redhat.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> Message-Id: <1477565348-5458-7-git-send-email-pbonzini@redhat.com> Signed-off-by: Fam Zheng <famz@redhat.com>
2016-10-28block: change drain to look only at one child at a timePaolo Bonzini1-21/+26
bdrv_requests_pending is checking children to also wait until internal requests (such as metadata writes) have completed. However, checking children is in general overkill. Children requests can be of two kinds: - requests caused by an operation on bs, e.g. a bdrv_aio_write to bs causing a write to bs->file->bs. In this case, the parent's in_flight count will always be incremented by at least one for every request in the child. - asynchronous metadata writes or flushes. Such writes can be started even if bs's in_flight count is zero, but not after the .bdrv_drain callback has been invoked. This patch therefore changes bdrv_drain to finish I/O in the parent (after which the parent's in_flight will be locked to zero), call bdrv_drain (after which the parent will not generate I/O on the child anymore), and then wait for internal I/O in the children to complete. Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> Reviewed-by: Fam Zheng <famz@redhat.com> Message-Id: <1477565348-5458-6-git-send-email-pbonzini@redhat.com> Signed-off-by: Fam Zheng <famz@redhat.com>
2016-10-28block: add BDS field to count in-flight requestsPaolo Bonzini2-29/+69
Unlike tracked_requests, this field also counts throttled requests, and remains non-zero if an AIO operation needs a BH to be "really" completed. With this change, it is no longer necessary to have a dummy BdrvTrackedRequest for requests that are never serialising, and it is no longer necessary to poll the AioContext once after bdrv_requests_pending(bs) returns false. Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> Reviewed-by: Fam Zheng <famz@redhat.com> Message-Id: <1477565348-5458-5-git-send-email-pbonzini@redhat.com> Signed-off-by: Fam Zheng <famz@redhat.com>
2016-10-28mirror: use bdrv_drained_begin/bdrv_drained_endPaolo Bonzini1-12/+23
Ensure that there are no changes between the last check to bdrv_get_dirty_count and the switch to the target. There is already a bdrv_drained_end call, we only need to ensure that bdrv_drained_begin is not called twice. Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> Reviewed-by: Fam Zheng <famz@redhat.com> Message-Id: <1477565348-5458-4-git-send-email-pbonzini@redhat.com> Signed-off-by: Fam Zheng <famz@redhat.com>
2016-10-28blockjob: introduce .drain callback for jobsPaolo Bonzini2-8/+44
This is required to decouple block jobs from running in an AioContext. With multiqueue block devices, a BlockDriverState does not really belong to a single AioContext. The solution is to first wait until all I/O operations are complete; then loop in the main thread for the block job to complete entirely. Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> Reviewed-by: Fam Zheng <famz@redhat.com> Message-Id: <1477565348-5458-3-git-send-email-pbonzini@redhat.com> Signed-off-by: Fam Zheng <famz@redhat.com>
2016-10-28replication: interrupt failover if the main device is closedPaolo Bonzini1-0/+3
Without this change, there is a race condition in tests/test-replication. Depending on how fast the failover job (active commit) runs, there is a chance of two bad things happening: 1) replication_done can be called after the secondary has been closed and hence when the BDRVReplicationState is not valid anymore. 2) two copies of the active disk are present during the /replication/secondary/stop test (that test runs immediately after /replication/secondary/start, which tests failover). This causes the corruption detector to fire. Reviewed-by: Wen Congyang <wency@cn.fujitsu.com> Reviewed-by: Changlong Xie <xiecl.fnst@cn.fujitsu.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> Reviewed-by: Fam Zheng <famz@redhat.com> Message-Id: <1477565348-5458-2-git-send-email-pbonzini@redhat.com> Signed-off-by: Fam Zheng <famz@redhat.com>
2016-10-27block/nbd: Use SocketAddress optionsMax Reitz1-10/+16
Drop the use of legacy options in favor of the SocketAddress representation, even for internal use (i.e. for storing the result of the filename parsing). Signed-off-by: Max Reitz <mreitz@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2016-10-27block/nbd: Accept SocketAddressMax Reitz1-62/+113
Add a new option "server" to the NBD block driver which accepts a SocketAddress. "path", "host" and "port" are still supported as legacy options and are mapped to their corresponding SocketAddress representation. Signed-off-by: Max Reitz <mreitz@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2016-10-27block/nbd: Add nbd_has_filename_options_conflict()Max Reitz1-6/+20
Right now, we have four possible options that conflict with specifying an NBD filename, and a future patch will add another one ("address"). This future option is a nested QDict that is flattened at this point, requiring us to test each option whether its key has an "address." prefix. Therefore, we will then need to iterate through all options (including the "export" option which was not covered so far). Adding this iteration logic now will simplify adding the new option later. A nice side effect is that the user will not receive a long list of five options which are not supposed to be specified with a filename, but we can actually print the problematic option. Reviewed-by: Kevin Wolf <kwolf@redhat.com> Signed-off-by: Max Reitz <mreitz@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2016-10-27block/nbd: Use qdict_put()Max Reitz1-7/+6
Instead of inlining this nice macro (i.e. resorting to qdict_put_obj(..., QOBJECT(...))), use it. Reviewed-by: Eric Blake <eblake@redhat.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com> Signed-off-by: Max Reitz <mreitz@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2016-10-27block/nbd: Default port in nbd_refresh_filename()Max Reitz1-13/+6
Instead of not emitting the port in nbd_refresh_filename(), just set it to the default if the user did not specify it. This makes the logic a bit simpler. Reviewed-by: Eric Blake <eblake@redhat.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com> Signed-off-by: Max Reitz <mreitz@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2016-10-27block/nbd: Reject port parameter without hostMax Reitz1-2/+5
Currently, a port that is passed along with a UNIX socket path is silently ignored. That is not exactly ideal, it should be an error instead. Reviewed-by: Eric Blake <eblake@redhat.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com> Signed-off-by: Max Reitz <mreitz@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2016-10-27block/nbd: Drop trailing "." in error messagesMax Reitz1-2/+2
Reviewed-by: Eric Blake <eblake@redhat.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com> Signed-off-by: Max Reitz <mreitz@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>