Re: [libvirt] [PATCH 1/1] Add disk error policy to domain XML
On 03/25/2010 09:15 AM, Daniel Veillard wrote:
On Wed, Mar 24, 2010 at 04:31:48PM -0400, David Allan wrote:
> ---
> docs/schemas/domain.rng | 12 +++++++++++-
> src/conf/domain_conf.c | 15 +++++++++++++++
> src/conf/domain_conf.h | 10 ++++++++++
> src/libvirt_private.syms | 2 +-
> src/qemu/qemu_conf.c | 12 +++++++++---
> tests/qemuhelptest.c | 1 +
> tests/qemuxml2argvtest.c | 3 +++
> 7 files changed, 50 insertions(+), 5 deletions(-)
>
> diff --git a/docs/schemas/domain.rng b/docs/schemas/domain.rng
> index 5a8c82b..b276da7 100644
> --- a/docs/schemas/domain.rng
> +++ b/docs/schemas/domain.rng
> @@ -521,7 +521,9 @@
> <ref name="driverCache"/>
> </group>
> </choice>
> -<empty/>
> +<optional>
> +<ref name="driverErrorPolicy"/>
> +</optional>
> </element>
> </define>
> <define name="driverFormat">
> @@ -543,6 +545,14 @@
> </choice>
> </attribute>
> </define>
> +<define name="driverErrorPolicy">
> +<attribute name="error_policy">
> +<choice>
> +<value>stop</value>
> +<value>ignore</value>
> +</choice>
> +</attribute>
> +</define>
> <define name="controller">
> <element name="controller">
> <choice>
okay, default being the attribute missing ...
I actually left default out because it's, well, the default; I ought to
have taken it out of the enum as well, but is there use in having it as
an actual value?
[...]
> --- a/src/qemu/qemu_conf.c
> +++ b/src/qemu/qemu_conf.c
> @@ -1215,10 +1215,8 @@ static unsigned long long qemudComputeCmdFlags(const char
*help,
>
> /* Keep disabled till we're actually ready to turn on JSON mode
> * The plan is todo it in 0.13.0 QEMU, but lets wait& see... */
> -#if 0
> - if (version>= 13000)
> + if (version>= 12000)
> flags |= QEMUD_CMD_FLAG_MONITOR_JSON;
> -#endif
>
> return flags;
> }
Hum, I assume it's a debugging left-over, please remove before commit
No, it wasn't a debugging leftover; I misunderstood when JSON went into
qemu. I'll put the #if 0 back, but is 13000 the correct version for the
not-enabled code, or do we not know the correct version yet? Where is
information available about what features are available in what versions
of qemu?
[...]
> diff --git a/tests/qemuhelptest.c b/tests/qemuhelptest.c
> index dfdac75..b3ab209 100644
> --- a/tests/qemuhelptest.c
> +++ b/tests/qemuhelptest.c
> @@ -227,6 +227,7 @@ mymain(int argc, char **argv)
> QEMUD_CMD_FLAG_BALLOON |
> QEMUD_CMD_FLAG_DEVICE |
> QEMUD_CMD_FLAG_SMP_TOPOLOGY |
> + QEMUD_CMD_FLAG_MONITOR_JSON |
> QEMUD_CMD_FLAG_RTC,
> 12001, 0, 0);
>
Hum again you're assuming JSON enabled for qemu 0.12 that sounds
problematic,
The patch looks fine except for this QEmu versioning for JSON support.
That need to be fixed, because I don't think it's generally okay
Daniel