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@xxxxxxxxxx>
> 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@xxxxxxxxxx +1-801-349-2682
Libvirt virtualization library http://libvirt.org
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list