On Thu, Aug 29, 2013 at 1:48 PM, Raffa Santi <[email protected]> wrote:

Subject: use the active form, rather than the passive one "Add..."

> This allows XEN hypervisors to enable sound support, if the host
> machines has required support.

This allows users to enable sound support in the Xen Hypervisor, perhaps?

> My test environment doesn't, so I
> can't be honestly sure this does actually enable sound support.
>

This doesn't belong to the changelog, at most to a comment. If you
want the changelog can be:
"This has been tested only up to the point of making sure the xen
config file has the required configuration values as per <link to xen
documentation>".

> Signed-off-by: Raffa Santi <[email protected]>
> ---
>  lib/constants.py         |    2 ++
>  lib/hypervisor/hv_xen.py |    8 ++++++++
>  man/gnt-instance.rst     |    2 +-
>  3 files changed, 11 insertions(+), 1 deletion(-)
>
> diff --git a/lib/constants.py b/lib/constants.py
> index 5263f87..5db185e 100644
> --- a/lib/constants.py
> +++ b/lib/constants.py
> @@ -2068,6 +2068,7 @@ HVC_DEFAULTS = {
>      HV_VIF_SCRIPT: "",
>      HV_XEN_CMD: XEN_CMD_XM,
>      HV_XEN_CPUID: "",
> +    HV_SOUNDHW: "none",

Nope, the string "none" is not a good default. Go for the empty string, perhaps?

>      },
>    HT_XEN_HVM: {
>      HV_BOOT_ORDER: "cd",
> @@ -2094,6 +2095,7 @@ HVC_DEFAULTS = {
>      HV_VIRIDIAN: False,
>      HV_XEN_CMD: XEN_CMD_XM,
>      HV_XEN_CPUID: "",
> +    HV_SOUNDHW: "none",
>      },
>    HT_KVM: {
>      HV_KVM_PATH: KVM_PATH,
> diff --git a/lib/hypervisor/hv_xen.py b/lib/hypervisor/hv_xen.py
> index aaa68ec..d9a0a9a 100644
> --- a/lib/hypervisor/hv_xen.py
> +++ b/lib/hypervisor/hv_xen.py
> @@ -973,6 +973,7 @@ class XenPvmHypervisor(XenHypervisor):
>      constants.HV_XEN_CMD:
>        hv_base.ParamInSet(True, constants.KNOWN_XEN_COMMANDS),
>      constants.HV_XEN_CPUID: hv_base.NO_CHECK,
> +    constants.HV_SOUNDHW: hv_base.NO_CHECK,
>      }
>
>    def _GetConfig(self, instance, startup_memory, block_devices):
> @@ -1056,6 +1057,9 @@ class XenPvmHypervisor(XenHypervisor):
>      if cpuid:
>        config.write("cpuid = %s\n" % _QuoteCpuidField(cpuid))
>
> +    if hvp[constants.HV_SOUNDHW]:
> +      config.write("soundhw = '%s'\n" % hvp[constants.HV_SOUNDHW])
> +

Doesn't this trigger if the string "none" is the default? By default
we don't want to have this.

>      return config.getvalue()
>
>
> @@ -1107,6 +1111,7 @@ class XenHvmHypervisor(XenHypervisor):
>      constants.HV_XEN_CMD:
>        hv_base.ParamInSet(True, constants.KNOWN_XEN_COMMANDS),
>      constants.HV_XEN_CPUID: hv_base.NO_CHECK,
> +    constants.HV_SOUNDHW: hv_base.NO_CHECK,
>      }
>
>    def _GetConfig(self, instance, startup_memory, block_devices):
> @@ -1240,4 +1245,7 @@ class XenHvmHypervisor(XenHypervisor):
>      if cpuid:
>        config.write("cpuid = %s\n" % _QuoteCpuidField(cpuid))
>
> +    if hvp[constants.HV_SOUNDHW]:
> +      config.write("soundhw = '%s'\n" % hvp[constants.HV_SOUNDHW])
> +
>      return config.getvalue()
> diff --git a/man/gnt-instance.rst b/man/gnt-instance.rst
> index ea3bad3..2ee54f0 100644
> --- a/man/gnt-instance.rst
> +++ b/man/gnt-instance.rst
> @@ -718,7 +718,7 @@ cpu\_sockets
>      Number of emulated CPU sockets.
>
>  soundhw
> -    Valid for the KVM hypervisor.
> +    Valid for the KVM and XEN hypervisors.
>
>      Comma separated list of emulated sounds cards, or "all" to enable
>      all the available ones.

Thanks,

Guido


-- 
Guido Trotter
Ganeti Engineering
Google Germany GmbH
Dienerstr. 12, 80331, München

Registergericht und -nummer: Hamburg, HRB 86891
Sitz der Gesellschaft: Hamburg
Geschäftsführer: Graham Law, Christine Elizabeth Flores
Steuernummer: 48/725/00206
Umsatzsteueridentifikationsnummer: DE813741370

Reply via email to