Re: [PATCH] apparmor: let image label setting loop over backing files
On Tue, Jan 19, 2021 at 11:43 AM Peter Krempa wrote:
>
> On Tue, Jan 19, 2021 at 11:23:16 +0100, Christian Ehrhardt wrote:
> > When adding a rule for an image file and that image file has a chain
> > of backing files then we need to add a rule for each of those files.
> >
> > To get that iterate over the backing file chain the same way as
> > dac/selinux already do and add a label for each.
> >
> > Fixes: https://gitlab.com/libvirt/libvirt/-/issues/118
> >
> > Signed-off-by: Christian Ehrhardt
> > ---
> > src/security/security_apparmor.c | 39 ++--
> > 1 file changed, 27 insertions(+), 12 deletions(-)
> >
> > diff --git a/src/security/security_apparmor.c
> > b/src/security/security_apparmor.c
> > index 29f0956d22..1f309c0c9f 100644
> > --- a/src/security/security_apparmor.c
> > +++ b/src/security/security_apparmor.c
> > @@ -756,22 +756,13 @@ AppArmorRestoreInputLabel(virSecurityManagerPtr mgr,
> >
> > /* Called when hotplugging */
> > static int
> > -AppArmorSetSecurityImageLabel(virSecurityManagerPtr mgr,
> > - virDomainDefPtr def,
> > - virStorageSourcePtr src,
> > - virSecurityDomainImageLabelFlags flags
> > G_GNUC_UNUSED)
> > +AppArmorSetSecurityImageLabelInternal(virSecurityManagerPtr mgr,
> > + virDomainDefPtr def,
> > + virStorageSourcePtr src)
> > {
> > -virSecurityLabelDefPtr secdef;
> > g_autofree char *vfioGroupDev = NULL;
> > const char *path;
> >
> > -secdef = virDomainDefGetSecurityLabelDef(def, SECURITY_APPARMOR_NAME);
> > -if (!secdef || !secdef->relabel)
> > -return 0;
> > -
> > -if (!secdef->imagelabel)
> > -return 0;
> > -
> > if (src->type == VIR_STORAGE_TYPE_NVME) {
> > const virStorageSourceNVMeDef *nvme = src->nvme;
> >
> > @@ -797,6 +788,30 @@ AppArmorSetSecurityImageLabel(virSecurityManagerPtr
> > mgr,
> > return reload_profile(mgr, def, path, true);
> > }
> >
> > +static int
> > +AppArmorSetSecurityImageLabel(virSecurityManagerPtr mgr,
> > + virDomainDefPtr def,
> > + virStorageSourcePtr src,
> > + virSecurityDomainImageLabelFlags flags
> > G_GNUC_UNUSED)
> > +{
> > +virSecurityLabelDefPtr secdef;
> > +virStorageSourcePtr n;
> > +
> > +secdef = virDomainDefGetSecurityLabelDef(def, SECURITY_APPARMOR_NAME);
> > +if (!secdef || !secdef->relabel)
> > +return 0;
> > +
> > +if (!secdef->imagelabel)
> > +return 0;
>
> So apparmor doesn't support per-image security labels?
>
> > +
> > +for (n = src; virStorageSourceIsBacking(n); n = n->backingStore) {
> > +if (AppArmorSetSecurityImageLabelInternal(mgr, def, n) < 0)
>
> It feels a bit suboptimal to fork+exec the aahelper for every single
> image. The selinux/dac drivers collect the list of things to do before
> forking when we are in the transaction mode (or do just chown/selinux
> labelling, which is trivial)
>
> Given that this is usually on an expensive code path, it's probably okay
> for now though.
We are ok with the part above in the thread we have so far.
But I've realized that I've forgotten Jim on my initial submission.
@Jim Fehlig any ack/nack/comment on this before I consider pushing it
now that 7.0 is out?
> Reviewed-by: Peter Krempa
>
>
> > +return -1;
> > +}
> > +
> > +return 0;
> > +}
> > +
> > static int
> > AppArmorSecurityVerify(virSecurityManagerPtr mgr G_GNUC_UNUSED,
> > virDomainDefPtr def)
> > --
> > 2.30.0
> >
>
--
Christian Ehrhardt
Staff Engineer, Ubuntu Server
Canonical Ltd
Re: [PATCH] apparmor: let image label setting loop over backing files
On Tue, Jan 19, 2021 at 12:28 PM Peter Krempa wrote:
>
> On Tue, Jan 19, 2021 at 12:15:31 +0100, Christian Ehrhardt wrote:
> > On Tue, Jan 19, 2021 at 11:43 AM Peter Krempa wrote:
> > >
> > > On Tue, Jan 19, 2021 at 11:23:16 +0100, Christian Ehrhardt wrote:
> > > > When adding a rule for an image file and that image file has a chain
> > > > of backing files then we need to add a rule for each of those files.
> > > >
> > > > To get that iterate over the backing file chain the same way as
> > > > dac/selinux already do and add a label for each.
> > > >
> > > > Fixes: https://gitlab.com/libvirt/libvirt/-/issues/118
> > > >
> > > > Signed-off-by: Christian Ehrhardt
> > > > ---
> > > > src/security/security_apparmor.c | 39 ++--
> > > > 1 file changed, 27 insertions(+), 12 deletions(-)
> > > >
> > > > diff --git a/src/security/security_apparmor.c
> > > > b/src/security/security_apparmor.c
> > > > index 29f0956d22..1f309c0c9f 100644
> > > > --- a/src/security/security_apparmor.c
> > > > +++ b/src/security/security_apparmor.c
> > > > @@ -756,22 +756,13 @@ AppArmorRestoreInputLabel(virSecurityManagerPtr
> > > > mgr,
>
> [...]
>
> > > > +static int
> > > > +AppArmorSetSecurityImageLabel(virSecurityManagerPtr mgr,
> > > > + virDomainDefPtr def,
> > > > + virStorageSourcePtr src,
> > > > + virSecurityDomainImageLabelFlags flags
> > > > G_GNUC_UNUSED)
> > > > +{
> > > > +virSecurityLabelDefPtr secdef;
> > > > +virStorageSourcePtr n;
> > > > +
> > > > +secdef = virDomainDefGetSecurityLabelDef(def,
> > > > SECURITY_APPARMOR_NAME);
> > > > +if (!secdef || !secdef->relabel)
> > > > +return 0;
> > > > +
> > > > +if (!secdef->imagelabel)
> > > > +return 0;
> > >
> > > So apparmor doesn't support per-image security labels?
> >
> > This was present before, it just got moved as part of this change.
> > IIRC for apparmor that is only generated once in AppArmorGenSecurityLabel
> > and later on only used to check if the struct is ok (if it would be NULL
> > that
> > would indicate a non initialized element).
> >
> > If I'm missing some further hidden meaning of "imagelabel" please let me
> > know.
>
> Here secdef->imagelabel is a global per-VM label, but you can configure
> a per disk (or rather per-image) label with a element under
> . See:
>
> https://libvirt.org/formatdomain.html#hard-drives-floppy-disks-cdroms
>
> right the first example.
>
> This allows to control labelling of individual files, but I'm not sure
> if such concept makes sense for apparmor.
AFAIU the concept would not make much sense for apparmor.
The seclabel/relable config per source are useful to control
dac/selinux and if they put labels on the entity of "a path/file".
But in that sense apparmor isn't controlling attributes of the path at
all, it is more like a whitelist associated to the qemu-process.
> For now it seems to be
> ignored, maybe it should be at least validated if it doesn't make sense.
I don't yet see how it would fit, but I appreciate the discussion -
thanks Peter!
--
Christian Ehrhardt
Staff Engineer, Ubuntu Server
Canonical Ltd
[libvirt PATCH v2 3/3] ci: Drop openSUSE Leap 15.1
Leap 15.1 will reach EOL on January 31st 2021, so we should not test on it during the current development cycle ending in March 1st. Signed-off-by: Erik Skultety --- .gitlab-ci.yml | 17 +--- ci/containers/ci-opensuse-151.Dockerfile | 100 --- 2 files changed, 2 insertions(+), 115 deletions(-) delete mode 100644 ci/containers/ci-opensuse-151.Dockerfile diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 995ccc8ead..8949adf7a0 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -172,11 +172,6 @@ x64-fedora-rawhide-container: variables: NAME: fedora-rawhide -x64-opensuse-151-container: - extends: .container_job - variables: -NAME: opensuse-151 - x64-opensuse-152-container: extends: .container_job variables: @@ -382,14 +377,6 @@ x64-fedora-rawhide-clang: CC: clang RPM: skip -x64-opensuse-151: - extends: .native_build_job - needs: -- x64-opensuse-151-container - variables: -NAME: opensuse-151 -RPM: skip - x64-opensuse-152: extends: .native_build_job needs: @@ -558,9 +545,9 @@ website: codestyle: stage: builds - image: $CI_REGISTRY_IMAGE/ci-opensuse-151:latest + image: $CI_REGISTRY_IMAGE/ci-opensuse-152:latest needs: -- x64-opensuse-151-container +- x64-opensuse-152-container before_script: - *script_variables script: diff --git a/ci/containers/ci-opensuse-151.Dockerfile b/ci/containers/ci-opensuse-151.Dockerfile deleted file mode 100644 index 9458d2de0c..00 --- a/ci/containers/ci-opensuse-151.Dockerfile +++ /dev/null @@ -1,100 +0,0 @@ -# THIS FILE WAS AUTO-GENERATED -# -# $ lcitool dockerfile opensuse-151 libvirt -# -# https://gitlab.com/libvirt/libvirt-ci/-/commit/b098ec6631a85880f818f2dd25c437d509e53680 -FROM registry.opensuse.org/opensuse/leap:15.1 - -RUN zypper update -y && \ -zypper install -y \ - audit-devel \ - augeas \ - augeas-lenses \ - avahi-devel \ - bash-completion \ - ca-certificates \ - ccache \ - clang \ - cppi \ - cyrus-sasl-devel \ - dbus-1-devel \ - device-mapper-devel \ - diffutils \ - dnsmasq \ - dwarves \ - ebtables \ - fuse-devel \ - gcc \ - gettext \ - git \ - glib2-devel \ - glibc-devel \ - glibc-locale \ - glusterfs-devel \ - iproute2 \ - kmod \ - libacl-devel \ - libapparmor-devel \ - libattr-devel \ - libblkid-devel \ - libcap-ng-devel \ - libcurl-devel \ - libgnutls-devel \ - libiscsi-devel \ - libnl3-devel \ - libnuma-devel \ - libpcap-devel \ - libpciaccess-devel \ - librbd-devel \ - libselinux-devel \ - libssh-devel \ - libssh2-devel \ - libtirpc-devel \ - libudev-devel \ - libwsman-devel \ - libxml2 \ - libxml2-devel \ - libxslt \ - libyajl-devel \ - lvm2 \ - make \ - nfs-utils \ - ninja \ - numad \ - open-iscsi \ - parted \ - parted-devel \ - perl \ - pkgconfig \ - polkit \ - python3 \ - python3-docutils \ - python3-flake8 \ - python3-pip \ - python3-setuptools \ - python3-wheel \ - qemu-tools \ - radvd \ - readline-devel \ - rpcgen \ - rpm-build \ - sanlock-devel \ - scrub \ - systemtap-sdt-devel \ - wireshark-devel \ - xen-devel \ - xfsprogs-devel && \ -zypper clean --all && \ -rpm -qa | sort > /packages.txt && \ -mkdir -p /usr/libexec/ccache-wrappers && \ -ln -s /usr/bin/ccache /usr/libexec/ccache-wrappers/cc && \ -ln -s /usr/bin/ccache /usr/libexec/ccache-wrappers/$(basename /usr/bin/gcc) - -RUN pip3 install \ - meson==0.54.0 - -ENV LANG "en_US.UTF-8" -ENV MAKE "/usr/bin/make" -ENV NINJA "/usr/bin/ninja" -ENV PYTHON "/usr/bin/python3" -ENV CCACHE_WRAPPERSDIR "/usr/libexec/ccache-wrappers" -- 2.29.2
[libvirt PATCH v2 0/3] Refresh Dockerfiles
Erik Skultety (3):
ci: Refresh Dockerfiles
ci: Add openSUSE Leap 15.2
ci: Drop openSUSE Leap 15.1
.gitlab-ci.yml | 14 +++---
ci/containers/ci-centos-7.Dockerfile | 4 ++--
ci/containers/ci-centos-8.Dockerfile | 2 +-
ci/containers/ci-centos-stream.Dockerfile | 9 +
.../ci-debian-10-cross-aarch64.Dockerfile | 2 +-
ci/containers/ci-debian-10-cross-armv6l.Dockerfile | 2 +-
ci/containers/ci-debian-10-cross-armv7l.Dockerfile | 2 +-
ci/containers/ci-debian-10-cross-i686.Dockerfile | 2 +-
ci/containers/ci-debian-10-cross-mips.Dockerfile | 4 ++--
.../ci-debian-10-cross-mips64el.Dockerfile | 2 +-
ci/containers/ci-debian-10-cross-mipsel.Dockerfile | 2 +-
.../ci-debian-10-cross-ppc64le.Dockerfile | 2 +-
ci/containers/ci-debian-10-cross-s390x.Dockerfile | 4 ++--
ci/containers/ci-debian-10.Dockerfile | 2 +-
.../ci-debian-sid-cross-aarch64.Dockerfile | 2 +-
.../ci-debian-sid-cross-armv6l.Dockerfile | 2 +-
.../ci-debian-sid-cross-armv7l.Dockerfile | 2 +-
ci/containers/ci-debian-sid-cross-i686.Dockerfile | 2 +-
.../ci-debian-sid-cross-mips64el.Dockerfile| 2 +-
.../ci-debian-sid-cross-mipsel.Dockerfile | 2 +-
.../ci-debian-sid-cross-ppc64le.Dockerfile | 2 +-
ci/containers/ci-debian-sid-cross-s390x.Dockerfile | 4 ++--
ci/containers/ci-debian-sid.Dockerfile | 2 +-
ci/containers/ci-fedora-32.Dockerfile | 2 +-
ci/containers/ci-fedora-33.Dockerfile | 2 +-
.../ci-fedora-rawhide-cross-mingw32.Dockerfile | 2 +-
.../ci-fedora-rawhide-cross-mingw64.Dockerfile | 2 +-
ci/containers/ci-fedora-rawhide.Dockerfile | 2 +-
...e-151.Dockerfile =3D> ci-opensuse-152.Dockerfile} | 10 +-
ci/containers/ci-ubuntu-1804.Dockerfile| 2 +-
ci/containers/ci-ubuntu-2004.Dockerfile| 2 +-
31 files changed, 49 insertions(+), 48 deletions(-)
rename ci/containers/{ci-opensuse-151.Dockerfile =3D> ci-opensuse-152.Docker=
file} (91%)
--=20
2.29.2
[libvirt PATCH v2 1/3] ci: Refresh Dockerfiles
In this refresh CentOS 7 now uses docker.io registry and the PowerTools repo name regression was fixed for CentOS Stream this time. Signed-off-by: Erik Skultety --- ci/containers/ci-centos-7.Dockerfile | 4 ++-- ci/containers/ci-centos-8.Dockerfile | 2 +- ci/containers/ci-centos-stream.Dockerfile| 9 + ci/containers/ci-debian-10-cross-aarch64.Dockerfile | 2 +- ci/containers/ci-debian-10-cross-armv6l.Dockerfile | 2 +- ci/containers/ci-debian-10-cross-armv7l.Dockerfile | 2 +- ci/containers/ci-debian-10-cross-i686.Dockerfile | 2 +- ci/containers/ci-debian-10-cross-mips.Dockerfile | 4 ++-- ci/containers/ci-debian-10-cross-mips64el.Dockerfile | 2 +- ci/containers/ci-debian-10-cross-mipsel.Dockerfile | 2 +- ci/containers/ci-debian-10-cross-ppc64le.Dockerfile | 2 +- ci/containers/ci-debian-10-cross-s390x.Dockerfile| 4 ++-- ci/containers/ci-debian-10.Dockerfile| 2 +- ci/containers/ci-debian-sid-cross-aarch64.Dockerfile | 2 +- ci/containers/ci-debian-sid-cross-armv6l.Dockerfile | 2 +- ci/containers/ci-debian-sid-cross-armv7l.Dockerfile | 2 +- ci/containers/ci-debian-sid-cross-i686.Dockerfile| 2 +- ci/containers/ci-debian-sid-cross-mips64el.Dockerfile| 2 +- ci/containers/ci-debian-sid-cross-mipsel.Dockerfile | 2 +- ci/containers/ci-debian-sid-cross-ppc64le.Dockerfile | 2 +- ci/containers/ci-debian-sid-cross-s390x.Dockerfile | 4 ++-- ci/containers/ci-debian-sid.Dockerfile | 2 +- ci/containers/ci-fedora-32.Dockerfile| 2 +- ci/containers/ci-fedora-33.Dockerfile| 2 +- ci/containers/ci-fedora-rawhide-cross-mingw32.Dockerfile | 2 +- ci/containers/ci-fedora-rawhide-cross-mingw64.Dockerfile | 2 +- ci/containers/ci-fedora-rawhide.Dockerfile | 2 +- ci/containers/ci-ubuntu-1804.Dockerfile | 2 +- ci/containers/ci-ubuntu-2004.Dockerfile | 2 +- 29 files changed, 37 insertions(+), 36 deletions(-) diff --git a/ci/containers/ci-centos-7.Dockerfile b/ci/containers/ci-centos-7.Dockerfile index 39ec95ca55..a40b9d158c 100644 --- a/ci/containers/ci-centos-7.Dockerfile +++ b/ci/containers/ci-centos-7.Dockerfile @@ -2,8 +2,8 @@ # # $ lcitool dockerfile centos-7 libvirt # -# https://gitlab.com/libvirt/libvirt-ci/-/commit/b098ec6631a85880f818f2dd25c437d509e53680 -FROM registry.centos.org/centos:7 +# https://gitlab.com/libvirt/libvirt-ci/-/commit/d527e0c012f476c293f3bc801b7da08bc85f98ef +FROM docker.io/library/centos:7 RUN yum update -y && \ echo 'skip_missing_names_on_install=0' >> /etc/yum.conf && \ diff --git a/ci/containers/ci-centos-8.Dockerfile b/ci/containers/ci-centos-8.Dockerfile index 8f240d2a33..551401635f 100644 --- a/ci/containers/ci-centos-8.Dockerfile +++ b/ci/containers/ci-centos-8.Dockerfile @@ -2,7 +2,7 @@ # # $ lcitool dockerfile centos-8 libvirt # -# https://gitlab.com/libvirt/libvirt-ci/-/commit/b098ec6631a85880f818f2dd25c437d509e53680 +# https://gitlab.com/libvirt/libvirt-ci/-/commit/d527e0c012f476c293f3bc801b7da08bc85f98ef FROM docker.io/library/centos:8 RUN dnf update -y && \ diff --git a/ci/containers/ci-centos-stream.Dockerfile b/ci/containers/ci-centos-stream.Dockerfile index 141b378438..868e8a4486 100644 --- a/ci/containers/ci-centos-stream.Dockerfile +++ b/ci/containers/ci-centos-stream.Dockerfile @@ -2,13 +2,14 @@ # # $ lcitool dockerfile centos-stream libvirt # -# https://gitlab.com/libvirt/libvirt-ci/-/commit/b098ec6631a85880f818f2dd25c437d509e53680 +# https://gitlab.com/libvirt/libvirt-ci/-/commit/d527e0c012f476c293f3bc801b7da08bc85f98ef FROM docker.io/library/centos:8 -RUN dnf update -y && \ -dnf install -y centos-release-stream && \ +RUN dnf install -y centos-release-stream && \ +dnf install -y centos-stream-release && \ +dnf update -y && \ dnf install 'dnf-command(config-manager)' -y && \ -dnf config-manager --set-enabled -y Stream-PowerTools && \ +dnf config-manager --set-enabled -y powertools && \ dnf install -y centos-release-advanced-virtualization && \ dnf install -y epel-release && \ dnf install -y \ diff --git a/ci/containers/ci-debian-10-cross-aarch64.Dockerfile b/ci/containers/ci-debian-10-cross-aarch64.Dockerfile index a8ec0ad135..6ae1d3c390 100644 --- a/ci/containers/ci-debian-10-cross-aarch64.Dockerfile +++ b/ci/containers/ci-debian-10-cross-aarch64.Dockerfile @@ -2,7 +2,7 @@ # # $ lcitool dockerfile --cross aarch64 debian-10 libvirt # -# https://gitlab.com/libvirt/libvirt-ci/-/commit/b098ec6631a85880f818f2dd25c437d509e53680 +# https://gitlab.com/libvirt/libvirt-ci/-/commit/d527e0c012f476c293f3bc801b7da08bc85f98ef FROM docker.io/library/debian:10-slim RUN export DEBIAN_FRONTEND=noninteractive && \ diff --git a/ci/containers/ci-debian-10-cross-armv6l.Dockerfile
[libvirt PATCH v2 2/3] ci: Add openSUSE Leap 15.2
Signed-off-by: Erik Skultety --- .gitlab-ci.yml | 13 +++ ci/containers/ci-opensuse-152.Dockerfile | 100 +++ 2 files changed, 113 insertions(+) create mode 100644 ci/containers/ci-opensuse-152.Dockerfile diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 673327fb9e..995ccc8ead 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -177,6 +177,11 @@ x64-opensuse-151-container: variables: NAME: opensuse-151 +x64-opensuse-152-container: + extends: .container_job + variables: +NAME: opensuse-152 + x64-ubuntu-1804-container: extends: .container_job variables: @@ -385,6 +390,14 @@ x64-opensuse-151: NAME: opensuse-151 RPM: skip +x64-opensuse-152: + extends: .native_build_job + needs: +- x64-opensuse-152-container + variables: +NAME: opensuse-152 +RPM: skip + x64-ubuntu-1804: extends: .native_build_job needs: diff --git a/ci/containers/ci-opensuse-152.Dockerfile b/ci/containers/ci-opensuse-152.Dockerfile new file mode 100644 index 00..7f97c3ab70 --- /dev/null +++ b/ci/containers/ci-opensuse-152.Dockerfile @@ -0,0 +1,100 @@ +# THIS FILE WAS AUTO-GENERATED +# +# $ lcitool dockerfile opensuse-152 libvirt +# +# https://gitlab.com/libvirt/libvirt-ci/-/commit/d527e0c012f476c293f3bc801b7da08bc85f98ef +FROM registry.opensuse.org/opensuse/leap:15.2 + +RUN zypper update -y && \ +zypper install -y \ + audit-devel \ + augeas \ + augeas-lenses \ + avahi-devel \ + bash-completion \ + ca-certificates \ + ccache \ + clang \ + cppi \ + cyrus-sasl-devel \ + dbus-1-devel \ + device-mapper-devel \ + diffutils \ + dnsmasq \ + dwarves \ + ebtables \ + fuse-devel \ + gcc \ + gettext-runtime \ + git \ + glib2-devel \ + glibc-devel \ + glibc-locale \ + glusterfs-devel \ + iproute2 \ + kmod \ + libacl-devel \ + libapparmor-devel \ + libattr-devel \ + libblkid-devel \ + libcap-ng-devel \ + libcurl-devel \ + libgnutls-devel \ + libiscsi-devel \ + libnl3-devel \ + libnuma-devel \ + libpcap-devel \ + libpciaccess-devel \ + librbd-devel \ + libselinux-devel \ + libssh-devel \ + libssh2-devel \ + libtirpc-devel \ + libudev-devel \ + libwsman-devel \ + libxml2 \ + libxml2-devel \ + libxslt \ + libyajl-devel \ + lvm2 \ + make \ + nfs-utils \ + ninja \ + numad \ + open-iscsi \ + parted \ + parted-devel \ + perl \ + pkgconfig \ + polkit \ + python3-base \ + python3-docutils \ + python3-flake8 \ + python3-pip \ + python3-setuptools \ + python3-wheel \ + qemu-tools \ + radvd \ + readline-devel \ + rpcgen \ + rpm-build \ + sanlock-devel \ + scrub \ + systemtap-sdt-devel \ + wireshark-devel \ + xen-devel \ + xfsprogs-devel && \ +zypper clean --all && \ +rpm -qa | sort > /packages.txt && \ +mkdir -p /usr/libexec/ccache-wrappers && \ +ln -s /usr/bin/ccache /usr/libexec/ccache-wrappers/cc && \ +ln -s /usr/bin/ccache /usr/libexec/ccache-wrappers/$(basename /usr/bin/gcc) + +RUN pip3 install \ + meson==0.54.0 + +ENV LANG "en_US.UTF-8" +ENV MAKE "/usr/bin/make" +ENV NINJA "/usr/bin/ninja" +ENV PYTHON "/usr/bin/python3" +ENV CCACHE_WRAPPERSDIR "/usr/libexec/ccache-wrappers" -- 2.29.2
Re: [libvirt PATCH 0/2] Refresh Dockerfiles
On Tue, Jan 19, 2021 at 04:30:19PM +, Daniel P. Berrangé wrote: > On Tue, Jan 19, 2021 at 05:22:12PM +0100, Erik Skultety wrote: > > Note that openSUSE Leap 15.1 container hasn't been removed yet as it reaches > > EOL on January 31, 2021, we'll build on both. > > We should remove it right now, because it'll be EOL by the time of > the next libvirt release on March 1st, so there's no need to test > code on it during this dev cycle. Fair enough, I'll remove it then. Erik
[PATCH v2] meson: build vstorage only on linux
This should fix CI error:
../dist-unpack/libvirt-7.1.0/src/storage/storage_backend_vstorage.c:10:10:
fatal error: 'mntent.h' file not found
#include
^~
on freebsd and mac.
Signed-off-by: Nikolay Shirokovskiy
---
meson.build | 15 +--
1 file changed, 13 insertions(+), 2 deletions(-)
diff --git a/meson.build b/meson.build
index e3e7ff7..d8a63ba 100644
--- a/meson.build
+++ b/meson.build
@@ -1957,8 +1957,19 @@ if conf.has('WITH_LIBVIRTD')
endif
if not get_option('storage_vstorage').disabled()
-use_storage = true
-conf.set('WITH_STORAGE_VSTORAGE', 1)
+vstorage_enable = true
+if host_machine.system() != 'linux'
+ if get_option('storage_fs').enabled()
+error('Vstorage is supported only on Linux')
+ else
+vstorage_enable = false
+ endif
+endif
+
+if vstorage_enable
+ use_storage = true
+ conf.set('WITH_STORAGE_VSTORAGE', 1)
+endif
endif
if not get_option('storage_zfs').disabled()
--
1.8.3.1
Re: [PATCH] meson: don't build vstorage where mntent.h is not present
On Tue, Jan 19, 2021 at 5:59 PM Pavel Hrdina wrote:
> On Tue, Jan 19, 2021 at 04:41:35PM +0300, Nikolay Shirokovskiy wrote:
> > This should fix CI error:
> >
> >
> ../dist-unpack/libvirt-7.1.0/src/storage/storage_backend_vstorage.c:10:10:
> fatal error: 'mntent.h' file not found
> > #include
> > ^~
> >
> > on freebsd and mac.
> >
> > Signed-off-by: Nikolay Shirokovskiy
> > ---
> > meson.build | 16 ++--
> > 1 file changed, 14 insertions(+), 2 deletions(-)
> >
> > diff --git a/meson.build b/meson.build
> > index e3e7ff7..a6b6169 100644
> > --- a/meson.build
> > +++ b/meson.build
> > @@ -1957,8 +1957,20 @@ if conf.has('WITH_LIBVIRTD')
> >endif
> >
> >if not get_option('storage_vstorage').disabled()
> > -use_storage = true
> > -conf.set('WITH_STORAGE_VSTORAGE', 1)
> > +vstorage_enable = true
> > +
> > +if not cc.has_header('mntent.h')
>
> This makes me question if it makes sense to build vstorage for anything
> else then linux? It looks like that on FreeBSD or macOS it will be never
> enabled and we already disable libvirtd and all storage drivers for
> windows so we might as well make this condition
>
> if host_machine.system() != 'linux'
>
> and claim that vstorage is supported only on linux.
>
> I see that the check is inspired by FS storage driver but if mntent.h is
> not available or difficult to get on FreeBSD or macOS we could make it
> easier for users instead of having them trying to get mntent.h.
>
Ok.
>
> > + if get_option('storage_fs').enabled()
> > +error(' is required for the FS storage driver')
>
> This should probably say "Virtuozzo storage driver".
>
>
Yep, fixing CI is a bit of a hurry :)
Nikolay
>
> > + else
> > +vstorage_enable = false
> > + endif
> > +endif
> > +
> > +if vstorage_enable
> > + use_storage = true
> > + conf.set('WITH_STORAGE_VSTORAGE', 1)
> > +endif
> >endif
> >
> >if not get_option('storage_zfs').disabled()
> > --
> > 1.8.3.1
> >
>
[PATCH] meson: don't build vstorage where mntent.h is not present
This should fix CI error:
../dist-unpack/libvirt-7.1.0/src/storage/storage_backend_vstorage.c:10:10:
fatal error: 'mntent.h' file not found
#include
^~
on freebsd and mac.
Signed-off-by: Nikolay Shirokovskiy
---
meson.build | 16 ++--
1 file changed, 14 insertions(+), 2 deletions(-)
diff --git a/meson.build b/meson.build
index e3e7ff7..a6b6169 100644
--- a/meson.build
+++ b/meson.build
@@ -1957,8 +1957,20 @@ if conf.has('WITH_LIBVIRTD')
endif
if not get_option('storage_vstorage').disabled()
-use_storage = true
-conf.set('WITH_STORAGE_VSTORAGE', 1)
+vstorage_enable = true
+
+if not cc.has_header('mntent.h')
+ if get_option('storage_fs').enabled()
+error(' is required for the FS storage driver')
+ else
+vstorage_enable = false
+ endif
+endif
+
+if vstorage_enable
+ use_storage = true
+ conf.set('WITH_STORAGE_VSTORAGE', 1)
+endif
endif
if not get_option('storage_zfs').disabled()
--
1.8.3.1
Re: [PATCH v2] meson: build vstorage only on linux
On Tue, Jan 19, 2021 at 07:31:42PM +0300, Nikolay Shirokovskiy wrote: > This should fix CI error: > > > ../dist-unpack/libvirt-7.1.0/src/storage/storage_backend_vstorage.c:10:10: > fatal error: 'mntent.h' file not found > #include > ^~ > > on freebsd and mac. > > Signed-off-by: Nikolay Shirokovskiy > --- > meson.build | 15 +-- > 1 file changed, 13 insertions(+), 2 deletions(-) Reviewed-by: Daniel P. Berrangé 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 :|
Re: [libvirt PATCH 0/2] Refresh Dockerfiles
On Tue, Jan 19, 2021 at 05:22:12PM +0100, Erik Skultety wrote: > Note that openSUSE Leap 15.1 container hasn't been removed yet as it reaches > EOL on January 31, 2021, we'll build on both. We should remove it right now, because it'll be EOL by the time of the next libvirt release on March 1st, so there's no need to test code on it during this dev cycle. 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 :|
[libvirt PATCH 2/2] ci: Add openSUSE Leap 15.2
Flip the codestyle job to this minor version from Leap 15.1 as well. Signed-off-by: Erik Skultety --- .gitlab-ci.yml | 17 +++- ci/containers/ci-opensuse-152.Dockerfile | 100 +++ 2 files changed, 115 insertions(+), 2 deletions(-) create mode 100644 ci/containers/ci-opensuse-152.Dockerfile diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 673327fb9e..8a8092390e 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -177,6 +177,11 @@ x64-opensuse-151-container: variables: NAME: opensuse-151 +x64-opensuse-152-container: + extends: .container_job + variables: +NAME: opensuse-152 + x64-ubuntu-1804-container: extends: .container_job variables: @@ -385,6 +390,14 @@ x64-opensuse-151: NAME: opensuse-151 RPM: skip +x64-opensuse-152: + extends: .native_build_job + needs: +- x64-opensuse-152-container + variables: +NAME: opensuse-152 +RPM: skip + x64-ubuntu-1804: extends: .native_build_job needs: @@ -545,9 +558,9 @@ website: codestyle: stage: builds - image: $CI_REGISTRY_IMAGE/ci-opensuse-151:latest + image: $CI_REGISTRY_IMAGE/ci-opensuse-152:latest needs: -- x64-opensuse-151-container +- x64-opensuse-152-container before_script: - *script_variables script: diff --git a/ci/containers/ci-opensuse-152.Dockerfile b/ci/containers/ci-opensuse-152.Dockerfile new file mode 100644 index 00..7f97c3ab70 --- /dev/null +++ b/ci/containers/ci-opensuse-152.Dockerfile @@ -0,0 +1,100 @@ +# THIS FILE WAS AUTO-GENERATED +# +# $ lcitool dockerfile opensuse-152 libvirt +# +# https://gitlab.com/libvirt/libvirt-ci/-/commit/d527e0c012f476c293f3bc801b7da08bc85f98ef +FROM registry.opensuse.org/opensuse/leap:15.2 + +RUN zypper update -y && \ +zypper install -y \ + audit-devel \ + augeas \ + augeas-lenses \ + avahi-devel \ + bash-completion \ + ca-certificates \ + ccache \ + clang \ + cppi \ + cyrus-sasl-devel \ + dbus-1-devel \ + device-mapper-devel \ + diffutils \ + dnsmasq \ + dwarves \ + ebtables \ + fuse-devel \ + gcc \ + gettext-runtime \ + git \ + glib2-devel \ + glibc-devel \ + glibc-locale \ + glusterfs-devel \ + iproute2 \ + kmod \ + libacl-devel \ + libapparmor-devel \ + libattr-devel \ + libblkid-devel \ + libcap-ng-devel \ + libcurl-devel \ + libgnutls-devel \ + libiscsi-devel \ + libnl3-devel \ + libnuma-devel \ + libpcap-devel \ + libpciaccess-devel \ + librbd-devel \ + libselinux-devel \ + libssh-devel \ + libssh2-devel \ + libtirpc-devel \ + libudev-devel \ + libwsman-devel \ + libxml2 \ + libxml2-devel \ + libxslt \ + libyajl-devel \ + lvm2 \ + make \ + nfs-utils \ + ninja \ + numad \ + open-iscsi \ + parted \ + parted-devel \ + perl \ + pkgconfig \ + polkit \ + python3-base \ + python3-docutils \ + python3-flake8 \ + python3-pip \ + python3-setuptools \ + python3-wheel \ + qemu-tools \ + radvd \ + readline-devel \ + rpcgen \ + rpm-build \ + sanlock-devel \ + scrub \ + systemtap-sdt-devel \ + wireshark-devel \ + xen-devel \ + xfsprogs-devel && \ +zypper clean --all && \ +rpm -qa | sort > /packages.txt && \ +mkdir -p /usr/libexec/ccache-wrappers && \ +ln -s /usr/bin/ccache /usr/libexec/ccache-wrappers/cc && \ +ln -s /usr/bin/ccache /usr/libexec/ccache-wrappers/$(basename /usr/bin/gcc) + +RUN pip3 install \ + meson==0.54.0 + +ENV LANG "en_US.UTF-8" +ENV MAKE "/usr/bin/make" +ENV NINJA "/usr/bin/ninja" +ENV PYTHON "/usr/bin/python3" +ENV CCACHE_WRAPPERSDIR "/usr/libexec/ccache-wrappers" -- 2.29.2
[libvirt PATCH 1/2] ci: Refresh Dockerfiles
In this refresh CentOS 7 now uses docker.io registry and the PowerTools repo name regression was fixed for CentOS Stream this time. Signed-off-by: Erik Skultety --- ci/containers/ci-centos-7.Dockerfile | 4 ++-- ci/containers/ci-centos-8.Dockerfile | 2 +- ci/containers/ci-centos-stream.Dockerfile| 9 + ci/containers/ci-debian-10-cross-aarch64.Dockerfile | 2 +- ci/containers/ci-debian-10-cross-armv6l.Dockerfile | 2 +- ci/containers/ci-debian-10-cross-armv7l.Dockerfile | 2 +- ci/containers/ci-debian-10-cross-i686.Dockerfile | 2 +- ci/containers/ci-debian-10-cross-mips.Dockerfile | 4 ++-- ci/containers/ci-debian-10-cross-mips64el.Dockerfile | 2 +- ci/containers/ci-debian-10-cross-mipsel.Dockerfile | 2 +- ci/containers/ci-debian-10-cross-ppc64le.Dockerfile | 2 +- ci/containers/ci-debian-10-cross-s390x.Dockerfile| 4 ++-- ci/containers/ci-debian-10.Dockerfile| 2 +- ci/containers/ci-debian-sid-cross-aarch64.Dockerfile | 2 +- ci/containers/ci-debian-sid-cross-armv6l.Dockerfile | 2 +- ci/containers/ci-debian-sid-cross-armv7l.Dockerfile | 2 +- ci/containers/ci-debian-sid-cross-i686.Dockerfile| 2 +- ci/containers/ci-debian-sid-cross-mips64el.Dockerfile| 2 +- ci/containers/ci-debian-sid-cross-mipsel.Dockerfile | 2 +- ci/containers/ci-debian-sid-cross-ppc64le.Dockerfile | 2 +- ci/containers/ci-debian-sid-cross-s390x.Dockerfile | 4 ++-- ci/containers/ci-debian-sid.Dockerfile | 2 +- ci/containers/ci-fedora-32.Dockerfile| 2 +- ci/containers/ci-fedora-33.Dockerfile| 2 +- ci/containers/ci-fedora-rawhide-cross-mingw32.Dockerfile | 2 +- ci/containers/ci-fedora-rawhide-cross-mingw64.Dockerfile | 2 +- ci/containers/ci-fedora-rawhide.Dockerfile | 2 +- ci/containers/ci-ubuntu-1804.Dockerfile | 2 +- ci/containers/ci-ubuntu-2004.Dockerfile | 2 +- 29 files changed, 37 insertions(+), 36 deletions(-) diff --git a/ci/containers/ci-centos-7.Dockerfile b/ci/containers/ci-centos-7.Dockerfile index 39ec95ca55..a40b9d158c 100644 --- a/ci/containers/ci-centos-7.Dockerfile +++ b/ci/containers/ci-centos-7.Dockerfile @@ -2,8 +2,8 @@ # # $ lcitool dockerfile centos-7 libvirt # -# https://gitlab.com/libvirt/libvirt-ci/-/commit/b098ec6631a85880f818f2dd25c437d509e53680 -FROM registry.centos.org/centos:7 +# https://gitlab.com/libvirt/libvirt-ci/-/commit/d527e0c012f476c293f3bc801b7da08bc85f98ef +FROM docker.io/library/centos:7 RUN yum update -y && \ echo 'skip_missing_names_on_install=0' >> /etc/yum.conf && \ diff --git a/ci/containers/ci-centos-8.Dockerfile b/ci/containers/ci-centos-8.Dockerfile index 8f240d2a33..551401635f 100644 --- a/ci/containers/ci-centos-8.Dockerfile +++ b/ci/containers/ci-centos-8.Dockerfile @@ -2,7 +2,7 @@ # # $ lcitool dockerfile centos-8 libvirt # -# https://gitlab.com/libvirt/libvirt-ci/-/commit/b098ec6631a85880f818f2dd25c437d509e53680 +# https://gitlab.com/libvirt/libvirt-ci/-/commit/d527e0c012f476c293f3bc801b7da08bc85f98ef FROM docker.io/library/centos:8 RUN dnf update -y && \ diff --git a/ci/containers/ci-centos-stream.Dockerfile b/ci/containers/ci-centos-stream.Dockerfile index 141b378438..868e8a4486 100644 --- a/ci/containers/ci-centos-stream.Dockerfile +++ b/ci/containers/ci-centos-stream.Dockerfile @@ -2,13 +2,14 @@ # # $ lcitool dockerfile centos-stream libvirt # -# https://gitlab.com/libvirt/libvirt-ci/-/commit/b098ec6631a85880f818f2dd25c437d509e53680 +# https://gitlab.com/libvirt/libvirt-ci/-/commit/d527e0c012f476c293f3bc801b7da08bc85f98ef FROM docker.io/library/centos:8 -RUN dnf update -y && \ -dnf install -y centos-release-stream && \ +RUN dnf install -y centos-release-stream && \ +dnf install -y centos-stream-release && \ +dnf update -y && \ dnf install 'dnf-command(config-manager)' -y && \ -dnf config-manager --set-enabled -y Stream-PowerTools && \ +dnf config-manager --set-enabled -y powertools && \ dnf install -y centos-release-advanced-virtualization && \ dnf install -y epel-release && \ dnf install -y \ diff --git a/ci/containers/ci-debian-10-cross-aarch64.Dockerfile b/ci/containers/ci-debian-10-cross-aarch64.Dockerfile index a8ec0ad135..6ae1d3c390 100644 --- a/ci/containers/ci-debian-10-cross-aarch64.Dockerfile +++ b/ci/containers/ci-debian-10-cross-aarch64.Dockerfile @@ -2,7 +2,7 @@ # # $ lcitool dockerfile --cross aarch64 debian-10 libvirt # -# https://gitlab.com/libvirt/libvirt-ci/-/commit/b098ec6631a85880f818f2dd25c437d509e53680 +# https://gitlab.com/libvirt/libvirt-ci/-/commit/d527e0c012f476c293f3bc801b7da08bc85f98ef FROM docker.io/library/debian:10-slim RUN export DEBIAN_FRONTEND=noninteractive && \ diff --git a/ci/containers/ci-debian-10-cross-armv6l.Dockerfile
[libvirt PATCH 0/2] Refresh Dockerfiles
Note that openSUSE Leap 15.1 container hasn't been removed yet as it reaches EOL on January 31, 2021, we'll build on both. Erik Skultety (2): ci: Refresh Dockerfiles ci: Add openSUSE Leap 15.2 .gitlab-ci.yml| 17 ++- ci/containers/ci-centos-7.Dockerfile | 4 +- ci/containers/ci-centos-8.Dockerfile | 2 +- ci/containers/ci-centos-stream.Dockerfile | 9 +- .../ci-debian-10-cross-aarch64.Dockerfile | 2 +- .../ci-debian-10-cross-armv6l.Dockerfile | 2 +- .../ci-debian-10-cross-armv7l.Dockerfile | 2 +- .../ci-debian-10-cross-i686.Dockerfile| 2 +- .../ci-debian-10-cross-mips.Dockerfile| 4 +- .../ci-debian-10-cross-mips64el.Dockerfile| 2 +- .../ci-debian-10-cross-mipsel.Dockerfile | 2 +- .../ci-debian-10-cross-ppc64le.Dockerfile | 2 +- .../ci-debian-10-cross-s390x.Dockerfile | 4 +- ci/containers/ci-debian-10.Dockerfile | 2 +- .../ci-debian-sid-cross-aarch64.Dockerfile| 2 +- .../ci-debian-sid-cross-armv6l.Dockerfile | 2 +- .../ci-debian-sid-cross-armv7l.Dockerfile | 2 +- .../ci-debian-sid-cross-i686.Dockerfile | 2 +- .../ci-debian-sid-cross-mips64el.Dockerfile | 2 +- .../ci-debian-sid-cross-mipsel.Dockerfile | 2 +- .../ci-debian-sid-cross-ppc64le.Dockerfile| 2 +- .../ci-debian-sid-cross-s390x.Dockerfile | 4 +- ci/containers/ci-debian-sid.Dockerfile| 2 +- ci/containers/ci-fedora-32.Dockerfile | 2 +- ci/containers/ci-fedora-33.Dockerfile | 2 +- ...ci-fedora-rawhide-cross-mingw32.Dockerfile | 2 +- ...ci-fedora-rawhide-cross-mingw64.Dockerfile | 2 +- ci/containers/ci-fedora-rawhide.Dockerfile| 2 +- ci/containers/ci-opensuse-152.Dockerfile | 100 ++ ci/containers/ci-ubuntu-1804.Dockerfile | 2 +- ci/containers/ci-ubuntu-2004.Dockerfile | 2 +- 31 files changed, 152 insertions(+), 38 deletions(-) create mode 100644 ci/containers/ci-opensuse-152.Dockerfile -- 2.29.2
Re: [libvirt PATCH v2] meson: Fix build with -Dtest_coverage=true
On Tue, Jan 19, 2021 at 03:31:56PM +0100, Jiri Denemark wrote: > As can be seen in commit 8a62a1592ae00eab4eb153c02661e56b9d8d9032 (from > autoconf era), the coverage flags have to be used also when linking > objects. However, this was not reflected when we switched to meson. > > Without this patch linking fails with undefined references to various > __gcov_* symbols. > > Signed-off-by: Jiri Denemark > --- > tests/meson.build | 8 > tools/nss/meson.build | 2 ++ > tools/wireshark/src/meson.build | 3 +++ > 3 files changed, 9 insertions(+), 4 deletions(-) Reviewed-by: Pavel Hrdina signature.asc Description: PGP signature
Re: [PATCH] meson: don't build vstorage where mntent.h is not present
On Tue, Jan 19, 2021 at 04:41:35PM +0300, Nikolay Shirokovskiy wrote:
> This should fix CI error:
>
>
> ../dist-unpack/libvirt-7.1.0/src/storage/storage_backend_vstorage.c:10:10:
> fatal error: 'mntent.h' file not found
> #include
> ^~
>
> on freebsd and mac.
>
> Signed-off-by: Nikolay Shirokovskiy
> ---
> meson.build | 16 ++--
> 1 file changed, 14 insertions(+), 2 deletions(-)
>
> diff --git a/meson.build b/meson.build
> index e3e7ff7..a6b6169 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -1957,8 +1957,20 @@ if conf.has('WITH_LIBVIRTD')
>endif
>
>if not get_option('storage_vstorage').disabled()
> -use_storage = true
> -conf.set('WITH_STORAGE_VSTORAGE', 1)
> +vstorage_enable = true
> +
> +if not cc.has_header('mntent.h')
This makes me question if it makes sense to build vstorage for anything
else then linux? It looks like that on FreeBSD or macOS it will be never
enabled and we already disable libvirtd and all storage drivers for
windows so we might as well make this condition
if host_machine.system() != 'linux'
and claim that vstorage is supported only on linux.
I see that the check is inspired by FS storage driver but if mntent.h is
not available or difficult to get on FreeBSD or macOS we could make it
easier for users instead of having them trying to get mntent.h.
> + if get_option('storage_fs').enabled()
> +error(' is required for the FS storage driver')
This should probably say "Virtuozzo storage driver".
Pavel
> + else
> +vstorage_enable = false
> + endif
> +endif
> +
> +if vstorage_enable
> + use_storage = true
> + conf.set('WITH_STORAGE_VSTORAGE', 1)
> +endif
>endif
>
>if not get_option('storage_zfs').disabled()
> --
> 1.8.3.1
>
signature.asc
Description: PGP signature
Re: [libvirt PATCH] docs: Clarify use of virtio-scsi model for SCSI controllers
On Tue, Jan 19, 2021 at 15:38:40 +0100, Andrea Bolognani wrote: > The current formulation can lead people to believe SCSI > controllers only allow the virtio-scsi model, but really the > only difference is that you have to use model='virtio-scsi' > where you would use model='virtio' for another device. > > Signed-off-by: Andrea Bolognani > --- > docs/formatdomain.rst | 6 -- > 1 file changed, 4 insertions(+), 2 deletions(-) > > diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst > index dd197d8f6a..af540391db 100644 > --- a/docs/formatdomain.rst > +++ b/docs/formatdomain.rst > @@ -3428,8 +3428,10 @@ machine types, accept the following ``model`` values: > While the information outlined above applies to most virtio devices, there > are a > few exceptions: > > -- for SCSI controllers, ``virtio-scsi`` must be used instead of ``virtio`` > for > - backwards compatibility reasons; > +- for SCSI controllers, there is no ``virtio`` model available due to > + historical reasons: use ``virtio-scsi`` instead, which behaves the same as > + ``virtio`` does for other devices. Both ``virtio-transitional`` and > + ``virtio-non-transitional`` work with SCSI controllers; > - some devices, such as GPUs and input devices (keyboard, tablet and mouse), > are only defined in the virtio 1.0 spec and as such don't have a > transitional > variant: the only accepted model is ``virtio``, which will result in a Reviewed-by: Jiri Denemark
[libvirt PATCH] docs: Clarify use of virtio-scsi model for SCSI controllers
The current formulation can lead people to believe SCSI controllers only allow the virtio-scsi model, but really the only difference is that you have to use model='virtio-scsi' where you would use model='virtio' for another device. Signed-off-by: Andrea Bolognani --- docs/formatdomain.rst | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst index dd197d8f6a..af540391db 100644 --- a/docs/formatdomain.rst +++ b/docs/formatdomain.rst @@ -3428,8 +3428,10 @@ machine types, accept the following ``model`` values: While the information outlined above applies to most virtio devices, there are a few exceptions: -- for SCSI controllers, ``virtio-scsi`` must be used instead of ``virtio`` for - backwards compatibility reasons; +- for SCSI controllers, there is no ``virtio`` model available due to + historical reasons: use ``virtio-scsi`` instead, which behaves the same as + ``virtio`` does for other devices. Both ``virtio-transitional`` and + ``virtio-non-transitional`` work with SCSI controllers; - some devices, such as GPUs and input devices (keyboard, tablet and mouse), are only defined in the virtio 1.0 spec and as such don't have a transitional variant: the only accepted model is ``virtio``, which will result in a -- 2.26.2
[libvirt PATCH v2] meson: Fix build with -Dtest_coverage=true
As can be seen in commit 8a62a1592ae00eab4eb153c02661e56b9d8d9032 (from autoconf era), the coverage flags have to be used also when linking objects. However, this was not reflected when we switched to meson. Without this patch linking fails with undefined references to various __gcov_* symbols. Signed-off-by: Jiri Denemark --- tests/meson.build | 8 tools/nss/meson.build | 2 ++ tools/wireshark/src/meson.build | 3 +++ 3 files changed, 9 insertions(+), 4 deletions(-) diff --git a/tests/meson.build b/tests/meson.build index f1d91ca50d..23255dd62a 100644 --- a/tests/meson.build +++ b/tests/meson.build @@ -26,7 +26,10 @@ tests_dep = declare_dependency( top_inc_dir, util_inc_dir, ], - link_args: libvirt_export_dynamic, + link_args: ( +libvirt_export_dynamic ++ coverage_flags + ), ) tests_env = [ @@ -228,9 +231,6 @@ executable( dependencies: [ tests_dep, ], - link_args: [ -coverage_flags, - ], ) diff --git a/tools/nss/meson.build b/tools/nss/meson.build index cf3eec9b24..198936f3d4 100644 --- a/tools/nss/meson.build +++ b/tools/nss/meson.build @@ -66,6 +66,7 @@ nss_libvirt_lib = shared_module( link_args: [ nss_libvirt_syms, libvirt_export_dynamic, +coverage_flags, ], link_whole: [ nss_libvirt_impl, @@ -81,6 +82,7 @@ nss_libvirt_guest_lib = shared_library( link_args: [ nss_libvirt_guest_syms, libvirt_export_dynamic, +coverage_flags, ], link_whole: [ nss_libvirt_guest_impl, diff --git a/tools/wireshark/src/meson.build b/tools/wireshark/src/meson.build index 49ccc9bb86..9b452dc5ca 100644 --- a/tools/wireshark/src/meson.build +++ b/tools/wireshark/src/meson.build @@ -12,6 +12,9 @@ shared_library( xdr_dep, tools_dep, ], + link_args: [ +coverage_flags + ], install: true, install_dir: wireshark_plugindir, ) -- 2.30.0
Re: [libvirt PATCH] meson: Fix build with -Dtest_coverage=true
On Mon, Jan 18, 2021 at 09:56:56 +0100, Pavel Hrdina wrote:
> On Mon, Jan 18, 2021 at 09:23:34AM +0100, Jiri Denemark wrote:
> > As can be seen in commit 8a62a1592ae00eab4eb153c02661e56b9d8d9032 (from
> > autoconf era), the coverage flags have to be used also when linking
> > objects. However, this was not reflected when we switched to meson.
> >
> > Signed-off-by: Jiri Denemark
> > ---
> > src/meson.build | 1 +
> > tests/meson.build | 8
> > tools/nss/meson.build | 2 ++
> > tools/wireshark/src/meson.build | 3 +++
> > 4 files changed, 14 insertions(+)
> >
> > diff --git a/src/meson.build b/src/meson.build
> > index 7c478219d6..980578d5d6 100644
> > --- a/src/meson.build
> > +++ b/src/meson.build
> > @@ -21,6 +21,7 @@ src_dep = declare_dependency(
> > + coverage_flags
> > + driver_modules_flags
> > + win32_link_flags
> > ++ coverage_flags
>
> You can see that it is already included few lines above.
Well, clearly I didn't see it :-)
> >),
> > )
> >
> > diff --git a/tests/meson.build b/tests/meson.build
> > index f1d91ca50d..c65487f5c2 100644
> > --- a/tests/meson.build
> > +++ b/tests/meson.build
> > @@ -202,6 +202,9 @@ foreach mock : mock_libs
> >libvirt_lib,
> >mock.get('link_with', []),
> > ],
> > +link_args: [
> > + coverage_flags,
> > +],
> >)
> > endforeach
> >
> > @@ -218,6 +221,7 @@ executable(
> >],
> >link_args: [
> > libvirt_no_indirect,
> > +coverage_flags
> >],
> > )
> >
> > @@ -566,6 +570,7 @@ foreach data : tests
> > ],
> > link_args: [
> >libvirt_no_indirect,
> > + coverage_flags,
> > ],
> > link_with: [
> >libvirt_lib,
> > @@ -644,6 +649,9 @@ foreach data : helpers
> > link_with: [
> >data['link_with'],
> > ],
> > +link_args: [
> > + coverage_flags,
> > +],
> > export_dynamic: true,
> >)
> > endforeach
>
> This should not be needed as well because coverage_flags is part of
> tests_dep in "compile_args" which is the same as CFLAGS in automake.
>
> The only place that uses COVERAGE_LDFLAGS is our fake SSH binary which
> should be translated to link_args and we already do that.
>
> If it has to be included when linking then we should remove the usage of
> coverage_flags from the fake SSH and add it into tests_dep as link_args
> in addition to compile_args.
Hmm, good point. It's needed everywhere, otherwise linking fails with
undefined references to __gcov_*. I moved it to tests_dep.
> > diff --git a/tools/nss/meson.build b/tools/nss/meson.build
> > index cf3eec9b24..198936f3d4 100644
> > --- a/tools/nss/meson.build
> > +++ b/tools/nss/meson.build
> > @@ -66,6 +66,7 @@ nss_libvirt_lib = shared_module(
> >link_args: [
> > nss_libvirt_syms,
> > libvirt_export_dynamic,
> > +coverage_flags,
> >],
> >link_whole: [
> > nss_libvirt_impl,
> > @@ -81,6 +82,7 @@ nss_libvirt_guest_lib = shared_library(
> >link_args: [
> > nss_libvirt_guest_syms,
> > libvirt_export_dynamic,
> > +coverage_flags,
> >],
> >link_whole: [
> > nss_libvirt_guest_impl,
> > diff --git a/tools/wireshark/src/meson.build
> > b/tools/wireshark/src/meson.build
> > index 49ccc9bb86..9b452dc5ca 100644
> > --- a/tools/wireshark/src/meson.build
> > +++ b/tools/wireshark/src/meson.build
> > @@ -12,6 +12,9 @@ shared_library(
> > xdr_dep,
> > tools_dep,
> >],
> > + link_args: [
> > +coverage_flags
> > + ],
> >install: true,
> >install_dir: wireshark_plugindir,
> > )
>
> The commit mentioned doesn't add COVERAGE_LDFLAGS into nss or wireshark
> libs and checking our code before the meson switch we don't have it
> there as well. It's not even in AM_LDFLAGS so looks like preexisting.
Coverage builds were working before the meson rewrite, while linking
fails without explicitly adding coverage_flags to link_args for nss and
wireshark libs now. I guess it must have been done via some libtool
dependency magic which brought the right flags from the internal libs
we're linking with.
Jirka
Re: [PATCH] storage: Linstor support
On Mon, Jan 18, 2021 at 02:12:04PM +0100, Peter Krempa wrote: > On Fri, Jan 15, 2021 at 08:16:47 +0100, Rene Peinthor wrote: > > Implement a LINSTOR backend storage driver. > > The Linstor client needs to be installed and it needs to be configured > > on the nodes used by the controller. > > > > It supports most pool/vol commands, except for pool-build/pool-delete > > and provides a block device in RAW file mode. > > Linstor supports more than just DRBD so it would also be possible to have > > it provide LVM, ZFS or NVME volumes, but the common case will be to provide > > DRBD volumes in a cluster. > > > > Sample pool XML: > > > > linstor > > > > > > libvirtgrp > > > > > > > > element must point to an already created LINSTOR > > resource-group, which is used to spawn resources/volumes. > > attribute should be the local linstor node name, > > if missing it will try to get the hosts uname and use that instead. > > > > Result volume XML sample: > > > > alpine12 > > libvirtgrp/alpine12 > > 5368709120 > > 5540028416 > > > > /dev/drbd1000 > > > > > > > > > > Signed-off-by: Rene Peinthor > > --- > > Firstly I'd like you to split the patch into more granular commits as > it's customary in our project ... > > > docs/schemas/storagepool.rng | 27 + > > docs/storage.html.in | 39 + > > include/libvirt/libvirt-storage.h | 1 + > > meson.build | 6 + > > meson_options.txt | 1 + > > po/POTFILES.in| 1 + > > src/conf/domain_conf.c| 1 + > > src/conf/storage_conf.c | 14 +- > > src/conf/storage_conf.h | 1 + > > src/conf/virstorageobj.c | 4 +-a > > Conf changes are usually separate > > > src/storage/meson.build | 25 + > > src/storage/storage_backend.c | 6 + > > src/storage/storage_backend_linstor.c | 803 ++ > > src/storage/storage_backend_linstor.h | 23 + > > src/storage/storage_backend_linstor_priv.h| 53 ++ > > src/storage/storage_driver.c | 1 + > > Implementation should also be separate > > > src/test/test_driver.c| 1 + > > tests/linstorjsondata/broken.json | 1 + > > tests/linstorjsondata/resource-group.json | 1 + > > .../linstorjsondata/resource-list-test2.json | 332 > > .../storage-pools-ssdpool.json| 72 ++ > > tests/linstorjsondata/storage-pools.json | 192 + > > tests/linstorjsondata/volume-def-list.json| 158 > > .../volume-definition-test2.json | 1 + > > tests/meson.build | 6 + > > tests/storagebackendlinstortest.c | 371 > > .../storagepoolcapsschemadata/poolcaps-fs.xml | 7 + > > .../poolcaps-full.xml | 7 + > > tests/storagepoolxml2argvtest.c | 1 + > > tests/storagepoolxml2xmlin/pool-linstor.xml | 8 + > > tests/storagevolxml2xmlin/vol-linstor.xml | 10 + > > Placing tests depends. Usually XML related tests go with the commits > implementing the parser/formatter or follow that commit. > > Other tests should be added separately. > > > tools/virsh-pool.c| 3 + > > This is adding the support for the new type so it goes where the type is > declared > > > 32 files changed, 2175 insertions(+), 2 deletions(-) > > [...] > > > diff --git a/docs/schemas/storagepool.rng b/docs/schemas/storagepool.rng > > index bd24b8b8d0..9b163e611d 100644 > > --- a/docs/schemas/storagepool.rng > > +++ b/docs/schemas/storagepool.rng > > @@ -26,6 +26,7 @@ > > > > > > > > + > > > > > > > > @@ -224,6 +225,21 @@ > > > > > > > > + > > + > > + linstor > > + > > + > > + > > + > > + > > + > > + > > + > > + > > + > > + > > + > > > > > > > > @@ -463,6 +479,17 @@ > > > > > > > > + > > + > > + > > + > > + > > + > > + > > + > > + > > + > > + > > > > > > > > diff --git a/docs/storage.html.in b/docs/storage.html.in > > index b2cf343933..9130fbd180 100644 > > --- a/docs/storage.html.in > > +++ b/docs/storage.html.in > > @@ -829,5 +829,44 @@ > > > > Valid volume format types > > The valid volume types are the same as for the directory pool. > > + > > + > > +LINSTOR pool > > + > > + This provides a pool using the LINSTOR software-defined-storage. > > + LINSTOR can provide block storage devices based on DRBD or basic > > + LVM/ZFS volumes. > > + > > + > > + > > + To use LINSTOR in libvirt, setup a working
[PATCH 3/3] storage: Add tests for the Linstor storage backend
---
tests/linstorjsondata/broken.json | 1 +
tests/linstorjsondata/resource-group.json | 1 +
.../linstorjsondata/resource-list-test2.json | 332
.../storage-pools-ssdpool.json| 72
tests/linstorjsondata/storage-pools.json | 192 +
tests/linstorjsondata/volume-def-list.json| 158
.../volume-definition-test2.json | 1 +
tests/meson.build | 6 +
tests/storagebackendlinstortest.c | 372 ++
tests/storagepoolxml2xmlin/pool-linstor.xml | 8 +
tests/storagevolxml2xmlin/vol-linstor.xml | 10 +
11 files changed, 1153 insertions(+)
create mode 100644 tests/linstorjsondata/broken.json
create mode 100644 tests/linstorjsondata/resource-group.json
create mode 100644 tests/linstorjsondata/resource-list-test2.json
create mode 100644 tests/linstorjsondata/storage-pools-ssdpool.json
create mode 100644 tests/linstorjsondata/storage-pools.json
create mode 100644 tests/linstorjsondata/volume-def-list.json
create mode 100644 tests/linstorjsondata/volume-definition-test2.json
create mode 100644 tests/storagebackendlinstortest.c
create mode 100644 tests/storagepoolxml2xmlin/pool-linstor.xml
create mode 100644 tests/storagevolxml2xmlin/vol-linstor.xml
diff --git a/tests/linstorjsondata/broken.json
b/tests/linstorjsondata/broken.json
new file mode 100644
index 00..bce5bde3c6
--- /dev/null
+++ b/tests/linstorjsondata/broken.json
@@ -0,0 +1 @@
+[[{"name":"DfltRscGrp","select_filter":{"place_count":2},id":"a52e934a-9fd9-44cb-9db1-716dcd13aae3"},{"name":"libvirtgrp","select_filter":{"place_count":2,"storage_pool":"thinpool","storage_pool_list":["thinpool"]},"uuid":"7ec0bdee-9176-470e-8f7d-532032434160"}]]
diff --git a/tests/linstorjsondata/resource-group.json
b/tests/linstorjsondata/resource-group.json
new file mode 100644
index 00..3a2f959ad7
--- /dev/null
+++ b/tests/linstorjsondata/resource-group.json
@@ -0,0 +1 @@
+[[{"name":"DfltRscGrp","select_filter":{"place_count":2},"uuid":"a52e934a-9fd9-44cb-9db1-716dcd13aae3"},{"name":"libvirtgrp","select_filter":{"place_count":2,"storage_pool":"thinpool","storage_pool_list":["thinpool"]},"uuid":"7ec0bdee-9176-470e-8f7d-532032434160"}]]
diff --git a/tests/linstorjsondata/resource-list-test2.json
b/tests/linstorjsondata/resource-list-test2.json
new file mode 100644
index 00..86e7f04d82
--- /dev/null
+++ b/tests/linstorjsondata/resource-list-test2.json
@@ -0,0 +1,332 @@
+[
+ [
+{
+ "name": "test2",
+ "node_name": "linstor1",
+ "props": {
+"StorPoolName": "thinpool"
+ },
+ "layer_object": {
+"children": [
+ {
+"type": "STORAGE",
+"storage": {
+ "storage_volumes": [
+{
+ "volume_number": 0,
+ "device_path": "/dev/scratch/test2_0",
+ "allocated_size_kib": 106496,
+ "usable_size_kib": 106496,
+ "disk_state": "[]"
+}
+ ]
+}
+ }
+],
+"type": "DRBD",
+"drbd": {
+ "drbd_resource_definition": {
+"peer_slots": 7,
+"al_stripes": 1,
+"al_stripe_size_kib": 32,
+"port": 7001,
+"transport_type": "IP",
+"secret": "2vD4CZEbaEAO3XIZ/ICv",
+"down": false
+ },
+ "node_id": 0,
+ "peer_slots": 7,
+ "al_stripes": 1,
+ "al_size": 32,
+ "drbd_volumes": [
+{
+ "drbd_volume_definition": {
+"volume_number": 0,
+"minor_number": 1001
+ },
+ "device_path": "/dev/drbd1001",
+ "backing_device": "/dev/scratch/test2_0",
+ "allocated_size_kib": 102460,
+ "usable_size_kib": 102400
+}
+ ],
+ "connections": {
+"linstor2": {
+ "connected": true,
+ "message": "Connected"
+},
+"linstor3": {
+ "connected": true,
+ "message": "Connected"
+}
+ },
+ "promotion_score": 10102,
+ "may_promote": true
+}
+ },
+ "state": {
+"in_use": false
+ },
+ "uuid": "a567d642-02ab-4dd3-8183-a726b20aa9d9",
+ "create_timestamp": 1606836534107,
+ "volumes": [
+{
+ "volume_number": 0,
+ "storage_pool_name": "thinpool",
+ "provider_kind": "LVM_THIN",
+ "device_path": "/dev/drbd1001",
+ "allocated_size_kib": 63,
+ "state": {
+"disk_state": "UpToDate"
+ },
+ "layer_data_list": [
+{
+ "type": "DRBD",
+ "data": {
+"drbd_volume_definition": {
+ "volume_number": 0,
+
[PATCH 0/3] Storage backend Linstor V2
Here is the updated PATCH with split commits, and changes from the first review. Rene Peinthor (3): storage: Linstor configuration storage: Linstor support storage: Add tests for the Linstor storage backend docs/schemas/storagepool.rng | 27 + docs/storage.html.in | 39 + include/libvirt/libvirt-storage.h | 1 + meson.build | 6 + meson_options.txt | 1 + po/POTFILES.in| 1 + src/conf/domain_conf.c| 1 + src/conf/storage_conf.c | 14 +- src/conf/storage_conf.h | 1 + src/conf/virstorageobj.c | 4 +- src/storage/meson.build | 25 + src/storage/storage_backend.c | 6 + src/storage/storage_backend_linstor.c | 781 ++ src/storage/storage_backend_linstor.h | 24 + src/storage/storage_backend_linstor_priv.h| 53 ++ src/storage/storage_driver.c | 1 + src/test/test_driver.c| 1 + tests/linstorjsondata/broken.json | 1 + tests/linstorjsondata/resource-group.json | 1 + .../linstorjsondata/resource-list-test2.json | 332 .../storage-pools-ssdpool.json| 72 ++ tests/linstorjsondata/storage-pools.json | 192 + tests/linstorjsondata/volume-def-list.json| 158 .../volume-definition-test2.json | 1 + tests/meson.build | 6 + tests/storagebackendlinstortest.c | 372 + .../storagepoolcapsschemadata/poolcaps-fs.xml | 7 + .../poolcaps-full.xml | 7 + tests/storagepoolxml2argvtest.c | 1 + tests/storagepoolxml2xmlin/pool-linstor.xml | 8 + tests/storagevolxml2xmlin/vol-linstor.xml | 10 + tools/virsh-pool.c| 3 + 32 files changed, 2155 insertions(+), 2 deletions(-) create mode 100644 src/storage/storage_backend_linstor.c create mode 100644 src/storage/storage_backend_linstor.h create mode 100644 src/storage/storage_backend_linstor_priv.h create mode 100644 tests/linstorjsondata/broken.json create mode 100644 tests/linstorjsondata/resource-group.json create mode 100644 tests/linstorjsondata/resource-list-test2.json create mode 100644 tests/linstorjsondata/storage-pools-ssdpool.json create mode 100644 tests/linstorjsondata/storage-pools.json create mode 100644 tests/linstorjsondata/volume-def-list.json create mode 100644 tests/linstorjsondata/volume-definition-test2.json create mode 100644 tests/storagebackendlinstortest.c create mode 100644 tests/storagepoolxml2xmlin/pool-linstor.xml create mode 100644 tests/storagevolxml2xmlin/vol-linstor.xml -- 2.30.0
[PATCH 2/3] storage: Linstor support
Implement a LINSTOR backend storage driver.
The Linstor client needs to be installed and it needs to be configured
on the nodes used by the controller.
It supports most pool/vol commands, except for pool-build/pool-delete
and provides a block device in RAW file mode.
Linstor supports more than just DRBD so it would also be possible to have
it provide LVM, ZFS or NVME volumes, but the common case will be to provide
DRBD volumes in a cluster.
Sample pool XML:
linstor
libvirtgrp
element must point to an already created LINSTOR
resource-group, which is used to spawn resources/volumes.
attribute should be the local linstor node name,
if missing it will try to get the hosts uname and use that instead.
Result volume XML sample:
alpine12
libvirtgrp/alpine12
5368709120
5540028416
/dev/drbd1000
Signed-off-by: Rene Peinthor
---
docs/storage.html.in | 39 +
meson.build| 6 +
meson_options.txt | 1 +
po/POTFILES.in | 1 +
src/storage/meson.build| 25 +
src/storage/storage_backend.c | 6 +
src/storage/storage_backend_linstor.c | 781 +
src/storage/storage_backend_linstor.h | 24 +
src/storage/storage_backend_linstor_priv.h | 53 ++
9 files changed, 936 insertions(+)
create mode 100644 src/storage/storage_backend_linstor.c
create mode 100644 src/storage/storage_backend_linstor.h
create mode 100644 src/storage/storage_backend_linstor_priv.h
diff --git a/docs/storage.html.in b/docs/storage.html.in
index b2cf343933..9130fbd180 100644
--- a/docs/storage.html.in
+++ b/docs/storage.html.in
@@ -829,5 +829,44 @@
Valid volume format types
The valid volume types are the same as for the directory pool.
+
+
+LINSTOR pool
+
+ This provides a pool using the LINSTOR software-defined-storage.
+ LINSTOR can provide block storage devices based on DRBD or basic
+ LVM/ZFS volumes.
+
+
+
+ To use LINSTOR in libvirt, setup a working LINSTOR cluster, documentation
+ for that is in the LINSTOR Users-guide.
+ And create a resource-group that will be used by libvirt, also make sure
+ the resource-group is setup in a way so that all nodes you want to use
with libvirt
+ will create a resource. So either use diskless-on-remaining or make sure
+ replica-count is the same as you have nodes in your cluster.
+
+
+Since 7.1.0.
+
+Example pool input
+
+pool type="linstor"
+namelinstorpool/name
+source
+namelibvirtrscgrp/name
+host name="linstornode"/
+/source
+/pool
+
+Valid pool format types
+
+ The LINSTOR volume pool does not use the pool format type element.
+
+
+Valid volume format types
+
+ The LINSTOR volume pool does not use the volume format type element.
+
diff --git a/meson.build b/meson.build
index e3e7ff758f..600881a3ab 100644
--- a/meson.build
+++ b/meson.build
@@ -1899,6 +1899,11 @@ if conf.has('WITH_LIBVIRTD')
error('Need libiscsi for iscsi-direct storage driver')
endif
+ if not get_option('storage_linstor').disabled()
+use_storage = true
+conf.set('WITH_STORAGE_LINSTOR', 1)
+ endif
+
if not get_option('storage_lvm').disabled()
lvm_enable = true
lvm_progs = [
@@ -2297,6 +2302,7 @@ storagedriver_summary = {
'Dir': conf.has('WITH_STORAGE_DIR'),
'FS': conf.has('WITH_STORAGE_FS'),
'NetFS': conf.has('WITH_STORAGE_FS'),
+ 'Linstor': conf.has('WITH_STORAGE_LINSTOR'),
'LVM': conf.has('WITH_STORAGE_LVM'),
'iSCSI': conf.has('WITH_STORAGE_ISCSI'),
'iscsi-direct': conf.has('WITH_STORAGE_ISCSI_DIRECT'),
diff --git a/meson_options.txt b/meson_options.txt
index e5d79c2b6b..247d88e0ee 100644
--- a/meson_options.txt
+++ b/meson_options.txt
@@ -79,6 +79,7 @@ option('storage_fs', type: 'feature', value: 'auto',
description: 'FileSystem ba
option('storage_gluster', type: 'feature', value: 'auto', description:
'Gluster backend for the storage driver')
option('storage_iscsi', type: 'feature', value: 'auto', description: 'iscsi
backend for the storage driver')
option('storage_iscsi_direct', type: 'feature', value: 'auto', description:
'iscsi-direct backend for the storage driver')
+option('storage_linstor', type: 'feature', value: 'auto', description:
'Linstor backend for the storage driver')
option('storage_lvm', type: 'feature', value: 'auto', description: 'LVM
backend for the storage driver')
option('storage_mpath', type: 'feature', value: 'auto', description: 'mpath
backend for the storage driver')
option('storage_rbd', type: 'feature', value: 'auto', description: 'RADOS
Block Device backend for the storage driver')
diff --git a/po/POTFILES.in b/po/POTFILES.in
index 14636d4b93..5d8ecfc61c 100644
--- a/po/POTFILES.in
+++ b/po/POTFILES.in
@@ -214,6 +214,7 @@
@SRCDIR@src/storage/storage_backend_gluster.c
[PATCH 1/3] storage: Linstor configuration
This adds Linstor storage defines and documentation
for the new storage backend for Linstor.
---
docs/schemas/storagepool.rng | 27 +++
include/libvirt/libvirt-storage.h | 1 +
src/conf/domain_conf.c| 1 +
src/conf/storage_conf.c | 14 +-
src/conf/storage_conf.h | 1 +
src/conf/virstorageobj.c | 4 ++-
src/storage/storage_driver.c | 1 +
src/test/test_driver.c| 1 +
.../storagepoolcapsschemadata/poolcaps-fs.xml | 7 +
.../poolcaps-full.xml | 7 +
tests/storagepoolxml2argvtest.c | 1 +
tools/virsh-pool.c| 3 +++
12 files changed, 66 insertions(+), 2 deletions(-)
diff --git a/docs/schemas/storagepool.rng b/docs/schemas/storagepool.rng
index bd24b8b8d0..9b163e611d 100644
--- a/docs/schemas/storagepool.rng
+++ b/docs/schemas/storagepool.rng
@@ -26,6 +26,7 @@
+
@@ -224,6 +225,21 @@
+
+
+ linstor
+
+
+
+
+
+
+
+
+
+
+
+
@@ -463,6 +479,17 @@
+
+
+
+
+
+
+
+
+
+
+
diff --git a/include/libvirt/libvirt-storage.h
b/include/libvirt/libvirt-storage.h
index 089e1e0bd1..6876ce6c5a 100644
--- a/include/libvirt/libvirt-storage.h
+++ b/include/libvirt/libvirt-storage.h
@@ -245,6 +245,7 @@ typedef enum {
VIR_CONNECT_LIST_STORAGE_POOLS_ZFS = 1 << 17,
VIR_CONNECT_LIST_STORAGE_POOLS_VSTORAGE = 1 << 18,
VIR_CONNECT_LIST_STORAGE_POOLS_ISCSI_DIRECT = 1 << 19,
+VIR_CONNECT_LIST_STORAGE_POOLS_LINSTOR = 1 << 20,
} virConnectListAllStoragePoolsFlags;
int virConnectListAllStoragePools(virConnectPtr conn,
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index fcf332fe44..6144bbd004 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -31379,6 +31379,7 @@
virDomainStorageSourceTranslateSourcePool(virStorageSourcePtr src,
case VIR_STORAGE_POOL_SCSI:
case VIR_STORAGE_POOL_ZFS:
case VIR_STORAGE_POOL_VSTORAGE:
+case VIR_STORAGE_POOL_LINSTOR:
if (!(src->path = virStorageVolGetPath(vol)))
return -1;
diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c
index 0c50529ace..9a0dda6374 100644
--- a/src/conf/storage_conf.c
+++ b/src/conf/storage_conf.c
@@ -60,7 +60,7 @@ VIR_ENUM_IMPL(virStoragePool,
"logical", "disk", "iscsi",
"iscsi-direct", "scsi", "mpath",
"rbd", "sheepdog", "gluster",
- "zfs", "vstorage",
+ "zfs", "vstorage", "linstor"
);
VIR_ENUM_IMPL(virStoragePoolFormatFileSystem,
@@ -304,6 +304,18 @@ static virStoragePoolTypeInfo poolTypeInfo[] = {
.formatToString = virStorageFileFormatTypeToString,
},
},
+{.poolType = VIR_STORAGE_POOL_LINSTOR,
+ .poolOptions = {
+ .flags = (VIR_STORAGE_POOL_SOURCE_HOST |
+ VIR_STORAGE_POOL_SOURCE_NETWORK |
+ VIR_STORAGE_POOL_SOURCE_NAME),
+ },
+ .volOptions = {
+ .defaultFormat = VIR_STORAGE_FILE_RAW,
+ .formatFromString = virStorageVolumeFormatFromString,
+ .formatToString = virStorageFileFormatTypeToString,
+ }
+},
};
diff --git a/src/conf/storage_conf.h b/src/conf/storage_conf.h
index ffd406e093..716bde942f 100644
--- a/src/conf/storage_conf.h
+++ b/src/conf/storage_conf.h
@@ -110,6 +110,7 @@ typedef enum {
VIR_STORAGE_POOL_GLUSTER, /* Gluster device */
VIR_STORAGE_POOL_ZFS, /* ZFS */
VIR_STORAGE_POOL_VSTORAGE, /* Virtuozzo Storage */
+VIR_STORAGE_POOL_LINSTOR, /* Linstor Storage */
VIR_STORAGE_POOL_LAST,
} virStoragePoolType;
diff --git a/src/conf/virstorageobj.c b/src/conf/virstorageobj.c
index 9fe8b3f28e..4a2a924eb2 100644
--- a/src/conf/virstorageobj.c
+++ b/src/conf/virstorageobj.c
@@ -1461,13 +1461,15 @@ virStoragePoolObjSourceFindDuplicateCb(const void
*payload,
case VIR_STORAGE_POOL_FS:
case VIR_STORAGE_POOL_LOGICAL:
case VIR_STORAGE_POOL_DISK:
+case VIR_STORAGE_POOL_LINSTOR:
case VIR_STORAGE_POOL_ZFS:
if ((data->def->type == VIR_STORAGE_POOL_ISCSI ||
data->def->type == VIR_STORAGE_POOL_ISCSI_DIRECT ||
data->def->type == VIR_STORAGE_POOL_FS ||
data->def->type == VIR_STORAGE_POOL_LOGICAL ||
data->def->type == VIR_STORAGE_POOL_DISK ||
- data->def->type == VIR_STORAGE_POOL_ZFS) &&
+ data->def->type == VIR_STORAGE_POOL_ZFS ||
+ data->def->type == VIR_STORAGE_POOL_LINSTOR) &&
virStoragePoolObjSourceMatchTypeDEVICE(obj, data->def))
return 1;
break;
diff --git
Re: [PATCH] storage: Linstor support
Hi! Thanks for the review, I applied all of your corrections and will retest the code and send a splitted version of the changes. > Also I'd like to ask you to provide a way to setup this storage on our > CI system so that we can compile-test the new driver. Well the easiest would be to use an Ubuntu VM, as Linbit provides a PPA with packages for DRBD and Linstor: https://launchpad.net/~linbit/+archive/ubuntu/linbit-drbd9-stack Add the PPA, install: drbd-dkms linstor-controller linstor-satellite linstor-client Add the node to the Linstor controller: linstor node create Add a storage pool to the node: linstor storage-pool create DfltStorPool Assuming you are only using 1 node for testing, you can create a resource-group with 1 replica: linstor resource-group create libvirtgrp --place-count 1 linstor volume-group create libvirtgrp With that you can use libvirtgrp in the pool definition and should be ready to go. It might be simpler to skip DRBD and just use the Linstor storage layer, then you don't need the drbd kernel module. And the resource-group create would look like this: linstor resource-group create libvirtgrp --place-count 1 -l storage > We'd also like to know if you are willing to continue maintaining this > storage driver, so that it doesn't become abandonware right at commit > time. Yeah sure, I'm willing to maintain it (or at least someone from Linbit will in the future). >So if you use it like this, it means that the storage must be accessible > via regular open() on the host system, is that so? (Asking because the > pool XML hints that the pool is accessed via a network protocol) Linstor will create a block device(DRBD, LVM, ZFS, NVME) on the node, that can be opened with open(), but in the case of DRBD/NVME it is some kind of network storage so I wasn't sure how this best fits in libvirt. > This should be defined via the build system once you detect where the > program is located, to prevent PATH env variable from playing a role in > which program is actually used. If it is ok I would also suggest to not let meson search for the linstor client, as said it is a pure runtime dependency and doesn't have any impact at compile time. Best regards, Rene
Re: [PATCH] apparmor: let image label setting loop over backing files
On Tue, Jan 19, 2021 at 12:15:31 +0100, Christian Ehrhardt wrote:
> On Tue, Jan 19, 2021 at 11:43 AM Peter Krempa wrote:
> >
> > On Tue, Jan 19, 2021 at 11:23:16 +0100, Christian Ehrhardt wrote:
> > > When adding a rule for an image file and that image file has a chain
> > > of backing files then we need to add a rule for each of those files.
> > >
> > > To get that iterate over the backing file chain the same way as
> > > dac/selinux already do and add a label for each.
> > >
> > > Fixes: https://gitlab.com/libvirt/libvirt/-/issues/118
> > >
> > > Signed-off-by: Christian Ehrhardt
> > > ---
> > > src/security/security_apparmor.c | 39 ++--
> > > 1 file changed, 27 insertions(+), 12 deletions(-)
> > >
> > > diff --git a/src/security/security_apparmor.c
> > > b/src/security/security_apparmor.c
> > > index 29f0956d22..1f309c0c9f 100644
> > > --- a/src/security/security_apparmor.c
> > > +++ b/src/security/security_apparmor.c
> > > @@ -756,22 +756,13 @@ AppArmorRestoreInputLabel(virSecurityManagerPtr mgr,
[...]
> > > +static int
> > > +AppArmorSetSecurityImageLabel(virSecurityManagerPtr mgr,
> > > + virDomainDefPtr def,
> > > + virStorageSourcePtr src,
> > > + virSecurityDomainImageLabelFlags flags
> > > G_GNUC_UNUSED)
> > > +{
> > > +virSecurityLabelDefPtr secdef;
> > > +virStorageSourcePtr n;
> > > +
> > > +secdef = virDomainDefGetSecurityLabelDef(def,
> > > SECURITY_APPARMOR_NAME);
> > > +if (!secdef || !secdef->relabel)
> > > +return 0;
> > > +
> > > +if (!secdef->imagelabel)
> > > +return 0;
> >
> > So apparmor doesn't support per-image security labels?
>
> This was present before, it just got moved as part of this change.
> IIRC for apparmor that is only generated once in AppArmorGenSecurityLabel
> and later on only used to check if the struct is ok (if it would be NULL that
> would indicate a non initialized element).
>
> If I'm missing some further hidden meaning of "imagelabel" please let me know.
Here secdef->imagelabel is a global per-VM label, but you can configure
a per disk (or rather per-image) label with a element under
. See:
https://libvirt.org/formatdomain.html#hard-drives-floppy-disks-cdroms
right the first example.
This allows to control labelling of individual files, but I'm not sure
if such concept makes sense for apparmor. For now it seems to be
ignored, maybe it should be at least validated if it doesn't make sense.
Re: [PATCH] apparmor: let image label setting loop over backing files
On Tue, Jan 19, 2021 at 11:43 AM Peter Krempa wrote:
>
> On Tue, Jan 19, 2021 at 11:23:16 +0100, Christian Ehrhardt wrote:
> > When adding a rule for an image file and that image file has a chain
> > of backing files then we need to add a rule for each of those files.
> >
> > To get that iterate over the backing file chain the same way as
> > dac/selinux already do and add a label for each.
> >
> > Fixes: https://gitlab.com/libvirt/libvirt/-/issues/118
> >
> > Signed-off-by: Christian Ehrhardt
> > ---
> > src/security/security_apparmor.c | 39 ++--
> > 1 file changed, 27 insertions(+), 12 deletions(-)
> >
> > diff --git a/src/security/security_apparmor.c
> > b/src/security/security_apparmor.c
> > index 29f0956d22..1f309c0c9f 100644
> > --- a/src/security/security_apparmor.c
> > +++ b/src/security/security_apparmor.c
> > @@ -756,22 +756,13 @@ AppArmorRestoreInputLabel(virSecurityManagerPtr mgr,
> >
> > /* Called when hotplugging */
> > static int
> > -AppArmorSetSecurityImageLabel(virSecurityManagerPtr mgr,
> > - virDomainDefPtr def,
> > - virStorageSourcePtr src,
> > - virSecurityDomainImageLabelFlags flags
> > G_GNUC_UNUSED)
> > +AppArmorSetSecurityImageLabelInternal(virSecurityManagerPtr mgr,
> > + virDomainDefPtr def,
> > + virStorageSourcePtr src)
> > {
> > -virSecurityLabelDefPtr secdef;
> > g_autofree char *vfioGroupDev = NULL;
> > const char *path;
> >
> > -secdef = virDomainDefGetSecurityLabelDef(def, SECURITY_APPARMOR_NAME);
> > -if (!secdef || !secdef->relabel)
> > -return 0;
> > -
> > -if (!secdef->imagelabel)
> > -return 0;
> > -
> > if (src->type == VIR_STORAGE_TYPE_NVME) {
> > const virStorageSourceNVMeDef *nvme = src->nvme;
> >
> > @@ -797,6 +788,30 @@ AppArmorSetSecurityImageLabel(virSecurityManagerPtr
> > mgr,
> > return reload_profile(mgr, def, path, true);
> > }
> >
> > +static int
> > +AppArmorSetSecurityImageLabel(virSecurityManagerPtr mgr,
> > + virDomainDefPtr def,
> > + virStorageSourcePtr src,
> > + virSecurityDomainImageLabelFlags flags
> > G_GNUC_UNUSED)
> > +{
> > +virSecurityLabelDefPtr secdef;
> > +virStorageSourcePtr n;
> > +
> > +secdef = virDomainDefGetSecurityLabelDef(def, SECURITY_APPARMOR_NAME);
> > +if (!secdef || !secdef->relabel)
> > +return 0;
> > +
> > +if (!secdef->imagelabel)
> > +return 0;
>
> So apparmor doesn't support per-image security labels?
This was present before, it just got moved as part of this change.
IIRC for apparmor that is only generated once in AppArmorGenSecurityLabel
and later on only used to check if the struct is ok (if it would be NULL that
would indicate a non initialized element).
If I'm missing some further hidden meaning of "imagelabel" please let me know.
> > +
> > +for (n = src; virStorageSourceIsBacking(n); n = n->backingStore) {
> > +if (AppArmorSetSecurityImageLabelInternal(mgr, def, n) < 0)
>
> It feels a bit suboptimal to fork+exec the aahelper for every single
> image.
I agree, but right now virt-aa-helper has no interface to take multiple
files in one pass.
Furthermore that also calls apparmor_parser which also could be done once.
Furthermore on the "append a rule" use case IMHO aa-helper isn't
doing much of it's original task - it becomes a overly huge "append a line and
call the parser".
I agree that we maybe should batch this. But when we touch it for that - at
least for the "append a rule" use case we might not want to use
virt-aa-helper at all.
But then this becomes a much bigger rewrite with moving e.g. the knowledge how
to get from UUID->filepath into libvirt to be usable from
virt-aa-helper (for other tasks)
but also from the labeling calls. A few more might move, like the
apparmor_parser call.
I've taken a note that it would be good to at least try and POC that, but TBH
that list is growing recently without much draining it :-)
But for the change presented I'd like to keep it to the issue that
we'd want to fix right now.
> The selinux/dac drivers collect the list of things to do before
> forking when we are in the transaction mode (or do just chown/selinux
> labelling, which is trivial)
>
> Given that this is usually on an expensive code path, it's probably okay
> for now though.
Thanks!
> Reviewed-by: Peter Krempa
>
>
> > +return -1;
> > +}
> > +
> > +return 0;
> > +}
> > +
> > static int
> > AppArmorSecurityVerify(virSecurityManagerPtr mgr G_GNUC_UNUSED,
> > virDomainDefPtr def)
> > --
> > 2.30.0
> >
>
--
Christian Ehrhardt
Staff Engineer, Ubuntu Server
Canonical Ltd
Re: [PATCH] apparmor: let image label setting loop over backing files
On Tue, Jan 19, 2021 at 11:23:16 +0100, Christian Ehrhardt wrote:
> When adding a rule for an image file and that image file has a chain
> of backing files then we need to add a rule for each of those files.
>
> To get that iterate over the backing file chain the same way as
> dac/selinux already do and add a label for each.
>
> Fixes: https://gitlab.com/libvirt/libvirt/-/issues/118
>
> Signed-off-by: Christian Ehrhardt
> ---
> src/security/security_apparmor.c | 39 ++--
> 1 file changed, 27 insertions(+), 12 deletions(-)
>
> diff --git a/src/security/security_apparmor.c
> b/src/security/security_apparmor.c
> index 29f0956d22..1f309c0c9f 100644
> --- a/src/security/security_apparmor.c
> +++ b/src/security/security_apparmor.c
> @@ -756,22 +756,13 @@ AppArmorRestoreInputLabel(virSecurityManagerPtr mgr,
>
> /* Called when hotplugging */
> static int
> -AppArmorSetSecurityImageLabel(virSecurityManagerPtr mgr,
> - virDomainDefPtr def,
> - virStorageSourcePtr src,
> - virSecurityDomainImageLabelFlags flags
> G_GNUC_UNUSED)
> +AppArmorSetSecurityImageLabelInternal(virSecurityManagerPtr mgr,
> + virDomainDefPtr def,
> + virStorageSourcePtr src)
> {
> -virSecurityLabelDefPtr secdef;
> g_autofree char *vfioGroupDev = NULL;
> const char *path;
>
> -secdef = virDomainDefGetSecurityLabelDef(def, SECURITY_APPARMOR_NAME);
> -if (!secdef || !secdef->relabel)
> -return 0;
> -
> -if (!secdef->imagelabel)
> -return 0;
> -
> if (src->type == VIR_STORAGE_TYPE_NVME) {
> const virStorageSourceNVMeDef *nvme = src->nvme;
>
> @@ -797,6 +788,30 @@ AppArmorSetSecurityImageLabel(virSecurityManagerPtr mgr,
> return reload_profile(mgr, def, path, true);
> }
>
> +static int
> +AppArmorSetSecurityImageLabel(virSecurityManagerPtr mgr,
> + virDomainDefPtr def,
> + virStorageSourcePtr src,
> + virSecurityDomainImageLabelFlags flags
> G_GNUC_UNUSED)
> +{
> +virSecurityLabelDefPtr secdef;
> +virStorageSourcePtr n;
> +
> +secdef = virDomainDefGetSecurityLabelDef(def, SECURITY_APPARMOR_NAME);
> +if (!secdef || !secdef->relabel)
> +return 0;
> +
> +if (!secdef->imagelabel)
> +return 0;
So apparmor doesn't support per-image security labels?
> +
> +for (n = src; virStorageSourceIsBacking(n); n = n->backingStore) {
> +if (AppArmorSetSecurityImageLabelInternal(mgr, def, n) < 0)
It feels a bit suboptimal to fork+exec the aahelper for every single
image. The selinux/dac drivers collect the list of things to do before
forking when we are in the transaction mode (or do just chown/selinux
labelling, which is trivial)
Given that this is usually on an expensive code path, it's probably okay
for now though.
Reviewed-by: Peter Krempa
> +return -1;
> +}
> +
> +return 0;
> +}
> +
> static int
> AppArmorSecurityVerify(virSecurityManagerPtr mgr G_GNUC_UNUSED,
> virDomainDefPtr def)
> --
> 2.30.0
>
[PATCH] apparmor: let image label setting loop over backing files
When adding a rule for an image file and that image file has a chain
of backing files then we need to add a rule for each of those files.
To get that iterate over the backing file chain the same way as
dac/selinux already do and add a label for each.
Fixes: https://gitlab.com/libvirt/libvirt/-/issues/118
Signed-off-by: Christian Ehrhardt
---
src/security/security_apparmor.c | 39 ++--
1 file changed, 27 insertions(+), 12 deletions(-)
diff --git a/src/security/security_apparmor.c b/src/security/security_apparmor.c
index 29f0956d22..1f309c0c9f 100644
--- a/src/security/security_apparmor.c
+++ b/src/security/security_apparmor.c
@@ -756,22 +756,13 @@ AppArmorRestoreInputLabel(virSecurityManagerPtr mgr,
/* Called when hotplugging */
static int
-AppArmorSetSecurityImageLabel(virSecurityManagerPtr mgr,
- virDomainDefPtr def,
- virStorageSourcePtr src,
- virSecurityDomainImageLabelFlags flags
G_GNUC_UNUSED)
+AppArmorSetSecurityImageLabelInternal(virSecurityManagerPtr mgr,
+ virDomainDefPtr def,
+ virStorageSourcePtr src)
{
-virSecurityLabelDefPtr secdef;
g_autofree char *vfioGroupDev = NULL;
const char *path;
-secdef = virDomainDefGetSecurityLabelDef(def, SECURITY_APPARMOR_NAME);
-if (!secdef || !secdef->relabel)
-return 0;
-
-if (!secdef->imagelabel)
-return 0;
-
if (src->type == VIR_STORAGE_TYPE_NVME) {
const virStorageSourceNVMeDef *nvme = src->nvme;
@@ -797,6 +788,30 @@ AppArmorSetSecurityImageLabel(virSecurityManagerPtr mgr,
return reload_profile(mgr, def, path, true);
}
+static int
+AppArmorSetSecurityImageLabel(virSecurityManagerPtr mgr,
+ virDomainDefPtr def,
+ virStorageSourcePtr src,
+ virSecurityDomainImageLabelFlags flags
G_GNUC_UNUSED)
+{
+virSecurityLabelDefPtr secdef;
+virStorageSourcePtr n;
+
+secdef = virDomainDefGetSecurityLabelDef(def, SECURITY_APPARMOR_NAME);
+if (!secdef || !secdef->relabel)
+return 0;
+
+if (!secdef->imagelabel)
+return 0;
+
+for (n = src; virStorageSourceIsBacking(n); n = n->backingStore) {
+if (AppArmorSetSecurityImageLabelInternal(mgr, def, n) < 0)
+return -1;
+}
+
+return 0;
+}
+
static int
AppArmorSecurityVerify(virSecurityManagerPtr mgr G_GNUC_UNUSED,
virDomainDefPtr def)
--
2.30.0
Re: [PATCH] meson: fix vstorage driver build
This patch is outdated by
https://www.redhat.com/archives/libvir-list/2021-January/msg00778.html
On Mon, Jan 18, 2021 at 3:02 PM Nikolay Shirokovskiy <
nshirokovs...@virtuozzo.com> wrote:
> It breaks on using - in VSTORAGE-MOUNT definition with:
>
> In file included from ../config.h:19:0,
> from ../src/util/viraudit.c:22:
> ./meson-config.h:180:17: error: ISO C99 requires whitespace after the
> macro name [-Werror]
> #define VSTORAGE-MOUNT "/usr/bin/vstorage-mount"
> ^
> ./meson-config.h:180:0: error: "VSTORAGE" redefined [-Werror]
> #define VSTORAGE-MOUNT "/usr/bin/vstorage-mount"
> ^
> ./meson-config.h:178:0: note: this is the location of the previous
> definition
> #define VSTORAGE "/usr/bin/vstorage"
> ^
> #define VSTORAGE-MOUNT "/usr/bin/vstorage-mount"
> ---
> meson.build | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/meson.build b/meson.build
> index b5277b4..aff2565 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -1974,7 +1974,7 @@ if conf.has('WITH_LIBVIRTD')
>conf.set('WITH_STORAGE_VSTORAGE', 1)
>foreach name : ['vstorage', 'vstorage-mount', 'umount']
> path = get_variable('@0@
> _prog'.format(name.underscorify())).path()
> -conf.set_quoted(name.to_upper(), path)
> +conf.set_quoted(name.underscorify().to_upper(), path)
>endforeach
> endif
>endif
> --
> 1.8.3.1
>
>
[PATCH] vstorage: remove build time checks for runtime binaries
Accoring to current agreement mentioned in list recently [1]. Now
vstorage driver will be build in default devs environment and also can
be included into CI. This also closes quite old abandoned thread on
alternative checks for binaries in case of this same driver [2].
[1] https://www.redhat.com/archives/libvir-list/2021-January/msg00750.html
[2] https://www.redhat.com/archives/libvir-list/2020-July/msg00697.html
Signed-off-by: Nikolay Shirokovskiy
---
meson.build| 22 ++
src/storage/storage_backend_vstorage.c | 4 ++--
2 files changed, 4 insertions(+), 22 deletions(-)
diff --git a/meson.build b/meson.build
index b5277b4..e3e7ff7 100644
--- a/meson.build
+++ b/meson.build
@@ -1957,26 +1957,8 @@ if conf.has('WITH_LIBVIRTD')
endif
if not get_option('storage_vstorage').disabled()
-vstorage_enable = true
-
-foreach name : ['vstorage', 'vstorage-mount', 'umount']
- set_variable(
-'@0@_prog'.format(name.underscorify()),
-find_program(name, required: get_option('storage_vstorage'), dirs:
libvirt_sbin_path)
- )
- if not get_variable('@0@_prog'.format(name.underscorify())).found()
-vstorage_enable = false
- endif
-endforeach
-
-if vstorage_enable
- use_storage = true
- conf.set('WITH_STORAGE_VSTORAGE', 1)
- foreach name : ['vstorage', 'vstorage-mount', 'umount']
-path = get_variable('@0@_prog'.format(name.underscorify())).path()
-conf.set_quoted(name.to_upper(), path)
- endforeach
-endif
+use_storage = true
+conf.set('WITH_STORAGE_VSTORAGE', 1)
endif
if not get_option('storage_zfs').disabled()
diff --git a/src/storage/storage_backend_vstorage.c
b/src/storage/storage_backend_vstorage.c
index 6cff9f1..7c67407 100644
--- a/src/storage/storage_backend_vstorage.c
+++ b/src/storage/storage_backend_vstorage.c
@@ -65,7 +65,7 @@ virStorageBackendVzPoolStart(virStoragePoolObjPtr pool)
mode = g_strdup_printf("%o", def->target.perms.mode);
-cmd = virCommandNewArgList(VSTORAGE_MOUNT,
+cmd = virCommandNewArgList("vstorage-mount",
"-c", def->source.name,
def->target.path,
"-m", mode,
@@ -129,7 +129,7 @@ virStorageBackendVzPoolStop(virStoragePoolObjPtr pool)
if ((rc = virStorageBackendVzIsMounted(pool)) != 1)
return rc;
-cmd = virCommandNewArgList(UMOUNT, def->target.path, NULL);
+cmd = virCommandNewArgList("umount", def->target.path, NULL);
return virCommandRun(cmd, NULL);
}
--
1.8.3.1
Re: [PATCH] vstorage: remove build time checks for runtime binaries
On 1/19/21 7:34 AM, Nikolay Shirokovskiy wrote:
Accoring to current agreement mentioned in list recently [1]. Now
vstorage driver will be build in default devs environment and also can
be included into CI. This also closes quite old abandoned thread on
alternative checks for binaries in case of this same driver [2].
[1] https://www.redhat.com/archives/libvir-list/2021-January/msg00750.html
[2] https://www.redhat.com/archives/libvir-list/2020-July/msg00697.html
Signed-off-by: Nikolay Shirokovskiy
---
meson.build| 22 ++
src/storage/storage_backend_vstorage.c | 4 ++--
2 files changed, 4 insertions(+), 22 deletions(-)
diff --git a/meson.build b/meson.build
index b5277b4..e3e7ff7 100644
--- a/meson.build
+++ b/meson.build
@@ -1957,26 +1957,8 @@ if conf.has('WITH_LIBVIRTD')
endif
if not get_option('storage_vstorage').disabled()
-vstorage_enable = true
-
-foreach name : ['vstorage', 'vstorage-mount', 'umount']
- set_variable(
-'@0@_prog'.format(name.underscorify()),
-find_program(name, required: get_option('storage_vstorage'), dirs:
libvirt_sbin_path)
- )
- if not get_variable('@0@_prog'.format(name.underscorify())).found()
-vstorage_enable = false
- endif
-endforeach
-
-if vstorage_enable
- use_storage = true
- conf.set('WITH_STORAGE_VSTORAGE', 1)
- foreach name : ['vstorage', 'vstorage-mount', 'umount']
-path = get_variable('@0@_prog'.format(name.underscorify())).path()
-conf.set_quoted(name.to_upper(), path)
- endforeach
-endif
+use_storage = true
+conf.set('WITH_STORAGE_VSTORAGE', 1)
endif
if not get_option('storage_zfs').disabled()
diff --git a/src/storage/storage_backend_vstorage.c
b/src/storage/storage_backend_vstorage.c
index 6cff9f1..7c67407 100644
--- a/src/storage/storage_backend_vstorage.c
+++ b/src/storage/storage_backend_vstorage.c
@@ -65,7 +65,7 @@ virStorageBackendVzPoolStart(virStoragePoolObjPtr pool)
mode = g_strdup_printf("%o", def->target.perms.mode);
-cmd = virCommandNewArgList(VSTORAGE_MOUNT,
+cmd = virCommandNewArgList("vstorage-mount",
"-c", def->source.name,
def->target.path,
"-m", mode,
These two hunks look good, although I have slight preference to keep
VSTORAGE_MOUNT and maybe just:
#define VSTORAGE_MOUNT "vstorage-mount"
somewhere in storage_backend_vstorage.c; but that is just cosmetics and
we have existing examples of your approach too:
libvirt.git $ git grep "virCommandNew.*(\"" | wc -l
27
@@ -129,7 +129,7 @@ virStorageBackendVzPoolStop(virStoragePoolObjPtr pool)
if ((rc = virStorageBackendVzIsMounted(pool)) != 1)
return rc;
-cmd = virCommandNewArgList(UMOUNT, def->target.path, NULL);
+cmd = virCommandNewArgList("umount", def->target.path, NULL);
return virCommandRun(cmd, NULL);
}
A-ha! So if FS driver is disabled, then UMOUNT is undefined? Well, an
empty string, whatever. If it is so then:
Reviewed-by: Michal Privoznik
Michal
