Re: [PATCH v2 2/4] conf: allow to map sound device to host device

2020-07-31 Thread Daniel P . Berrangé
On Tue, Jul 28, 2020 at 06:58:41PM +0400, Roman Bogorodskiy wrote:
> Introduce a new device element "" which allows
> to map guest sound device specified using the ""
> element to specific audio backend.
> 
> Example:
> 
>   
>  
>   
>   
>  
>  
>   

Thinking again about this design, I wonder if we should simplify a
little by using an integer index value instead of string ID ?

   
   
  
  
   

my original suggestion of an ID string was biased from my view of
QEMU's underlying impl that uses string IDs, but the string ID is
not really useful to other hypervisors, and even in QEMU an integer
index is sufficient at libvirt level, as we can deterministically
turn it into a ID string.

The use of an integer index is more aligned with what we do for
 for example.  I'm also considering that if we add
support for multi-seat we might just use an integer ID there to
enumerate seat numbers.

Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|



Re: [PATCH v2 2/4] conf: allow to map sound device to host device

2020-07-30 Thread Daniel P . Berrangé
On Tue, Jul 28, 2020 at 06:58:41PM +0400, Roman Bogorodskiy wrote:
> Introduce a new device element "" which allows
> to map guest sound device specified using the ""
> element to specific audio backend.
> 
> Example:
> 
>   
>  
>   
>   
>  
>  
>   
> 
> This block maps to OSS audio backend on the host using
> /dev/dsp0 device for both input (recording)
> and output (playback).
> 
> OSS is the only backend supported so far.
> 
> Signed-off-by: Roman Bogorodskiy 
> ---
>  docs/schemas/domaincommon.rng  |  36 

Also docs/formatdomain.html.in needs an update

>  src/conf/domain_capabilities.c |   4 +
>  src/conf/domain_conf.c | 156 -
>  src/conf/domain_conf.h |  24 +
>  src/conf/virconftypes.h|   3 +
>  src/libvirt_private.syms   |   2 +
>  src/qemu/qemu_command.c|   1 +
>  src/qemu/qemu_domain.c |   1 +
>  src/qemu/qemu_domain_address.c |   2 +
>  src/qemu/qemu_driver.c |   5 ++
>  src/qemu/qemu_hotplug.c|   3 +
>  src/qemu/qemu_validate.c   |   1 +
>  12 files changed, 236 insertions(+), 2 deletions(-)


> diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
> index 241149af24..fefd428ccd 100644
> --- a/src/conf/domain_conf.h
> +++ b/src/conf/domain_conf.h
> @@ -85,6 +85,7 @@ typedef enum {
>  VIR_DOMAIN_DEVICE_MEMORY,
>  VIR_DOMAIN_DEVICE_IOMMU,
>  VIR_DOMAIN_DEVICE_VSOCK,
> +VIR_DOMAIN_DEVICE_AUDIO,
>  
>  VIR_DOMAIN_DEVICE_LAST
>  } virDomainDeviceType;
> @@ -116,6 +117,7 @@ struct _virDomainDeviceDef {
>  virDomainMemoryDefPtr memory;
>  virDomainIOMMUDefPtr iommu;
>  virDomainVsockDefPtr vsock;
> +virDomainAudioDefPtr audio;
>  } data;
>  };
>  
> @@ -1415,6 +1417,23 @@ struct _virDomainSoundDef {
>  
>  size_t ncodecs;
>  virDomainSoundCodecDefPtr *codecs;
> +
> +char *audioId;
> +};
> +
> +typedef enum {
> +VIR_DOMAIN_AUDIO_TYPE_OSS,
> +
> +VIR_DOMAIN_AUDIO_TYPE_LAST
> +} virDomainAudioType;
> +
> +struct _virDomainAudioDef {
> +int type;
> +
> +char *id;
> +
> +/* OSS specific configuration */
> +char *inputDev, *outputDev;

Since we're expecting multiple backends, lets go straight for a union
here.

   union {
 struct {
 char *inputDev, *outputDev;
 } oss;
   } backend;


Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|



[PATCH v2 2/4] conf: allow to map sound device to host device

2020-07-28 Thread Roman Bogorodskiy
Introduce a new device element "" which allows
to map guest sound device specified using the ""
element to specific audio backend.

Example:

  
 
  
  
 
 
  

This block maps to OSS audio backend on the host using
/dev/dsp0 device for both input (recording)
and output (playback).

OSS is the only backend supported so far.

Signed-off-by: Roman Bogorodskiy 
---
 docs/schemas/domaincommon.rng  |  36 
 src/conf/domain_capabilities.c |   4 +
 src/conf/domain_conf.c | 156 -
 src/conf/domain_conf.h |  24 +
 src/conf/virconftypes.h|   3 +
 src/libvirt_private.syms   |   2 +
 src/qemu/qemu_command.c|   1 +
 src/qemu/qemu_domain.c |   1 +
 src/qemu/qemu_domain_address.c |   2 +
 src/qemu/qemu_driver.c |   5 ++
 src/qemu/qemu_hotplug.c|   3 +
 src/qemu/qemu_validate.c   |   1 +
 12 files changed, 236 insertions(+), 2 deletions(-)

diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index a810f569c6..b0a5e08ba0 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -4366,12 +4366,47 @@
 
   
 
+
+  
+
+  
+
+  
+
 
   
 
   
 
   
+  
+
+  
+
+  
+  
+
+  oss
+
+  
+  
+
+  
+
+  
+
+  
+
+
+  
+
+  
+
+  
+
+  
+
+  
   
 
   
@@ -5286,6 +5321,7 @@
 
 
 
+
 
 
 
diff --git a/src/conf/domain_capabilities.c b/src/conf/domain_capabilities.c
index 837b004334..165a792cdf 100644
--- a/src/conf/domain_capabilities.c
+++ b/src/conf/domain_capabilities.c
@@ -692,6 +692,10 @@ virDomainCapsDeviceDefValidate(const virDomainCaps *caps,
 ret = virDomainCapsDeviceVideoDefValidate(caps, dev->data.video);
 break;
 
+case VIR_DOMAIN_DEVICE_AUDIO:
+/* TODO: add validation */
+break;
+
 case VIR_DOMAIN_DEVICE_DISK:
 case VIR_DOMAIN_DEVICE_REDIRDEV:
 case VIR_DOMAIN_DEVICE_NET:
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 7ecd2818b9..50a5c3387d 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -323,6 +323,7 @@ VIR_ENUM_IMPL(virDomainDevice,
   "memory",
   "iommu",
   "vsock",
+  "audio",
 );
 
 VIR_ENUM_IMPL(virDomainDiskDevice,
@@ -728,6 +729,11 @@ VIR_ENUM_IMPL(virDomainSoundModel,
   "usb",
 );
 
+VIR_ENUM_IMPL(virDomainAudioType,
+  VIR_DOMAIN_AUDIO_TYPE_LAST,
+  "oss",
+);
+
 VIR_ENUM_IMPL(virDomainKeyWrapCipherName,
   VIR_DOMAIN_KEY_WRAP_CIPHER_NAME_LAST,
   "aes",
@@ -2850,6 +2856,21 @@ void virDomainSoundDefFree(virDomainSoundDefPtr def)
 virDomainSoundCodecDefFree(def->codecs[i]);
 VIR_FREE(def->codecs);
 
+VIR_FREE(def->audioId);
+
+VIR_FREE(def);
+}
+
+void virDomainAudioDefFree(virDomainAudioDefPtr def)
+{
+if (!def)
+return;
+
+VIR_FREE(def->id);
+
+VIR_FREE(def->inputDev);
+VIR_FREE(def->outputDev);
+
 VIR_FREE(def);
 }
 
@@ -3206,6 +3227,9 @@ void virDomainDeviceDefFree(virDomainDeviceDefPtr def)
 case VIR_DOMAIN_DEVICE_VSOCK:
 virDomainVsockDefFree(def->data.vsock);
 break;
+case VIR_DOMAIN_DEVICE_AUDIO:
+virDomainAudioDefFree(def->data.audio);
+break;
 case VIR_DOMAIN_DEVICE_LAST:
 case VIR_DOMAIN_DEVICE_NONE:
 break;
@@ -3466,6 +3490,10 @@ void virDomainDefFree(virDomainDefPtr def)
 virDomainSoundDefFree(def->sounds[i]);
 VIR_FREE(def->sounds);
 
+for (i = 0; i < def->naudios; i++)
+virDomainAudioDefFree(def->audios[i]);
+VIR_FREE(def->audios);
+
 for (i = 0; i < def->nvideos; i++)
 virDomainVideoDefFree(def->videos[i]);
 VIR_FREE(def->videos);
@@ -4054,6 +4082,7 @@ virDomainDeviceGetInfo(virDomainDeviceDefPtr device)
 case VIR_DOMAIN_DEVICE_LEASE:
 case VIR_DOMAIN_DEVICE_GRAPHICS:
 case VIR_DOMAIN_DEVICE_IOMMU:
+case VIR_DOMAIN_DEVICE_AUDIO:
 case VIR_DOMAIN_DEVICE_LAST:
 case VIR_DOMAIN_DEVICE_NONE:
 break;
@@ -4148,6 +4177,9 @@ virDomainDeviceSetData(virDomainDeviceDefPtr device,
 case VIR_DOMAIN_DEVICE_LEASE:
 device->data.lease = devicedata;
 break;
+case VIR_DOMAIN_DEVICE_AUDIO:
+device->data.audio = devicedata;
+break;
 case VIR_DOMAIN_DEVICE_NONE:
 case VIR_DOMAIN_DEVICE_LAST:
 break;
@@ -4414,6 +4446,7 @@ virDomainDeviceInfoIterateInternal(virDomainDefPtr def,
 case VIR_DOMAIN_DEVICE_MEMORY:
 case VIR_DOMAIN_DEVICE_IOMMU:
 case VIR_DOMAIN_DEVICE_VSOCK:
+case VIR_DOMAIN_DEVICE_AUDIO: