[libvirt] [PATCH] Add actions to virDomainLifecycle enum

[Eric Blake]

On 07/29/2010 03:54 PM, Jim Fehlig wrote:
> I'm not too fond of the schema change but thus far haven't found a way
> to condense it.  Suggestions welcomed :-).

I don't see any of DV's promised comments, and it obviously didn't make
it into 0.8.3 :(

>>From a27589eb861fd487cb07e537b5da25125599e8a5 Mon Sep 17 00:00:00 2001
> From: Jim Fehlig <jfehlig at novell.com>
> Date: Thu, 29 Jul 2010 12:21:47 -0600
> Subject: [PATCH] Add actions to virDomainLifecycle enum
> 
> Xen supports on_crash actions coredump-{destroy,restart}.  libvirt
> cannot parse config returned by xend that contains either of these
> actions
> 
> xen52 # xm li -l test | grep on_crash
>     (on_crash coredump-restart)
> xen52 # virsh dumpxml test
> error: internal error unknown lifecycle type coredump-restart
> 
> This patch includes these additional actions in virDomainLifecycle
> enum.  Docs have also been updated, although the schema changes
> might be further collapsed.
> ---
>  docs/formatdomain.html.in |   14 ++++++++++++++
>  docs/schemas/domain.rng   |   24 +++++++++++++++++++++++-

Thanks for remembering the docs alongside the patch!

> +    <p>
> +      on_crash supports these additional actions.

Should we add a <since>0.8.4</since> tag here?

> +    </p>
> +
> +    <dl>
> +      <dt><code>coredump-destroy</code></dt>
> +      <dd>The crashed domain's core will be dumped, and then the
> +        domain will be terminated completely and all resources
> +        released</dd>
> +      <dt><code>coredump-restart</code></dt>
> +      <dd>The crashed domain's core will be dumped, and then the
> +        domain will be restarted with the same configuration</dd>
> +    </dl>
> +
>      <h3><a name="elementsFeatures">Hypervisor features</a></h3>
>  
>      <p>
> diff --git a/docs/schemas/domain.rng b/docs/schemas/domain.rng
> index b2783b0..d384652 100644
> --- a/docs/schemas/domain.rng
> +++ b/docs/schemas/domain.rng
> @@ -1177,7 +1177,7 @@
>        </optional>
>        <optional>
>          <element name="on_crash">
> -          <ref name="offOptions"/>
> +          <ref name="crashOptions"/>
>          </element>
>        </optional>
>      </interleave>
> @@ -1199,6 +1199,28 @@
>      </choice>
>    </define>
>    <!--
> +      Options when a domain crashes:
> +      destroy: The domain is cleaned up
> +      restart: A new domain is started in place of the old one
> +      preserve: The domain will remain in memory until it is destroyed manually
> +      rename-restart: a variant of the previous one but where the old domain is
> +                      renamed before being saved to allow a restart
> +      coredump-destroy: The crashed domain's core will be dumped, and then the
> +                        domain will be terminated completely and all resources
> +                        released
> +      coredump-restart: The crashed domain's core will be dumped, and then the                               domain will be restarted with the same configuration
> +    -->
> +  <define name="crashOptions">
> +    <choice>
> +      <value>destroy</value>
> +      <value>restart</value>
> +      <value>preserve</value>
> +      <value>rename-restart</value>
> +      <value>coredump-destroy</value>
> +      <value>coredump-restart</value>
> +    </choice>
> +  </define>

I don't know if it works to inline <ref name="offOptions"/> into the
<choice> block rather than open-coding the first four <value>s, but I'm
assuming it doesn't, and that your fears about this being a non-optimal
.rng representation are unfounded.

> +++ b/src/conf/domain_conf.c
> @@ -81,7 +81,9 @@ VIR_ENUM_IMPL(virDomainLifecycle, VIR_DOMAIN_LIFECYCLE_LAST,
>                "destroy",
>                "restart",
>                "rename-restart",
> -              "preserve")
> +              "preserve",
> +              "coredump-destroy",
> +              "coredump-restart")

Hmm.  These two new values are only valid for on_crash, but I don't see
any code that rejects them for on_reboot or on_poweroff.  Do we need a
separate enum here, or do we just need to add better checking to the
remaining clients to detect enum values they can't support?

So, we'll need a v2 of the patch, once you've sorted the answer to that
question.

-- 
Eric Blake   eblake at redhat.com    +1-801-349-2682
Libvirt virtualization library http://libvirt.org

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 620 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20100810/14fea709/attachment-0001.sig>