doc: add "setup" to list of migration states

On a slow VM (e.g., nested), you see the "setup" state when you query the
migration status.

Signed-off-by: Peter Feiner <peter@gridcentric.ca>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
scripts/qapi.py: Avoid syntax not supported by Python 2.4

The Python "except Foo as x" syntax was only introduced in
Python 2.6, but we aim to support Python 2.4 and later.
Use the old-style "except Foo, x" syntax instead, thus
fixing configure/compile on systems with older Python.

Reported-by: Peter Maydell <peter.maydell@linaro.org>
Tested-by: Andreas Färber <andreas.faerber@web.de>
Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
qapi: zero-initialize all QMP command parameters

In general QMP command parameter values are specified by consumers of the
QMP/HMP interface, but in the case of optional parameters these values may
be left uninitialized.

It is considered a bug for code to make use of optional parameters that have
not been flagged as being present by the marshalling code (via corresponding
has_<parameter> parameter), however our marshalling code will still pass
these uninitialized values on to the corresponding QMP function (to then
be ignored). Some compilers (clang in particular) consider this unsafe
however, and generate warnings as a result. As reported by Peter Maydell:

  This is something clang's -fsanitize=undefined spotted. The
  code generated by qapi-commands.py in qmp-marshal.c for
  qmp_marshal_* functions where there are some optional
  arguments looks like this:

      bool has_force = false;
      bool force;

      mi = qmp_input_visitor_new_strict(QOBJECT(args));
      v = qmp_input_get_visitor(mi);
      visit_type_str(v, &device, "device", errp);
      visit_start_optional(v, &has_force, "force", errp);
      if (has_force) {
          visit_type_bool(v, &force, "force", errp);
      }
      visit_end_optional(v, errp);
      qmp_input_visitor_cleanup(mi);

      if (error_is_set(errp)) {
          goto out;
      }
      qmp_eject(device, has_force, force, errp);

  In the case where has_force is false, we never initialize
  force, but then we use it by passing it to qmp_eject.
  I imagine we don't then actually use the value, but clang
  complains in particular for 'bool' variables because the value
  that ends up being loaded from memory for 'force' is not either
  0 or 1 (being uninitialized stack contents).

Fix this by initializing all QMP command parameters to {0} in the
marshalling code prior to passing them on to the QMP functions.

Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
Reported-by: Peter Maydell <peter.maydell@linaro.org>
Tested-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
Merge remote-tracking branch 'remotes/qmp-unstable/queue/qmp' into staging

* remotes/qmp-unstable/queue/qmp:
  qapi: zero-initialize all QMP command parameters
  scripts/qapi.py: Avoid syntax not supported by Python 2.4
  doc: add "setup" to list of migration states

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
qemu-iotests: Handle cache mode option in 091

We should allow testing this on tmpfs. Any cache setting in iotests
should try to obey $CACHEMODE.

The cache mode is still "none" by default but overridable

Signed-off-by: Fam Zheng <famz@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
QemuOpt: add unit tests

Cover basic aspects and API usage for QemuOpt. The current implementation
covers the API's planned to be changed by Chunyan Liu in his QEMUOptionParameter
replacement/cleanup job.

Other APIs should be covered in future improvements.

Signed-off-by: Leandro Dorileo <l@dorileo.org>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
qcow2: Fix memory leak in COW error path

This triggers if bs->drv becomes NULL in a concurrent request. This is
currently only the case when corruption prevention kicks in (i.e. at
most once per image, and after that it produces I/O errors).

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
aio: Fix use-after-free in cancellation path

The current flow of canceling a thread from THREAD_ACTIVE state is:

  1) Caller wants to cancel a request, so it calls thread_pool_cancel.

  2) thread_pool_cancel waits on the conditional variable
     elem->check_cancel.

  3) The worker thread changes state to THREAD_DONE once the task is
     done, and notifies elem->check_cancel to allow thread_pool_cancel
     to continue execution, and signals the notifier (pool->notifier) to
     allow callback function to be called later. But because of the
     global mutex, the notifier won't get processed until step 4) and 5)
     are done.

  4) thread_pool_cancel continues, leaving the notifier signaled, it
     just returns to caller.

  5) Caller thinks the request is already canceled successfully, so it
     releases any related data, such as freeing elem->common.opaque.

  6) In the next main loop iteration, the notifier handler,
     event_notifier_ready, is called. It finds the canceled thread in
     THREAD_DONE state, so calls elem->common.cb, with an (likely)
     dangling opaque pointer. This is a use-after-free.

Fix it by calling event_notifier_ready before leaving
thread_pool_cancel.

Test case update: This change will let cancel complete earlier than
test-thread-pool.c expects, so update the code to check this case: if
it's already done, done_cb sets .aiocb to NULL, skip calling
bdrv_aio_cancel on them.

Reported-by: Ulrich Obergfell <uobergfe@redhat.com>
Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Fam Zheng <famz@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
iotests: Use _img_info in test 089

Currently, test 089 uses $QEMU_IMG info manually in order to obtain the
according output. However, the iotests should generally use _img_info as
this filters out more irrelevant information such as the host image size
or format specific information. Therefore, test 089 should use _img_info
as well.

Signed-off-by: Max Reitz <mreitz@redhat.com>
Reported-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
block: Add BlockOpType enum

This adds the enum of all the operations that can be taken on a block
device.

Signed-off-by: Fam Zheng <famz@redhat.com>
Reviewed-by: Benoit Canet <benoit@irqsave.net>
Reviewed-by: Jeff Cody <jcody@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
block: Introduce op_blockers to BlockDriverState

BlockDriverState.op_blockers is an array of lists with BLOCK_OP_TYPE_MAX
elements. Each list is a list of blockers of an operation type
(BlockOpType), that marks this BDS as currently blocked for a certain
type of operation with reason errors stored in the list. The rule of
usage is:

 * BDS user who wants to take an operation should check if there's any
   blocker of the type with bdrv_op_is_blocked().

 * BDS user who wants to block certain types of operation, should call
   bdrv_op_block (or bdrv_op_block_all to block all types of operations,
   which is similar to the existing bdrv_set_in_use()).

 * A blocker is only referenced by op_blockers, so the lifecycle is
   managed by caller, and shouldn't be lost until unblock, so typically
   a caller does these:

   - Allocate a blocker with error_setg or similar, call bdrv_op_block()
     to block some operations.
   - Hold the blocker, do his job.
   - Unblock operations that it blocked, with the same reason pointer
     passed to bdrv_op_unblock().
   - Release the blocker with error_free().

Signed-off-by: Fam Zheng <famz@redhat.com>
Reviewed-by: Benoit Canet <benoit@irqsave.net>
Reviewed-by: Jeff Cody <jcody@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
block: Replace in_use with operation blocker

This drops BlockDriverState.in_use with op_blockers:

  - Call bdrv_op_block_all in place of bdrv_set_in_use(bs, 1).

  - Call bdrv_op_unblock_all in place of bdrv_set_in_use(bs, 0).

  - Check bdrv_op_is_blocked() in place of bdrv_in_use(bs).

    The specific types are used, e.g. in place of starting block backup,
    bdrv_op_is_blocked(bs, BLOCK_OP_TYPE_BACKUP, ...).

    There is one exception in block_job_create, where
    bdrv_op_blocker_is_empty() is used, because we don't know the operation
    type here. This doesn't matter because in a few commits away we will drop
    the check and move it to callers that _do_ know the type.

  - Check bdrv_op_blocker_is_empty() in place of assert(!bs->in_use).

Note: there is only bdrv_op_block_all and bdrv_op_unblock_all callers at
this moment. So although the checks are specific to op types, this
changes can still be seen as identical logic with previously with
in_use. The difference is error message are improved because of blocker
error info.

Signed-off-by: Fam Zheng <famz@redhat.com>
Reviewed-by: Jeff Cody <jcody@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
block: Move op_blocker check from block_job_create to its caller

It makes no sense to check for "any" blocker on bs, we are here only
because of the mechanical conversion from in_use to op_blockers. Remove
it now, and let the callers check specific operation types. Backup and
mirror already have it, add checker to stream and commit.

Signed-off-by: Fam Zheng <famz@redhat.com>
Reviewed-by: Benoit Canet <benoit@irqsave.net>
Reviewed-by: Jeff Cody <jcody@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
block: Add bdrv_set_backing_hd()

This is the common but non-trivial steps to assign or change the
backing_hd of BDS.

Signed-off-by: Fam Zheng <famz@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Jeff Cody <jcody@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
block: Use bdrv_set_backing_hd everywhere

We need to handle the coming backing_blocker properly, so don't open
code the assignment, instead, call bdrv_set_backing_hd to change
backing_hd.

Signed-off-by: Fam Zheng <famz@redhat.com>
Reviewed-by: Jeff Cody <jcody@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
block: Add backing_blocker in BlockDriverState

This makes use of op_blocker and blocks all the operations except for
commit target, on each BlockDriverState->backing_hd.

The asserts for op_blocker in bdrv_swap are removed because with this
change, the target of block commit has at least the backing blocker of
its child, so the assertion is not true. Callers should do their check.

Signed-off-by: Fam Zheng <famz@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
block: Drop redundant bdrv_refresh_limits

The above bdrv_set_backing_hd already does this.

Signed-off-by: Fam Zheng <famz@redhat.com>
Reviewed-by: Jeff Cody <jcody@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
docs: Define refcount_bits value

The 'refcount_bits' term used in the description of refcount block entry is
not defined in the specification. The definition is added in the
'refcount_order' section where refcount_bits was used as 'width in bits'.

Signed-off-by: Maria Kustova <maria.k@catit.be>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
blockdev: Don't use qerror_report_err() in drive_init()

qerror_report_err() is a transitional interface to help with
converting existing HMP commands to QMP.  It should not be used
elsewhere.

drive_init() is not meant to be used by QMP commands.  It uses both
qerror_report_err() and error_report().  Convert the former to the
latter.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
blockdev: Don't use qerror_report() in do_drive_del()

qerror_report() is a transitional interface to help with converting
existing HMP commands to QMP.  It should not be used elsewhere.

do_drive_del() is an HMP command that won't be converted to QMP (we'll
create a new QMP command instead).  It uses both qerror_report() and
error_report().  Convert the former to the latter.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>