[PATCH] maint: fix "mixing declarations and code" errors
clang 14.0.5 complains: ../src/bhyve/bhyve_device.c:42:29: error: mixing declarations and code is incompatible with standards before C99 [-Werror,-Wdeclaration-after-statement] virDomainPCIAddressSet *addrs = opaque; ^ 1 error generated. And a few similar errors in some other places, mainly bhyve related. Apply a trivial fix to resolve that. Signed-off-by: Roman Bogorodskiy --- src/bhyve/bhyve_device.c| 6 -- tests/bhyvexml2argvmock.c | 4 ++-- tests/domaincapstest.c | 3 ++- tests/networkxml2conftest.c | 16 +--- 4 files changed, 17 insertions(+), 12 deletions(-) diff --git a/src/bhyve/bhyve_device.c b/src/bhyve/bhyve_device.c index 5654028ca5..e4d14c4102 100644 --- a/src/bhyve/bhyve_device.c +++ b/src/bhyve/bhyve_device.c @@ -36,11 +36,13 @@ bhyveCollectPCIAddress(virDomainDef *def G_GNUC_UNUSED, virDomainDeviceInfo *info, void *opaque) { +virDomainPCIAddressSet *addrs = NULL; +virPCIDeviceAddress *addr = NULL; if (info->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DRIVE) return 0; -virDomainPCIAddressSet *addrs = opaque; -virPCIDeviceAddress *addr = &info->addr.pci; +addrs = opaque; +addr = &info->addr.pci; if (addr->domain == 0 && addr->bus == 0 && addr->slot == 0) { return 0; diff --git a/tests/bhyvexml2argvmock.c b/tests/bhyvexml2argvmock.c index 9b77f97e5f..fe76564d51 100644 --- a/tests/bhyvexml2argvmock.c +++ b/tests/bhyvexml2argvmock.c @@ -25,10 +25,10 @@ init_syms(void) DIR * opendir(const char *path) { -init_syms(); - g_autofree char *path_override = NULL; +init_syms(); + if (STREQ(path, "fakefirmwaredir")) { path_override = g_strdup(FAKEFIRMWAREDIR); } else if (STREQ(path, "fakefirmwareemptydir")) { diff --git a/tests/domaincapstest.c b/tests/domaincapstest.c index b4cb1894c2..b3cf4426f3 100644 --- a/tests/domaincapstest.c +++ b/tests/domaincapstest.c @@ -397,8 +397,9 @@ mymain(void) #define DO_TEST_BHYVE(Name, Emulator, BhyveCaps, Type) \ do { \ g_autofree char *name = NULL; \ +struct testData data; \ name = g_strdup_printf("bhyve_%s.x86_64", Name); \ -struct testData data = { \ +data = (struct testData) { \ .name = name, \ .emulator = Emulator, \ .arch = "x86_64", \ diff --git a/tests/networkxml2conftest.c b/tests/networkxml2conftest.c index 726f073ddc..d18985e060 100644 --- a/tests/networkxml2conftest.c +++ b/tests/networkxml2conftest.c @@ -50,14 +50,16 @@ testCompareXMLToConfFiles(const char *inxml, const char *outconf, /* Any changes to this function ^^ should be reflected here too. */ #ifndef __linux__ -char * tmp; +{ +char * tmp; -if (!(tmp = virStringReplace(confactual, - "except-interface=lo0\n", - "except-interface=lo\n"))) -goto fail; -VIR_FREE(confactual); -confactual = g_steal_pointer(&tmp); +if (!(tmp = virStringReplace(confactual, + "except-interface=lo0\n", + "except-interface=lo\n"))) +goto fail; +VIR_FREE(confactual); +confactual = g_steal_pointer(&tmp); +} #endif if (virTestCompareToFile(confactual, outconf) < 0) -- 2.38.0
[PATCH] meson: fix cpuset_getaffinity() detection
The cpuset_getaffinity() function is checked in sys/cpuset.h to see if BSD CPU affinity APIs are available. This check requires including sys/param.h to work properly, otherwise the test program fails with unrelated errors like: /usr/include/sys/cpuset.h:155:1: error: unknown type name '__BEGIN_DECLS' __BEGIN_DECLS ^ /usr/include/sys/cpuset.h:156:12: error: unknown type name 'cpusetid_t'; did you mean 'cpuset_t'? int cpuset(cpusetid_t *); and so forth. Signed-off-by: Roman Bogorodskiy --- meson.build | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/meson.build b/meson.build index e4f36e8574..ad0cd44aca 100644 --- a/meson.build +++ b/meson.build @@ -709,7 +709,7 @@ if (cc.has_header_symbol('net/if_bridgevar.h', 'BRDGSFD', prefix: brd_required_h endif # Check for BSD CPU affinity availability -if cc.has_header_symbol('sys/cpuset.h', 'cpuset_getaffinity') +if cc.has_header_symbol('sys/cpuset.h', 'cpuset_getaffinity', prefix: '#include ') conf.set('WITH_BSD_CPU_AFFINITY', 1) endif -- 2.33.1
Re: [PATCH] meson: improve CPU affinity routines check
Martin Kletzander wrote: > On Mon, Nov 22, 2021 at 03:22:11PM +0400, Roman Bogorodskiy wrote: > > Martin Kletzander wrote: > > > >> On Sun, Nov 21, 2021 at 07:58:55PM +0400, Roman Bogorodskiy wrote: > >> >Recently, FreeBSD has got sched_get/setaffinity(3) implementations and > >> >the sched.h header as well [1]. To make these routines visible, > >> >users have to define _WITH_CPU_SET_T. > >> > > >> >This breaks current detection. Specifically, meson sees the > >> >sched_getaffinity() symbol and defines WITH_SCHED_GETAFFINITY. This > >> >define unlocks Linux implementation of virProcessSetAffinity() and other > >> >functions, which fails to build on FreeBSD because cpu_set_t is not > >> >visible as _WITH_CPU_SET_T is not defined. > >> > > >> >For now, change detection to the following: > >> > > >> > - Instead of checking sched_getaffinity(), check if 'cpu_set_t' is > >> > available through sched.h > >> > - Explicitly check the sched.h header instead of assuming its presence > >> > if WITH_SCHED_SETSCHEDULER is defined > >> > > >> > >> Makes sense, although even the sched_getaffinity() is guarded by the new > >> _WITH_CPU_SET_T so there should be no error with the current code, am I > > > >Nope, as I tried to explain in the commit message, the current code is > >not working properly as it's using cc.has_function() for 'sched_getaffinity' > >and this check passes because it's checking the specified function in > >the standard library, so it works regardless of _WITH_CPU_SET_T. And as > >it finds the sched_getaffinity(), the build still fails because we don't > >set _WITH_CPU_SET_T. > > > > Looking at the patches you linked to it seems like sched_getaffinity() > is hidden under #ifdef _WITH_CPU_SET_T, which made me think that > cc.has_function() should not find it. Apparently, has_function() doesn't need it to present in the header, at least as I understand from this snippet from meson-logs/meson-log.txt: Running compile: Working directory: /usr/home/novel/code/libvirt/build/meson-private/tmpzkxagnh0 Command line: ccache cc /usr/home/novel/code/libvirt/build/meson-private/tmpzkxagnh0/testfile.c -o /usr/home/novel/code/libvirt/build/meson-private/tmpzkxagnh0/output.exe -D_FILE_OFFSET_BITS=64 -O0 -Werror=implicit-function-declaration -std=gnu99 Code: #define sched_getaffinity meson_disable_define_of_sched_getaffinity #include #undef sched_getaffinity #ifdef __cplusplus extern "C" #endif char sched_getaffinity (void); #if defined __stub_sched_getaffinity || defined __stub___sched_getaffinity fail fail fail this function is not going to work #endif int main(void) { return sched_getaffinity (); } Compiler stdout: Compiler stderr: Checking for function "sched_getaffinity" : YES > Anyway the code is fine as it is, I was just wondering. > > Reviewed-by: Martin Kletzander Thanks, will push shortly. PS Studying meson-log.txt, I also noticed that detection of cpuset_getaffinity() is also not working properly, I'll address it separately. > >> right? And if I understand correctly we cannot use the FreeBSD > >> implementation as is because they do not have all the CPU_* macros we > >> need, right? > > > >Frankly speaking, I haven't yet tested this implementation. Currently, > >virprocess.c is using the original FreeBSD interface: > >cpuset_(get|set)affinity. > >Generally, it would be a good thing to use the same implementation for > >both Linux and FreeBSD, however, this Linux-compatible interface in > >FreeBSD is not yet available in any FreeBSD release, so we'll have to keep > >the old code for some time anyway, and also I need to test if it > >actually works for us and figure out how to better detect and pass this > >_WITH_CPU_SET_T > >with meson. > > > > Maybe one day =) > > >> >1: > >> >https://cgit.freebsd.org/src/commit/?id=43736b71dd051212d5c55be9fa21c45993017fbb > >> >https://cgit.freebsd.org/src/commit/?id=160b4b922b6021848b6b48afc894d16b879b7af2 > >> >https://cgit.freebsd.org/src/commit/?id=90fa9705d5cd29cf11c5dc7319299788dec2546a > >> > > >> >Signed-off-by: Roman Bogorodskiy > >> >--- > >> > meson.build | 4 +++- > >> > src/util/virprocess.c | 8 > >> > 2 files changed, 7 insertions(+), 5 deletions(-) > >> > > >>
Re: [PATCH] meson: improve CPU affinity routines check
Martin Kletzander wrote: > On Sun, Nov 21, 2021 at 07:58:55PM +0400, Roman Bogorodskiy wrote: > >Recently, FreeBSD has got sched_get/setaffinity(3) implementations and > >the sched.h header as well [1]. To make these routines visible, > >users have to define _WITH_CPU_SET_T. > > > >This breaks current detection. Specifically, meson sees the > >sched_getaffinity() symbol and defines WITH_SCHED_GETAFFINITY. This > >define unlocks Linux implementation of virProcessSetAffinity() and other > >functions, which fails to build on FreeBSD because cpu_set_t is not > >visible as _WITH_CPU_SET_T is not defined. > > > >For now, change detection to the following: > > > > - Instead of checking sched_getaffinity(), check if 'cpu_set_t' is > > available through sched.h > > - Explicitly check the sched.h header instead of assuming its presence > > if WITH_SCHED_SETSCHEDULER is defined > > > > Makes sense, although even the sched_getaffinity() is guarded by the new > _WITH_CPU_SET_T so there should be no error with the current code, am I Nope, as I tried to explain in the commit message, the current code is not working properly as it's using cc.has_function() for 'sched_getaffinity' and this check passes because it's checking the specified function in the standard library, so it works regardless of _WITH_CPU_SET_T. And as it finds the sched_getaffinity(), the build still fails because we don't set _WITH_CPU_SET_T. > right? And if I understand correctly we cannot use the FreeBSD > implementation as is because they do not have all the CPU_* macros we > need, right? Frankly speaking, I haven't yet tested this implementation. Currently, virprocess.c is using the original FreeBSD interface: cpuset_(get|set)affinity. Generally, it would be a good thing to use the same implementation for both Linux and FreeBSD, however, this Linux-compatible interface in FreeBSD is not yet available in any FreeBSD release, so we'll have to keep the old code for some time anyway, and also I need to test if it actually works for us and figure out how to better detect and pass this _WITH_CPU_SET_T with meson. > >1: > >https://cgit.freebsd.org/src/commit/?id=43736b71dd051212d5c55be9fa21c45993017fbb > >https://cgit.freebsd.org/src/commit/?id=160b4b922b6021848b6b48afc894d16b879b7af2 > >https://cgit.freebsd.org/src/commit/?id=90fa9705d5cd29cf11c5dc7319299788dec2546a > > > >Signed-off-by: Roman Bogorodskiy > >--- > > meson.build | 4 +++- > > src/util/virprocess.c | 8 > > 2 files changed, 7 insertions(+), 5 deletions(-) > > > >diff --git a/meson.build b/meson.build > >index 9022bcfdc9..e4f36e8574 100644 > >--- a/meson.build > >+++ b/meson.build > >@@ -553,7 +553,6 @@ functions = [ > > 'posix_fallocate', > > 'posix_memalign', > > 'prlimit', > >- 'sched_getaffinity', > > 'sched_setscheduler', > > 'setgroups', > > 'setns', > >@@ -602,6 +601,7 @@ headers = [ > > 'net/if.h', > > 'pty.h', > > 'pwd.h', > >+ 'sched.h', > > 'sys/auxv.h', > > 'sys/ioctl.h', > > 'sys/mount.h', > >@@ -671,6 +671,8 @@ symbols = [ > > > > # Check for BSD approach for setting MAC addr > > [ 'net/if_dl.h', 'link_addr', '#include \n#include > > ' ], > >+ > >+ [ 'sched.h', 'cpu_set_t' ], > > ] > > > > if host_machine.system() == 'linux' > >diff --git a/src/util/virprocess.c b/src/util/virprocess.c > >index 6de3f36f52..81de90200e 100644 > >--- a/src/util/virprocess.c > >+++ b/src/util/virprocess.c > >@@ -35,7 +35,7 @@ > > # include > > # include > > #endif > >-#if WITH_SCHED_SETSCHEDULER > >+#if WITH_SCHED_H > > # include > > #endif > > > >@@ -480,7 +480,7 @@ int virProcessKillPainfully(pid_t pid, bool force) > > return virProcessKillPainfullyDelay(pid, force, 0, false); > > } > > > >-#if WITH_SCHED_GETAFFINITY > >+#if WITH_DECL_CPU_SET_T > > > > int virProcessSetAffinity(pid_t pid, virBitmap *map, bool quiet) > > { > >@@ -626,7 +626,7 @@ virProcessGetAffinity(pid_t pid) > > return ret; > > } > > > >-#else /* WITH_SCHED_GETAFFINITY */ > >+#else /* WITH_DECL_CPU_SET_T */ > > > > int virProcessSetAffinity(pid_t pid G_GNUC_UNUSED, > > virBitmap *map G_GNUC_UNUSED, > >@@ -646,7 +646,7 @@ virProcessGetAffinity(pid_t pid G_GNUC_UNUSED) > > _("Process CPU affinity is not supported on this > > platform")); > > return NULL; > > } > >-#endif /* WITH_SCHED_GETAFFINITY */ > >+#endif /* WITH_DECL_CPU_SET_T */ > > > > > > int virProcessGetPids(pid_t pid, size_t *npids, pid_t **pids) > >-- > >2.33.1 > > Roman Bogorodskiy signature.asc Description: PGP signature
[PATCH] meson: improve CPU affinity routines check
Recently, FreeBSD has got sched_get/setaffinity(3) implementations and the sched.h header as well [1]. To make these routines visible, users have to define _WITH_CPU_SET_T. This breaks current detection. Specifically, meson sees the sched_getaffinity() symbol and defines WITH_SCHED_GETAFFINITY. This define unlocks Linux implementation of virProcessSetAffinity() and other functions, which fails to build on FreeBSD because cpu_set_t is not visible as _WITH_CPU_SET_T is not defined. For now, change detection to the following: - Instead of checking sched_getaffinity(), check if 'cpu_set_t' is available through sched.h - Explicitly check the sched.h header instead of assuming its presence if WITH_SCHED_SETSCHEDULER is defined 1: https://cgit.freebsd.org/src/commit/?id=43736b71dd051212d5c55be9fa21c45993017fbb https://cgit.freebsd.org/src/commit/?id=160b4b922b6021848b6b48afc894d16b879b7af2 https://cgit.freebsd.org/src/commit/?id=90fa9705d5cd29cf11c5dc7319299788dec2546a Signed-off-by: Roman Bogorodskiy --- meson.build | 4 +++- src/util/virprocess.c | 8 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/meson.build b/meson.build index 9022bcfdc9..e4f36e8574 100644 --- a/meson.build +++ b/meson.build @@ -553,7 +553,6 @@ functions = [ 'posix_fallocate', 'posix_memalign', 'prlimit', - 'sched_getaffinity', 'sched_setscheduler', 'setgroups', 'setns', @@ -602,6 +601,7 @@ headers = [ 'net/if.h', 'pty.h', 'pwd.h', + 'sched.h', 'sys/auxv.h', 'sys/ioctl.h', 'sys/mount.h', @@ -671,6 +671,8 @@ symbols = [ # Check for BSD approach for setting MAC addr [ 'net/if_dl.h', 'link_addr', '#include \n#include ' ], + + [ 'sched.h', 'cpu_set_t' ], ] if host_machine.system() == 'linux' diff --git a/src/util/virprocess.c b/src/util/virprocess.c index 6de3f36f52..81de90200e 100644 --- a/src/util/virprocess.c +++ b/src/util/virprocess.c @@ -35,7 +35,7 @@ # include # include #endif -#if WITH_SCHED_SETSCHEDULER +#if WITH_SCHED_H # include #endif @@ -480,7 +480,7 @@ int virProcessKillPainfully(pid_t pid, bool force) return virProcessKillPainfullyDelay(pid, force, 0, false); } -#if WITH_SCHED_GETAFFINITY +#if WITH_DECL_CPU_SET_T int virProcessSetAffinity(pid_t pid, virBitmap *map, bool quiet) { @@ -626,7 +626,7 @@ virProcessGetAffinity(pid_t pid) return ret; } -#else /* WITH_SCHED_GETAFFINITY */ +#else /* WITH_DECL_CPU_SET_T */ int virProcessSetAffinity(pid_t pid G_GNUC_UNUSED, virBitmap *map G_GNUC_UNUSED, @@ -646,7 +646,7 @@ virProcessGetAffinity(pid_t pid G_GNUC_UNUSED) _("Process CPU affinity is not supported on this platform")); return NULL; } -#endif /* WITH_SCHED_GETAFFINITY */ +#endif /* WITH_DECL_CPU_SET_T */ int virProcessGetPids(pid_t pid, size_t *npids, pid_t **pids) -- 2.33.1
[PATCH v2] bhyve: add support
Implement "" support for bhyve driver. As there are not really lot of options, try to find "BHYVE_UEFI.fd" firmware which is installed by the sysutils/uefi-edk2-bhyve FreeBSD port. If not found, just use the first found firmware in the firmwares directory (which is configurable via config file). Signed-off-by: Roman Bogorodskiy --- Changes from v1: - Fixed various leaks, - Re-implemented testing using opendir() mock. po/POTFILES.in| 1 + src/bhyve/bhyve_domain.c | 5 + src/bhyve/bhyve_firmware.c| 91 +++ src/bhyve/bhyve_firmware.h| 30 ++ src/bhyve/bhyve_process.c | 15 +++ src/bhyve/bhyve_process.h | 5 + src/bhyve/bhyve_utils.h | 2 + src/bhyve/meson.build | 1 + tests/bhyvefirmwaredata/empty/.keepme | 0 .../three_firmwares/BHYVE_UEFI.fd | 0 .../three_firmwares/BHYVE_UEFI_CSM.fd | 0 .../three_firmwares/refind_x64.efi| 0 .../bhyvexml2argv-firmware-efi.args | 11 +++ .../bhyvexml2argv-firmware-efi.ldargs | 1 + .../bhyvexml2argv-firmware-efi.xml| 22 + tests/bhyvexml2argvmock.c | 33 +++ tests/bhyvexml2argvtest.c | 52 --- 17 files changed, 257 insertions(+), 12 deletions(-) create mode 100644 src/bhyve/bhyve_firmware.c create mode 100644 src/bhyve/bhyve_firmware.h create mode 100644 tests/bhyvefirmwaredata/empty/.keepme create mode 100644 tests/bhyvefirmwaredata/three_firmwares/BHYVE_UEFI.fd create mode 100644 tests/bhyvefirmwaredata/three_firmwares/BHYVE_UEFI_CSM.fd create mode 100644 tests/bhyvefirmwaredata/three_firmwares/refind_x64.efi create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-firmware-efi.args create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-firmware-efi.ldargs create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-firmware-efi.xml diff --git a/po/POTFILES.in b/po/POTFILES.in index 80c5f145be..413783ee35 100644 --- a/po/POTFILES.in +++ b/po/POTFILES.in @@ -14,6 +14,7 @@ @SRCDIR@src/bhyve/bhyve_command.c @SRCDIR@src/bhyve/bhyve_domain.c @SRCDIR@src/bhyve/bhyve_driver.c +@SRCDIR@src/bhyve/bhyve_firmware.c @SRCDIR@src/bhyve/bhyve_monitor.c @SRCDIR@src/bhyve/bhyve_parse_command.c @SRCDIR@src/bhyve/bhyve_process.c diff --git a/src/bhyve/bhyve_domain.c b/src/bhyve/bhyve_domain.c index 8fbc554a0a..209e4d3905 100644 --- a/src/bhyve/bhyve_domain.c +++ b/src/bhyve/bhyve_domain.c @@ -64,6 +64,9 @@ bhyveDomainDefNeedsISAController(virDomainDefPtr def) if (def->os.bootloader == NULL && def->os.loader) return true; +if (def->os.firmware == VIR_DOMAIN_OS_DEF_FIRMWARE_EFI) +return true; + if (def->nserials || def->nconsoles) return true; @@ -230,6 +233,8 @@ virDomainDefParserConfig virBhyveDriverDomainDefParserConfig = { .domainPostParseCallback = bhyveDomainDefPostParse, .assignAddressesCallback = bhyveDomainDefAssignAddresses, .deviceValidateCallback = bhyveDomainDeviceDefValidate, + +.features = VIR_DOMAIN_DEF_FEATURE_FW_AUTOSELECT, }; static void diff --git a/src/bhyve/bhyve_firmware.c b/src/bhyve/bhyve_firmware.c new file mode 100644 index 00..2055f34288 --- /dev/null +++ b/src/bhyve/bhyve_firmware.c @@ -0,0 +1,91 @@ +/* + * bhyve_firmware.c: bhyve firmware management + * + * Copyright (C) 2021 Roman Bogorodskiy + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library. If not, see + * <http://www.gnu.org/licenses/>. + * + */ + +#include +#include + +#include "viralloc.h" +#include "virlog.h" +#include "virfile.h" +#include "bhyve_conf.h" +#include "bhyve_firmware.h" + +#define VIR_FROM_THIS VIR_FROM_BHYVE + +VIR_LOG_INIT("bhyve.bhyve_firmware"); + + +#define BHYVE_DEFAULT_FIRMWARE "BHYVE_UEFI.fd" + +int +bhyveFirmwareFillDomain(bhyveConnPtr driver, +virDomainDefPtr def, +unsigned int flags) +{ +g_autoptr(DIR) dir = NULL; +g_autoptr(virBhyveDriverConfig) cfg = virBhyveDriverGetConfig(driver); +const char *firmware_dir = cfg->firmwareDir; +struct dirent *entry; +g_autofree char *ma
Re: [PATCH] meson: tools: depend on keycode generated sources
Andrea Bolognani wrote: > On Fri, 2021-03-05 at 13:43 +0100, Ján Tomko wrote: > > On a Friday in 2021, Andrea Bolognani wrote: > > > On Thu, 2021-03-04 at 17:47 +0100, Ján Tomko wrote: > > > > On a Wednesday in 2021, Roman Bogorodskiy wrote: > > > > > +keycode_dep = declare_dependency(sources: keycode_gen_sources) > > > > > > > > Please format this as: > > > > > > > > keycode_dep = declare_dependency( > > > >sources: keycode_gen_sources > > > > ) > > > > > > > > to match the prevailing style. > > > > > > Small correction: it should be > > > > > > keycode_dep = declare_dependency( > > >sources: keycode_gen_sources, > > > ) > > > > > > Note the additional comma, which allows us to have cleaner diffs when > > > making further changes, and the indentation being only two spaces > > > instead of three. > > > > The three spaces come from your MUA misquoting me. I see two spaces in > > my version of the e-mail, as well as the list archive: > > https://listman.redhat.com/archives/libvir-list/2021-March/msg00252.html > > > > (Not that my MUA is any better in that regard - the indentation in my > > quoting of Roman's patch is wrong too) > > That's interesting: if I look at the HTML version you linked above or > copy and paste the snippet from it, the indentation is indeed two > spaces; however, if I look at the copy in my local mailbox or at the > full mbox taken from > > https://listman.redhat.com/archives/libvir-list/2021-March.txt.gz > > there are three spaces. > > Looking at the headers for your message, I see > > Content-Type: multipart/signed; micalg=pgp-sha256; > protocol="application/pgp-signature"; boundary="kmvAAwZj779MjF+K" > > followed by > > Content-Type: text/plain; charset=iso-8859-1; format=flowed > Content-Disposition: inline > Content-Transfer-Encoding: quoted-printable > > and the body contains stuff like > > keycode_dep =3D declare_dependency( > > Reviewed-by: J=E1n Tomko > > so I think perhaps your MUA's configuration might be to blame for the > weirdness we're seeing? Honestly, I just don't understand email well > enough to be able to tell :) > > -- > Andrea Bolognani / Red Hat / Virtualization > That's interesting indeed, because my MUA shows 2 space indentation in all code snippets from this thread. FWIW, the patch was merged with the formatting fixes applied, thanks. Roman Bogorodskiy signature.asc Description: PGP signature
[PATCH] meson: tools: depend on keycode generated sources
Tools depend on keycode generated sources, so declare that as an explicit dependency, otherwise it might fail with: ../tools/virsh-completer-domain.c:35:10: fatal error: 'virkeynametable_linux.h' file not found ^ Signed-off-by: Roman Bogorodskiy --- I noticed that build failure on FreeBSD 11.x. For some reason, it doesn't show up on newer versions. This is strange, but as the fix appears to be straight-forward, I didn't spend much time figuring out the reason. In case you're interested, a full failing build log is here: https://people.freebsd.org/~novel/misc/libvirt-7.1.0.log It doesn't seem to even try to generate these files. When I inspected filesystem state, these files were missing. src/util/meson.build | 2 ++ tools/meson.build| 1 + 2 files changed, 3 insertions(+) diff --git a/src/util/meson.build b/src/util/meson.build index 0080825bd0..cd3fe18524 100644 --- a/src/util/meson.build +++ b/src/util/meson.build @@ -168,6 +168,8 @@ foreach name : keyname_list ) endforeach +keycode_dep = declare_dependency(sources: keycode_gen_sources) + io_helper_sources = [ 'iohelper.c', ] diff --git a/tools/meson.build b/tools/meson.build index b8c6802f0a..42dc609439 100644 --- a/tools/meson.build +++ b/tools/meson.build @@ -186,6 +186,7 @@ executable( tools_dep, readline_dep, thread_dep, +keycode_dep, ], link_args: [ coverage_flags, -- 2.30.0
[PATCH] build-aux: require GNU grep on FreeBSD
FreeBSD 13.x and newer ship BSD grep which apparently has some performance issues causing certain syntax check tests to run longer than the default 30 seconds timeout used by meson. However, GNU grep is still available through the textproc/gnugrep port, so require it on FreeBSD if /usr/bin/grep is a BSD grep to make checks pass in a reasonable time. Signed-off-by: Roman Bogorodskiy --- build-aux/Makefile.in | 1 + build-aux/meson.build | 24 +++- build-aux/syntax-check.mk | 1 - 3 files changed, 20 insertions(+), 6 deletions(-) diff --git a/build-aux/Makefile.in b/build-aux/Makefile.in index 0424ff39fc..415a8df305 100644 --- a/build-aux/Makefile.in +++ b/build-aux/Makefile.in @@ -5,6 +5,7 @@ top_builddir = @top_builddir@ FLAKE8 = @flake8_path@ RUNUTF8 = @runutf8@ PYTHON = @PYTHON3@ +GREP = @GREP@ # include syntax-check.mk file include $(top_srcdir)/build-aux/syntax-check.mk diff --git a/build-aux/meson.build b/build-aux/meson.build index c506feefd2..c44ed6821c 100644 --- a/build-aux/meson.build +++ b/build-aux/meson.build @@ -10,18 +10,32 @@ syntax_check_conf.set('flake8_path', flake8_path) syntax_check_conf.set('runutf8', ' '.join(runutf8)) syntax_check_conf.set('PYTHON3', python3_prog.path()) -configure_file( - input: 'Makefile.in', - output: '@BASENAME@', - configuration: syntax_check_conf, -) + +grep_prog = find_program('grep') if host_machine.system() == 'freebsd' make_prog = find_program('gmake') + + grep_cmd = run_command(grep_prog, '--version') + if grep_cmd.stdout().startswith('grep (BSD grep') +grep_prog = find_program('/usr/local/bin/grep') +grep_cmd = run_command(grep_prog, '--version') +if grep_cmd.stdout().startswith('grep (BSD grep') + error('GNU grep not found') +endif + endif else make_prog = find_program('make') endif +syntax_check_conf.set('GREP', grep_prog.path()) + +configure_file( + input: 'Makefile.in', + output: '@BASENAME@', + configuration: syntax_check_conf, +) + rc = run_command( 'sed', '-n', 's/^\\(sc_[a-zA-Z0-9_-]*\\):.*/\\1/p', diff --git a/build-aux/syntax-check.mk b/build-aux/syntax-check.mk index e51877648a..6f6603fa6f 100644 --- a/build-aux/syntax-check.mk +++ b/build-aux/syntax-check.mk @@ -27,7 +27,6 @@ ME := build-aux/syntax-check.mk # of the module description. But some packages import this file directly, # ignoring the module description. AWK ?= awk -GREP ?= grep # FreeBSD (and probably some other OSes too) ships own version of sed(1), not # compatible with the GNU sed. GNU sed is available as gsed(1), so use this # instead -- 2.30.0
Re: [PATCH] build-aux: increase tests timeout
Daniel P. Berrangé wrote: > On Sun, Feb 28, 2021 at 05:00:01PM +0400, Roman Bogorodskiy wrote: > > Peter Krempa wrote: > > > > > On Sun, Feb 28, 2021 at 08:24:58 +0400, Roman Bogorodskiy wrote: > > > > Meson default timeout for test() is 30 seconds. This may be not enough > > > > for some tests like sc_prohibit_nonreentrant or > > > > sc_libvirt_unmarked_diagnostics, so set it to 60 seconds. > > > > > > Recently [1] we've established that we'll not be raising the timeout > > > arbitrarily to compensate for a possibly slow hardware unless it's a > > > widespread problem. > > > > > > The tests you are complaining about are pretty fast on my system: > > > > > > 203/334 libvirt:syntax-check / sc_prohibit_nonreentrant > > >OK 0.23s > > > 315/334 libvirt:syntax-check / sc_unmarked_diagnostics > > >OK 0.63s > > > > > > On a laptop: > > > 204/335 libvirt:syntax-check / sc_prohibit_nonreentrant > > >OK 0.44s > > > 316/335 libvirt:syntax-check / sc_unmarked_diagnostics > > >OK 0.78s > > > > > > And on a random sample from our (linux) CI runs: > > > 53/158 libvirt:syntax-check / sc_libvirt_unmarked_diagnostics OK > > > 0.6185753345489502 s > > > 27/158 libvirt:syntax-check / sc_prohibit_nonreentrant OK > > > 0.2680661678314209 s > > > > > > Given the almost 2 orders of magnitude difference, I think something is > > > broken on your system and should be investigated first before attempting > > > to increase the timeout. > > > > I *think* the reason it's slow on my system is because BSD grep is > > slower than GNU grep. > > > > I don't have a solid evidence of that though, except that 10 years old > > post [1] and a basic test: > > > > $ time gmake -C /usr/home/novel/code/libvirt/build/build-aux > > sc_prohibit_nonreentrant > > gmake: Entering directory '/usr/home/novel/code/libvirt/build/build-aux' > > prohibit_nonreentrant > > gmake: Leaving directory '/usr/home/novel/code/libvirt/build/build-aux' > > gmake -C /usr/home/novel/code/libvirt/build/build-aux > > sc_prohibit_nonreentran 48,21s user 0,06s system 100% cpu 48,199 total > > $ time PATH="/usr/local/bin:$PATH" gmake -C > > /usr/home/novel/code/libvirt/build/build-aux sc_prohibit_nonreentrant > > gmake: Entering directory '/usr/home/novel/code/libvirt/build/build-aux' > > prohibit_nonreentrant > > gmake: Leaving directory '/usr/home/novel/code/libvirt/build/build-aux' > > PATH="/usr/local/bin:$PATH" gmake -C sc_prohibit_nonreentrant 0,23s user > > 0,02s system 119% cpu 0,215 total > > $ > > > > Here, the PATH override is used because on FreeBSD the original BSD grep > > is installed in /usr/bin/grep, and GNU grep is installed in > > /usr/local/bin/grep from the gnugrep package (or textproc/gnugrep port). > > We already special case sed for BSD in syntax-check.mk, so how about we > make it also try the GNU grep explicitly too. Yes, I tried that and it works. What I don't quite like about this approach is that unlike sed, where GNU sed is installed as 'gsed' which is easy to detect, GNU grep is installed as 'grep', but to a different prefix. We can hardcode GREP ?= /usr/local/bin/grep, but there are some cases when it won't work: - If a user installs software to a different prefix (not /usr/local), - If a user has some other grep installed in /usr/local instead of GNU grep. For example, there's a textproc/bsdgrep port which installs grep to the same place. Moreover, it looks like FreeBSD 12.x and older use GNU grep by default, so with this hardcode we'll be using a wrong grep on older FreeBSD versions. A general solution would be to make sure we're dealing with GNU grep by checking `grep --version` and looking for some specific bits to GNU grep, maybe "Free Software Foundation" or something like that, as BSD grep mentions it's GNU grep compatible, so cannot search for just "GNU grep". This doesn't seem to be very convenient with meson... More simple, however less general solution would be use /usr/local/bin/grep only for FreeBSD >= 13.0. PS In addition to that, for quite some time it was possible to build FreeBSD with GNU grep even after BSD grep became a default. This switch was removed only in Dec 2020 and BSD grep became the only option. I guess it's safe to ignore this case, but that's overall is just feels too messy. > > 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 :| > Roman Bogorodskiy signature.asc Description: PGP signature
Re: [PATCH] build-aux: increase tests timeout
Peter Krempa wrote: > On Sun, Feb 28, 2021 at 08:24:58 +0400, Roman Bogorodskiy wrote: > > Meson default timeout for test() is 30 seconds. This may be not enough > > for some tests like sc_prohibit_nonreentrant or > > sc_libvirt_unmarked_diagnostics, so set it to 60 seconds. > > Recently [1] we've established that we'll not be raising the timeout > arbitrarily to compensate for a possibly slow hardware unless it's a > widespread problem. > > The tests you are complaining about are pretty fast on my system: > > 203/334 libvirt:syntax-check / sc_prohibit_nonreentrant >OK 0.23s > 315/334 libvirt:syntax-check / sc_unmarked_diagnostics >OK 0.63s > > On a laptop: > 204/335 libvirt:syntax-check / sc_prohibit_nonreentrant >OK 0.44s > 316/335 libvirt:syntax-check / sc_unmarked_diagnostics >OK 0.78s > > And on a random sample from our (linux) CI runs: > 53/158 libvirt:syntax-check / sc_libvirt_unmarked_diagnostics OK > 0.6185753345489502 s > 27/158 libvirt:syntax-check / sc_prohibit_nonreentrant OK > 0.2680661678314209 s > > Given the almost 2 orders of magnitude difference, I think something is > broken on your system and should be investigated first before attempting > to increase the timeout. I *think* the reason it's slow on my system is because BSD grep is slower than GNU grep. I don't have a solid evidence of that though, except that 10 years old post [1] and a basic test: $ time gmake -C /usr/home/novel/code/libvirt/build/build-aux sc_prohibit_nonreentrant gmake: Entering directory '/usr/home/novel/code/libvirt/build/build-aux' prohibit_nonreentrant gmake: Leaving directory '/usr/home/novel/code/libvirt/build/build-aux' gmake -C /usr/home/novel/code/libvirt/build/build-aux sc_prohibit_nonreentran 48,21s user 0,06s system 100% cpu 48,199 total $ time PATH="/usr/local/bin:$PATH" gmake -C /usr/home/novel/code/libvirt/build/build-aux sc_prohibit_nonreentrant gmake: Entering directory '/usr/home/novel/code/libvirt/build/build-aux' prohibit_nonreentrant gmake: Leaving directory '/usr/home/novel/code/libvirt/build/build-aux' PATH="/usr/local/bin:$PATH" gmake -C sc_prohibit_nonreentrant 0,23s user 0,02s system 119% cpu 0,215 total $ Here, the PATH override is used because on FreeBSD the original BSD grep is installed in /usr/bin/grep, and GNU grep is installed in /usr/local/bin/grep from the gnugrep package (or textproc/gnugrep port). 1: https://lists.freebsd.org/pipermail/freebsd-current/2010-August/019310.html > > Signed-off-by: Roman Bogorodskiy > > --- > > On my system these two tests always timeout with the default value. > > That's what I have after increasing timeout: > > > > 157/288 libvirt:syntax-check / sc_prohibit_nonreentrant > > OK 52.18s > > 183/288 libvirt:syntax-check / sc_libvirt_unmarked_diagnostics > > OK 31.48s > > [1] thread start: > https://listman.redhat.com/archives/libvir-list/2021-January/msg01148.html > https://listman.redhat.com/archives/libvir-list/2021-January/msg01219.html > Roman Bogorodskiy signature.asc Description: PGP signature
[PATCH] build-aux: increase tests timeout
Meson default timeout for test() is 30 seconds. This may be not enough for some tests like sc_prohibit_nonreentrant or sc_libvirt_unmarked_diagnostics, so set it to 60 seconds. Signed-off-by: Roman Bogorodskiy --- On my system these two tests always timeout with the default value. That's what I have after increasing timeout: 157/288 libvirt:syntax-check / sc_prohibit_nonreentrant OK 52.18s 183/288 libvirt:syntax-check / sc_libvirt_unmarked_diagnostics OK 31.48s build-aux/meson.build | 1 + 1 file changed, 1 insertion(+) diff --git a/build-aux/meson.build b/build-aux/meson.build index c506feefd2..4acd5e16c1 100644 --- a/build-aux/meson.build +++ b/build-aux/meson.build @@ -44,6 +44,7 @@ if git potfiles_dep, ], suite: 'syntax-check', + timeout: 60, ) endforeach endif -- 2.30.0
[PATCH] bhyve: add support
Implement "" support for bhyve driver. As there are not really lot of options, try to find "BHYVE_UEFI.fd" firmware which is installed by the sysutils/uefi-edk2-bhyve FreeBSD port. If not found, just use the first found firmware in the firmwares directory (which is configurable via config file). Signed-off-by: Roman Bogorodskiy --- Not extremely happy about the LIBVIRT_BHYVE_FIRMWARE_DIR_OVERRIDE knob, but not sure how to test this otherwise. po/POTFILES.in| 1 + src/bhyve/bhyve_domain.c | 5 + src/bhyve/bhyve_firmware.c| 91 +++ src/bhyve/bhyve_firmware.h| 30 ++ src/bhyve/bhyve_process.c | 15 +++ src/bhyve/bhyve_process.h | 5 + src/bhyve/meson.build | 1 + .../bhyvexml2argv-firmware-efi.args | 11 +++ .../bhyvexml2argv-firmware-efi.ldargs | 1 + .../bhyvexml2argv-firmware-efi.xml| 22 + tests/bhyvexml2argvtest.c | 83 ++--- 11 files changed, 254 insertions(+), 11 deletions(-) create mode 100644 src/bhyve/bhyve_firmware.c create mode 100644 src/bhyve/bhyve_firmware.h create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-firmware-efi.args create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-firmware-efi.ldargs create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-firmware-efi.xml diff --git a/po/POTFILES.in b/po/POTFILES.in index 80c5f145be..413783ee35 100644 --- a/po/POTFILES.in +++ b/po/POTFILES.in @@ -14,6 +14,7 @@ @SRCDIR@src/bhyve/bhyve_command.c @SRCDIR@src/bhyve/bhyve_domain.c @SRCDIR@src/bhyve/bhyve_driver.c +@SRCDIR@src/bhyve/bhyve_firmware.c @SRCDIR@src/bhyve/bhyve_monitor.c @SRCDIR@src/bhyve/bhyve_parse_command.c @SRCDIR@src/bhyve/bhyve_process.c diff --git a/src/bhyve/bhyve_domain.c b/src/bhyve/bhyve_domain.c index 8fbc554a0a..209e4d3905 100644 --- a/src/bhyve/bhyve_domain.c +++ b/src/bhyve/bhyve_domain.c @@ -64,6 +64,9 @@ bhyveDomainDefNeedsISAController(virDomainDefPtr def) if (def->os.bootloader == NULL && def->os.loader) return true; +if (def->os.firmware == VIR_DOMAIN_OS_DEF_FIRMWARE_EFI) +return true; + if (def->nserials || def->nconsoles) return true; @@ -230,6 +233,8 @@ virDomainDefParserConfig virBhyveDriverDomainDefParserConfig = { .domainPostParseCallback = bhyveDomainDefPostParse, .assignAddressesCallback = bhyveDomainDefAssignAddresses, .deviceValidateCallback = bhyveDomainDeviceDefValidate, + +.features = VIR_DOMAIN_DEF_FEATURE_FW_AUTOSELECT, }; static void diff --git a/src/bhyve/bhyve_firmware.c b/src/bhyve/bhyve_firmware.c new file mode 100644 index 00..ccc3a5ffc8 --- /dev/null +++ b/src/bhyve/bhyve_firmware.c @@ -0,0 +1,91 @@ +/* + * bhyve_firmware.c: bhyve firmware management + * + * Copyright (C) 2021 Roman Bogorodskiy + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library. If not, see + * <http://www.gnu.org/licenses/>. + * + */ + +#include +#include + +#include "viralloc.h" +#include "virlog.h" +#include "virfile.h" +#include "bhyve_conf.h" +#include "bhyve_firmware.h" + +#define VIR_FROM_THIS VIR_FROM_BHYVE + +VIR_LOG_INIT("bhyve.bhyve_firmware"); + + +#define BHYVE_DEFAULT_FIRMWARE "BHYVE_UEFI.fd" + +int +bhyveFirmwareFillDomain(bhyveConnPtr driver, +virDomainDefPtr def, +unsigned int flags) +{ +g_autoptr(DIR) dir = NULL; +virBhyveDriverConfigPtr cfg = virBhyveDriverGetConfig(driver); +const char *firmware_dir_cfg = cfg->firmwareDir; +const char *firmware_dir_env = NULL, *firmware_dir = NULL; +struct dirent *entry; +char *matching_firmware = NULL; +char *first_found = NULL; + +virCheckFlags(0, -1); + +if (def->os.firmware == VIR_DOMAIN_OS_DEF_FIRMWARE_NONE) +return 0; + +if (virDirOpenIfExists(&dir, firmware_dir_cfg) > 0) { +while ((virDirRead(dir, &entry, firmware_dir)) > 0) { +if (STREQ(entry->d_name, BHYVE_DEFAULT_FIRMWARE)) { +matching_firmware = g_strdup(entry->d_name); +break; +} +if (!first_found) +f
[PATCH] bhyve: auto allocate nmdm console paths
Currently, nmdm console device requires user to specify master and slave path attributes (such as /dev/nmdm0A and /dev/nmdm0B respectively). However, making user find a non-occupied device name might be not convenient, especially for the remote connections. Update the logic to make these attributes optional. In case if not specified, use /dev/nmdm$UUID[AB], where $UUID is a domain's UUID. With this schema it's unlikely nmdm device will clash with other domains or even other non-bhyve nmdm devices. Signed-off-by: Roman Bogorodskiy --- src/bhyve/bhyve_domain.c | 14 ++ src/conf/domain_validate.c| 15 +++ ...gv-console-master-slave-not-specified.args | 11 + ...-console-master-slave-not-specified.ldargs | 3 ++ ...rgv-console-master-slave-not-specified.xml | 24 ++ tests/bhyvexml2argvtest.c | 1 + ...out-console-master-slave-not-specified.xml | 44 +++ tests/bhyvexml2xmltest.c | 1 + 8 files changed, 104 insertions(+), 9 deletions(-) create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-console-master-slave-not-specified.args create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-console-master-slave-not-specified.ldargs create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-console-master-slave-not-specified.xml create mode 100644 tests/bhyvexml2xmloutdata/bhyvexml2xmlout-console-master-slave-not-specified.xml diff --git a/src/bhyve/bhyve_domain.c b/src/bhyve/bhyve_domain.c index 1d99ba6c96..8fbc554a0a 100644 --- a/src/bhyve/bhyve_domain.c +++ b/src/bhyve/bhyve_domain.c @@ -170,6 +170,20 @@ bhyveDomainDeviceDefPostParse(virDomainDeviceDefPtr dev, dev->data.video->type = VIR_DOMAIN_VIDEO_TYPE_GOP; } +if (dev->type == VIR_DOMAIN_DEVICE_CHR && +dev->data.chr->source->type == VIR_DOMAIN_CHR_TYPE_NMDM) { +virDomainChrDefPtr chr = dev->data.chr; + +if (!chr->source->data.nmdm.master) { +char uuidstr[VIR_UUID_STRING_BUFLEN]; + +virUUIDFormat(def->uuid, uuidstr); + +chr->source->data.nmdm.master = g_strdup_printf("/dev/nmdm%sA", uuidstr); +chr->source->data.nmdm.slave = g_strdup_printf("/dev/nmdm%sB", uuidstr); +} +} + return 0; } diff --git a/src/conf/domain_validate.c b/src/conf/domain_validate.c index 222a9386f6..de3661fa3d 100644 --- a/src/conf/domain_validate.c +++ b/src/conf/domain_validate.c @@ -579,17 +579,14 @@ virDomainChrSourceDefValidate(const virDomainChrSourceDef *src_def, break; case VIR_DOMAIN_CHR_TYPE_NMDM: -if (!src_def->data.nmdm.master) { -virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Missing master path attribute for nmdm device")); -return -1; +if ((src_def->data.nmdm.master && !src_def->data.nmdm.slave) || +(!src_def->data.nmdm.master && src_def->data.nmdm.slave)) { +virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Should define both master and slave " + "path attributes for nmdm device")); +return -1; } -if (!src_def->data.nmdm.slave) { -virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Missing slave path attribute for nmdm device")); -return -1; -} break; case VIR_DOMAIN_CHR_TYPE_TCP: diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-console-master-slave-not-specified.args b/tests/bhyvexml2argvdata/bhyvexml2argv-console-master-slave-not-specified.args new file mode 100644 index 00..b24918e7eb --- /dev/null +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-console-master-slave-not-specified.args @@ -0,0 +1,11 @@ +/usr/sbin/bhyve \ +-c 1 \ +-m 214 \ +-u \ +-H \ +-P \ +-s 0:0,hostbridge \ +-s 1:0,lpc \ +-s 2:0,ahci,hd:/tmp/freebsd.img \ +-s 3:0,virtio-net,faketapdev,mac=52:54:00:b1:42:eb \ +-l com1,/dev/nmdmdf3be7e7-a104-11e3-aeb0-50e5492bd3dcA bhyve diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-console-master-slave-not-specified.ldargs b/tests/bhyvexml2argvdata/bhyvexml2argv-console-master-slave-not-specified.ldargs new file mode 100644 index 00..32538b558e --- /dev/null +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-console-master-slave-not-specified.ldargs @@ -0,0 +1,3 @@ +/usr/sbin/bhyveload \ +-m 214 \ +-d /tmp/freebsd.img bhyve diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-console-master-slave-not-specified.xml b/tests/bhyvexml2argvdata/bhyvexml2argv-console-master-slave-not-specified.xml new file mode 100644 index 00..6e198a49d1 --- /dev/
Re: [PATCH] bhyve: relax emulator binary value check
Michal Privoznik wrote: > On 2/4/21 4:47 PM, Roman Bogorodskiy wrote: > > Currently, requesting domain capabilities fails when the specified > > emulator binary does not equal to "/usr/sbin/bhyve". Relax this check > > to allow any path with basename "bhyve", as it's a common case when > > binary is specified without an absolute path. > > > > Signed-off-by: Roman Bogorodskiy > > --- > > I'm not sure how useful is this check in general: at this point we don't > > use the user specified emulator value anyway, just use BHYVE constant. > > Probably it would be better to just drop the entire "else" block? > > Yes, I was just about to suggest that. We don't do any check like this > for QEMU driver. Just execute user provided binary (with sume arguments > appended to get QMP monitor) and query caps. > > Looking into bhyve driver code though, it doesn't use from > domain XML, does it? What I'm getting at is that there is no way for a > user to specify different binary to run than /usr/sbin/bhyve. So it > doesn't really make sense to provide emulatorbin at all. Yes, currently we just use BHYVE (from config.h) to build commands. In general, I've just realized that it's a little bit messy right now. Even if we switch command line builder to use user specified value, this will not work properly because we still use virFindFileInPath("bhyve"); to populate driver->bhyvecaps. I guess the right way would be to move most of the code from virBhyveProbeCaps() to virBhyveDomainCapsFill() and try to use the user specified binary. > Since I can argue both ways, merge this patch or just remove the block > completely. I'll just remove the block then. Thanks, > Reviewed-by: Michal Privoznik > > Michal > Roman Bogorodskiy signature.asc Description: PGP signature
[PATCH] bhyve: relax emulator binary value check
Currently, requesting domain capabilities fails when the specified emulator binary does not equal to "/usr/sbin/bhyve". Relax this check to allow any path with basename "bhyve", as it's a common case when binary is specified without an absolute path. Signed-off-by: Roman Bogorodskiy --- I'm not sure how useful is this check in general: at this point we don't use the user specified emulator value anyway, just use BHYVE constant. Probably it would be better to just drop the entire "else" block? src/bhyve/bhyve_driver.c | 13 - 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/src/bhyve/bhyve_driver.c b/src/bhyve/bhyve_driver.c index 78c3241293..277be8dbb0 100644 --- a/src/bhyve/bhyve_driver.c +++ b/src/bhyve/bhyve_driver.c @@ -1647,11 +1647,14 @@ bhyveConnectGetDomainCapabilities(virConnectPtr conn, if (emulatorbin == NULL) { emulatorbin = "/usr/sbin/bhyve"; -} else if (STRNEQ(emulatorbin, "/usr/sbin/bhyve")) { -virReportError(VIR_ERR_INVALID_ARG, - _("unknown emulator binary: %s"), - emulatorbin); -goto cleanup; +} else { +g_autofree char *emulatorbasename = g_path_get_basename(emulatorbin); +if (STRNEQ(emulatorbasename, "bhyve")) { +virReportError(VIR_ERR_INVALID_ARG, + _("unknown emulator binary: %s"), + emulatorbin); +goto cleanup; +} } if (!(caps = virBhyveDomainCapsBuild(conn->privateData, emulatorbin, -- 2.30.0
[PATCH] virfile: workaround for when posix_fallocate() is not supported by FS
posix_fallocate() might be not supported by a filesystem, for example, it's not supported by ZFS. In that case it fails with return code 22 (EINVAL), and thus safezero_posix_fallocate() returns -1. As safezero_posix_fallocate() is the first function tried by safezero() and it tries other functions only when it returns -2, it fails immediately without falling back to other methods, such as safezero_slow(). Fix that by returning -2 if posix_fallocate() returns EINVAL, to give safezero() a chance to try other functions. Signed-off-by: Roman Bogorodskiy --- src/util/virfile.c | 10 ++ 1 file changed, 10 insertions(+) diff --git a/src/util/virfile.c b/src/util/virfile.c index 609cafdd34..67a350deb3 100644 --- a/src/util/virfile.c +++ b/src/util/virfile.c @@ -1119,6 +1119,16 @@ safezero_posix_fallocate(int fd, off_t offset, off_t len) int ret = posix_fallocate(fd, offset, len); if (ret == 0) return 0; +else if (ret == EINVAL) +/* EINVAL is returned when either: + - Operation is not supported by the underlying filesystem, + - offset or len argument values are invalid. + Assuming that offset and len are valid, this error means + the operation is not supported, and we need to fall back + to other methods. +*/ +return -2; + errno = ret; return -1; } -- 2.30.0
Re: [PATCH 2/5] bhyve: remove redundant code that adds "template" netdev name
Laine Stump wrote: > The FreeBSD version of virNetDevTapCreate() now calls > virNetDevGenerateName(), and virNetDevGenerateName() understands that > a blank ifname should be replaced with a generated name based on a > device-type-specific template - so there is no longer any need for the > higher level functions to stuff a template name ("vnet%d") into > ifname. > > Signed-off-by: Laine Stump For this and 1/5: Reviewed-by: Roman Bogorodskiy Thanks for this cleanup. > --- > src/bhyve/bhyve_command.c | 7 --- > 1 file changed, 7 deletions(-) > > diff --git a/src/bhyve/bhyve_command.c b/src/bhyve/bhyve_command.c > index 4cf98c0eb1..daf313c9c1 100644 > --- a/src/bhyve/bhyve_command.c > +++ b/src/bhyve/bhyve_command.c > @@ -79,13 +79,6 @@ bhyveBuildNetArgStr(const virDomainDef *def, > goto cleanup; > } > > -if (!net->ifname || > -STRPREFIX(net->ifname, VIR_NET_GENERATED_VNET_PREFIX) || > -strchr(net->ifname, '%')) { > -VIR_FREE(net->ifname); > -net->ifname = g_strdup(VIR_NET_GENERATED_VNET_PREFIX "%d"); > -} > - > if (!dryRun) { > if (virNetDevTapCreateInBridgePort(brname, &net->ifname, &net->mac, > def->uuid, NULL, NULL, 0, > -- > 2.28.0 > Roman Bogorodskiy signature.asc Description: PGP signature
Re: [PATCH] bhyve: implement virtio-9p support
Daniel P. Berrangé wrote: > On Thu, Oct 08, 2020 at 05:06:16PM +0400, Roman Bogorodskiy wrote: > > Recently virtio-9p support was added to bhyve. > > > > On the host side it looks this way: > > > > bhyve -s 25:0,virtio-9p,sharename=/path/to/shared/dir > > > > It could also have ",ro" suffix to make share read-only. > > > > In the Linux guest, this share is mounted with: > > > > mount -t 9p sharename /mnt/sharename > > > > In the guest user will see the same permissions and ownership > > information for this directory as on the host. No uid/gid remapping is > > supported, so those could resolve to wrong user or group names. > > > > The same applies to the other side: chowning/chmodding in the guest will > > set specified ownership and permissions on the host. > > > > In libvirt domain XML it's modeled using the 'filesystem' element: > > > > > > > > > > > > > > diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-fs-9p-readonly.xml > > b/tests/bhyvexml2argvdata/bhyvexml2argv-fs-9p-readonly.xml > > new file mode 100644 > > index 00..6341236654 > > --- /dev/null > > +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-fs-9p-readonly.xml > > @@ -0,0 +1,28 @@ > > + > > + bhyve > > + df3be7e7-a104-11e3-aeb0-50e5492bd3dc > > + 219136 > > + 1 > > + > > +hvm > > + > > + > > + > > + > > + > > + > > + > > + > > + > > + > > + > > + > > + > function='0x0'/> > > + > > + > > This is missing the type="mount" attribute which should be mandatory. > It suggests we're not validating the type in the driver, before accessing > the element, which is dangerous. > > > + > > + > > + > > + > > + > > + > > The other demo XML files are the same. Hm, as I can see in the schema, type="mount" is default. That's what I see in virDomainFSDefParseXML() @ src/conf/domain_conf.c as well. I also check that in the driver, and there's a test for it: tests/bhyvexml2argvdata/bhyvexml2argv-fs-9p-unsupported-type.xml Are you referring to something different? > 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 :| > Roman Bogorodskiy signature.asc Description: PGP signature
Re: [PATCH 1/3] bhyve: fix virtio-9p src/dst order
Michal Privoznik wrote: > On 10/10/20 6:13 AM, Roman Bogorodskiy wrote: > > For the virtio-9p bhyve command line argument, the proper order > > is mount_tag=/path/to/host/dir, not the opposite. > > > > Signed-off-by: Roman Bogorodskiy > > --- > > src/bhyve/bhyve_command.c | 2 +- > > tests/bhyvexml2argvdata/bhyvexml2argv-fs-9p-readonly.args | 2 +- > > tests/bhyvexml2argvdata/bhyvexml2argv-fs-9p.args | 2 +- > > 3 files changed, 3 insertions(+), 3 deletions(-) > > Ooops, sorry for not spotting this during review. > > Reviewed-by: Michal Privoznik > > Michal > My fault, apparently I didn't test the final version where I've already replaced my hardcoded test paths with the actual data from XML. Thanks Roman Bogorodskiy signature.asc Description: PGP signature
[PATCH 3/3] news: document bhyve virtio-9p support
Signed-off-by: Roman Bogorodskiy --- NEWS.rst | 4 1 file changed, 4 insertions(+) diff --git a/NEWS.rst b/NEWS.rst index e708f06e9e..bc35458f38 100644 --- a/NEWS.rst +++ b/NEWS.rst @@ -25,6 +25,10 @@ v6.9.0 (unreleased) ``virConnectGetVersion()``, and ``virDomainGetAutostart()`` APIs have been implemented in the Hyper-V driver. + * bhyve: implement virtio-9p filesystem support + +Implement virito-9p shared filesystem using the element. + * **Improvements** * **Bug fixes** -- 2.28.0
[PATCH 1/3] bhyve: fix virtio-9p src/dst order
For the virtio-9p bhyve command line argument, the proper order is mount_tag=/path/to/host/dir, not the opposite. Signed-off-by: Roman Bogorodskiy --- src/bhyve/bhyve_command.c | 2 +- tests/bhyvexml2argvdata/bhyvexml2argv-fs-9p-readonly.args | 2 +- tests/bhyvexml2argvdata/bhyvexml2argv-fs-9p.args | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/bhyve/bhyve_command.c b/src/bhyve/bhyve_command.c index 7606840f45..acf3a5a433 100644 --- a/src/bhyve/bhyve_command.c +++ b/src/bhyve/bhyve_command.c @@ -607,8 +607,8 @@ bhyveBuildFSArgStr(const virDomainDef *def G_GNUC_UNUSED, virCommandAddArgFormat(cmd, "%d:%d,virtio-9p,%s=%s%s", fs->info.addr.pci.slot, fs->info.addr.pci.function, - fs->src->path, fs->dst, + fs->src->path, virBufferCurrentContent(¶ms)); return 0; diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-fs-9p-readonly.args b/tests/bhyvexml2argvdata/bhyvexml2argv-fs-9p-readonly.args index 193895574d..bfcd88e366 100644 --- a/tests/bhyvexml2argvdata/bhyvexml2argv-fs-9p-readonly.args +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-fs-9p-readonly.args @@ -7,4 +7,4 @@ -s 0:0,hostbridge \ -s 2:0,ahci,hd:/tmp/freebsd.img \ -s 3:0,virtio-net,faketapdev,mac=52:54:00:b9:94:02 \ --s 4:0,virtio-9p,/shared/dir=shared_dir,ro bhyve +-s 4:0,virtio-9p,shared_dir=/shared/dir,ro bhyve diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-fs-9p.args b/tests/bhyvexml2argvdata/bhyvexml2argv-fs-9p.args index 0d27954432..e890f7400b 100644 --- a/tests/bhyvexml2argvdata/bhyvexml2argv-fs-9p.args +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-fs-9p.args @@ -7,4 +7,4 @@ -s 0:0,hostbridge \ -s 2:0,ahci,hd:/tmp/freebsd.img \ -s 3:0,virtio-net,faketapdev,mac=52:54:00:b9:94:02 \ --s 4:0,virtio-9p,/shared/dir=shared_dir bhyve +-s 4:0,virtio-9p,shared_dir=/shared/dir bhyve -- 2.28.0
[PATCH 2/3] docs: bhyve: document virtio-9p support
Signed-off-by: Roman Bogorodskiy --- docs/drvbhyve.html.in | 21 + 1 file changed, 21 insertions(+) diff --git a/docs/drvbhyve.html.in b/docs/drvbhyve.html.in index 49d4aa5878..228e8b2bd5 100644 --- a/docs/drvbhyve.html.in +++ b/docs/drvbhyve.html.in @@ -482,6 +482,27 @@ to the guest, with ich7 being the only supported model now, and the audio element specifies how the guest device is mapped to the host sound device. +Virtio-9p filesystem + +As of https://svnweb.freebsd.org/changeset/base/366413";>FreeBSD changeset r366413 +bhyve supports sharing arbitrary directory tree between the guest and the host. +It's supported in libvirt since 6.9.0. + + +... + <filesystem> +<source dir='/shared/dir'/> +<target dir='shared_dir'/> + </filesystem> +... + + +This share could be made read only by adding the <readonly/> sub-element. + +In the Linux guest, this could be mounted using: + +mount -t 9p shared_dir /mnt/shared_dir + Wiring guest memory Since 4.4.0, it's possible to specify that guest memory should -- 2.28.0
Re: [PATCH] bhyve: implement virtio-9p support
Roman Bogorodskiy wrote: > Recently virtio-9p support was added to bhyve. > > On the host side it looks this way: > > bhyve -s 25:0,virtio-9p,sharename=/path/to/shared/dir > > It could also have ",ro" suffix to make share read-only. > > In the Linux guest, this share is mounted with: > > mount -t 9p sharename /mnt/sharename > > In the guest user will see the same permissions and ownership > information for this directory as on the host. No uid/gid remapping is > supported, so those could resolve to wrong user or group names. > > The same applies to the other side: chowning/chmodding in the guest will > set specified ownership and permissions on the host. > > In libvirt domain XML it's modeled using the 'filesystem' element: > > > > > > > Optional 'readonly' sub-element enables read-only mode. > > Signed-off-by: Roman Bogorodskiy > --- > src/bhyve/bhyve_capabilities.c| 14 > src/bhyve/bhyve_capabilities.h| 1 + > src/bhyve/bhyve_command.c | 72 +++ > src/bhyve/bhyve_device.c | 10 +++ > src/libvirt_private.syms | 1 + > .../bhyvexml2argv-fs-9p-readonly.args | 10 +++ > .../bhyvexml2argv-fs-9p-readonly.ldargs | 3 + > .../bhyvexml2argv-fs-9p-readonly.xml | 28 > ...exml2argv-fs-9p-unsupported-accessmode.xml | 27 +++ > ...bhyvexml2argv-fs-9p-unsupported-driver.xml | 28 > .../bhyvexml2argv-fs-9p.args | 10 +++ > .../bhyvexml2argv-fs-9p.ldargs| 3 + > .../bhyvexml2argvdata/bhyvexml2argv-fs-9p.xml | 27 +++ > tests/bhyvexml2argvtest.c | 9 ++- > .../bhyvexml2xmlout-fs-9p.xml | 38 ++ > tests/bhyvexml2xmltest.c | 1 + > 16 files changed, 281 insertions(+), 1 deletion(-) > create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-fs-9p-readonly.args > create mode 100644 > tests/bhyvexml2argvdata/bhyvexml2argv-fs-9p-readonly.ldargs > create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-fs-9p-readonly.xml > create mode 100644 > tests/bhyvexml2argvdata/bhyvexml2argv-fs-9p-unsupported-accessmode.xml > create mode 100644 > tests/bhyvexml2argvdata/bhyvexml2argv-fs-9p-unsupported-driver.xml > create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-fs-9p.args > create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-fs-9p.ldargs > create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-fs-9p.xml > create mode 100644 tests/bhyvexml2xmloutdata/bhyvexml2xmlout-fs-9p.xml This is missing tests/bhyvexml2argvdata/bhyvexml2argv-fs-9p-unsupported-type.xml It doesn't seem to be important enough to justify sending v2 just because of it, so it's here: https://gitlab.com/rbogorodskiy/libvirt/-/blob/bhyve-9p/tests/bhyvexml2argvdata/bhyvexml2argv-fs-9p-unsupported-type.xml Roman Bogorodskiy signature.asc Description: PGP signature
[PATCH] bhyve: implement virtio-9p support
Recently virtio-9p support was added to bhyve. On the host side it looks this way: bhyve -s 25:0,virtio-9p,sharename=/path/to/shared/dir It could also have ",ro" suffix to make share read-only. In the Linux guest, this share is mounted with: mount -t 9p sharename /mnt/sharename In the guest user will see the same permissions and ownership information for this directory as on the host. No uid/gid remapping is supported, so those could resolve to wrong user or group names. The same applies to the other side: chowning/chmodding in the guest will set specified ownership and permissions on the host. In libvirt domain XML it's modeled using the 'filesystem' element: Optional 'readonly' sub-element enables read-only mode. Signed-off-by: Roman Bogorodskiy --- src/bhyve/bhyve_capabilities.c| 14 src/bhyve/bhyve_capabilities.h| 1 + src/bhyve/bhyve_command.c | 72 +++ src/bhyve/bhyve_device.c | 10 +++ src/libvirt_private.syms | 1 + .../bhyvexml2argv-fs-9p-readonly.args | 10 +++ .../bhyvexml2argv-fs-9p-readonly.ldargs | 3 + .../bhyvexml2argv-fs-9p-readonly.xml | 28 ...exml2argv-fs-9p-unsupported-accessmode.xml | 27 +++ ...bhyvexml2argv-fs-9p-unsupported-driver.xml | 28 .../bhyvexml2argv-fs-9p.args | 10 +++ .../bhyvexml2argv-fs-9p.ldargs| 3 + .../bhyvexml2argvdata/bhyvexml2argv-fs-9p.xml | 27 +++ tests/bhyvexml2argvtest.c | 9 ++- .../bhyvexml2xmlout-fs-9p.xml | 38 ++ tests/bhyvexml2xmltest.c | 1 + 16 files changed, 281 insertions(+), 1 deletion(-) create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-fs-9p-readonly.args create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-fs-9p-readonly.ldargs create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-fs-9p-readonly.xml create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-fs-9p-unsupported-accessmode.xml create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-fs-9p-unsupported-driver.xml create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-fs-9p.args create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-fs-9p.ldargs create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-fs-9p.xml create mode 100644 tests/bhyvexml2xmloutdata/bhyvexml2xmlout-fs-9p.xml diff --git a/src/bhyve/bhyve_capabilities.c b/src/bhyve/bhyve_capabilities.c index 96cfe8357a..8a9acf52b0 100644 --- a/src/bhyve/bhyve_capabilities.c +++ b/src/bhyve/bhyve_capabilities.c @@ -344,6 +344,17 @@ bhyveProbeCapsVNCPassword(unsigned int *caps, char *binary) } +static int +bhyveProbeCapsVirtio9p(unsigned int *caps, char *binary) +{ +return bhyveProbeCapsDeviceHelper(caps, binary, + "-s", + "0,virtio-9p", + "pci slot 0:0: unknown device \"hda\"", + BHYVE_CAP_VIRTIO_9P); +} + + int virBhyveProbeCaps(unsigned int *caps) { @@ -378,6 +389,9 @@ virBhyveProbeCaps(unsigned int *caps) if ((ret = bhyveProbeCapsVNCPassword(caps, binary))) goto out; +if ((ret = bhyveProbeCapsVirtio9p(caps, binary))) +goto out; + out: VIR_FREE(binary); return ret; diff --git a/src/bhyve/bhyve_capabilities.h b/src/bhyve/bhyve_capabilities.h index b2a16b0189..1b25c000b5 100644 --- a/src/bhyve/bhyve_capabilities.h +++ b/src/bhyve/bhyve_capabilities.h @@ -51,6 +51,7 @@ typedef enum { BHYVE_CAP_CPUTOPOLOGY = 1 << 6, BHYVE_CAP_SOUND_HDA = 1 << 7, BHYVE_CAP_VNC_PASSWORD = 1 << 8, +BHYVE_CAP_VIRTIO_9P = 1 << 9, } virBhyveCapsFlags; int virBhyveProbeGrubCaps(virBhyveGrubCapsFlags *caps); diff --git a/src/bhyve/bhyve_command.c b/src/bhyve/bhyve_command.c index 7526f10fb1..7606840f45 100644 --- a/src/bhyve/bhyve_command.c +++ b/src/bhyve/bhyve_command.c @@ -547,6 +547,73 @@ bhyveBuildSoundArgStr(const virDomainDef *def G_GNUC_UNUSED, return 0; } +static int +bhyveBuildFSArgStr(const virDomainDef *def G_GNUC_UNUSED, + virDomainFSDefPtr fs, + virCommandPtr cmd) +{ +g_auto(virBuffer) params = VIR_BUFFER_INITIALIZER; + +switch ((virDomainFSType) fs->type) { +case VIR_DOMAIN_FS_TYPE_MOUNT: +break; +case VIR_DOMAIN_FS_TYPE_BLOCK: +case VIR_DOMAIN_FS_TYPE_FILE: +case VIR_DOMAIN_FS_TYPE_TEMPLATE: +case VIR_DOMAIN_FS_TYPE_RAM: +case VIR_DOMAIN_FS_TYPE_BIND: +case VIR_DOMAIN_FS_TYPE_VOLUME: +case VIR_DOMAIN_FS_TYPE_LAST: +virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("unsupported filesystem type '%s'"), + virDomainFSTypeToString(fs->t
Re: [PATCH 11/21] bhyve: parse: Set video device model when parsing bhyve commandline
Peter Krempa wrote: > Add the proper video device type when parsing bhyve's commandline into a > XML. > > Signed-off-by: Peter Krempa > --- > src/bhyve/bhyve_parse_command.c | 2 ++ > tests/bhyveargv2xmldata/bhyveargv2xml-vnc-listen.xml | 2 +- > tests/bhyveargv2xmldata/bhyveargv2xml-vnc-password.xml | 2 +- > tests/bhyveargv2xmldata/bhyveargv2xml-vnc-resolution.xml | 2 +- > tests/bhyveargv2xmldata/bhyveargv2xml-vnc-vga-io.xml | 2 +- > tests/bhyveargv2xmldata/bhyveargv2xml-vnc-vga-off.xml| 2 +- > tests/bhyveargv2xmldata/bhyveargv2xml-vnc-vga-on.xml | 2 +- > tests/bhyveargv2xmldata/bhyveargv2xml-vnc.xml| 2 +- > 8 files changed, 9 insertions(+), 7 deletions(-) Reviewed-by: Roman Bogorodskiy Roman Bogorodskiy signature.asc Description: PGP signature
Re: [PATCH 10/21] bhyveargv2xmldata: Remove XML files for console2/3/4 test case
Peter Krempa wrote: > The test case is invoked using DO_TEST_FAIL so the XML files are > actually unexpected, unused and actually don't even conform to the RNG > schema for . > > Signed-off-by: Peter Krempa > --- > .../bhyveargv2xml-console2.xml| 15 --- > .../bhyveargv2xml-console3.xml| 27 --- > .../bhyveargv2xml-console4.xml| 15 --- > 3 files changed, 57 deletions(-) > delete mode 100644 tests/bhyveargv2xmldata/bhyveargv2xml-console2.xml > delete mode 100644 tests/bhyveargv2xmldata/bhyveargv2xml-console3.xml > delete mode 100644 tests/bhyveargv2xmldata/bhyveargv2xml-console4.xml Reviewed-by: Roman Bogorodskiy Roman Bogorodskiy signature.asc Description: PGP signature
Re: [RFC] bhyve: modeling virtio-9p
Daniel P. Berrangé wrote: > On Tue, Oct 06, 2020 at 06:51:52PM +0400, Roman Bogorodskiy wrote: > > Hi, > > > > Recently bhyve got virtio-9p support. Modeling it appears to be pretty > > straight-forward, but probably I'm missing something, so decided to > > discuss first before proceeding with the implementation. > > > > On the host side it looks like this: > > > > bhyve -s 25:0,virtio-9p,distfiles=/workspace/distfiles > > > > Mounting it in a (Linux) guest looks this way: > > > > mount -t 9p distfiles /mnt/distfiles > > > > lspci(8) shows it like this: > > > > 00:1f.0 SCSI storage controller: Red Hat, Inc. Virtio filesystem > > Subsystem: Red Hat, Inc. Virtio filesystem > > Flags: bus master, fast devsel, latency 64, IRQ 20 > > I/O ports at 2200 [size=512] > > Memory at c2004000 (32-bit, non-prefetchable) [size=8K] > > Expansion ROM at c0007000 [virtual] [disabled] [size=2K] > > Capabilities: [40] MSI-X: Enable+ Count=2 Masked- > > Capabilities: [4c] MSI: Enable- Count=1/1 Maskable- 64bit+ > > Kernel driver in use: virtio-pci > > > > I was thinking about presenting it like this: > > > > > > > > This driver type is for virtio-fuse, which is different from virtio-9p. > In QEMU we support type=path and type=handle as two different QEMU > bakends for 9p. Or simply omit "type" entirely and it defaults to > 9p in QEMU. So I think you can just omit "type" for bhyve too. > > > > > > > > > > > There's also an optional element for readonly mounts, which > > is also supported by bhyve. > > Yep. > > Also consider the "accessmode" attribute - you'll want to validate > whichever value(s) of that make sense given the bhyve impl of 9p. It looks like the bhyve implementation doesn't do any uid/gid remapping, i.e. if I chown file on the host, I see same numeric ids (but they corresponding to different users/groups on my test Linux guest compared to the FreeBSD host). The same happens when I chown files in the guest and verify results in the host. Sounds like 'passthrough' is the right choice for this behavior? > 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 :| > Roman Bogorodskiy signature.asc Description: PGP signature
[RFC] bhyve: modeling virtio-9p
Hi, Recently bhyve got virtio-9p support. Modeling it appears to be pretty straight-forward, but probably I'm missing something, so decided to discuss first before proceeding with the implementation. On the host side it looks like this: bhyve -s 25:0,virtio-9p,distfiles=/workspace/distfiles Mounting it in a (Linux) guest looks this way: mount -t 9p distfiles /mnt/distfiles lspci(8) shows it like this: 00:1f.0 SCSI storage controller: Red Hat, Inc. Virtio filesystem Subsystem: Red Hat, Inc. Virtio filesystem Flags: bus master, fast devsel, latency 64, IRQ 20 I/O ports at 2200 [size=512] Memory at c2004000 (32-bit, non-prefetchable) [size=8K] Expansion ROM at c0007000 [virtual] [disabled] [size=2K] Capabilities: [40] MSI-X: Enable+ Count=2 Masked- Capabilities: [4c] MSI: Enable- Count=1/1 Maskable- 64bit+ Kernel driver in use: virtio-pci I was thinking about presenting it like this: There's also an optional element for readonly mounts, which is also supported by bhyve. Does this look reasonable? Roman Bogorodskiy signature.asc Description: PGP signature
[PATCH] docs: bhyve: style improvements
- Wrap long lines in "domxml-to-native" example so it fits content width, - For changeset revision links, use "FreeBSD changeset rN" or "changeset rN" instead of just "rN" to make it more readable. Signed-off-by: Roman Bogorodskiy --- docs/drvbhyve.html.in | 24 ++-- 1 file changed, 14 insertions(+), 10 deletions(-) diff --git a/docs/drvbhyve.html.in b/docs/drvbhyve.html.in index 2e58cc50e2..49d4aa5878 100644 --- a/docs/drvbhyve.html.in +++ b/docs/drvbhyve.html.in @@ -232,8 +232,8 @@ Then virsh console command can be used to connect to the text conso of a guest. NB: Some versions of bhyve have a bug that prevents guests from booting -until the console is opened by a client. This bug was fixed in FreeBSD -https://svnweb.freebsd.org/changeset/base/262884";>r262884. If +until the console is opened by a client. This bug was fixed in +https://svnweb.freebsd.org/changeset/base/262884";>FreeBSD changeset r262884. If an older version is used, one either has to open a console manually with virsh console to let a guest boot or start a guest using: @@ -272,7 +272,9 @@ tweak them. # virsh -c "bhyve:///system" domxml-to-native --format bhyve-argv --xml /path/to/bhyve.xml /usr/sbin/bhyveload -m 214 -d /home/user/vm1.img vm1 -/usr/sbin/bhyve -c 2 -m 214 -A -I -H -P -s 0:0,hostbridge -s 3:0,virtio-net,tap0,mac=52:54:00:5d:74:e3 -s 2:0,virtio-blk,/home/user/vm1.img -s 1,lpc -l com1,/dev/nmdm0A vm1 +/usr/sbin/bhyve -c 2 -m 214 -A -I -H -P -s 0:0,hostbridge \ +-s 3:0,virtio-net,tap0,mac=52:54:00:5d:74:e3 -s 2:0,virtio-blk,/home/user/vm1.img \ +-s 1,lpc -l com1,/dev/nmdm0A vm1 Using ZFS volumes @@ -416,10 +418,11 @@ Make sure you understand the risks associated with this feature before using it. Clock configuration Originally bhyve supported only localtime for RTC. Support for UTC time was introduced in -https://svnweb.freebsd.org/changeset/base/284894";>r284894 for 10-STABLE and -in https://svnweb.freebsd.org/changeset/base/279225";>r279225 for -CURRENT. -It's possible to use this in libvirt since 1.2.18, just place the -following to domain XML: +https://svnweb.freebsd.org/changeset/base/284894";>FreeBSD changeset r284894 +for 10-STABLE and +in https://svnweb.freebsd.org/changeset/base/279225";>changeset r279225 +for -CURRENT. It's possible to use this in libvirt since 1.2.18, +just place the following to domain XML: <domain type="bhyve"> @@ -443,8 +446,8 @@ you'll need to explicitly specify 'localtime' in this case: e1000 NIC -As of https://svnweb.freebsd.org/changeset/base/302504";>r302504 bhyve -supports Intel e1000 network adapter emulation. It's supported in libvirt +As of https://svnweb.freebsd.org/changeset/base/302504";>FreeBSD changeset r302504 +bhyve supports Intel e1000 network adapter emulation. It's supported in libvirt since 3.1.0 and could be used as follows: @@ -497,7 +500,8 @@ be wired and cannot be swapped out as follows: Since 4.5.0, it's possible to specify guest CPU topology, if bhyve supports that. Support for specifying guest CPU topology was added to bhyve in -https://svnweb.freebsd.org/changeset/base/332298";>r332298 for -CURRENT. +https://svnweb.freebsd.org/changeset/base/332298";>FreeBSD changeset r332298 +for -CURRENT. Example: <domain type="bhyve"> -- 2.28.0
Re: [libvirt PATCH 1/7] bhyve: use g_new0 instead of VIR_ALLOC*
Ján Tomko wrote: > Signed-off-by: Ján Tomko > --- > src/bhyve/bhyve_capabilities.c | 3 +-- > src/bhyve/bhyve_domain.c| 9 +++-- > src/bhyve/bhyve_driver.c| 3 +-- > src/bhyve/bhyve_parse_command.c | 7 +++ > 4 files changed, 8 insertions(+), 14 deletions(-) > Reviewed-by: Roman Bogorodskiy Roman Bogorodskiy signature.asc Description: PGP signature
Re: [PATCH] docs: bhyve: document sound device and VNC bits
Andrea Bolognani wrote: > On Thu, 2020-09-24 at 20:18 +0400, Roman Bogorodskiy wrote: > > +Note: VNC password authentication is known to be cryptographically weak. > > +Additionally, the password is passed as a command line argument in clear > > text. > > +Make sure you understand the risks associated with this feature before > > using it. > > Great warning :) > > > +Sound device > > + > > +As of > href="https://svnweb.freebsd.org/changeset/base/349355";>r349355 bhyve > > +supports sound device emulation. > > Is this changeset part of any FreeBSD release yet? If so, I would use > its version number here; if not, at least clarify it by saying "as of > [FreeBSD changeset r349355]" or something like that. It's not part of any released version. It'll be available with FreeBSD 13.0-RELEASE, which is planned to be released next year. > With the link text tweaked, > > Reviewed-by: Andrea Bolognani Thanks. I'll fix this link and will push the change as it should be safe for the freeze. Also, this document has a number of other similarly formatted links. I'll update those in a separate patch, along with some other nits I noticed. > -- > Andrea Bolognani / Red Hat / Virtualization > Roman Bogorodskiy signature.asc Description: PGP signature
[PATCH] docs: bhyve: document sound device and VNC bits
* Document sound device support, * Document VNC password configuration and framebuffer resolution. Signed-off-by: Roman Bogorodskiy --- docs/drvbhyve.html.in | 47 +++ 1 file changed, 47 insertions(+) diff --git a/docs/drvbhyve.html.in b/docs/drvbhyve.html.in index ca511eeccd..cffb63f1ad 100644 --- a/docs/drvbhyve.html.in +++ b/docs/drvbhyve.html.in @@ -389,6 +389,30 @@ it with the port attribute): <graphics type='vnc' autoport='yes'> +Since 6.8.0, it's possible to set framebuffer resolution +using the resolution sub-element: + + + <video> + <model type='gop' heads='1' primary='yes'> + <resolution x='800' y='600'/> + </model> + </video> + + +Since 6.8.0, VNC server can be configured to use +password based authentication: + + + <graphics type='vnc' port='5904' passwd='foobar'> +<listen type='address' address='127.0.0.1'/> + </graphics> + + +Note: VNC password authentication is known to be cryptographically weak. +Additionally, the password is passed as a command line argument in clear text. +Make sure you understand the risks associated with this feature before using it. + Clock configuration Originally bhyve supported only localtime for RTC. Support for UTC time was introduced in @@ -432,6 +456,29 @@ supports Intel e1000 network adapter emulation. It's supported in libvirt ... +Sound device + +As of https://svnweb.freebsd.org/changeset/base/349355";>r349355 bhyve +supports sound device emulation. It's supported in libvirt +since 6.7.0. + + +... + <sound model='ich7'> +<audio id='1'/> + </sound> + <audio id='1' type='oss'> +<input dev='/dev/dsp0'/> +<output dev='/dev/dsp0'/> + </audio> +... + + +Here, the sound element specifies the sound device as it's exposed +to the guest, with ich7 being the only supported model now, +and the audio element specifies how the guest device is mapped +to the host sound device. + Wiring guest memory Since 4.4.0, it's possible to specify that guest memory should -- 2.27.0
Re: [PATCH v2 1/4] bhyve: support parsing fbuf PCI device
Roman Bogorodskiy wrote: > From: Fabian Freyer > > Add a new helper function, bhyveParsePCIFbuf, to parse the bhyve-argv > parameters for a frame-buffer device to and > definitions. > > For now, only the listen address, port, and vga mode are detected. > Unsupported parameters are silently skipped. > > This involves upgrading the private API to expose the > virDomainGraphicsDefNew helper function, which is used by > bhyveParsePCIFbuf. > > Signed-off-by: Fabian Freyer > Signed-off-by: Roman Bogorodskiy > --- > src/bhyve/bhyve_parse_command.c | 91 ++- > src/libvirt_private.syms | 1 + > .../bhyveargv2xml-vnc-listen.args | 10 ++ > .../bhyveargv2xml-vnc-listen.xml | 22 + > .../bhyveargv2xml-vnc-vga-io.args | 10 ++ > .../bhyveargv2xml-vnc-vga-io.xml | 22 + > .../bhyveargv2xml-vnc-vga-off.args| 10 ++ > .../bhyveargv2xml-vnc-vga-off.xml | 23 + > .../bhyveargv2xml-vnc-vga-on.args | 10 ++ > .../bhyveargv2xml-vnc-vga-on.xml | 23 + > .../bhyveargv2xmldata/bhyveargv2xml-vnc.args | 10 ++ > tests/bhyveargv2xmldata/bhyveargv2xml-vnc.xml | 22 + > tests/bhyveargv2xmltest.c | 5 + > 13 files changed, 258 insertions(+), 1 deletion(-) > create mode 100644 tests/bhyveargv2xmldata/bhyveargv2xml-vnc-listen.args > create mode 100644 tests/bhyveargv2xmldata/bhyveargv2xml-vnc-listen.xml > create mode 100644 tests/bhyveargv2xmldata/bhyveargv2xml-vnc-vga-io.args > create mode 100644 tests/bhyveargv2xmldata/bhyveargv2xml-vnc-vga-io.xml > create mode 100644 tests/bhyveargv2xmldata/bhyveargv2xml-vnc-vga-off.args > create mode 100644 tests/bhyveargv2xmldata/bhyveargv2xml-vnc-vga-off.xml > create mode 100644 tests/bhyveargv2xmldata/bhyveargv2xml-vnc-vga-on.args > create mode 100644 tests/bhyveargv2xmldata/bhyveargv2xml-vnc-vga-on.xml > create mode 100644 tests/bhyveargv2xmldata/bhyveargv2xml-vnc.args > create mode 100644 tests/bhyveargv2xmldata/bhyveargv2xml-vnc.xml > > diff --git a/src/bhyve/bhyve_parse_command.c b/src/bhyve/bhyve_parse_command.c > index 50a5e88408..388c565317 100644 > --- a/src/bhyve/bhyve_parse_command.c > +++ b/src/bhyve/bhyve_parse_command.c > @@ -4,7 +4,7 @@ > * Copyright (C) 2006-2016 Red Hat, Inc. > * Copyright (C) 2006 Daniel P. Berrange > * Copyright (c) 2011 NetApp, Inc. > - * Copyright (C) 2016 Fabian Freyer > + * Copyright (C) 2020 Fabian Freyer > * > * This library is free software; you can redistribute it and/or > * modify it under the terms of the GNU Lesser General Public > @@ -553,6 +553,93 @@ bhyveParsePCINet(virDomainDefPtr def, > return -1; > } > > +static int > +bhyveParsePCIFbuf(virDomainDefPtr def, > + virDomainXMLOptionPtr xmlopt, > + unsigned caps G_GNUC_UNUSED, > + unsigned bus, > + unsigned slot, > + unsigned function, > + const char *config) > +{ > +/* -s slot,fbuf,wait,vga=on|io|off,rfb=:port,w=width,h=height */ > + > +virDomainVideoDefPtr video = NULL; > +virDomainGraphicsDefPtr graphics = NULL; > +char **params = NULL; > +char *param = NULL, *separator = NULL; > +size_t nparams = 0; > +unsigned int i = 0; Interesting, some of the CI jobs complain that this should be "size_t" (which I'll fix before merging), and some don't (including my local environment). Wondering if that's caused by different sed versions or something else. > + > +if (!(video = virDomainVideoDefNew(xmlopt))) > +goto cleanup; > + > + if (!(graphics = virDomainGraphicsDefNew(xmlopt))) > +goto cleanup; > + > +graphics->type = VIR_DOMAIN_GRAPHICS_TYPE_VNC; > +video->info.addr.pci.bus = bus; > +video->info.addr.pci.slot = slot; > +video->info.addr.pci.function = function; Roman Bogorodskiy signature.asc Description: PGP signature
[PATCH v2 2/4] bhyve: add support for setting fbuf resolution
From: Fabian Freyer The resolution of the VNC framebuffer can now be set via the resolution definition introduced in 5.9.0. Also, add "gop" to the list of model types the sub-element is valid for. Signed-off-by: Fabian Freyer Signed-off-by: Roman Bogorodskiy --- NEWS.rst | 5 +++ docs/formatdomain.rst | 3 +- src/bhyve/bhyve_command.c | 3 ++ src/bhyve/bhyve_parse_command.c | 20 .../bhyveargv2xml-vnc-resolution.args | 10 ++ .../bhyveargv2xml-vnc-resolution.xml | 24 ++ tests/bhyveargv2xmltest.c | 1 + .../bhyvexml2argv-vnc-resolution.args | 10 ++ .../bhyvexml2argv-vnc-resolution.ldargs | 1 + .../bhyvexml2argv-vnc-resolution.xml | 20 tests/bhyvexml2argvtest.c | 1 + .../bhyvexml2xmlout-vnc-resolution.xml| 31 +++ tests/bhyvexml2xmltest.c | 1 + 13 files changed, 129 insertions(+), 1 deletion(-) create mode 100644 tests/bhyveargv2xmldata/bhyveargv2xml-vnc-resolution.args create mode 100644 tests/bhyveargv2xmldata/bhyveargv2xml-vnc-resolution.xml create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-vnc-resolution.args create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-vnc-resolution.ldargs create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-vnc-resolution.xml create mode 100644 tests/bhyvexml2xmloutdata/bhyvexml2xmlout-vnc-resolution.xml diff --git a/NEWS.rst b/NEWS.rst index 685c5e2225..bb48f5bd43 100644 --- a/NEWS.rst +++ b/NEWS.rst @@ -20,6 +20,11 @@ v6.8.0 (unreleased) attribute of the device's element can be used to disable the filtering and allow all guest writes to the configuration space. + * bhyve: Support setting the framebuffer resolution + +Libvirt can now set the framebuffer's "w" and "h" parameters +using the ``resolution`` element. + * **Improvements** * qemu: Allow migration over UNIX sockets diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst index d5930a4ac1..888db5ea29 100644 --- a/docs/formatdomain.rst +++ b/docs/formatdomain.rst @@ -5822,7 +5822,8 @@ A video device. :since:`Since 5.9.0` , the ``model`` element may also have an optional ``resolution`` sub-element. The ``resolution`` element has attributes ``x`` and ``y`` to set the minimum resolution for the video device. This - sub-element is valid for model types "vga", "qxl", "bochs", and "virtio". + sub-element is valid for model types "vga", "qxl", "bochs", "gop", + and "virtio". ``acceleration`` Configure if video acceleration should be enabled. diff --git a/src/bhyve/bhyve_command.c b/src/bhyve/bhyve_command.c index 4df5baabdf..176a339d5a 100644 --- a/src/bhyve/bhyve_command.c +++ b/src/bhyve/bhyve_command.c @@ -468,6 +468,9 @@ bhyveBuildGraphicsArgStr(const virDomainDef *def, return -1; } +if (video->res) +virBufferAsprintf(&opt, ",w=%d,h=%d", video->res->x, video->res->y); + if (video->driver) virBufferAsprintf(&opt, ",vga=%s", virDomainVideoVGAConfTypeToString(video->driver->vgaconf)); diff --git a/src/bhyve/bhyve_parse_command.c b/src/bhyve/bhyve_parse_command.c index 388c565317..c6abdfacf3 100644 --- a/src/bhyve/bhyve_parse_command.c +++ b/src/bhyve/bhyve_parse_command.c @@ -621,6 +621,26 @@ bhyveParsePCIFbuf(virDomainDefPtr def, if (virStrToLong_i(param, NULL, 10, &graphics->data.vnc.port)) goto error; } + +if (STRPREFIX(param, "w=")) { +param += strlen("w="); + +if (video->res == NULL) +video->res = g_new0(virDomainVideoResolutionDef, 1); + +if (virStrToLong_uip(param, NULL, 10, &video->res->x)) +goto error; +} + +if (STRPREFIX(param, "h=")) { +param += strlen("h="); + +if (video->res == NULL) +video->res = g_new0(virDomainVideoResolutionDef, 1); + +if (virStrToLong_uip(param, NULL, 10, &video->res->y)) +goto error; +} } cleanup: diff --git a/tests/bhyveargv2xmldata/bhyveargv2xml-vnc-resolution.args b/tests/bhyveargv2xmldata/bhyveargv2xml-vnc-resolution.args new file mode 100644 index 00..e5e2c0f2e8 --- /dev/null +++ b/tests/bhyveargv2xmldata/bhyveargv2xml-vnc-resolution.args @@ -0,0 +1,10 @@ +/usr/sbin/bhyve \ +-c 1 \ +-m 214 \ +-u \ +-H \ +-P \ +-s 0:0,hostbridge \ +-l bootrom,/path/to/test.fd \ +-s 2:0,fbuf,tcp=127.0.0.1:5904,w=1920,h=1080 \ +-s 1,lpc bhyve diff --git a/tests/bhyveargv2xmldata/bhyveargv2xml
[PATCH v2 4/4] bhyve: add VNC password support
From: Fabian Freyer Support setting a password for the VNC framebuffer using the passwd attribute on the element, if the driver has the BHYVE_CAP_VNC_PASSWORD capability. Note that virsh domxml-from-native does not output the password in the generated XML, as VIR_DOMAIN_DEF_FORMAT_SECURE is not set when formatting the domain definition. Signed-off-by: Fabian Freyer Signed-off-by: Roman Bogorodskiy --- NEWS.rst | 7 +++ src/bhyve/bhyve_command.c | 33 +- src/bhyve/bhyve_parse_command.c | 5 +++ .../bhyveargv2xml-vnc-password.args | 10 + .../bhyveargv2xml-vnc-password.xml| 22 ++ tests/bhyveargv2xmltest.c | 3 +- .../bhyvexml2argv-vnc-password-comma.xml | 26 +++ .../bhyvexml2argv-vnc-password.args | 12 + .../bhyvexml2argv-vnc-password.ldargs | 1 + .../bhyvexml2argv-vnc-password.xml| 26 +++ tests/bhyvexml2argvtest.c | 8 +++- .../bhyvexml2xmlout-vnc-password.xml | 44 +++ tests/bhyvexml2xmltest.c | 1 + 13 files changed, 185 insertions(+), 13 deletions(-) create mode 100644 tests/bhyveargv2xmldata/bhyveargv2xml-vnc-password.args create mode 100644 tests/bhyveargv2xmldata/bhyveargv2xml-vnc-password.xml create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-vnc-password-comma.xml create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-vnc-password.args create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-vnc-password.ldargs create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-vnc-password.xml create mode 100644 tests/bhyvexml2xmloutdata/bhyvexml2xmlout-vnc-password.xml diff --git a/NEWS.rst b/NEWS.rst index bb48f5bd43..c949cb941b 100644 --- a/NEWS.rst +++ b/NEWS.rst @@ -25,6 +25,13 @@ v6.8.0 (unreleased) Libvirt can now set the framebuffer's "w" and "h" parameters using the ``resolution`` element. + * bhyve: Support VNC password authentication + +Libvirt can now probe whether the bhyve binary supports +VNC password authentication. In case it does, a VNC password +can now be passed using the ``passwd`` attribute on +the element. + * **Improvements** * qemu: Allow migration over UNIX sockets diff --git a/src/bhyve/bhyve_command.c b/src/bhyve/bhyve_command.c index 176a339d5a..1b48438168 100644 --- a/src/bhyve/bhyve_command.c +++ b/src/bhyve/bhyve_command.c @@ -424,17 +424,6 @@ bhyveBuildGraphicsArgStr(const virDomainDef *def, return -1; } -if (graphics->data.vnc.auth.passwd) { -virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("vnc password auth not supported")); -return -1; -} else { - /* Bhyve doesn't support VNC Auth yet, so print a warning about - * unauthenticated VNC sessions */ - VIR_WARN("%s", _("Security warning: currently VNC auth is not" - " supported.")); -} - if (glisten->address) { escapeAddr = strchr(glisten->address, ':') != NULL; if (escapeAddr) @@ -468,6 +457,28 @@ bhyveBuildGraphicsArgStr(const virDomainDef *def, return -1; } +if (graphics->data.vnc.auth.passwd) { +if (!(bhyveDriverGetBhyveCaps(driver) & BHYVE_CAP_VNC_PASSWORD)) { +virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("VNC Passwort authentication not supported " + "by bhyve")); +return -1; +} + +if (strchr(graphics->data.vnc.auth.passwd, ',')) { +virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("Password may not contain ',' character")); +return -1; +} + +virBufferAsprintf(&opt, ",password=%s", graphics->data.vnc.auth.passwd); +} else { +if (!(bhyveDriverGetBhyveCaps(driver) & BHYVE_CAP_VNC_PASSWORD)) +VIR_WARN("%s", _("Security warning: VNC auth is not supported.")); +else +VIR_WARN("%s", _("Security warning: VNC is used without authentication.")); +} + if (video->res) virBufferAsprintf(&opt, ",w=%d,h=%d", video->res->x, video->res->y); diff --git a/src/bhyve/bhyve_parse_command.c b/src/bhyve/bhyve_parse_command.c index c6abdfacf3..05cb8eb7d6 100644 --- a/src/bhyve/bhyve_parse_command.c +++ b/src/bhyve/bhyve_parse_command.c @@ -641,6 +641,11 @@ bhyveParsePCIFbuf(virDomainDefPtr def, if (virStrToLong_uip(param, NULL, 10, &video->res->y)) goto e
[PATCH v2 0/4] bhyve: framebuffer resolution and VNC password
Changes from v2: No functional changes, only rebase. Fabian Freyer (4): bhyve: support parsing fbuf PCI device bhyve: add support for setting fbuf resolution bhyve: probe for VNC password capability bhyve: add VNC password support NEWS.rst | 12 ++ docs/formatdomain.rst | 3 +- src/bhyve/bhyve_capabilities.c| 16 ++- src/bhyve/bhyve_capabilities.h| 1 + src/bhyve/bhyve_command.c | 36 -- src/bhyve/bhyve_parse_command.c | 116 +- src/libvirt_private.syms | 1 + .../bhyveargv2xml-vnc-listen.args | 10 ++ .../bhyveargv2xml-vnc-listen.xml | 22 .../bhyveargv2xml-vnc-password.args | 10 ++ .../bhyveargv2xml-vnc-password.xml| 22 .../bhyveargv2xml-vnc-resolution.args | 10 ++ .../bhyveargv2xml-vnc-resolution.xml | 24 .../bhyveargv2xml-vnc-vga-io.args | 10 ++ .../bhyveargv2xml-vnc-vga-io.xml | 22 .../bhyveargv2xml-vnc-vga-off.args| 10 ++ .../bhyveargv2xml-vnc-vga-off.xml | 23 .../bhyveargv2xml-vnc-vga-on.args | 10 ++ .../bhyveargv2xml-vnc-vga-on.xml | 23 .../bhyveargv2xmldata/bhyveargv2xml-vnc.args | 10 ++ tests/bhyveargv2xmldata/bhyveargv2xml-vnc.xml | 22 tests/bhyveargv2xmltest.c | 9 +- .../bhyvexml2argv-vnc-password-comma.xml | 26 .../bhyvexml2argv-vnc-password.args | 12 ++ .../bhyvexml2argv-vnc-password.ldargs | 1 + .../bhyvexml2argv-vnc-password.xml| 26 .../bhyvexml2argv-vnc-resolution.args | 10 ++ .../bhyvexml2argv-vnc-resolution.ldargs | 1 + .../bhyvexml2argv-vnc-resolution.xml | 20 +++ tests/bhyvexml2argvtest.c | 9 +- .../bhyvexml2xmlout-vnc-password.xml | 44 +++ .../bhyvexml2xmlout-vnc-resolution.xml| 31 + tests/bhyvexml2xmltest.c | 2 + 33 files changed, 588 insertions(+), 16 deletions(-) create mode 100644 tests/bhyveargv2xmldata/bhyveargv2xml-vnc-listen.args create mode 100644 tests/bhyveargv2xmldata/bhyveargv2xml-vnc-listen.xml create mode 100644 tests/bhyveargv2xmldata/bhyveargv2xml-vnc-password.args create mode 100644 tests/bhyveargv2xmldata/bhyveargv2xml-vnc-password.xml create mode 100644 tests/bhyveargv2xmldata/bhyveargv2xml-vnc-resolution.args create mode 100644 tests/bhyveargv2xmldata/bhyveargv2xml-vnc-resolution.xml create mode 100644 tests/bhyveargv2xmldata/bhyveargv2xml-vnc-vga-io.args create mode 100644 tests/bhyveargv2xmldata/bhyveargv2xml-vnc-vga-io.xml create mode 100644 tests/bhyveargv2xmldata/bhyveargv2xml-vnc-vga-off.args create mode 100644 tests/bhyveargv2xmldata/bhyveargv2xml-vnc-vga-off.xml create mode 100644 tests/bhyveargv2xmldata/bhyveargv2xml-vnc-vga-on.args create mode 100644 tests/bhyveargv2xmldata/bhyveargv2xml-vnc-vga-on.xml create mode 100644 tests/bhyveargv2xmldata/bhyveargv2xml-vnc.args create mode 100644 tests/bhyveargv2xmldata/bhyveargv2xml-vnc.xml create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-vnc-password-comma.xml create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-vnc-password.args create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-vnc-password.ldargs create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-vnc-password.xml create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-vnc-resolution.args create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-vnc-resolution.ldargs create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-vnc-resolution.xml create mode 100644 tests/bhyvexml2xmloutdata/bhyvexml2xmlout-vnc-password.xml create mode 100644 tests/bhyvexml2xmloutdata/bhyvexml2xmlout-vnc-resolution.xml -- 2.27.0
[PATCH v2 1/4] bhyve: support parsing fbuf PCI device
From: Fabian Freyer Add a new helper function, bhyveParsePCIFbuf, to parse the bhyve-argv parameters for a frame-buffer device to and definitions. For now, only the listen address, port, and vga mode are detected. Unsupported parameters are silently skipped. This involves upgrading the private API to expose the virDomainGraphicsDefNew helper function, which is used by bhyveParsePCIFbuf. Signed-off-by: Fabian Freyer Signed-off-by: Roman Bogorodskiy --- src/bhyve/bhyve_parse_command.c | 91 ++- src/libvirt_private.syms | 1 + .../bhyveargv2xml-vnc-listen.args | 10 ++ .../bhyveargv2xml-vnc-listen.xml | 22 + .../bhyveargv2xml-vnc-vga-io.args | 10 ++ .../bhyveargv2xml-vnc-vga-io.xml | 22 + .../bhyveargv2xml-vnc-vga-off.args| 10 ++ .../bhyveargv2xml-vnc-vga-off.xml | 23 + .../bhyveargv2xml-vnc-vga-on.args | 10 ++ .../bhyveargv2xml-vnc-vga-on.xml | 23 + .../bhyveargv2xmldata/bhyveargv2xml-vnc.args | 10 ++ tests/bhyveargv2xmldata/bhyveargv2xml-vnc.xml | 22 + tests/bhyveargv2xmltest.c | 5 + 13 files changed, 258 insertions(+), 1 deletion(-) create mode 100644 tests/bhyveargv2xmldata/bhyveargv2xml-vnc-listen.args create mode 100644 tests/bhyveargv2xmldata/bhyveargv2xml-vnc-listen.xml create mode 100644 tests/bhyveargv2xmldata/bhyveargv2xml-vnc-vga-io.args create mode 100644 tests/bhyveargv2xmldata/bhyveargv2xml-vnc-vga-io.xml create mode 100644 tests/bhyveargv2xmldata/bhyveargv2xml-vnc-vga-off.args create mode 100644 tests/bhyveargv2xmldata/bhyveargv2xml-vnc-vga-off.xml create mode 100644 tests/bhyveargv2xmldata/bhyveargv2xml-vnc-vga-on.args create mode 100644 tests/bhyveargv2xmldata/bhyveargv2xml-vnc-vga-on.xml create mode 100644 tests/bhyveargv2xmldata/bhyveargv2xml-vnc.args create mode 100644 tests/bhyveargv2xmldata/bhyveargv2xml-vnc.xml diff --git a/src/bhyve/bhyve_parse_command.c b/src/bhyve/bhyve_parse_command.c index 50a5e88408..388c565317 100644 --- a/src/bhyve/bhyve_parse_command.c +++ b/src/bhyve/bhyve_parse_command.c @@ -4,7 +4,7 @@ * Copyright (C) 2006-2016 Red Hat, Inc. * Copyright (C) 2006 Daniel P. Berrange * Copyright (c) 2011 NetApp, Inc. - * Copyright (C) 2016 Fabian Freyer + * Copyright (C) 2020 Fabian Freyer * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public @@ -553,6 +553,93 @@ bhyveParsePCINet(virDomainDefPtr def, return -1; } +static int +bhyveParsePCIFbuf(virDomainDefPtr def, + virDomainXMLOptionPtr xmlopt, + unsigned caps G_GNUC_UNUSED, + unsigned bus, + unsigned slot, + unsigned function, + const char *config) +{ +/* -s slot,fbuf,wait,vga=on|io|off,rfb=:port,w=width,h=height */ + +virDomainVideoDefPtr video = NULL; +virDomainGraphicsDefPtr graphics = NULL; +char **params = NULL; +char *param = NULL, *separator = NULL; +size_t nparams = 0; +unsigned int i = 0; + +if (!(video = virDomainVideoDefNew(xmlopt))) +goto cleanup; + +if (!(graphics = virDomainGraphicsDefNew(xmlopt))) +goto cleanup; + +graphics->type = VIR_DOMAIN_GRAPHICS_TYPE_VNC; +video->info.addr.pci.bus = bus; +video->info.addr.pci.slot = slot; +video->info.addr.pci.function = function; + +if (!config) +goto error; + +if (!(params = virStringSplitCount(config, ",", 0, &nparams))) +goto error; + +for (i = 0; i < nparams; i++) { +param = params[i]; +if (!video->driver && VIR_ALLOC(video->driver) < 0) +goto error; + +if (STREQ(param, "vga=on")) +video->driver->vgaconf = VIR_DOMAIN_VIDEO_VGACONF_ON; + +if (STREQ(param, "vga=io")) +video->driver->vgaconf = VIR_DOMAIN_VIDEO_VGACONF_IO; + +if (STREQ(param, "vga=off")) +video->driver->vgaconf = VIR_DOMAIN_VIDEO_VGACONF_OFF; + +if (STRPREFIX(param, "rfb=") || STRPREFIX(param, "tcp=")) { +/* fortunately, this is the same length as "tcp=" */ +param += strlen("rfb="); + +if (!(separator = strchr(param, ':'))) +goto error; + +*separator = '\0'; + +if (separator != param) +virDomainGraphicsListenAppendAddress(graphics, param); +else +/* Default to 127.0.0.1, just like bhyve does */ +virDomainGraphicsListenAppendAddress(graphics, "127.0.0.1"); + +param = ++separator; +if (virStrToLong_i(param, NULL, 10, &graphics->data.vnc.port)) +goto error; +
[PATCH v2 3/4] bhyve: probe for VNC password capability
From: Fabian Freyer Introduces the BHYVE_CAP_VNC_PASSWORD capability, which is probed by parsing the error message from the bhyve command. When it is not supported, bhyve -s 0,fbuf,password= will return an error message. Signed-off-by: Fabian Freyer Signed-off-by: Roman Bogorodskiy --- src/bhyve/bhyve_capabilities.c | 16 +++- src/bhyve/bhyve_capabilities.h | 1 + 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/src/bhyve/bhyve_capabilities.c b/src/bhyve/bhyve_capabilities.c index 36f3985335..523a31e287 100644 --- a/src/bhyve/bhyve_capabilities.c +++ b/src/bhyve/bhyve_capabilities.c @@ -3,7 +3,7 @@ * * Copyright (C) 2014 Roman Bogorodskiy * Copyright (C) 2014 Semihalf - * Copyright (C) 2016 Fabian Freyer + * Copyright (C) 2020 Fabian Freyer * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public @@ -334,6 +334,17 @@ bhyveProbeCapsSoundHda(unsigned int *caps, char *binary) } +static int +bhyveProbeCapsVNCPassword(unsigned int *caps, char *binary) +{ +return bhyveProbeCapsDeviceHelper(caps, binary, + "-s", + "0,fbuf,password=", + "Invalid fbuf emulation \"password\"", + BHYVE_CAP_VNC_PASSWORD); +} + + int virBhyveProbeCaps(unsigned int *caps) { @@ -365,6 +376,9 @@ virBhyveProbeCaps(unsigned int *caps) if ((ret = bhyveProbeCapsSoundHda(caps, binary))) goto out; +if ((ret = bhyveProbeCapsVNCPassword(caps, binary))) +goto out; + out: VIR_FREE(binary); return ret; diff --git a/src/bhyve/bhyve_capabilities.h b/src/bhyve/bhyve_capabilities.h index 1ac9ff4283..b2a16b0189 100644 --- a/src/bhyve/bhyve_capabilities.h +++ b/src/bhyve/bhyve_capabilities.h @@ -50,6 +50,7 @@ typedef enum { BHYVE_CAP_XHCI = 1 << 5, BHYVE_CAP_CPUTOPOLOGY = 1 << 6, BHYVE_CAP_SOUND_HDA = 1 << 7, +BHYVE_CAP_VNC_PASSWORD = 1 << 8, } virBhyveCapsFlags; int virBhyveProbeGrubCaps(virBhyveGrubCapsFlags *caps); -- 2.27.0
[PATCH] bhyve: add missing test files
Fixes: 4277e61e22b7532dc476c44a356081053da470f8 Signed-off-by: Roman Bogorodskiy --- Pushed as build breaker & trivial. ...yvexml2argv-addr-non-isa-controller-on-slot-1.args | 11 +++ ...exml2argv-addr-non-isa-controller-on-slot-1.ldargs | 1 + 2 files changed, 12 insertions(+) create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-addr-non-isa-controller-on-slot-1.args create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-addr-non-isa-controller-on-slot-1.ldargs diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-addr-non-isa-controller-on-slot-1.args b/tests/bhyvexml2argvdata/bhyvexml2argv-addr-non-isa-controller-on-slot-1.args new file mode 100644 index 00..cbbf768d71 --- /dev/null +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-addr-non-isa-controller-on-slot-1.args @@ -0,0 +1,11 @@ +/usr/sbin/bhyve \ +-c 1 \ +-m 214 \ +-u \ +-H \ +-P \ +-s 0:0,hostbridge \ +-l bootrom,/path/to/test.fd \ +-s 2:0,lpc \ +-s 3:0,ahci,hd:/tmp/freebsd.img \ +-s 1:0,virtio-net,faketapdev,mac=52:54:00:b9:94:02 bhyve diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-addr-non-isa-controller-on-slot-1.ldargs b/tests/bhyvexml2argvdata/bhyvexml2argv-addr-non-isa-controller-on-slot-1.ldargs new file mode 100644 index 00..421376db9e --- /dev/null +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-addr-non-isa-controller-on-slot-1.ldargs @@ -0,0 +1 @@ +dummy -- 2.27.0
Re: [PATCH v6 3/3] bhyve: soften requirements for slot 1
Daniel P. Berrangé wrote: > On Sun, Sep 20, 2020 at 07:21:15PM +0400, Roman Bogorodskiy wrote: > > Currently, slot 1 is only allowed to be used by the LPC device. > > Relax this requirement and allow to use slot 1 if it was explicitly > > specified by the user for any other device type. In this case the LPC > > device will have the next available address. > > > > If slot 1 was not used by the user, it'll be reserved for the LPC > > device, even if it is not configured to make address assignment > > consistent in case the LPC device becomes necessary (e.g. the user > > adds a console or a video device which require LPC). > > > > Signed-off-by: Roman Bogorodskiy > > --- > > po/POTFILES.in| 1 - > > src/bhyve/bhyve_device.c | 50 +-- > > ...argv-addr-non-isa-controller-on-slot-1.xml | 1 + > > tests/bhyvexml2argvtest.c | 2 +- > > 4 files changed, 25 insertions(+), 29 deletions(-) > > We're missing the bhyvexml2argv-addr-non-isa-controller-on-slot-1.args > file from the commit, so this breaks CI. I presume you forgot to commit > it from your local checkout. Indeed, I'll add that shortly. Sorry for the breakage. > > 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 :| > Roman Bogorodskiy signature.asc Description: PGP signature
[PATCH v6 2/3] bhyve: support 'isa' controller for LPC
Support modeling of the 'isa' controller for bhyve. User can manually define any PCI slot for the 'isa' controller, including PCI slot 1, but other devices are not allowed to use this address. When domain configuration requires the 'isa' controller to be present, automatically add it on domain post-parse stage. Now, as this controller is always available when needed, it's not necessary to implicitly add it to the bhyve command line, so remove bhyveBuildLPCArgStr(). Also, make bhyveDomainDefNeedsISAController() static as it's no longer used outside of bhyve_domain.c. As more than one ISA controller is not supported by bhyve, and multiple controllers with the same index are forbidden, so forbid ISA controllers with non-zero index for bhyve. Signed-off-by: Roman Bogorodskiy --- src/bhyve/bhyve_command.c | 27 +++--- src/bhyve/bhyve_device.c | 23 +--- src/bhyve/bhyve_domain.c | 25 +++-- src/bhyve/bhyve_domain.h | 2 -- ...ml2argv-addr-isa-controller-on-slot-1.args | 10 ++ ...2argv-addr-isa-controller-on-slot-1.ldargs | 3 ++ ...xml2argv-addr-isa-controller-on-slot-1.xml | 26 ++ ...l2argv-addr-isa-controller-on-slot-31.args | 10 ++ ...argv-addr-isa-controller-on-slot-31.ldargs | 3 ++ ...ml2argv-addr-isa-controller-on-slot-31.xml | 26 ++ ...argv-addr-non-isa-controller-on-slot-1.xml | 23 .../bhyvexml2argv-console.args| 2 +- .../bhyvexml2argv-isa-controller.args | 10 ++ .../bhyvexml2argv-isa-controller.ldargs | 3 ++ .../bhyvexml2argv-isa-controller.xml | 24 + ...bhyvexml2argv-isa-multiple-controllers.xml | 25 + .../bhyvexml2argv-serial-grub-nocons.args | 2 +- .../bhyvexml2argv-serial-grub.args| 2 +- .../bhyvexml2argv-serial.args | 2 +- .../bhyvexml2argvdata/bhyvexml2argv-uefi.args | 4 +-- .../bhyvexml2argv-vnc-autoport.args | 4 +-- .../bhyvexml2argv-vnc-vgaconf-io.args | 4 +-- .../bhyvexml2argv-vnc-vgaconf-off.args| 4 +-- .../bhyvexml2argv-vnc-vgaconf-on.args | 4 +-- .../bhyvexml2argvdata/bhyvexml2argv-vnc.args | 4 +-- tests/bhyvexml2argvtest.c | 5 +++ ...l2xmlout-addr-isa-controller-on-slot-1.xml | 36 +++ ...2xmlout-addr-isa-controller-on-slot-31.xml | 36 +++ .../bhyvexml2xmlout-console.xml | 3 ++ .../bhyvexml2xmlout-isa-controller.xml| 36 +++ .../bhyvexml2xmlout-serial-grub-nocons.xml| 3 ++ .../bhyvexml2xmlout-serial-grub.xml | 3 ++ .../bhyvexml2xmlout-serial.xml| 3 ++ .../bhyvexml2xmlout-vnc-autoport.xml | 3 ++ .../bhyvexml2xmlout-vnc-vgaconf-io.xml| 3 ++ .../bhyvexml2xmlout-vnc-vgaconf-off.xml | 3 ++ .../bhyvexml2xmlout-vnc-vgaconf-on.xml| 3 ++ .../bhyvexml2xmlout-vnc.xml | 3 ++ tests/bhyvexml2xmltest.c | 3 ++ 39 files changed, 378 insertions(+), 37 deletions(-) create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-1.args create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-1.ldargs create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-1.xml create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-31.args create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-31.ldargs create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-31.xml create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-addr-non-isa-controller-on-slot-1.xml create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-isa-controller.args create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-isa-controller.ldargs create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-isa-controller.xml create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-isa-multiple-controllers.xml create mode 100644 tests/bhyvexml2xmloutdata/bhyvexml2xmlout-addr-isa-controller-on-slot-1.xml create mode 100644 tests/bhyvexml2xmloutdata/bhyvexml2xmlout-addr-isa-controller-on-slot-31.xml create mode 100644 tests/bhyvexml2xmloutdata/bhyvexml2xmlout-isa-controller.xml diff --git a/src/bhyve/bhyve_command.c b/src/bhyve/bhyve_command.c index 5289e409fa..4df5baabdf 100644 --- a/src/bhyve/bhyve_command.c +++ b/src/bhyve/bhyve_command.c @@ -329,7 +329,8 @@ bhyveBuildControllerArgStr(const virDomainDef *def, virDomainControllerDefPtr controller, bhyveConnPtr driver, virCommandPtr cmd, - unsigned *nusbcontrollers) + unsigned *nusbcontrollers, + u
[PATCH v6 3/3] bhyve: soften requirements for slot 1
Currently, slot 1 is only allowed to be used by the LPC device. Relax this requirement and allow to use slot 1 if it was explicitly specified by the user for any other device type. In this case the LPC device will have the next available address. If slot 1 was not used by the user, it'll be reserved for the LPC device, even if it is not configured to make address assignment consistent in case the LPC device becomes necessary (e.g. the user adds a console or a video device which require LPC). Signed-off-by: Roman Bogorodskiy --- po/POTFILES.in| 1 - src/bhyve/bhyve_device.c | 50 +-- ...argv-addr-non-isa-controller-on-slot-1.xml | 1 + tests/bhyvexml2argvtest.c | 2 +- 4 files changed, 25 insertions(+), 29 deletions(-) diff --git a/po/POTFILES.in b/po/POTFILES.in index 54f78e7861..9782b2beb4 100644 --- a/po/POTFILES.in +++ b/po/POTFILES.in @@ -12,7 +12,6 @@ @SRCDIR@src/admin/libvirt-admin.c @SRCDIR@src/bhyve/bhyve_capabilities.c @SRCDIR@src/bhyve/bhyve_command.c -@SRCDIR@src/bhyve/bhyve_device.c @SRCDIR@src/bhyve/bhyve_domain.c @SRCDIR@src/bhyve/bhyve_driver.c @SRCDIR@src/bhyve/bhyve_monitor.c diff --git a/src/bhyve/bhyve_device.c b/src/bhyve/bhyve_device.c index 2295acf552..e2e1efd97e 100644 --- a/src/bhyve/bhyve_device.c +++ b/src/bhyve/bhyve_device.c @@ -42,21 +42,8 @@ bhyveCollectPCIAddress(virDomainDefPtr def G_GNUC_UNUSED, virDomainPCIAddressSetPtr addrs = opaque; virPCIDeviceAddressPtr addr = &info->addr.pci; -if (addr->domain == 0 && addr->bus == 0) { -if (addr->slot == 0) { +if (addr->domain == 0 && addr->bus == 0 && addr->slot == 0) { return 0; -} else if (addr->slot == 1) { -if (!(device->type == VIR_DOMAIN_DEVICE_CONTROLLER && - device->data.controller->type == VIR_DOMAIN_CONTROLLER_TYPE_ISA)) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", -_("PCI bus 0 slot 1 is reserved for the implicit " - "LPC PCI-ISA bridge")); - return -1; -} else { -/* We reserve slot 1 for LPC in bhyveAssignDevicePCISlots(), so exit early */ -return 0; -} -} } if (virDomainPCIAddressReserveAddr(addrs, addr, @@ -98,29 +85,38 @@ bhyveAssignDevicePCISlots(virDomainDefPtr def, size_t i; virPCIDeviceAddress lpc_addr; -/* explicitly reserve slot 1 for LPC-ISA bridge */ memset(&lpc_addr, 0, sizeof(lpc_addr)); lpc_addr.slot = 0x1; -if (virDomainPCIAddressReserveAddr(addrs, &lpc_addr, - VIR_PCI_CONNECT_TYPE_PCI_DEVICE, 0) < 0) { -return -1; -} +/* If the user didn't explicitly specify slot 1 for some of the devices, + reserve it for LPC, even if there's no LPC device configured. + If the slot 1 is used by some other device, LPC will have an address + auto-assigned. -for (i = 0; i < def->ncontrollers; i++) { - if ((def->controllers[i]->type == VIR_DOMAIN_CONTROLLER_TYPE_ISA) && - virDeviceInfoPCIAddressIsWanted(&def->controllers[i]->info)) { - def->controllers[i]->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI; - def->controllers[i]->info.addr.pci = lpc_addr; - break; - } + The idea behind that is to try to use slot 1 for the LPC device unless + user specifically configured otherwise.*/ +if (!virDomainPCIAddressSlotInUse(addrs, &lpc_addr)) { +if (virDomainPCIAddressReserveAddr(addrs, &lpc_addr, + VIR_PCI_CONNECT_TYPE_PCI_DEVICE, 0) < 0) { +return -1; +} + +for (i = 0; i < def->ncontrollers; i++) { + if ((def->controllers[i]->type == VIR_DOMAIN_CONTROLLER_TYPE_ISA) && + virDeviceInfoPCIAddressIsWanted(&def->controllers[i]->info)) { + def->controllers[i]->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI; + def->controllers[i]->info.addr.pci = lpc_addr; + break; + } +} } for (i = 0; i < def->ncontrollers; i++) { if ((def->controllers[i]->type == VIR_DOMAIN_CONTROLLER_TYPE_PCI) || (def->controllers[i]->type == VIR_DOMAIN_CONTROLLER_TYPE_SATA) || ((def->controllers[i]->type == VIR_DOMAIN_CONTROLLER_TYPE_USB) && - (def->controllers[i]->model == VIR_DOMAIN_CONTROLLER_MODEL_USB_NEC_XHCI))) { + (def->controllers[i]->model == VIR_DOMAIN_CONTROLLER_MODEL_USB_NEC_XHCI)) || +def-&g
[PATCH v6 0/3] bhyve: support 'isa' controller for LPC
Changes since v5: Added patch 3/3 to allow to use slot 1 for devices other than LPC. Technically, it can be squashed into the second patch, but this way it should be easier to review; I can squash it before merging, but I also it's probably self-contained enough to stay separated. No changes in 1-2/3 except rebasing. Roman Bogorodskiy (3): conf: add 'isa' controller type bhyve: support 'isa' controller for LPC bhyve: soften requirements for slot 1 docs/schemas/domaincommon.rng | 6 +++ po/POTFILES.in| 1 - src/bhyve/bhyve_command.c | 27 +++--- src/bhyve/bhyve_device.c | 37 --- src/bhyve/bhyve_domain.c | 25 - src/bhyve/bhyve_domain.h | 2 - src/conf/domain_conf.c| 9 + src/conf/domain_conf.h| 8 src/qemu/qemu_command.c | 1 + src/qemu/qemu_domain.c| 1 + src/qemu/qemu_domain_address.c| 1 + src/qemu/qemu_validate.c | 1 + src/vbox/vbox_common.c| 1 + ...ml2argv-addr-isa-controller-on-slot-1.args | 10 + ...2argv-addr-isa-controller-on-slot-1.ldargs | 3 ++ ...xml2argv-addr-isa-controller-on-slot-1.xml | 26 + ...l2argv-addr-isa-controller-on-slot-31.args | 10 + ...argv-addr-isa-controller-on-slot-31.ldargs | 3 ++ ...ml2argv-addr-isa-controller-on-slot-31.xml | 26 + ...argv-addr-non-isa-controller-on-slot-1.xml | 24 .../bhyvexml2argv-console.args| 2 +- .../bhyvexml2argv-isa-controller.args | 10 + .../bhyvexml2argv-isa-controller.ldargs | 3 ++ .../bhyvexml2argv-isa-controller.xml | 24 ...bhyvexml2argv-isa-multiple-controllers.xml | 25 + .../bhyvexml2argv-serial-grub-nocons.args | 2 +- .../bhyvexml2argv-serial-grub.args| 2 +- .../bhyvexml2argv-serial.args | 2 +- .../bhyvexml2argvdata/bhyvexml2argv-uefi.args | 4 +- .../bhyvexml2argv-vnc-autoport.args | 4 +- .../bhyvexml2argv-vnc-vgaconf-io.args | 4 +- .../bhyvexml2argv-vnc-vgaconf-off.args| 4 +- .../bhyvexml2argv-vnc-vgaconf-on.args | 4 +- .../bhyvexml2argvdata/bhyvexml2argv-vnc.args | 4 +- tests/bhyvexml2argvtest.c | 5 +++ ...l2xmlout-addr-isa-controller-on-slot-1.xml | 36 ++ ...2xmlout-addr-isa-controller-on-slot-31.xml | 36 ++ .../bhyvexml2xmlout-console.xml | 3 ++ .../bhyvexml2xmlout-isa-controller.xml| 36 ++ .../bhyvexml2xmlout-serial-grub-nocons.xml| 3 ++ .../bhyvexml2xmlout-serial-grub.xml | 3 ++ .../bhyvexml2xmlout-serial.xml| 3 ++ .../bhyvexml2xmlout-vnc-autoport.xml | 3 ++ .../bhyvexml2xmlout-vnc-vgaconf-io.xml| 3 ++ .../bhyvexml2xmlout-vnc-vgaconf-off.xml | 3 ++ .../bhyvexml2xmlout-vnc-vgaconf-on.xml| 3 ++ .../bhyvexml2xmlout-vnc.xml | 3 ++ tests/bhyvexml2xmltest.c | 3 ++ 48 files changed, 412 insertions(+), 47 deletions(-) create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-1.args create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-1.ldargs create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-1.xml create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-31.args create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-31.ldargs create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-31.xml create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-addr-non-isa-controller-on-slot-1.xml create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-isa-controller.args create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-isa-controller.ldargs create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-isa-controller.xml create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-isa-multiple-controllers.xml create mode 100644 tests/bhyvexml2xmloutdata/bhyvexml2xmlout-addr-isa-controller-on-slot-1.xml create mode 100644 tests/bhyvexml2xmloutdata/bhyvexml2xmlout-addr-isa-controller-on-slot-31.xml create mode 100644 tests/bhyvexml2xmloutdata/bhyvexml2xmlout-isa-controller.xml -- 2.27.0
[PATCH v6 1/3] conf: add 'isa' controller type
Introduce 'isa' controller type. In domain XML it looks this way: ... ... Currently, this is needed for the bhyve driver to allow choosing a specific PCI address for that. In bhyve, this controller is used to attach serial ports and a boot ROM. Signed-off-by: Roman Bogorodskiy --- docs/schemas/domaincommon.rng | 6 ++ src/conf/domain_conf.c | 9 + src/conf/domain_conf.h | 8 src/qemu/qemu_command.c| 1 + src/qemu/qemu_domain.c | 1 + src/qemu/qemu_domain_address.c | 1 + src/qemu/qemu_validate.c | 1 + src/vbox/vbox_common.c | 1 + 8 files changed, 28 insertions(+) diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index a1d6d19e2f..4b7e460148 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -2419,6 +2419,12 @@ + + + + isa + + diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 4d296f7bcb..9289c147fe 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -400,6 +400,7 @@ VIR_ENUM_IMPL(virDomainController, "usb", "pci", "xenbus", + "isa", ); VIR_ENUM_IMPL(virDomainControllerModelPCI, @@ -445,6 +446,9 @@ VIR_ENUM_IMPL(virDomainControllerModelSCSI, "virtio-non-transitional", ); +VIR_ENUM_IMPL(virDomainControllerModelISA, VIR_DOMAIN_CONTROLLER_MODEL_ISA_LAST, +); + VIR_ENUM_IMPL(virDomainControllerModelUSB, VIR_DOMAIN_CONTROLLER_MODEL_USB_LAST, "piix3-uhci", @@ -2337,6 +2341,7 @@ virDomainControllerDefNew(virDomainControllerType type) case VIR_DOMAIN_CONTROLLER_TYPE_SCSI: case VIR_DOMAIN_CONTROLLER_TYPE_SATA: case VIR_DOMAIN_CONTROLLER_TYPE_CCID: +case VIR_DOMAIN_CONTROLLER_TYPE_ISA: case VIR_DOMAIN_CONTROLLER_TYPE_LAST: break; } @@ -11047,6 +11052,8 @@ virDomainControllerModelTypeFromString(const virDomainControllerDef *def, return virDomainControllerModelIDETypeFromString(model); else if (def->type == VIR_DOMAIN_CONTROLLER_TYPE_VIRTIO_SERIAL) return virDomainControllerModelVirtioSerialTypeFromString(model); +else if (def->type == VIR_DOMAIN_CONTROLLER_TYPE_ISA) +return virDomainControllerModelISATypeFromString(model); return -1; } @@ -11066,6 +11073,8 @@ virDomainControllerModelTypeToString(virDomainControllerDefPtr def, return virDomainControllerModelIDETypeToString(model); else if (def->type == VIR_DOMAIN_CONTROLLER_TYPE_VIRTIO_SERIAL) return virDomainControllerModelVirtioSerialTypeToString(model); +else if (def->type == VIR_DOMAIN_CONTROLLER_TYPE_ISA) +return virDomainControllerModelISATypeToString(model); return NULL; } diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index cf76f340ee..4724206828 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -599,6 +599,7 @@ typedef enum { VIR_DOMAIN_CONTROLLER_TYPE_USB, VIR_DOMAIN_CONTROLLER_TYPE_PCI, VIR_DOMAIN_CONTROLLER_TYPE_XENBUS, +VIR_DOMAIN_CONTROLLER_TYPE_ISA, VIR_DOMAIN_CONTROLLER_TYPE_LAST } virDomainControllerType; @@ -690,6 +691,12 @@ typedef enum { VIR_DOMAIN_CONTROLLER_MODEL_VIRTIO_SERIAL_LAST } virDomainControllerModelVirtioSerial; +typedef enum { +VIR_DOMAIN_CONTROLLER_MODEL_ISA_DEFAULT = -1, + +VIR_DOMAIN_CONTROLLER_MODEL_ISA_LAST +} virDomainControllerModelISA; + #define IS_USB2_CONTROLLER(ctrl) \ (((ctrl)->type == VIR_DOMAIN_CONTROLLER_TYPE_USB) && \ ((ctrl)->model == VIR_DOMAIN_CONTROLLER_MODEL_USB_ICH9_EHCI1 || \ @@ -3597,6 +3604,7 @@ VIR_ENUM_DECL(virDomainControllerModelSCSI); VIR_ENUM_DECL(virDomainControllerModelUSB); VIR_ENUM_DECL(virDomainControllerModelIDE); VIR_ENUM_DECL(virDomainControllerModelVirtioSerial); +VIR_ENUM_DECL(virDomainControllerModelISA); VIR_ENUM_DECL(virDomainFS); VIR_ENUM_DECL(virDomainFSDriver); VIR_ENUM_DECL(virDomainFSAccessMode); diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 0ba348e911..91b59538aa 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -2681,6 +2681,7 @@ qemuBuildControllerDevStr(const virDomainDef *domainDef, case VIR_DOMAIN_CONTROLLER_TYPE_IDE: case VIR_DOMAIN_CONTROLLER_TYPE_FDC: case VIR_DOMAIN_CONTROLLER_TYPE_XENBUS: +case VIR_DOMAIN_CONTROLLER_TYPE_ISA: case VIR_DOMAIN_CONTROLLER_TYPE_LAST: virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("Unsupported controller type: %s"), diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index ac443c5ddc..a73a77f2e9 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -4930,6 +4930,7 @@
Re: [PATCH v5 2/2] bhyve: support 'isa' controller for LPC
Daniel P. Berrangé wrote: > On Thu, Sep 17, 2020 at 05:48:38PM +0400, Roman Bogorodskiy wrote: > > Daniel P. Berrangé wrote: > > > > > On Wed, Sep 16, 2020 at 07:14:39PM +0400, Roman Bogorodskiy wrote: > > > > Daniel P. Berrangé wrote: > > > > > > > > > On Sat, Aug 01, 2020 at 10:16:57AM +0400, Roman Bogorodskiy wrote: > > > > > > diff --git a/src/bhyve/bhyve_device.c b/src/bhyve/bhyve_device.c > > > > > > index fc52280361..52a055f205 100644 > > > > > > --- a/src/bhyve/bhyve_device.c > > > > > > +++ b/src/bhyve/bhyve_device.c > > > > > > @@ -46,10 +46,16 @@ bhyveCollectPCIAddress(virDomainDefPtr def > > > > > > G_GNUC_UNUSED, > > > > > > if (addr->slot == 0) { > > > > > > return 0; > > > > > > } else if (addr->slot == 1) { > > > > > > -virReportError(VIR_ERR_INTERNAL_ERROR, "%s", > > > > > > - _("PCI bus 0 slot 1 is reserved for the > > > > > > implicit " > > > > > > - "LPC PCI-ISA bridge")); > > > > > > -return -1; > > > > > > +if (!(device->type == VIR_DOMAIN_DEVICE_CONTROLLER && > > > > > > + device->data.controller->type == > > > > > > VIR_DOMAIN_CONTROLLER_TYPE_ISA)) { > > > > > > + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", > > > > > > +_("PCI bus 0 slot 1 is reserved > > > > > > for the implicit " > > > > > > + "LPC PCI-ISA bridge")); > > > > > > + return -1; > > > > > > +} else { > > > > > > +/* We reserve slot 1 for LPC in > > > > > > bhyveAssignDevicePCISlots(), so exit early */ > > > > > > +return 0; > > > > > > +} > > > > > > > > > > I forgot to respond to your followup comments on v4 > > > > > https://www.redhat.com/archives/libvir-list/2020-July/msg01761.html > > > > > > > > > > > > > > > > > > > IIUC, this series makes it possible to put the TPC in a different > > > > > > > slot, so does it still make sense to forbid use of slot 1 as a > > > > > > > hardcoded rule ? > > > > > > > > > > > > IIRC, the idea behind that is to give some time window for users to > > > > > > allow moving guests from the new version to the old one. If we > > > > > > allow to > > > > > > use slot 1, it won't be possible to move the guest to the old > > > > > > libvirt as > > > > > > it will complain slot 1 should be used only for LPC. > > > > > > > > > > If the user has decided to move their LPC to a slot != 1, then it is > > > > > already impossible to migrate the guest to an old libvirt. > > > > > > > > > > If the user wants to explicitly specify another device in slot 1, then > > > > > we should not prevent that. > > > > > > > > > > We just need to make sure that if no LPC is in the XML, and no other > > > > > device explicitly has slot 1, then make sure to auto-assign LPC in > > > > > slot 1 > > > > > not some other device. > > > > > > > > I've started playing with that and remembered some more corner cases. > > > > > > > > To elaborate on your example, i.e. "no explicit LPC in XML AND no other > > > > device explicitly on slot 1": these conditions are not specific enough > > > > to > > > > tell whether an LPC device will be added or not. > > > > > > > > In case if the LPC device was not explicitly specified by a user, > > > > the bhyve driver tries to add it if it's needed (it's required > > > > for serial console, bootloader, and video devices; > > > > see bhyveDomainDefNeedsISAController()). Otherwise a domain will start > > > > without the LPC device. > > > > > > > > This could lead to a case when a user starts a domain in configuration &
Re: [PATCH v5 2/2] bhyve: support 'isa' controller for LPC
Daniel P. Berrangé wrote: > On Wed, Sep 16, 2020 at 07:14:39PM +0400, Roman Bogorodskiy wrote: > > Daniel P. Berrangé wrote: > > > > > On Sat, Aug 01, 2020 at 10:16:57AM +0400, Roman Bogorodskiy wrote: > > > > Support modeling of the 'isa' controller for bhyve. User can manually > > > > define any PCI slot for the 'isa' controller, including PCI slot 1, > > > > but other devices are not allowed to use this address. > > > > > > > > When domain configuration requires the 'isa' controller to be present, > > > > automatically add it on domain post-parse stage. > > > > > > > > Now, as this controller is always available when needed, it's not > > > > necessary to implicitly add it to the bhyve command line, so remove > > > > bhyveBuildLPCArgStr(). > > > > > > > > Also, make bhyveDomainDefNeedsISAController() static as it's no longer > > > > used outside of bhyve_domain.c. > > > > > > > > As more than one ISA controller is not supported by bhyve, > > > > and multiple controllers with the same index are forbidden, > > > > so forbid ISA controllers with non-zero index for bhyve. > > > > > > > > Signed-off-by: Roman Bogorodskiy > > > > --- > > > > src/bhyve/bhyve_command.c | 27 +++--- > > > > src/bhyve/bhyve_device.c | 23 +--- > > > > src/bhyve/bhyve_domain.c | 25 +++-- > > > > src/bhyve/bhyve_domain.h | 2 -- > > > > ...ml2argv-addr-isa-controller-on-slot-1.args | 10 ++ > > > > ...2argv-addr-isa-controller-on-slot-1.ldargs | 3 ++ > > > > ...xml2argv-addr-isa-controller-on-slot-1.xml | 26 ++ > > > > ...l2argv-addr-isa-controller-on-slot-31.args | 10 ++ > > > > ...argv-addr-isa-controller-on-slot-31.ldargs | 3 ++ > > > > ...ml2argv-addr-isa-controller-on-slot-31.xml | 26 ++ > > > > ...argv-addr-non-isa-controller-on-slot-1.xml | 23 > > > > .../bhyvexml2argv-console.args| 2 +- > > > > .../bhyvexml2argv-isa-controller.args | 10 ++ > > > > .../bhyvexml2argv-isa-controller.ldargs | 3 ++ > > > > .../bhyvexml2argv-isa-controller.xml | 24 + > > > > ...bhyvexml2argv-isa-multiple-controllers.xml | 25 + > > > > .../bhyvexml2argv-serial-grub-nocons.args | 2 +- > > > > .../bhyvexml2argv-serial-grub.args| 2 +- > > > > .../bhyvexml2argv-serial.args | 2 +- > > > > .../bhyvexml2argvdata/bhyvexml2argv-uefi.args | 4 +-- > > > > .../bhyvexml2argv-vnc-autoport.args | 4 +-- > > > > .../bhyvexml2argv-vnc-vgaconf-io.args | 4 +-- > > > > .../bhyvexml2argv-vnc-vgaconf-off.args| 4 +-- > > > > .../bhyvexml2argv-vnc-vgaconf-on.args | 4 +-- > > > > .../bhyvexml2argvdata/bhyvexml2argv-vnc.args | 4 +-- > > > > tests/bhyvexml2argvtest.c | 5 +++ > > > > ...l2xmlout-addr-isa-controller-on-slot-1.xml | 36 +++ > > > > ...2xmlout-addr-isa-controller-on-slot-31.xml | 36 +++ > > > > .../bhyvexml2xmlout-console.xml | 3 ++ > > > > .../bhyvexml2xmlout-isa-controller.xml| 36 +++ > > > > .../bhyvexml2xmlout-serial-grub-nocons.xml| 3 ++ > > > > .../bhyvexml2xmlout-serial-grub.xml | 3 ++ > > > > .../bhyvexml2xmlout-serial.xml| 3 ++ > > > > .../bhyvexml2xmlout-vnc-autoport.xml | 3 ++ > > > > .../bhyvexml2xmlout-vnc-vgaconf-io.xml| 3 ++ > > > > .../bhyvexml2xmlout-vnc-vgaconf-off.xml | 3 ++ > > > > .../bhyvexml2xmlout-vnc-vgaconf-on.xml| 3 ++ > > > > .../bhyvexml2xmlout-vnc.xml | 3 ++ > > > > tests/bhyvexml2xmltest.c | 3 ++ > > > > 39 files changed, 378 insertions(+), 37 deletions(-) > > > > create mode 100644 > > > > tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-1.args > > > > create mode 100644 > > > > tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-1.ldargs > > > > create mode 100644 > > > > tests/bhyvexml2argvdata/bhyvexml2arg
Re: [PATCH v5 2/2] bhyve: support 'isa' controller for LPC
Daniel P. Berrangé wrote: > On Sat, Aug 01, 2020 at 10:16:57AM +0400, Roman Bogorodskiy wrote: > > Support modeling of the 'isa' controller for bhyve. User can manually > > define any PCI slot for the 'isa' controller, including PCI slot 1, > > but other devices are not allowed to use this address. > > > > When domain configuration requires the 'isa' controller to be present, > > automatically add it on domain post-parse stage. > > > > Now, as this controller is always available when needed, it's not > > necessary to implicitly add it to the bhyve command line, so remove > > bhyveBuildLPCArgStr(). > > > > Also, make bhyveDomainDefNeedsISAController() static as it's no longer > > used outside of bhyve_domain.c. > > > > As more than one ISA controller is not supported by bhyve, > > and multiple controllers with the same index are forbidden, > > so forbid ISA controllers with non-zero index for bhyve. > > > > Signed-off-by: Roman Bogorodskiy > > --- > > src/bhyve/bhyve_command.c | 27 +++--- > > src/bhyve/bhyve_device.c | 23 +--- > > src/bhyve/bhyve_domain.c | 25 +++-- > > src/bhyve/bhyve_domain.h | 2 -- > > ...ml2argv-addr-isa-controller-on-slot-1.args | 10 ++ > > ...2argv-addr-isa-controller-on-slot-1.ldargs | 3 ++ > > ...xml2argv-addr-isa-controller-on-slot-1.xml | 26 ++ > > ...l2argv-addr-isa-controller-on-slot-31.args | 10 ++ > > ...argv-addr-isa-controller-on-slot-31.ldargs | 3 ++ > > ...ml2argv-addr-isa-controller-on-slot-31.xml | 26 ++ > > ...argv-addr-non-isa-controller-on-slot-1.xml | 23 > > .../bhyvexml2argv-console.args| 2 +- > > .../bhyvexml2argv-isa-controller.args | 10 ++ > > .../bhyvexml2argv-isa-controller.ldargs | 3 ++ > > .../bhyvexml2argv-isa-controller.xml | 24 + > > ...bhyvexml2argv-isa-multiple-controllers.xml | 25 + > > .../bhyvexml2argv-serial-grub-nocons.args | 2 +- > > .../bhyvexml2argv-serial-grub.args| 2 +- > > .../bhyvexml2argv-serial.args | 2 +- > > .../bhyvexml2argvdata/bhyvexml2argv-uefi.args | 4 +-- > > .../bhyvexml2argv-vnc-autoport.args | 4 +-- > > .../bhyvexml2argv-vnc-vgaconf-io.args | 4 +-- > > .../bhyvexml2argv-vnc-vgaconf-off.args| 4 +-- > > .../bhyvexml2argv-vnc-vgaconf-on.args | 4 +-- > > .../bhyvexml2argvdata/bhyvexml2argv-vnc.args | 4 +-- > > tests/bhyvexml2argvtest.c | 5 +++ > > ...l2xmlout-addr-isa-controller-on-slot-1.xml | 36 +++ > > ...2xmlout-addr-isa-controller-on-slot-31.xml | 36 +++ > > .../bhyvexml2xmlout-console.xml | 3 ++ > > .../bhyvexml2xmlout-isa-controller.xml| 36 +++ > > .../bhyvexml2xmlout-serial-grub-nocons.xml| 3 ++ > > .../bhyvexml2xmlout-serial-grub.xml | 3 ++ > > .../bhyvexml2xmlout-serial.xml| 3 ++ > > .../bhyvexml2xmlout-vnc-autoport.xml | 3 ++ > > .../bhyvexml2xmlout-vnc-vgaconf-io.xml| 3 ++ > > .../bhyvexml2xmlout-vnc-vgaconf-off.xml | 3 ++ > > .../bhyvexml2xmlout-vnc-vgaconf-on.xml| 3 ++ > > .../bhyvexml2xmlout-vnc.xml | 3 ++ > > tests/bhyvexml2xmltest.c | 3 ++ > > 39 files changed, 378 insertions(+), 37 deletions(-) > > create mode 100644 > > tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-1.args > > create mode 100644 > > tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-1.ldargs > > create mode 100644 > > tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-1.xml > > create mode 100644 > > tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-31.args > > create mode 100644 > > tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-31.ldargs > > create mode 100644 > > tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-31.xml > > create mode 100644 > > tests/bhyvexml2argvdata/bhyvexml2argv-addr-non-isa-controller-on-slot-1.xml > > create mode 100644 > > tests/bhyvexml2argvdata/bhyvexml2argv-isa-controller.args > > create mode 100644 > > tests/bhyvexml2argvdata/bhyvexml2argv-isa-controller.ldargs > > create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-isa-controller.xml > > cr
Re: [PATCH v2] Fix linkage to libutil and libkvm on FreeBSD 11
Daniel P. Berrangé wrote: > We are currently adding -lutil and -lkvm to the linker using the > add_project_link_arguments method. On FreeBSD 11.4, this results in > build errors because the args appear too early in the command line. > > We need to pass the libraries as dependencies so that they get placed > at the same point in the linker args as other dependencies. > > Signed-off-by: Daniel P. Berrangé > --- > meson.build | 17 - > src/bhyve/meson.build | 2 ++ > src/util/meson.build | 2 ++ > 3 files changed, 12 insertions(+), 9 deletions(-) Reviewed-by: Roman Bogorodskiy Roman Bogorodskiy signature.asc Description: PGP signature
Re: [PATCH v5 0/2] bhyve: support 'isa' controller for LPC
Roman Bogorodskiy wrote: > Changes from v4: > > - Document 'isa' controller type in docs/formatdomain.html.in. >While here, add 'xenbus' which was also missing. > > Roman Bogorodskiy (2): > conf: add 'isa' controller type > bhyve: support 'isa' controller for LPC > > docs/formatdomain.html.in | 6 ++-- > docs/schemas/domaincommon.rng | 6 > src/bhyve/bhyve_command.c | 27 +++--- > src/bhyve/bhyve_device.c | 23 +--- > src/bhyve/bhyve_domain.c | 25 +++-- > src/bhyve/bhyve_domain.h | 2 -- > src/conf/domain_conf.c| 9 + > src/conf/domain_conf.h| 8 + > src/qemu/qemu_command.c | 1 + > src/qemu/qemu_domain.c| 1 + > src/qemu/qemu_domain_address.c| 1 + > src/qemu/qemu_validate.c | 1 + > src/vbox/vbox_common.c| 1 + > ...ml2argv-addr-isa-controller-on-slot-1.args | 10 ++ > ...2argv-addr-isa-controller-on-slot-1.ldargs | 3 ++ > ...xml2argv-addr-isa-controller-on-slot-1.xml | 26 ++ > ...l2argv-addr-isa-controller-on-slot-31.args | 10 ++ > ...argv-addr-isa-controller-on-slot-31.ldargs | 3 ++ > ...ml2argv-addr-isa-controller-on-slot-31.xml | 26 ++ > ...argv-addr-non-isa-controller-on-slot-1.xml | 23 > .../bhyvexml2argv-console.args| 2 +- > .../bhyvexml2argv-isa-controller.args | 10 ++ > .../bhyvexml2argv-isa-controller.ldargs | 3 ++ > .../bhyvexml2argv-isa-controller.xml | 24 + > ...bhyvexml2argv-isa-multiple-controllers.xml | 25 + > .../bhyvexml2argv-serial-grub-nocons.args | 2 +- > .../bhyvexml2argv-serial-grub.args| 2 +- > .../bhyvexml2argv-serial.args | 2 +- > .../bhyvexml2argvdata/bhyvexml2argv-uefi.args | 4 +-- > .../bhyvexml2argv-vnc-autoport.args | 4 +-- > .../bhyvexml2argv-vnc-vgaconf-io.args | 4 +-- > .../bhyvexml2argv-vnc-vgaconf-off.args| 4 +-- > .../bhyvexml2argv-vnc-vgaconf-on.args | 4 +-- > .../bhyvexml2argvdata/bhyvexml2argv-vnc.args | 4 +-- > tests/bhyvexml2argvtest.c | 5 +++ > ...l2xmlout-addr-isa-controller-on-slot-1.xml | 36 +++ > ...2xmlout-addr-isa-controller-on-slot-31.xml | 36 +++ > .../bhyvexml2xmlout-console.xml | 3 ++ > .../bhyvexml2xmlout-isa-controller.xml| 36 +++ > .../bhyvexml2xmlout-serial-grub-nocons.xml| 3 ++ > .../bhyvexml2xmlout-serial-grub.xml | 3 ++ > .../bhyvexml2xmlout-serial.xml| 3 ++ > .../bhyvexml2xmlout-vnc-autoport.xml | 3 ++ > .../bhyvexml2xmlout-vnc-vgaconf-io.xml| 3 ++ > .../bhyvexml2xmlout-vnc-vgaconf-off.xml | 3 ++ > .../bhyvexml2xmlout-vnc-vgaconf-on.xml| 3 ++ > .../bhyvexml2xmlout-vnc.xml | 3 ++ > tests/bhyvexml2xmltest.c | 3 ++ > 48 files changed, 409 insertions(+), 40 deletions(-) > create mode 100644 > tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-1.args > create mode 100644 > tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-1.ldargs > create mode 100644 > tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-1.xml > create mode 100644 > tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-31.args > create mode 100644 > tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-31.ldargs > create mode 100644 > tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-31.xml > create mode 100644 > tests/bhyvexml2argvdata/bhyvexml2argv-addr-non-isa-controller-on-slot-1.xml > create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-isa-controller.args > create mode 100644 > tests/bhyvexml2argvdata/bhyvexml2argv-isa-controller.ldargs > create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-isa-controller.xml > create mode 100644 > tests/bhyvexml2argvdata/bhyvexml2argv-isa-multiple-controllers.xml > create mode 100644 > tests/bhyvexml2xmloutdata/bhyvexml2xmlout-addr-isa-controller-on-slot-1.xml > create mode 100644 > tests/bhyvexml2xmloutdata/bhyvexml2xmlout-addr-isa-controller-on-slot-31.xml > create mode 100644 > tests/bhyvexml2xmloutdata/bhyvexml2xmlout-isa-controller.xml ping? Roman Bogorodskiy signature.asc Description: PGP signature
Re: [libvirt PATCH] remove HAL node device driver
Populate with known devices */ > -driver->privateData = hal_ctx; > - > -/* We need to unlock state now, since setting these callbacks cause > - * a dbus RPC call, and while this call is waiting for the reply, > - * a signal may already arrive, triggering the callback and thus > - * requiring the lock ! > - */ > -nodeDeviceUnlock(); > - > -/* Register HAL event callbacks */ > -if (!libhal_ctx_set_device_added(hal_ctx, device_added) || > -!libhal_ctx_set_device_removed(hal_ctx, device_removed) || > -!libhal_ctx_set_device_new_capability(hal_ctx, device_cap_added) || > -!libhal_ctx_set_device_lost_capability(hal_ctx, device_cap_lost) || > -!libhal_ctx_set_device_property_modified(hal_ctx, > device_prop_modified) || > -!libhal_device_property_watch_all(hal_ctx, &err)) { > -virReportError(VIR_ERR_INTERNAL_ERROR, "%s", > - _("setting up HAL callbacks failed")); > -goto failure; > -} > - > -udi = libhal_get_all_devices(hal_ctx, &num_devs, &err); > -if (udi == NULL) { > -virReportError(VIR_ERR_INTERNAL_ERROR, "%s", > - _("libhal_get_all_devices failed")); > -goto failure; > -} > -for (i = 0; i < num_devs; i++) { > -dev_create(udi[i]); > -VIR_FREE(udi[i]); > -} > -VIR_FREE(udi); > - > -nodeDeviceLock(); > -driver->initialized = true; > -nodeDeviceUnlock(); > -virCondBroadcast(&driver->initCond); > - > -return VIR_DRV_STATE_INIT_COMPLETE; > - > - failure: > -if (dbus_error_is_set(&err)) { > -virReportError(VIR_ERR_INTERNAL_ERROR, > - _("%s: %s"), err.name, err.message); > -dbus_error_free(&err); > -} > -virNodeDeviceObjListFree(driver->devs); > -if (hal_ctx) > -(void)libhal_ctx_free(hal_ctx); > -nodeDeviceUnlock(); > -VIR_FREE(driver); > - > -return ret; > -} > - > - > -static int > -nodeStateCleanup(void) > -{ > -if (driver) { > -nodeDeviceLock(); > -LibHalContext *hal_ctx = DRV_STATE_HAL_CTX(driver); > -virNodeDeviceObjListFree(driver->devs); > -(void)libhal_ctx_shutdown(hal_ctx, NULL); > -(void)libhal_ctx_free(hal_ctx); > -if (driver->lockFD != -1) > -virPidFileRelease(driver->stateDir, "driver", driver->lockFD); > - > -VIR_FREE(driver->stateDir); > -nodeDeviceUnlock(); > -virCondDestroy(&driver->initCond); > -virMutexDestroy(&driver->lock); > -VIR_FREE(driver); > -return 0; > -} > -return -1; > -} > - > - > -static int > -nodeStateReload(void) > -{ > -DBusError err; > -char **udi = NULL; > -int num_devs; > -size_t i; > -LibHalContext *hal_ctx; > - > -VIR_INFO("Reloading HAL device state"); > -nodeDeviceLock(); > -VIR_INFO("Removing existing objects"); > -virNodeDeviceObjListFree(driver->devs); > -nodeDeviceUnlock(); > - > -hal_ctx = DRV_STATE_HAL_CTX(driver); > -VIR_INFO("Creating new objects"); > -dbus_error_init(&err); > -udi = libhal_get_all_devices(hal_ctx, &num_devs, &err); > -if (udi == NULL) { > -virReportError(VIR_ERR_INTERNAL_ERROR, "%s", > - _("libhal_get_all_devices failed")); > -return -1; > -} > -for (i = 0; i < num_devs; i++) { > -dev_create(udi[i]); > -VIR_FREE(udi[i]); > -} > -VIR_FREE(udi); > -VIR_INFO("HAL device reload complete"); > - > -return 0; > -} > - > - > -static virNodeDeviceDriver halNodeDeviceDriver = { > -.name = "HAL", > -.nodeNumOfDevices = nodeNumOfDevices, /* 0.5.0 */ > -.nodeListDevices = nodeListDevices, /* 0.5.0 */ > -.connectListAllNodeDevices = nodeConnectListAllNodeDevices, /* 0.10.2 */ > -.nodeDeviceLookupByName = nodeDeviceLookupByName, /* 0.5.0 */ > -.nodeDeviceLookupSCSIHostByWWN = nodeDeviceLookupSCSIHostByWWN, /* 1.0.2 > */ > -.nodeDeviceGetXMLDesc = nodeDeviceGetXMLDesc, /* 0.5.0 */ > -.nodeDeviceGetParent = nodeDeviceGetParent, /* 0.5.0 */ > -.nodeDeviceNumOfCaps = nodeDeviceNumOfCaps, /* 0.5.0 */ > -.nodeDeviceListCaps = nodeDeviceListCaps, /* 0.5.0 */ > -.nodeDeviceCreateXML = nodeDeviceCreateXML, /* 0.6.5 */ > -.nodeDeviceDestroy = nodeDeviceDestroy, /* 0.6.5 */ > -}; > - > - > -static virHypervisorDriver halHypervisorDriver = { > -.name = "nodedev", > -.connectOpen = nodeConnectOpen, /* 4.1.0 */ > -.connectClose = nodeConnectClose, /* 4.1.0 */ > -.connectIsEncrypted = nodeConnectIsEncrypted, /* 4.1.0 */ > -.connectIsSecure = nodeConnectIsSecure, /* 4.1.0 */ > -.connectIsAlive = nodeConnectIsAlive, /* 4.1.0 */ > -}; > - > - > -static virConnectDriver halConnectDriver = { > -.localOnly = true, > -.uriSchemes = (const char *[]){ "nodedev", NULL }, > -.hypervisorDriver = &halHypervisorDriver, > -.nodeDeviceDriver = &halNodeDeviceDriver, > -}; > - > - > -static virStateDriver halStateDriver = { > -.name = "HAL", > -.stateInitialize = nodeStateInitialize, /* 0.5.0 */ > -.stateCleanup = nodeStateCleanup, /* 0.5.0 */ > -.stateReload = nodeStateReload, /* 0.5.0 */ > -}; > - > -int > -halNodeRegister(void) > -{ > -if (virRegisterConnectDriver(&halConnectDriver, false) < 0) > -return -1; > -if (virSetSharedNodeDeviceDriver(&halNodeDeviceDriver) < 0) > -return -1; > -return virRegisterStateDriver(&halStateDriver); > -} > diff --git a/src/node_device/node_device_hal.h > b/src/node_device/node_device_hal.h > deleted file mode 100644 > index 5e9c25ae34..00 > --- a/src/node_device/node_device_hal.h > +++ /dev/null > @@ -1,22 +0,0 @@ > -/* > - * node_device_hal.h: node device enumeration - HAL-based implementation > - * > - * Copyright (C) 2009 Red Hat, Inc. > - * > - * This library is free software; you can redistribute it and/or > - * modify it under the terms of the GNU Lesser General Public > - * License as published by the Free Software Foundation; either > - * version 2.1 of the License, or (at your option) any later version. > - * > - * This library is distributed in the hope that it will be useful, > - * but WITHOUT ANY WARRANTY; without even the implied warranty of > - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > - * Lesser General Public License for more details. > - * > - * You should have received a copy of the GNU Lesser General Public > - * License along with this library. If not, see > - * <http://www.gnu.org/licenses/>. > - * > - */ > - > -#pragma once > -- > 2.26.2 > Roman Bogorodskiy signature.asc Description: PGP signature
Re: [PATCH] Fix linkage to libutil and libkvm on FreeBSD 11
Daniel P. Berrangé wrote: > We are currently adding -lutil and -lkvm to the linker using the > add_project_link_arguments method. On FreeBSD 11.4, this results in > build errors because the args appear too early in the command line. > > We need to pass the libraries as dependencies so that they get placed > at the same point in the linker args as other dependencies. > > Signed-off-by: Daniel P. Berrangé > --- > meson.build | 16 +++- > src/bhyve/meson.build | 15 +++ > src/util/meson.build | 26 +- > 3 files changed, 35 insertions(+), 22 deletions(-) > > Using the CI patch I posted earlier to add FreeBSD 11: > > https://gitlab.com/berrange/libvirt/-/pipelines/185834799 > > > diff --git a/meson.build b/meson.build > index 1eadea33bf..c30ff187aa 100644 > --- a/meson.build > +++ b/meson.build > @@ -1086,7 +1086,8 @@ endif > # Check for BSD kvm (kernel memory interface) > if host_machine.system() == 'freebsd' >kvm_dep = cc.find_library('kvm') > - add_project_link_arguments('-lkvm', language: 'c') > +else > + kvm_dep = disabler() > endif > > libiscsi_version = '1.18.0' > @@ -1203,11 +1204,9 @@ have_gnu_gettext_tools = false > if not get_option('nls').disabled() >have_gettext = cc.has_function('gettext') >if not have_gettext > -intl_lib = cc.find_library('intl', required: false) > -have_gettext = intl_lib.found() > -if have_gettext > - add_project_link_arguments('-lintl', language: 'c') > -endif > +intl_dep = cc.find_library('intl', required: false) > + else > +intl_dep = disabler() >endif >if not have_gettext and get_option('nls').enabled() > error('gettext() is required to build libvirt') Looks like this error is issued even if we found intl_dep. Should this check be updated to: if not (have_gettext or intl_dep.found()) and get_option('nls').enabled() error(...) ? > @@ -1235,6 +1234,8 @@ if not get_option('nls').disabled() >have_gnu_gettext_tools = true > endif >endif > +else > + intl_dep = disabler() > endif > > numactl_dep = cc.find_library('numa', required: get_option('numactl')) > @@ -1402,9 +1403,6 @@ if udev_dep.found() > endif > > util_dep = cc.find_library('util', required: false) > -if util_dep.found() > - add_project_link_arguments('-lutil', language: 'c') > -endif > > if not get_option('virtualport').disabled() >if cc.has_header_symbol('linux/if_link.h', 'IFLA_PORT_MAX') Roman Bogorodskiy signature.asc Description: PGP signature
Re: [PATCH] tests: add FreeBSD dependencies
Daniel P. Berrangé wrote: > On Thu, Sep 03, 2020 at 01:49:11PM +0200, Pavel Hrdina wrote: > > On Thu, Sep 03, 2020 at 03:21:51PM +0400, Roman Bogorodskiy wrote: > > > Daniel P. Berrangé wrote: > > > > > > > On Thu, Sep 03, 2020 at 02:52:41PM +0400, Roman Bogorodskiy wrote: > > > > > Daniel P. Berrangé wrote: > > > > > > > > > > > On Thu, Sep 03, 2020 at 02:21:37PM +0400, Roman Bogorodskiy wrote: > > > > > > > Add some FreeBSD-specific libraries (-lutil, -lkvm) to tests > > > > > > > dependencies. > > > > > > > > > > > > > > Without that, FreeBSD 11.x, which uses the GNU ld, fails to link > > > > > > > tests. > > > > > > > Interestingly, newer FreeBSD versions that use LLVM ld tolerate > > > > > > > this > > > > > > > behaviour and builds successfully as is. > > > > > > > > > > > > Hmm, we need a CI job for FreeBSD 11 added > > > > > > > > > > > > Cirrus supports FreeBSD 11.4 so ought to be possible to add it to > > > > > > our > > > > > > matrix. > > > > > > > > > > > > > > > > > > > > Signed-off-by: Roman Bogorodskiy > > > > > > > --- > > > > > > > tests/meson.build | 2 ++ > > > > > > > 1 file changed, 2 insertions(+) > > > > > > > > > > > > > > diff --git a/tests/meson.build b/tests/meson.build > > > > > > > index ad13e2de60..ea2458efbc 100644 > > > > > > > --- a/tests/meson.build > > > > > > > +++ b/tests/meson.build > > > > > > > @@ -10,11 +10,13 @@ tests_dep = declare_dependency( > > > > > > > dlopen_dep, > > > > > > > glib_dep, > > > > > > > gnutls_dep, > > > > > > > +kvm_dep, > > > > > > > > > > > > Makes sense, as we don't reference kvm_dep anywhere. > > > > > > > > > > > > > libnl_dep, > > > > > > > libxml_dep, > > > > > > > rpc_dep, > > > > > > > sasl_dep, > > > > > > > selinux_dep, > > > > > > > +util_dep, > > > > > > > > > > > > In the top level meson.build, we appear to add -lutil as a linker > > > > > > arg to the entire project, so i'm surprised this was needed. > > > > > > > > > > -lutil was actually the first issue spotted, and when fixed, -lkvm > > > > > showed up. > > > > > > > > > > Here's the original report I got: > > > > > > > > > > https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=249056 > > > > > > > > > > It contains some initial thoughts on this issue; there I also assumed > > > > > that add_global_link_arguments() could fix the issue, but was > > > > > satisfied > > > > > with the current solution. > > > > > > > > Hmm, this is all very strange if I'm reading the the > > > > add_project_link_arguments doc correctly it should apply to everything > > > > we have. The difference between add_project and add_global is just > > > > about meson subprojects, but the tests are in our main project. > > > > > > > > If add_project_link_arguments isn't working as we expect, we might be > > > > better removing its use entirely in favour of explicitly adding the > > > > _deps objects in the particular places they are needed. > > > > > > I've just tried this change: > > > > > > --- meson.build.orig2020-09-03 11:14:20.233605000 + > > > +++ meson.build 2020-09-03 11:14:43.661313000 + > > > @@ -1405,7 +1405,7 @@ > > > > > > util_dep = cc.find_library('util', required: false) > > > if util_dep.found() > > > - add_project_link_arguments('-lutil', language: 'c') > > > + add_global_link_arguments('-lutil', language: 'c') > > > endif > > > > > > if not get_option('virtualport').disabled() > > > > > > It doesn't work, the build is still failing with it. > > >
Re: [PATCH] tests: add FreeBSD dependencies
Daniel P. Berrangé wrote: > On Thu, Sep 03, 2020 at 02:52:41PM +0400, Roman Bogorodskiy wrote: > > Daniel P. Berrangé wrote: > > > > > On Thu, Sep 03, 2020 at 02:21:37PM +0400, Roman Bogorodskiy wrote: > > > > Add some FreeBSD-specific libraries (-lutil, -lkvm) to tests > > > > dependencies. > > > > > > > > Without that, FreeBSD 11.x, which uses the GNU ld, fails to link tests. > > > > Interestingly, newer FreeBSD versions that use LLVM ld tolerate this > > > > behaviour and builds successfully as is. > > > > > > Hmm, we need a CI job for FreeBSD 11 added > > > > > > Cirrus supports FreeBSD 11.4 so ought to be possible to add it to our > > > matrix. > > > > > > > > > > > Signed-off-by: Roman Bogorodskiy > > > > --- > > > > tests/meson.build | 2 ++ > > > > 1 file changed, 2 insertions(+) > > > > > > > > diff --git a/tests/meson.build b/tests/meson.build > > > > index ad13e2de60..ea2458efbc 100644 > > > > --- a/tests/meson.build > > > > +++ b/tests/meson.build > > > > @@ -10,11 +10,13 @@ tests_dep = declare_dependency( > > > > dlopen_dep, > > > > glib_dep, > > > > gnutls_dep, > > > > +kvm_dep, > > > > > > Makes sense, as we don't reference kvm_dep anywhere. > > > > > > > libnl_dep, > > > > libxml_dep, > > > > rpc_dep, > > > > sasl_dep, > > > > selinux_dep, > > > > +util_dep, > > > > > > In the top level meson.build, we appear to add -lutil as a linker > > > arg to the entire project, so i'm surprised this was needed. > > > > -lutil was actually the first issue spotted, and when fixed, -lkvm > > showed up. > > > > Here's the original report I got: > > > > https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=249056 > > > > It contains some initial thoughts on this issue; there I also assumed > > that add_global_link_arguments() could fix the issue, but was satisfied > > with the current solution. > > Hmm, this is all very strange if I'm reading the the > add_project_link_arguments doc correctly it should apply to everything > we have. The difference between add_project and add_global is just > about meson subprojects, but the tests are in our main project. > > If add_project_link_arguments isn't working as we expect, we might be > better removing its use entirely in favour of explicitly adding the > _deps objects in the particular places they are needed. I've just tried this change: --- meson.build.orig2020-09-03 11:14:20.233605000 + +++ meson.build 2020-09-03 11:14:43.661313000 + @@ -1405,7 +1405,7 @@ util_dep = cc.find_library('util', required: false) if util_dep.found() - add_project_link_arguments('-lutil', language: 'c') + add_global_link_arguments('-lutil', language: 'c') endif if not get_option('virtualport').disabled() It doesn't work, the build is still failing with it. > > > 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 :| > Roman Bogorodskiy signature.asc Description: PGP signature
Re: [PATCH] tests: add FreeBSD dependencies
Daniel P. Berrangé wrote: > On Thu, Sep 03, 2020 at 02:21:37PM +0400, Roman Bogorodskiy wrote: > > Add some FreeBSD-specific libraries (-lutil, -lkvm) to tests dependencies. > > > > Without that, FreeBSD 11.x, which uses the GNU ld, fails to link tests. > > Interestingly, newer FreeBSD versions that use LLVM ld tolerate this > > behaviour and builds successfully as is. > > Hmm, we need a CI job for FreeBSD 11 added > > Cirrus supports FreeBSD 11.4 so ought to be possible to add it to our > matrix. > > > > > Signed-off-by: Roman Bogorodskiy > > --- > > tests/meson.build | 2 ++ > > 1 file changed, 2 insertions(+) > > > > diff --git a/tests/meson.build b/tests/meson.build > > index ad13e2de60..ea2458efbc 100644 > > --- a/tests/meson.build > > +++ b/tests/meson.build > > @@ -10,11 +10,13 @@ tests_dep = declare_dependency( > > dlopen_dep, > > glib_dep, > > gnutls_dep, > > +kvm_dep, > > Makes sense, as we don't reference kvm_dep anywhere. > > > libnl_dep, > > libxml_dep, > > rpc_dep, > > sasl_dep, > > selinux_dep, > > +util_dep, > > In the top level meson.build, we appear to add -lutil as a linker > arg to the entire project, so i'm surprised this was needed. -lutil was actually the first issue spotted, and when fixed, -lkvm showed up. Here's the original report I got: https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=249056 It contains some initial thoughts on this issue; there I also assumed that add_global_link_arguments() could fix the issue, but was satisfied with the current solution. > 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 :| > Roman Bogorodskiy signature.asc Description: PGP signature
[PATCH] tests: add FreeBSD dependencies
Add some FreeBSD-specific libraries (-lutil, -lkvm) to tests dependencies. Without that, FreeBSD 11.x, which uses the GNU ld, fails to link tests. Interestingly, newer FreeBSD versions that use LLVM ld tolerate this behaviour and builds successfully as is. Signed-off-by: Roman Bogorodskiy --- tests/meson.build | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/meson.build b/tests/meson.build index ad13e2de60..ea2458efbc 100644 --- a/tests/meson.build +++ b/tests/meson.build @@ -10,11 +10,13 @@ tests_dep = declare_dependency( dlopen_dep, glib_dep, gnutls_dep, +kvm_dep, libnl_dep, libxml_dep, rpc_dep, sasl_dep, selinux_dep, +util_dep, xdr_dep, yajl_dep, ], -- 2.27.0
Re: [PATCH] vsh: Define HAVE_STDARG_H before including readline
Pavel Hrdina wrote: > On Wed, Sep 02, 2020 at 05:47:43PM +0200, Michal Privoznik wrote: > > As it turned out my previous commits which switched from HAVE_ to > > WITH_ and dropped stdarg.h detection were a bit too aggressive. > > Because of reasons described in 9ea3424a178 we need to define > > HAVE_STDARG_H before including readline otherwise macos build > > fails. Honestly, I still don't fully understand the problem so I > > am not going to bother you with "explanation". > > > > Signed-off-by: Michal Privoznik > > --- > > tools/vsh.c | 4 > > 1 file changed, 4 insertions(+) > > > > diff --git a/tools/vsh.c b/tools/vsh.c > > index 11f493f969..d063a7f8cb 100644 > > --- a/tools/vsh.c > > +++ b/tools/vsh.c > > @@ -30,6 +30,10 @@ > > #include > > > > #if WITH_READLINE > > +/* In order to have proper rl_message declaration with older > > + * versions of readline, we have to declare this. See 9ea3424a178 > > + * for more info. */ > > +# define HAVE_STDARG_H > > # include > > # include > > #endif > > Is this enough to fix the issue? As we include readline.h in virsh.c and > virt-admin.c as well. > > Pavel It's not enough, at least to fix build on FreeBSD. Like you said, I had to make similar change in tools/virsh.c and tools/virt-admin.c to get a successful build. Roman Bogorodskiy signature.asc Description: PGP signature
Re: [PATCH] meson: Check for stdarg.h
Ján Tomko wrote: > On a Wednesday in 2020, Daniel P. Berrangé wrote: > >On Wed, Sep 02, 2020 at 02:07:54PM +0200, Michal Privoznik wrote: > >> As it turns out, one of my previous commits in which I removed > >> checking for stdarg.h was too aggressive. Long story short, the > >> readline public headers rely on stdarg.h and what is worse, they > >> expect us to declare the autotools style of macro (HAVE_STDARG_H) > >> if the header file exists. If we don't do it then compiling virsh > >> on macos fails. > >> > >> See 9ea3424a178 for more info. > > > >Ewww > > > >Deprecated in 2000, removed in 2013, then immediately readded to > >"fix" apps which still relied on K&R C with no function prototypes. > > > >The readline maintainer is more forgiving of ancient application code > >than I would be :-) 30 years since arrival of ANSI C is enough time > >to update code to use function prototypes, especially if you want to > >build against a readline library released in 2020, as opposed to the > >old version released years ago. > > > >> > >> Fixes: 85808b73846f93d656b4c81b6ebddd2dc3881bf6 > >> Signed-off-by: Michal Privoznik > >> --- > >> meson.build | 8 ++-- > >> 1 file changed, 6 insertions(+), 2 deletions(-) > >> > >> diff --git a/meson.build b/meson.build > >> index 1aad385ad1..98f7545495 100644 > >> --- a/meson.build > >> +++ b/meson.build > >> @@ -1333,8 +1333,12 @@ if readline_dep.found() > >> endif > >>endif > >> > >> - # We need this to avoid compilation issues with modern compilers. > >> - # See 9ea3424a178 for a more detailed explanation > >> + # We need both of these hacks to avoid compilation issues with modern > >> + # compilers. See 9ea3424a178 for a more detailed explanation. > >> + if cc.has_header('stdarg.h') > >> +conf.set('HAVE_STDARG_H', 1) > >> + endif > > > >Do we have any platforms which lack stdarg.h ? eg can be just add > >"#define HAVE_STDARG_H 1" unconditionally in the virsh code before > >it includes the readline headers ? > > > > It's part of the C99 standard: > https://pubs.opengroup.org/onlinepubs/009695399/basedefs/stdarg.h.html > > (but I'm still confused why this builds on other platforms even though > HAVE_STDARG_H is not defined anywhere) > > Jano > It fails to build on FreeBSD as well: In file included from ../tools/vsh.c:33: /usr/local/include/readline/readline.h:398:23: error: this function declaration is not a prototype [-Werror,-Wstrict-prototypes] extern int rl_message (); ^ void 1 error generated. Roman Bogorodskiy signature.asc Description: PGP signature
Re: [libvirt PATCH] util: virnetdevtap: stats: fix txdrop on FreeBSD
Ján Tomko wrote: > For older FreeBSD, we needed an ifdef guard to use > if_data.ifi_oqdrops, which was introduced by: > > commit 61bbdbb94ce3e2f5e969c9bddb443427db07bf61 > Implement interface stats for BSD > > But when we dropped the check because we deprecated > building on FreeBSD-10 in: > > commit 83131d9714db7ee77ab220186b6b0d8b6c22b09e > configure: drop check for unsupported FreeBSD > > We started building the wrong side of the ifdef. > > Signed-off-by: Ján Tomko > Fixes: 83131d9714db7ee77ab220186b6b0d8b6c22b09e Reviewed-by: Roman Bogorodskiy Roman Bogorodskiy signature.asc Description: PGP signature
Re: [PATCH] build-aux: use GNU sed for syntax-check on FreeBSD
Roman Bogorodskiy wrote: > Roman Bogorodskiy wrote: > > > BSD sed(1) and GNU sed(1) syntax are not compatible, and as > > syntax-check.mk uses the GNU flavor, set SED variable to > > 'gsed' by default. > > > > Signed-off-by: Roman Bogorodskiy > > --- > > build-aux/syntax-check.mk | 8 > > 1 file changed, 8 insertions(+) > > > > ping ping Roman Bogorodskiy signature.asc Description: PGP signature
Re: [PATCH] nss: Finish renaming of HAVE_BSD_NSS macro
Michal Privoznik wrote: > When switching to meson, some of HAVE_* macros were renamed to > WITH_ because they did not reflect whether the build platform has > or doesn't have something, but whether we are building with some > functionality turned on or off. This is the case with > HAVE_BSD_NSS macro too. As a result, the NSS plugin built on BSD > did not expose nss_module_register() function which made the > plugin unusable: > > https://www.redhat.com/archives/libvir-list/2020-September/msg0.html > > Fixes: c74268705557a6781788ba011492c15df2e3df11 > Reported-by: Roman Bogorodskiy > Signed-off-by: Michal Privoznik Reviewed-by: Roman Bogorodskiy > --- > tools/nss/libvirt_nss.c | 6 +++--- > tools/nss/libvirt_nss.h | 4 ++-- > 2 files changed, 5 insertions(+), 5 deletions(-) > > diff --git a/tools/nss/libvirt_nss.c b/tools/nss/libvirt_nss.c > index 3b89f72742..6331c65131 100644 > --- a/tools/nss/libvirt_nss.c > +++ b/tools/nss/libvirt_nss.c > @@ -37,7 +37,7 @@ > #include > > > -#if defined(HAVE_BSD_NSS) > +#if defined(WITH_BSD_NSS) > # include > #endif > > @@ -451,7 +451,7 @@ NSS_NAME(gethostbyname4)(const char *name, struct > gaih_addrtuple **pat, > } > #endif /* HAVE_STRUCT_GAIH_ADDRTUPLE */ > > -#if defined(HAVE_BSD_NSS) > +#if defined(WITH_BSD_NSS) > NSS_METHOD_PROTOTYPE(_nss_compat_getaddrinfo); > NSS_METHOD_PROTOTYPE(_nss_compat_gethostbyname2_r); > > @@ -598,4 +598,4 @@ nss_module_register(const char *name > __attribute__((unused)), > *unregister = NULL; > return methods; > } > -#endif /* HAVE_BSD_NSS */ > +#endif /* WITH_BSD_NSS */ > diff --git a/tools/nss/libvirt_nss.h b/tools/nss/libvirt_nss.h > index 95ebafdc71..121b9e8722 100644 > --- a/tools/nss/libvirt_nss.h > +++ b/tools/nss/libvirt_nss.h > @@ -84,8 +84,8 @@ NSS_NAME(gethostbyname4)(const char *name, struct > gaih_addrtuple **pat, > int *herrnop, int32_t *ttlp); > #endif /* HAVE_STRUCT_GAIH_ADDRTUPLE */ > > -#if defined(HAVE_BSD_NSS) > +#if defined(WITH_BSD_NSS) > ns_mtab* > nss_module_register(const char *name, unsigned int *size, > nss_module_unregister_fn *unregister); > -#endif /* HAVE_BSD_NSS */ > +#endif /* WITH_BSD_NSS */ > -- > 2.26.2 > Roman Bogorodskiy signature.asc Description: PGP signature
Re: Symbol missing in nss_libvirt.so
Michal Privoznik wrote: > On 9/1/20 5:20 AM, Roman Bogorodskiy wrote: > > WITH_BSD_NSS value is properly set: > > > > $ grep WITH_BSD_NSS build/meson-config.h > > #define WITH_BSD_NSS 1 > > $ > > Ah, I think this is the problem. In libvirt_nss.c we check for > HAVE_BSD_NSS. Can you please check if something like this fixes the problem: > Oh, I've overlooked WITH vs HAVE. Yes, the suggested patch fixes the problem, thanks. > > diff --git i/tools/nss/libvirt_nss.c w/tools/nss/libvirt_nss.c > index 3b89f72742..6331c65131 100644 > --- i/tools/nss/libvirt_nss.c > +++ w/tools/nss/libvirt_nss.c > @@ -37,7 +37,7 @@ > #include > > > -#if defined(HAVE_BSD_NSS) > +#if defined(WITH_BSD_NSS) > # include > #endif > > @@ -451,7 +451,7 @@ NSS_NAME(gethostbyname4)(const char *name, struct > gaih_addrtuple **pat, > } > #endif /* HAVE_STRUCT_GAIH_ADDRTUPLE */ > > -#if defined(HAVE_BSD_NSS) > +#if defined(WITH_BSD_NSS) > NSS_METHOD_PROTOTYPE(_nss_compat_getaddrinfo); > NSS_METHOD_PROTOTYPE(_nss_compat_gethostbyname2_r); > > @@ -598,4 +598,4 @@ nss_module_register(const char *name > __attribute__((unused)), > *unregister = NULL; > return methods; > } > -#endif /* HAVE_BSD_NSS */ > +#endif /* WITH_BSD_NSS */ > diff --git i/tools/nss/libvirt_nss.h w/tools/nss/libvirt_nss.h > index 95ebafdc71..121b9e8722 100644 > --- i/tools/nss/libvirt_nss.h > +++ w/tools/nss/libvirt_nss.h > @@ -84,8 +84,8 @@ NSS_NAME(gethostbyname4)(const char *name, struct > gaih_addrtuple **pat, >int *herrnop, int32_t *ttlp); > #endif /* HAVE_STRUCT_GAIH_ADDRTUPLE */ > > -#if defined(HAVE_BSD_NSS) > +#if defined(WITH_BSD_NSS) > ns_mtab* > nss_module_register(const char *name, unsigned int *size, > nss_module_unregister_fn *unregister); > -#endif /* HAVE_BSD_NSS */ > +#endif /* WITH_BSD_NSS */ > > > Michal > Roman Bogorodskiy signature.asc Description: PGP signature
Symbol missing in nss_libvirt.so
S=64 -Wall -Winvalid-pch -Wextra -std=gnu99 -O2 -g -Werror -fno-common -W -fexceptions -fasynchronous-unwind-tables -fstack-protector-strong -Wdouble-promotion -Wno-unused-function -fPIC -pthread -DLIBVIRT_NSS -MD -MQ 'tools/nss/96b7fa1@@nss_libvirt_impl@sta/libvirt_nss.c.o' -MF 'tools/nss/96b7fa1@@nss_libvirt_impl@sta/libvirt_nss.c.o.d' -o 'tools/nss/96b7fa1@@nss_libvirt_impl@sta/libvirt_nss.c.o' -c ../tools/nss/libvirt_nss.c [540/1087] cc -o tools/nss/nss_libvirt.so.1 -Wl,--as-needed -Wl,--allow-shlib-undefined -shared -fPIC -Wl,--start-group -Wl,-soname,nss_libvirt.so.1 -Wl,--whole-archive tools/nss/libnss_libvirt_impl.a -Wl,--no-whole-archive -lkvm -lutil -Wl,--version-script=/usr/home/novel/code/libvirt/tools/nss/libvirt_nss_bsd.syms -Wl,-export-dynamic -Wl,-z,relro -Wl,-z,now -Wl,--no-copy-dt-needed-entries -Wl,-z,defs /usr/local/lib/libxml2.so /usr/local/lib/libglib-2.0.so /usr/local/lib/libintl.so /usr/local/lib/libgobject-2.0.so /usr/local/lib/libgio-2.0.so /usr/local/lib/libyajl.so -Wl,--end-group [541/1087] cc -o tools/virt-admin 'tools/f9d35d4@@virt-admin@exe/virt-admin.c.o' 'tools/f9d35d4@@virt-admin@exe/virt-admin-completer.c.o' -Wl,--as-needed -Wl,--no-undefined -pie -Wl,--start-group -lkvm -lutil src/libvirt-admin.so.0.6007.0 tools/libvirt_shell.a src/libvirt.so.0.6007.0 -Wl,-z,relro -Wl,-z,now -Wl,--no-copy-dt-needed-entries -Wl,-z,defs /usr/local/lib/libxml2.so /usr/local/lib/libglib-2.0.so /usr/local/lib/libintl.so /usr/local/lib/libgobject-2.0.so /usr/local/lib/libgio-2.0.so /usr/local/lib/libreadline.so -Wl,--end-group -Wl,-z,relro -Wl,-z,now -Wl,--no-copy-dt-needed-entries -Wl,-z,defs '-Wl,-rpath,$ORIGIN/../src:$ORIGIN/' -Wl,-rpath-link,/usr/home/novel/code/libvirt/build/src -Wl,-rpath-link,/usr/home/novel/code/libvirt/build/tools [542/1087] cc -o tools/nss/nss_libvirt_guest.so.1 -Wl,--as-needed -Wl,--no-undefined -shared -fPIC -Wl,--start-group -Wl,-soname,nss_libvirt_guest.so.1 -Wl,--whole-archive tools/nss/libnss_libvirt_guest_impl.a -Wl,--no-whole-archive -lkvm -lutil -Wl,--version-script=/usr/home/novel/code/libvirt/tools/nss/libvirt_nss_bsd.syms -Wl,-export-dynamic -Wl,-z,relro -Wl,-z,now -Wl,--no-copy-dt-needed-entries -Wl,-z,defs /usr/local/lib/libxml2.so /usr/local/lib/libglib-2.0.so /usr/local/lib/libintl.so /usr/local/lib/libgobject-2.0.so /usr/local/lib/libgio-2.0.so /usr/local/lib/libyajl.so -Wl,--end-group Any pointers how to debug that? Roman Bogorodskiy signature.asc Description: PGP signature
[PATCH] news: mention bhyve sound support
Signed-off-by: Roman Bogorodskiy --- NEWS.rst | 5 + 1 file changed, 5 insertions(+) diff --git a/NEWS.rst b/NEWS.rst index 2d30d5a5e8..9ded7731f8 100644 --- a/NEWS.rst +++ b/NEWS.rst @@ -25,6 +25,11 @@ v6.7.0 (unreleased) the ``device_model_args`` setting in xl.cfg(5). The libvirt xen driver now supports this using XML extensions. + * bhyve: Sound device support + +This feature allows to configure guest sound device using +the element, and map it to the host sound device using +the element. * **Improvements** -- 2.27.0
[PATCH] meson: don't install sysconf files unconditionally
There's no need to install sysconf files when init script installation was not requested, i.e. when configured with init_script=none. Signed-off-by: Roman Bogorodskiy --- src/meson.build | 16 +--- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/src/meson.build b/src/meson.build index d058cec2e4..5d8deaf548 100644 --- a/src/meson.build +++ b/src/meson.build @@ -858,13 +858,15 @@ if conf.has('WITH_LIBVIRTD') endif endif -foreach sysconf : sysconf_files - install_data( -sysconf['file'], -install_dir: sysconfdir / 'sysconfig', -rename: [ sysconf['name'] ], - ) -endforeach +if init_script != 'none' + foreach sysconf : sysconf_files +install_data( + sysconf['file'], + install_dir: sysconfdir / 'sysconfig', + rename: [ sysconf['name'] ], +) + endforeach +endif if conf.has('WITH_DTRACE_PROBES') custom_target( -- 2.27.0
Re: [PATCH] build-aux: use GNU sed for syntax-check on FreeBSD
Roman Bogorodskiy wrote: > BSD sed(1) and GNU sed(1) syntax are not compatible, and as > syntax-check.mk uses the GNU flavor, set SED variable to > 'gsed' by default. > > Signed-off-by: Roman Bogorodskiy > --- > build-aux/syntax-check.mk | 8 > 1 file changed, 8 insertions(+) > ping Roman Bogorodskiy signature.asc Description: PGP signature
[PATCH v4 2/5] bhyve: implement sound device support
bhyve supports intel hda sound devices that could be specified on the command like using "-1:0,hda,play=$play_dev,rec=$rec_dev", where "1:0" is a PCI address, and "$play_dev" and "$rec_dev" point to the playback and recording device on the host respectively. Currently, schema of the 'sound' element doesn't allow specifying neither playback nor recording devices, so for now hardcode /dev/dsp0, which is the first audio device on the host. Signed-off-by: Roman Bogorodskiy --- src/bhyve/bhyve_capabilities.c| 14 src/bhyve/bhyve_capabilities.h| 1 + src/bhyve/bhyve_command.c | 33 + src/bhyve/bhyve_device.c | 9 + .../bhyvexml2argv-sound.args | 10 ++ .../bhyvexml2argv-sound.ldargs| 3 ++ .../bhyvexml2argvdata/bhyvexml2argv-sound.xml | 24 + tests/bhyvexml2argvtest.c | 6 +++- .../bhyvexml2xmlout-sound.xml | 36 +++ tests/bhyvexml2xmltest.c | 1 + 10 files changed, 136 insertions(+), 1 deletion(-) create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-sound.args create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-sound.ldargs create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-sound.xml create mode 100644 tests/bhyvexml2xmloutdata/bhyvexml2xmlout-sound.xml diff --git a/src/bhyve/bhyve_capabilities.c b/src/bhyve/bhyve_capabilities.c index fb8829d571..36f3985335 100644 --- a/src/bhyve/bhyve_capabilities.c +++ b/src/bhyve/bhyve_capabilities.c @@ -323,6 +323,17 @@ bhyveProbeCapsXHCIController(unsigned int *caps, char *binary) } +static int +bhyveProbeCapsSoundHda(unsigned int *caps, char *binary) +{ +return bhyveProbeCapsDeviceHelper(caps, binary, + "-s", + "0,hda", + "pci slot 0:0: unknown device \"hda\"", + BHYVE_CAP_SOUND_HDA); +} + + int virBhyveProbeCaps(unsigned int *caps) { @@ -351,6 +362,9 @@ virBhyveProbeCaps(unsigned int *caps) if ((ret = bhyveProbeCapsXHCIController(caps, binary))) goto out; +if ((ret = bhyveProbeCapsSoundHda(caps, binary))) +goto out; + out: VIR_FREE(binary); return ret; diff --git a/src/bhyve/bhyve_capabilities.h b/src/bhyve/bhyve_capabilities.h index 12926cf423..1ac9ff4283 100644 --- a/src/bhyve/bhyve_capabilities.h +++ b/src/bhyve/bhyve_capabilities.h @@ -49,6 +49,7 @@ typedef enum { BHYVE_CAP_FBUF = 1 << 4, BHYVE_CAP_XHCI = 1 << 5, BHYVE_CAP_CPUTOPOLOGY = 1 << 6, +BHYVE_CAP_SOUND_HDA = 1 << 7, } virBhyveCapsFlags; int virBhyveProbeGrubCaps(virBhyveGrubCapsFlags *caps); diff --git a/src/bhyve/bhyve_command.c b/src/bhyve/bhyve_command.c index 86e6640359..1f42f71347 100644 --- a/src/bhyve/bhyve_command.c +++ b/src/bhyve/bhyve_command.c @@ -475,6 +475,34 @@ bhyveBuildGraphicsArgStr(const virDomainDef *def, } +static int +bhyveBuildSoundArgStr(const virDomainDef *def G_GNUC_UNUSED, + virDomainSoundDefPtr sound, + bhyveConnPtr driver, + virCommandPtr cmd) +{ +if (!(bhyveDriverGetBhyveCaps(driver) & BHYVE_CAP_SOUND_HDA)) { +/* Currently, bhyve only supports "hda" sound devices, so + if it's not supported, sound devices are not supported at all */ +virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("Sound devices emulation is not supported " + "by given bhyve binary")); +return -1; +} + +if (sound->model != VIR_DOMAIN_SOUND_MODEL_ICH7) { +virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("Sound device model is not supported")); +return -1; +} + +virCommandAddArg(cmd, "-s"); +virCommandAddArgFormat(cmd, "%d:%d,hda,play=/dev/dsp0", + sound->info.addr.pci.slot, + sound->info.addr.pci.function); +return 0; +} + virCommandPtr virBhyveProcessBuildBhyveCmd(bhyveConnPtr driver, virDomainDefPtr def, bool dryRun) @@ -619,6 +647,11 @@ virBhyveProcessBuildBhyveCmd(bhyveConnPtr driver, virDomainDefPtr def, } } +for (i = 0; i < def->nsounds; i++) { +if (bhyveBuildSoundArgStr(def, def->sounds[i], driver, cmd) < 0) +goto error; +} + if (bhyveDomainDefNeedsISAController(def)) bhyveBuildLPCArgStr(def, cmd); diff --git a/src/bhyve/bhyve_device.c b/src/bhyve/bhyve_device.c index fc52280361..3c8ff587e0 100644 --- a/src/bhyve/bhyve_device.c +++ b/src/bh
[PATCH v4 5/5] docs: formatdomain: document element
Document the new element which allows to specify host audio backend for a guest device, and update the element description with the new sub-element which specifies the other end of the mapping. Signed-off-by: Roman Bogorodskiy --- docs/formatdomain.rst | 49 +++ 1 file changed, 49 insertions(+) diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst index 8365fc8bbb..fae138c2dd 100644 --- a/docs/formatdomain.rst +++ b/docs/formatdomain.rst @@ -6572,6 +6572,55 @@ Valid values are: Each ``sound`` element has an optional sub-element which can tie the device to a particular PCI slot, `documented above <#elementsAddress>`__. +:since:`Since 6.7.0`, a sound device could be optionally mapped to the specific +host audio backend using the sub-element: + +:: + + ... + + + + + + ... + +Where ``1`` is an id of the `audio device <#elementsAudio>`__. +This is supported for bhyve only. + +:anchor:`` + +Audio devices +~ + +A virtual audio device corresponds to a host audio backend that is mapped +to the guest sound device. :since:`Since 6.7.0, bhyve only` + +``type`` + The required ``type`` attribute specifies audio backend type. + Currently, the only supported value is 'oss'. + +``id`` + Integer id of the audio device. Must be greater than 0. + +The 'oss' audio type supports additional configuration: + +:: + + ... + + + + + + + +``input`` + Input device. The required ``dev`` attribute specifies device path. + +``output`` + Output device. The required ``dev`` attribute specifies device path. + :anchor:`` Watchdog device -- 2.27.0
[PATCH v4 1/5] conf: add 'ich7' sound model
Add 'ich7' sound model. This is a preparation for sound support in bhyve, as 'ich7' is the only model it supports. Signed-off-by: Roman Bogorodskiy --- docs/schemas/domaincommon.rng | 1 + src/conf/domain_conf.c | 1 + src/conf/domain_conf.h | 1 + src/qemu/qemu_command.c| 1 + src/qemu/qemu_domain_address.c | 1 + src/qemu/qemu_validate.c | 1 + 6 files changed, 6 insertions(+) diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 0d0dcbc5ce..fb9638f3f6 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -4360,6 +4360,7 @@ pcspk ac97 ich6 + ich7 ich9 usb diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 8e7981bf25..8d0d342504 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -726,6 +726,7 @@ VIR_ENUM_IMPL(virDomainSoundModel, "ich6", "ich9", "usb", + "ich7", ); VIR_ENUM_IMPL(virDomainKeyWrapCipherName, diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 68be32614c..7b60c28c6d 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1401,6 +1401,7 @@ typedef enum { VIR_DOMAIN_SOUND_MODEL_ICH6, VIR_DOMAIN_SOUND_MODEL_ICH9, VIR_DOMAIN_SOUND_MODEL_USB, +VIR_DOMAIN_SOUND_MODEL_ICH7, VIR_DOMAIN_SOUND_MODEL_LAST } virDomainSoundModel; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 01812cd39b..ec3d4c8d99 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -4026,6 +4026,7 @@ qemuBuildSoundDevStr(const virDomainDef *def, model = "sb16"; break; case VIR_DOMAIN_SOUND_MODEL_PCSPK: /* pc-speaker is handled separately */ +case VIR_DOMAIN_SOUND_MODEL_ICH7: case VIR_DOMAIN_SOUND_MODEL_LAST: return NULL; } diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c index 058cbda2a2..d25fb653d3 100644 --- a/src/qemu/qemu_domain_address.c +++ b/src/qemu/qemu_domain_address.c @@ -759,6 +759,7 @@ qemuDomainDeviceCalculatePCIConnectFlags(virDomainDeviceDefPtr dev, case VIR_DOMAIN_SOUND_MODEL_SB16: case VIR_DOMAIN_SOUND_MODEL_PCSPK: case VIR_DOMAIN_SOUND_MODEL_USB: +case VIR_DOMAIN_SOUND_MODEL_ICH7: case VIR_DOMAIN_SOUND_MODEL_LAST: return 0; } diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c index 488f258d00..4cd377c8bc 100644 --- a/src/qemu/qemu_validate.c +++ b/src/qemu/qemu_validate.c @@ -3588,6 +3588,7 @@ qemuValidateDomainDeviceDefSound(virDomainSoundDefPtr sound, case VIR_DOMAIN_SOUND_MODEL_SB16: case VIR_DOMAIN_SOUND_MODEL_PCSPK: break; +case VIR_DOMAIN_SOUND_MODEL_ICH7: case VIR_DOMAIN_SOUND_MODEL_LAST: virReportError(VIR_ERR_INTERNAL_ERROR, _("sound card model '%s' is not supported by qemu"), -- 2.27.0
[PATCH v4 4/5] bhyve: allow to specify host sound device
Allow to map sound playback and recording devices to host devices using "" OSS audio backend. Signed-off-by: Roman Bogorodskiy --- src/bhyve/bhyve_command.c | 35 +-- src/conf/domain_conf.c| 13 +++ src/conf/domain_conf.h| 4 +++ src/libvirt_private.syms | 1 + .../bhyvexml2argv-sound.args | 2 +- .../bhyvexml2argvdata/bhyvexml2argv-sound.xml | 8 - .../bhyvexml2xmlout-sound.xml | 5 +++ 7 files changed, 63 insertions(+), 5 deletions(-) diff --git a/src/bhyve/bhyve_command.c b/src/bhyve/bhyve_command.c index 1f42f71347..5289e409fa 100644 --- a/src/bhyve/bhyve_command.c +++ b/src/bhyve/bhyve_command.c @@ -478,9 +478,12 @@ bhyveBuildGraphicsArgStr(const virDomainDef *def, static int bhyveBuildSoundArgStr(const virDomainDef *def G_GNUC_UNUSED, virDomainSoundDefPtr sound, + virDomainAudioDefPtr audio, bhyveConnPtr driver, virCommandPtr cmd) { +g_auto(virBuffer) params = VIR_BUFFER_INITIALIZER; + if (!(bhyveDriverGetBhyveCaps(driver) & BHYVE_CAP_SOUND_HDA)) { /* Currently, bhyve only supports "hda" sound devices, so if it's not supported, sound devices are not supported at all */ @@ -497,9 +500,33 @@ bhyveBuildSoundArgStr(const virDomainDef *def G_GNUC_UNUSED, } virCommandAddArg(cmd, "-s"); -virCommandAddArgFormat(cmd, "%d:%d,hda,play=/dev/dsp0", + +if (audio) { +switch ((virDomainAudioType) audio->type) { +case VIR_DOMAIN_AUDIO_TYPE_OSS: +if (audio->backend.oss.inputDev) +virBufferAsprintf(¶ms, ",play=%s", + audio->backend.oss.inputDev); + +if (audio->backend.oss.outputDev) +virBufferAsprintf(¶ms, ",rec=%s", + audio->backend.oss.outputDev); + +break; + +case VIR_DOMAIN_AUDIO_TYPE_LAST: +virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("unsupported audio backend '%s'"), + virDomainAudioTypeTypeToString(audio->type)); +return -1; +} +} + +virCommandAddArgFormat(cmd, "%d:%d,hda%s", sound->info.addr.pci.slot, - sound->info.addr.pci.function); + sound->info.addr.pci.function, + virBufferCurrentContent(¶ms)); + return 0; } @@ -648,7 +675,9 @@ virBhyveProcessBuildBhyveCmd(bhyveConnPtr driver, virDomainDefPtr def, } for (i = 0; i < def->nsounds; i++) { -if (bhyveBuildSoundArgStr(def, def->sounds[i], driver, cmd) < 0) +if (bhyveBuildSoundArgStr(def, def->sounds[i], + virDomainDefFindAudioForSound(def, def->sounds[i]), + driver, cmd) < 0) goto error; } diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 9c05f301dd..921775edc4 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -31497,6 +31497,19 @@ virDomainDefFindDevice(virDomainDefPtr def, } +virDomainAudioDefPtr +virDomainDefFindAudioForSound(virDomainDefPtr def, + virDomainSoundDefPtr sound) +{ +size_t i; +for (i = 0; i < def->naudios; i++) +if (def->audios[i]->id == sound->audioId) +return def->audios[i]; + +return NULL; +} + + char * virDomainObjGetMetadata(virDomainObjPtr vm, int type, diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index e0827fee74..8a0f26f5c0 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -3712,6 +3712,10 @@ int virDomainDefFindDevice(virDomainDefPtr def, virDomainDeviceDefPtr dev, bool reportError); +virDomainAudioDefPtr +virDomainDefFindAudioForSound(virDomainDefPtr def, + virDomainSoundDefPtr sound); + const char *virDomainChrSourceDefGetPath(virDomainChrSourceDefPtr chr); void virDomainChrSourceDefClear(virDomainChrSourceDefPtr def); diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 35bf9f08ef..f950a68179 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -298,6 +298,7 @@ virDomainDefCheckABIStability; virDomainDefCheckABIStabilityFlags; virDomainDefCompatibleDevice; virDomainDefCopy; +virDomainDefFindAudioForSound; virDomainDefFindDevice; virDomainDefFormat; virDomainDefFormatConvertXMLFlags; diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-sound.args b/tests/bhyvexml2argvdata/bhyvexml2argv-sound.args index c242708ff
[PATCH v4 0/5] bhyve: implement sound device support
Changes from v3: - In conf and bhyve code, cast audio->type to virDomainAudioType and use switch to make compile force handling of all possible cases, - Patch 'tests: schema: test bhyvexml2xmloutdata schemas' removed from the series as it was pushed separately. Roman Bogorodskiy (5): conf: add 'ich7' sound model bhyve: implement sound device support conf: allow to map sound device to host device bhyve: allow to specify host sound device docs: formatdomain: document element docs/formatdomain.rst | 49 + docs/schemas/domaincommon.rng | 37 src/bhyve/bhyve_capabilities.c| 14 ++ src/bhyve/bhyve_capabilities.h| 1 + src/bhyve/bhyve_command.c | 62 ++ src/bhyve/bhyve_device.c | 9 + src/conf/domain_capabilities.c| 4 + src/conf/domain_conf.c| 196 +- src/conf/domain_conf.h| 33 +++ src/conf/virconftypes.h | 3 + src/libvirt_private.syms | 3 + src/qemu/qemu_command.c | 2 + src/qemu/qemu_domain.c| 1 + src/qemu/qemu_domain_address.c| 3 + src/qemu/qemu_driver.c| 5 + src/qemu/qemu_hotplug.c | 3 + src/qemu/qemu_validate.c | 2 + .../bhyvexml2argv-sound.args | 10 + .../bhyvexml2argv-sound.ldargs| 3 + .../bhyvexml2argvdata/bhyvexml2argv-sound.xml | 30 +++ tests/bhyvexml2argvtest.c | 6 +- .../bhyvexml2xmlout-sound.xml | 41 tests/bhyvexml2xmltest.c | 1 + 23 files changed, 515 insertions(+), 3 deletions(-) create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-sound.args create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-sound.ldargs create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-sound.xml create mode 100644 tests/bhyvexml2xmloutdata/bhyvexml2xmlout-sound.xml -- 2.27.0
[PATCH v4 3/5] conf: allow to map sound device to host device
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 | 182 - src/conf/domain_conf.h | 28 + 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, 266 insertions(+), 2 deletions(-) diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index fb9638f3f6..c933c71035 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -4372,12 +4372,47 @@ + + + + + + + + + + + + + + + oss + + + + + + + + + + + + + + + + + + + + @@ -5303,6 +5338,7 @@ + diff --git a/src/conf/domain_capabilities.c b/src/conf/domain_capabilities.c index d61108e125..59ad8937a4 100644 --- a/src/conf/domain_capabilities.c +++ b/src/conf/domain_capabilities.c @@ -688,6 +688,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 8d0d342504..9c05f301dd 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, @@ -729,6 +730,11 @@ VIR_ENUM_IMPL(virDomainSoundModel, "ich7", ); +VIR_ENUM_IMPL(virDomainAudioType, + VIR_DOMAIN_AUDIO_TYPE_LAST, + "oss", +); + VIR_ENUM_IMPL(virDomainKeyWrapCipherName, VIR_DOMAIN_KEY_WRAP_CIPHER_NAME_LAST, "aes", @@ -2864,6 +2870,24 @@ void virDomainSoundDefFree(virDomainSoundDefPtr def) VIR_FREE(def); } +void virDomainAudioDefFree(virDomainAudioDefPtr def) +{ +if (!def) +return; + +switch ((virDomainAudioType) def->type) { +case VIR_DOMAIN_AUDIO_TYPE_OSS: +VIR_FREE(def->backend.oss.inputDev); +VIR_FREE(def->backend.oss.outputDev); +break; + +case VIR_DOMAIN_AUDIO_TYPE_LAST: +break; +} + +VIR_FREE(def); +} + virDomainSoundDefPtr virDomainSoundDefRemove(virDomainDefPtr def, size_t idx) { @@ -3217,6 +3241,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; @@ -3477,6 +3504,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); @@ -4065,6 +4096,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; @@ -4159,6 +4191,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
Re: [PATCH v3 5/6] tests: schema: test bhyvexml2xmloutdata schemas
Daniel P. Berrangé wrote: > On Fri, Aug 07, 2020 at 07:09:34PM +0400, Roman Bogorodskiy wrote: > > Signed-off-by: Roman Bogorodskiy > > --- > > tests/virschematest.c | 3 ++- > > 1 file changed, 2 insertions(+), 1 deletion(-) > > Reviewed-by: Daniel P. Berrangé > Will merging this patch alone and sending the rest of the patches updated as v4 be the right way to proceed with this series? Roman Bogorodskiy signature.asc Description: PGP signature
Re: [PATCH] meson: fix some FreeBSD checks regressions
Pavel Hrdina wrote: > On Sat, Aug 08, 2020 at 01:22:09PM +0400, Roman Bogorodskiy wrote: > > * Add missing prerequisite headers for checking link_addr(3) > >in net/if_dl.h, > > * Add missing prerequisite headers for checking BRDGSFD, BRDGADD, > >BRDGDEL in net/if_bridgevar.h, > > * When checking for ifconfig(8), set not only IFCONFIG value, > >but also IFCONFIG_PATH as it's used in util/virnetdevip.c. > > > > Signed-off-by: Roman Bogorodskiy > > --- > > meson.build | 14 +- > > 1 file changed, 9 insertions(+), 5 deletions(-) > > It would be probably better to split this into 3 patches as it fixes > three different issues but it's good enough. Thanks for the reviews, pushed as 3 separate patches. > > I really hate a lot the fact that in order to use some headers you have > to include some other headers. It's so annoying and ridiculous. > Indeed. It's also frustrating that there's no way (I guess) to differentiate cases when the symbol is not present, or there's an error in the check itself. > > Thanks for addressing these regressions! > > Reviewed-by: Pavel Hrdina Roman Bogorodskiy signature.asc Description: PGP signature
[PATCH] meson: fix some FreeBSD checks regressions
* Add missing prerequisite headers for checking link_addr(3) in net/if_dl.h, * Add missing prerequisite headers for checking BRDGSFD, BRDGADD, BRDGDEL in net/if_bridgevar.h, * When checking for ifconfig(8), set not only IFCONFIG value, but also IFCONFIG_PATH as it's used in util/virnetdevip.c. Signed-off-by: Roman Bogorodskiy --- meson.build | 14 +- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/meson.build b/meson.build index 19b4795527..0913308bec 100644 --- a/meson.build +++ b/meson.build @@ -770,7 +770,7 @@ symbols = [ [ 'linux/if_vlan.h', 'GET_VLAN_VID_CMD' ], # Check for BSD approach for setting MAC addr - [ 'net/if_dl.h', 'link_addr' ], + [ 'net/if_dl.h', 'link_addr', '#include \n#include ' ], ] if host_machine.system() == 'linux' @@ -791,15 +791,18 @@ if host_machine.system() == 'linux' endif foreach symbol : symbols - if cc.has_header_symbol(symbol[0], symbol[1], args: '-D_GNU_SOURCE') + if cc.has_header_symbol(symbol[0], symbol[1], args: '-D_GNU_SOURCE', prefix: symbol.get(2, '')) conf.set('HAVE_DECL_@0@'.format(symbol[1].to_upper()), 1) endif endforeach # Check for BSD approach for bridge management -if (cc.has_header_symbol('net/if_bridgevar.h', 'BRDGSFD') and -cc.has_header_symbol('net/if_bridgevar.h', 'BRDGADD') and -cc.has_header_symbol('net/if_bridgevar.h', 'BRDGDEL')) +brd_required_headers = '''#include +#include +#include ''' +if (cc.has_header_symbol('net/if_bridgevar.h', 'BRDGSFD', prefix: brd_required_headers) and +cc.has_header_symbol('net/if_bridgevar.h', 'BRDGADD', prefix: brd_required_headers) and +cc.has_header_symbol('net/if_bridgevar.h', 'BRDGDEL', prefix: brd_required_headers)) conf.set('HAVE_BSD_BRIDGE_MGMT', 1) endif @@ -900,6 +903,7 @@ foreach name : required_programs prog = find_program(name, required: true, dirs: libvirt_sbin_path) varname = name.underscorify() conf.set_quoted(varname.to_upper(), prog.path()) + conf.set_quoted('@0@_PATH'.format(varname.to_upper()), prog.path()) set_variable('@0@_prog'.format(varname), prog) endforeach -- 2.27.0
Re: [PATCH v3 4/6] bhyve: allow to specify host sound device
Daniel P. Berrangé wrote: > On Fri, Aug 07, 2020 at 07:09:33PM +0400, Roman Bogorodskiy wrote: > > Allow to map sound playback and recording devices to host devices > > using "" OSS audio backend. > > > > Signed-off-by: Roman Bogorodskiy > > --- > > src/bhyve/bhyve_command.c | 26 --- > > src/conf/domain_conf.c| 13 ++ > > src/conf/domain_conf.h| 4 +++ > > src/libvirt_private.syms | 1 + > > .../bhyvexml2argv-sound.args | 2 +- > > .../bhyvexml2argvdata/bhyvexml2argv-sound.xml | 8 +- > > .../bhyvexml2xmlout-sound.xml | 5 > > 7 files changed, 54 insertions(+), 5 deletions(-) > > > > diff --git a/src/bhyve/bhyve_command.c b/src/bhyve/bhyve_command.c > > index 1f42f71347..8e7907b882 100644 > > --- a/src/bhyve/bhyve_command.c > > +++ b/src/bhyve/bhyve_command.c > > @@ -478,9 +478,12 @@ bhyveBuildGraphicsArgStr(const virDomainDef *def, > > static int > > bhyveBuildSoundArgStr(const virDomainDef *def G_GNUC_UNUSED, > >virDomainSoundDefPtr sound, > > + virDomainAudioDefPtr audio, > >bhyveConnPtr driver, > >virCommandPtr cmd) > > { > > +g_auto(virBuffer) params = VIR_BUFFER_INITIALIZER; > > + > > if (!(bhyveDriverGetBhyveCaps(driver) & BHYVE_CAP_SOUND_HDA)) { > > /* Currently, bhyve only supports "hda" sound devices, so > > if it's not supported, sound devices are not supported at all */ > > @@ -497,9 +500,24 @@ bhyveBuildSoundArgStr(const virDomainDef *def > > G_GNUC_UNUSED, > > } > > > > virCommandAddArg(cmd, "-s"); > > -virCommandAddArgFormat(cmd, "%d:%d,hda,play=/dev/dsp0", > > + > > +if (audio) { > > +if (audio->type == VIR_DOMAIN_AUDIO_TYPE_OSS) { > > Should use a switch() here and report enum range errors in the > LAST/default case. > > This will force us to fix bhyve when we add more audio types > later to do proper error reporting. > > > +if (audio->backend.oss.inputDev) > > +virBufferAsprintf(¶ms, ",play=%s", > > + audio->backend.oss.inputDev); > > + > > +if (audio->backend.oss.outputDev) > > +virBufferAsprintf(¶ms, ",rec=%s", > > + audio->backend.oss.outputDev); > > +} > > +} > > + > > +virCommandAddArgFormat(cmd, "%d:%d,hda%s", > > sound->info.addr.pci.slot, > > - sound->info.addr.pci.function); > > + sound->info.addr.pci.function, > > + virBufferCurrentContent(¶ms)); > > + > > What happens if you dont give an any play arg - does it just assume > the default /dev/dsp0 ? It doesn't assume these defaults. When started this way, guest will have a sound device, but playback (and likely recording) will not work because the sound device will fail to open. I'm not sure what's the purpose of things configured this way. > 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 :| > Roman Bogorodskiy signature.asc Description: PGP signature
[PATCH v3 0/6] bhyve: implement sound device support
Changes since v2: - Add 'ich7' sound device model, - Convert audio id from string to integer, - Use 'union' to store audio backend specific configuration, - Document changes in formatdomain, - Don't use hash for sound<->audio mapping lookups. Roman Bogorodskiy (6): conf: add 'ich7' sound model bhyve: implement sound device support conf: allow to map sound device to host device bhyve: allow to specify host sound device tests: schema: test bhyvexml2xmloutdata schemas docs: formatdomain: document element docs/formatdomain.rst | 49 + docs/schemas/domaincommon.rng | 37 src/bhyve/bhyve_capabilities.c| 14 ++ src/bhyve/bhyve_capabilities.h| 1 + src/bhyve/bhyve_command.c | 53 + src/bhyve/bhyve_device.c | 9 + src/conf/domain_capabilities.c| 4 + src/conf/domain_conf.c| 190 +- src/conf/domain_conf.h| 33 +++ src/conf/virconftypes.h | 3 + src/libvirt_private.syms | 3 + src/qemu/qemu_command.c | 2 + src/qemu/qemu_domain.c| 1 + src/qemu/qemu_domain_address.c| 3 + src/qemu/qemu_driver.c| 5 + src/qemu/qemu_hotplug.c | 3 + src/qemu/qemu_validate.c | 2 + .../bhyvexml2argv-sound.args | 10 + .../bhyvexml2argv-sound.ldargs| 3 + .../bhyvexml2argvdata/bhyvexml2argv-sound.xml | 30 +++ tests/bhyvexml2argvtest.c | 6 +- .../bhyvexml2xmlout-sound.xml | 41 tests/bhyvexml2xmltest.c | 1 + tests/virschematest.c | 3 +- 24 files changed, 502 insertions(+), 4 deletions(-) create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-sound.args create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-sound.ldargs create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-sound.xml create mode 100644 tests/bhyvexml2xmloutdata/bhyvexml2xmlout-sound.xml -- 2.27.0
[PATCH v3 1/6] conf: add 'ich7' sound model
Add 'ich7' sound model. This is a preparation for sound support in bhyve, as 'ich7' is the only model it supports. Signed-off-by: Roman Bogorodskiy --- docs/schemas/domaincommon.rng | 1 + src/conf/domain_conf.c | 1 + src/conf/domain_conf.h | 1 + src/qemu/qemu_command.c| 1 + src/qemu/qemu_domain_address.c | 1 + src/qemu/qemu_validate.c | 1 + 6 files changed, 6 insertions(+) diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 0d0dcbc5ce..fb9638f3f6 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -4360,6 +4360,7 @@ pcspk ac97 ich6 + ich7 ich9 usb diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index ef67efa1da..f9cdc4efa9 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -726,6 +726,7 @@ VIR_ENUM_IMPL(virDomainSoundModel, "ich6", "ich9", "usb", + "ich7", ); VIR_ENUM_IMPL(virDomainKeyWrapCipherName, diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 011bf66cb4..411b9b99e4 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1401,6 +1401,7 @@ typedef enum { VIR_DOMAIN_SOUND_MODEL_ICH6, VIR_DOMAIN_SOUND_MODEL_ICH9, VIR_DOMAIN_SOUND_MODEL_USB, +VIR_DOMAIN_SOUND_MODEL_ICH7, VIR_DOMAIN_SOUND_MODEL_LAST } virDomainSoundModel; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 01812cd39b..ec3d4c8d99 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -4026,6 +4026,7 @@ qemuBuildSoundDevStr(const virDomainDef *def, model = "sb16"; break; case VIR_DOMAIN_SOUND_MODEL_PCSPK: /* pc-speaker is handled separately */ +case VIR_DOMAIN_SOUND_MODEL_ICH7: case VIR_DOMAIN_SOUND_MODEL_LAST: return NULL; } diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c index 058cbda2a2..d25fb653d3 100644 --- a/src/qemu/qemu_domain_address.c +++ b/src/qemu/qemu_domain_address.c @@ -759,6 +759,7 @@ qemuDomainDeviceCalculatePCIConnectFlags(virDomainDeviceDefPtr dev, case VIR_DOMAIN_SOUND_MODEL_SB16: case VIR_DOMAIN_SOUND_MODEL_PCSPK: case VIR_DOMAIN_SOUND_MODEL_USB: +case VIR_DOMAIN_SOUND_MODEL_ICH7: case VIR_DOMAIN_SOUND_MODEL_LAST: return 0; } diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c index 488f258d00..4cd377c8bc 100644 --- a/src/qemu/qemu_validate.c +++ b/src/qemu/qemu_validate.c @@ -3588,6 +3588,7 @@ qemuValidateDomainDeviceDefSound(virDomainSoundDefPtr sound, case VIR_DOMAIN_SOUND_MODEL_SB16: case VIR_DOMAIN_SOUND_MODEL_PCSPK: break; +case VIR_DOMAIN_SOUND_MODEL_ICH7: case VIR_DOMAIN_SOUND_MODEL_LAST: virReportError(VIR_ERR_INTERNAL_ERROR, _("sound card model '%s' is not supported by qemu"), -- 2.27.0
[PATCH v3 5/6] tests: schema: test bhyvexml2xmloutdata schemas
Signed-off-by: Roman Bogorodskiy --- tests/virschematest.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/virschematest.c b/tests/virschematest.c index 8720031375..17eb2a4b34 100644 --- a/tests/virschematest.c +++ b/tests/virschematest.c @@ -201,7 +201,8 @@ mymain(void) DO_TEST_DIR("domain.rng", "domainschemadata", "qemuxml2argvdata", "xmconfigdata", "qemuxml2xmloutdata", "lxcxml2xmldata", -"lxcxml2xmloutdata", "bhyvexml2argvdata", "genericxml2xmlindata", +"lxcxml2xmloutdata", "bhyvexml2argvdata", +"bhyvexml2xmloutdata", "genericxml2xmlindata", "genericxml2xmloutdata", "xlconfigdata", "libxlxml2domconfigdata", "qemuhotplugtestdomains"); DO_TEST_DIR("domaincaps.rng", "domaincapsdata"); -- 2.27.0
[PATCH v3 3/6] conf: allow to map sound device to host device
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 | 176 - src/conf/domain_conf.h | 28 ++ 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, 260 insertions(+), 2 deletions(-) diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index fb9638f3f6..c933c71035 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -4372,12 +4372,47 @@ + + + + + + + + + + + + + + + oss + + + + + + + + + + + + + + + + + + + + @@ -5303,6 +5338,7 @@ + diff --git a/src/conf/domain_capabilities.c b/src/conf/domain_capabilities.c index d61108e125..59ad8937a4 100644 --- a/src/conf/domain_capabilities.c +++ b/src/conf/domain_capabilities.c @@ -688,6 +688,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 f9cdc4efa9..165d5d5f7a 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, @@ -729,6 +730,11 @@ VIR_ENUM_IMPL(virDomainSoundModel, "ich7", ); +VIR_ENUM_IMPL(virDomainAudioType, + VIR_DOMAIN_AUDIO_TYPE_LAST, + "oss", +); + VIR_ENUM_IMPL(virDomainKeyWrapCipherName, VIR_DOMAIN_KEY_WRAP_CIPHER_NAME_LAST, "aes", @@ -2872,6 +2878,24 @@ void virDomainSoundDefFree(virDomainSoundDefPtr def) VIR_FREE(def); } +void virDomainAudioDefFree(virDomainAudioDefPtr def) +{ +if (!def) +return; + +switch (def->type) { +case VIR_DOMAIN_AUDIO_TYPE_OSS: +VIR_FREE(def->backend.oss.inputDev); +VIR_FREE(def->backend.oss.outputDev); +break; + +case VIR_DOMAIN_AUDIO_TYPE_LAST: +break; +} + +VIR_FREE(def); +} + virDomainSoundDefPtr virDomainSoundDefRemove(virDomainDefPtr def, size_t idx) { @@ -3225,6 +3249,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; @@ -3485,6 +3512,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); @@ -4073,6 +4104,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; @@ -4167,6 +4199,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
[PATCH v3 6/6] docs: formatdomain: document element
Document the new element which allows to specify host audio backend for a guest device, and update the element description with the new sub-element which specified the other end of the mapping. Signed-off-by: Roman Bogorodskiy --- docs/formatdomain.rst | 49 +++ 1 file changed, 49 insertions(+) diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst index 218f0c1718..fc1aedc2ec 100644 --- a/docs/formatdomain.rst +++ b/docs/formatdomain.rst @@ -6572,6 +6572,55 @@ Valid values are: Each ``sound`` element has an optional sub-element which can tie the device to a particular PCI slot, `documented above <#elementsAddress>`__. +:since:`Since 6.7.0`, a sound device could be optionally mapped to the specific +host audio backend using the sub-element: + +:: + + ... + + + + + + ... + +Where ``1`` is an id of the `audio device <#elementsAudio>`__. +This is supported for bhyve only. + +:anchor:`` + +Audio devices +~ + +A virtual audio device corresponds to a host audio backend that is mapped +to the guest sound device. :since:`Since 6.7.0, bhyve only` + +``type`` + The required ``type`` attribute specifies audio backend type. + Currently, the only supported value is 'oss'. + +``id`` + Integer id of the audio device. Must be greater than 0. + +The 'oss' audio type supports additional configuration: + +:: + + ... + + + + + + + +``input`` + Input device. The required ``dev`` attribute specifies device path. + +``output`` + Output device. The required ``dev`` attribute specifies device path. + :anchor:`` Watchdog device -- 2.27.0
[PATCH v3 2/6] bhyve: implement sound device support
bhyve supports intel hda sound devices that could be specified on the command like using "-1:0,hda,play=$play_dev,rec=$rec_dev", where "1:0" is a PCI address, and "$play_dev" and "$rec_dev" point to the playback and recording device on the host respectively. Currently, schema of the 'sound' element doesn't allow specifying neither playback nor recording devices, so for now hardcode /dev/dsp0, which is the first audio device on the host. Signed-off-by: Roman Bogorodskiy --- src/bhyve/bhyve_capabilities.c| 14 src/bhyve/bhyve_capabilities.h| 1 + src/bhyve/bhyve_command.c | 33 + src/bhyve/bhyve_device.c | 9 + .../bhyvexml2argv-sound.args | 10 ++ .../bhyvexml2argv-sound.ldargs| 3 ++ .../bhyvexml2argvdata/bhyvexml2argv-sound.xml | 24 + tests/bhyvexml2argvtest.c | 6 +++- .../bhyvexml2xmlout-sound.xml | 36 +++ tests/bhyvexml2xmltest.c | 1 + 10 files changed, 136 insertions(+), 1 deletion(-) create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-sound.args create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-sound.ldargs create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-sound.xml create mode 100644 tests/bhyvexml2xmloutdata/bhyvexml2xmlout-sound.xml diff --git a/src/bhyve/bhyve_capabilities.c b/src/bhyve/bhyve_capabilities.c index fb8829d571..36f3985335 100644 --- a/src/bhyve/bhyve_capabilities.c +++ b/src/bhyve/bhyve_capabilities.c @@ -323,6 +323,17 @@ bhyveProbeCapsXHCIController(unsigned int *caps, char *binary) } +static int +bhyveProbeCapsSoundHda(unsigned int *caps, char *binary) +{ +return bhyveProbeCapsDeviceHelper(caps, binary, + "-s", + "0,hda", + "pci slot 0:0: unknown device \"hda\"", + BHYVE_CAP_SOUND_HDA); +} + + int virBhyveProbeCaps(unsigned int *caps) { @@ -351,6 +362,9 @@ virBhyveProbeCaps(unsigned int *caps) if ((ret = bhyveProbeCapsXHCIController(caps, binary))) goto out; +if ((ret = bhyveProbeCapsSoundHda(caps, binary))) +goto out; + out: VIR_FREE(binary); return ret; diff --git a/src/bhyve/bhyve_capabilities.h b/src/bhyve/bhyve_capabilities.h index 12926cf423..1ac9ff4283 100644 --- a/src/bhyve/bhyve_capabilities.h +++ b/src/bhyve/bhyve_capabilities.h @@ -49,6 +49,7 @@ typedef enum { BHYVE_CAP_FBUF = 1 << 4, BHYVE_CAP_XHCI = 1 << 5, BHYVE_CAP_CPUTOPOLOGY = 1 << 6, +BHYVE_CAP_SOUND_HDA = 1 << 7, } virBhyveCapsFlags; int virBhyveProbeGrubCaps(virBhyveGrubCapsFlags *caps); diff --git a/src/bhyve/bhyve_command.c b/src/bhyve/bhyve_command.c index 86e6640359..1f42f71347 100644 --- a/src/bhyve/bhyve_command.c +++ b/src/bhyve/bhyve_command.c @@ -475,6 +475,34 @@ bhyveBuildGraphicsArgStr(const virDomainDef *def, } +static int +bhyveBuildSoundArgStr(const virDomainDef *def G_GNUC_UNUSED, + virDomainSoundDefPtr sound, + bhyveConnPtr driver, + virCommandPtr cmd) +{ +if (!(bhyveDriverGetBhyveCaps(driver) & BHYVE_CAP_SOUND_HDA)) { +/* Currently, bhyve only supports "hda" sound devices, so + if it's not supported, sound devices are not supported at all */ +virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("Sound devices emulation is not supported " + "by given bhyve binary")); +return -1; +} + +if (sound->model != VIR_DOMAIN_SOUND_MODEL_ICH7) { +virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("Sound device model is not supported")); +return -1; +} + +virCommandAddArg(cmd, "-s"); +virCommandAddArgFormat(cmd, "%d:%d,hda,play=/dev/dsp0", + sound->info.addr.pci.slot, + sound->info.addr.pci.function); +return 0; +} + virCommandPtr virBhyveProcessBuildBhyveCmd(bhyveConnPtr driver, virDomainDefPtr def, bool dryRun) @@ -619,6 +647,11 @@ virBhyveProcessBuildBhyveCmd(bhyveConnPtr driver, virDomainDefPtr def, } } +for (i = 0; i < def->nsounds; i++) { +if (bhyveBuildSoundArgStr(def, def->sounds[i], driver, cmd) < 0) +goto error; +} + if (bhyveDomainDefNeedsISAController(def)) bhyveBuildLPCArgStr(def, cmd); diff --git a/src/bhyve/bhyve_device.c b/src/bhyve/bhyve_device.c index fc52280361..3c8ff587e0 100644 --- a/src/bhyve/bhyve_device.c +++ b/src/bh
[PATCH v3 4/6] bhyve: allow to specify host sound device
Allow to map sound playback and recording devices to host devices using "" OSS audio backend. Signed-off-by: Roman Bogorodskiy --- src/bhyve/bhyve_command.c | 26 --- src/conf/domain_conf.c| 13 ++ src/conf/domain_conf.h| 4 +++ src/libvirt_private.syms | 1 + .../bhyvexml2argv-sound.args | 2 +- .../bhyvexml2argvdata/bhyvexml2argv-sound.xml | 8 +- .../bhyvexml2xmlout-sound.xml | 5 7 files changed, 54 insertions(+), 5 deletions(-) diff --git a/src/bhyve/bhyve_command.c b/src/bhyve/bhyve_command.c index 1f42f71347..8e7907b882 100644 --- a/src/bhyve/bhyve_command.c +++ b/src/bhyve/bhyve_command.c @@ -478,9 +478,12 @@ bhyveBuildGraphicsArgStr(const virDomainDef *def, static int bhyveBuildSoundArgStr(const virDomainDef *def G_GNUC_UNUSED, virDomainSoundDefPtr sound, + virDomainAudioDefPtr audio, bhyveConnPtr driver, virCommandPtr cmd) { +g_auto(virBuffer) params = VIR_BUFFER_INITIALIZER; + if (!(bhyveDriverGetBhyveCaps(driver) & BHYVE_CAP_SOUND_HDA)) { /* Currently, bhyve only supports "hda" sound devices, so if it's not supported, sound devices are not supported at all */ @@ -497,9 +500,24 @@ bhyveBuildSoundArgStr(const virDomainDef *def G_GNUC_UNUSED, } virCommandAddArg(cmd, "-s"); -virCommandAddArgFormat(cmd, "%d:%d,hda,play=/dev/dsp0", + +if (audio) { +if (audio->type == VIR_DOMAIN_AUDIO_TYPE_OSS) { +if (audio->backend.oss.inputDev) +virBufferAsprintf(¶ms, ",play=%s", + audio->backend.oss.inputDev); + +if (audio->backend.oss.outputDev) +virBufferAsprintf(¶ms, ",rec=%s", + audio->backend.oss.outputDev); +} +} + +virCommandAddArgFormat(cmd, "%d:%d,hda%s", sound->info.addr.pci.slot, - sound->info.addr.pci.function); + sound->info.addr.pci.function, + virBufferCurrentContent(¶ms)); + return 0; } @@ -648,7 +666,9 @@ virBhyveProcessBuildBhyveCmd(bhyveConnPtr driver, virDomainDefPtr def, } for (i = 0; i < def->nsounds; i++) { -if (bhyveBuildSoundArgStr(def, def->sounds[i], driver, cmd) < 0) +if (bhyveBuildSoundArgStr(def, def->sounds[i], + virDomainDefFindAudioForSound(def, def->sounds[i]), + driver, cmd) < 0) goto error; } diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 165d5d5f7a..28ff4d3994 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -31499,6 +31499,19 @@ virDomainDefFindDevice(virDomainDefPtr def, } +virDomainAudioDefPtr +virDomainDefFindAudioForSound(virDomainDefPtr def, + virDomainSoundDefPtr sound) +{ +size_t i; +for (i = 0; i < def->naudios; i++) +if (def->audios[i]->id == sound->audioId) +return def->audios[i]; + +return NULL; +} + + char * virDomainObjGetMetadata(virDomainObjPtr vm, int type, diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index fdf706a7e0..d9d7b094ac 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -3712,6 +3712,10 @@ int virDomainDefFindDevice(virDomainDefPtr def, virDomainDeviceDefPtr dev, bool reportError); +virDomainAudioDefPtr +virDomainDefFindAudioForSound(virDomainDefPtr def, + virDomainSoundDefPtr sound); + const char *virDomainChrSourceDefGetPath(virDomainChrSourceDefPtr chr); void virDomainChrSourceDefClear(virDomainChrSourceDefPtr def); diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 35bf9f08ef..f950a68179 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -298,6 +298,7 @@ virDomainDefCheckABIStability; virDomainDefCheckABIStabilityFlags; virDomainDefCompatibleDevice; virDomainDefCopy; +virDomainDefFindAudioForSound; virDomainDefFindDevice; virDomainDefFormat; virDomainDefFormatConvertXMLFlags; diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-sound.args b/tests/bhyvexml2argvdata/bhyvexml2argv-sound.args index c242708ff1..05ff4965dd 100644 --- a/tests/bhyvexml2argvdata/bhyvexml2argv-sound.args +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-sound.args @@ -7,4 +7,4 @@ -s 0:0,hostbridge \ -s 2:0,ahci,hd:/tmp/freebsd.img \ -s 3:0,virtio-net,faketapdev,mac=52:54:00:b9:94:02 \ --s 4:0,hda,play=/dev/dsp0 bhyve +-s 4:0,hda,play=/dev/dsp0,rec=/dev/dsp0 bhyve diff
[PATCH] build-aux: use GNU sed for syntax-check on FreeBSD
BSD sed(1) and GNU sed(1) syntax are not compatible, and as syntax-check.mk uses the GNU flavor, set SED variable to 'gsed' by default. Signed-off-by: Roman Bogorodskiy --- build-aux/syntax-check.mk | 8 1 file changed, 8 insertions(+) I'm not sure if this requires a more comprehensive solution. I have mixed feeling about this. If we try to just use gsed like in this patch, it'll fail because we don't require gsed to be installed. OTOH, an alternative solution like checking for gsed in meson.build, and probably even generation of some files with variables to be sourced by .mk files feels like too much of a hassle, esp. in this context. diff --git a/build-aux/syntax-check.mk b/build-aux/syntax-check.mk index 6eb59cf90e..bbfcb63152 100644 --- a/build-aux/syntax-check.mk +++ b/build-aux/syntax-check.mk @@ -28,7 +28,15 @@ ME := build-aux/syntax-check.mk # ignoring the module description. AWK ?= awk GREP ?= grep +# FreeBSD (and probably some other OSes too) ships own version of sed(1), not +# compatible with the GNU sed. GNU sed is available as gsed(1), so use this +# instead +UNAME := $(shell uname) +ifeq ($(UNAME),FreeBSD) +SED ?= gsed +else SED ?= sed +endif # Helper variables. _empty = -- 2.27.0
[PATCH] tests: qemublocktest: fix crashing with SIGBUS
Commit bcbb026993 converted qemublocktest to use g_autoptr for virQEMUCaps. To prevent it from crashing, don't explicitly call virObjectUnref() on this object. Signed-off-by: Roman Bogorodskiy --- tests/qemublocktest.c | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/qemublocktest.c b/tests/qemublocktest.c index fb5319d7bd..0685b703a1 100644 --- a/tests/qemublocktest.c +++ b/tests/qemublocktest.c @@ -1394,7 +1394,6 @@ mymain(void) cleanup: qemuTestDriverFree(&driver); VIR_FREE(capslatest_x86_64); -virObjectUnref(caps_x86_64); return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE; } -- 2.27.0
Re: [libvirt PATCH 1/3] bhyve: fix NULL pointer check position
Ján Tomko wrote: > src/bhyve/bhyve_parse_command.c:437:9: warning: Either the condition > '!config' is redundant or there is possible null pointer dereference: > config. [nullPointerRedundantCheck] > > src/bhyve/bhyve_parse_command.c:280:23: warning: Either the condition > '!separator' is redundant or there is pointer arithmetic > with NULL pointer. [nullPointerArithmeticRedundantCheck] > > Signed-off-by: Ján Tomko > --- > src/bhyve/bhyve_parse_command.c | 14 +++--- > 1 file changed, 7 insertions(+), 7 deletions(-) > > diff --git a/src/bhyve/bhyve_parse_command.c b/src/bhyve/bhyve_parse_command.c > index b2d2280872..5e9cf7ba13 100644 > --- a/src/bhyve/bhyve_parse_command.c > +++ b/src/bhyve/bhyve_parse_command.c > @@ -277,11 +277,11 @@ bhyveParseBhyveLPCArg(virDomainDefPtr def, > char *type = NULL; > > separator = strchr(arg, ','); > + > +if (!separator) > +goto error; > + > param = separator + 1; > - > -if (!separator) > -goto error; > - > type = g_strndup(arg, separator - arg); > > /* Only support com%d */ > @@ -434,14 +434,14 @@ bhyveParsePCIDisk(virDomainDefPtr def, > disk->info.addr.pci.slot = pcislot; > disk->info.addr.pci.function = function; > > +if (!config) > +goto error; > + > if (STRPREFIX(config, "/dev/")) > disk->src->type = VIR_STORAGE_TYPE_BLOCK; > else > disk->src->type = VIR_STORAGE_TYPE_FILE; > > -if (!config) > -goto error; > - > separator = strchr(config, ','); > if (separator) > disk->src->path = g_strndup(config, separator - config); Reviewed-by: Roman Bogorodskiy > -- > 2.26.2 > Roman Bogorodskiy signature.asc Description: PGP signature
[PATCH v5 2/2] bhyve: support 'isa' controller for LPC
Support modeling of the 'isa' controller for bhyve. User can manually define any PCI slot for the 'isa' controller, including PCI slot 1, but other devices are not allowed to use this address. When domain configuration requires the 'isa' controller to be present, automatically add it on domain post-parse stage. Now, as this controller is always available when needed, it's not necessary to implicitly add it to the bhyve command line, so remove bhyveBuildLPCArgStr(). Also, make bhyveDomainDefNeedsISAController() static as it's no longer used outside of bhyve_domain.c. As more than one ISA controller is not supported by bhyve, and multiple controllers with the same index are forbidden, so forbid ISA controllers with non-zero index for bhyve. Signed-off-by: Roman Bogorodskiy --- src/bhyve/bhyve_command.c | 27 +++--- src/bhyve/bhyve_device.c | 23 +--- src/bhyve/bhyve_domain.c | 25 +++-- src/bhyve/bhyve_domain.h | 2 -- ...ml2argv-addr-isa-controller-on-slot-1.args | 10 ++ ...2argv-addr-isa-controller-on-slot-1.ldargs | 3 ++ ...xml2argv-addr-isa-controller-on-slot-1.xml | 26 ++ ...l2argv-addr-isa-controller-on-slot-31.args | 10 ++ ...argv-addr-isa-controller-on-slot-31.ldargs | 3 ++ ...ml2argv-addr-isa-controller-on-slot-31.xml | 26 ++ ...argv-addr-non-isa-controller-on-slot-1.xml | 23 .../bhyvexml2argv-console.args| 2 +- .../bhyvexml2argv-isa-controller.args | 10 ++ .../bhyvexml2argv-isa-controller.ldargs | 3 ++ .../bhyvexml2argv-isa-controller.xml | 24 + ...bhyvexml2argv-isa-multiple-controllers.xml | 25 + .../bhyvexml2argv-serial-grub-nocons.args | 2 +- .../bhyvexml2argv-serial-grub.args| 2 +- .../bhyvexml2argv-serial.args | 2 +- .../bhyvexml2argvdata/bhyvexml2argv-uefi.args | 4 +-- .../bhyvexml2argv-vnc-autoport.args | 4 +-- .../bhyvexml2argv-vnc-vgaconf-io.args | 4 +-- .../bhyvexml2argv-vnc-vgaconf-off.args| 4 +-- .../bhyvexml2argv-vnc-vgaconf-on.args | 4 +-- .../bhyvexml2argvdata/bhyvexml2argv-vnc.args | 4 +-- tests/bhyvexml2argvtest.c | 5 +++ ...l2xmlout-addr-isa-controller-on-slot-1.xml | 36 +++ ...2xmlout-addr-isa-controller-on-slot-31.xml | 36 +++ .../bhyvexml2xmlout-console.xml | 3 ++ .../bhyvexml2xmlout-isa-controller.xml| 36 +++ .../bhyvexml2xmlout-serial-grub-nocons.xml| 3 ++ .../bhyvexml2xmlout-serial-grub.xml | 3 ++ .../bhyvexml2xmlout-serial.xml| 3 ++ .../bhyvexml2xmlout-vnc-autoport.xml | 3 ++ .../bhyvexml2xmlout-vnc-vgaconf-io.xml| 3 ++ .../bhyvexml2xmlout-vnc-vgaconf-off.xml | 3 ++ .../bhyvexml2xmlout-vnc-vgaconf-on.xml| 3 ++ .../bhyvexml2xmlout-vnc.xml | 3 ++ tests/bhyvexml2xmltest.c | 3 ++ 39 files changed, 378 insertions(+), 37 deletions(-) create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-1.args create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-1.ldargs create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-1.xml create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-31.args create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-31.ldargs create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-31.xml create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-addr-non-isa-controller-on-slot-1.xml create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-isa-controller.args create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-isa-controller.ldargs create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-isa-controller.xml create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-isa-multiple-controllers.xml create mode 100644 tests/bhyvexml2xmloutdata/bhyvexml2xmlout-addr-isa-controller-on-slot-1.xml create mode 100644 tests/bhyvexml2xmloutdata/bhyvexml2xmlout-addr-isa-controller-on-slot-31.xml create mode 100644 tests/bhyvexml2xmloutdata/bhyvexml2xmlout-isa-controller.xml diff --git a/src/bhyve/bhyve_command.c b/src/bhyve/bhyve_command.c index 22d0b24ec4..2a3e10d649 100644 --- a/src/bhyve/bhyve_command.c +++ b/src/bhyve/bhyve_command.c @@ -329,7 +329,8 @@ bhyveBuildControllerArgStr(const virDomainDef *def, virDomainControllerDefPtr controller, bhyveConnPtr driver, virCommandPtr cmd, - unsigned *nusbcontrollers) + unsigned *nusbcontrollers, + u
[PATCH v5 1/2] conf: add 'isa' controller type
Introduce 'isa' controller type. In domain XML it looks this way: ... ... Currently, this is needed for the bhyve driver to allow choosing a specific PCI address for that. In bhyve, this controller is used to attach serial ports and a boot ROM. Signed-off-by: Roman Bogorodskiy --- docs/formatdomain.html.in | 6 +++--- docs/schemas/domaincommon.rng | 6 ++ src/conf/domain_conf.c | 9 + src/conf/domain_conf.h | 8 src/qemu/qemu_command.c| 1 + src/qemu/qemu_domain.c | 1 + src/qemu/qemu_domain_address.c | 1 + src/qemu/qemu_validate.c | 1 + src/vbox/vbox_common.c | 1 + 9 files changed, 31 insertions(+), 3 deletions(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 6b67a09bb3..f70e5abab8 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -4706,9 +4706,9 @@ Each controller has a mandatory attribute type, - which must be one of 'ide', 'fdc', 'scsi', 'sata', 'usb', - 'ccid', 'virtio-serial' or 'pci', and a mandatory - attribute index which is the decimal integer + which must be one of 'ide', 'fdc', 'scsi', 'sata', 'usb', 'ccid', 'virtio-serial', + 'xenbus', 'pci', or 'isa' (since 6.7.0), + and a mandatory attribute index which is the decimal integer describing in which order the bus controller is encountered (for use in controller attributes of <address> elements). diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 8cbbd7e6e9..11793a5ef5 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -2416,6 +2416,12 @@ + + + + isa + + diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 386b04b5b8..44b7a524ec 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -399,6 +399,7 @@ VIR_ENUM_IMPL(virDomainController, "usb", "pci", "xenbus", + "isa", ); VIR_ENUM_IMPL(virDomainControllerModelPCI, @@ -444,6 +445,9 @@ VIR_ENUM_IMPL(virDomainControllerModelSCSI, "virtio-non-transitional", ); +VIR_ENUM_IMPL(virDomainControllerModelISA, VIR_DOMAIN_CONTROLLER_MODEL_ISA_LAST, +); + VIR_ENUM_IMPL(virDomainControllerModelUSB, VIR_DOMAIN_CONTROLLER_MODEL_USB_LAST, "piix3-uhci", @@ -2318,6 +2322,7 @@ virDomainControllerDefNew(virDomainControllerType type) case VIR_DOMAIN_CONTROLLER_TYPE_SCSI: case VIR_DOMAIN_CONTROLLER_TYPE_SATA: case VIR_DOMAIN_CONTROLLER_TYPE_CCID: +case VIR_DOMAIN_CONTROLLER_TYPE_ISA: case VIR_DOMAIN_CONTROLLER_TYPE_LAST: break; } @@ -10994,6 +10999,8 @@ virDomainControllerModelTypeFromString(const virDomainControllerDef *def, return virDomainControllerModelIDETypeFromString(model); else if (def->type == VIR_DOMAIN_CONTROLLER_TYPE_VIRTIO_SERIAL) return virDomainControllerModelVirtioSerialTypeFromString(model); +else if (def->type == VIR_DOMAIN_CONTROLLER_TYPE_ISA) +return virDomainControllerModelISATypeFromString(model); return -1; } @@ -11013,6 +11020,8 @@ virDomainControllerModelTypeToString(virDomainControllerDefPtr def, return virDomainControllerModelIDETypeToString(model); else if (def->type == VIR_DOMAIN_CONTROLLER_TYPE_VIRTIO_SERIAL) return virDomainControllerModelVirtioSerialTypeToString(model); +else if (def->type == VIR_DOMAIN_CONTROLLER_TYPE_ISA) +return virDomainControllerModelISATypeToString(model); return NULL; } diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 6e9da298b4..e193cf9c5f 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -595,6 +595,7 @@ typedef enum { VIR_DOMAIN_CONTROLLER_TYPE_USB, VIR_DOMAIN_CONTROLLER_TYPE_PCI, VIR_DOMAIN_CONTROLLER_TYPE_XENBUS, +VIR_DOMAIN_CONTROLLER_TYPE_ISA, VIR_DOMAIN_CONTROLLER_TYPE_LAST } virDomainControllerType; @@ -686,6 +687,12 @@ typedef enum { VIR_DOMAIN_CONTROLLER_MODEL_VIRTIO_SERIAL_LAST } virDomainControllerModelVirtioSerial; +typedef enum { +VIR_DOMAIN_CONTROLLER_MODEL_ISA_DEFAULT = -1, + +VIR_DOMAIN_CONTROLLER_MODEL_ISA_LAST +} virDomainControllerModelISA; + #define IS_USB2_CONTROLLER(ctrl) \ (((ctrl)->type == VIR_DOMAIN_CONTROLLER_TYPE_USB) && \ ((ctrl)->model == VIR_DOMAIN_CONTROLLER_MODEL_USB_ICH9_EHCI1 || \ @@ -3558,6 +3565,7 @@ VIR_ENUM_DECL(virDomainControllerModelSCSI); VIR_ENUM_DECL(virDomainControl
[PATCH v5 0/2] bhyve: support 'isa' controller for LPC
Changes from v4: - Document 'isa' controller type in docs/formatdomain.html.in. While here, add 'xenbus' which was also missing. Roman Bogorodskiy (2): conf: add 'isa' controller type bhyve: support 'isa' controller for LPC docs/formatdomain.html.in | 6 ++-- docs/schemas/domaincommon.rng | 6 src/bhyve/bhyve_command.c | 27 +++--- src/bhyve/bhyve_device.c | 23 +--- src/bhyve/bhyve_domain.c | 25 +++-- src/bhyve/bhyve_domain.h | 2 -- src/conf/domain_conf.c| 9 + src/conf/domain_conf.h| 8 + src/qemu/qemu_command.c | 1 + src/qemu/qemu_domain.c| 1 + src/qemu/qemu_domain_address.c| 1 + src/qemu/qemu_validate.c | 1 + src/vbox/vbox_common.c| 1 + ...ml2argv-addr-isa-controller-on-slot-1.args | 10 ++ ...2argv-addr-isa-controller-on-slot-1.ldargs | 3 ++ ...xml2argv-addr-isa-controller-on-slot-1.xml | 26 ++ ...l2argv-addr-isa-controller-on-slot-31.args | 10 ++ ...argv-addr-isa-controller-on-slot-31.ldargs | 3 ++ ...ml2argv-addr-isa-controller-on-slot-31.xml | 26 ++ ...argv-addr-non-isa-controller-on-slot-1.xml | 23 .../bhyvexml2argv-console.args| 2 +- .../bhyvexml2argv-isa-controller.args | 10 ++ .../bhyvexml2argv-isa-controller.ldargs | 3 ++ .../bhyvexml2argv-isa-controller.xml | 24 + ...bhyvexml2argv-isa-multiple-controllers.xml | 25 + .../bhyvexml2argv-serial-grub-nocons.args | 2 +- .../bhyvexml2argv-serial-grub.args| 2 +- .../bhyvexml2argv-serial.args | 2 +- .../bhyvexml2argvdata/bhyvexml2argv-uefi.args | 4 +-- .../bhyvexml2argv-vnc-autoport.args | 4 +-- .../bhyvexml2argv-vnc-vgaconf-io.args | 4 +-- .../bhyvexml2argv-vnc-vgaconf-off.args| 4 +-- .../bhyvexml2argv-vnc-vgaconf-on.args | 4 +-- .../bhyvexml2argvdata/bhyvexml2argv-vnc.args | 4 +-- tests/bhyvexml2argvtest.c | 5 +++ ...l2xmlout-addr-isa-controller-on-slot-1.xml | 36 +++ ...2xmlout-addr-isa-controller-on-slot-31.xml | 36 +++ .../bhyvexml2xmlout-console.xml | 3 ++ .../bhyvexml2xmlout-isa-controller.xml| 36 +++ .../bhyvexml2xmlout-serial-grub-nocons.xml| 3 ++ .../bhyvexml2xmlout-serial-grub.xml | 3 ++ .../bhyvexml2xmlout-serial.xml| 3 ++ .../bhyvexml2xmlout-vnc-autoport.xml | 3 ++ .../bhyvexml2xmlout-vnc-vgaconf-io.xml| 3 ++ .../bhyvexml2xmlout-vnc-vgaconf-off.xml | 3 ++ .../bhyvexml2xmlout-vnc-vgaconf-on.xml| 3 ++ .../bhyvexml2xmlout-vnc.xml | 3 ++ tests/bhyvexml2xmltest.c | 3 ++ 48 files changed, 409 insertions(+), 40 deletions(-) create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-1.args create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-1.ldargs create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-1.xml create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-31.args create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-31.ldargs create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-addr-isa-controller-on-slot-31.xml create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-addr-non-isa-controller-on-slot-1.xml create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-isa-controller.args create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-isa-controller.ldargs create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-isa-controller.xml create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-isa-multiple-controllers.xml create mode 100644 tests/bhyvexml2xmloutdata/bhyvexml2xmlout-addr-isa-controller-on-slot-1.xml create mode 100644 tests/bhyvexml2xmloutdata/bhyvexml2xmlout-addr-isa-controller-on-slot-31.xml create mode 100644 tests/bhyvexml2xmloutdata/bhyvexml2xmlout-isa-controller.xml -- 2.27.0
Re: [libvirt PATCH 0/4] tests: bhyve: use more g_auto
Laine Stump wrote: > On 7/28/20 7:44 PM, Ján Tomko wrote: > > Ján Tomko (4): > >tests: bhyve: split variable declarations > >tests: bhyve: use g_autofree where possible > >tests: bhyve: use g_autoptr where possible > >tests: bhyve: remove unnecessary labels > > > > tests/bhyveargv2xmltest.c | 47 ++- > > tests/bhyvexml2argvtest.c | 33 ++- > > tests/bhyvexml2xmltest.c | 6 ++--- > > 3 files changed, 31 insertions(+), 55 deletions(-) > > > > Series > > > Reviewed-by: Laine Stump > Reviewed-by: Roman Bogorodskiy Roman Bogorodskiy signature.asc Description: PGP signature
[PATCH v2 4/4] tests: schema: test bhyvexml2xmloutdata schemas
Signed-off-by: Roman Bogorodskiy --- tests/virschematest.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/virschematest.c b/tests/virschematest.c index 8720031375..17eb2a4b34 100644 --- a/tests/virschematest.c +++ b/tests/virschematest.c @@ -201,7 +201,8 @@ mymain(void) DO_TEST_DIR("domain.rng", "domainschemadata", "qemuxml2argvdata", "xmconfigdata", "qemuxml2xmloutdata", "lxcxml2xmldata", -"lxcxml2xmloutdata", "bhyvexml2argvdata", "genericxml2xmlindata", +"lxcxml2xmloutdata", "bhyvexml2argvdata", +"bhyvexml2xmloutdata", "genericxml2xmlindata", "genericxml2xmloutdata", "xlconfigdata", "libxlxml2domconfigdata", "qemuhotplugtestdomains"); DO_TEST_DIR("domaincaps.rng", "domaincapsdata"); -- 2.27.0
[PATCH v2 1/4] bhyve: implement sound device support
bhyve supports intel hda sound devices that could be specified on the command like using "-1:0,hda,play=$play_dev,rec=$rec_dev", where "1:0" is a PCI address, and "$play_dev" and "$rec_dev" point to the playback and recording device on the host respectively. Currently, schema of the 'sound' element doesn't allow specifying neither playback nor recording devices, so for now hardcode /dev/dsp0, which is the first audio device on the host. One more simplification is to stick to "ich6" model, however the device shows as "ich7" in a guest. Signed-off-by: Roman Bogorodskiy --- src/bhyve/bhyve_capabilities.c| 14 src/bhyve/bhyve_capabilities.h| 1 + src/bhyve/bhyve_command.c | 33 + src/bhyve/bhyve_device.c | 9 + .../bhyvexml2argv-sound.args | 10 ++ .../bhyvexml2argv-sound.ldargs| 3 ++ .../bhyvexml2argvdata/bhyvexml2argv-sound.xml | 24 + tests/bhyvexml2argvtest.c | 6 +++- .../bhyvexml2xmlout-sound.xml | 36 +++ tests/bhyvexml2xmltest.c | 1 + 10 files changed, 136 insertions(+), 1 deletion(-) create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-sound.args create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-sound.ldargs create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-sound.xml create mode 100644 tests/bhyvexml2xmloutdata/bhyvexml2xmlout-sound.xml diff --git a/src/bhyve/bhyve_capabilities.c b/src/bhyve/bhyve_capabilities.c index fb8829d571..36f3985335 100644 --- a/src/bhyve/bhyve_capabilities.c +++ b/src/bhyve/bhyve_capabilities.c @@ -323,6 +323,17 @@ bhyveProbeCapsXHCIController(unsigned int *caps, char *binary) } +static int +bhyveProbeCapsSoundHda(unsigned int *caps, char *binary) +{ +return bhyveProbeCapsDeviceHelper(caps, binary, + "-s", + "0,hda", + "pci slot 0:0: unknown device \"hda\"", + BHYVE_CAP_SOUND_HDA); +} + + int virBhyveProbeCaps(unsigned int *caps) { @@ -351,6 +362,9 @@ virBhyveProbeCaps(unsigned int *caps) if ((ret = bhyveProbeCapsXHCIController(caps, binary))) goto out; +if ((ret = bhyveProbeCapsSoundHda(caps, binary))) +goto out; + out: VIR_FREE(binary); return ret; diff --git a/src/bhyve/bhyve_capabilities.h b/src/bhyve/bhyve_capabilities.h index 12926cf423..1ac9ff4283 100644 --- a/src/bhyve/bhyve_capabilities.h +++ b/src/bhyve/bhyve_capabilities.h @@ -49,6 +49,7 @@ typedef enum { BHYVE_CAP_FBUF = 1 << 4, BHYVE_CAP_XHCI = 1 << 5, BHYVE_CAP_CPUTOPOLOGY = 1 << 6, +BHYVE_CAP_SOUND_HDA = 1 << 7, } virBhyveCapsFlags; int virBhyveProbeGrubCaps(virBhyveGrubCapsFlags *caps); diff --git a/src/bhyve/bhyve_command.c b/src/bhyve/bhyve_command.c index 22d0b24ec4..64af0b3598 100644 --- a/src/bhyve/bhyve_command.c +++ b/src/bhyve/bhyve_command.c @@ -475,6 +475,34 @@ bhyveBuildGraphicsArgStr(const virDomainDef *def, } +static int +bhyveBuildSoundArgStr(const virDomainDef *def G_GNUC_UNUSED, + virDomainSoundDefPtr sound, + bhyveConnPtr driver, + virCommandPtr cmd) +{ +if (!(bhyveDriverGetBhyveCaps(driver) & BHYVE_CAP_SOUND_HDA)) { +/* Currently, bhyve only supports "hda" sound devices, so + if it's not supported, sound devices are not supported at all */ +virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("Sound devices emulation is not supported " + "by given bhyve binary")); +return -1; +} + +if (sound->model != VIR_DOMAIN_SOUND_MODEL_ICH6) { +virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("Sound device model is not supported")); +return -1; +} + +virCommandAddArg(cmd, "-s"); +virCommandAddArgFormat(cmd, "%d:%d,hda,play=/dev/dsp0", + sound->info.addr.pci.slot, + sound->info.addr.pci.function); +return 0; +} + virCommandPtr virBhyveProcessBuildBhyveCmd(bhyveConnPtr driver, virDomainDefPtr def, bool dryRun) @@ -619,6 +647,11 @@ virBhyveProcessBuildBhyveCmd(bhyveConnPtr driver, virDomainDefPtr def, } } +for (i = 0; i < def->nsounds; i++) { +if (bhyveBuildSoundArgStr(def, def->sounds[i], driver, cmd) < 0) +goto error; +} + if (bhyveDomainDefNeedsISAController(def)) bhyveBuildLPCArgStr(def, cmd); diff --git a/src/bhyve/bhyve_devi
[PATCH v2 3/4] bhyve: allow to specify host sound device
Allow to map sound playback and recording devices to host devices using "" OSS audio backend. Signed-off-by: Roman Bogorodskiy --- src/bhyve/bhyve_command.c | 37 +-- .../bhyvexml2argv-sound.args | 2 +- .../bhyvexml2argvdata/bhyvexml2argv-sound.xml | 8 +++- .../bhyvexml2xmlout-sound.xml | 5 +++ 4 files changed, 47 insertions(+), 5 deletions(-) diff --git a/src/bhyve/bhyve_command.c b/src/bhyve/bhyve_command.c index 64af0b3598..9c48c31066 100644 --- a/src/bhyve/bhyve_command.c +++ b/src/bhyve/bhyve_command.c @@ -478,9 +478,13 @@ bhyveBuildGraphicsArgStr(const virDomainDef *def, static int bhyveBuildSoundArgStr(const virDomainDef *def G_GNUC_UNUSED, virDomainSoundDefPtr sound, + virHashTablePtr audios, bhyveConnPtr driver, virCommandPtr cmd) { +g_auto(virBuffer) params = VIR_BUFFER_INITIALIZER; +virDomainAudioDefPtr audio; + if (!(bhyveDriverGetBhyveCaps(driver) & BHYVE_CAP_SOUND_HDA)) { /* Currently, bhyve only supports "hda" sound devices, so if it's not supported, sound devices are not supported at all */ @@ -497,9 +501,25 @@ bhyveBuildSoundArgStr(const virDomainDef *def G_GNUC_UNUSED, } virCommandAddArg(cmd, "-s"); -virCommandAddArgFormat(cmd, "%d:%d,hda,play=/dev/dsp0", + +if (audios) { +audio = virHashLookup(audios, sound->audioId); + +if (audio) { +if (audio->type == VIR_DOMAIN_AUDIO_TYPE_OSS) { +if (audio->inputDev) +virBufferAsprintf(¶ms, ",play=%s", audio->inputDev); + +if (audio->outputDev) +virBufferAsprintf(¶ms, ",rec=%s", audio->outputDev); +} +} +} +virCommandAddArgFormat(cmd, "%d:%d,hda%s", sound->info.addr.pci.slot, - sound->info.addr.pci.function); + sound->info.addr.pci.function, + virBufferCurrentContent(¶ms)); + return 0; } @@ -519,6 +539,7 @@ virBhyveProcessBuildBhyveCmd(bhyveConnPtr driver, virDomainDefPtr def, size_t i; unsigned nusbcontrollers = 0; unsigned nvcpus = virDomainDefGetVcpus(def); +virHashTablePtr audios = NULL; /* CPUs */ virCommandAddArg(cmd, "-c"); @@ -647,11 +668,20 @@ virBhyveProcessBuildBhyveCmd(bhyveConnPtr driver, virDomainDefPtr def, } } +if (def->naudios > 0) { +audios = virHashCreate(def->naudios, NULL); + +for (i = 0; i < def->naudios; i++) +virHashAddEntry(audios, def->audios[i]->id, def->audios[i]); +} + for (i = 0; i < def->nsounds; i++) { -if (bhyveBuildSoundArgStr(def, def->sounds[i], driver, cmd) < 0) +if (bhyveBuildSoundArgStr(def, def->sounds[i], audios, driver, cmd) < 0) goto error; } +virHashFree(audios); + if (bhyveDomainDefNeedsISAController(def)) bhyveBuildLPCArgStr(def, cmd); @@ -675,6 +705,7 @@ virBhyveProcessBuildBhyveCmd(bhyveConnPtr driver, virDomainDefPtr def, return cmd; error: +virHashFree(audios); virCommandFree(cmd); return NULL; } diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-sound.args b/tests/bhyvexml2argvdata/bhyvexml2argv-sound.args index c242708ff1..05ff4965dd 100644 --- a/tests/bhyvexml2argvdata/bhyvexml2argv-sound.args +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-sound.args @@ -7,4 +7,4 @@ -s 0:0,hostbridge \ -s 2:0,ahci,hd:/tmp/freebsd.img \ -s 3:0,virtio-net,faketapdev,mac=52:54:00:b9:94:02 \ --s 4:0,hda,play=/dev/dsp0 bhyve +-s 4:0,hda,play=/dev/dsp0,rec=/dev/dsp0 bhyve diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-sound.xml b/tests/bhyvexml2argvdata/bhyvexml2argv-sound.xml index dd4f0491a2..f622208b79 100644 --- a/tests/bhyvexml2argvdata/bhyvexml2argv-sound.xml +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-sound.xml @@ -19,6 +19,12 @@ - + + + + + + + diff --git a/tests/bhyvexml2xmloutdata/bhyvexml2xmlout-sound.xml b/tests/bhyvexml2xmloutdata/bhyvexml2xmlout-sound.xml index f5da7c23c8..529629f165 100644 --- a/tests/bhyvexml2xmloutdata/bhyvexml2xmlout-sound.xml +++ b/tests/bhyvexml2xmloutdata/bhyvexml2xmlout-sound.xml @@ -30,7 +30,12 @@ + + + + + -- 2.27.0
[PATCH v2 2/4] conf: allow to map sound device to host device
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(virDoma
[PATCH v2 0/4] bhyve: implement sound device support
Changes from v1: Main change is the addition of the "" element that allows to map the "" device to the host audio backend. Would appreciate initial feedback on this one, and then I'll proceed with adding more validation. Roman Bogorodskiy (4): bhyve: implement sound device support conf: allow to map sound device to host device bhyve: allow to specify host sound device tests: schema: test bhyvexml2xmloutdata schemas docs/schemas/domaincommon.rng | 36 src/bhyve/bhyve_capabilities.c| 14 ++ src/bhyve/bhyve_capabilities.h| 1 + src/bhyve/bhyve_command.c | 64 +++ src/bhyve/bhyve_device.c | 9 + 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 + .../bhyvexml2argv-sound.args | 10 ++ .../bhyvexml2argv-sound.ldargs| 3 + .../bhyvexml2argvdata/bhyvexml2argv-sound.xml | 30 tests/bhyvexml2argvtest.c | 6 +- .../bhyvexml2xmlout-sound.xml | 41 + tests/bhyvexml2xmltest.c | 1 + tests/virschematest.c | 3 +- 23 files changed, 416 insertions(+), 4 deletions(-) create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-sound.args create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-sound.ldargs create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-sound.xml create mode 100644 tests/bhyvexml2xmloutdata/bhyvexml2xmlout-sound.xml -- 2.27.0
Re: [PATCH v4 2/2] bhyve: support 'isa' controller for LPC
Daniel P. Berrangé wrote: > On Wed, Jul 15, 2020 at 07:25:45PM +0400, Roman Bogorodskiy wrote: > > Support modeling of the 'isa' controller for bhyve. User can manually > > define any PCI slot for the 'isa' controller, including PCI slot 1, > > but other devices are not allowed to use this address. > > > > When domain configuration requires the 'isa' controller to be present, > > automatically add it on domain post-parse stage. > > > > Now, as this controller is always available when needed, it's not > > necessary to implicitly add it to the bhyve command line, so remove > > bhyveBuildLPCArgStr(). > > > > Also, make bhyveDomainDefNeedsISAController() static as it's no longer > > used outside of bhyve_domain.c. > > > > As more than one ISA controller is not supported by bhyve, > > and multiple controllers with the same index are forbidden, > > so forbid ISA controllers with non-zero index for bhyve. > > > > Signed-off-by: Roman Bogorodskiy > > > diff --git a/src/bhyve/bhyve_device.c b/src/bhyve/bhyve_device.c > > index fc52280361..52a055f205 100644 > > --- a/src/bhyve/bhyve_device.c > > +++ b/src/bhyve/bhyve_device.c > > @@ -46,10 +46,16 @@ bhyveCollectPCIAddress(virDomainDefPtr def > > G_GNUC_UNUSED, > > if (addr->slot == 0) { > > return 0; > > } else if (addr->slot == 1) { > > -virReportError(VIR_ERR_INTERNAL_ERROR, "%s", > > - _("PCI bus 0 slot 1 is reserved for the > > implicit " > > - "LPC PCI-ISA bridge")); > > -return -1; > > +if (!(device->type == VIR_DOMAIN_DEVICE_CONTROLLER && > > + device->data.controller->type == > > VIR_DOMAIN_CONTROLLER_TYPE_ISA)) { > > + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", > > +_("PCI bus 0 slot 1 is reserved for the > > implicit " > > + "LPC PCI-ISA bridge")); > > + return -1; > > +} else { > > +/* We reserve slot 1 for LPC in > > bhyveAssignDevicePCISlots(), so exit early */ > > +return 0; > > +} > > IIUC, this series makes it possible to put the TPC in a different > slot, so does it still make sense to forbid use of slot 1 as a > hardcoded rule ? IIRC, the idea behind that is to give some time window for users to allow moving guests from the new version to the old one. If we allow to use slot 1, it won't be possible to move the guest to the old libvirt as it will complain slot 1 should be used only for 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 :| > Roman Bogorodskiy signature.asc Description: PGP signature
Re: [PATCH 2/4] conf: allow to map sound device to host device
Daniel P. Berrangé wrote: > On Sat, Jul 18, 2020 at 04:31:16PM +0400, Roman Bogorodskiy wrote: > > Extend device element to accept "soundDevice" > > sub-element which allows to map guest sound device to host > > sound device. > > > > Example > > > > > > > > > > IIUC, FreeBSD's audio subsystem is the classic OSS API ? The sound(4) manpage[1] claims it supports most of the OSS ioctls(). > > > > > The "playback" attribute points to the playback device, > > and "recording" attribute points to the recording device. > > I'm thinking about how we'll have to deal with QEMU's sound backend > options, and alignment with BHyve / FreeBSD. > > In QEMU there are multiple backends, OSS, ALSA, PulseAudio, SPICE, > VNC, and many more. The backends have many properties, and many > of the properties can be set separately for input and output. > > IIUC, QEMU can expose multiple sound devices to the guest too. > > I think this means that we can have a M:N relationship between > a sound device, and an audio backend, not just 1:1. > > Assuming I'm right about the M:N relationship, I assume that > of multiple cards all do playback concurrently, something > will have todo mixing of the streams ? How will that work > with audio capture, is only one card allowed to capture at > any time ? In case of FreeBSD, the sound driver does mixing on its own, i.e. you can boot multiple guests pointed to /dev/dsp0 and audio streams from these guests will be played properly. I didn't test capturing though. > I'm copying Gerd to confirm the above... > > Anyway, if we have M:N relation, then we'll need separate > configuration elements. > > So I think we'd need to allow something like this: > > > > > > > > > > > > > > > > > > > > > > Here we have one sound device connected to OSS, and two sound > devices connected to SPICE. > > > > > Signed-off-by: Roman Bogorodskiy > > --- > > docs/schemas/domaincommon.rng | 15 +++ > > src/conf/domain_conf.c| 24 > > src/conf/domain_conf.h| 3 +++ > > 3 files changed, 42 insertions(+) > > > > diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng > > index a810f569c6..b11b3f2af6 100644 > > --- a/docs/schemas/domaincommon.rng > > +++ b/docs/schemas/domaincommon.rng > > @@ -4346,6 +4346,18 @@ > > > > > > > > + > > + > > + > > + > > + > > + > > + > > + > > + > > + > > + > > + > > > > > > > > @@ -4366,6 +4378,9 @@ > > > > > > > > + > > + > > + > > > > > > > > diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c > > index 7ecd2818b9..b678a2319d 100644 > > --- a/src/conf/domain_conf.c > > +++ b/src/conf/domain_conf.c > > @@ -2850,6 +2850,9 @@ void virDomainSoundDefFree(virDomainSoundDefPtr def) > > virDomainSoundCodecDefFree(def->codecs[i]); > > VIR_FREE(def->codecs); > > > > +VIR_FREE(def->playback); > > +VIR_FREE(def->recording); > > + > > VIR_FREE(def); > > } > > > > @@ -14984,6 +14987,8 @@ virDomainSoundDefParseXML(virDomainXMLOptionPtr > > xmlopt, > > virDomainSoundDefPtr def; > > VIR_XPATH_NODE_AUTORESTORE(ctxt); > > g_autofree char *model = NULL; > > +char *recording = NULL; > > +xmlNodePtr soundDeviceNode; > > > > if (VIR_ALLOC(def) < 0) > > return NULL; > > @@ -15024,6 +15029,14 @@ virDomainSoundDefParseXML(virDomainXMLOptionPtr > > xmlopt, > > } > > } > > > > +soundDeviceNode = virXPathNode("./soundDevice", ctxt); > > +if (soundDeviceNode) { > > +def->playback = virXMLPropString(soundDeviceNode, "playback"); > > +recording = virXMLPropString(soundDeviceNode, "recording"); > > +if (recording) > > +def->recording = recording; > > +} > > + > > if (virDomainDeviceInfoParseXML(xmlopt, node, &def->info, flags) < 0) > > goto error; > > > >
[PATCH 3/4] bhyve: allow to specify host sound device
Allow to map sound playback and recording devices to host devices using the: element under ''. Signed-off-by: Roman Bogorodskiy --- src/bhyve/bhyve_command.c | 13 +++-- tests/bhyvexml2argvdata/bhyvexml2argv-sound.args| 2 +- tests/bhyvexml2argvdata/bhyvexml2argv-sound.xml | 4 +++- tests/bhyvexml2xmloutdata/bhyvexml2xmlout-sound.xml | 1 + 4 files changed, 16 insertions(+), 4 deletions(-) diff --git a/src/bhyve/bhyve_command.c b/src/bhyve/bhyve_command.c index 64af0b3598..62109f2db3 100644 --- a/src/bhyve/bhyve_command.c +++ b/src/bhyve/bhyve_command.c @@ -481,6 +481,8 @@ bhyveBuildSoundArgStr(const virDomainDef *def G_GNUC_UNUSED, bhyveConnPtr driver, virCommandPtr cmd) { +g_auto(virBuffer) params = VIR_BUFFER_INITIALIZER; + if (!(bhyveDriverGetBhyveCaps(driver) & BHYVE_CAP_SOUND_HDA)) { /* Currently, bhyve only supports "hda" sound devices, so if it's not supported, sound devices are not supported at all */ @@ -497,9 +499,16 @@ bhyveBuildSoundArgStr(const virDomainDef *def G_GNUC_UNUSED, } virCommandAddArg(cmd, "-s"); -virCommandAddArgFormat(cmd, "%d:%d,hda,play=/dev/dsp0", + +if (sound->playback) +virBufferAsprintf(¶ms, ",play=%s", sound->playback); +if (sound->recording) +virBufferAsprintf(¶ms, ",rec=%s", sound->recording); + +virCommandAddArgFormat(cmd, "%d:%d,hda%s", sound->info.addr.pci.slot, - sound->info.addr.pci.function); + sound->info.addr.pci.function, + virBufferCurrentContent(¶ms)); return 0; } diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-sound.args b/tests/bhyvexml2argvdata/bhyvexml2argv-sound.args index c242708ff1..05ff4965dd 100644 --- a/tests/bhyvexml2argvdata/bhyvexml2argv-sound.args +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-sound.args @@ -7,4 +7,4 @@ -s 0:0,hostbridge \ -s 2:0,ahci,hd:/tmp/freebsd.img \ -s 3:0,virtio-net,faketapdev,mac=52:54:00:b9:94:02 \ --s 4:0,hda,play=/dev/dsp0 bhyve +-s 4:0,hda,play=/dev/dsp0,rec=/dev/dsp0 bhyve diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-sound.xml b/tests/bhyvexml2argvdata/bhyvexml2argv-sound.xml index dd4f0491a2..db6ebc51e3 100644 --- a/tests/bhyvexml2argvdata/bhyvexml2argv-sound.xml +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-sound.xml @@ -19,6 +19,8 @@ - + + + diff --git a/tests/bhyvexml2xmloutdata/bhyvexml2xmlout-sound.xml b/tests/bhyvexml2xmloutdata/bhyvexml2xmlout-sound.xml index f5da7c23c8..9c0007c827 100644 --- a/tests/bhyvexml2xmloutdata/bhyvexml2xmlout-sound.xml +++ b/tests/bhyvexml2xmloutdata/bhyvexml2xmlout-sound.xml @@ -30,6 +30,7 @@ + -- 2.27.0
[PATCH 4/4] tests: schema: test bhyvexml2xmloutdata schemas
Signed-off-by: Roman Bogorodskiy --- tests/virschematest.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/virschematest.c b/tests/virschematest.c index 8720031375..17eb2a4b34 100644 --- a/tests/virschematest.c +++ b/tests/virschematest.c @@ -201,7 +201,8 @@ mymain(void) DO_TEST_DIR("domain.rng", "domainschemadata", "qemuxml2argvdata", "xmconfigdata", "qemuxml2xmloutdata", "lxcxml2xmldata", -"lxcxml2xmloutdata", "bhyvexml2argvdata", "genericxml2xmlindata", +"lxcxml2xmloutdata", "bhyvexml2argvdata", +"bhyvexml2xmloutdata", "genericxml2xmlindata", "genericxml2xmloutdata", "xlconfigdata", "libxlxml2domconfigdata", "qemuhotplugtestdomains"); DO_TEST_DIR("domaincaps.rng", "domaincapsdata"); -- 2.27.0
[PATCH 2/4] conf: allow to map sound device to host device
Extend device element to accept "soundDevice" sub-element which allows to map guest sound device to host sound device. Example The "playback" attribute points to the playback device, and "recording" attribute points to the recording device. Signed-off-by: Roman Bogorodskiy --- docs/schemas/domaincommon.rng | 15 +++ src/conf/domain_conf.c| 24 src/conf/domain_conf.h| 3 +++ 3 files changed, 42 insertions(+) diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index a810f569c6..b11b3f2af6 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -4346,6 +4346,18 @@ + + + + + + + + + + + + @@ -4366,6 +4378,9 @@ + + + diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 7ecd2818b9..b678a2319d 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -2850,6 +2850,9 @@ void virDomainSoundDefFree(virDomainSoundDefPtr def) virDomainSoundCodecDefFree(def->codecs[i]); VIR_FREE(def->codecs); +VIR_FREE(def->playback); +VIR_FREE(def->recording); + VIR_FREE(def); } @@ -14984,6 +14987,8 @@ virDomainSoundDefParseXML(virDomainXMLOptionPtr xmlopt, virDomainSoundDefPtr def; VIR_XPATH_NODE_AUTORESTORE(ctxt); g_autofree char *model = NULL; +char *recording = NULL; +xmlNodePtr soundDeviceNode; if (VIR_ALLOC(def) < 0) return NULL; @@ -15024,6 +15029,14 @@ virDomainSoundDefParseXML(virDomainXMLOptionPtr xmlopt, } } +soundDeviceNode = virXPathNode("./soundDevice", ctxt); +if (soundDeviceNode) { +def->playback = virXMLPropString(soundDeviceNode, "playback"); +recording = virXMLPropString(soundDeviceNode, "recording"); +if (recording) +def->recording = recording; +} + if (virDomainDeviceInfoParseXML(xmlopt, node, &def->info, flags) < 0) goto error; @@ -15056,6 +15069,9 @@ virDomainSoundDefEquals(const virDomainSoundDef *a, !virDomainDeviceInfoAddressIsEqual(&a->info, &b->info)) return false; +if ((a->playback != b->playback) || (a->recording != b->recording)) +return false; + return true; } @@ -27284,6 +27300,14 @@ virDomainSoundDefFormat(virBufferPtr buf, for (i = 0; i < def->ncodecs; i++) virDomainSoundCodecDefFormat(&childBuf, def->codecs[i]); +if (def->playback) { +virBufferAsprintf(&childBuf, "playback); +if (def->recording) +virBufferAsprintf(&childBuf, " recording='%s'/>\n", def->recording); +else +virBufferAddLit(&childBuf, "/>\n"); +} + if (virDomainDeviceInfoFormat(&childBuf, &def->info, flags) < 0) return -1; diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 241149af24..b501f48442 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1415,6 +1415,9 @@ struct _virDomainSoundDef { size_t ncodecs; virDomainSoundCodecDefPtr *codecs; + +char *playback; +char *recording; }; typedef enum { -- 2.27.0