Re: [pve-devel] [PATCH qemu-server v2 0/3] Fix #2041 and #413
Hi, On 1/10/19 9:51 AM, Andreas Steinel wrote: > Hi all, > > On Wed, Jan 9, 2019 at 10:34 AM Dominik Csapak wrote: > >> thanks for the patches, >> generally look ok, but a few high level things: >> >> it would be nicer to not introduce breaking changes which gets fixed >> with the next patch, better would be: >> >> 1/3 add option >> 2/3 add x when option is set >> 3/3 add y when option is set >> >> this way, there is no breaking change introduced >> > > Yeah, they are the changes I made in my feature branch. I'm unfamiliar with > git-email, > so this is maybe due to some strange side effects. I'm used to do pull > request, so the > owner can just merge the branch as a single commit, so that the patch > supplier does > not need to take take of merging it beforehand. git rebase can really help with this, you can reorder, squash/fixup or edit commit ranges relatively easily there. Also, for pull requests a correct order and splitted up by logical-belonging-together is quite nice, projects which squash a whole request into a single commit do not sound enjoyable to review, to be honest :) > Next time I tried to create a new branch and merge all my changes into that > one and > do a git-email stuff afterwards. > > >> also i am not sure if we want to link the audio device so closely with >> spice? @all is there a good use case for having and audio devices >> without spice? (vnc has no audio, rdp does not need a device, with >> gpu passthrough you most likely have an hdmi audio or usb audio device, >> so that only leaves spice?) >> > > I concur. > > then as alexandre mentioned, this is only necessary with non q35 >> machines, as we have there an intel-hda audio device anyway >> (we could do this as a follow up though) >> >> also i think the logic for the winversion is in reverse, >> as i think linux has support for intel hda? >> >> so we probably want: winver < 6 -> AC97 else intel hda ? >> or is there any special reason why non windows machines get ac97? >> > > Yes, that makes sense. I try to implement the q35 thing. > Did you had time to look into this, we'd still like to see such a feature you proposed. ___ pve-devel mailing list pve-devel@pve.proxmox.com https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH qemu-server v2 0/3] Fix #2041 and #413
hi, thanks for the patches, generally look ok, but a few high level things: it would be nicer to not introduce breaking changes which gets fixed with the next patch, better would be: 1/3 add option 2/3 add x when option is set 3/3 add y when option is set this way, there is no breaking change introduced also i am not sure if we want to link the audio device so closely with spice? @all is there a good use case for having and audio devices without spice? (vnc has no audio, rdp does not need a device, with gpu passthrough you most likely have an hdmi audio or usb audio device, so that only leaves spice?) then as alexandre mentioned, this is only necessary with non q35 machines, as we have there an intel-hda audio device anyway (we could do this as a follow up though) also i think the logic for the winversion is in reverse, as i think linux has support for intel hda? so we probably want: winver < 6 -> AC97 else intel hda ? or is there any special reason why non windows machines get ac97? On 1/8/19 11:57 PM, Andreas Steinel wrote: Add a new option 'spicedesktop' to enable audio and folder sharing. More precisely, add another serial port in order to get the service spice-webdavd on Linux and Windows working. Afterwards you can use remote-viewer and enable folder sharing therein to get a new virtual inside of your guest for sharing files. Based on the Windows version, there are two possible sound implementations used like it is described on the SPICE page on the wiki. Signed-off-by: Andreas Steinel Andreas Steinel (3): Fix #2041: add spice webdav / folder sharing fix #413: add SPICE audio device Adding new config option 'spicedesktop' PVE/QemuServer.pm | 22 ++ PVE/QemuServer/PCI.pm | 2 ++ 2 files changed, 24 insertions(+) ___ pve-devel mailing list pve-devel@pve.proxmox.com https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH qemu-server v2 0/3] Fix #2041 and #413
Hi, they are also ich9-intel-hda as sound controller. (maybe when q35 is used) could be great to use intel hda for linux too, and not only modern windows. - Mail original - De: "Andreas Steinel" À: "pve-devel" Envoyé: Mardi 8 Janvier 2019 23:57:58 Objet: [pve-devel] [PATCH qemu-server v2 0/3] Fix #2041 and #413 Add a new option 'spicedesktop' to enable audio and folder sharing. More precisely, add another serial port in order to get the service spice-webdavd on Linux and Windows working. Afterwards you can use remote-viewer and enable folder sharing therein to get a new virtual inside of your guest for sharing files. Based on the Windows version, there are two possible sound implementations used like it is described on the SPICE page on the wiki. Signed-off-by: Andreas Steinel Andreas Steinel (3): Fix #2041: add spice webdav / folder sharing fix #413: add SPICE audio device Adding new config option 'spicedesktop' PVE/QemuServer.pm | 22 ++ PVE/QemuServer/PCI.pm | 2 ++ 2 files changed, 24 insertions(+) -- 2.11.0 ___ pve-devel mailing list pve-devel@pve.proxmox.com https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel ___ pve-devel mailing list pve-devel@pve.proxmox.com https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH qemu-server v2 0/3] Fix #2041 and #413
Add a new option 'spicedesktop' to enable audio and folder sharing. More precisely, add another serial port in order to get the service spice-webdavd on Linux and Windows working. Afterwards you can use remote-viewer and enable folder sharing therein to get a new virtual inside of your guest for sharing files. Based on the Windows version, there are two possible sound implementations used like it is described on the SPICE page on the wiki. Signed-off-by: Andreas Steinel Andreas Steinel (3): Fix #2041: add spice webdav / folder sharing fix #413: add SPICE audio device Adding new config option 'spicedesktop' PVE/QemuServer.pm | 22 ++ PVE/QemuServer/PCI.pm | 2 ++ 2 files changed, 24 insertions(+) -- 2.11.0 ___ pve-devel mailing list pve-devel@pve.proxmox.com https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
