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

Re: [libvirt] [PATCH 1/1] Add disk error policy to domain XML



On Fri, Mar 26, 2010 at 12:02:36PM -0400, David Allan wrote:
> * Fixes per feedback from Dan and Daniel
> * Added test datafiles
> * Re-disabled JSON flags
> * Added code to print the error policy attribute when generating XML
> ---
>  docs/schemas/domain.rng                            |   12 +++++++-
>  src/conf/domain_conf.c                             |   18 +++++++++++
>  src/conf/domain_conf.h                             |   10 ++++++
>  src/libvirt_private.syms                           |    2 +-
>  src/qemu/qemu_conf.c                               |   17 +++++++++-
>  tests/qemuargv2xmltest.c                           |    3 ++
>  .../qemuxml2argv-disk-drive-error-policy-stop.args |    1 +
>  .../qemuxml2argv-disk-drive-error-policy-stop.xml  |   32 ++++++++++++++++++++
>  tests/qemuxml2argvtest.c                           |    3 ++
>  9 files changed, 94 insertions(+), 4 deletions(-)
>  create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-drive-error-policy-stop.args
>  create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-drive-error-policy-stop.xml
> 
> 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>

  hum,

>      </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>

  Since driverErrorPolicy can only define attributes, I think it's
better to keep the <empty/> in the previous block (but after the new
optional)

>    <define name="controller">
>      <element name="controller">
>        <choice>
[...]
> diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
> index 902eecb..e2a7070 100644
> --- a/src/qemu/qemu_conf.c
> +++ b/src/qemu/qemu_conf.c
> @@ -1213,13 +1213,12 @@ static unsigned long long qemudComputeCmdFlags(const char *help,
>      if (version >= 10000)
>          flags |= QEMUD_CMD_FLAG_0_10;
> 
> +#if 0
>      /* 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)
>          flags |= QEMUD_CMD_FLAG_MONITOR_JSON;
>  #endif
> -
>      return flags;
>  }
> 

  hum, that chunk is better left out as it's equivalent and will clash
with other patches modifying this,

  Just 2 minor issues whic can be fixed before pushing,

  ACK

Daniel

-- 
Daniel Veillard      | libxml Gnome XML XSLT toolkit  http://xmlsoft.org/
daniel veillard com  | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library  http://libvirt.org/


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