Skip to content
Snippets Groups Projects
  1. Jan 08, 2022
  2. Jan 07, 2022
  3. Dec 21, 2021
  4. Nov 16, 2021
    • Hanna Reitz's avatar
      transactions: Invoke clean() after everything else · 079bff69
      Hanna Reitz authored
      
      Invoke the transaction drivers' .clean() methods only after all
      .commit() or .abort() handlers are done.
      
      This makes it easier to have nested transactions where the top-level
      transactions pass objects to lower transactions that the latter can
      still use throughout their commit/abort phases, while the top-level
      transaction keeps a reference that is released in its .clean() method.
      
      (Before this commit, that is also possible, but the top-level
      transaction would need to take care to invoke tran_add() before the
      lower-level transaction does.  This commit makes the ordering
      irrelevant, which is just a bit nicer.)
      
      Signed-off-by: default avatarHanna Reitz <hreitz@redhat.com>
      Message-Id: <20211111120829.81329-8-hreitz@redhat.com>
      Reviewed-by: default avatarVladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
      Signed-off-by: default avatarKevin Wolf <kwolf@redhat.com>
      Message-Id: <20211115145409.176785-8-kwolf@redhat.com>
      Signed-off-by: default avatarHanna Reitz <hreitz@redhat.com>
      079bff69
  5. Nov 10, 2021
  6. Nov 08, 2021
  7. Nov 04, 2021
  8. Nov 02, 2021
  9. Oct 28, 2021
  10. Oct 07, 2021
    • Hanna Reitz's avatar
      job: Add job_cancel_requested() · 08b83bff
      Hanna Reitz authored
      Most callers of job_is_cancelled() actually want to know whether the job
      is on its way to immediate termination.  For example, we refuse to pause
      jobs that are cancelled; but this only makes sense for jobs that are
      really actually cancelled.
      
      A mirror job that is cancelled during READY with force=false should
      absolutely be allowed to pause.  This "cancellation" (which is actually
      a kind of completion) may take an indefinite amount of time, and so
      should behave like any job during normal operation.  For example, with
      on-target-error=stop, the job should stop on write errors.  (In
      contrast, force-cancelled jobs should not get write errors, as they
      should just terminate and not do further I/O.)
      
      Therefore, redefine job_is_cancelled() to only return true for jobs that
      are force-cancelled (which as of HEAD^ means any job that interprets the
      cancellation request as a request for immediate termination), and add
      job_cancel_requested() as the general variant, which returns true for
      any jobs which have been requested to be cancelled, whether it be
      immediately or after an arbitrarily long completion phase.
      
      Finally, here is a justification for how different job_is_cancelled()
      invocations are treated by this patch:
      
      - block/mirror.c (mirror_run()):
        - The first invocation is a while loop that should loop until the job
          has been cancelled or scheduled for completion.  What kind of cancel
          does not matter, only the fact that the job is supposed to end.
      
        - The second invocation wants to know whether the job has been
          soft-cancelled.  Calling job_cancel_requested() is a bit too broad,
          but if the job were force-cancelled, we should leave the main loop
          as soon as possible anyway, so this should not matter here.
      
        - The last two invocations already check force_cancel, so they should
          continue to use job_is_cancelled().
      
      - block/backup.c, block/commit.c, block/stream.c, anything in tests/:
        These jobs know only force-cancel, so there is no difference between
        job_is_cancelled() and job_cancel_requested().  We can continue using
        job_is_cancelled().
      
      - job.c:
        - job_pause_point(), job_yield(), job_sleep_ns(): Only force-cancelled
          jobs should be prevented from being paused.  Continue using job_is_cancelled().
      
        - job_update_rc(), job_finalize_single(), job_finish_sync(): These
          functions are all called after the job has left its main loop.  The
          mirror job (the only job that can be soft-cancelled) will clear
          .cancelled before leaving the main loop if it has been
          soft-cancelled.  Therefore, these functions will observe .cancelled
          to be true only if the job has been force-cancelled.  We can
          continue to use job_is_cancelled().
          (Furthermore, conceptually, a soft-cancelled mirror job should not
          report to have been cancelled.  It should report completion (see
          also the block-job-cancel QAPI documentation).  Therefore, it makes
          sense for these functions not to distinguish between a
          soft-cancelled mirror job and a job that has completed as normal.)
      
        - job_completed_txn_abort(): All jobs other than @job have been
          force-cancelled.  job_is_cancelled() must be true for them.
          Regarding @job itself: job_completed_txn_abort() is mostly called
          when the job's return value is not 0.  A soft-cancelled mirror has a
          return value of 0, and so will not end up here then.
          However, job_cancel() invokes job_completed_txn_abort() if the job
          has been deferred to the main loop, which is mostly the case for
          completed jobs (which skip the assertion), but not for sure.
          To be safe, use job_cancel_requested() in this assertion.
      
        - job_complete(): This is function eventually invoked by the user
          (through qmp_block_job_complete() or qmp_job_complete(), or
          job_complete_sync(), which comes from qemu-img).  The intention here
          is to prevent a user from invoking job-complete after the job has
          been cancelled.  This should also apply to soft cancelling: After a
          mirror job has been soft-cancelled, the user should not be able to
          decide otherwise and have it complete as normal (i.e. pivoting to
          the target).
      
        - job_cancel(): Both functions are equivalent (see comment there), but
          we want to use job_is_cancelled(), because this shows that we call
          job_completed_txn_abort() only for force-cancelled jobs.  (As
          explained for job_update_rc(), soft-cancelled jobs should be treated
          as if they have completed as normal.)
      
      Buglink: https://gitlab.com/qemu-project/qemu/-/issues/462
      
      
      Signed-off-by: default avatarHanna Reitz <hreitz@redhat.com>
      Reviewed-by: default avatarEric Blake <eblake@redhat.com>
      Reviewed-by: default avatarVladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
      Message-Id: <20211006151940.214590-9-hreitz@redhat.com>
      Signed-off-by: default avatarVladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
      08b83bff
    • Hanna Reitz's avatar
      jobs: Give Job.force_cancel more meaning · 73895f38
      Hanna Reitz authored
      
      We largely have two cancel modes for jobs:
      
      First, there is actual cancelling.  The job is terminated as soon as
      possible, without trying to reach a consistent result.
      
      Second, we have mirror in the READY state.  Technically, the job is not
      really cancelled, but it just is a different completion mode.  The job
      can still run for an indefinite amount of time while it tries to reach a
      consistent result.
      
      We want to be able to clearly distinguish which cancel mode a job is in
      (when it has been cancelled).  We can use Job.force_cancel for this, but
      right now it only reflects cancel requests from the user with
      force=true, but clearly, jobs that do not even distinguish between
      force=false and force=true are effectively always force-cancelled.
      
      So this patch has Job.force_cancel signify whether the job will
      terminate as soon as possible (force_cancel=true) or whether it will
      effectively remain running despite being "cancelled"
      (force_cancel=false).
      
      To this end, we let jobs that provide JobDriver.cancel() tell the
      generic job code whether they will terminate as soon as possible or not,
      and for jobs that do not provide that method we assume they will.
      
      Signed-off-by: default avatarHanna Reitz <hreitz@redhat.com>
      Reviewed-by: default avatarEric Blake <eblake@redhat.com>
      Reviewed-by: default avatarVladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
      Reviewed-by: default avatarKevin Wolf <kwolf@redhat.com>
      Message-Id: <20211006151940.214590-7-hreitz@redhat.com>
      Signed-off-by: default avatarVladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
      73895f38
    • Hanna Reitz's avatar
      job: @force parameter for job_cancel_sync() · 4cfb3f05
      Hanna Reitz authored
      Callers should be able to specify whether they want job_cancel_sync() to
      force-cancel the job or not.
      
      In fact, almost all invocations do not care about consistency of the
      result and just want the job to terminate as soon as possible, so they
      should pass force=true.  The replication block driver is the exception,
      specifically the active commit job it runs.
      
      As for job_cancel_sync_all(), all callers want it to force-cancel all
      jobs, because that is the point of it: To cancel all remaining jobs as
      quickly as possible (generally on process termination).  So make it
      invoke job_cancel_sync() with force=true.
      
      This changes some iotest outputs, because quitting qemu while a mirror
      job is active will now lead to it being cancelled instead of completed,
      which is what we want.  (Cancelling a READY mirror job with force=false
      may take an indefinite amount of time, which we do not want when
      quitting.  If users want consistent results, they must have all jobs be
      done before they quit qemu.)
      
      Buglink: https://gitlab.com/qemu-project/qemu/-/issues/462
      
      
      Signed-off-by: default avatarHanna Reitz <hreitz@redhat.com>
      Reviewed-by: default avatarEric Blake <eblake@redhat.com>
      Reviewed-by: default avatarVladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
      Message-Id: <20211006151940.214590-6-hreitz@redhat.com>
      Signed-off-by: default avatarVladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
      4cfb3f05
  11. Oct 05, 2021
  12. Sep 29, 2021
  13. Sep 07, 2021
  14. Sep 02, 2021
  15. Aug 27, 2021
  16. Jul 26, 2021
  17. Jul 23, 2021
  18. Jul 21, 2021
  19. Jul 14, 2021
  20. Jul 09, 2021
  21. Jul 06, 2021
  22. Jul 05, 2021
    • Stefan Hajnoczi's avatar
      util/async: add a human-readable name to BHs for debugging · 0f08586c
      Stefan Hajnoczi authored
      
      It can be difficult to debug issues with BHs in production environments.
      Although BHs can usually be identified by looking up their ->cb()
      function pointer, this requires debug information for the program. It is
      also not possible to print human-readable diagnostics about BHs because
      they have no identifier.
      
      This patch adds a name to each BH. The name is not unique per instance
      but differentiates between cb() functions, which is usually enough. It's
      done by changing aio_bh_new() and friends to macros that stringify cb.
      
      The next patch will use the name field when reporting leaked BHs.
      
      Signed-off-by: default avatarStefan Hajnoczi <stefanha@redhat.com>
      Reviewed-by: default avatarPhilippe Mathieu-Daudé <philmd@redhat.com>
      Message-Id: <20210414200247.917496-2-stefanha@redhat.com>
      0f08586c
Loading