[PATCH v2 1/2] libxl: add validation if sound device is supported

Xen supports only subset of libvirt's sound devices, and starting with Xen 4.17 it is enforced by libxl. Verify it early. Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com> --- src/libxl/libxl_domain.c | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c index 2d53250895..6507e34469 100644 --- a/src/libxl/libxl_domain.c +++ b/src/libxl/libxl_domain.c @@ -312,6 +312,27 @@ libxlDomainDefValidate(const virDomainDef *def, return -1; } + if (def->nsounds > 0) { + virDomainSoundDef *snd = def->sounds[0]; + switch (snd->model) { + case VIR_DOMAIN_SOUND_MODEL_ICH6: + case VIR_DOMAIN_SOUND_MODEL_ES1370: + case VIR_DOMAIN_SOUND_MODEL_AC97: + case VIR_DOMAIN_SOUND_MODEL_SB16: + break; + default: + case VIR_DOMAIN_SOUND_MODEL_PCSPK: + case VIR_DOMAIN_SOUND_MODEL_ICH7: + case VIR_DOMAIN_SOUND_MODEL_USB: + case VIR_DOMAIN_SOUND_MODEL_ICH9: + case VIR_DOMAIN_SOUND_MODEL_LAST: + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("unsupported audio model %s"), + virDomainSoundModelTypeToString(snd->model)); + return -1; + } + } + return 0; } -- 2.37.3

Xen 4.17 has strict parsing of 'soundhw' option that allows only specific values (instead of passing through any value directly to qemu's -soundhw option, it uses -device now). For 'intel-hda' audio device, it requires "hda" string. "hda" works with older libxl too. Other supported models are the same as in libvirt XML. Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com> --- Changes in v2: - move validation to libxlDomainDefValidate --- src/libxl/libxl_conf.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c index d13e48abb2..5ae60b76c1 100644 --- a/src/libxl/libxl_conf.c +++ b/src/libxl/libxl_conf.c @@ -593,7 +593,10 @@ libxlMakeDomBuildInfo(virDomainDef *def, */ virDomainSoundDef *snd = def->sounds[0]; - b_info->u.hvm.soundhw = g_strdup(virDomainSoundModelTypeToString(snd->model)); + if (snd->model == VIR_DOMAIN_SOUND_MODEL_ICH6) + b_info->u.hvm.soundhw = g_strdup("hda"); + else + b_info->u.hvm.soundhw = g_strdup(virDomainSoundModelTypeToString(snd->model)); } for (i = 0; i < def->os.nBootDevs; i++) { -- 2.37.3

On 12/20/22 23:52, Marek Marczykowski-Górecki wrote:
Xen 4.17 has strict parsing of 'soundhw' option that allows only specific values (instead of passing through any value directly to qemu's -soundhw option, it uses -device now). For 'intel-hda' audio device, it requires "hda" string. "hda" works with older libxl too. Other supported models are the same as in libvirt XML.
Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com> --- Changes in v2: - move validation to libxlDomainDefValidate --- src/libxl/libxl_conf.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c index d13e48abb2..5ae60b76c1 100644 --- a/src/libxl/libxl_conf.c +++ b/src/libxl/libxl_conf.c @@ -593,7 +593,10 @@ libxlMakeDomBuildInfo(virDomainDef *def, */ virDomainSoundDef *snd = def->sounds[0];
- b_info->u.hvm.soundhw = g_strdup(virDomainSoundModelTypeToString(snd->model)); + if (snd->model == VIR_DOMAIN_SOUND_MODEL_ICH6) + b_info->u.hvm.soundhw = g_strdup("hda"); + else + b_info->u.hvm.soundhw = g_strdup(virDomainSoundModelTypeToString(snd->model)); }
for (i = 0; i < def->os.nBootDevs; i++) {
I find this more readable as: const char *model = virDomainSoundModelTypeToString(snd->model); if (snd->model == VIR_DOMAIN_SOUND_MODEL_ICH6) model = "hda"; b_info->u.hvm.soundhw = g_strdup(model); I'll change it just before pushing. Reviewed-by: Michal Privoznik <mprivozn@redhat.com> Michal

On 12/20/22 23:52, Marek Marczykowski-Górecki wrote:
Xen supports only subset of libvirt's sound devices, and starting with Xen 4.17 it is enforced by libxl. Verify it early.
Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com> --- src/libxl/libxl_domain.c | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+)
diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c index 2d53250895..6507e34469 100644 --- a/src/libxl/libxl_domain.c +++ b/src/libxl/libxl_domain.c @@ -312,6 +312,27 @@ libxlDomainDefValidate(const virDomainDef *def, return -1; }
+ if (def->nsounds > 0) { + virDomainSoundDef *snd = def->sounds[0]; + switch (snd->model) {
Nit pick, the variable declaration block and code should be separated by an empty line. It's more readable that way.
+ case VIR_DOMAIN_SOUND_MODEL_ICH6: + case VIR_DOMAIN_SOUND_MODEL_ES1370: + case VIR_DOMAIN_SOUND_MODEL_AC97: + case VIR_DOMAIN_SOUND_MODEL_SB16: + break; + default: + case VIR_DOMAIN_SOUND_MODEL_PCSPK: + case VIR_DOMAIN_SOUND_MODEL_ICH7: + case VIR_DOMAIN_SOUND_MODEL_USB: + case VIR_DOMAIN_SOUND_MODEL_ICH9: + case VIR_DOMAIN_SOUND_MODEL_LAST: + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("unsupported audio model %s"), + virDomainSoundModelTypeToString(snd->model)); + return -1; + } + } + return 0; }
Reviewed-by: Michal Privoznik <mprivozn@redhat.com> Michal
participants (2)
-
Marek Marczykowski-Górecki
-
Michal Prívozník