After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 666599 - No sound in VM started by boxes
No sound in VM started by boxes
Status: RESOLVED FIXED
Product: gnome-boxes
Classification: Applications
Component: general
unspecified
Other Linux
: Normal normal
: --
Assigned To: GNOME Boxes maintainer(s)
GNOME Boxes maintainer(s)
Depends on:
Blocks:
 
 
Reported: 2011-12-20 16:41 UTC by Christophe Fergeau
Modified: 2016-03-31 13:56 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Add a sound card device to VMs (1.23 KB, patch)
2011-12-22 20:45 UTC, Zeeshan Ali
reviewed Details | Review
Add a sound card device to VMs (1.23 KB, patch)
2011-12-23 14:32 UTC, Zeeshan Ali
committed Details | Review

Description Christophe Fergeau 2011-12-20 16:41:49 UTC
When starting a VM with boxes, it doesn't have a soundcard. There is no "sound" device in the XML passed to libvirt ( http://libvirt.org/formatdomain.html#elementsSound ). Support is missing from libvirt-gconfig too.
Comment 1 Christophe Fergeau 2011-12-21 12:06:01 UTC
libvirt-gconfig patch at https://www.redhat.com/archives/libvir-list/2011-December/msg00928.html
Comment 2 Zeeshan Ali 2011-12-22 20:45:42 UTC
Created attachment 204113 [details] [review]
Add a sound card device to VMs
Comment 3 Marc-Andre Lureau 2011-12-22 23:07:01 UTC
Review of attachment 204113 [details] [review]:

::: src/vm-configurator.vala
@@ +53,3 @@
 
+        var sound = new DomainSound ();
+        sound.set_model (DomainSoundModel.ICH6);

AC97 is probably a better fallback. I think it won't work with Win7, that's it.

Furthermore, my experience is that the ac97 driver in qemu is much more robust and given that it is also older and used widely, has less bugs.

otherwise, fine patch
Comment 4 Zeeshan Ali 2011-12-22 23:14:15 UTC
(In reply to comment #3)
> Review of attachment 204113 [details] [review]:
> 
> ::: src/vm-configurator.vala
> @@ +53,3 @@
> 
> +        var sound = new DomainSound ();
> +        sound.set_model (DomainSoundModel.ICH6);
> 
> AC97 is probably a better fallback. I think it won't work with Win7, that's it.
> 
> Furthermore, my experience is that the ac97 driver in qemu is much more robust
> and given that it is also older and used widely, has less bugs.
> 
> otherwise, fine patch

Well the new patch I submitted on bug#666739 pretty much obsoletes this patch.
Comment 5 Zeeshan Ali 2011-12-22 23:22:42 UTC
(In reply to comment #4)
> (In reply to comment #3)
> > Review of attachment 204113 [details] [review] [details]:
> > 
> > ::: src/vm-configurator.vala
> > @@ +53,3 @@
> > 
> > +        var sound = new DomainSound ();
> > +        sound.set_model (DomainSoundModel.ICH6);
> > 
> > AC97 is probably a better fallback. I think it won't work with Win7, that's it.
> > 
> > Furthermore, my experience is that the ac97 driver in qemu is much more robust
> > and given that it is also older and used widely, has less bugs.
> > 
> > otherwise, fine patch
> 
> Well the new patch I submitted on bug#666739 pretty much obsoletes this patch.

But that one can't be applied until Daniel is back it seems so I still submitted this here so we can have some sound support added before we merge that.

Daniel recommended we use ICH6 for all modern OSs so thats why I chose that. I think its better to choose something that could be fixed when broken over something that is known to not work for certain OSs and we cant do anything about it.
Comment 6 Marc-Andre Lureau 2011-12-23 00:31:08 UTC
(In reply to comment #5)
> 
> Daniel recommended we use ICH6 for all modern OSs so thats why I chose that. I
> think its better to choose something that could be fixed when broken over
> something that is known to not work for certain OSs and we cant do anything
> about it.

I would disagree with Daniel on this one then. ICH6 is known to be needed only for Win7, and doesn't give you any advantage over AC97, quite the contrary (I think memory model is harder on CPU for emulation etc..). So no, AC97 is a better choice by default, even if ICH6 is supported by recents guests.

The qemu driver is fairly recent and not thoroughly tested.

Have you ever tried to compare AC97 on XP and ICH6 with Win7? Trust me.
Comment 7 Zeeshan Ali 2011-12-23 03:33:45 UTC
(In reply to comment #6)
> (In reply to comment #5)
> > 
> > Daniel recommended we use ICH6 for all modern OSs so thats why I chose that. I
> > think its better to choose something that could be fixed when broken over
> > something that is known to not work for certain OSs and we cant do anything
> > about it.
> 
> I would disagree with Daniel on this one then. ICH6 is known to be needed only
> for Win7, and doesn't give you any advantage over AC97, quite the contrary (I
> think memory model is harder on CPU for emulation etc..). So no, AC97 is a
> better choice by default, even if ICH6 is supported by recents guests.
> 
> The qemu driver is fairly recent and not thoroughly tested.
> 
> Have you ever tried to compare AC97 on XP and ICH6 with Win7? Trust me.

I have not and more importantly I had forgotten about your background in audio. :) I'll change the patch..
Comment 8 Zeeshan Ali 2011-12-23 14:32:09 UTC
Created attachment 204139 [details] [review]
Add a sound card device to VMs
Comment 9 Marc-Andre Lureau 2011-12-24 00:01:14 UTC
Review of attachment 204139 [details] [review]:

::: src/vm-configurator.vala
@@ +53,3 @@
 
+        var sound = new DomainSound ();
+        sound.set_model (DomainSoundModel.AC97);

do you have a way to check if it's a win7 guest here?
Comment 10 Marc-Andre Lureau 2011-12-24 00:02:12 UTC
this patch is dropped anyway right after, so why do you bother?
Comment 11 Zeeshan Ali 2011-12-24 02:53:13 UTC
(In reply to comment #9)
> Review of attachment 204139 [details] [review]:
> 
> ::: src/vm-configurator.vala
> @@ +53,3 @@
> 
> +        var sound = new DomainSound ();
> +        sound.set_model (DomainSoundModel.AC97);
> 
> do you have a way to check if it's a win7 guest here?

Yeah but it would be a bit of a hack and since this patch will be obsolete in 2-3 weeks time, I think making audio work for everything other than win7 till then is a good idea.

(In reply to comment #10)
> this patch is dropped anyway right after, so why do you bother?

Those patches will have to wait *at least* till Daniel returns and therefore its better to ignore that for the moment.
Comment 12 Zeeshan Ali 2011-12-28 16:34:20 UTC
Attachment 204139 [details] pushed as d9757b6 - Add a sound card device to VMs