From nobody Tue May 14 13:16:03 2024 Delivered-To: importer@patchew.org Received-SPF: none (zohomail.com: 8.43.85.245 is neither permitted nor denied by domain of lists.libvirt.org) client-ip=8.43.85.245; envelope-from=devel-bounces@lists.libvirt.org; helo=lists.libvirt.org; Authentication-Results: mx.zohomail.com; spf=none (zohomail.com: 8.43.85.245 is neither permitted nor denied by domain of lists.libvirt.org) smtp.mailfrom=devel-bounces@lists.libvirt.org; dmarc=fail(p=none dis=none) header.from=redhat.com Return-Path: Received: from lists.libvirt.org (lists.libvirt.org [8.43.85.245]) by mx.zohomail.com with SMTPS id 1700078162304551.8656838858043; Wed, 15 Nov 2023 11:56:02 -0800 (PST) Received: by lists.libvirt.org (Postfix, from userid 996) id 678E9188E; Wed, 15 Nov 2023 14:56:00 -0500 (EST) Received: from lists.libvirt.org (localhost [IPv6:::1]) by lists.libvirt.org (Postfix) with ESMTP id 275EF17D7; Wed, 15 Nov 2023 14:52:56 -0500 (EST) Received: by lists.libvirt.org (Postfix, from userid 996) id 2A7C21867; Wed, 15 Nov 2023 14:52:53 -0500 (EST) Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by lists.libvirt.org (Postfix) with ESMTPS id 1406F17D7 for ; Wed, 15 Nov 2023 14:52:52 -0500 (EST) Received: from mimecast-mx02.redhat.com (mx-ext.redhat.com [66.187.233.73]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-168-L_mgkOh9OFONl_MUPKo_DQ-1; Wed, 15 Nov 2023 14:52:50 -0500 Received: from smtp.corp.redhat.com (int-mx07.intmail.prod.int.rdu2.redhat.com [10.11.54.7]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id D6DCE3C000AA for ; Wed, 15 Nov 2023 19:52:49 +0000 (UTC) Received: from himantopus.redhat.com (unknown [10.22.9.226]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 9E9A11C060AE; Wed, 15 Nov 2023 19:52:49 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on lists.libvirt.org X-Spam-Level: X-Spam-Status: No, score=-0.8 required=5.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H3, RCVD_IN_MSPIKE_WL,SPF_HELO_NONE,T_SCC_BODY_TEXT_LINE autolearn=unavailable autolearn_force=no version=3.4.4 X-MC-Unique: L_mgkOh9OFONl_MUPKo_DQ-1 From: Jonathon Jongsma To: devel@lists.libvirt.org Subject: [libvirt PATCH v2] qemu: add runtime config option for nbdkit Date: Wed, 15 Nov 2023 13:52:48 -0600 Message-ID: <20231115195248.676606-1-jjongsma@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.4.1 on 10.11.54.7 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Message-ID-Hash: XJQCWHMDD6KZBZHET64IDRFUZJX4OPJL X-Message-ID-Hash: XJQCWHMDD6KZBZHET64IDRFUZJX4OPJL X-MailFrom: jjongsma@redhat.com X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; emergency; loop; banned-address; member-moderation; header-match-config-1; header-match-config-2; header-match-config-3; header-match-devel.lists.libvirt.org-0; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; suspicious-header CC: Andrea Bolognani X-Mailman-Version: 3.2.2 Precedence: list List-Id: Development discussions about the libvirt library & tools Archived-At: List-Archive: List-Help: List-Post: List-Subscribe: List-Unsubscribe: Content-Type: text/plain; charset="utf-8"; x-default="true" Content-Transfer-Encoding: quoted-printable X-ZM-MESSAGEID: 1700078162689100001 Currently when we build with nbdkit support, libvirt will always try to use nbdkit to access remote disk sources when it is available. But without an up-to-date selinux policy allowing this, it will fail. Because the required selinux policies are not yet widely available, we have disabled nbdkit support on rpm builds for all distributions before Fedora 40. Unfortunately, this makes it more difficult to test nbdkit support. After someone updates to the necessary selinux policies, they would also need to rebuild libvirt to enable nbdkit support. By introducing a configure option (storage_use_nbdkit), we can build packages with nbdkit support but have it disabled by default. Signed-off-by: Jonathon Jongsma Suggested-by: Andrea Bolognani --- Responding to one part of Andrea's review on v1: > The main concern I have is how this will be handled for upgrades. At > some point we want to make nbdkit the default, right? But that would > mean changing how existing installations behave. I guess that is fine > in this case? Performing the switch transparently was always the plan > after all... I still feel a tiny bit uneasy about that though. Yes, I feel the same tiny bit of unease. I think it will be OK though. For upgrades, we have to consider three basic cases: 1. user doesn't make any configuration changes - by default nbdkit will not be used. When the default changes in the future, nbdkit will start being used after upgrade. 2. user explicitly enables nbdkit - if the user decides to enable nbdkit explicitly via config file, libvirt will start using nbdkit to serve remote disks. In order for this to work, the user would have had to install a custom selinux policy. When the default changes in the future (because the selinux policy is included downstream) libvirt will continue using nbdkit for remote disks after upgrade 3. user explicitly disables nbdkit - there is very little reason for the user to explicitly disable this configure option until there is a widely-available selinux policy that makes it possible to *enable* the option. I can think of two potential reaons it might be done: A) the user wanted to test the nbdkit feature so they explicitly enabled it in the config file. After finding out that it failed due to missing selinux policies, the user explicitly disabled it in the config file rather than just deleting the configuration option. B) the user doesn't want nbdkit to be used even if the default changes in the future. The only slightly problematic option for upgrades is 3A. If a user disabled the option and then upgraded to a libvirt version that defaulted to enabling nbdkit, that new default would not be used. If the downstream distribution then removed the qemu block drivers for curl and ssh, remote disks may no longer work. But since there's very little reason for a user to explicitly disable this configure option while it is disabled by default, I don't see this as a scenario that is likely to happen to anybody aside from a developer or two. libvirt.spec.in | 18 ++++++++++++++---- meson.build | 4 ++++ meson_options.txt | 3 ++- src/qemu/libvirtd_qemu.aug | 3 +++ src/qemu/meson.build | 2 ++ src/qemu/qemu.conf.in | 10 ++++++++++ src/qemu/qemu_conf.c | 15 +++++++++++++++ src/qemu/qemu_conf.h | 2 ++ src/qemu/qemu_domain.c | 3 +++ tests/qemuxml2argvtest.c | 15 +++++++++------ 10 files changed, 64 insertions(+), 11 deletions(-) diff --git a/libvirt.spec.in b/libvirt.spec.in index f50c451e73..5107b06677 100644 --- a/libvirt.spec.in +++ b/libvirt.spec.in @@ -96,6 +96,7 @@ %define with_sanlock 0 %define with_numad 0 %define with_nbdkit 0 +%define with_nbdkit_config_default 0 %define with_firewalld_zone 0 %define with_netcf 0 %define with_libssh2 0 @@ -174,15 +175,17 @@ %endif %endif =20 -# We should only enable nbdkit support if the OS ships a SELinux policy th= at -# allows libvirt to launch it. Right now that's not the case anywhere, but -# things should be fine by the time Fedora 40 is released. +# We want to build with nbdkit support, but should only enable nbdkit by +# default if the OS ships a SELinux policy that allows libvirt to launch i= t. +# Right now that's not the case anywhere, but things should be fine by the= time +# Fedora 40 is released. # # TODO: add RHEL 9 once a minor release that contains the necessary SELinux # bits exists (we only support the most recent minor release) %if %{with_qemu} + %define with_nbdkit 0%{!?_without_nbdkit:1} %if 0%{?fedora} >=3D 40 - %define with_nbdkit 0%{!?_without_nbdkit:1} + %define with_nbdkit_config_default 0%{!?_without_nbdkit_config_def= ault:1} %endif %endif =20 @@ -1184,6 +1187,12 @@ exit 1 %define arg_nbdkit -Dnbdkit=3Ddisabled %endif =20 +%if %{with_nbdkit_config_default} + %define arg_nbdkit_config_default -Dnbdkit_config_default=3Dtrue +%else + %define arg_nbdkit_config_default -Dnbdkit_config_default=3Dfalse +%endif + %if %{with_fuse} %define arg_fuse -Dfuse=3Denabled %else @@ -1298,6 +1307,7 @@ export SOURCE_DATE_EPOCH=3D$(stat --printf=3D'%Y' %{_= specdir}/libvirt.spec) %{?arg_sanlock} \ -Dlibpcap=3Denabled \ %{?arg_nbdkit} \ + %{?arg_nbdkit_config_default} \ -Dlibnl=3Denabled \ -Daudit=3Denabled \ -Ddtrace=3Denabled \ diff --git a/meson.build b/meson.build index 62481a008d..ce8ea1300f 100644 --- a/meson.build +++ b/meson.build @@ -1012,6 +1012,10 @@ if not conf.has('WITH_NBDKIT') libnbd_dep =3D dependency('', required: false) endif =20 +# default value for storage_use_nbdkit config option +use_nbdkit_default =3D get_option('nbdkit_config_default') +conf.set10('USE_NBDKIT_DEFAULT', use_nbdkit_default) + libnl_version =3D '3.0' if not get_option('libnl').disabled() and host_machine.system() =3D=3D 'li= nux' libnl_dep =3D dependency('libnl-3.0', version: '>=3D' + libnl_version, r= equired: get_option('libnl')) diff --git a/meson_options.txt b/meson_options.txt index a0928102bf..af76cc967e 100644 --- a/meson_options.txt +++ b/meson_options.txt @@ -104,7 +104,8 @@ option('loader_nvram', type: 'string', value: '', descr= iption: 'Pass list of pai option('login_shell', type: 'feature', value: 'auto', description: 'build = virt-login-shell') option('nss', type: 'feature', value: 'auto', description: 'enable Name Se= rvice Switch plugin for resolving guest IP addresses') option('numad', type: 'feature', value: 'auto', description: 'use numad to= manage CPU placement dynamically') -option('nbdkit', type: 'feature', value: 'auto', description: 'use nbdkit = to access network disks') +option('nbdkit', type: 'feature', value: 'auto', description: 'Build nbdki= t storage backend') +option('nbdkit_config_default', type: 'boolean', value: 'false', descripti= on: 'Whether to use nbdkit storage backend for network disks by default (co= nfigurable)') option('pm_utils', type: 'feature', value: 'auto', description: 'use pm-ut= ils for power management') option('sysctl_config', type: 'feature', value: 'auto', description: 'Whet= her to install sysctl configs') option('tls_priority', type: 'string', value: 'NORMAL', description: 'set = the default TLS session priority string') diff --git a/src/qemu/libvirtd_qemu.aug b/src/qemu/libvirtd_qemu.aug index ed097ea3d9..43485b43fb 100644 --- a/src/qemu/libvirtd_qemu.aug +++ b/src/qemu/libvirtd_qemu.aug @@ -147,6 +147,8 @@ module Libvirtd_qemu =3D =20 let capability_filters_entry =3D str_array_entry "capability_filters" =20 + let storage_entry =3D bool_entry "storage_use_nbdkit" + (* Each entry in the config is one of the following ... *) let entry =3D default_tls_entry | vnc_entry @@ -170,6 +172,7 @@ module Libvirtd_qemu =3D | nbd_entry | swtpm_entry | capability_filters_entry + | storage_entry | obsolete_entry =20 let comment =3D [ label "#comment" . del /#[ \t]*/ "# " . store /([^ \= t\n][^\n]*)?/ . del /\n/ "\n" ] diff --git a/src/qemu/meson.build b/src/qemu/meson.build index 2279fef2ca..4c3e1dee78 100644 --- a/src/qemu/meson.build +++ b/src/qemu/meson.build @@ -137,6 +137,7 @@ if conf.has('WITH_QEMU') qemu_user_group_conf =3D configuration_data({ 'QEMU_USER': qemu_user, 'QEMU_GROUP': qemu_group, + 'USE_NBDKIT_DEFAULT': use_nbdkit_default.to_int(), }) qemu_conf =3D configure_file( input: 'qemu.conf.in', @@ -147,6 +148,7 @@ if conf.has('WITH_QEMU') qemu_user_group_hack_conf =3D configuration_data({ 'QEMU_USER': qemu_user, 'QEMU_GROUP': qemu_group, + 'USE_NBDKIT_DEFAULT': use_nbdkit_default.to_int(), # This hack is necessary because the output file is going to be # used as input for another configure_file() call later, which # will take care of substituting @CONFIG@ with useful data diff --git a/src/qemu/qemu.conf.in b/src/qemu/qemu.conf.in index 6897e0f760..d12ff9f641 100644 --- a/src/qemu/qemu.conf.in +++ b/src/qemu/qemu.conf.in @@ -974,3 +974,13 @@ # "full" - both QEMU and its helper processes are placed into separate # scheduling group #sched_core =3D "none" + +# Using nbdkit to access remote disk sources +# +# If this is set then libvirt will use nbdkit to access remote disk sources +# when available. nbdkit will export an NBD share to QEMU rather than havi= ng +# QEMU attempt to access the remote server directly. +# +# Possible values are 0 or 1. Default value is @USE_NBDKIT_DEFAULT@. +# +# storage_use_nbdkit =3D @USE_NBDKIT_DEFAULT@ diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index 513b5ebb1e..fbc32822ec 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -285,6 +285,7 @@ virQEMUDriverConfig *virQEMUDriverConfigNew(bool privil= eged, return NULL; =20 cfg->deprecationBehavior =3D g_strdup("none"); + cfg->storageUseNbdkit =3D USE_NBDKIT_DEFAULT; =20 return g_steal_pointer(&cfg); } @@ -1065,6 +1066,17 @@ virQEMUDriverConfigLoadCapsFiltersEntry(virQEMUDrive= rConfig *cfg, } =20 =20 +static int +virQEMUDriverConfigLoadStorageEntry(virQEMUDriverConfig *cfg, + virConf *conf) +{ + if (virConfGetValueBool(conf, "storage_use_nbdkit", &cfg->storageUseNb= dkit) < 0) + return -1; + + return 0; +} + + int virQEMUDriverConfigLoadFile(virQEMUDriverConfig *cfg, const char *filename, bool privileged) @@ -1136,6 +1148,9 @@ int virQEMUDriverConfigLoadFile(virQEMUDriverConfig *= cfg, if (virQEMUDriverConfigLoadCapsFiltersEntry(cfg, conf) < 0) return -1; =20 + if (virQEMUDriverConfigLoadStorageEntry(cfg, conf) < 0) + return -1; + return 0; } =20 diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h index 1a3ba3a0fb..36049b4bfa 100644 --- a/src/qemu/qemu_conf.h +++ b/src/qemu/qemu_conf.h @@ -230,6 +230,8 @@ struct _virQEMUDriverConfig { =20 char *deprecationBehavior; =20 + bool storageUseNbdkit; + virQEMUSchedCore schedCore; }; =20 diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index ae19ce884b..f8dda6c898 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -10333,6 +10333,9 @@ qemuDomainPrepareStorageSourceNbdkit(virStorageSour= ce *src, { g_autoptr(qemuNbdkitCaps) nbdkit =3D NULL; =20 + if (!cfg->storageUseNbdkit) + return false; + if (virStorageSourceGetActualType(src) !=3D VIR_STORAGE_TYPE_NETWORK) return false; =20 diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 24997c0aaa..e7a364f897 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -1115,7 +1115,6 @@ mymain(void) DO_TEST_CAPS_LATEST("disk-cdrom-empty-network-invalid"); DO_TEST_CAPS_LATEST("disk-cdrom-bus-other"); DO_TEST_CAPS_LATEST("disk-cdrom-network"); - DO_TEST_CAPS_LATEST_NBDKIT("disk-cdrom-network-nbdkit", QEMU_NBDKIT_CA= PS_PLUGIN_CURL); DO_TEST_CAPS_LATEST("disk-cdrom-tray"); DO_TEST_CAPS_LATEST("disk-floppy"); DO_TEST_CAPS_LATEST("disk-floppy-q35"); @@ -1161,8 +1160,6 @@ mymain(void) DO_TEST_CAPS_VER("disk-network-sheepdog", "6.0.0"); DO_TEST_CAPS_LATEST("disk-network-source-auth"); DO_TEST_CAPS_LATEST("disk-network-source-curl"); - DO_TEST_CAPS_LATEST_NBDKIT("disk-network-source-curl-nbdkit", QEMU_NBD= KIT_CAPS_PLUGIN_CURL); - DO_TEST_CAPS_LATEST_NBDKIT("disk-network-source-curl-nbdkit-backing", = QEMU_NBDKIT_CAPS_PLUGIN_CURL); DO_TEST_CAPS_LATEST("disk-network-nfs"); driver.config->vxhsTLS =3D 1; driver.config->nbdTLSx509secretUUID =3D g_strdup("6fd3f62d-9fe7-4a4e-a= 869-7acd6376d8ea"); @@ -1173,13 +1170,10 @@ mymain(void) DO_TEST_CAPS_LATEST("disk-network-tlsx509-nbd-hostname"); DO_TEST_CAPS_VER("disk-network-tlsx509-vxhs", "5.0.0"); DO_TEST_CAPS_LATEST("disk-network-http"); - DO_TEST_CAPS_LATEST_NBDKIT("disk-network-http-nbdkit", QEMU_NBDKIT_CAP= S_PLUGIN_CURL); VIR_FREE(driver.config->nbdTLSx509secretUUID); VIR_FREE(driver.config->vxhsTLSx509secretUUID); driver.config->vxhsTLS =3D 0; DO_TEST_CAPS_LATEST("disk-network-ssh"); - DO_TEST_CAPS_LATEST_NBDKIT("disk-network-ssh-nbdkit", QEMU_NBDKIT_CAPS= _PLUGIN_SSH); - DO_TEST_CAPS_LATEST_NBDKIT("disk-network-ssh-password", QEMU_NBDKIT_CA= PS_PLUGIN_SSH); DO_TEST_CAPS_LATEST("disk-no-boot"); DO_TEST_CAPS_LATEST("disk-nvme"); DO_TEST_CAPS_VER("disk-vhostuser-numa", "4.2.0"); @@ -1249,6 +1243,15 @@ mymain(void) DO_TEST_CAPS_LATEST("disk-geometry"); DO_TEST_CAPS_LATEST("disk-blockio"); =20 + driver.config->storageUseNbdkit =3D 1; + DO_TEST_CAPS_LATEST_NBDKIT("disk-cdrom-network-nbdkit", QEMU_NBDKIT_CA= PS_PLUGIN_CURL); + DO_TEST_CAPS_LATEST_NBDKIT("disk-network-source-curl-nbdkit", QEMU_NBD= KIT_CAPS_PLUGIN_CURL); + DO_TEST_CAPS_LATEST_NBDKIT("disk-network-source-curl-nbdkit-backing", = QEMU_NBDKIT_CAPS_PLUGIN_CURL); + DO_TEST_CAPS_LATEST_NBDKIT("disk-network-http-nbdkit", QEMU_NBDKIT_CAP= S_PLUGIN_CURL); + DO_TEST_CAPS_LATEST_NBDKIT("disk-network-ssh-nbdkit", QEMU_NBDKIT_CAPS= _PLUGIN_SSH); + DO_TEST_CAPS_LATEST_NBDKIT("disk-network-ssh-password", QEMU_NBDKIT_CA= PS_PLUGIN_SSH); + driver.config->storageUseNbdkit =3D 0; + DO_TEST_CAPS_VER("disk-virtio-scsi-reservations", "5.2.0"); DO_TEST_CAPS_LATEST("disk-virtio-scsi-reservations"); =20 --=20 2.41.0 _______________________________________________ Devel mailing list -- devel@lists.libvirt.org To unsubscribe send an email to devel-leave@lists.libvirt.org