aboutsummaryrefslogtreecommitdiff
path: root/block
AgeCommit message (Collapse)Author
2018-03-27Merge remote-tracking branch 'remotes/maxreitz/tags/pull-block-2018-03-26' ↵Peter Maydell
into staging A fix for dirty bitmap migration through shared storage, and a VMDK patch keeping us from creating too large extents. # gpg: Signature made Mon 26 Mar 2018 21:17:05 BST # gpg: using RSA key F407DB0061D5CF40 # gpg: Good signature from "Max Reitz <mreitz@redhat.com>" # Primary key fingerprint: 91BE B60A 30DB 3E88 57D1 1829 F407 DB00 61D5 CF40 * remotes/maxreitz/tags/pull-block-2018-03-26: vmdk: return ERROR when cluster sector is larger than vmdk limitation iotests: enable shared migration cases in 169 qcow2: fix bitmaps loading when bitmaps already exist qcow2-bitmap: add qcow2_reopen_bitmaps_rw_hint() Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
2018-03-26vmdk: return ERROR when cluster sector is larger than vmdk limitationyuchenlin
VMDK has a hard limitation of extent size, which is due to the size of grain table entry is 32 bits. It means it can only point to a grain located at offset = 2^32. To avoid writing the user data beyond limitation and record a useless offset in grain table. We should return ERROR here. Signed-off-by: yuchenlin <yuchenlin@synology.com> Message-id: 20180322133337.28024-1-yuchenlin@synology.com Reviewed-by: Fam Zheng <famz@redhat.com> Signed-off-by: Max Reitz <mreitz@redhat.com>
2018-03-26qcow2: fix bitmaps loading when bitmaps already existVladimir Sementsov-Ogievskiy
On reopen with existing bitmaps, instead of loading bitmaps, lets reopen them if needed. This also fixes bitmaps migration through shared storage. Consider the case. Persistent bitmaps are stored on bdrv_inactivate. Then, on destination process_incoming_migration_bh() calls bdrv_invalidate_cache_all() which leads to qcow2_load_autoloading_dirty_bitmaps() which fails if bitmaps are already loaded on destination start. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Message-id: 20180320170521.32152-3-vsementsov@virtuozzo.com Signed-off-by: Max Reitz <mreitz@redhat.com>
2018-03-26qcow2-bitmap: add qcow2_reopen_bitmaps_rw_hint()Vladimir Sementsov-Ogievskiy
Add version of qcow2_reopen_bitmaps_rw, which do the same work but also return a hint about was header updated or not. This will be used in the following fix for bitmaps reloading after migration. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Reviewed-by: John Snow <jsnow@redhat.com> Reviewed-by: Max Reitz <mreitz@redhat.com> Message-id: 20180320170521.32152-2-vsementsov@virtuozzo.com Signed-off-by: Max Reitz <mreitz@redhat.com>
2018-03-26vhdx: Check for 4 GB maximum log size on creationKevin Wolf
It's unclear what the real maximum is, but we use an uint32_t to store the log size in vhdx_co_create(), so we should check that the given value fits in 32 bits. Signed-off-by: Kevin Wolf <kwolf@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Reviewed-by: Jeff Cody <jcody@redhat.com>
2018-03-26vhdx: Don't use error_setg_errno() with constant errnoKevin Wolf
error_setg_errno() is meant for cases where we got an errno from the OS that can add useful extra information to an error message. It's pointless if we pass a constant errno, these cases should use plain error_setg(). Signed-off-by: Kevin Wolf <kwolf@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Reviewed-by: Jeff Cody <jcody@redhat.com>
2018-03-26vhdx: Require power-of-two block size on createKevin Wolf
Images with a non-power-of-two block size are invalid and cannot be opened. Reject such block sizes when creating an image. Signed-off-by: Kevin Wolf <kwolf@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Reviewed-by: Jeff Cody <jcody@redhat.com>
2018-03-26parallels: Check maximum cluster size on createKevin Wolf
It's unclear what the real maximum cluster size is for the Parallels format, but let's at least make sure that we don't get integer overflows in our .bdrv_co_create implementation. Signed-off-by: Kevin Wolf <kwolf@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com>
2018-03-26luks: Turn another invalid assertion into checkKevin Wolf
Commit e39e959e fixed an invalid assertion in the .bdrv_length implementation, but left a similar assertion in place for .bdrv_truncate. Instead of crashing when the user requests a too large image size, fail gracefully. A file size of exactly INT64_MAX caused failure before, but is actually legal. Signed-off-by: Kevin Wolf <kwolf@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
2018-03-26vdi: Fix build with CONFIG_VDI_DEBUGKevin Wolf
Use qemu_uuid_unparse() instead of uuid_unparse() to make vdi.c compile again when CONFIG_VDI_DEBUG is set. In order to prevent future bitrot, replace '#ifdef CONFIG_VDI_DEBUG' by 'if (VDI_DEBUG)' so that the compiler always sees the code. Signed-off-by: Kevin Wolf <kwolf@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com>
2018-03-26vdi: Change 'static' create option to 'preallocation' in QMPKevin Wolf
What static=on really does is what we call metadata preallocation for other block drivers. While we can still change the QMP interface, make it more consistent by using 'preallocation' for VDI, too. This doesn't implement any new functionality, so the only supported preallocation modes are 'off' and 'metadata' for now. Signed-off-by: Kevin Wolf <kwolf@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com>
2018-03-26qcow2: Reset free_cluster_index when allocating a new refcount blockAlberto Garcia
When we try to allocate new clusters we first look for available ones starting from s->free_cluster_index and once we find them we increase their reference counts. Before we get to call update_refcount() to do this last step s->free_cluster_index is already pointing to the next cluster after the ones we are trying to allocate. During update_refcount() it may happen however that we also need to allocate a new refcount block in order to store the refcounts of these new clusters (and to complicate things further that may also require us to grow the refcount table). After all this we don't know if the clusters that we originally tried to allocate are still available, so we return -EAGAIN to ask the caller to restart the search for free clusters. This is what can happen in a common scenario: 1) We want to allocate a new cluster and we see that cluster N is free. 2) We try to increase N's refcount but all refcount blocks are full, so we allocate a new one at N+1 (where s->free_cluster_index was pointing at). 3) Once we're done we return -EAGAIN to look again for a free cluster, but now s->free_cluster_index points at N+2, so that's the one we allocate. Cluster N remains unallocated and we have a hole in the qcow2 file. This can be reproduced easily: qemu-img create -f qcow2 -o cluster_size=512 hd.qcow2 1M qemu-io -c 'write 0 124k' hd.qcow2 After this the image has 132608 bytes (256 clusters), and the refcount block is full. If we write 512 more bytes it should allocate two new clusters: the data cluster itself and a new refcount block. qemu-io -c 'write 124k 512' hd.qcow2 However the image has now three new clusters (259 in total), and the first one of them is empty (and unallocated): dd if=hd.qcow2 bs=512c skip=256 count=1 | hexdump -C If we write larger amounts of data in the last step instead of the 512 bytes used in this example we can create larger holes in the qcow2 file. What this patch does is reset s->free_cluster_index to its previous value when alloc_refcount_block() returns -EAGAIN. This way the caller will try to allocate again the original clusters if they are still free. The output of iotest 026 also needs to be updated because now that images have no holes some tests fail at a different point and the number of leaked clusters is different. Signed-off-by: Alberto Garcia <berto@igalia.com> Reviewed-by: Eric Blake <eblake@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2018-03-26block/blkreplay: Remove protocol-related fieldsFabiano Rosas
The blkreplay driver is not a protocol so it should implement bdrv_open instead of bdrv_file_open and not provide a protocol_name. Attempts to invoke this driver using protocol syntax (i.e. blkreplay:<filename:options:...>) will now fail gracefully: $ qemu-img info blkreplay:foo qemu-img: Could not open 'blkreplay:foo': Unknown protocol 'blkreplay' Signed-off-by: Fabiano Rosas <farosas@linux.vnet.ibm.com> Reviewed-by: Pavel Dovgalyuk <pavel.dovgaluk@ispras.ru> Reviewed-by: Max Reitz <mreitz@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2018-03-26block/throttle: Remove protocol-related fieldsFabiano Rosas
The throttle driver is not a protocol so it should implement bdrv_open instead of bdrv_file_open and not provide a protocol_name. Attempts to invoke this driver using protocol syntax (i.e. throttle:<filename:options:...>) will now fail gracefully: $ qemu-img info throttle:foo qemu-img: Could not open 'throttle:foo': Unknown protocol 'throttle' Signed-off-by: Fabiano Rosas <farosas@linux.vnet.ibm.com> Reviewed-by: Max Reitz <mreitz@redhat.com> Reviewed-by: Alberto Garcia <berto@igalia.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2018-03-26block/quorum: Remove protocol-related fieldsFabiano Rosas
The quorum driver is not a protocol so it should implement bdrv_open instead of bdrv_file_open and not provide a protocol_name. Attempts to invoke this driver using protocol syntax (i.e. quorum:<filename:options:...>) will now fail gracefully: $ qemu-img info quorum:foo qemu-img: Could not open 'quorum:foo': Unknown protocol 'quorum' Signed-off-by: Fabiano Rosas <farosas@linux.vnet.ibm.com> Reviewed-by: Max Reitz <mreitz@redhat.com> Reviewed-by: Alberto Garcia <berto@igalia.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2018-03-26block/replication: Remove protocol_name fieldFabiano Rosas
The protocol_name field is used when selecting a driver via protocol syntax (i.e. <protocol_name>:<filename:options:...>). Drivers that are only selected explicitly (e.g. driver=replication,mode=primary,...) should not have a protocol_name. This patch removes the protocol_name field from the brdv_replication structure so that attempts to invoke this driver using protocol syntax will fail gracefully: $ qemu-img info replication:foo qemu-img: Could not open 'replication:': Unknown protocol 'replication' Buglink: https://bugs.launchpad.net/qemu/+bug/1726733 Signed-off-by: Fabiano Rosas <farosas@linux.vnet.ibm.com> Reviewed-by: Max Reitz <mreitz@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2018-03-19qapi: add block latency histogram interfaceVladimir Sementsov-Ogievskiy
Set (and clear) histograms through new command block-latency-histogram-set and show new statistics in query-blockstats results. For now, the command is marked experimental with prefix 'x-', to gain experience with the interface without being stuck with design decisions. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Message-Id: <20180309165212.97144-3-vsementsov@virtuozzo.com> Reviewed-by: Eric Blake <eblake@redhat.com> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> [eblake: fix typos, mention x- prefix in commit message] Signed-off-by: Eric Blake <eblake@redhat.com>
2018-03-19block/accounting: introduce latency histogramVladimir Sementsov-Ogievskiy
Introduce latency histogram statics for block devices. For each accounted operation type, the latency region [0, +inf) is divided into subregions by several points. Then, calculate hits for each subregion. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Message-Id: <20180309165212.97144-2-vsementsov@virtuozzo.com> Reviewed-by: Eric Blake <eblake@redhat.com> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Signed-off-by: Eric Blake <eblake@redhat.com>
2018-03-19qapi: Replace qobject_to_X(o) by qobject_to(X, o)Max Reitz
This patch was generated using the following Coccinelle script: @@ expression Obj; @@ ( - qobject_to_qnum(Obj) + qobject_to(QNum, Obj) | - qobject_to_qstring(Obj) + qobject_to(QString, Obj) | - qobject_to_qdict(Obj) + qobject_to(QDict, Obj) | - qobject_to_qlist(Obj) + qobject_to(QList, Obj) | - qobject_to_qbool(Obj) + qobject_to(QBool, Obj) ) and a bit of manual fix-up for overly long lines and three places in tests/check-qjson.c that Coccinelle did not find. Signed-off-by: Max Reitz <mreitz@redhat.com> Reviewed-by: Alberto Garcia <berto@igalia.com> Message-Id: <20180224154033.29559-4-mreitz@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> [eblake: swap order from qobject_to(o, X), rebase to master, also a fix to latent false-positive compiler complaint about hw/i386/acpi-build.c] Signed-off-by: Eric Blake <eblake@redhat.com>
2018-03-19iscsi: fix iSER compilationPaolo Bonzini
This fails in Fedora 28. Reported-by: Andreas Schwab <schwab@suse.de> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2018-03-19vvfat: Fix inherit_options flagsFam Zheng
Overriding flags violates the precedence rules of bdrv_reopen_queue_child. Just like the read-only option, no-flush should be put into the options. The same is done in bdrv_temp_snapshot_options. Reported-by: Stefan Hajnoczi <stefanha@redhat.com> Signed-off-by: Fam Zheng <famz@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2018-03-19block/mirror: change the semantic of 'force' of block-job-cancelLiang Li
When doing drive mirror to a low speed shared storage, if there was heavy BLK IO write workload in VM after the 'ready' event, drive mirror block job can't be canceled immediately, it would keep running until the heavy BLK IO workload stopped in the VM. Libvirt depends on the current block-job-cancel semantics, which is that when used without a flag after the 'ready' event, the command blocks until data is in sync. However, these semantics are awkward in other situations, for example, people may use drive mirror for realtime backups while still wanting to use block live migration. Libvirt cannot start a block live migration while another drive mirror is in progress, but the user would rather abandon the backup attempt as broken and proceed with the live migration than be stuck waiting for the current drive mirror backup to finish. The drive-mirror command already includes a 'force' flag, which libvirt does not use, although it documented the flag as only being useful to quit a job which is paused. However, since quitting a paused job has the same effect as abandoning a backup in a non-paused job (namely, the destination file is not in sync, and the command completes immediately), we can just improve the documentation to make the force flag obviously useful. Cc: Paolo Bonzini <pbonzini@redhat.com> Cc: Jeff Cody <jcody@redhat.com> Cc: Kevin Wolf <kwolf@redhat.com> Cc: Max Reitz <mreitz@redhat.com> Cc: Eric Blake <eblake@redhat.com> Cc: John Snow <jsnow@redhat.com> Reported-by: Huaitong Han <huanhuaitong@didichuxing.com> Signed-off-by: Huaitong Han <huanhuaitong@didichuxing.com> Signed-off-by: Liang Li <liliangleo@didichuxing.com> Signed-off-by: Jeff Cody <jcody@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2018-03-19vpc: Require aligned size in .bdrv_co_createKevin Wolf
Perform the rounding to match a CHS geometry only in the legacy code path in .bdrv_co_create_opts. QMP now requires that the user already passes a CHS aligned image size, unless force-size=true is given. CHS alignment is required to make the image compatible with Virtual PC, but not for use with newer Microsoft hypervisors. Signed-off-by: Kevin Wolf <kwolf@redhat.com> Reviewed-by: Max Reitz <mreitz@redhat.com>
2018-03-19vpc: Support .bdrv_co_createKevin Wolf
This adds the .bdrv_co_create driver callback to vpc, which enables image creation over QMP. Signed-off-by: Kevin Wolf <kwolf@redhat.com> Reviewed-by: Max Reitz <mreitz@redhat.com>
2018-03-19vhdx: Support .bdrv_co_createKevin Wolf
This adds the .bdrv_co_create driver callback to vhdx, which enables image creation over QMP. Signed-off-by: Kevin Wolf <kwolf@redhat.com> Reviewed-by: Max Reitz <mreitz@redhat.com>
2018-03-19vdi: Make comments consistent with other driversKevin Wolf
This makes the .bdrv_co_create(_opts) implementation of vdi look more like the other recently converted block drivers. Signed-off-by: Kevin Wolf <kwolf@redhat.com> Reviewed-by: Max Reitz <mreitz@redhat.com>
2018-03-19qed: Support .bdrv_co_createKevin Wolf
This adds the .bdrv_co_create driver callback to qed, which enables image creation over QMP. Signed-off-by: Kevin Wolf <kwolf@redhat.com> Reviewed-by: Max Reitz <mreitz@redhat.com>
2018-03-19qcow: Support .bdrv_co_createKevin Wolf
This adds the .bdrv_co_create driver callback to qcow, which enables image creation over QMP. Signed-off-by: Kevin Wolf <kwolf@redhat.com> Reviewed-by: Max Reitz <mreitz@redhat.com> Reviewed-by: Jeff Cody <jcody@redhat.com>
2018-03-19parallels: Support .bdrv_co_createKevin Wolf
This adds the .bdrv_co_create driver callback to parallels, which enables image creation over QMP. Signed-off-by: Kevin Wolf <kwolf@redhat.com> Reviewed-by: Max Reitz <mreitz@redhat.com> Reviewed-by: Jeff Cody <jcody@redhat.com>
2018-03-19vdi: Implement .bdrv_co_createMax Reitz
Signed-off-by: Max Reitz <mreitz@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2018-03-19vdi: Move file creation to vdi_co_create_optsMax Reitz
Signed-off-by: Max Reitz <mreitz@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2018-03-19vdi: Pull option parsing from vdi_co_createMax Reitz
In preparation of QAPI-fying VDI image creation, we have to create a BlockdevCreateOptionsVdi type which is received by a (future) vdi_co_create(). vdi_co_create_opts() now converts the QemuOpts object into such a BlockdevCreateOptionsVdi object. The protocol-layer file is still created in vdi_co_do_create() (and BlockdevCreateOptionsVdi.file is set to an empty string), but that will be addressed by a follow-up patch. Note that cluster-size is not part of the QAPI schema because it is not supported by default. Signed-off-by: Max Reitz <mreitz@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2018-03-19luks: Catch integer overflow for huge sizesKevin Wolf
When you request an image size close to UINT64_MAX, the addition of the crypto header may cause an integer overflow. Catch it instead of silently truncating the image size. Signed-off-by: Kevin Wolf <kwolf@redhat.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
2018-03-19luks: Turn invalid assertion into checkKevin Wolf
The .bdrv_getlength implementation of the crypto block driver asserted that the payload offset isn't after EOF. This is an invalid assertion to make as the image file could be corrupted. Instead, check it and return -EIO if the file is too small for the payload offset. Zero length images are fine, so trigger -EIO only on offset > len, not on offset >= len as the assertion did before. Signed-off-by: Kevin Wolf <kwolf@redhat.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
2018-03-19luks: Support .bdrv_co_createKevin Wolf
This adds the .bdrv_co_create driver callback to luks, which enables image creation over QMP. Signed-off-by: Kevin Wolf <kwolf@redhat.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
2018-03-19luks: Create block_crypto_co_create_generic()Kevin Wolf
Everything that refers to the protocol layer or QemuOpts is moved out of block_crypto_create_generic(), so that the remaining function is suitable to be called by a .bdrv_co_create implementation. LUKS is the only driver that actually implements the old interface, and we don't intend to use it in any new drivers, so put the moved out code directly into a LUKS function rather than creating a generic intermediate one. Signed-off-by: Kevin Wolf <kwolf@redhat.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com>
2018-03-19luks: Separate image file creation from formattingKevin Wolf
The crypto driver used to create the image file in a callback from the crypto subsystem. If we want to implement .bdrv_co_create, this needs to go away because that callback will get a reference to an already existing block node. Move the image file creation to block_crypto_create_generic(). Signed-off-by: Kevin Wolf <kwolf@redhat.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com>
2018-03-19blockjobs: add block-job-finalizeJohn Snow
Instead of automatically transitioning from PENDING to CONCLUDED, gate the .prepare() and .commit() phases behind an explicit acknowledgement provided by the QMP monitor if auto_finalize = false has been requested. This allows us to perform graph changes in prepare and/or commit so that graph changes do not occur autonomously without knowledge of the controlling management layer. Transactions that have reached the "PENDING" state together can all be moved to invoke their finalization methods by issuing block_job_finalize to any one job in the transaction. Jobs in a transaction with mixed job->auto_finalize settings will all remain stuck in the "PENDING" state, as if the entire transaction was specified with auto_finalize = false. Jobs that specified auto_finalize = true, however, will still not emit the PENDING event. Signed-off-by: John Snow <jsnow@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2018-03-19blockjobs: ensure abort is called for cancelled jobsJohn Snow
Presently, even if a job is canceled post-completion as a result of a failing peer in a transaction, it will still call .commit because nothing has updated or changed its return code. The reason why this does not cause problems currently is because backup's implementation of .commit checks for cancellation itself. I'd like to simplify this contract: (1) Abort is called if the job/transaction fails (2) Commit is called if the job/transaction succeeds To this end: A job's return code, if 0, will be forcibly set as -ECANCELED if that job has already concluded. Remove the now redundant check in the backup job implementation. We need to check for cancellation in both block_job_completed AND block_job_completed_single, because jobs may be cancelled between those two calls; for instance in transactions. This also necessitates an ABORTING -> ABORTING transition to be allowed. The check in block_job_completed could be removed, but there's no point in starting to attempt to succeed a transaction that we know in advance will fail. This does NOT affect mirror jobs that are "canceled" during their synchronous phase. The mirror job itself forcibly sets the canceled property to false prior to ceding control, so such cases will invoke the "commit" callback. Signed-off-by: John Snow <jsnow@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2018-03-19blockjobs: add block_job_dismissJohn Snow
For jobs that have reached their CONCLUDED state, prior to having their last reference put down (meaning jobs that have completed successfully, unsuccessfully, or have been canceled), allow the user to dismiss the job's lingering status report via block-job-dismiss. This gives management APIs the chance to conclusively determine if a job failed or succeeded, even if the event broadcast was missed. Note: block_job_do_dismiss and block_job_decommission happen to do exactly the same thing, but they're called from different semantic contexts, so both aliases are kept to improve readability. Note 2: Don't worry about the 0x04 flag definition for AUTO_DISMISS, she has a friend coming in a future patch to fill the hole where 0x02 is. Verbs: Dismiss: operates on CONCLUDED jobs only. Signed-off-by: John Snow <jsnow@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2018-03-19blockjobs: add block_job_verb permission tableJohn Snow
Which commands ("verbs") are appropriate for jobs in which state is also somewhat burdensome to keep track of. As of this commit, it looks rather useless, but begins to look more interesting the more states we add to the STM table. A recurring theme is that no verb will apply to an 'undefined' job. Further, it's not presently possible to restrict the "pause" or "resume" verbs any more than they are in this commit because of the asynchronous nature of how jobs enter the PAUSED state; justifications for some seemingly erroneous applications are given below. ===== Verbs ===== Cancel: Any state except undefined. Pause: Any state except undefined; 'created': Requests that the job pauses as it starts. 'running': Normal usage. (PAUSED) 'paused': The job may be paused for internal reasons, but the user may wish to force an indefinite user-pause, so this is allowed. 'ready': Normal usage. (STANDBY) 'standby': Same logic as above. Resume: Any state except undefined; 'created': Will lift a user's pause-on-start request. 'running': Will lift a pause request before it takes effect. 'paused': Normal usage. 'ready': Will lift a pause request before it takes effect. 'standby': Normal usage. Set-speed: Any state except undefined, though ready may not be meaningful. Complete: Only a 'ready' job may accept a complete request. ======= Changes ======= (1) To facilitate "nice" error checking, all five major block-job verb interfaces in blockjob.c now support an errp parameter: - block_job_user_cancel is added as a new interface. - block_job_user_pause gains an errp paramter - block_job_user_resume gains an errp parameter - block_job_set_speed already had an errp parameter. - block_job_complete already had an errp parameter. (2) block-job-pause and block-job-resume will no longer no-op when trying to pause an already paused job, or trying to resume a job that isn't paused. These functions will now report that they did not perform the action requested because it was not possible. iotests have been adjusted to address this new behavior. (3) block-job-complete doesn't worry about checking !block_job_started, because the permission table guards against this. (4) test-bdrv-drain's job implementation needs to announce that it is 'ready' now, in order to be completed. Signed-off-by: John Snow <jsnow@redhat.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2018-03-19blockjobs: add state transition tableJohn Snow
The state transition table has mostly been implied. We're about to make it a bit more complex, so let's make the STM explicit instead. Perform state transitions with a function that for now just asserts the transition is appropriate. Transitions: Undefined -> Created: During job initialization. Created -> Running: Once the job is started. Jobs cannot transition from "Created" to "Paused" directly, but will instead synchronously transition to running to paused immediately. Running -> Paused: Normal workflow for pauses. Running -> Ready: Normal workflow for jobs reaching their sync point. (e.g. mirror) Ready -> Standby: Normal workflow for pausing ready jobs. Paused -> Running: Normal resume. Standby -> Ready: Resume of a Standby job. +---------+ |UNDEFINED| +--+------+ | +--v----+ |CREATED| +--+----+ | +--v----+ +------+ |RUNNING<----->PAUSED| +--+----+ +------+ | +--v--+ +-------+ |READY<------->STANDBY| +-----+ +-------+ Notably, there is no state presently defined as of this commit that deals with a job after the "running" or "ready" states, so this table will be adjusted alongside the commits that introduce those states. Signed-off-by: John Snow <jsnow@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2018-03-19blockjobs: model single jobs as transactionsJohn Snow
model all independent jobs as single job transactions. It's one less case we have to worry about when we add more states to the transition machine. This way, we can just treat all job lifetimes exactly the same. This helps tighten assertions of the STM graph and removes some conditionals that would have been needed in the coming commits adding a more explicit job lifetime management API. Signed-off-by: John Snow <jsnow@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2018-03-16Merge remote-tracking branch 'remotes/jnsnow/tags/bitmaps-pull-request' into ↵Peter Maydell
staging # gpg: Signature made Tue 13 Mar 2018 21:11:43 GMT # gpg: using RSA key 7DEF8106AAFC390E # gpg: Good signature from "John Snow (John Huston) <jsnow@redhat.com>" # Primary key fingerprint: FAEB 9711 A12C F475 812F 18F2 88A9 064D 1835 61EB # Subkey fingerprint: F9B7 ABDB BCAC DF95 BE76 CBD0 7DEF 8106 AAFC 390E * remotes/jnsnow/tags/bitmaps-pull-request: iotests: add dirty bitmap postcopy test iotests: add dirty bitmap migration test migration: add postcopy migration of dirty bitmaps migration: allow qmp command migrate-start-postcopy for any postcopy migration: add is_active_iterate handler migration/qemu-file: add qemu_put_counted_string() migration: include migrate_dirty_bitmaps in migrate_postcopy qapi: add dirty-bitmaps migration capability migration: introduce postcopy-only pending dirty-bitmap: add locked state block/dirty-bitmap: add _locked version of bdrv_reclaim_dirty_bitmap block/dirty-bitmap: fix locking in bdrv_reclaim_dirty_bitmap block/dirty-bitmap: add bdrv_dirty_bitmap_enable_successor() Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
2018-03-16Merge remote-tracking branch 'remotes/ericb/tags/pull-nbd-2018-03-13-v2' ↵Peter Maydell
into staging nbd patches for 2018-03-13 - Eric Blake: iotests: Fix stuck NBD process on 33 - Vladimir Sementsov-Ogievskiy: 0/5 nbd server fixing and refactoring before BLOCK_STATUS - Eric Blake: nbd/server: Honor FUA request on NBD_CMD_TRIM - Stefan Hajnoczi: 0/2 block: fix nbd-server-stop crash after blockdev-snapshot-sync - Vladimir Sementsov-Ogievskiy: nbd block status base:allocation # gpg: Signature made Tue 13 Mar 2018 20:48:37 GMT # gpg: using RSA key A7A16B4A2527436A # gpg: Good signature from "Eric Blake <eblake@redhat.com>" # gpg: aka "Eric Blake (Free Software Programmer) <ebb9@byu.net>" # gpg: aka "[jpeg image of size 6874]" # Primary key fingerprint: 71C2 CC22 B1C4 6029 27D2 F3AA A7A1 6B4A 2527 436A * remotes/ericb/tags/pull-nbd-2018-03-13-v2: iotests: new test 209 for NBD BLOCK_STATUS iotests: add file_path helper iotests.py: tiny refactor: move system imports up nbd: BLOCK_STATUS for standard get_block_status function: client part block/nbd-client: save first fatal error in nbd_iter_error nbd: BLOCK_STATUS for standard get_block_status function: server part nbd/server: add nbd_read_opt_name helper nbd/server: add nbd_opt_invalid helper iotests: add 208 nbd-server + blockdev-snapshot-sync test case block: let blk_add/remove_aio_context_notifier() tolerate BDS changes nbd/server: Honor FUA request on NBD_CMD_TRIM nbd/server: refactor nbd_trip: split out nbd_handle_request nbd/server: refactor nbd_trip: cmd_read and generic reply nbd/server: fix: check client->closing before sending reply nbd/server: fix sparse read nbd/server: move nbd_co_send_structured_error up iotests: Fix stuck NBD process on 33 Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
2018-03-13dirty-bitmap: add locked stateVladimir Sementsov-Ogievskiy
Add special state, when qmp operations on the bitmap are disabled. It is needed during bitmap migration. "Frozen" state is not appropriate here, because it looks like bitmap is unchanged. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Reviewed-by: John Snow <jsnow@redhat.com> Message-id: 20180207155837.92351-5-vsementsov@virtuozzo.com Signed-off-by: John Snow <jsnow@redhat.com>
2018-03-13block/dirty-bitmap: add _locked version of bdrv_reclaim_dirty_bitmapVladimir Sementsov-Ogievskiy
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Reviewed-by: John Snow <jsnow@redhat.com> Message-id: 20180207155837.92351-4-vsementsov@virtuozzo.com Signed-off-by: John Snow <jsnow@redhat.com>
2018-03-13block/dirty-bitmap: fix locking in bdrv_reclaim_dirty_bitmapVladimir Sementsov-Ogievskiy
Like other setters here these functions should take a lock. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Reviewed-by: Fam Zheng <famz@redhat.com> Reviewed-by: John Snow <jsnow@redhat.com> Message-id: 20180207155837.92351-3-vsementsov@virtuozzo.com Signed-off-by: John Snow <jsnow@redhat.com>
2018-03-13nbd: BLOCK_STATUS for standard get_block_status function: client partVladimir Sementsov-Ogievskiy
Minimal realization: only one extent in server answer is supported. Flag NBD_CMD_FLAG_REQ_ONE is used to force this behavior. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Message-Id: <20180312152126.286890-6-vsementsov@virtuozzo.com> Reviewed-by: Eric Blake <eblake@redhat.com> [eblake: grammar tweaks, fix min_block check and 32-bit cap, use -1 instead of errno on failure in nbd_negotiate_simple_meta_context, ensure that block status makes progress on success] Signed-off-by: Eric Blake <eblake@redhat.com>
2018-03-13block/nbd-client: save first fatal error in nbd_iter_errorVladimir Sementsov-Ogievskiy
It is ok, that fatal error hides previous not fatal, but hiding first fatal error is a bad feature. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Reviewed-by: Eric Blake <eblake@redhat.com> Message-Id: <20180312152126.286890-5-vsementsov@virtuozzo.com> Signed-off-by: Eric Blake <eblake@redhat.com>