[libvirt PATCH] ci: Swap mipsel and ppc64le builds
Debian sid is currently broken on mipsel, so use Debian 10 for that architecture; at the same time, move the ppc64le build from Debian 10 to Debian sid to keep things balanced. Signed-off-by: Andrea Bolognani --- Pushed as a CI fix. .gitlab-ci.yml | 16 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 0def25ff32..bfb66a652d 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -257,12 +257,12 @@ mips64el-debian-10-container: NAME: debian-10-cross-mips64el mipsel-debian-10-container: - <<: *container_optional_job_definition + <<: *container_extra_job_definition variables: NAME: debian-10-cross-mipsel ppc64le-debian-10-container: - <<: *container_extra_job_definition + <<: *container_optional_job_definition variables: NAME: debian-10-cross-ppc64le @@ -302,12 +302,12 @@ mips64el-debian-sid-container: NAME: debian-sid-cross-mips64el mipsel-debian-sid-container: - <<: *container_extra_job_definition + <<: *container_optional_job_definition variables: NAME: debian-sid-cross-mipsel ppc64le-debian-sid-container: - <<: *container_optional_job_definition + <<: *container_extra_job_definition variables: NAME: debian-sid-cross-ppc64le @@ -426,11 +426,11 @@ aarch64-debian-10: NAME: debian-10 CROSS: aarch64 -ppc64le-debian-10: +mipsel-debian-10: <<: *cross_build_extra_job_definition variables: NAME: debian-10 -CROSS: ppc64le +CROSS: mipsel s390x-debian-10: <<: *cross_build_default_job_definition @@ -450,11 +450,11 @@ i686-debian-sid: NAME: debian-sid CROSS: i686 -mipsel-debian-sid: +ppc64le-debian-sid: <<: *cross_build_extra_job_definition variables: NAME: debian-sid -CROSS: mipsel +CROSS: ppc64le mingw32-fedora-rawhide: <<: *cross_build_default_job_definition -- 2.25.4
Re: [libvirt PATCH v2 08/10] nodedev: add mdev support to virNodeDeviceDestroy()
On Thu, 2020-06-11 at 16:00 +0200, Erik Skultety wrote:
> On Tue, Jun 09, 2020 at 04:43:48PM -0500, Jonathon Jongsma wrote:
> > Add the ability to destroy mdev node devices via the mdevctl
> > utility.
> >
> > Signed-off-by: Jonathon Jongsma
> > ---
> > src/node_device/node_device_driver.c | 46
> >
> > src/node_device/node_device_driver.h | 3 ++
> > 2 files changed, 49 insertions(+)
> >
> > diff --git a/src/node_device/node_device_driver.c
> > b/src/node_device/node_device_driver.c
> > index dbc7eb4d1e..c956bb55fc 100644
> > --- a/src/node_device/node_device_driver.c
> > +++ b/src/node_device/node_device_driver.c
> > @@ -790,6 +790,45 @@ nodeDeviceCreateXML(virConnectPtr conn,
> > }
> >
> >
> > +virCommandPtr
> > +nodeDeviceGetMdevctlStopCommand(const char *uuid,
> > +bool persist)
> > +{
> > +g_autofree char *mdevctl = virFindFileInPath(MDEVCTL);
> > +const char *subcommand;
> > +
> > +if (!mdevctl)
> > +return NULL;
> > +
> > +if (persist)
> > +subcommand = "undefine";
>
> "undefine" is a NOP on active mdevs IIRC, so again the helper name is
> confusing.
Oh, you're right. This part was meant to plan ahead for persistent
mediated devices, but since it's not yet used (and since it doesn't
have the effect intended, as you point out), it should probably just be
removed from this patch series.
>
> > +else
> > +subcommand = "stop";
> > +
> > +virCommandPtr cmd = virCommandNewArgList(mdevctl,
> > + subcommand,
> > + "-u",
> > + uuid,
> > + NULL);
> > +
> > +return cmd;
> > +}
>
> Like I mentioned already, we could have a generic translator to the
> mdevctl
> subcommands.
>
> Regards,
> Erik
[PATCH 1/1] qemuDomainSetNumaParamsLive: set nodeset for root cgroup
This function handles the change of NUMA nodeset for a given guest, setting CpusetMems for the emulator, vcpus and IOThread sub-groups. It doesn't set the same nodeset to the root cgroup though. This means that cpuset.mems of the root cgroup ends up holding the new nodeset and the old nodeset as well. For a guest with placement=strict, nodeset='0', doing virsh numatune 0 8 --live Will make cpuset.mems of emulator, vcpus and iothread to be "8", but cpuset.mems of the root cgroup will be "0,8". This means that any new tasks that ends up landing in the root cgroup, aside from the emulator/vcpus/iothread sub-groups, will be split between the old nodeset and the new nodeset, which is not what we want. Signed-off-by: Daniel Henrique Barboza --- src/qemu/qemu_driver.c | 4 1 file changed, 4 insertions(+) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 88517ba6a7..59d322c8f3 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -9702,6 +9702,10 @@ qemuDomainSetNumaParamsLive(virDomainObjPtr vm, virCgroupFree(_temp); } +/* set nodeset for root cgroup */ +if (virCgroupSetCpusetMems(priv->cgroup, nodeset_str) < 0) +goto cleanup; + ret = 0; cleanup: virCgroupFree(_temp); -- 2.26.2
Re: [libvirt PATCH v4 3/3] ci: Update build system integration
On Thu, 2020-06-11 at 17:45 +0100, Daniel P. Berrangé wrote: > On Thu, Jun 11, 2020 at 12:03:08PM +0200, Andrea Bolognani wrote: > > -# The default tag is ':latest' but if the container > > +# The default tag is ':master' but if the container > > # repo above uses different conventions this can override it > > -CI_IMAGE_TAG = :latest > > +CI_IMAGE_TAG = :master > > Drop this since we went back to latest Right, good catch :) -- Andrea Bolognani / Red Hat / Virtualization
[PATCH 1/1] manpages/virsh.rst: clarify numatune memory migration on Linux
On Linux, changing the nodeset on 'numatune' does not imply that the guest memory will be migrated on the spot to the new nodeset. The memory migration is tied on guest usage of the memory pages, and an idle guest will take longer to have its memory migrated to the new nodeset. This is a behavior explained in detail in the Linux kernel documentation in Documentation/admin-guide/cgroup-v1/cpusets.rst. The user doesn't need this level of detail though - just needs his/her expectations under check. Running 'numastat' and hoping for instant memory migration from the previous nodeset to the new one is not viable. There's also parts of the memory that are locked by QEMU in the same place, e.g. when VFIO devices are present. Let's also mention it as another factor that impacts the results the user might expect from NUMA memory migration with numatune. Signed-off-by: Daniel Henrique Barboza --- docs/manpages/virsh.rst | 7 +++ 1 file changed, 7 insertions(+) diff --git a/docs/manpages/virsh.rst b/docs/manpages/virsh.rst index 969a4d5543..ff32338f43 100644 --- a/docs/manpages/virsh.rst +++ b/docs/manpages/virsh.rst @@ -3395,6 +3395,13 @@ If *--live* is specified, set scheduler information of a running guest. If *--config* is specified, affect the next boot of a persistent guest. If *--current* is specified, affect the current guest state. +For running guests in Linux hosts, the changes made in the domain's +numa parameters does not imply that the guest memory will be moved to a +different nodeset immediately. The memory migration depends on the +guest activity, and the memory of an idle guest will remain in its +previous nodeset for longer. The presence of VFIO devices will also +lock parts of the guest memory in the same nodeset used to start the +guest, regardless of nodeset changes. perf -- 2.26.2
Re: [libvirt PATCH v4 2/3] ci: Use GitLab container registry
On Thu, Jun 11, 2020 at 12:03:07PM +0200, Andrea Bolognani wrote: > Instead of using pre-built containers hosted on Quay, build > containers as part of the GitLab CI pipeline and upload them to the > GitLab container registry for later use. > > This will not significantly slow down builds, because containers are > only rebuilt when the corresponding Dockerfile has been modified. > > Signed-off-by: Andrea Bolognani > --- > .gitlab-ci.yml| 255 +- > ci/containers/README.rst | 14 + > ci/containers/libvirt-centos-7.Dockerfile | 137 ++ > ci/containers/libvirt-centos-8.Dockerfile | 108 > .../libvirt-centos-stream.Dockerfile | 109 > ...libvirt-debian-10-cross-aarch64.Dockerfile | 122 + > .../libvirt-debian-10-cross-armv6l.Dockerfile | 120 + > .../libvirt-debian-10-cross-armv7l.Dockerfile | 121 + > .../libvirt-debian-10-cross-i686.Dockerfile | 121 + > .../libvirt-debian-10-cross-mips.Dockerfile | 121 + > ...ibvirt-debian-10-cross-mips64el.Dockerfile | 121 + > .../libvirt-debian-10-cross-mipsel.Dockerfile | 121 + > ...libvirt-debian-10-cross-ppc64le.Dockerfile | 121 + > .../libvirt-debian-10-cross-s390x.Dockerfile | 121 + > ci/containers/libvirt-debian-10.Dockerfile| 112 > .../libvirt-debian-9-cross-aarch64.Dockerfile | 126 + > .../libvirt-debian-9-cross-armv6l.Dockerfile | 124 + > .../libvirt-debian-9-cross-armv7l.Dockerfile | 125 + > .../libvirt-debian-9-cross-mips.Dockerfile| 125 + > ...libvirt-debian-9-cross-mips64el.Dockerfile | 125 + > .../libvirt-debian-9-cross-mipsel.Dockerfile | 125 + > .../libvirt-debian-9-cross-ppc64le.Dockerfile | 125 + > .../libvirt-debian-9-cross-s390x.Dockerfile | 125 + > ci/containers/libvirt-debian-9.Dockerfile | 116 > ...ibvirt-debian-sid-cross-aarch64.Dockerfile | 122 + > ...libvirt-debian-sid-cross-armv6l.Dockerfile | 120 + > ...libvirt-debian-sid-cross-armv7l.Dockerfile | 121 + > .../libvirt-debian-sid-cross-i686.Dockerfile | 121 + > .../libvirt-debian-sid-cross-mips.Dockerfile | 121 + > ...bvirt-debian-sid-cross-mips64el.Dockerfile | 121 + > ...libvirt-debian-sid-cross-mipsel.Dockerfile | 120 + > ...ibvirt-debian-sid-cross-ppc64le.Dockerfile | 121 + > .../libvirt-debian-sid-cross-s390x.Dockerfile | 121 + > ci/containers/libvirt-debian-sid.Dockerfile | 112 > ci/containers/libvirt-fedora-31.Dockerfile| 109 > ci/containers/libvirt-fedora-32.Dockerfile| 109 > ...rt-fedora-rawhide-cross-mingw32.Dockerfile | 130 + > ...rt-fedora-rawhide-cross-mingw64.Dockerfile | 130 + > .../libvirt-fedora-rawhide.Dockerfile | 110 > ci/containers/libvirt-opensuse-151.Dockerfile | 109 > ci/containers/libvirt-ubuntu-1804.Dockerfile | 117 > ci/containers/libvirt-ubuntu-2004.Dockerfile | 113 > ci/containers/refresh | 41 +++ > 43 files changed, 5103 insertions(+), 5 deletions(-) > create mode 100644 ci/containers/README.rst > create mode 100644 ci/containers/libvirt-centos-7.Dockerfile > create mode 100644 ci/containers/libvirt-centos-8.Dockerfile > create mode 100644 ci/containers/libvirt-centos-stream.Dockerfile > create mode 100644 ci/containers/libvirt-debian-10-cross-aarch64.Dockerfile > create mode 100644 ci/containers/libvirt-debian-10-cross-armv6l.Dockerfile > create mode 100644 ci/containers/libvirt-debian-10-cross-armv7l.Dockerfile > create mode 100644 ci/containers/libvirt-debian-10-cross-i686.Dockerfile > create mode 100644 ci/containers/libvirt-debian-10-cross-mips.Dockerfile > create mode 100644 ci/containers/libvirt-debian-10-cross-mips64el.Dockerfile > create mode 100644 ci/containers/libvirt-debian-10-cross-mipsel.Dockerfile > create mode 100644 ci/containers/libvirt-debian-10-cross-ppc64le.Dockerfile > create mode 100644 ci/containers/libvirt-debian-10-cross-s390x.Dockerfile > create mode 100644 ci/containers/libvirt-debian-10.Dockerfile > create mode 100644 ci/containers/libvirt-debian-9-cross-aarch64.Dockerfile > create mode 100644 ci/containers/libvirt-debian-9-cross-armv6l.Dockerfile > create mode 100644 ci/containers/libvirt-debian-9-cross-armv7l.Dockerfile > create mode 100644 ci/containers/libvirt-debian-9-cross-mips.Dockerfile > create mode 100644 ci/containers/libvirt-debian-9-cross-mips64el.Dockerfile > create mode 100644 ci/containers/libvirt-debian-9-cross-mipsel.Dockerfile > create mode 100644 ci/containers/libvirt-debian-9-cross-ppc64le.Dockerfile > create mode 100644 ci/containers/libvirt-debian-9-cross-s390x.Dockerfile > create mode 100644 ci/containers/libvirt-debian-9.Dockerfile > create mode 100644
Re: [libvirt PATCH v4 3/3] ci: Update build system integration
On Thu, Jun 11, 2020 at 12:03:08PM +0200, Andrea Bolognani wrote: > The ci-* targets need to know where our container images are stored > and how they are called to work, so now that we use the GitLab > container registry instead of Quay some changes are necessary. > > Signed-off-by: Andrea Bolognani > --- > ci/Makefile | 10 +- > ci/list-images.sh | 24 ++-- > 2 files changed, 11 insertions(+), 23 deletions(-) > > diff --git a/ci/Makefile b/ci/Makefile > index bc1dac11e3..dc8012f33b 100644 > --- a/ci/Makefile > +++ b/ci/Makefile > @@ -50,11 +50,11 @@ CI_BUILD_SCRIPT = $(CI_ROOTDIR)/build.sh > # Location of the container images we're going to pull > # Can be useful to overridde to use a locally built > # image instead > -CI_IMAGE_PREFIX = quay.io/libvirt/buildenv-libvirt- > +CI_IMAGE_PREFIX = registry.gitlab.com/libvirt/libvirt/ci- > > -# The default tag is ':latest' but if the container > +# The default tag is ':master' but if the container > # repo above uses different conventions this can override it > -CI_IMAGE_TAG = :latest > +CI_IMAGE_TAG = :master Drop this since we went back to latest 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 :|
[libvirt-glib PATCH] gitlab: introduce CI jobs testing git master & distro libvirt
The glib build needs to validate two axis - A variety of libvirt versions - Native and mingw We test a variety of libvirt versions by running a build against the distro provided libvirt packages. All that is then missing is a build against the latest libvirt git master, which only needs to be run on a single distro, for which CentOS 8 is picked as a stable long life base. For mingw, we only test against libvirt git master, using Fedora rawhide. It is expected that any issues against older libvirt will have already been caught by the native builds. Signed-off-by: Daniel P. Berrangé --- .gitlab-ci.yml| 228 ++ ci/libvirt-centos-7.Dockerfile| 88 +++ ci/libvirt-centos-8.Dockerfile| 64 + ci/libvirt-centos-stream.Dockerfile | 58 + ci/libvirt-debian-10.Dockerfile | 58 + ci/libvirt-debian-9.Dockerfile| 61 + ci/libvirt-debian-sid.Dockerfile | 58 + ci/libvirt-fedora-31.Dockerfile | 55 + ci/libvirt-fedora-32.Dockerfile | 55 + ...rt-fedora-rawhide-cross-mingw32.Dockerfile | 133 ++ ...rt-fedora-rawhide-cross-mingw64.Dockerfile | 133 ++ ci/libvirt-fedora-rawhide.Dockerfile | 56 + ci/libvirt-opensuse-151.Dockerfile| 57 + ci/libvirt-ubuntu-1804.Dockerfile | 61 + ci/libvirt-ubuntu-2004.Dockerfile | 58 + ci/refresh| 36 +++ 16 files changed, 1259 insertions(+) create mode 100644 ci/libvirt-centos-7.Dockerfile create mode 100644 ci/libvirt-centos-8.Dockerfile create mode 100644 ci/libvirt-centos-stream.Dockerfile create mode 100644 ci/libvirt-debian-10.Dockerfile create mode 100644 ci/libvirt-debian-9.Dockerfile create mode 100644 ci/libvirt-debian-sid.Dockerfile create mode 100644 ci/libvirt-fedora-31.Dockerfile create mode 100644 ci/libvirt-fedora-32.Dockerfile create mode 100644 ci/libvirt-fedora-rawhide-cross-mingw32.Dockerfile create mode 100644 ci/libvirt-fedora-rawhide-cross-mingw64.Dockerfile create mode 100644 ci/libvirt-fedora-rawhide.Dockerfile create mode 100644 ci/libvirt-opensuse-151.Dockerfile create mode 100644 ci/libvirt-ubuntu-1804.Dockerfile create mode 100644 ci/libvirt-ubuntu-2004.Dockerfile create mode 100755 ci/refresh diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 50dae92..da1e588 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -1,6 +1,97 @@ stages: - prebuild + - containers + - builds + - docs + +.container_job_template: _job_definition + image: docker:stable + stage: containers + services: +- docker:dind + before_script: +- export TAG="$CI_REGISTRY_IMAGE/ci-$NAME:latest" +- export COMMON_TAG="$CI_REGISTRY/libvirt/libvirt-glib/ci-$NAME:latest" +- docker info +- docker login registry.gitlab.com -u "$CI_REGISTRY_USER" -p "$CI_REGISTRY_PASSWORD" + script: +- docker pull "$TAG" || docker pull "$COMMON_TAG" || true +- docker build --cache-from "$TAG" --cache-from "$COMMON_TAG" --tag "$TAG" -f "ci/libvirt-$NAME.Dockerfile" ci +- docker push "$TAG" + after_script: +- docker logout + +.script_variables: _variables | + export MAKEFLAGS="-j$(getconf _NPROCESSORS_ONLN)" + export VROOT="$SCRATCH_DIR/vroot" + export CCACHE_BASEDIR="$(pwd)" + export CCACHE_DIR="$CCACHE_BASEDIR/ccache" + export CCACHE_MAXSIZE="500M" + export PATH="$CCACHE_WRAPPERSDIR:$VROOT/bin:$PATH" + export SCRATCH_DIR="/tmp/scratch" + export PKG_CONFIG_PATH="$VROOT/lib/pkgconfig" + +.git_native_build_job_template: _native_build_job_definition + image: $CI_REGISTRY_IMAGE/ci-$NAME:latest + stage: builds + before_script: +- *script_variables +- export LD_LIBRARY_PATH="$VROOT/lib" + script: +- pushd "$PWD" +- mkdir -p "$SCRATCH_DIR" +- cd "$SCRATCH_DIR" +- git clone --depth 1 https://gitlab.com/libvirt/libvirt.git +- mkdir libvirt/build +- cd libvirt/build +- ../autogen.sh --prefix="$VROOT" --without-libvirtd +- $MAKE install +- popd +- mkdir build +- cd build +- ../autogen.sh --prefix="$VROOT" +- $MAKE install +- $MAKE dist +- if test -x /usr/bin/rpmbuild && test "$RPM" != "skip" ; then rpmbuild --nodeps -ta libvirt-glib*.tar.gz ; fi + +.dist_native_build_job_template: _native_build_job_definition + image: $CI_REGISTRY_IMAGE/ci-$NAME:latest + stage: builds + before_script: +- *script_variables + script: +- mkdir build +- cd build +- ../autogen.sh --prefix="$VROOT" +- $MAKE install +- $MAKE dist +- if test -x /usr/bin/rpmbuild && test "$RPM" != "skip" ; then rpmbuild -ta libvirt-glib*.tar.gz ; fi + +# Default cross build jobs that are always run +.git_cross_build_default_job_template: _cross_build_default_job_definition + image: $CI_REGISTRY_IMAGE/ci-$NAME:latest + stage: builds + cache: +paths: + - ccache/ +
Does 'numad' interacts with memory_migration with 'numatune'?
Hi, While investigating a 'virsh numatune' behavior in Power 9 guests I came across this doubt and couldn't find a direct answer. numad role, as far as [1] goes, is automatic NUMA affinity only. As far as Libvirt and my understanding goes , numad is used for placement='auto' setups, which aren't even allowed for numatune operations in the first place. Problem is that I'm not sure if the mere presence of numad running in the host might be accelerating the memory migration triggered by numatune, regardless of placement settings. My first answer would be no, but several examples in the internet shows all the RAM in the guest being migrated from one NUMA node to the other almost instantly*, and aside from them being done in x86 I wonder whether numad is having any impact on that. The reason I'm asking is because I don't have a x86 setup with multiple NUMA nodes to compare results, and numad is broken sparse NUMA setups for some time now ([2] tells the story if you're interested), and Power 8/9 happens to operate with sparse NUMA setups, so no numad for me. If someone can confirm my suspicion (i.e. numad has no interference in NUMA memory migration triggered by numatune) I appreciate. Thanks, DHB * or at very least no one cared to point out that the memory is migrated according to the paging demanding of the guest, as I see happen in Power guests and working as intended according to kernel cgroup docs. [1] https://access.redhat.com/documentation/en-us/red_hat_enterprise_linux/7/html/performance_tuning_guide/sect-red_hat_enterprise_linux-performance_tuning_guide-tool_reference-numad [2] https://bugs.launchpad.net/ubuntu/+source/numad/+bug/1832915
Re: [libvirt PATCH v2 07/10] nodedev: Add testing for 'mdevctl start'
On 6/11/20 4:32 PM, Erik Skultety wrote: On Thu, Jun 11, 2020 at 04:15:56PM +0200, Michal Privoznik wrote: On 6/11/20 3:40 PM, Erik Skultety wrote: On Wed, Jun 10, 2020 at 08:01:47PM +0200, Michal Privoznik wrote: On 6/9/20 11:43 PM, Jonathon Jongsma wrote: Test that we run 'mdevctl' with the proper arguments when creating new mediated devices with virNodeDeviceCreateXML(). Signed-off-by: Jonathon Jongsma --- build-aux/syntax-check.mk | 2 +- tests/Makefile.am | 14 + ...019_36ea_4111_8f0a_8c9a70e21366-start.argv | 1 + ...019_36ea_4111_8f0a_8c9a70e21366-start.json | 1 + ...d39_495e_4243_ad9f_beb3f14c23d9-start.argv | 1 + ...d39_495e_4243_ad9f_beb3f14c23d9-start.json | 1 + ...916_1ca8_49ac_b176_871d16c13076-start.argv | 1 + ...916_1ca8_49ac_b176_871d16c13076-start.json | 1 + tests/nodedevmdevctltest.c| 257 ++ ...v_d069d019_36ea_4111_8f0a_8c9a70e21366.xml | 8 + ...v_d2441d39_495e_4243_ad9f_beb3f14c23d9.xml | 10 + ...v_fedc4916_1ca8_49ac_b176_871d16c13076.xml | 9 + 12 files changed, 305 insertions(+), 1 deletion(-) create mode 100644 tests/nodedevmdevctldata/mdev_d069d019_36ea_4111_8f0a_8c9a70e21366-start.argv create mode 100644 tests/nodedevmdevctldata/mdev_d069d019_36ea_4111_8f0a_8c9a70e21366-start.json create mode 100644 tests/nodedevmdevctldata/mdev_d2441d39_495e_4243_ad9f_beb3f14c23d9-start.argv create mode 100644 tests/nodedevmdevctldata/mdev_d2441d39_495e_4243_ad9f_beb3f14c23d9-start.json create mode 100644 tests/nodedevmdevctldata/mdev_fedc4916_1ca8_49ac_b176_871d16c13076-start.argv create mode 100644 tests/nodedevmdevctldata/mdev_fedc4916_1ca8_49ac_b176_871d16c13076-start.json create mode 100644 tests/nodedevmdevctltest.c create mode 100644 tests/nodedevschemadata/mdev_d069d019_36ea_4111_8f0a_8c9a70e21366.xml create mode 100644 tests/nodedevschemadata/mdev_d2441d39_495e_4243_ad9f_beb3f14c23d9.xml create mode 100644 tests/nodedevschemadata/mdev_fedc4916_1ca8_49ac_b176_871d16c13076.xml diff --git a/build-aux/syntax-check.mk b/build-aux/syntax-check.mk index bf8832a2a5..d47a92b530 100644 --- a/build-aux/syntax-check.mk +++ b/build-aux/syntax-check.mk @@ -2015,7 +2015,7 @@ exclude_file_name_regexp--sc_prohibit_close = \ (\.p[yl]$$|\.spec\.in$$|^docs/|^(src/util/vir(file|event)\.c|src/libvirt-stream\.c|tests/(vir.+mock\.c|commandhelper\.c|qemusecuritymock\.c)|tools/nss/libvirt_nss_(leases|macs)\.c)$$) exclude_file_name_regexp--sc_prohibit_empty_lines_at_EOF = \ - (^tests/(virhostcpu|virpcitest)data/|docs/js/.*\.js|docs/fonts/.*\.woff|\.diff|tests/virconfdata/no-newline\.conf$$) + (^tests/(nodedevmdevctl|virhostcpu|virpcitest)data/|docs/js/.*\.js|docs/fonts/.*\.woff|\.diff|tests/virconfdata/no-newline\.conf$$) exclude_file_name_regexp--sc_prohibit_fork_wrappers = \ (^(src/(util/(vircommand|virdaemon)|lxc/lxc_controller)|tests/testutils)\.c$$) diff --git a/tests/Makefile.am b/tests/Makefile.am index f5766a7790..13cbdbb31e 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -388,6 +388,10 @@ test_programs += storagevolxml2xmltest test_programs += nodedevxml2xmltest +if WITH_NODE_DEVICES +test_programs += nodedevmdevctltest +endif WITH_NODE_DEVICES + test_programs += interfacexml2xmltest test_programs += cputest @@ -970,6 +974,16 @@ nodedevxml2xmltest_SOURCES = \ testutils.c testutils.h nodedevxml2xmltest_LDADD = $(LDADDS) +if WITH_NODE_DEVICES +nodedevmdevctltest_SOURCES = \ + nodedevmdevctltest.c \ + testutils.c testutils.h + +nodedevmdevctltest_LDADD = \ + ../src/libvirt_driver_nodedev_impl.la \ + $(LDADDS) +endif WITH_NODE_DEVICES + interfacexml2xmltest_SOURCES = \ interfacexml2xmltest.c \ testutils.c testutils.h diff --git a/tests/nodedevmdevctldata/mdev_d069d019_36ea_4111_8f0a_8c9a70e21366-start.argv b/tests/nodedevmdevctldata/mdev_d069d019_36ea_4111_8f0a_8c9a70e21366-start.argv new file mode 100644 index 00..dae6dedf7f --- /dev/null +++ b/tests/nodedevmdevctldata/mdev_d069d019_36ea_4111_8f0a_8c9a70e21366-start.argv @@ -0,0 +1 @@ +/usr/sbin/mdevctl start -p :00:02.0 --jsonfile /dev/stdin This is a problem. If there is no mdevctl found during configure (my case), then MDEVCTL is going to be plain "mdevctl". Which is okay for the nodedev driver, because it uses virCommandRun() which uses virFindFileInPath() to get the real path at runtime. But for tests it won't fly. Alternativelly, if the mdevctl lived under say /bin or any other location than /usr/sbin the expected output would be different. In virnetdevbandwidthtest.c (which uses TC) I've worked around this by using TC directly to construct expected output. However, I guess we can safely assume that either mdevctl is present at build time so that its location is Why can we assume it safely? config.log doesn't complain and
Re: [libvirt PATCH v2 3/3] network: wire up support for IPv6 NAT rules
On 6/10/20 12:14 AM, Laine Stump wrote: On 6/9/20 12:17 PM, Daniel P. Berrangé wrote: Now that we have support for IPv6 in the iptables helpers, and a new option in the XML schema, we can wire up support for it in the network driver. Signed-off-by: Daniel P. Berrangé --- src/network/bridge_driver_linux.c | 23 +- .../nat-ipv6-masquerade-linux.args | 228 ++ .../nat-ipv6-masquerade.xml | 17 ++ tests/networkxml2firewalltest.c | 1 + 4 files changed, 262 insertions(+), 7 deletions(-) create mode 100644 tests/networkxml2firewalldata/nat-ipv6-masquerade-linux.args create mode 100644 tests/networkxml2firewalldata/nat-ipv6-masquerade.xml diff --git a/src/network/bridge_driver_linux.c b/src/network/bridge_driver_linux.c index b0bd207250..fcb3803965 100644 --- a/src/network/bridge_driver_linux.c +++ b/src/network/bridge_driver_linux.c @@ -307,7 +307,8 @@ int networkCheckRouteCollision(virNetworkDefPtr def) return ret; } -static const char networkLocalMulticast[] = "224.0.0.0/24"; +static const char networkLocalMulticastIPv4[] = "224.0.0.0/24"; +static const char networkLocalMulticastIPv6[] = "ffx2::/16"; Once I got everything built and tried starting a network with ipv6 nat, I got this error message: virsh net-start ipv6 error: Failed to start network ipv6 error: COMMAND_FAILED: '/usr/sbin/ip6tables -w10 -w --table nat --insert LIBVIRT_PRT --source 2001:4978:2ac:5::/80 --destination ffx2::/16 --jump RETURN' failed: ip6tables v1.8.3 (legacy): host/network `ffx2::' not found Try `ip6tables -h' or 'ip6tables --help' for more information. Do we need to do something different for multicast traffic in the case of IPv6? Other than that it all looks good, so Reviewed-by: Laine Stump once the problem with multicast ffx2::/16 as the destination of a rule is resolved. Based on discussion on IRC, apparently the "x" "ffx2" in the standards docs is intended to mean "any value for this digit", but so far only "ff02" is assigned/used, so we're in agreement that we should just change ffx2 (both here and in the test results file) to ff02.
Re: [PATCH] conf: convert network_conf.c to use g_auto* pointers
On 6/11/20 2:59 AM, Erik Skultety wrote:
On Thu, Jun 11, 2020 at 12:09:34AM -0400, Laine Stump wrote:
On 6/10/20 3:51 AM, Erik Skultety wrote:
On Wed, Jun 10, 2020 at 12:16:50AM -0400, Laine Stump wrote:
This was mostly boilerplate conversion, but in one case I needed to
define several differently named char* to take the place of a single
char *tmp that was re-used multiple times, and in another place there
was a single char* that was used at the toplevel of the function, and
then later used repeatedly inside a for loop, so I defined a new
separate char* inside the loop.
Signed-off-by: Laine Stump
---
This should be applied on top of Dan's IPv6 NAT patch series (it was
reviewing that series that showed me this file hadn't yet been
converted).
...
@@ -689,14 +678,12 @@ virNetworkDHCPDefParseXML(const char *networkName,
if (server &&
virSocketAddrParse(, server, AF_UNSPEC) < 0) {
-VIR_FREE(file);
-VIR_FREE(server);
goto cleanup;
}
def->bootfile = file;
+file = NULL;
g_steal_pointer would do as well
Reviewed-by: Erik Skultety
Well, Duh! Where was my brain while I was doing that mindless conversion??
This made me realized there's actually several places with the x = y; y =
NULL; pattern associated with no-autofree'd pointers. If it's okay with you,
I'll squash the following into the patch before I push it (or, if you'd
prefer I can push it separately):
Yeah, looking at the diff, it would be better to push it in a separate patch.
From ba0a291015766d74eacb81104abf63a85c0690a0 Mon Sep 17 00:00:00 2001
From: Laine Stump
Date: Thu, 11 Jun 2020 00:04:39 -0400
Subject: [PATCH] conf: use g_steal_pointer in network_conf.c
Signed-off-by: Laine Stump
---
src/conf/network_conf.c | 21 +++--
1 file changed, 7 insertions(+), 14 deletions(-)
diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c
index 290111be59..538f038279 100644
--- a/src/conf/network_conf.c
+++ b/src/conf/network_conf.c
@@ -617,12 +617,9 @@ virNetworkDHCPHostDefParseXML(const char *networkName,
cur = cur->next;
}
-host->mac = mac;
-mac = NULL;
-host->id = id;
-id = NULL;
-host->name = name;
-name = NULL;
+host->mac = g_steak_pointer();
s/steak/steal
Reviewed-by: Erik Skultety
Heh. Hadn't even done my final smoke test yet. (That's what happens when
it's midnight and you want to get to bed, but also want to have
something in the other person's inbox for when they wake up :-)
Thanks, and don't worry, I'll be sure to run "all the tests" before I
push :-)
+host->id = g_steal_pointer();
+host->name = g_steal_pointer();
if (ip)
host->ip = inaddr;
@@ -681,8 +678,7 @@ virNetworkDHCPDefParseXML(const char *networkName,
goto cleanup;
}
-def->bootfile = file;
-file = NULL;
+def->bootfile = g_steal_pointer();
def->bootserver = inaddr;
}
@@ -1542,9 +1538,8 @@ virNetworkForwardDefParseXML(const char *networkName,
return -1;
if (forwardDev) {
-def->ifs[0].device.dev = forwardDev;
+def->ifs[0].device.dev = g_steal_pointer();
def->ifs[0].type = VIR_NETWORK_FORWARD_HOSTDEV_DEVICE_NETDEV;
-forwardDev = NULL;
def->nifs++;
}
@@ -1585,8 +1580,7 @@ virNetworkForwardDefParseXML(const char *networkName,
}
}
-def->ifs[i].device.dev = forwardDevi;
-forwardDevi = NULL;
+def->ifs[i].device.dev = g_steal_pointer();
def->ifs[i].type = VIR_NETWORK_FORWARD_HOSTDEV_DEVICE_NETDEV;
def->nifs++;
}
@@ -1662,8 +1656,7 @@ virNetworkForwardDefParseXML(const char *networkName,
return -1;
}
-def->pfs->dev = forwardDev;
-forwardDev = NULL;
+def->pfs->dev = g_steal_pointer();
def->npfs++;
}
--
2.25.4
Re: [libvirt PATCH v2 07/10] nodedev: Add testing for 'mdevctl start'
On Thu, Jun 11, 2020 at 04:15:56PM +0200, Michal Privoznik wrote: > On 6/11/20 3:40 PM, Erik Skultety wrote: > > On Wed, Jun 10, 2020 at 08:01:47PM +0200, Michal Privoznik wrote: > > > On 6/9/20 11:43 PM, Jonathon Jongsma wrote: > > > > Test that we run 'mdevctl' with the proper arguments when creating new > > > > mediated devices with virNodeDeviceCreateXML(). > > > > > > > > Signed-off-by: Jonathon Jongsma > > > > --- > > > >build-aux/syntax-check.mk | 2 +- > > > >tests/Makefile.am | 14 + > > > >...019_36ea_4111_8f0a_8c9a70e21366-start.argv | 1 + > > > >...019_36ea_4111_8f0a_8c9a70e21366-start.json | 1 + > > > >...d39_495e_4243_ad9f_beb3f14c23d9-start.argv | 1 + > > > >...d39_495e_4243_ad9f_beb3f14c23d9-start.json | 1 + > > > >...916_1ca8_49ac_b176_871d16c13076-start.argv | 1 + > > > >...916_1ca8_49ac_b176_871d16c13076-start.json | 1 + > > > >tests/nodedevmdevctltest.c| 257 > > > > ++ > > > >...v_d069d019_36ea_4111_8f0a_8c9a70e21366.xml | 8 + > > > >...v_d2441d39_495e_4243_ad9f_beb3f14c23d9.xml | 10 + > > > >...v_fedc4916_1ca8_49ac_b176_871d16c13076.xml | 9 + > > > >12 files changed, 305 insertions(+), 1 deletion(-) > > > >create mode 100644 > > > > tests/nodedevmdevctldata/mdev_d069d019_36ea_4111_8f0a_8c9a70e21366-start.argv > > > >create mode 100644 > > > > tests/nodedevmdevctldata/mdev_d069d019_36ea_4111_8f0a_8c9a70e21366-start.json > > > >create mode 100644 > > > > tests/nodedevmdevctldata/mdev_d2441d39_495e_4243_ad9f_beb3f14c23d9-start.argv > > > >create mode 100644 > > > > tests/nodedevmdevctldata/mdev_d2441d39_495e_4243_ad9f_beb3f14c23d9-start.json > > > >create mode 100644 > > > > tests/nodedevmdevctldata/mdev_fedc4916_1ca8_49ac_b176_871d16c13076-start.argv > > > >create mode 100644 > > > > tests/nodedevmdevctldata/mdev_fedc4916_1ca8_49ac_b176_871d16c13076-start.json > > > >create mode 100644 tests/nodedevmdevctltest.c > > > >create mode 100644 > > > > tests/nodedevschemadata/mdev_d069d019_36ea_4111_8f0a_8c9a70e21366.xml > > > >create mode 100644 > > > > tests/nodedevschemadata/mdev_d2441d39_495e_4243_ad9f_beb3f14c23d9.xml > > > >create mode 100644 > > > > tests/nodedevschemadata/mdev_fedc4916_1ca8_49ac_b176_871d16c13076.xml > > > > > > > > diff --git a/build-aux/syntax-check.mk b/build-aux/syntax-check.mk > > > > index bf8832a2a5..d47a92b530 100644 > > > > --- a/build-aux/syntax-check.mk > > > > +++ b/build-aux/syntax-check.mk > > > > @@ -2015,7 +2015,7 @@ exclude_file_name_regexp--sc_prohibit_close = \ > > > > > > > > (\.p[yl]$$|\.spec\.in$$|^docs/|^(src/util/vir(file|event)\.c|src/libvirt-stream\.c|tests/(vir.+mock\.c|commandhelper\.c|qemusecuritymock\.c)|tools/nss/libvirt_nss_(leases|macs)\.c)$$) > > > >exclude_file_name_regexp--sc_prohibit_empty_lines_at_EOF = \ > > > > - > > > > (^tests/(virhostcpu|virpcitest)data/|docs/js/.*\.js|docs/fonts/.*\.woff|\.diff|tests/virconfdata/no-newline\.conf$$) > > > > + > > > > (^tests/(nodedevmdevctl|virhostcpu|virpcitest)data/|docs/js/.*\.js|docs/fonts/.*\.woff|\.diff|tests/virconfdata/no-newline\.conf$$) > > > >exclude_file_name_regexp--sc_prohibit_fork_wrappers = \ > > > > > > > > (^(src/(util/(vircommand|virdaemon)|lxc/lxc_controller)|tests/testutils)\.c$$) > > > > diff --git a/tests/Makefile.am b/tests/Makefile.am > > > > index f5766a7790..13cbdbb31e 100644 > > > > --- a/tests/Makefile.am > > > > +++ b/tests/Makefile.am > > > > @@ -388,6 +388,10 @@ test_programs += storagevolxml2xmltest > > > >test_programs += nodedevxml2xmltest > > > > +if WITH_NODE_DEVICES > > > > +test_programs += nodedevmdevctltest > > > > +endif WITH_NODE_DEVICES > > > > + > > > >test_programs += interfacexml2xmltest > > > >test_programs += cputest > > > > @@ -970,6 +974,16 @@ nodedevxml2xmltest_SOURCES = \ > > > > testutils.c testutils.h > > > >nodedevxml2xmltest_LDADD = $(LDADDS) > > > > +if WITH_NODE_DEVICES > > > > +nodedevmdevctltest_SOURCES = \ > > > > + nodedevmdevctltest.c \ > > > > + testutils.c testutils.h > > > > + > > > > +nodedevmdevctltest_LDADD = \ > > > > + ../src/libvirt_driver_nodedev_impl.la \ > > > > + $(LDADDS) > > > > +endif WITH_NODE_DEVICES > > > > + > > > >interfacexml2xmltest_SOURCES = \ > > > > interfacexml2xmltest.c \ > > > > testutils.c testutils.h > > > > diff --git > > > > a/tests/nodedevmdevctldata/mdev_d069d019_36ea_4111_8f0a_8c9a70e21366-start.argv > > > > > > > > b/tests/nodedevmdevctldata/mdev_d069d019_36ea_4111_8f0a_8c9a70e21366-start.argv > > > > new file mode 100644 > > > > index 00..dae6dedf7f > > > > --- /dev/null > > > > +++ > > > > b/tests/nodedevmdevctldata/mdev_d069d019_36ea_4111_8f0a_8c9a70e21366-start.argv > > > > @@ -0,0 +1 @@ > > > > +/usr/sbin/mdevctl start -p :00:02.0 --jsonfile /dev/stdin > > > > > > This is a problem.
Re: [libvirt PATCH v2 09/10] nodedev: Add testing for 'mdevctl stop'
On Tue, Jun 09, 2020 at 04:43:49PM -0500, Jonathon Jongsma wrote:
> Test that we run 'mdevctl' with the proper arguments when we destroy
> mediated devices with virNodeDeviceDestroy()
>
> Signed-off-by: Jonathon Jongsma
> ---
> tests/nodedevmdevctldata/mdevctl-stop.argv | 1 +
> tests/nodedevmdevctltest.c | 43 ++
> 2 files changed, 44 insertions(+)
> create mode 100644 tests/nodedevmdevctldata/mdevctl-stop.argv
>
> diff --git a/tests/nodedevmdevctldata/mdevctl-stop.argv
> b/tests/nodedevmdevctldata/mdevctl-stop.argv
> new file mode 100644
> index 00..25ee7145ce
> --- /dev/null
> +++ b/tests/nodedevmdevctldata/mdevctl-stop.argv
> @@ -0,0 +1 @@
> +/usr/sbin/mdevctl stop -u e2451f73-c95b-4124-b900-e008af37c576
> diff --git a/tests/nodedevmdevctltest.c b/tests/nodedevmdevctltest.c
> index 32a22246c2..58ebf976e2 100644
> --- a/tests/nodedevmdevctltest.c
> +++ b/tests/nodedevmdevctltest.c
> @@ -98,6 +98,42 @@ testMdevctlStartHelper(const void *data)
> jsonfile);
> }
>
> +static int
> +testMdevctlStopHelper(const void *data)
This is not very symmetric to the StartHelper, maybe we can call this one
MdevctlStop directly?
> +{
> +const char *uuid = data;
> +virBuffer buf = VIR_BUFFER_INITIALIZER;
> +const char *actualCmdline = NULL;
> +int ret = -1;
> +g_autoptr(virCommand) cmd = NULL;
> +
> +g_autofree char *cmdlinefile =
> +g_strdup_printf("%s/nodedevmdevctldata/mdevctl-stop.argv",
> +abs_srcdir);
> +
> +cmd = nodeDeviceGetMdevctlStopCommand(uuid, false);
> +
> +if (!cmd)
> +goto cleanup;
> +
> +virCommandSetDryRun(, NULL, NULL);
> +if (virCommandRun(cmd, NULL) < 0)
> +goto cleanup;
> +
> +if (!(actualCmdline = virBufferCurrentContent()))
> +goto cleanup;
> +
> +if (virTestCompareToFile(actualCmdline, cmdlinefile) < 0)
> +goto cleanup;
> +
> +ret = 0;
> +
> + cleanup:
> +virBufferFreeAndReset();
> +virCommandSetDryRun(NULL, NULL, NULL);
> +return ret;
> +}
> +
> static void
> nodedevTestDriverFree(virNodeDeviceDriverStatePtr drv)
> {
> @@ -248,6 +284,13 @@ mymain(void)
> DO_TEST("mdev_fedc4916_1ca8_49ac_b176_871d16c13076");
> DO_TEST("mdev_d2441d39_495e_4243_ad9f_beb3f14c23d9");
>
> +// Test mdevctl stop command, pass an arbitrary uuid
/* ... */
> +if (virTestRun("mdevctl stop", testMdevctlStopHelper,
> + "e2451f73-c95b-4124-b900-e008af37c576") < 0)
Can we remain consistent with the usage of macros and rework DO_TEST_FULL so
that mdevctl stop could be run through the DO_TEST macro?
Erik
Re: [libvirt PATCH v2 10/10] docs: note node device fields that are read-only
On Tue, Jun 09, 2020 at 04:43:50PM -0500, Jonathon Jongsma wrote: > As noted by Erik Skultety, we use the same XML schema to report > existing devices and to define new devices. However, some schema > elements are "read-only". In other words, they are used to report > information from the node device driver and cannot be used to define a > new device. Note these in the documentation. > > Signed-off-by: Jonathon Jongsma > --- Reviewed-by: Erik Skultety
Re: [libvirt PATCH v2 07/10] nodedev: Add testing for 'mdevctl start'
On 6/11/20 3:40 PM, Erik Skultety wrote: On Wed, Jun 10, 2020 at 08:01:47PM +0200, Michal Privoznik wrote: On 6/9/20 11:43 PM, Jonathon Jongsma wrote: Test that we run 'mdevctl' with the proper arguments when creating new mediated devices with virNodeDeviceCreateXML(). Signed-off-by: Jonathon Jongsma --- build-aux/syntax-check.mk | 2 +- tests/Makefile.am | 14 + ...019_36ea_4111_8f0a_8c9a70e21366-start.argv | 1 + ...019_36ea_4111_8f0a_8c9a70e21366-start.json | 1 + ...d39_495e_4243_ad9f_beb3f14c23d9-start.argv | 1 + ...d39_495e_4243_ad9f_beb3f14c23d9-start.json | 1 + ...916_1ca8_49ac_b176_871d16c13076-start.argv | 1 + ...916_1ca8_49ac_b176_871d16c13076-start.json | 1 + tests/nodedevmdevctltest.c| 257 ++ ...v_d069d019_36ea_4111_8f0a_8c9a70e21366.xml | 8 + ...v_d2441d39_495e_4243_ad9f_beb3f14c23d9.xml | 10 + ...v_fedc4916_1ca8_49ac_b176_871d16c13076.xml | 9 + 12 files changed, 305 insertions(+), 1 deletion(-) create mode 100644 tests/nodedevmdevctldata/mdev_d069d019_36ea_4111_8f0a_8c9a70e21366-start.argv create mode 100644 tests/nodedevmdevctldata/mdev_d069d019_36ea_4111_8f0a_8c9a70e21366-start.json create mode 100644 tests/nodedevmdevctldata/mdev_d2441d39_495e_4243_ad9f_beb3f14c23d9-start.argv create mode 100644 tests/nodedevmdevctldata/mdev_d2441d39_495e_4243_ad9f_beb3f14c23d9-start.json create mode 100644 tests/nodedevmdevctldata/mdev_fedc4916_1ca8_49ac_b176_871d16c13076-start.argv create mode 100644 tests/nodedevmdevctldata/mdev_fedc4916_1ca8_49ac_b176_871d16c13076-start.json create mode 100644 tests/nodedevmdevctltest.c create mode 100644 tests/nodedevschemadata/mdev_d069d019_36ea_4111_8f0a_8c9a70e21366.xml create mode 100644 tests/nodedevschemadata/mdev_d2441d39_495e_4243_ad9f_beb3f14c23d9.xml create mode 100644 tests/nodedevschemadata/mdev_fedc4916_1ca8_49ac_b176_871d16c13076.xml diff --git a/build-aux/syntax-check.mk b/build-aux/syntax-check.mk index bf8832a2a5..d47a92b530 100644 --- a/build-aux/syntax-check.mk +++ b/build-aux/syntax-check.mk @@ -2015,7 +2015,7 @@ exclude_file_name_regexp--sc_prohibit_close = \ (\.p[yl]$$|\.spec\.in$$|^docs/|^(src/util/vir(file|event)\.c|src/libvirt-stream\.c|tests/(vir.+mock\.c|commandhelper\.c|qemusecuritymock\.c)|tools/nss/libvirt_nss_(leases|macs)\.c)$$) exclude_file_name_regexp--sc_prohibit_empty_lines_at_EOF = \ - (^tests/(virhostcpu|virpcitest)data/|docs/js/.*\.js|docs/fonts/.*\.woff|\.diff|tests/virconfdata/no-newline\.conf$$) + (^tests/(nodedevmdevctl|virhostcpu|virpcitest)data/|docs/js/.*\.js|docs/fonts/.*\.woff|\.diff|tests/virconfdata/no-newline\.conf$$) exclude_file_name_regexp--sc_prohibit_fork_wrappers = \ (^(src/(util/(vircommand|virdaemon)|lxc/lxc_controller)|tests/testutils)\.c$$) diff --git a/tests/Makefile.am b/tests/Makefile.am index f5766a7790..13cbdbb31e 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -388,6 +388,10 @@ test_programs += storagevolxml2xmltest test_programs += nodedevxml2xmltest +if WITH_NODE_DEVICES +test_programs += nodedevmdevctltest +endif WITH_NODE_DEVICES + test_programs += interfacexml2xmltest test_programs += cputest @@ -970,6 +974,16 @@ nodedevxml2xmltest_SOURCES = \ testutils.c testutils.h nodedevxml2xmltest_LDADD = $(LDADDS) +if WITH_NODE_DEVICES +nodedevmdevctltest_SOURCES = \ + nodedevmdevctltest.c \ + testutils.c testutils.h + +nodedevmdevctltest_LDADD = \ + ../src/libvirt_driver_nodedev_impl.la \ + $(LDADDS) +endif WITH_NODE_DEVICES + interfacexml2xmltest_SOURCES = \ interfacexml2xmltest.c \ testutils.c testutils.h diff --git a/tests/nodedevmdevctldata/mdev_d069d019_36ea_4111_8f0a_8c9a70e21366-start.argv b/tests/nodedevmdevctldata/mdev_d069d019_36ea_4111_8f0a_8c9a70e21366-start.argv new file mode 100644 index 00..dae6dedf7f --- /dev/null +++ b/tests/nodedevmdevctldata/mdev_d069d019_36ea_4111_8f0a_8c9a70e21366-start.argv @@ -0,0 +1 @@ +/usr/sbin/mdevctl start -p :00:02.0 --jsonfile /dev/stdin This is a problem. If there is no mdevctl found during configure (my case), then MDEVCTL is going to be plain "mdevctl". Which is okay for the nodedev driver, because it uses virCommandRun() which uses virFindFileInPath() to get the real path at runtime. But for tests it won't fly. Alternativelly, if the mdevctl lived under say /bin or any other location than /usr/sbin the expected output would be different. In virnetdevbandwidthtest.c (which uses TC) I've worked around this by using TC directly to construct expected output. However, I guess we can safely assume that either mdevctl is present at build time so that its location is Why can we assume it safely? config.log doesn't complain and since we didn't put this in a BuildRequires stanza, even if you do dnf builddep, you won't get the program. Note that the situation with TC is
Re: [libvirt PATCH v2 08/10] nodedev: add mdev support to virNodeDeviceDestroy()
On Tue, Jun 09, 2020 at 04:43:48PM -0500, Jonathon Jongsma wrote:
> Add the ability to destroy mdev node devices via the mdevctl utility.
>
> Signed-off-by: Jonathon Jongsma
> ---
> src/node_device/node_device_driver.c | 46
> src/node_device/node_device_driver.h | 3 ++
> 2 files changed, 49 insertions(+)
>
> diff --git a/src/node_device/node_device_driver.c
> b/src/node_device/node_device_driver.c
> index dbc7eb4d1e..c956bb55fc 100644
> --- a/src/node_device/node_device_driver.c
> +++ b/src/node_device/node_device_driver.c
> @@ -790,6 +790,45 @@ nodeDeviceCreateXML(virConnectPtr conn,
> }
>
>
> +virCommandPtr
> +nodeDeviceGetMdevctlStopCommand(const char *uuid,
> +bool persist)
> +{
> +g_autofree char *mdevctl = virFindFileInPath(MDEVCTL);
> +const char *subcommand;
> +
> +if (!mdevctl)
> +return NULL;
> +
> +if (persist)
> +subcommand = "undefine";
"undefine" is a NOP on active mdevs IIRC, so again the helper name is
confusing.
> +else
> +subcommand = "stop";
> +
> +virCommandPtr cmd = virCommandNewArgList(mdevctl,
> + subcommand,
> + "-u",
> + uuid,
> + NULL);
> +
> +return cmd;
> +}
Like I mentioned already, we could have a generic translator to the mdevctl
subcommands.
Regards,
Erik
Re: [libvirt PATCH v2 07/10] nodedev: Add testing for 'mdevctl start'
On Tue, Jun 09, 2020 at 04:43:47PM -0500, Jonathon Jongsma wrote:
> Test that we run 'mdevctl' with the proper arguments when creating new
> mediated devices with virNodeDeviceCreateXML().
>
> Signed-off-by: Jonathon Jongsma
> ---
...
> +
> +// this function will set a stdin buffer containing the json
> configuration
> +// of the device. This json value is captured in the mock wrapper above
nitpick: /* ... */ instead of //
> +cmd = nodeDeviceGetMdevctlStartCommand(def, false, );
> +
> +if (!cmd)
> +goto cleanup;
> +
> +virCommandSetDryRun(, testCommandDryRunCallback, );
> +if (virCommandRun(cmd, NULL) < 0)
> +goto cleanup;
> +
> +if (!(actualCmdline = virBufferCurrentContent()))
> +goto cleanup;
> +
> +if (virTestCompareToFile(actualCmdline, startcmdfile) < 0)
> +goto cleanup;
> +
> +if (virTestCompareToFile(stdinbuf, startjsonfile) < 0)
> +goto cleanup;
> +
> +ret = 0;
> +
> + cleanup:
> +virBufferFreeAndReset();
> +virCommandSetDryRun(NULL, NULL, NULL);
> +virNodeDeviceObjEndAPI();
> +return ret;
> +}
> +
> +static int
> +testMdevctlStartHelper(const void *data)
> +{
> +const struct testInfo *info = data;
> +
> +g_autofree char *mdevxml = g_strdup_printf("%s/nodedevschemadata/%s.xml",
> + abs_srcdir, info->filename);
> +g_autofree char *cmdlinefile =
> g_strdup_printf("%s/nodedevmdevctldata/%s-start.argv",
> + abs_srcdir,
> info->filename);
> +g_autofree char *jsonfile =
> g_strdup_printf("%s/nodedevmdevctldata/%s-start.json",
> + abs_srcdir,
> info->filename);
> +
> +return testMdevctlStart(info->shouldFail, info->virt_type,
> +info->create, mdevxml, cmdlinefile,
> +jsonfile);
> +}
> +
> +static void
> +nodedevTestDriverFree(virNodeDeviceDriverStatePtr drv)
> +{
> +if (!drv)
> +return;
> +
> +virCondDestroy(>initCond);
> +virMutexDestroy(>lock);
> +VIR_FREE(drv->stateDir);
> +VIR_FREE(drv);
> +}
> +
> +/* Add a fake root 'computer' device */
> +static virNodeDeviceDefPtr
> +fakeRootDevice(void)
> +{
> +virNodeDeviceDefPtr def = NULL;
> +
> +if (VIR_ALLOC(def) != 0 || VIR_ALLOC(def->caps) != 0) {
> +virNodeDeviceDefFree(def);
> +return NULL;
> +}
> +
> +def->name = g_strdup("computer");
> +
> +return def;
> +}
> +
> +/* Add a fake pci device that can be used as a parent device for mediated
> + * devices. For our purposes, it only needs to have a name that matches the
> + * parent of the mdev, and it needs a PCI address
> + */
> +static virNodeDeviceDefPtr
> +fakeParentDevice(void)
> +{
> +virNodeDeviceDefPtr def = NULL;
> +virNodeDevCapPCIDevPtr pci_dev;
> +
> +if (VIR_ALLOC(def) != 0 || VIR_ALLOC(def->caps) != 0) {
> +virNodeDeviceDefFree(def);
> +return NULL;
> +}
> +
> +def->name = g_strdup("pci__00_02_0");
> +def->parent = g_strdup("computer");
> +
> +def->caps->data.type = VIR_NODE_DEV_CAP_PCI_DEV;
> +pci_dev = >caps->data.pci_dev;
> +pci_dev->domain = 0;
> +pci_dev->bus = 0;
> +pci_dev->slot = 2;
> +pci_dev->function = 0;
> +
> +return def;
> +}
> +
> +static int
> +addDevice(virNodeDeviceDefPtr def)
> +{
> +if (!def)
> +return -1;
> +
> +virNodeDeviceObjPtr obj = virNodeDeviceObjListAssignDef(driver->devs,
> def);
> +
> +if (!obj) {
> +virNodeDeviceDefFree(def);
> +return -1;
> +}
> +
> +virNodeDeviceObjEndAPI();
> +return 0;
> +}
> +
> +static int
> +nodedevTestDriverAddTestDevices(void)
> +{
> +if (addDevice(fakeRootDevice()) < 0 ||
> +addDevice(fakeParentDevice()) < 0)
> +return -1;
> +
> +return 0;
> +}
> +
> +/* Bare minimum driver init to be able to test nodedev functionality */
> +static int
> +nodedevTestDriverInit(void)
> +{
> +int ret = -1;
> +char statedir[] = abs_builddir "/nodedevstatedir-XX";
> +if (VIR_ALLOC(driver) < 0)
> +return -1;
> +
> +if (!g_mkdtemp(statedir)) {
> +fprintf(stderr, "Cannot create fake stateDir");
> +goto error;
> +}
> +
> +driver->stateDir = g_strdup(statedir);
> +driver->lockFD = -1;
> +if (virMutexInit(>lock) < 0 ||
> +virCondInit(>initCond) < 0) {
> +VIR_TEST_DEBUG("Unable to initialize test nodedev driver");
> +goto error;
> +}
> +
> +if (!(driver->devs = virNodeDeviceObjListNew()))
> +goto error;
> +
> +return 0;
> +
> + error:
> +nodedevTestDriverFree(driver);
> +return ret;
> +}
> +
> +static int
> +mymain(void)
> +{
> +int ret = 0;
> +
> +if (nodedevTestDriverInit() < 0)
> +return EXIT_FAILURE;
> +
> +if (nodedevTestDriverAddTestDevices() < 0) {
> +ret = EXIT_FAILURE;
> +goto
Re: [libvirt PATCH v2 05/10] nodedev: add mdev support to virNodeDeviceCreateXML()
On Tue, Jun 09, 2020 at 04:43:45PM -0500, Jonathon Jongsma wrote:
> With recent additions to the node device xml schema, an xml schema can
> now describe a mdev device sufficiently for libvirt to create and start
> the device using the mdevctl utility.
>
> Note that some of the the configuration for a mediated device must be
> passed to mdevctl as a JSON-formatted file. In order to avoid creating
> and cleaning up temporary files, the JSON is instead fed to stdin and we
> pass the filename /dev/stdin to mdevctl. While this may not be portable,
> neither are mediated devices, so I don't believe it should cause any
> problems.
>
> Signed-off-by: Jonathon Jongsma
> ---
...
>
> +static int
> +virNodeDeviceObjListFindMediatedDeviceByUUIDCallback(const void *payload,
> + const void *name
> G_GNUC_UNUSED,
> + const void *opaque
> G_GNUC_UNUSED)
opaque is not unused.
...
> +
> +
> +virCommandPtr
> +nodeDeviceGetMdevctlStartCommand(virNodeDeviceDefPtr def,
> + bool persist,
> + char **uuid_out)
> +{
> +virCommandPtr cmd;
> +const char *subcommand;
> +g_autofree char *json = NULL;
> +g_autofree char *mdevctl = virFindFileInPath(MDEVCTL);
> +g_autofree char *parent_pci = nodeDeviceFindAddressByName(def->parent);
> +
> +if (!mdevctl)
> +return NULL;
> +
> +if (!parent_pci) {
> +virReportError(VIR_ERR_NO_NODE_DEVICE,
> + _("unable to find PCI address for parent device
> '%s'"), def->parent);
> +return NULL;
> +}
> +
> +if (nodeDeviceDefToMdevctlConfig(def, ) < 0) {
> +virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> + _("couldn't convert node device def to mdevctl
> JSON"));
> +return NULL;
> +}
> +
> +if (persist)
> +subcommand = "define";
"define" doesn't create an mdev unless '--auto' is passed, so having this as
part of MdevctlStartCommand feels confusing, I think this helper should be
named GetMdevctlCmdline and let the caller pass the correct subcommand.
Regards,
Erik
Re: [libvirt PATCH v2 06/10] nodedev: Build a non-loadable driver lib
On Tue, Jun 09, 2020 at 04:43:46PM -0500, Jonathon Jongsma wrote: > In order to test the nodedev driver, we need to link against a > non-loadable module. Similar to other loadable modules already in the > repository, create an _impl library that can be linked against the unit > tests and then create a loadable module from that. > > Signed-off-by: Jonathon Jongsma > --- Reviewed-by: Erik Skultety
Re: [libvirt PATCH v2 04/10] nodedev: store mdev UUID in mdev caps
On Tue, Jun 09, 2020 at 04:43:44PM -0500, Jonathon Jongsma wrote: > In order to allow libvirt to create and start new mediated devices, we > need to be able to verify that the device has been started. In order to > do this, we'll need to save the UUID of newly-discovered devices within > the virNodeDevCapMdev structure. This allows us to search the device > list by UUID and verify whether the expected device has been started. > > Signed-off-by: Jonathon Jongsma > --- Reviewed-by: Erik Skultety
Re: [libvirt PATCH v2 03/10] nodedev: refactor nodeDeviceFindNewDevice()
On Tue, Jun 09, 2020 at 04:43:43PM -0500, Jonathon Jongsma wrote: > In preparation for creating mediated devices in libvirt, we will need to > wait for new mediated devices to be created as well. Refactor > nodeDeviceFindNewDevice() so that we can re-use the main logic from this > function to wait for different device types by passing a different > 'find' function. > > Signed-off-by: Jonathon Jongsma > --- Reviewed-by: Erik Skultety
Re: [libvirt PATCH v2 01/10] nodedev: factor out nodeDeviceHasCapability()
On Tue, Jun 09, 2020 at 04:43:41PM -0500, Jonathon Jongsma wrote: > Currently nodeDeviceCreateXML() and nodeDeviceDestroy() only support > NPIV HBAs, but we want to be able to create mdev devices as well. This > is a first step to enabling that support. > > Signed-off-by: Jonathon Jongsma > --- Reviewed-by: Erik Skultety
Re: [libvirt PATCH v2 02/10] nodedev: add support for mdev attributes
On Tue, Jun 09, 2020 at 04:43:42PM -0500, Jonathon Jongsma wrote: > Mediated devices support arbitrary vendor-specific attributes that can > be attached to a mediated device. These attributes are ordered, and are > written to sysfs in order after a device is created. This patch adds > support for these attributes to the mdev data types and XML schema. > > Signed-off-by: Jonathon Jongsma > --- > docs/formatnode.html.in | 7 + > docs/schemas/nodedev.rng| 6 > src/conf/node_device_conf.c | 59 +++-- > src/conf/node_device_conf.h | 2 ++ > src/libvirt_private.syms| 2 ++ > src/util/virmdev.c | 12 > src/util/virmdev.h | 11 +++ > 7 files changed, 96 insertions(+), 3 deletions(-) > > diff --git a/docs/formatnode.html.in b/docs/formatnode.html.in > index 76eae928de..a46b73254b 100644 > --- a/docs/formatnode.html.in > +++ b/docs/formatnode.html.in > @@ -393,6 +393,13 @@ > which holds the IOMMU group number the mediated device > belongs >to. > > + attr > + > +This optional element can occur multiple times. It > represents a > +vendor-specific attribute that is used to configure this > +mediated device. It has two required attributes: > +name and value. > + > > >ccw > diff --git a/docs/schemas/nodedev.rng b/docs/schemas/nodedev.rng > index fe6ffa0b53..a1ce09af54 100644 > --- a/docs/schemas/nodedev.rng > +++ b/docs/schemas/nodedev.rng > @@ -634,6 +634,12 @@ > > > > + > + > + > + > + > + Only contextually related, but this patch should be preceded by one that makes iommugroup an optional element (this change would have to go to the parser as well). Since before this series, mdev XMLs were either not internally parsed at all or it would have come from inside libvirt, I'm saying this because even though we wouldn't break backwards compatibility, because we'd be relaxing the RNG and parser (which is ok), but I'd still like to see that change to take effect before this series is fully applied. With Michal's comments: Reviewed-by: Erik Skultety
Re: [libvirt PATCH v2 07/10] nodedev: Add testing for 'mdevctl start'
On Wed, Jun 10, 2020 at 08:01:47PM +0200, Michal Privoznik wrote: > On 6/9/20 11:43 PM, Jonathon Jongsma wrote: > > Test that we run 'mdevctl' with the proper arguments when creating new > > mediated devices with virNodeDeviceCreateXML(). > > > > Signed-off-by: Jonathon Jongsma > > --- > > build-aux/syntax-check.mk | 2 +- > > tests/Makefile.am | 14 + > > ...019_36ea_4111_8f0a_8c9a70e21366-start.argv | 1 + > > ...019_36ea_4111_8f0a_8c9a70e21366-start.json | 1 + > > ...d39_495e_4243_ad9f_beb3f14c23d9-start.argv | 1 + > > ...d39_495e_4243_ad9f_beb3f14c23d9-start.json | 1 + > > ...916_1ca8_49ac_b176_871d16c13076-start.argv | 1 + > > ...916_1ca8_49ac_b176_871d16c13076-start.json | 1 + > > tests/nodedevmdevctltest.c| 257 ++ > > ...v_d069d019_36ea_4111_8f0a_8c9a70e21366.xml | 8 + > > ...v_d2441d39_495e_4243_ad9f_beb3f14c23d9.xml | 10 + > > ...v_fedc4916_1ca8_49ac_b176_871d16c13076.xml | 9 + > > 12 files changed, 305 insertions(+), 1 deletion(-) > > create mode 100644 > > tests/nodedevmdevctldata/mdev_d069d019_36ea_4111_8f0a_8c9a70e21366-start.argv > > create mode 100644 > > tests/nodedevmdevctldata/mdev_d069d019_36ea_4111_8f0a_8c9a70e21366-start.json > > create mode 100644 > > tests/nodedevmdevctldata/mdev_d2441d39_495e_4243_ad9f_beb3f14c23d9-start.argv > > create mode 100644 > > tests/nodedevmdevctldata/mdev_d2441d39_495e_4243_ad9f_beb3f14c23d9-start.json > > create mode 100644 > > tests/nodedevmdevctldata/mdev_fedc4916_1ca8_49ac_b176_871d16c13076-start.argv > > create mode 100644 > > tests/nodedevmdevctldata/mdev_fedc4916_1ca8_49ac_b176_871d16c13076-start.json > > create mode 100644 tests/nodedevmdevctltest.c > > create mode 100644 > > tests/nodedevschemadata/mdev_d069d019_36ea_4111_8f0a_8c9a70e21366.xml > > create mode 100644 > > tests/nodedevschemadata/mdev_d2441d39_495e_4243_ad9f_beb3f14c23d9.xml > > create mode 100644 > > tests/nodedevschemadata/mdev_fedc4916_1ca8_49ac_b176_871d16c13076.xml > > > > diff --git a/build-aux/syntax-check.mk b/build-aux/syntax-check.mk > > index bf8832a2a5..d47a92b530 100644 > > --- a/build-aux/syntax-check.mk > > +++ b/build-aux/syntax-check.mk > > @@ -2015,7 +2015,7 @@ exclude_file_name_regexp--sc_prohibit_close = \ > > > > (\.p[yl]$$|\.spec\.in$$|^docs/|^(src/util/vir(file|event)\.c|src/libvirt-stream\.c|tests/(vir.+mock\.c|commandhelper\.c|qemusecuritymock\.c)|tools/nss/libvirt_nss_(leases|macs)\.c)$$) > > exclude_file_name_regexp--sc_prohibit_empty_lines_at_EOF = \ > > - > > (^tests/(virhostcpu|virpcitest)data/|docs/js/.*\.js|docs/fonts/.*\.woff|\.diff|tests/virconfdata/no-newline\.conf$$) > > + > > (^tests/(nodedevmdevctl|virhostcpu|virpcitest)data/|docs/js/.*\.js|docs/fonts/.*\.woff|\.diff|tests/virconfdata/no-newline\.conf$$) > > exclude_file_name_regexp--sc_prohibit_fork_wrappers = \ > > > > (^(src/(util/(vircommand|virdaemon)|lxc/lxc_controller)|tests/testutils)\.c$$) > > diff --git a/tests/Makefile.am b/tests/Makefile.am > > index f5766a7790..13cbdbb31e 100644 > > --- a/tests/Makefile.am > > +++ b/tests/Makefile.am > > @@ -388,6 +388,10 @@ test_programs += storagevolxml2xmltest > > test_programs += nodedevxml2xmltest > > +if WITH_NODE_DEVICES > > +test_programs += nodedevmdevctltest > > +endif WITH_NODE_DEVICES > > + > > test_programs += interfacexml2xmltest > > test_programs += cputest > > @@ -970,6 +974,16 @@ nodedevxml2xmltest_SOURCES = \ > > testutils.c testutils.h > > nodedevxml2xmltest_LDADD = $(LDADDS) > > +if WITH_NODE_DEVICES > > +nodedevmdevctltest_SOURCES = \ > > + nodedevmdevctltest.c \ > > + testutils.c testutils.h > > + > > +nodedevmdevctltest_LDADD = \ > > + ../src/libvirt_driver_nodedev_impl.la \ > > + $(LDADDS) > > +endif WITH_NODE_DEVICES > > + > > interfacexml2xmltest_SOURCES = \ > > interfacexml2xmltest.c \ > > testutils.c testutils.h > > diff --git > > a/tests/nodedevmdevctldata/mdev_d069d019_36ea_4111_8f0a_8c9a70e21366-start.argv > > > > b/tests/nodedevmdevctldata/mdev_d069d019_36ea_4111_8f0a_8c9a70e21366-start.argv > > new file mode 100644 > > index 00..dae6dedf7f > > --- /dev/null > > +++ > > b/tests/nodedevmdevctldata/mdev_d069d019_36ea_4111_8f0a_8c9a70e21366-start.argv > > @@ -0,0 +1 @@ > > +/usr/sbin/mdevctl start -p :00:02.0 --jsonfile /dev/stdin > > This is a problem. If there is no mdevctl found during configure (my case), > then MDEVCTL is going to be plain "mdevctl". Which is okay for the nodedev > driver, because it uses virCommandRun() which uses virFindFileInPath() to > get the real path at runtime. But for tests it won't fly. Alternativelly, if > the mdevctl lived under say /bin or any other location than /usr/sbin the > expected output would be different. > > In virnetdevbandwidthtest.c (which uses TC) I've worked around this by using > TC directly to construct expected output. However, I guess we can safely >
Re: Libvirt compile issues
On Wed, Jun 03, 2020 at 12:24:27PM +0100, Daniel P. Berrangé wrote: > On Tue, Jun 02, 2020 at 03:30:00PM +0300, K. Kahurani wrote: > > -- Forwarded message - > > From: K. Kahurani > > Date: Sun, May 31, 2020 at 2:26 PM > > Subject: Hello > > To: > > > > > > Hello, > > > > Hopefully this finds you well. > > > > This is not a bug report but more or less an inquiry as based on the > > information acquired from Jim Fehlig a while back, it is possible you would > > have some important information regarding this. > > > > It is currently not possible for me to compile libvirt right from the word > > go while configuring with the error [1]. From the look of it and a bit of > > searching on the internet, it does look like the package portablexdr is > > obsolete. > > > > Could it be that this is a known issue? Could it be there already is a > > workaround this? Do you suppose this should go into the mailing list? > > > > [1] > > checking for WIRESHARK_DISSECTOR... no > > checking for xdrmem_create in -lportablexdr... no > > checking for library containing xdrmem_create... no > > configure: error: Cannot find a XDR library > > You're missing a build pre-requisite library for XDR. On Linux distros > this is provided by "tirpc" these days, try libtirpc-devel RPM or > libtirpc-dev Debian package. Thanks a lot! My distribution/distro is openSUSE. Posting this here, for list purposes as someone might find it worthwhile. $zypper in libpciaccess-devel device-mapper-devel rst2html5 libtirpc-devel rpcgen Other packages were probably already installed. $zypper addrepo https://download.opensuse.org/repositories/Publishing/openSUSE_Tumbleweed/Publishing.repo Had to add the above repo for package rst2html5 Sincerely, David > > > 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] qemuxml2*test: Add cases for CPU pinning to large host CPU IDs
Signed-off-by: Jiri Denemark
---
.../cputune-cpuset-big-id.x86_64-latest.args | 39 +
.../cputune-cpuset-big-id.xml | 33 ++
tests/qemuxml2argvtest.c | 1 +
.../cputune-cpuset-big-id.x86_64-latest.xml | 43 +++
tests/qemuxml2xmltest.c | 1 +
5 files changed, 117 insertions(+)
create mode 100644
tests/qemuxml2argvdata/cputune-cpuset-big-id.x86_64-latest.args
create mode 100644 tests/qemuxml2argvdata/cputune-cpuset-big-id.xml
create mode 100644
tests/qemuxml2xmloutdata/cputune-cpuset-big-id.x86_64-latest.xml
diff --git a/tests/qemuxml2argvdata/cputune-cpuset-big-id.x86_64-latest.args
b/tests/qemuxml2argvdata/cputune-cpuset-big-id.x86_64-latest.args
new file mode 100644
index 00..b086ebe776
--- /dev/null
+++ b/tests/qemuxml2argvdata/cputune-cpuset-big-id.x86_64-latest.args
@@ -0,0 +1,39 @@
+LC_ALL=C \
+PATH=/bin \
+HOME=/tmp/lib/domain--1-QEMUGuest1 \
+USER=test \
+LOGNAME=test \
+XDG_DATA_HOME=/tmp/lib/domain--1-QEMUGuest1/.local/share \
+XDG_CACHE_HOME=/tmp/lib/domain--1-QEMUGuest1/.cache \
+XDG_CONFIG_HOME=/tmp/lib/domain--1-QEMUGuest1/.config \
+QEMU_AUDIO_DRV=none \
+/usr/bin/qemu-system-x86_64 \
+-name guest=QEMUGuest1,debug-threads=on \
+-S \
+-object secret,id=masterKey0,format=raw,\
+file=/tmp/lib/domain--1-QEMUGuest1/master-key.aes \
+-machine pc,accel=tcg,usb=off,dump-guest-core=off \
+-cpu qemu64 \
+-m 214 \
+-overcommit mem-lock=off \
+-smp 2,sockets=2,cores=1,threads=1 \
+-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \
+-display none \
+-no-user-config \
+-nodefaults \
+-chardev socket,id=charmonitor,fd=1729,server,nowait \
+-mon chardev=charmonitor,id=monitor,mode=control \
+-rtc base=utc \
+-no-shutdown \
+-no-acpi \
+-boot strict=on \
+-device piix3-usb-uhci,id=usb,bus=pci.0,addr=0x1.0x2 \
+-blockdev '{"driver":"host_device","filename":"/dev/HostVG/QEMUGuest1",\
+"node-name":"libvirt-1-storage","auto-read-only":true,"discard":"unmap"}' \
+-blockdev '{"node-name":"libvirt-1-format","read-only":false,"driver":"raw",\
+"file":"libvirt-1-storage"}' \
+-device ide-hd,bus=ide.0,unit=0,drive=libvirt-1-format,id=ide0-0-0,bootindex=1
\
+-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x2 \
+-sandbox on,obsolete=deny,elevateprivileges=deny,spawn=deny,\
+resourcecontrol=deny \
+-msg timestamp=on
diff --git a/tests/qemuxml2argvdata/cputune-cpuset-big-id.xml
b/tests/qemuxml2argvdata/cputune-cpuset-big-id.xml
new file mode 100644
index 00..52e2df4082
--- /dev/null
+++ b/tests/qemuxml2argvdata/cputune-cpuset-big-id.xml
@@ -0,0 +1,33 @@
+
+ QEMUGuest1
+ c7a5fdbd-edaf-9455-926a-d65c16db1809
+ 219136
+ 219136
+ 2
+
+
+
+
+
+hvm
+
+
+
+ destroy
+ restart
+ destroy
+
+/usr/bin/qemu-system-x86_64
+
+
+
+
+
+
+
+
+
+
+
+
+
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index 02f8846e57..d5b2a21b5a 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -1901,6 +1901,7 @@ mymain(void)
DO_TEST("vcpu-placement-static",
QEMU_CAPS_KVM,
QEMU_CAPS_OBJECT_IOTHREAD);
+DO_TEST_CAPS_LATEST("cputune-cpuset-big-id");
DO_TEST("numatune-memory", NONE);
DO_TEST_PARSE_ERROR("numatune-memory-invalid-nodeset", NONE);
diff --git a/tests/qemuxml2xmloutdata/cputune-cpuset-big-id.x86_64-latest.xml
b/tests/qemuxml2xmloutdata/cputune-cpuset-big-id.x86_64-latest.xml
new file mode 100644
index 00..8405829bb6
--- /dev/null
+++ b/tests/qemuxml2xmloutdata/cputune-cpuset-big-id.x86_64-latest.xml
@@ -0,0 +1,43 @@
+
+ QEMUGuest1
+ c7a5fdbd-edaf-9455-926a-d65c16db1809
+ 219136
+ 219136
+ 2
+
+
+
+
+
+hvm
+
+
+
+qemu64
+
+
+ destroy
+ restart
+ destroy
+
+/usr/bin/qemu-system-x86_64
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c
index 157e686f2a..7fc8a7d61f 100644
--- a/tests/qemuxml2xmltest.c
+++ b/tests/qemuxml2xmltest.c
@@ -553,6 +553,7 @@ mymain(void)
DO_TEST("vcpu-placement-static",
QEMU_CAPS_KVM,
QEMU_CAPS_OBJECT_IOTHREAD);
+DO_TEST_CAPS_LATEST("cputune-cpuset-big-id");
DO_TEST("smp", NONE);
DO_TEST("iothreads", NONE);
--
2.27.0
[libvirt PATCH 1/2] conf: Increase cpuset length limit for CPU pinning
Domains are now allowed to be pinned to host CPUs with IDs up to 16383.
The new limit is as arbitrary as the old one. It's just bigger.
Signed-off-by: Jiri Denemark
---
src/conf/domain_conf.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index bda8fb6bce..41715c75f2 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -2314,7 +2314,7 @@ struct _virDomainHugePage {
unsigned long long size;/* hugepage size in KiB */
};
-#define VIR_DOMAIN_CPUMASK_LEN 1024
+#define VIR_DOMAIN_CPUMASK_LEN 16384
struct _virDomainIOThreadIDDef {
bool autofill;
--
2.27.0
[libvirt PATCH 0/2] conf: Increase cpuset length limit for CPU pinning
The tests in patch 2/2 would fail without the first patch. Jiri Denemark (2): conf: Increase cpuset length limit for CPU pinning qemuxml2*test: Add cases for CPU pinning to large host CPU IDs src/conf/domain_conf.h| 2 +- .../cputune-cpuset-big-id.x86_64-latest.args | 39 + .../cputune-cpuset-big-id.xml | 33 ++ tests/qemuxml2argvtest.c | 1 + .../cputune-cpuset-big-id.x86_64-latest.xml | 43 +++ tests/qemuxml2xmltest.c | 1 + 6 files changed, 118 insertions(+), 1 deletion(-) create mode 100644 tests/qemuxml2argvdata/cputune-cpuset-big-id.x86_64-latest.args create mode 100644 tests/qemuxml2argvdata/cputune-cpuset-big-id.xml create mode 100644 tests/qemuxml2xmloutdata/cputune-cpuset-big-id.x86_64-latest.xml -- 2.27.0
Re: [PATCH 7/7] Remove use of variables passed only to 'VIR_FREE'
On a Thursday in 2020, Ján Tomko wrote: On a Thursday in 2020, Peter Krempa wrote: Compilers are not very good at detecting this problem. Fixed by manual inspection of compilation warnings after replacing 'VIR_FREE' with an empty macro. Also, I see you're building without vz_sdk, I found two more occurrences False alarm, it did not inspect the macro where it was used correctly. Jano with the following coccinelle spatch: @@ identifier i; type T; constant C; @@ -T i = C; <+... when != i -VIR_FREE(i); ...+> Jano signature.asc Description: PGP signature
Re: [PATCH 7/7] Remove use of variables passed only to 'VIR_FREE'
On a Thursday in 2020, Peter Krempa wrote: Compilers are not very good at detecting this problem. Fixed by manual inspection of compilation warnings after replacing 'VIR_FREE' with an empty macro. Also, I see you're building without vz_sdk, I found two more occurrences with the following coccinelle spatch: @@ identifier i; type T; constant C; @@ -T i = C; <+... when != i -VIR_FREE(i); ...+> Jano signature.asc Description: PGP signature
[PATCH 1/2] util: Move virIsDevMapperDevice() to virdevmapper.c
When introducing virdevmapper.c (in v4.3.0-rc1~427) I didn't
realize there is a function that calls in devmapper. The function
is called virIsDevMapperDevice() and lives in virutil.c. Now that
we have a special file for handling devmapper move it there.
Signed-off-by: Michal Privoznik
---
src/libvirt_private.syms | 2 +-
src/storage/parthelper.c | 2 +-
src/storage/storage_backend_disk.c | 1 +
src/util/virdevmapper.c| 24
src/util/virdevmapper.h| 3 +++
src/util/virutil.c | 24
src/util/virutil.h | 2 --
7 files changed, 30 insertions(+), 28 deletions(-)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index a6af44fe1c..247d7f4741 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1943,6 +1943,7 @@ virDBusSetSharedBus;
# util/virdevmapper.h
virDevMapperGetTargets;
+virIsDevMapperDevice;
# util/virdnsmasq.h
@@ -3432,7 +3433,6 @@ virGetUserShell;
virHostGetDRMRenderNode;
virHostHasIOMMU;
virIndexToDiskName;
-virIsDevMapperDevice;
virMemoryLimitIsSet;
virMemoryLimitTruncate;
virMemoryMaxValue;
diff --git a/src/storage/parthelper.c b/src/storage/parthelper.c
index 761a7f93fc..812e90d3cb 100644
--- a/src/storage/parthelper.c
+++ b/src/storage/parthelper.c
@@ -36,10 +36,10 @@
#include
#include
-#include "virutil.h"
#include "virfile.h"
#include "virstring.h"
#include "virgettext.h"
+#include "virdevmapper.h"
/* we don't need to include the full internal.h just for this */
#define STREQ(a, b) (strcmp(a, b) == 0)
diff --git a/src/storage/storage_backend_disk.c
b/src/storage/storage_backend_disk.c
index 35b07abbfe..eae23ec24a 100644
--- a/src/storage/storage_backend_disk.c
+++ b/src/storage/storage_backend_disk.c
@@ -32,6 +32,7 @@
#include "virutil.h"
#include "configmake.h"
#include "virstring.h"
+#include "virdevmapper.h"
#define VIR_FROM_THIS VIR_FROM_STORAGE
diff --git a/src/util/virdevmapper.c b/src/util/virdevmapper.c
index 79dbc3d02a..600e1f6322 100644
--- a/src/util/virdevmapper.c
+++ b/src/util/virdevmapper.c
@@ -212,3 +212,27 @@ virDevMapperGetTargets(const char *path G_GNUC_UNUSED,
return -1;
}
#endif /* ! WITH_DEVMAPPER */
+
+
+#if WITH_DEVMAPPER
+bool
+virIsDevMapperDevice(const char *dev_name)
+{
+struct stat buf;
+
+if (!stat(dev_name, ) &&
+S_ISBLK(buf.st_mode) &&
+dm_is_dm_major(major(buf.st_rdev)))
+return true;
+
+return false;
+}
+
+#else /* ! WITH_DEVMAPPER */
+
+bool
+virIsDevMapperDevice(const char *dev_name G_GNUC_UNUSED)
+{
+return false;
+}
+#endif /* ! WITH_DEVMAPPER */
diff --git a/src/util/virdevmapper.h b/src/util/virdevmapper.h
index 87bbc63cfd..834900692e 100644
--- a/src/util/virdevmapper.h
+++ b/src/util/virdevmapper.h
@@ -25,3 +25,6 @@
int
virDevMapperGetTargets(const char *path,
char ***devPaths) G_GNUC_NO_INLINE;
+
+bool
+virIsDevMapperDevice(const char *dev_name) ATTRIBUTE_NONNULL(1);
diff --git a/src/util/virutil.c b/src/util/virutil.c
index fb46501142..929afb82d0 100644
--- a/src/util/virutil.c
+++ b/src/util/virutil.c
@@ -37,10 +37,6 @@
#include
-#if WITH_DEVMAPPER
-# include
-#endif
-
#ifdef HAVE_GETPWUID_R
# include
# include
@@ -1340,26 +1336,6 @@ void virWaitForDevices(void)
ignore_value(virCommandRun(cmd, ));
}
-#if WITH_DEVMAPPER
-bool
-virIsDevMapperDevice(const char *dev_name)
-{
-struct stat buf;
-
-if (!stat(dev_name, ) &&
-S_ISBLK(buf.st_mode) &&
-dm_is_dm_major(major(buf.st_rdev)))
-return true;
-
-return false;
-}
-#else
-bool virIsDevMapperDevice(const char *dev_name G_GNUC_UNUSED)
-{
-return false;
-}
-#endif
-
bool
virValidateWWN(const char *wwn)
{
diff --git a/src/util/virutil.h b/src/util/virutil.h
index 49b4bf440f..ef01fd9e36 100644
--- a/src/util/virutil.h
+++ b/src/util/virutil.h
@@ -114,8 +114,6 @@ bool virDoesUserExist(const char *name);
bool virDoesGroupExist(const char *name);
-bool virIsDevMapperDevice(const char *dev_name) ATTRIBUTE_NONNULL(1);
-
bool virValidateWWN(const char *wwn);
int virGetDeviceID(const char *path,
--
2.26.2
[PATCH 0/2] virDevMapperGetTargetsImpl: Check for dm major properly
*** BLURB HERE *** Michal Prívozník (2): util: Move virIsDevMapperDevice() to virdevmapper.c virDevMapperGetTargetsImpl: Check for dm major properly src/libvirt_private.syms | 2 +- src/storage/parthelper.c | 2 +- src/storage/storage_backend_disk.c | 1 + src/util/virdevmapper.c| 33 ++ src/util/virdevmapper.h| 3 +++ src/util/virutil.c | 24 -- src/util/virutil.h | 2 -- 7 files changed, 31 insertions(+), 36 deletions(-) -- 2.26.2
[PATCH 2/2] virDevMapperGetTargetsImpl: Check for dm major properly
In v6.4.0-rc1~143 I've introduced a check that is supposed to
return from the function early, if given path is not a dm target.
While the idea is still valid, the implementation had a flaw.
It calls stat() over given path and the uses major(sb.st_dev) to
learn the major of the device. This is then passed to
dm_is_dm_major() which returns true or false depending whether
the device is under devmapper's control or not.
The problem with this approach is in how the major of the device
is obtained - paths managed by devmapper are special files and
thus we want to be using st_rdev instead of st_dev to obtain the
major number. Well, that's what virIsDevMapperDevice() does
already so might as well us that.
Fixes: 01626c668ecfbe465d18799ac4628e6127ea1d47
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1839992
Signed-off-by: Michal Privoznik
---
src/util/virdevmapper.c | 9 +
1 file changed, 1 insertion(+), 8 deletions(-)
diff --git a/src/util/virdevmapper.c b/src/util/virdevmapper.c
index 600e1f6322..40a82285f9 100644
--- a/src/util/virdevmapper.c
+++ b/src/util/virdevmapper.c
@@ -64,7 +64,6 @@ virDevMapperGetTargetsImpl(const char *path,
char ***devPaths_ret,
unsigned int ttl)
{
-struct stat sb;
struct dm_task *dmt = NULL;
struct dm_deps *deps;
struct dm_info info;
@@ -83,13 +82,7 @@ virDevMapperGetTargetsImpl(const char *path,
return ret;
}
-if (stat(path, ) < 0) {
-if (errno == ENOENT)
-return 0;
-return -1;
-}
-
-if (!dm_is_dm_major(major(sb.st_dev)))
+if (!virIsDevMapperDevice(path))
return 0;
if (!(dmt = dm_task_create(DM_DEVICE_DEPS))) {
--
2.26.2
[PATCH] network: Fix a race condition when shutdown & start vm at the same time
when shutdown vm, the qemuProcessStop cleanup virtual interface in two steps:
1. qemuProcessKill kill qemu process, and vif disappeared
2. ovs-vsctl del-port from the brige
if start a vm in the middle of the two steps, the new vm will reused the vif,
but removed from bridge by step 2
Signed-off-by: Bingsong Si
---
src/qemu/qemu_process.c | 8 +---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index d36088ba98..706248815a 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -7483,9 +7483,11 @@ void qemuProcessStop(virQEMUDriverPtr driver,
if (vport->virtPortType == VIR_NETDEV_VPORT_PROFILE_MIDONET) {
ignore_value(virNetDevMidonetUnbindPort(vport));
} else if (vport->virtPortType ==
VIR_NETDEV_VPORT_PROFILE_OPENVSWITCH) {
-ignore_value(virNetDevOpenvswitchRemovePort(
- virDomainNetGetActualBridgeName(net),
- net->ifname));
+virMacAddr mac;
+if (virNetDevGetMAC(net->ifname, ) < 0 ||
!virMacAddrCmp(, >mac))
+ignore_value(virNetDevOpenvswitchRemovePort(
+ virDomainNetGetActualBridgeName(net),
+ net->ifname));
}
}
--
2.18.2
Re: [PATCH 7/7] Remove use of variables passed only to 'VIR_FREE'
On a Thursday in 2020, Peter Krempa wrote: Compilers are not very good at detecting this problem. Fixed by manual inspection of compilation warnings after replacing 'VIR_FREE' with an empty macro. Signed-off-by: Peter Krempa --- src/locking/sanlock_helper.c | 2 -- src/logging/log_daemon.c | 2 -- src/nwfilter/nwfilter_ebiptables_driver.c | 2 -- src/openvz/openvz_driver.c| 2 -- src/security/security_selinux.c | 2 -- src/test/test_driver.c| 2 -- src/util/virstorageencryption.c | 4 src/util/virsysinfo.c | 2 -- src/vmware/vmware_conf.c | 4 src/vmware/vmware_driver.c| 4 tests/networkxml2firewalltest.c | 2 -- tests/xmconfigtest.c | 2 -- tools/virsh-checkpoint.c | 2 -- 13 files changed, 32 deletions(-) Reviewed-by: Ján Tomko Jano signature.asc Description: PGP signature
Re: [PATCH 6/7] vboxDomainScreenshot: Don't pass uninitialized 'screenData' to VIR_FREE
On a Thursday in 2020, Peter Krempa wrote:
If one of the early checks to get screen resolution fails 'screenData'
would be passed to VIR_FREE uninitialized. Unfortunately the compiler
isn't able to detect this when VIR_FREE is implemented using
g_clear_pointer.
Signed-off-by: Peter Krempa
---
src/vbox/vbox_common.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/vbox/vbox_common.c b/src/vbox/vbox_common.c
index a834a971f0..85935ba731 100644
--- a/src/vbox/vbox_common.c
+++ b/src/vbox/vbox_common.c
@@ -7418,7 +7418,7 @@ vboxDomainScreenshot(virDomainPtr dom,
if (display) {
PRUint32 width, height, bitsPerPixel;
PRUint32 screenDataSize;
-PRUint8 *screenData;
+PRUint8 *screenData = NULL;
PRInt32 xOrigin, yOrigin;
rc = gVBoxAPI.UIDisplay.GetScreenResolution(display, screen,
Reviewed-by: Ján Tomko
Jano
signature.asc
Description: PGP signature
Re: [PATCH 5/7] remoteDispatchDomainMigratePrepare: Don't pass uninitialized variable to VIR_FREE
On a Thursday in 2020, Peter Krempa wrote: 'uri_out' may be passed to VIR_FREE uninitialized if 'conn' is NULL. Unfortunately the compiler isn't able to detect this problem when VIR_FREE is implemented using g_clear_pointer. Initialize the variable. Signed-off-by: Peter Krempa --- src/remote/remote_daemon_dispatch.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/remote/remote_daemon_dispatch.c b/src/remote/remote_daemon_dispatch.c index 6f67d2fb30..69e9aa09b9 100644 --- a/src/remote/remote_daemon_dispatch.c +++ b/src/remote/remote_daemon_dispatch.c @@ -3030,7 +3030,7 @@ remoteDispatchDomainMigratePrepare(virNetServerPtr server G_GNUC_UNUSED, char *cookie = NULL; int cookielen = 0; char *uri_in; -char **uri_out; +char **uri_out = NULL; char *dname; int rv = -1; virConnectPtr conn = remoteGetHypervisorConn(client); Same problem is present in: remoteDispatchDomainMigratePrepare2 remoteDispatchDomainMigratePrepare3 remoteDispatchDomainMigratePrepare3Params Reviewed-by: Ján Tomko Jano signature.asc Description: PGP signature
Re: [libvirt PATCH v3 3/4] ci: Use GitLab container registry
On Thu, 2020-06-11 at 10:22 +0100, Daniel P. Berrangé wrote: > On Wed, Jun 10, 2020 at 07:29:53PM +0200, Andrea Bolognani wrote: > > But yeah, maybe I'm overthinking this. If the pipeline produces the > > containers it consumes, then whether you label them as "latest" > > or "master" or "a0sd90lv_k1" doesn't really make any difference, > > because the next pipeline is going to build the containers again > > before using them. > > I just never much like the idea of things growing without bounds, > even if someone else is paying for storage. That's fair. > > There is still one scenario in which reusing the same name could lead > > to unwanted results, however, and that is when two or more pipelines > > are running at the same time. Right now this is allowed, but by > > using resource groups > > > > https://docs.gitlab.com/ee/ci/yaml/#resource_group > > > > we should be able to prevent that from happening. > > NB, running multiple pipelines in parallel is only a problem if those > pipelines actually contain changes to the dockerfile recipes. Otherwise > the rebuild of container images is essentially a no-op. Yeah. I posted a v4 which doesn't really do anything to prevent this scenario from happening: if that turns our to be a real problem in the future, we can introduce resource groups then. -- Andrea Bolognani / Red Hat / Virtualization
[libvirt PATCH v4 2/3] ci: Use GitLab container registry
Instead of using pre-built containers hosted on Quay, build containers as part of the GitLab CI pipeline and upload them to the GitLab container registry for later use. This will not significantly slow down builds, because containers are only rebuilt when the corresponding Dockerfile has been modified. Signed-off-by: Andrea Bolognani --- .gitlab-ci.yml| 255 +- ci/containers/README.rst | 14 + ci/containers/libvirt-centos-7.Dockerfile | 137 ++ ci/containers/libvirt-centos-8.Dockerfile | 108 .../libvirt-centos-stream.Dockerfile | 109 ...libvirt-debian-10-cross-aarch64.Dockerfile | 122 + .../libvirt-debian-10-cross-armv6l.Dockerfile | 120 + .../libvirt-debian-10-cross-armv7l.Dockerfile | 121 + .../libvirt-debian-10-cross-i686.Dockerfile | 121 + .../libvirt-debian-10-cross-mips.Dockerfile | 121 + ...ibvirt-debian-10-cross-mips64el.Dockerfile | 121 + .../libvirt-debian-10-cross-mipsel.Dockerfile | 121 + ...libvirt-debian-10-cross-ppc64le.Dockerfile | 121 + .../libvirt-debian-10-cross-s390x.Dockerfile | 121 + ci/containers/libvirt-debian-10.Dockerfile| 112 .../libvirt-debian-9-cross-aarch64.Dockerfile | 126 + .../libvirt-debian-9-cross-armv6l.Dockerfile | 124 + .../libvirt-debian-9-cross-armv7l.Dockerfile | 125 + .../libvirt-debian-9-cross-mips.Dockerfile| 125 + ...libvirt-debian-9-cross-mips64el.Dockerfile | 125 + .../libvirt-debian-9-cross-mipsel.Dockerfile | 125 + .../libvirt-debian-9-cross-ppc64le.Dockerfile | 125 + .../libvirt-debian-9-cross-s390x.Dockerfile | 125 + ci/containers/libvirt-debian-9.Dockerfile | 116 ...ibvirt-debian-sid-cross-aarch64.Dockerfile | 122 + ...libvirt-debian-sid-cross-armv6l.Dockerfile | 120 + ...libvirt-debian-sid-cross-armv7l.Dockerfile | 121 + .../libvirt-debian-sid-cross-i686.Dockerfile | 121 + .../libvirt-debian-sid-cross-mips.Dockerfile | 121 + ...bvirt-debian-sid-cross-mips64el.Dockerfile | 121 + ...libvirt-debian-sid-cross-mipsel.Dockerfile | 120 + ...ibvirt-debian-sid-cross-ppc64le.Dockerfile | 121 + .../libvirt-debian-sid-cross-s390x.Dockerfile | 121 + ci/containers/libvirt-debian-sid.Dockerfile | 112 ci/containers/libvirt-fedora-31.Dockerfile| 109 ci/containers/libvirt-fedora-32.Dockerfile| 109 ...rt-fedora-rawhide-cross-mingw32.Dockerfile | 130 + ...rt-fedora-rawhide-cross-mingw64.Dockerfile | 130 + .../libvirt-fedora-rawhide.Dockerfile | 110 ci/containers/libvirt-opensuse-151.Dockerfile | 109 ci/containers/libvirt-ubuntu-1804.Dockerfile | 117 ci/containers/libvirt-ubuntu-2004.Dockerfile | 113 ci/containers/refresh | 41 +++ 43 files changed, 5103 insertions(+), 5 deletions(-) create mode 100644 ci/containers/README.rst create mode 100644 ci/containers/libvirt-centos-7.Dockerfile create mode 100644 ci/containers/libvirt-centos-8.Dockerfile create mode 100644 ci/containers/libvirt-centos-stream.Dockerfile create mode 100644 ci/containers/libvirt-debian-10-cross-aarch64.Dockerfile create mode 100644 ci/containers/libvirt-debian-10-cross-armv6l.Dockerfile create mode 100644 ci/containers/libvirt-debian-10-cross-armv7l.Dockerfile create mode 100644 ci/containers/libvirt-debian-10-cross-i686.Dockerfile create mode 100644 ci/containers/libvirt-debian-10-cross-mips.Dockerfile create mode 100644 ci/containers/libvirt-debian-10-cross-mips64el.Dockerfile create mode 100644 ci/containers/libvirt-debian-10-cross-mipsel.Dockerfile create mode 100644 ci/containers/libvirt-debian-10-cross-ppc64le.Dockerfile create mode 100644 ci/containers/libvirt-debian-10-cross-s390x.Dockerfile create mode 100644 ci/containers/libvirt-debian-10.Dockerfile create mode 100644 ci/containers/libvirt-debian-9-cross-aarch64.Dockerfile create mode 100644 ci/containers/libvirt-debian-9-cross-armv6l.Dockerfile create mode 100644 ci/containers/libvirt-debian-9-cross-armv7l.Dockerfile create mode 100644 ci/containers/libvirt-debian-9-cross-mips.Dockerfile create mode 100644 ci/containers/libvirt-debian-9-cross-mips64el.Dockerfile create mode 100644 ci/containers/libvirt-debian-9-cross-mipsel.Dockerfile create mode 100644 ci/containers/libvirt-debian-9-cross-ppc64le.Dockerfile create mode 100644 ci/containers/libvirt-debian-9-cross-s390x.Dockerfile create mode 100644 ci/containers/libvirt-debian-9.Dockerfile create mode 100644 ci/containers/libvirt-debian-sid-cross-aarch64.Dockerfile create mode 100644 ci/containers/libvirt-debian-sid-cross-armv6l.Dockerfile create mode 100644 ci/containers/libvirt-debian-sid-cross-armv7l.Dockerfile create mode 100644
[libvirt PATCH v4 1/3] ci: Use variables to build image names
This removes a lot of repetition and makes the configuration much easier to read. Signed-off-by: Andrea Bolognani Reviewed-by: Daniel P. Berrangé --- .gitlab-ci.yml | 79 -- 1 file changed, 57 insertions(+), 22 deletions(-) diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 8d9313e415..11e106810c 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -17,6 +17,7 @@ stages: # Default native build jobs that are always run .native_build_default_job_template: _build_default_job_definition stage: builds + image: quay.io/libvirt/buildenv-libvirt-$NAME:latest cache: paths: - ccache/ @@ -64,6 +65,7 @@ stages: # Default cross build jobs that are always run .cross_build_default_job_template: _build_default_job_definition stage: builds + image: quay.io/libvirt/buildenv-libvirt-$NAME-cross-$CROSS:latest cache: paths: - ccache/ @@ -89,47 +91,58 @@ stages: x64-debian-9: <<: *native_build_extra_job_definition - image: quay.io/libvirt/buildenv-libvirt-debian-9:latest + variables: +NAME: debian-9 x64-debian-10: <<: *native_build_default_job_definition - image: quay.io/libvirt/buildenv-libvirt-debian-10:latest + variables: +NAME: debian-10 x64-debian-sid: <<: *native_build_extra_job_definition - image: quay.io/libvirt/buildenv-libvirt-debian-sid:latest + variables: +NAME: debian-sid x64-centos-7: <<: *native_build_default_job_definition - image: quay.io/libvirt/buildenv-libvirt-centos-7:latest + variables: +NAME: centos-7 x64-centos-8: <<: *native_build_extra_job_definition - image: quay.io/libvirt/buildenv-libvirt-centos-8:latest + variables: +NAME: centos-8 x64-fedora-31: <<: *native_build_extra_job_definition - image: quay.io/libvirt/buildenv-libvirt-fedora-31:latest + variables: +NAME: fedora-31 x64-fedora-32: <<: *native_build_default_job_definition - image: quay.io/libvirt/buildenv-libvirt-fedora-32:latest + variables: +NAME: fedora-32 x64-fedora-rawhide: <<: *native_build_default_job_definition - image: quay.io/libvirt/buildenv-libvirt-fedora-rawhide:latest + variables: +NAME: fedora-rawhide x64-opensuse-151: <<: *native_build_default_job_definition - image: quay.io/libvirt/buildenv-libvirt-opensuse-151:latest + variables: +NAME: opensuse-151 x64-ubuntu-1804: <<: *native_build_extra_job_definition - image: quay.io/libvirt/buildenv-libvirt-ubuntu-1804:latest + variables: +NAME: ubuntu-1804 x64-ubuntu-2004: <<: *native_build_default_job_definition - image: quay.io/libvirt/buildenv-libvirt-ubuntu-2004:latest + variables: +NAME: ubuntu-2004 x64-freebsd-12-build: <<: *cirrus_build_default_job_definition @@ -146,47 +159,69 @@ x64-macos-1015-build: armv6l-debian-9: <<: *cross_build_extra_job_definition - image: quay.io/libvirt/buildenv-libvirt-debian-9-cross-armv6l:latest + variables: +NAME: debian-9 +CROSS: armv6l mips64el-debian-9: <<: *cross_build_extra_job_definition - image: quay.io/libvirt/buildenv-libvirt-debian-9-cross-mips64el:latest + variables: +NAME: debian-9 +CROSS: mips64el mips-debian-9: <<: *cross_build_extra_job_definition - image: quay.io/libvirt/buildenv-libvirt-debian-9-cross-mips:latest + variables: +NAME: debian-9 +CROSS: mips aarch64-debian-10: <<: *cross_build_extra_job_definition - image: quay.io/libvirt/buildenv-libvirt-debian-10-cross-aarch64:latest + variables: +NAME: debian-10 +CROSS: aarch64 ppc64le-debian-10: <<: *cross_build_extra_job_definition - image: quay.io/libvirt/buildenv-libvirt-debian-10-cross-ppc64le:latest + variables: +NAME: debian-10 +CROSS: ppc64le s390x-debian-10: <<: *cross_build_default_job_definition - image: quay.io/libvirt/buildenv-libvirt-debian-10-cross-s390x:latest + variables: +NAME: debian-10 +CROSS: s390x armv7l-debian-sid: <<: *cross_build_default_job_definition - image: quay.io/libvirt/buildenv-libvirt-debian-sid-cross-armv7l:latest + variables: +NAME: debian-sid +CROSS: armv7l i686-debian-sid: <<: *cross_build_extra_job_definition - image: quay.io/libvirt/buildenv-libvirt-debian-sid-cross-i686:latest + variables: +NAME: debian-sid +CROSS: i686 mipsel-debian-sid: <<: *cross_build_extra_job_definition - image: quay.io/libvirt/buildenv-libvirt-debian-sid-cross-mipsel:latest + variables: +NAME: debian-sid +CROSS: mipsel mingw32-fedora-rawhide: <<: *cross_build_default_job_definition - image: quay.io/libvirt/buildenv-libvirt-fedora-rawhide-cross-mingw32:latest + variables: +NAME: fedora-rawhide +CROSS: mingw32 mingw64-fedora-rawhide: <<: *cross_build_default_job_definition - image: quay.io/libvirt/buildenv-libvirt-fedora-rawhide-cross-mingw64:latest + variables: +NAME: fedora-rawhide +CROSS: mingw64 # This artifact published by this job is downloaded by libvirt.org
[libvirt PATCH v4 3/3] ci: Update build system integration
The ci-* targets need to know where our container images are stored
and how they are called to work, so now that we use the GitLab
container registry instead of Quay some changes are necessary.
Signed-off-by: Andrea Bolognani
---
ci/Makefile | 10 +-
ci/list-images.sh | 24 ++--
2 files changed, 11 insertions(+), 23 deletions(-)
diff --git a/ci/Makefile b/ci/Makefile
index bc1dac11e3..dc8012f33b 100644
--- a/ci/Makefile
+++ b/ci/Makefile
@@ -50,11 +50,11 @@ CI_BUILD_SCRIPT = $(CI_ROOTDIR)/build.sh
# Location of the container images we're going to pull
# Can be useful to overridde to use a locally built
# image instead
-CI_IMAGE_PREFIX = quay.io/libvirt/buildenv-libvirt-
+CI_IMAGE_PREFIX = registry.gitlab.com/libvirt/libvirt/ci-
-# The default tag is ':latest' but if the container
+# The default tag is ':master' but if the container
# repo above uses different conventions this can override it
-CI_IMAGE_TAG = :latest
+CI_IMAGE_TAG = :master
# We delete the virtual root after completion, set
# to 0 if you need to keep it around for debugging
@@ -243,11 +243,11 @@ ci-list-images:
@echo
@echo "Available x86 container images:"
@echo
- @sh list-images.sh "$(CI_ENGINE)" "$(CI_IMAGE_PREFIX)" | grep -v cross
+ @sh list-images.sh "$(CI_IMAGE_PREFIX)" | grep -v cross
@echo
@echo "Available cross-compiler container images:"
@echo
- @sh list-images.sh "$(CI_ENGINE)" "$(CI_IMAGE_PREFIX)" | grep cross
+ @sh list-images.sh "$(CI_IMAGE_PREFIX)" | grep cross
@echo
ci-help:
diff --git a/ci/list-images.sh b/ci/list-images.sh
index 35efdb6982..b85b132253 100644
--- a/ci/list-images.sh
+++ b/ci/list-images.sh
@@ -1,26 +1,14 @@
#!/bin/sh
-engine="$1"
-prefix="$2"
+prefix="${1##registry.gitlab.com/}"
-do_podman() {
-# Podman freaks out if the search term ends with a dash, which ours
-# by default does, so let's strip it. The repository name is the
-# second field in the output, and it already starts with the registry
-podman search --limit 100 "${prefix%-}" | while read _ repo _; do
-echo "$repo"
-done
-}
+PROJECT_ID=192693
-do_docker() {
-# Docker doesn't include the registry name in the output, so we have
-# to add it. The repository name is the first field in the output
-registry="${prefix%%/*}"
-docker search --limit 100 "$prefix" | while read repo _; do
-echo "$registry/$repo"
-done
+all_repos() {
+ curl -s
"https://gitlab.com/api/v4/projects/$PROJECT_ID/registry/repositories?per_page=100;
\
+| tr , '\n' | grep '"path":' | sed 's,"path":",,g;s,"$,,g'
}
-"do_$engine" | grep "^$prefix" | sed "s,^$prefix,,g" | while read repo; do
+all_repos | grep "^$prefix" | sed "s,^$prefix,,g" | while read repo; do
echo "$repo"
done | sort -u
--
2.25.4
[libvirt PATCH v4 0/3] ci: Use GitLab container registry
Changes from [v3]: * use 'latest' as tag name, consistently with how we do it for other repositories; * name the various Dockerfiles the same as the lcitool host instead of processing the name, which again is the same behavior seen in other repositories; * since we're running all builds as part of the same stage now, there is no need to add an additional stage between 'containers' and 'builds', so the corresponding patch has been dropped. Changes from [v2]: * use $CI_COMMIT_REF_SLUG instead of 'master' as tag name, so that it's possible to test changes to the Dockerfiles that affect the subsequent build jobs in a feature branch; * add CentOS Stream; * rename 'preliminary_checks' stage to 'sanity_checks'. Changes from [v1]: * only build containers necessary for extra jobs when said jobs are actually going to run; * rename container build jobs to '$arch-$os-container'; * rename 'other' stage to 'preliminary_checks' and move it before native builds; * simplify build system integration. [v1] https://www.redhat.com/archives/libvir-list/2020-May/msg01183.html [v2] https://www.redhat.com/archives/libvir-list/2020-June/msg00067.html [v3] https://www.redhat.com/archives/libvir-list/2020-June/msg00412.html Andrea Bolognani (3): ci: Use variables to build image names ci: Use GitLab container registry ci: Update build system integration .gitlab-ci.yml| 330 -- ci/Makefile | 10 +- ci/containers/README.rst | 14 + ci/containers/libvirt-centos-7.Dockerfile | 137 ci/containers/libvirt-centos-8.Dockerfile | 108 ++ .../libvirt-centos-stream.Dockerfile | 109 ++ ...libvirt-debian-10-cross-aarch64.Dockerfile | 122 +++ .../libvirt-debian-10-cross-armv6l.Dockerfile | 120 +++ .../libvirt-debian-10-cross-armv7l.Dockerfile | 121 +++ .../libvirt-debian-10-cross-i686.Dockerfile | 121 +++ .../libvirt-debian-10-cross-mips.Dockerfile | 121 +++ ...ibvirt-debian-10-cross-mips64el.Dockerfile | 121 +++ .../libvirt-debian-10-cross-mipsel.Dockerfile | 121 +++ ...libvirt-debian-10-cross-ppc64le.Dockerfile | 121 +++ .../libvirt-debian-10-cross-s390x.Dockerfile | 121 +++ ci/containers/libvirt-debian-10.Dockerfile| 112 ++ .../libvirt-debian-9-cross-aarch64.Dockerfile | 126 +++ .../libvirt-debian-9-cross-armv6l.Dockerfile | 124 +++ .../libvirt-debian-9-cross-armv7l.Dockerfile | 125 +++ .../libvirt-debian-9-cross-mips.Dockerfile| 125 +++ ...libvirt-debian-9-cross-mips64el.Dockerfile | 125 +++ .../libvirt-debian-9-cross-mipsel.Dockerfile | 125 +++ .../libvirt-debian-9-cross-ppc64le.Dockerfile | 125 +++ .../libvirt-debian-9-cross-s390x.Dockerfile | 125 +++ ci/containers/libvirt-debian-9.Dockerfile | 116 ++ ...ibvirt-debian-sid-cross-aarch64.Dockerfile | 122 +++ ...libvirt-debian-sid-cross-armv6l.Dockerfile | 120 +++ ...libvirt-debian-sid-cross-armv7l.Dockerfile | 121 +++ .../libvirt-debian-sid-cross-i686.Dockerfile | 121 +++ .../libvirt-debian-sid-cross-mips.Dockerfile | 121 +++ ...bvirt-debian-sid-cross-mips64el.Dockerfile | 121 +++ ...libvirt-debian-sid-cross-mipsel.Dockerfile | 120 +++ ...ibvirt-debian-sid-cross-ppc64le.Dockerfile | 121 +++ .../libvirt-debian-sid-cross-s390x.Dockerfile | 121 +++ ci/containers/libvirt-debian-sid.Dockerfile | 112 ++ ci/containers/libvirt-fedora-31.Dockerfile| 109 ++ ci/containers/libvirt-fedora-32.Dockerfile| 109 ++ ...rt-fedora-rawhide-cross-mingw32.Dockerfile | 130 +++ ...rt-fedora-rawhide-cross-mingw64.Dockerfile | 130 +++ .../libvirt-fedora-rawhide.Dockerfile | 110 ++ ci/containers/libvirt-opensuse-151.Dockerfile | 109 ++ ci/containers/libvirt-ubuntu-1804.Dockerfile | 117 +++ ci/containers/libvirt-ubuntu-2004.Dockerfile | 113 ++ ci/containers/refresh | 41 +++ ci/list-images.sh | 24 +- 45 files changed, 5169 insertions(+), 48 deletions(-) create mode 100644 ci/containers/README.rst create mode 100644 ci/containers/libvirt-centos-7.Dockerfile create mode 100644 ci/containers/libvirt-centos-8.Dockerfile create mode 100644 ci/containers/libvirt-centos-stream.Dockerfile create mode 100644 ci/containers/libvirt-debian-10-cross-aarch64.Dockerfile create mode 100644 ci/containers/libvirt-debian-10-cross-armv6l.Dockerfile create mode 100644 ci/containers/libvirt-debian-10-cross-armv7l.Dockerfile create mode 100644 ci/containers/libvirt-debian-10-cross-i686.Dockerfile create mode 100644 ci/containers/libvirt-debian-10-cross-mips.Dockerfile create mode 100644 ci/containers/libvirt-debian-10-cross-mips64el.Dockerfile create mode 100644 ci/containers/libvirt-debian-10-cross-mipsel.Dockerfile create mode 100644
Re: [PATCH 4/7] virTPMEmulatorInit: Don't use temporary variable to free path
On a Thursday in 2020, Peter Krempa wrote: Use VIR_FREE directly. Signed-off-by: Peter Krempa --- src/util/virtpm.c | 5 + 1 file changed, 1 insertion(+), 4 deletions(-) Reviewed-by: Ján Tomko Jano signature.asc Description: PGP signature
Re: [PATCH 3/7] cputest: Avoid use of temporary variable in DO_TEST macro
On a Thursday in 2020, Peter Krempa wrote: Use g_free directly to free the returned pointer from virTestLogContentAndReset rather than store it in a temp variable. Needed back when VIR_FREE was mandated in: 8fe454ce90899945b1d16674668a0208657b6e61 Signed-off-by: Peter Krempa --- tests/cputest.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) Reviewed-by: Ján Tomko Jano signature.asc Description: PGP signature
Re: [PATCH 2/7] testVirFindSCSIHostByPCI: Remove unused 'path_addr'
On a Thursday in 2020, Peter Krempa wrote: The path is formatted but then just freed without any use. Unused since its introduction in ef48a1b613d16b699c175c214c0819d85d4c1801 Signed-off-by: Peter Krempa --- tests/scsihosttest.c | 4 1 file changed, 4 deletions(-) Reviewed-by: Ján Tomko Jano signature.asc Description: PGP signature
Re: [PATCH 1/7] libxlDomainMigrationDstPrepareDef: remove use of temporary variable
On a Thursday in 2020, Peter Krempa wrote: We can free 'def->name' directly. Signed-off-by: Peter Krempa --- src/libxl/libxl_migration.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) Reviewed-by: Ján Tomko Jano signature.asc Description: PGP signature
[PATCH 0/7] More cleanups of unused and uninitialized variables
Recent commit showed that compilers are not able to detect unused variables when passed to VIR_FREE, so I fixed all the other instances the patch didn't fix. Peter Krempa (7): libxlDomainMigrationDstPrepareDef: remove use of temporary variable testVirFindSCSIHostByPCI: Remove unused 'path_addr' cputest: Avoid use of temporary variable in DO_TEST macro virTPMEmulatorInit: Don't use temporary variable to free path remoteDispatchDomainMigratePrepare: Don't pass uninitialized variable to VIR_FREE vboxDomainScreenshot: Don't pass uninitialized 'screenData' to VIR_FREE Remove use of variables passed only to 'VIR_FREE' src/libxl/libxl_migration.c | 4 +--- src/locking/sanlock_helper.c | 2 -- src/logging/log_daemon.c | 2 -- src/nwfilter/nwfilter_ebiptables_driver.c | 2 -- src/openvz/openvz_driver.c| 2 -- src/remote/remote_daemon_dispatch.c | 2 +- src/security/security_selinux.c | 2 -- src/test/test_driver.c| 2 -- src/util/virstorageencryption.c | 4 src/util/virsysinfo.c | 2 -- src/util/virtpm.c | 5 + src/vbox/vbox_common.c| 2 +- src/vmware/vmware_conf.c | 4 src/vmware/vmware_driver.c| 4 tests/cputest.c | 4 +--- tests/networkxml2firewalltest.c | 2 -- tests/scsihosttest.c | 4 tests/xmconfigtest.c | 2 -- tools/virsh-checkpoint.c | 2 -- 19 files changed, 5 insertions(+), 48 deletions(-) -- 2.26.2
[PATCH 7/7] Remove use of variables passed only to 'VIR_FREE'
Compilers are not very good at detecting this problem. Fixed by manual
inspection of compilation warnings after replacing 'VIR_FREE' with an
empty macro.
Signed-off-by: Peter Krempa
---
src/locking/sanlock_helper.c | 2 --
src/logging/log_daemon.c | 2 --
src/nwfilter/nwfilter_ebiptables_driver.c | 2 --
src/openvz/openvz_driver.c| 2 --
src/security/security_selinux.c | 2 --
src/test/test_driver.c| 2 --
src/util/virstorageencryption.c | 4
src/util/virsysinfo.c | 2 --
src/vmware/vmware_conf.c | 4
src/vmware/vmware_driver.c| 4
tests/networkxml2firewalltest.c | 2 --
tests/xmconfigtest.c | 2 --
tools/virsh-checkpoint.c | 2 --
13 files changed, 32 deletions(-)
diff --git a/src/locking/sanlock_helper.c b/src/locking/sanlock_helper.c
index 50deccfd67..26f225e639 100644
--- a/src/locking/sanlock_helper.c
+++ b/src/locking/sanlock_helper.c
@@ -50,7 +50,6 @@ main(int argc, char **argv)
const char *uri;
const char *uuid;
virDomainLockFailureAction action;
-char *xml = NULL;
virConnectPtr conn = NULL;
virDomainPtr dom = NULL;
int ret = EXIT_FAILURE;
@@ -102,7 +101,6 @@ main(int argc, char **argv)
virObjectUnref(dom);
if (conn)
virConnectClose(conn);
-VIR_FREE(xml);
return ret;
}
diff --git a/src/logging/log_daemon.c b/src/logging/log_daemon.c
index 7017db2dcc..028f771f14 100644
--- a/src/logging/log_daemon.c
+++ b/src/logging/log_daemon.c
@@ -526,7 +526,6 @@ virLogDaemonPreExecRestart(const char *state_file,
char *state = NULL;
virJSONValuePtr object = virJSONValueNewObject();
char *magic;
-virHashKeyValuePairPtr pairs = NULL;
VIR_DEBUG("Running pre-restart exec");
@@ -577,7 +576,6 @@ virLogDaemonPreExecRestart(const char *state_file,
abort(); /* This should be impossible to reach */
cleanup:
-VIR_FREE(pairs);
VIR_FREE(state);
virJSONValueFree(object);
return -1;
diff --git a/src/nwfilter/nwfilter_ebiptables_driver.c
b/src/nwfilter/nwfilter_ebiptables_driver.c
index 235a002495..6fc8044c8d 100644
--- a/src/nwfilter/nwfilter_ebiptables_driver.c
+++ b/src/nwfilter/nwfilter_ebiptables_driver.c
@@ -3368,7 +3368,6 @@ ebiptablesApplyNewRules(const char *ifname,
bool haveEbtables = false;
bool haveIptables = false;
bool haveIp6tables = false;
-char *errmsg = NULL;
struct ebtablesSubChainInst **subchains = NULL;
size_t nsubchains = 0;
int ret = -1;
@@ -3568,7 +3567,6 @@ ebiptablesApplyNewRules(const char *ifname,
virHashFree(chains_in_set);
virHashFree(chains_out_set);
-VIR_FREE(errmsg);
return ret;
}
diff --git a/src/openvz/openvz_driver.c b/src/openvz/openvz_driver.c
index 0a08c63b1b..79a100c343 100644
--- a/src/openvz/openvz_driver.c
+++ b/src/openvz/openvz_driver.c
@@ -145,7 +145,6 @@ static int openvzSetInitialConfig(virDomainDefPtr vmdef)
{
int ret = -1;
int vpsid;
-char * confdir = NULL;
virCommandPtr cmd = NULL;
if (vmdef->nfss > 1) {
@@ -194,7 +193,6 @@ static int openvzSetInitialConfig(virDomainDefPtr vmdef)
ret = 0;
cleanup:
-VIR_FREE(confdir);
virCommandFree(cmd);
return ret;
diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c
index e6819af26c..7359a45a96 100644
--- a/src/security/security_selinux.c
+++ b/src/security/security_selinux.c
@@ -812,7 +812,6 @@ virSecuritySELinuxGenLabel(virSecurityManagerPtr mgr,
{
int rc = -1;
char *mcs = NULL;
-char *scontext = NULL;
context_t ctx = NULL;
const char *range;
virSecurityLabelDefPtr seclabel;
@@ -949,7 +948,6 @@ virSecuritySELinuxGenLabel(virSecurityManagerPtr mgr,
if (ctx)
context_free(ctx);
-VIR_FREE(scontext);
VIR_FREE(mcs);
VIR_FREE(sens);
diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index 0dc91e1577..7a1db21718 100644
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -8977,7 +8977,6 @@ testDomainCheckpointCreateXML(virDomainPtr domain,
{
testDriverPtr privconn = domain->conn->privateData;
virDomainObjPtr vm = NULL;
-char *xml = NULL;
virDomainMomentObjPtr chk = NULL;
virDomainCheckpointPtr checkpoint = NULL;
virDomainMomentObjPtr current = NULL;
@@ -9064,7 +9063,6 @@ testDomainCheckpointCreateXML(virDomainPtr domain,
}
virDomainObjEndAPI();
-VIR_FREE(xml);
return checkpoint;
}
diff --git a/src/util/virstorageencryption.c b/src/util/virstorageencryption.c
index 34c356b5a3..94ccaf1e9a 100644
--- a/src/util/virstorageencryption.c
+++ b/src/util/virstorageencryption.c
@@ -146,8 +146,6 @@ virStorageEncryptionSecretParse(xmlXPathContextPtr ctxt,
VIR_XPATH_NODE_AUTORESTORE(ctxt);
virStorageEncryptionSecretPtr ret;
char *type_str = NULL;
-char *uuidstr = NULL;
[PATCH 4/7] virTPMEmulatorInit: Don't use temporary variable to free path
Use VIR_FREE directly.
Signed-off-by: Peter Krempa
---
src/util/virtpm.c | 5 +
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/src/util/virtpm.c b/src/util/virtpm.c
index c734bf941a..71c1a2ecb3 100644
--- a/src/util/virtpm.c
+++ b/src/util/virtpm.c
@@ -290,7 +290,6 @@ virTPMEmulatorInit(void)
g_autofree char *path = NULL;
bool findit = *prgs[i].path == NULL;
struct stat statbuf;
-char *tmp;
if (!findit) {
/* has executables changed? */
@@ -303,9 +302,7 @@ virTPMEmulatorInit(void)
}
if (findit) {
-tmp = *prgs[i].path;
-VIR_FREE(tmp);
-*prgs[i].path = NULL;
+VIR_FREE(*prgs[i].path);
path = virFindFileInPath(prgs[i].name);
if (!path) {
--
2.26.2
[PATCH 3/7] cputest: Avoid use of temporary variable in DO_TEST macro
Use g_free directly to free the returned pointer from
virTestLogContentAndReset rather than store it in a temp variable.
Signed-off-by: Peter Krempa
---
tests/cputest.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/tests/cputest.c b/tests/cputest.c
index b66ea7850e..0cf6870574 100644
--- a/tests/cputest.c
+++ b/tests/cputest.c
@@ -997,10 +997,8 @@ mymain(void)
flags, result \
}; \
char *testLabel; \
-char *tmp; \
\
-tmp = virTestLogContentAndReset(); \
-VIR_FREE(tmp); \
+g_free(virTestLogContentAndReset());\
\
testLabel = g_strdup_printf("%s(%s): %s", #api, \
virArchToString(arch), name); \
--
2.26.2
[PATCH 6/7] vboxDomainScreenshot: Don't pass uninitialized 'screenData' to VIR_FREE
If one of the early checks to get screen resolution fails 'screenData'
would be passed to VIR_FREE uninitialized. Unfortunately the compiler
isn't able to detect this when VIR_FREE is implemented using
g_clear_pointer.
Signed-off-by: Peter Krempa
---
src/vbox/vbox_common.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/vbox/vbox_common.c b/src/vbox/vbox_common.c
index a834a971f0..85935ba731 100644
--- a/src/vbox/vbox_common.c
+++ b/src/vbox/vbox_common.c
@@ -7418,7 +7418,7 @@ vboxDomainScreenshot(virDomainPtr dom,
if (display) {
PRUint32 width, height, bitsPerPixel;
PRUint32 screenDataSize;
-PRUint8 *screenData;
+PRUint8 *screenData = NULL;
PRInt32 xOrigin, yOrigin;
rc = gVBoxAPI.UIDisplay.GetScreenResolution(display, screen,
--
2.26.2
[PATCH 5/7] remoteDispatchDomainMigratePrepare: Don't pass uninitialized variable to VIR_FREE
'uri_out' may be passed to VIR_FREE uninitialized if 'conn' is NULL. Unfortunately the compiler isn't able to detect this problem when VIR_FREE is implemented using g_clear_pointer. Initialize the variable. Signed-off-by: Peter Krempa --- src/remote/remote_daemon_dispatch.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/remote/remote_daemon_dispatch.c b/src/remote/remote_daemon_dispatch.c index 6f67d2fb30..69e9aa09b9 100644 --- a/src/remote/remote_daemon_dispatch.c +++ b/src/remote/remote_daemon_dispatch.c @@ -3030,7 +3030,7 @@ remoteDispatchDomainMigratePrepare(virNetServerPtr server G_GNUC_UNUSED, char *cookie = NULL; int cookielen = 0; char *uri_in; -char **uri_out; +char **uri_out = NULL; char *dname; int rv = -1; virConnectPtr conn = remoteGetHypervisorConn(client); -- 2.26.2
[PATCH 2/7] testVirFindSCSIHostByPCI: Remove unused 'path_addr'
The path is formatted but then just freed without any use.
Signed-off-by: Peter Krempa
---
tests/scsihosttest.c | 4
1 file changed, 4 deletions(-)
diff --git a/tests/scsihosttest.c b/tests/scsihosttest.c
index b58318d295..4b2779eabf 100644
--- a/tests/scsihosttest.c
+++ b/tests/scsihosttest.c
@@ -202,12 +202,9 @@ testVirFindSCSIHostByPCI(const void *data G_GNUC_UNUSED)
unsigned int unique_id2 = 2;
const char *pci_addr1 = ":00:1f.1";
const char *pci_addr2 = ":00:1f.2";
-char *path_addr = NULL;
char *ret_host = NULL;
int ret = -1;
-path_addr = g_strdup_printf("%s/%s", abs_srcdir, "sysfs/class/scsi_host");
-
if (!(ret_host = virSCSIHostFindByPCI(TEST_SCSIHOST_CLASS_PATH,
pci_addr1, unique_id1)) ||
STRNEQ(ret_host, "host0"))
@@ -236,7 +233,6 @@ testVirFindSCSIHostByPCI(const void *data G_GNUC_UNUSED)
cleanup:
VIR_FREE(ret_host);
-VIR_FREE(path_addr);
return ret;
}
--
2.26.2
[PATCH 1/7] libxlDomainMigrationDstPrepareDef: remove use of temporary variable
We can free 'def->name' directly.
Signed-off-by: Peter Krempa
---
src/libxl/libxl_migration.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/src/libxl/libxl_migration.c b/src/libxl/libxl_migration.c
index defdda5ed6..9d253346eb 100644
--- a/src/libxl/libxl_migration.c
+++ b/src/libxl/libxl_migration.c
@@ -449,7 +449,6 @@ libxlDomainMigrationDstPrepareDef(libxlDriverPrivatePtr
driver,
{
libxlDriverConfigPtr cfg = libxlDriverConfigGet(driver);
virDomainDefPtr def;
-char *name = NULL;
if (!dom_xml) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
@@ -464,13 +463,12 @@ libxlDomainMigrationDstPrepareDef(libxlDriverPrivatePtr
driver,
goto cleanup;
if (dname) {
-name = def->name;
+VIR_FREE(def->name);
def->name = g_strdup(dname);
}
cleanup:
virObjectUnref(cfg);
-VIR_FREE(name);
return def;
}
--
2.26.2
Re: [libvirt PATCH v3 3/4] ci: Use GitLab container registry
On Wed, Jun 10, 2020 at 07:29:53PM +0200, Andrea Bolognani wrote: > On Wed, 2020-06-10 at 17:11 +0100, Daniel P. Berrangé wrote: > > On Wed, Jun 10, 2020 at 05:34:13PM +0200, Andrea Bolognani wrote: > > > +.container_default_job_template: _default_job_definition > > > + image: docker:stable > > > + stage: containers > > > + services: > > > +- docker:dind > > > + before_script: > > > +- export TAG="$CI_REGISTRY_IMAGE/ci-$NAME:$CI_COMMIT_REF_SLUG" > > > +- export COMMON_TAG="$CI_REGISTRY/libvirt/libvirt/ci-$NAME:master" > > > > This is different to what we've done on all the other repos. I originally > > used this, but noted that it results in a ever growing set of tags being > > published in the container registry, as users will have a new branch name > > for every piece of work. It also means you'll never a get a cache hit > > from the user's registry across feature branches, though that is mitigated > > to by fact that we'll consider the global cache too I guess. > > We can have an additional > > --cache-from $CI_REGISTRY_IMAGE/ci-$NAME:master > > to further reduce the possibility of getting a cache miss. > > Note that you can configure an expiration policy for tags > > > https://docs.gitlab.com/ee/user/packages/container_registry/#managing-project-expiration-policy-through-the-ui > > but apparently it has to happen on a per-project basis instead of > being something that you can set globally for your account. > > Is having a lot of tags such a big problem? It seems like it's not > unusual... See > > https://hub.docker.com/_/nginx?tab=tags > > for an extreme example. > > But yeah, maybe I'm overthinking this. If the pipeline produces the > containers it consumes, then whether you label them as "latest" > or "master" or "a0sd90lv_k1" doesn't really make any difference, > because the next pipeline is going to build the containers again > before using them. I just never much like the idea of things growing without bounds, even if someone else is paying for storage. > There is still one scenario in which reusing the same name could lead > to unwanted results, however, and that is when two or more pipelines > are running at the same time. Right now this is allowed, but by > using resource groups > > https://docs.gitlab.com/ee/ci/yaml/#resource_group > > we should be able to prevent that from happening. NB, running multiple pipelines in parallel is only a problem if those pipelines actually contain changes to the dockerfile recipes. Otherwise the rebuild of container images is essentially a no-op. 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 v2] docs: Document CIRRUS_GITHUB_REPO variable
On Wed, Jun 10, 2020 at 07:32:16PM +0200, Andrea Bolognani wrote: > This needs to be set for every repository for Cirrus CI integration > to work. > > Signed-off-by: Andrea Bolognani > --- > docs/newreposetup.rst | 16 > 1 file changed, 16 insertions(+) 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: [PATCH v2 1/7] util: introduce a parser for kernel cmdline arguments
On Wed, Jun 10, 2020 at 03:55:14PM +0200, Paulo de Rezende Pinatti wrote:
>
> On 08/06/20 16:35, Erik Skultety wrote:
> > On Fri, May 29, 2020 at 12:10:03PM +0200, Paulo de Rezende Pinatti wrote:
> > > Introduce two utility functions to parse a kernel command
> > > line string according to the kernel code parsing rules in
> > > order to enable the caller to perform operations such as
> > > verifying whether certain argument=value combinations are
> > > present or retrieving an argument's value.
> > >
> > > Signed-off-by: Paulo de Rezende Pinatti
> > > Signed-off-by: Boris Fiuczynski
> > > ---
> > > src/libvirt_private.syms | 2 +
> > > src/util/virutil.c | 169 +++
> > > src/util/virutil.h | 17
> > > tests/utiltest.c | 141
> > > 4 files changed, 329 insertions(+)
> > >
> > > diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> > > index a6af44fe1c..a206a943c5 100644
> > > --- a/src/libvirt_private.syms
> > > +++ b/src/libvirt_private.syms
> > > @@ -3433,6 +3433,8 @@ virHostGetDRMRenderNode;
> > > virHostHasIOMMU;
> > > virIndexToDiskName;
> > > virIsDevMapperDevice;
> > > +virKernelCmdlineMatchParam;
> > > +virKernelCmdlineNextParam;
> > > virMemoryLimitIsSet;
> > > virMemoryLimitTruncate;
> > > virMemoryMaxValue;
> > > diff --git a/src/util/virutil.c b/src/util/virutil.c
> > > index fb46501142..749c9d7116 100644
> > > --- a/src/util/virutil.c
> > > +++ b/src/util/virutil.c
> > > @@ -1725,6 +1725,175 @@ virHostGetDRMRenderNode(void)
> > > return ret;
> > > }
> > >
> > > +
> > > +static const char *virKernelCmdlineSkipDbQuote(const char *cmdline,
> >
> > minor nitpick: we can call ^this simply ...SkipQuote, we don't need to be so
> > specific about it being a double quotation mark, do we?
> >
>
> Sure, changed to SkipQuote.
>
> > > + bool *is_quoted)
> > > +{
> > > +if (cmdline[0] == '"') {
> > > +*is_quoted = !(*is_quoted);
> > > +cmdline++;
> > > +}
> > > +return cmdline;
> > > +}
> > > +
> > > +
> > > +static size_t virKernelCmdlineSearchForward(const char *cmdline,
> > > +bool *is_quoted,
> > > +bool include_equal)
> >
> > Hmm, what if instead we tried to find and return the index of the '='
> > character
> > but iterated all the way until the next applicable (i.e. taking quotation
> > into
> > account) space and saved that end of arg/arg=val parameter into **res. The
> > caller of this function would then continue directly from *res with the next
> > arg/arg=val parameter.
> > We could then call this something like virKernelCmdlineFindEqual and return
> > -1
> > if there is no '=' sign, indicating that it's a standalone parameter with no
> > value.
> >
>
> Yes, we can do that. It would look like this:
>
> size_t index;
nitpick: ^this is a standard iteration variable, so naming it "i" might look
more "expected"
> size_t equal_index = 0;
>
> for (index = 0; cmdline[index]; index++) {
> if (!(is_quoted) && g_ascii_isspace(cmdline[index]))
> break;
> if (equal_index == 0 && cmdline[index] == '=') {
> equal_index = index;
> continue;
> }
> virKernelCmdlineSkipQuote(cmdline + index, _quoted);
> }
> *res = cmdline + index;
> return equal_index;
>
>
> I found it more convenient to use 0 rather than -1 so that we can also
> handle the case when the argument itself starts with the equal sign.
Yes, that's fine, but please provide a doc string for this function mentioning
this.
...
>
> So the function would look like this now:
>
> virSkipSpaces();
> cmdline = virKernelCmdlineSkipQuote(cmdline, _quoted);
> equal_index = virKernelCmdlineFindEqual(cmdline, is_quoted, );
>
> if (next == cmdline)
> return next;
>
> /* param has no value */
> if (equal_index == 0) {
> if (is_quoted && next[-1] == '"')
> *param = g_strndup(cmdline, next - cmdline - 1);
> else
> *param = g_strndup(cmdline, next - cmdline);
> return next;
> }
>
> *param = g_strndup(cmdline, equal_index);
>
> if (cmdline[equal_index + 1] == '"') {
> is_quoted = true;
> equal_index++;
> }
>
> if (is_quoted && next[-1] == '"')
> *val = g_strndup(cmdline + equal_index + 1,
> next - cmdline - equal_index - 2);
> else
> *val = g_strndup(cmdline + equal_index + 1,
> next - cmdline - equal_index - 1);
> return next;
>
> There's still a bit of handling required due to the kernel's quoting rules.
> I took the liberty of using 'next' in place of 'res' as I think it conveys
> better its purpose. Let me know if that looks good to you.
That is fine, looks good.
>
> > > +}
> > > +
> > >
Re: [PATCH] conf: remove unused variable
On Thu, Jun 11, 2020 at 11:26:29AM +0800, Yi Li wrote: > Signed-off-by: Yi Li > --- Reviewed-by: Erik Skultety
Re: [PATCH] conf: convert network_conf.c to use g_auto* pointers
On Thu, Jun 11, 2020 at 12:09:34AM -0400, Laine Stump wrote:
> On 6/10/20 3:51 AM, Erik Skultety wrote:
> > On Wed, Jun 10, 2020 at 12:16:50AM -0400, Laine Stump wrote:
> > > This was mostly boilerplate conversion, but in one case I needed to
> > > define several differently named char* to take the place of a single
> > > char *tmp that was re-used multiple times, and in another place there
> > > was a single char* that was used at the toplevel of the function, and
> > > then later used repeatedly inside a for loop, so I defined a new
> > > separate char* inside the loop.
> > >
> > > Signed-off-by: Laine Stump
> > > ---
> > >
> > > This should be applied on top of Dan's IPv6 NAT patch series (it was
> > > reviewing that series that showed me this file hadn't yet been
> > > converted).
> > ...
> >
> > > @@ -689,14 +678,12 @@ virNetworkDHCPDefParseXML(const char *networkName,
> > > if (server &&
> > > virSocketAddrParse(, server, AF_UNSPEC) < 0) {
> > > -VIR_FREE(file);
> > > -VIR_FREE(server);
> > > goto cleanup;
> > > }
> > > def->bootfile = file;
> > > +file = NULL;
> > g_steal_pointer would do as well
> >
> > Reviewed-by: Erik Skultety
> >
>
> Well, Duh! Where was my brain while I was doing that mindless conversion??
>
>
> This made me realized there's actually several places with the x = y; y =
> NULL; pattern associated with no-autofree'd pointers. If it's okay with you,
> I'll squash the following into the patch before I push it (or, if you'd
> prefer I can push it separately):
Yeah, looking at the diff, it would be better to push it in a separate patch.
> From ba0a291015766d74eacb81104abf63a85c0690a0 Mon Sep 17 00:00:00 2001
> From: Laine Stump
> Date: Thu, 11 Jun 2020 00:04:39 -0400
> Subject: [PATCH] conf: use g_steal_pointer in network_conf.c
>
> Signed-off-by: Laine Stump
> ---
> src/conf/network_conf.c | 21 +++--
> 1 file changed, 7 insertions(+), 14 deletions(-)
>
> diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c
> index 290111be59..538f038279 100644
> --- a/src/conf/network_conf.c
> +++ b/src/conf/network_conf.c
> @@ -617,12 +617,9 @@ virNetworkDHCPHostDefParseXML(const char *networkName,
> cur = cur->next;
> }
>
> -host->mac = mac;
> -mac = NULL;
> -host->id = id;
> -id = NULL;
> -host->name = name;
> -name = NULL;
> +host->mac = g_steak_pointer();
s/steak/steal
Reviewed-by: Erik Skultety
> +host->id = g_steal_pointer();
> +host->name = g_steal_pointer();
> if (ip)
> host->ip = inaddr;
>
> @@ -681,8 +678,7 @@ virNetworkDHCPDefParseXML(const char *networkName,
> goto cleanup;
> }
>
> -def->bootfile = file;
> -file = NULL;
> +def->bootfile = g_steal_pointer();
> def->bootserver = inaddr;
> }
>
> @@ -1542,9 +1538,8 @@ virNetworkForwardDefParseXML(const char *networkName,
> return -1;
>
> if (forwardDev) {
> -def->ifs[0].device.dev = forwardDev;
> +def->ifs[0].device.dev = g_steal_pointer();
> def->ifs[0].type = VIR_NETWORK_FORWARD_HOSTDEV_DEVICE_NETDEV;
> -forwardDev = NULL;
> def->nifs++;
> }
>
> @@ -1585,8 +1580,7 @@ virNetworkForwardDefParseXML(const char *networkName,
> }
> }
>
> -def->ifs[i].device.dev = forwardDevi;
> -forwardDevi = NULL;
> +def->ifs[i].device.dev = g_steal_pointer();
> def->ifs[i].type = VIR_NETWORK_FORWARD_HOSTDEV_DEVICE_NETDEV;
> def->nifs++;
> }
> @@ -1662,8 +1656,7 @@ virNetworkForwardDefParseXML(const char *networkName,
> return -1;
> }
>
> -def->pfs->dev = forwardDev;
> -forwardDev = NULL;
> +def->pfs->dev = g_steal_pointer();
> def->npfs++;
> }
>
> --
> 2.25.4
>
Re: [RFC PATCH 00/41] qemu: Rewrite checkpoint code for 'block-dirty-bitmap-populate'
On Tue, Jun 09, 2020 at 17:00:07 +0200, Peter Krempa wrote: > Use 'block-dirty-bitmap-populate' and change how we create bitmaps > corresponding to checkpoints to simplify the code and also properly > integrate with backing chain images created outside of libvirt. > > This patchset changes how we approach checkpoints by keeping one bitmap > per checkpoint and disk and not propagating the bitmaps into overlays on > snapshots. This massively simplifies the code handling all the > operations during blockjobs and backups. > > While the change isn't compatible with checkpoints created previously, > we didn't yet enable the support for checkpoints/backups. > > Note that 'block-dirty-bitmap-populate' is _not_ in finished state in > qemu yet, so I'm posting this as RFC and as reference for qemu > developers of the usefulnes of it. > > > The changes can be fetched by: > > git fetch https://gitlab.com/pipo.sk/libvirt.git checkpoint-bitmap-populate > > Note that the above branch also contains a commit enabling incremental > backup for simpler testing. > > I've pushed the appropriate qemu patches for convenience here: > > git fetch https://gitlab.com/pipo.sk/qemu.git block-dirty-bitmap-populate I'll be posting a new version which doesn't require the 'block-dirty-bitmap-populate' blockjob at the beginning but still simplifies the internals and I'll then base the rest of this series on top of that once the qemu design is ready.
