Re: [libvirt] [PATCH 0/1] bhyve: Make LPC slot number configurable
Hi, This series was run against 'syntax-check' test by patchew.org, which failed, please find the details below: Type: series Message-id: 20180810112233.44540-1-i...@conquex.com Subject: [libvirt] [PATCH 0/1] bhyve: Make LPC slot number configurable === TEST SCRIPT BEGIN === #!/bin/bash # Testing script will be invoked under the git checkout with # HEAD pointing to a commit that has the patches applied on top of "base" # branch time bash -c './autogen.sh && make syntax-check' === TEST SCRIPT END === Updating bcb55ab053bc79561b55d0394490f4b64e0f2d01 Switched to a new branch 'test' e0ac4cf637 bhyve: Make LPC slot number configurable === OUTPUT BEGIN === Updating submodules... Submodule 'gnulib' (https://git.savannah.gnu.org/git/gnulib.git/) registered for path '.gnulib' Submodule 'keycodemapdb' (https://gitlab.com/keycodemap/keycodemapdb.git) registered for path 'src/keycodemapdb' Cloning into '/var/tmp/patchew-tester-tmp-fryj5ja2/src/.gnulib'... Cloning into '/var/tmp/patchew-tester-tmp-fryj5ja2/src/src/keycodemapdb'... Submodule path '.gnulib': checked out '68df637b5f1b5c10370f6981d2a43a5cf74368df' Submodule path 'src/keycodemapdb': checked out '16e5b0787687d8904dad2c026107409eb9bfcb95' Running bootstrap... ./bootstrap: Bootstrapping from checked-out libvirt sources... ./bootstrap: consider installing git-merge-changelog from gnulib ./bootstrap: getting gnulib files... running: libtoolize --install --copy libtoolize: putting auxiliary files in AC_CONFIG_AUX_DIR, 'build-aux'. libtoolize: copying file 'build-aux/config.guess' libtoolize: copying file 'build-aux/config.sub' libtoolize: copying file 'build-aux/install-sh' libtoolize: copying file 'build-aux/ltmain.sh' libtoolize: putting macros in AC_CONFIG_MACRO_DIRS, 'm4'. libtoolize: copying file 'm4/libtool.m4' libtoolize: copying file 'm4/ltoptions.m4' libtoolize: copying file 'm4/ltsugar.m4' libtoolize: copying file 'm4/ltversion.m4' libtoolize: copying file 'm4/lt~obsolete.m4' ./bootstrap: .gnulib/gnulib-tool--no-changelog --aux-dir=build-aux --doc-base=doc --lib=libgnu --m4-base=m4/ --source-base=gnulib/lib/ --tests-base=gnulib/tests --local-dir=gnulib/local--lgpl=2 --with-tests --makefile-name=gnulib.mk --avoid=pt_chown --avoid=lock-tests --libtool --import ... Module list with included dependencies (indented): absolute-header accept accept-tests alloca alloca-opt alloca-opt-tests allocator areadlink areadlink-tests arpa_inet arpa_inet-tests assure autobuild base64 base64-tests binary-io binary-io-tests bind bind-tests bitrotate bitrotate-tests btowc btowc-tests builtin-expect byteswap byteswap-tests c-ctype c-ctype-tests c-strcase c-strcase-tests c-strcasestr c-strcasestr-tests calloc-posix canonicalize-lgpl canonicalize-lgpl-tests careadlinkat chown chown-tests clock-time cloexec cloexec-tests close close-tests configmake connect connect-tests count-leading-zeros count-leading-zeros-tests count-one-bits count-one-bits-tests ctype ctype-tests dirname-lgpl dosname double-slash-root dup dup-tests dup2 dup2-tests environ environ-tests errno errno-tests error execinfo exitfail extensions extern-inline fatal-signal fclose fclose-tests fcntl fcntl-h fcntl-h-tests fcntl-tests fd-hook fdatasync fdatasync-tests fdopen fdopen-tests fflush fflush-tests ffs ffs-tests ffsl ffsl-tests fgetc-tests filename flexmember float float-tests fnmatch fnmatch-tests fpieee fpucw fpurge fpurge-tests fputc-tests fread-tests freading freading-tests fseek fseek-tests fseeko fseeko-tests fstat fstat-tests fsync fsync-tests ftell ftell-tests ftello ftello-tests ftruncate ftruncate-tests func func-tests fwrite-tests getaddrinfo getaddrinfo-tests getcwd-lgpl getcwd-lgpl-tests getdelim getdelim-tests getdtablesize getdtablesize-tests getgroups getgroups-tests gethostname gethostname-tests getline getline-tests getopt-posix getopt-posix-tests getpagesize getpass getpeername getpeername-tests getprogname getprogname-tests getsockname getsockname-tests getsockopt getsockopt-tests gettext-h gettimeofday gettimeofday-tests getugroups gitlog-to-changelog gnumakefile grantpt grantpt-tests hard-locale havelib host-cpu-c-abi hostent ignore-value ignore-value-tests include_next inet_ntop inet_ntop-tests inet_pton inet_pton-tests intprops intprops-tests inttypes inttypes-incomplete inttypes-tests ioctl ioctl-tests isatty isatty-tests isblank isblank-tests isnand
Re: [libvirt] [PATCH 0/1] bhyve: Make LPC slot number configurable
Ivan Mishonov wrote: > On 08/13/2018 11:00 AM, Daniel P. Berrangé wrote: > > > On Sun, Aug 12, 2018 at 08:22:08PM +0400, Roman Bogorodskiy wrote: > >>Ivan Mishonov wrote: > >> > >>> Yes, that makes sense. I'll try to find some time next week to redo my > >>> code and send another patch. Since my time for working on libvirt is > >>> very limited can you confirm that the LPC configuration should look like > >>> this: > >>> > >>> > >>> > >>> > >> This looks reasonable to me. However, it adds some corner cases we need to > >> handle: > >> > >> 1. I'm wondering if we should still default to 31 if this entry is not > >> specified? > >> We can generate this entry when post-processing XML, but I'm not sure > >> what's the best way to handle upgrades for the existing domains... > > It depends if the BHyve driver is at a point where you consider stable > > upgrades important or not. It could be valid for you to just change the > > default to 31 if you think its better and upgrade stability is not > > required yet. My general view on upgrades is that it's best-effort based at this point. In this specific case, I hope it won't affect many users as we can't run guests that a picky about LPC, and other guests should be fine with the changed LPC slot... Though I'll test if my existing guests will handle that fine. I guess when bhyve supports migrations (there are some WIP patches for that), we'll have less flexibility in moving devices/slots around... > I decided to check what vm-bhyve does. > https://github.com/churchers/vm-bhyve/blob/master/lib/vm-run#L367. They > seem to always place LPC at slot 31 so I guess it's safe to move it > > > >> 2. According to bhyve(8) manual page, lpc is only supported on bus 0, so > >> need to add 'isa-bridge' specific validation to check that. > > If its only supported in 1 address, then arguably you don't need to add > > this at all - just fix the historically mistaken use of 31 in the code > > and leave it out of XML. > Yes, we might not need that option at all Good, so let's stick to the simpler approach for now, and we can always make things configurable if there's a demand for that. > > Regards, > > Daniel > Roman Bogorodskiy signature.asc Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/1] bhyve: Make LPC slot number configurable
On 08/13/2018 11:00 AM, Daniel P. Berrangé wrote: On Sun, Aug 12, 2018 at 08:22:08PM +0400, Roman Bogorodskiy wrote: Ivan Mishonov wrote: Yes, that makes sense. I'll try to find some time next week to redo my code and send another patch. Since my time for working on libvirt is very limited can you confirm that the LPC configuration should look like this: This looks reasonable to me. However, it adds some corner cases we need to handle: 1. I'm wondering if we should still default to 31 if this entry is not specified? We can generate this entry when post-processing XML, but I'm not sure what's the best way to handle upgrades for the existing domains... It depends if the BHyve driver is at a point where you consider stable upgrades important or not. It could be valid for you to just change the default to 31 if you think its better and upgrade stability is not required yet. I decided to check what vm-bhyve does. https://github.com/churchers/vm-bhyve/blob/master/lib/vm-run#L367. They seem to always place LPC at slot 31 so I guess it's safe to move it 2. According to bhyve(8) manual page, lpc is only supported on bus 0, so need to add 'isa-bridge' specific validation to check that. If its only supported in 1 address, then arguably you don't need to add this at all - just fix the historically mistaken use of 31 in the code and leave it out of XML. Yes, we might not need that option at all Regards, Daniel -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/1] bhyve: Make LPC slot number configurable
On Sun, Aug 12, 2018 at 08:22:08PM +0400, Roman Bogorodskiy wrote: > Ivan Mishonov wrote: > > > Yes, that makes sense. I'll try to find some time next week to redo my > > code and send another patch. Since my time for working on libvirt is > > very limited can you confirm that the LPC configuration should look like > > this: > > > > > > > > > > This looks reasonable to me. However, it adds some corner cases we need to > handle: > > 1. I'm wondering if we should still default to 31 if this entry is not > specified? > We can generate this entry when post-processing XML, but I'm not sure > what's the best way to handle upgrades for the existing domains... It depends if the BHyve driver is at a point where you consider stable upgrades important or not. It could be valid for you to just change the default to 31 if you think its better and upgrade stability is not required yet. > 2. According to bhyve(8) manual page, lpc is only supported on bus 0, so > need to add 'isa-bridge' specific validation to check that. If its only supported in 1 address, then arguably you don't need to add this at all - just fix the historically mistaken use of 31 in the code and leave it out of XML. 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 :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/1] bhyve: Make LPC slot number configurable
Ivan Mishonov wrote: > Yes, that makes sense. I'll try to find some time next week to redo my > code and send another patch. Since my time for working on libvirt is > very limited can you confirm that the LPC configuration should look like > this: > > > > This looks reasonable to me. However, it adds some corner cases we need to handle: 1. I'm wondering if we should still default to 31 if this entry is not specified? We can generate this entry when post-processing XML, but I'm not sure what's the best way to handle upgrades for the existing domains... 2. According to bhyve(8) manual page, lpc is only supported on bus 0, so need to add 'isa-bridge' specific validation to check that. Roman Bogorodskiy signature.asc Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/1] bhyve: Make LPC slot number configurable
On Fri, Aug 10, 2018 at 06:36:46PM +0300, Ivan Mishonov wrote: > Yes, that makes sense. I'll try to find some time next week to redo my code > and send another patch. Since my time for working on libvirt is very limited > can you confirm that the LPC configuration should look like this: > > > > > > Also can you send me an example of how you imagine the setting > for unimplemented MSRs? Leading from this: https://libvirt.org/formatdomain.html#elementsFeatures I would perhaps think ... ... 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 :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/1] bhyve: Make LPC slot number configurable
Yes, that makes sense. I'll try to find some time next week to redo my code and send another patch. Since my time for working on libvirt is very limited can you confirm that the LPC configuration should look like this: Also can you send me an example of how you imagine the setting for unimplemented MSRs? Regards, Ivan On 08/10/2018 06:24 PM, Daniel P. Berrangé wrote: On Fri, Aug 10, 2018 at 06:11:57PM +0300, Ivan Mishonov wrote: I'd like to hear Roman's opinion on this too since he wrote the initial implementation. As for the command line arguments I was looking at since it's doing exactly the same thing and I thought it would be nice to be consistent with it It would still be reasonable to allow for arbitrary passthrough of new features which have no XML defined for them. I just think it is reasonable to model these two example explicitly. The namespaced passthrough is intended for short term hacks primarily. On 08/10/2018 05:57 PM, Daniel P. Berrangé wrote: On Fri, Aug 10, 2018 at 05:47:40PM +0300, Ivan Mishonov wrote: Yes, this is totally doable. I just don't know if it's a good idea to add a new device type specifically for bhyve LPC and nothing else. Even if we do it like this I'll still have to send another patch including the bhyve XML namespace as we need to be able to pass extra command line options to the bhyve process related to unimplemented MSRs on AMD Zen systems. I thought I'd do the 2 things in a similar manner as both of them are strictly bhyve specific IMHO the LPC thing is definitely in scope for correct modelling in the XML. For the MSRs option, it is probable we'd consider that in scope as well. Currently KVM has a global "ignore unknown msrs" option in the kernel module, but I think it is conceptually reasonable to expect that to be settable on a per-VM basis. Probably would do the MSRs thing as a flag, as we stuff lots of random feature toggles under there Regards, Daniel Regards, Daniel -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/1] bhyve: Make LPC slot number configurable
On Fri, Aug 10, 2018 at 06:11:57PM +0300, Ivan Mishonov wrote: > I'd like to hear Roman's opinion on this too since he wrote the initial > implementation. As for the command line arguments I was looking at > since it's doing exactly the same thing and I thought it > would be nice to be consistent with it It would still be reasonable to allow for arbitrary passthrough of new features which have no XML defined for them. I just think it is reasonable to model these two example explicitly. The namespaced passthrough is intended for short term hacks primarily. > On 08/10/2018 05:57 PM, Daniel P. Berrangé wrote: > > On Fri, Aug 10, 2018 at 05:47:40PM +0300, Ivan Mishonov wrote: > > > Yes, this is totally doable. I just don't know if it's a good idea to add > > > a > > > new device type specifically for bhyve LPC and nothing else. Even if we do > > > it like this I'll still have to send another patch including the bhyve XML > > > namespace as we need to be able to pass extra command line options to the > > > bhyve process related to unimplemented MSRs on AMD Zen systems. I thought > > > I'd do the 2 things in a similar manner as both of them are strictly bhyve > > > specific > > IMHO the LPC thing is definitely in scope for correct modelling in > > the XML. > > > > For the MSRs option, it is probable we'd consider that in scope as > > well. Currently KVM has a global "ignore unknown msrs" option in the > > kernel module, but I think it is conceptually reasonable to expect > > that to be settable on a per-VM basis. > > > > Probably would do the MSRs thing as a flag, as we stuff > > lots of random feature toggles under there > > > > Regards, > > Daniel > 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 :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/1] bhyve: Make LPC slot number configurable
I'd like to hear Roman's opinion on this too since he wrote the initial implementation. As for the command line arguments I was looking at since it's doing exactly the same thing and I thought it would be nice to be consistent with it Regards, Ivan On 08/10/2018 05:57 PM, Daniel P. Berrangé wrote: On Fri, Aug 10, 2018 at 05:47:40PM +0300, Ivan Mishonov wrote: Yes, this is totally doable. I just don't know if it's a good idea to add a new device type specifically for bhyve LPC and nothing else. Even if we do it like this I'll still have to send another patch including the bhyve XML namespace as we need to be able to pass extra command line options to the bhyve process related to unimplemented MSRs on AMD Zen systems. I thought I'd do the 2 things in a similar manner as both of them are strictly bhyve specific IMHO the LPC thing is definitely in scope for correct modelling in the XML. For the MSRs option, it is probable we'd consider that in scope as well. Currently KVM has a global "ignore unknown msrs" option in the kernel module, but I think it is conceptually reasonable to expect that to be settable on a per-VM basis. Probably would do the MSRs thing as a flag, as we stuff lots of random feature toggles under there Regards, Daniel -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/1] bhyve: Make LPC slot number configurable
On Fri, Aug 10, 2018 at 05:47:40PM +0300, Ivan Mishonov wrote: > Yes, this is totally doable. I just don't know if it's a good idea to add a > new device type specifically for bhyve LPC and nothing else. Even if we do > it like this I'll still have to send another patch including the bhyve XML > namespace as we need to be able to pass extra command line options to the > bhyve process related to unimplemented MSRs on AMD Zen systems. I thought > I'd do the 2 things in a similar manner as both of them are strictly bhyve > specific IMHO the LPC thing is definitely in scope for correct modelling in the XML. For the MSRs option, it is probable we'd consider that in scope as well. Currently KVM has a global "ignore unknown msrs" option in the kernel module, but I think it is conceptually reasonable to expect that to be settable on a per-VM basis. Probably would do the MSRs thing as a flag, as we stuff lots of random feature toggles under there 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 :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/1] bhyve: Make LPC slot number configurable
Yes, this is totally doable. I just don't know if it's a good idea to add a new device type specifically for bhyve LPC and nothing else. Even if we do it like this I'll still have to send another patch including the bhyve XML namespace as we need to be able to pass extra command line options to the bhyve process related to unimplemented MSRs on AMD Zen systems. I thought I'd do the 2 things in a similar manner as both of them are strictly bhyve specific Regards, Ivan On 08/10/2018 04:58 PM, Daniel P. Berrangé wrote: On Fri, Aug 10, 2018 at 03:41:09PM +0300, Ivan Mishonov wrote: This is how it's described in bhyve(8). Basically you need to configure one for each bhyve VM. Otherwise the VM won't boot. I'm not sure if there's an equivalent device in other Hypervisors. I've used KVM for quite some time and didn't need to configure anything similar lpc LPC PCI-ISA bridge with COM1 and COM2 16550 serial ports and a boot ROM. The LPC bridge emulation can only be configured on bus 0. Ok, so we have the same thing in QEMU, but it is a built-in part of the machine type which we can't move between different PCI slots, so we have never had to represent it in the XML. I think conceptually we'd probably map this to a new controller type eg So you'd just need to define the new "isa-bridge" controller type for the parser and have the bhyve driver honour the , if provided. Regards, Daniel -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/1] bhyve: Make LPC slot number configurable
On Fri, Aug 10, 2018 at 03:41:09PM +0300, Ivan Mishonov wrote: > This is how it's described in bhyve(8). Basically you need to configure one > for each bhyve VM. Otherwise the VM won't boot. I'm not sure if there's an > equivalent device in other Hypervisors. I've used KVM for quite some time > and didn't need to configure anything similar > > lpc LPC PCI-ISA bridge with COM1 and COM2 > 16550 serial ports and a boot ROM. > The LPC bridge emulation can only be > configured on bus 0. Ok, so we have the same thing in QEMU, but it is a built-in part of the machine type which we can't move between different PCI slots, so we have never had to represent it in the XML. I think conceptually we'd probably map this to a new controller type eg So you'd just need to define the new "isa-bridge" controller type for the parser and have the bhyve driver honour the , if provided. 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 :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/1] bhyve: Make LPC slot number configurable
This is how it's described in bhyve(8). Basically you need to configure one for each bhyve VM. Otherwise the VM won't boot. I'm not sure if there's an equivalent device in other Hypervisors. I've used KVM for quite some time and didn't need to configure anything similar lpc LPC PCI-ISA bridge with COM1 and COM2 16550 serial ports and a boot ROM. The LPC bridge emulation can only be configured on bus 0. Regards, Ivan On 08/10/2018 03:25 PM, Daniel P. Berrangé wrote: On Fri, Aug 10, 2018 at 03:20:06PM +0300, Ivan Mishonov wrote: The problem is that LPC is not defined as a device in the XML. Is there a device type we can use for the LPC slot configuration similarly to disk, NIC etc? I couldn't find one, but I'm not that familiar with the code so maybe I'm missing something What is the LPC ? Regards, Daniel -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/1] bhyve: Make LPC slot number configurable
On Fri, Aug 10, 2018 at 03:20:06PM +0300, Ivan Mishonov wrote: > The problem is that LPC is not defined as a device in the XML. Is there a > device type we can use for the LPC slot configuration similarly to disk, NIC > etc? I couldn't find one, but I'm not that familiar with the code so maybe > I'm missing something What is the LPC ? 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 :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/1] bhyve: Make LPC slot number configurable
The problem is that LPC is not defined as a device in the XML. Is there a device type we can use for the LPC slot configuration similarly to disk, NIC etc? I couldn't find one, but I'm not that familiar with the code so maybe I'm missing something Regards, Ivan On 08/10/2018 02:34 PM, Daniel P. Berrangé wrote: On Fri, Aug 10, 2018 at 02:22:32PM +0300, Ivan Mishonov wrote: Windows UEFI guests didn't have working keyboard input through VNC when LPC is on slot 1. The FreeBSD Handbook examples use slot 31 for UEFI guests. Borrowed most of the code from the qemu:commandline implementation and made LPC slot number configurable e.g. Isn't the LPC slot number just referring to a PCI address slot ? If so, the XML should be using our PCI addressing data. Regards, Daniel -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/1] bhyve: Make LPC slot number configurable
On Fri, Aug 10, 2018 at 02:22:32PM +0300, Ivan Mishonov wrote: > Windows UEFI guests didn't have working keyboard input through VNC > when LPC is on slot 1. The FreeBSD Handbook examples use slot 31 > for UEFI guests. Borrowed most of the code from the qemu:commandline > implementation and made LPC slot number configurable e.g. > xmlns:bhyve='http://libvirt.org/schemas/domain/bhyve/1.0'> > Isn't the LPC slot number just referring to a PCI address slot ? If so, the XML should be using our PCI addressing data. 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 :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list