Re: [PATCH] Use PAUSED state for domains that are starting up

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Jiri Denemark wrote:
> When libvirt is starting a domain, it reports the state as SHUTOFF until
> it's RUNNING. This is not ideal because domain startup may take a long
> time (usually because of some configuration issues, firewalls blocking
> access to network disks, etc.) and domain lists provided by libvirt look
> awkward. One can see weird shutoff domains with IDs in a list of active
> domains or even shutoff transient domains. In any case, it looks more
> like a bug in libvirt than a normal state a domain goes through.
>   

I recently noticed similar behavior in the libxl driver when starting
large memory domains, which can take considerable time.

> Signed-off-by: Jiri Denemark <jdenemar@xxxxxxxxxx>
> ---
>
> After going through all hypervisor drivers it seems none of them needs
> to be updated. That is, this patch matches the one I sent as RFC for
> design discussions.
>
>  include/libvirt/libvirt-domain.h |  1 +
>  src/conf/domain_conf.c           |  3 ++-
>  src/qemu/qemu_process.c          | 22 ++++++++++++++--------
>  tools/virsh-domain-monitor.c     |  3 ++-
>  4 files changed, 19 insertions(+), 10 deletions(-)
>
> diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h
> index 9487b80..d5c148b 100644
> --- a/include/libvirt/libvirt-domain.h
> +++ b/include/libvirt/libvirt-domain.h
> @@ -116,6 +116,7 @@ typedef enum {
>      VIR_DOMAIN_PAUSED_SHUTTING_DOWN = 8, /* paused during shutdown process */
>      VIR_DOMAIN_PAUSED_SNAPSHOT = 9,      /* paused while creating a snapshot */
>      VIR_DOMAIN_PAUSED_CRASHED = 10,     /* paused due to a guest crash */
> +    VIR_DOMAIN_PAUSED_STARTING_UP = 11, /* the domain is being started */
>   

Cool.  I'll use this in libxlDomainStart() in the libxl driver.

>  
>  # ifdef VIR_ENUM_SENTINELS
>      VIR_DOMAIN_PAUSED_LAST
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index cc8616b..783c382 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -661,7 +661,8 @@ VIR_ENUM_IMPL(virDomainPausedReason, VIR_DOMAIN_PAUSED_LAST,
>                "from snapshot",
>                "shutdown",
>                "snapshot",
> -              "panicked")
> +              "panicked",
> +              "starting up")
>  
>  VIR_ENUM_IMPL(virDomainShutdownReason, VIR_DOMAIN_SHUTDOWN_LAST,
>                "unknown",
> diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
> index d1f089d..649b3df 100644
> --- a/src/qemu/qemu_process.c
> +++ b/src/qemu/qemu_process.c
> @@ -3418,6 +3418,7 @@ qemuProcessUpdateState(virQEMUDriverPtr driver, virDomainObjPtr vm)
>      virDomainState state;
>      virDomainPausedReason reason;
>      virDomainState newState = VIR_DOMAIN_NOSTATE;
> +    int oldReason;
>      int newReason;
>      bool running;
>      char *msg = NULL;
> @@ -3431,9 +3432,16 @@ qemuProcessUpdateState(virQEMUDriverPtr driver, virDomainObjPtr vm)
>      if (ret < 0)
>          return -1;
>  
> -    state = virDomainObjGetState(vm, NULL);
> +    state = virDomainObjGetState(vm, &oldReason);
>  
> -    if (state == VIR_DOMAIN_PAUSED && running) {
> +    if (running &&
> +        (state == VIR_DOMAIN_SHUTOFF ||
> +         (state == VIR_DOMAIN_PAUSED &&
> +          oldReason == VIR_DOMAIN_PAUSED_STARTING_UP))) {
> +        newState = VIR_DOMAIN_RUNNING;
> +        newReason = VIR_DOMAIN_RUNNING_BOOTED;
> +        ignore_value(VIR_STRDUP_QUIET(msg, "finished booting"));
> +    } else if (state == VIR_DOMAIN_PAUSED && running) {
>          newState = VIR_DOMAIN_RUNNING;
>          newReason = VIR_DOMAIN_RUNNING_UNPAUSED;
>          ignore_value(VIR_STRDUP_QUIET(msg, "was unpaused"));
> @@ -3452,10 +3460,6 @@ qemuProcessUpdateState(virQEMUDriverPtr driver, virDomainObjPtr vm)
>              ignore_value(virAsprintf(&msg, "was paused (%s)",
>                                   virDomainPausedReasonTypeToString(reason)));
>          }
> -    } else if (state == VIR_DOMAIN_SHUTOFF && running) {
> -        newState = VIR_DOMAIN_RUNNING;
> -        newReason = VIR_DOMAIN_RUNNING_BOOTED;
> -        ignore_value(VIR_STRDUP_QUIET(msg, "finished booting"));
>      }
>  
>      if (newState != VIR_DOMAIN_NOSTATE) {
> @@ -3828,7 +3832,9 @@ qemuProcessReconnect(void *opaque)
>          goto error;
>  
>      state = virDomainObjGetState(obj, &reason);
> -    if (state == VIR_DOMAIN_SHUTOFF) {
> +    if (state == VIR_DOMAIN_SHUTOFF ||
> +        (state == VIR_DOMAIN_PAUSED &&
> +         reason == VIR_DOMAIN_PAUSED_STARTING_UP)) {
>          VIR_DEBUG("Domain '%s' wasn't fully started yet, killing it",
>                    obj->def->name);
>          goto error;
> @@ -4459,7 +4465,7 @@ int qemuProcessStart(virConnectPtr conn,
>  
>      vm->def->id = qemuDriverAllocateID(driver);
>      qemuDomainSetFakeReboot(driver, vm, false);
> -    virDomainObjSetState(vm, VIR_DOMAIN_SHUTOFF, VIR_DOMAIN_SHUTOFF_UNKNOWN);
> +    virDomainObjSetState(vm, VIR_DOMAIN_PAUSED, VIR_DOMAIN_PAUSED_STARTING_UP);
>  
>      if (virAtomicIntInc(&driver->nactive) == 1 && driver->inhibitCallback)
>          driver->inhibitCallback(true, driver->inhibitOpaque);
> diff --git a/tools/virsh-domain-monitor.c b/tools/virsh-domain-monitor.c
> index 464ac11..30ddae4 100644
> --- a/tools/virsh-domain-monitor.c
> +++ b/tools/virsh-domain-monitor.c
> @@ -199,7 +199,8 @@ VIR_ENUM_IMPL(vshDomainPausedReason,
>                N_("from snapshot"),
>                N_("shutting down"),
>                N_("creating snapshot"),
> -              N_("crashed"))
> +              N_("crashed"),
> +              N_("starting up"))
>  
>  VIR_ENUM_DECL(vshDomainShutdownReason)
>   
> VIR_ENUM_IMPL(vshDomainShutdownReason,

I'm no expert in the qemu driver code, but this looks sane to me.  ACK.

Regards,
Jim

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list




[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]