[libvirt] [PATCH 11/13] virsh: Refactor block job waiting in cmdBlockPull

John Ferlan jferlan at redhat.com
Mon Jul 20 20:47:54 UTC 2015



On 07/15/2015 12:34 PM, Peter Krempa wrote:
> Introduce helper function that will provide logic for waiting for block
> job completion so the 3 open coded places can be unified and improved.
> 
> This patch introduces the whole logic and uses it to fix
> cmdBlockJobPull. The vshBlockJobWait funtion provides common logic for
> block job waiting that should be robust enough to work across all
> previous versions of libvirt. Since virsh allows to pass user-provided
> strings as paths of block devices we can't reliably use block job events
> for detection of block job states so the function contains a great deal
> of fallback logic.
> ---
>  tools/virsh-domain.c | 326 ++++++++++++++++++++++++++++++++++++++-------------
>  1 file changed, 244 insertions(+), 82 deletions(-)
> 

...

>  /*
>   * "blockcommit" command
>   */
> @@ -2660,19 +2881,11 @@ cmdBlockPull(vshControl *ctl, const vshCmd *cmd)
>      bool verbose = vshCommandOptBool(cmd, "verbose");
>      bool async = vshCommandOptBool(cmd, "async");
>      int timeout = 0;
> -    struct sigaction sig_action;
> -    struct sigaction old_sig_action;
> -    sigset_t sigmask, oldsigmask;
> -    struct timeval start;
> -    struct timeval curr;
>      const char *path = NULL;
>      const char *base = NULL;
>      unsigned long bandwidth = 0;
> -    bool quit = false;
> -    int abort_flags = 0;
> -    int status = -1;
> -    int cb_id = -1;
>      unsigned int flags = 0;
> +    vshBlockJobWaitDataPtr bjWait = NULL;
> 
>      VSH_REQUIRE_OPTION("verbose", "wait");
>      VSH_REQUIRE_OPTION("async", "wait");
> @@ -2694,35 +2907,13 @@ cmdBlockPull(vshControl *ctl, const vshCmd *cmd)
>      if (vshCommandOptBool(cmd, "keep-relative"))
>          flags |= VIR_DOMAIN_BLOCK_REBASE_RELATIVE;
> 
> -    if (async)
> -        abort_flags |= VIR_DOMAIN_BLOCK_JOB_ABORT_ASYNC;
> -
> -    if (blocking) {
> -        sigemptyset(&sigmask);
> -        sigaddset(&sigmask, SIGINT);
> -
> -        intCaught = 0;
> -        sig_action.sa_sigaction = vshCatchInt;
> -        sig_action.sa_flags = SA_SIGINFO;
> -        sigemptyset(&sig_action.sa_mask);
> -        sigaction(SIGINT, &sig_action, &old_sig_action);
> -
> -        GETTIMEOFDAY(&start);
> -    }
> -
>      if (!(dom = vshCommandOptDomain(ctl, cmd, NULL)))
>          return false;
> 
> -    virConnectDomainEventGenericCallback cb =
> -        VIR_DOMAIN_EVENT_CALLBACK(vshBlockJobStatusHandler);
> -
> -    if ((cb_id = virConnectDomainEventRegisterAny(ctl->conn,
> -                                                  dom,
> -                                                  VIR_DOMAIN_EVENT_ID_BLOCK_JOB,
> -                                                  cb,
> -                                                  &status,
> -                                                  NULL)) < 0)
> -        vshResetLibvirtError();
> +    if (blocking &&
> +        !(bjWait = vshBlockJobWaitInit(ctl, dom, path, _("Block Pull"), verbose,
> +                                       timeout, async)))
> +        goto cleanup;
> 
>      if (base || flags) {
>          if (virDomainBlockRebase(dom, path, base, bandwidth, flags) < 0)
> @@ -2738,61 +2929,32 @@ cmdBlockPull(vshControl *ctl, const vshCmd *cmd)
>          goto cleanup;
>      }
> 
> -    while (blocking) {
> -        virDomainBlockJobInfo info;
> -        int result;
> -
> -        pthread_sigmask(SIG_BLOCK, &sigmask, &oldsigmask);
> -        result = virDomainGetBlockJobInfo(dom, path, &info, 0);
> -        pthread_sigmask(SIG_SETMASK, &oldsigmask, NULL);
> +    /* Execution continues here only if --wait or friends were specifed */

specified

This is also repeated in 12/13 and 13/13


> +    switch (vshBlockJobWait(bjWait)) {
> +        case -1:
> +            goto cleanup;
> 
> -        if (result < 0) {
> -            vshError(ctl, _("failed to query job for disk %s"), path);
> +        case VIR_DOMAIN_BLOCK_JOB_CANCELED:
> +            vshPrint(ctl, "\n%s", _("Pull aborted"));
>              goto cleanup;
> -        }
> -        if (result == 0)
>              break;
> 
> -        if (verbose)
> -            vshPrintJobProgress(_("Block Pull"), info.end - info.cur, info.end);
> -
> -        GETTIMEOFDAY(&curr);
> -        if (intCaught || (timeout &&
> -                          (((int)(curr.tv_sec - start.tv_sec)  * 1000 +
> -                            (int)(curr.tv_usec - start.tv_usec) / 1000) >
> -                           timeout))) {
> -            vshDebug(ctl, VSH_ERR_DEBUG,
> -                     intCaught ? "interrupted" : "timeout");
> -            intCaught = 0;
> -            timeout = 0;
> -            status = VIR_DOMAIN_BLOCK_JOB_CANCELED;
> -            if (virDomainBlockJobAbort(dom, path, abort_flags) < 0) {
> -                vshError(ctl, _("failed to abort job for disk %s"), path);
> -                goto cleanup;
> -            }
> -            if (abort_flags & VIR_DOMAIN_BLOCK_JOB_ABORT_ASYNC)
> -                break;
> -        } else {
> -            usleep(500 * 1000);
> -        }
> -    }
> -
> -    if (status == VIR_DOMAIN_BLOCK_JOB_CANCELED)
> -        quit = true;
> +        case VIR_DOMAIN_BLOCK_JOB_FAILED:
> +            vshPrint(ctl, "\n%s", _("Pull failed"));
> +            goto cleanup;
> +            break;
> 
> -    if (verbose && !quit) {
> -        /* printf [100 %] */
> -        vshPrintJobProgress(_("Block Pull"), 0, 1);
> +        case VIR_DOMAIN_BLOCK_JOB_READY:
> +        case VIR_DOMAIN_BLOCK_JOB_COMPLETED:
> +            vshPrint(ctl, "\n%s", _("Pull complete"));
> +            break;
>      }
> -    vshPrint(ctl, "\n%s", quit ? _("Pull aborted") : _("Pull complete"));
> 
>      ret = true;
> +
>   cleanup:
>      virDomainFree(dom);
> -    if (blocking)
> -        sigaction(SIGINT, &old_sig_action, NULL);
> -    if (cb_id >= 0)
> -        virConnectDomainEventDeregisterAny(ctl->conn, cb_id);
> +    vshBlockJobWaitFree(bjWait);
>      return ret;
>  }
> 




More information about the libvir-list mailing list