[libvirt PATCH 6/9] esx: reorder code to avoid need to VIR_FREE mimeType

mimeType is initialized to NULL, and then only set in one place, just before a check (not involving mimeType) that then VIR_FREEs mimeType if it fails. If we just reorder the code to do the check prior to setting mimeType, then there won't be any need to VIR_FREE(mimeType) on failure (because it

[libvirt PATCH 8/9] esx: eliminate unnecessary cleanup: labels and result variables

switching to g_autofree left many cleanup: sections empty. Signed-off-by: Laine Stump --- src/esx/esx_driver.c | 22 ++- src/esx/esx_storage_backend_vmfs.c | 22 +-- src/esx/esx_util.c | 8 ++ src/esx/esx_vi.c | 45

[libvirt PATCH 9/9] esx: replace some VIR_FREE with g_clear_pointer(x, g_free)

These are all cases when 1) the pointer is passed by reference from the caller (ie.e. **) and expects it to be NULL on return if there is an error, or 2) the variable holding the pointer is being checked or re-used in the same function, but not right away. Signed-off-by: Laine Stump ---

[libvirt PATCH 1/9] esx: use g_autofree for char* where it is trivially possible

All of these strings are allocated once, freed once, and are never returned out of the function where they are created, used, and are freed. Signed-off-by: Laine Stump --- src/esx/esx_driver.c | 128 + src/esx/esx_storage_backend_vmfs.c | 102

[libvirt PATCH 4/9] esx: switch VIR_FREE->g_free in esx*Free*()

Although the three functions esxFreePrivate(), esxFreeStreamPrivate(), and esxUtil_FreeParsedUri() are calling VIR_FREE on *object, and so in theory the caller of the function might rely on "object" (the free function's arg) being set to NULL, in practice these functions are only called from a

[libvirt PATCH 7/9] esx: switch VIR_FREE->g_free when the pointer will immediately go out of scope

Or when it will be immediately have a new value assigned to it. Signed-off-by: Laine Stump --- src/esx/esx_driver.c| 4 ++-- src/esx/esx_network_driver.c| 2 +- src/esx/esx_storage_backend_iscsi.c | 4 ++-- src/esx/esx_storage_backend_vmfs.c | 4 ++-- src/esx/esx_util.c

[libvirt PATCH 5/9] esx: use g_steal_pointer+g_autofree on return value

If we put the potential return string into the g_autofreed tmpResult, and the move it to the returned "result" only as a final step ater, we can avoid the need to explicitly VIR_FREE (or g_free) on failure. Signed-off-by: Laine Stump --- src/esx/esx_driver.c | 12 1 file changed, 4

[libvirt PATCH 2/9] esx: use g_autofree when made possible by reducing scope

These strings were being VIR_FREEd multiple times because they were defined at the top of a function, but then set each time through a loop. But they are only used inside that loop, so they can be converted to use g_autofree if their definition is also placed inside that loop. Signed-off-by:

[libvirt PATCH 0/9] eliminate (almost) all VIR_FREE in esx directory

142 more down, 3084 to go :-) This goes through all the standard methods of eliminating VIR_FREE - first converting as many pointers as possible to g_autofree, then converting a few more *Free* functions (that were more questionable than previous Frees) to use g_free, then some trivial

[libvirt PATCH 3/9] esx: fix memory leak by switching to g_autofree

volumeName was defined at the top of the function, then a new string was assigned to it each time through a loop, but after the first iteration of the loop, the previous string wasn't freed before allocating a new string the next time. By reducing the scope of volumeName to be just the loop, and

Re: [libvirt PATCH v2 5/6] qemu: implement virDomainGetMessages API

On 2/9/21 11:01 AM, Daniel P. Berrangé wrote: > Signed-off-by: Daniel P. Berrangé > --- > src/conf/domain_conf.c | 17 > src/conf/domain_conf.h | 1 + > src/libvirt_private.syms | 2 ++ > src/qemu/qemu_domain.h | 3 +++ > src/qemu/qemu_driver.c | 58

Re: [libvirt RFC PATCH 0/5] eliminating VIR_FREE in the *Clear() functions

On 2/12/21 5:25 AM, Daniel P. Berrangé wrote: On Fri, Feb 12, 2021 at 11:07:21AM +0100, Erik Skultety wrote: On Fri, Feb 12, 2021 at 10:43:56AM +0100, Martin Kletzander wrote: On Fri, Feb 12, 2021 at 12:54:02AM -0500, Laine Stump wrote: I've looked at a few of these, and one thing I've found

[PATCH 11/25] virJSONValueObjectInsert: Clear @value on successful insertion

The function takes ownership of @value on success so the proper semantics will be to clear out the @value pointer. Convert @value to a double pointer to do this. Signed-off-by: Peter Krempa --- src/util/virjson.c | 13 +++-- 1 file changed, 7 insertions(+), 6 deletions(-) diff --git

[PATCH 23/25] virJSONParserHandle*: Refactor memory cleanup and drop NULL checks

virJSONValueNew* won't return error nowadays so NULL checks are not necessary. The memory can be cleared via g_autoptr. Signed-off-by: Peter Krempa --- src/util/virjson.c | 68 -- 1 file changed, 24 insertions(+), 44 deletions(-) diff --git

[PATCH 18/25] virMACMapHashDumper: Refactor array addition

Use automatic memory freeing and don't check return value of virJSONValueNewString as it can't fail. Signed-off-by: Peter Krempa --- src/util/virmacmap.c | 8 +++- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/src/util/virmacmap.c b/src/util/virmacmap.c index

[PATCH 13/25] virJSONValue(Array|Object)Append*: Simplify handling of appended object

Use g_autofree for the pointer of the added object and remove the NULL checks for values returned by virJSONValueNew* (except virJSONValueNewNumberDouble) since they can't fail nowadays. Signed-off-by: Peter Krempa --- src/util/virjson.c | 88 ++ 1

[PATCH 22/25] qemuAgentSetVCPUsCommand: Refactor cleanup

Signed-off-by: Peter Krempa --- src/qemu/qemu_agent.c | 39 +-- 1 file changed, 13 insertions(+), 26 deletions(-) diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c index 9aec0fdb4b..9a74b802b8 100644 --- a/src/qemu/qemu_agent.c +++

[PATCH 21/25] qemuMonitorJSONTransactionAdd: Refactor cleanup

Signed-off-by: Peter Krempa --- src/qemu/qemu_monitor_json.c | 25 +++-- 1 file changed, 11 insertions(+), 14 deletions(-) diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 94365c775b..76401c4d3c 100644 --- a/src/qemu/qemu_monitor_json.c +++

[PATCH 19/25] testQEMUSchemaValidateObjectMergeVariantMember: Fix theoretical leak

Signed-off-by: Peter Krempa --- tests/testutilsqemuschema.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/testutilsqemuschema.c b/tests/testutilsqemuschema.c index 101687e657..4bb303a427 100644 --- a/tests/testutilsqemuschema.c +++ b/tests/testutilsqemuschema.c @@

[PATCH 24/25] virJSONValueNewNumber: Take ownership of passed string

Avoid pointless copies of temporary strings when constructing number JSON objects. Signed-off-by: Peter Krempa --- src/util/virjson.c | 36 +--- 1 file changed, 17 insertions(+), 19 deletions(-) diff --git a/src/util/virjson.c b/src/util/virjson.c index

[PATCH 25/25] virJSONParserInsertValue: Take double pointer for @value

The function calls virJSONValueObjectAppend/virJSONValueArrayAppend, so by taking a double pointer we can drop the pointer clearing from callers. Signed-off-by: Peter Krempa --- src/util/virjson.c | 26 ++ 1 file changed, 10 insertions(+), 16 deletions(-) diff --git

[PATCH 14/25] virJSONValueNewArrayFromBitmap: Refactor cleanup

Use g_autoptr for the JSON value objects and remove the cleanup label and inline freeing of objects. Signed-off-by: Peter Krempa --- src/util/virjson.c | 22 +++--- 1 file changed, 7 insertions(+), 15 deletions(-) diff --git a/src/util/virjson.c b/src/util/virjson.c index

[PATCH 20/25] virJSONValueArrayAppend: Clear pointer when taking ownership of passed value

The parent array takes ownership of the inserted value once all checks pass. Don't make the callers second-guess when that happens and modify the function to take a double pointer so that it can be cleared once the ownership is taken. Signed-off-by: Peter Krempa --- src/locking/lock_daemon.c

[PATCH 15/25] virJSONValueObjectAddVArgs: Use autofree for the temporary bitmap

Signed-off-by: Peter Krempa --- src/util/virjson.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/util/virjson.c b/src/util/virjson.c index e4d71d3e09..7b52525797 100644 --- a/src/util/virjson.c +++ b/src/util/virjson.c @@ -305,7 +305,7 @@

[PATCH 17/25] qemuAgentMakeStringsArray: Refactor cleanup

Signed-off-by: Peter Krempa --- src/qemu/qemu_agent.c | 18 ++ 1 file changed, 6 insertions(+), 12 deletions(-) diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c index 4712aeb529..d6816ef9de 100644 --- a/src/qemu/qemu_agent.c +++ b/src/qemu/qemu_agent.c @@ -1175,23

[PATCH 12/25] virJSONValueCopy: Don't use virJSONValue(Object|Array)Append

We know the exact number of keys or array members for the copied objects so we can pre-allocate the arrays rather than inserting into them in a loop incurring realloc copy penalty. Also virJSONValueCopy now can't fail since all of the functions allocating the different cases use just

[PATCH 16/25] virJSONValueObjectAppend: Clear pointer when taking ownership of passed value

The parent object takes ownership of the inserted value once all checks pass. Don't make the callers second-guess when that happens and modify the function to take a double pointer so that it can be cleared once the ownership is taken. Signed-off-by: Peter Krempa --- src/locking/lock_daemon.c

[PATCH 09/25] virLockSpacePreExecRestart: Refactor memory cleanup

Switch to using the 'g_auto*' helpers. Signed-off-by: Peter Krempa --- src/util/virlockspace.c | 51 + 1 file changed, 21 insertions(+), 30 deletions(-) diff --git a/src/util/virlockspace.c b/src/util/virlockspace.c index 7df319004e..1b6b51b649 100644

[PATCH 10/25] qemuAgentMakeCommand: Refactor memory cleanup

Switch to using the 'g_auto*' helpers. Signed-off-by: Peter Krempa --- src/qemu/qemu_agent.c | 29 - 1 file changed, 12 insertions(+), 17 deletions(-) diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c index 51cc00c618..4712aeb529 100644 ---

[PATCH 07/25] virNetServerPreExecRestart: Drop error reporting from virJSONValueObjectAppend* calls

The functions report errors already and the error can nowadays only happen on programmer errors (if the passed virJSONValue isn't an object), which won't happen. Remove the reporting. Signed-off-by: Peter Krempa --- src/rpc/virnetserver.c | 42 ++ 1 file

[PATCH 08/25] virNetServerPreExecRestart: Refactor memory cleanup

Switch to using the 'g_auto*' helpers. Signed-off-by: Peter Krempa --- src/rpc/virnetserver.c | 41 - 1 file changed, 16 insertions(+), 25 deletions(-) diff --git a/src/rpc/virnetserver.c b/src/rpc/virnetserver.c index ab8dafb1bb..ee402e8ea0 100644 ---

[PATCH 06/25] virNetServerClientPreExecRestart: Refactor memory cleanup

Switch to using the 'g_auto*' helpers. Signed-off-by: Peter Krempa --- src/rpc/virnetserverclient.c | 26 +++--- 1 file changed, 11 insertions(+), 15 deletions(-) diff --git a/src/rpc/virnetserverclient.c b/src/rpc/virnetserverclient.c index 4d01e87e21..0789ad9154 100644

[PATCH 05/25] virNetServerServicePreExecRestart: Refactor memory cleanup

Switch to using the 'g_auto*' helpers. Signed-off-by: Peter Krempa --- src/rpc/virnetserverservice.c | 36 ++- 1 file changed, 14 insertions(+), 22 deletions(-) diff --git a/src/rpc/virnetserverservice.c b/src/rpc/virnetserverservice.c index

[PATCH 04/25] virNetDaemonPreExecRestart: Refactor memory cleanup

Switch to using the 'g_auto*' helpers. Signed-off-by: Peter Krempa --- src/rpc/virnetdaemon.c | 27 +++ 1 file changed, 11 insertions(+), 16 deletions(-) diff --git a/src/rpc/virnetdaemon.c b/src/rpc/virnetdaemon.c index 2c18da790b..6132615093 100644 ---

[PATCH 03/25] virLogHandlerPreExecRestart: Refactor memory cleanup

Switch to using the 'g_auto*' helpers. Signed-off-by: Peter Krempa --- src/logging/log_handler.c | 42 --- 1 file changed, 17 insertions(+), 25 deletions(-) diff --git a/src/logging/log_handler.c b/src/logging/log_handler.c index a77c1e0250..cacf9584cd

[PATCH 01/25] virLockDaemonPreExecRestart: Refactor memory cleanup

Switch to using the 'g_auto*' helpers. Signed-off-by: Peter Krempa --- src/locking/lock_daemon.c | 81 +-- 1 file changed, 35 insertions(+), 46 deletions(-) diff --git a/src/locking/lock_daemon.c b/src/locking/lock_daemon.c index 94fe374df6..5913c0cb9c

[PATCH 00/25] Clear pointers in virJSONValue(Object|Array)Append and other cleanups

Peter Krempa (25): virLockDaemonPreExecRestart: Refactor memory cleanup virLogDaemonPreExecRestart: Refactor memory cleanup virLogHandlerPreExecRestart: Refactor memory cleanup virNetDaemonPreExecRestart: Refactor memory cleanup virNetServerServicePreExecRestart: Refactor memory cleanup

[PATCH 02/25] virLogDaemonPreExecRestart: Refactor memory cleanup

Switch to using the 'g_auto*' helpers. Signed-off-by: Peter Krempa --- src/logging/log_daemon.c | 54 +--- 1 file changed, 23 insertions(+), 31 deletions(-) diff --git a/src/logging/log_daemon.c b/src/logging/log_daemon.c index 770f6dd273..ceffa6a368 100644

Re: [PATCH] qemu_shim: URI escape root directory

On Fri, Feb 12, 2021 at 05:42:19PM +0100, Michal Privoznik wrote: > The root directory can be provided by user (or a temporary one is > generated) and is always formatted into connection URI for both > secret driver and QEMU driver, like this: > > qemu:///embed?root=$root > > But if it so

[PATCH] qemu_shim: URI escape root directory

The root directory can be provided by user (or a temporary one is generated) and is always formatted into connection URI for both secret driver and QEMU driver, like this: qemu:///embed?root=$root But if it so happens that there is an URI unfriendly character in root directory or path to it

[libvirt PATCH v2 2/3] ci: Add temporary workaround for Fedora Rawhide

The .repo files for Fedora Rawhide are already pointing to the Fedora 35 key, but all RPMs are still signed with the Fedora 34 key, resulting in GPG key at file:///etc/pki/rpm-gpg/RPM-GPG-KEY-fedora-35-x86_64 (0x9867C58F) is already installed The GPG keys listed for the "Fedora - Rawhide -

[libvirt PATCH v2 3/3] ci: Build on FreeBSD 12.2

The FreeBSD 12.1 image on Cirrus CI is currently broken, but that's okay because a FreeBSD 12.2 image is also available and we'd rather build on the more up-to-date target anyway. Signed-off-by: Andrea Bolognani --- .gitlab-ci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff

[libvirt PATCH v2 1/3] ci: Refresh Dockerfiles

Signed-off-by: Andrea Bolognani --- ci/containers/ci-centos-7.Dockerfile | 3 ++- ci/containers/ci-centos-8.Dockerfile | 3 ++- ci/containers/ci-centos-stream.Dockerfile| 3 ++- ci/containers/ci-debian-10-cross-aarch64.Dockerfile | 3

[libvirt PATCH v2 0/3] ci: Paint the pipeline green

Various workarounds that are necessary due to breakages in external services and distribution archives, plus fixes for a couple of issues that were discovered in the process. Changes from [v1]: * the first three patches have been dropped from the series as they've been pushed already; *

Re: [libvirt PATCH 00/10] [RFC] clang-tidy CI integration

On Fri, 2021-02-12 at 14:12 +, Daniel P. Berrangé wrote: > On Fri, Feb 12, 2021 at 02:25:24PM +0100, Tim Wiederhake wrote: > > "clang-tidy" is a static code analysis tool for c and c++ code. See > > https://listman.redhat.com/archives/libvir-list/2021-January/msg01152.html > > for some issues

Re: [libvirt PATCH 00/10] [RFC] clang-tidy CI integration

On Fri, 2021-02-12 at 14:48 +0100, Ján Tomko wrote: > On a Friday in 2021, Tim Wiederhake wrote: > > "clang-tidy" is a static code analysis tool for c and c++ code. See > > https://listman.redhat.com/archives/libvir-list/2021-January/msg01152.html > > for some issues found by clang-tidy and more

Re: [libvirt PATCH v2 4/4] ci: Makefile: Expose the CI_USER_LOGIN variable for users to use

On Fri, Feb 12, 2021 at 04:04:04PM +0100, Andrea Bolognani wrote: > On Fri, 2021-02-12 at 15:19 +0100, Erik Skultety wrote: > > @@ -239,6 +241,7 @@ ci-help: > > @echo "CI_CLEAN=0 - do not delete '$(CI_SCRATCHDIR)' after > > completion" > > @echo "CI_REUSE=1 -

Re: [libvirt PATCH v2 4/4] ci: Makefile: Expose the CI_USER_LOGIN variable for users to use

On Fri, 2021-02-12 at 15:19 +0100, Erik Skultety wrote: > @@ -239,6 +241,7 @@ ci-help: > @echo "CI_CLEAN=0 - do not delete '$(CI_SCRATCHDIR)' after > completion" > @echo "CI_REUSE=1 - re-use existing '$(CI_SCRATCHDIR)' > content" > @echo "

[libvirt PATCH v2 3/4] ci: Drop the prepare.sh script

The purpose of this script was to prepare a customized environment in the container, but was actually never used and it required the usage of sudo to switch the environment from root's context to a regular user's one. The thing is that once someone needs a custom script they would very likely to

[libvirt PATCH v2 4/4] ci: Makefile: Expose the CI_USER_LOGIN variable for users to use

More often than not I find myself debugging in the containers which means that I need to have root inside, but without manually tweaking the Makefile each time the execution would simply fail thanks to the uid/gid mapping we do. What if we expose the CI_USER_LOGIN variable, so that when needed,

[libvirt PATCH v2 2/4] ci: Run podman command directly without wrapping it with prepare.sh

The prepare.sh script isn't currently used and forces us to make use of sudo to switch the user inside the container from root to $USER which created a problem on our Debian Slim-based containers which don't have the 'sudo' package installed. This patch removes the sudo invocation and instead runs

[libvirt PATCH v2 1/4] ci: Specify the shebang sequence for build.sh

This is necessary for the follow up patch, because the default entrypoint for a Dockerfile is exec. Signed-off-by: Erik Skultety Reviewed-by: Andrea Bolognani --- ci/build.sh | 2 ++ 1 file changed, 2 insertions(+) diff --git a/ci/build.sh b/ci/build.sh index 740b46a935..0f23df1fa3 100644 ---

[libvirt PATCH v2 0/4] ci: Adjustments to remove our dependency on sudo

Our Debian containers don't have sudo pre-installed and the only reason we actually needed it was to run a custom prepare script which as it turns out does nothing really. Since v1: - applied quoting to the variables as asked during the review process - simplified setting of CI_PODMAN_ARGS -

Re: [libvirt PATCH 00/10] [RFC] clang-tidy CI integration

On Fri, Feb 12, 2021 at 02:25:24PM +0100, Tim Wiederhake wrote: > "clang-tidy" is a static code analysis tool for c and c++ code. See > https://listman.redhat.com/archives/libvir-list/2021-January/msg01152.html > for some issues found by clang-tidy and more background information. > > Meson has

Re: [libvirt PATCH 3/4] ci: Expose CI_USER_LOGIN as a Makefile variable for users to use

On Fri, 2021-02-12 at 13:41 +0100, Erik Skultety wrote: > On Fri, Feb 12, 2021 at 01:27:42PM +0100, Andrea Bolognani wrote: > > On Wed, 2021-02-10 at 18:00 +0100, Erik Skultety wrote: > > > # We also need the user's login and home directory to prepare the > > > # environment the way some

Re: [libvirt PATCH 00/10] [RFC] clang-tidy CI integration

On a Friday in 2021, Tim Wiederhake wrote: "clang-tidy" is a static code analysis tool for c and c++ code. See https://listman.redhat.com/archives/libvir-list/2021-January/msg01152.html for some issues found by clang-tidy and more background information. Meson has support for clang-tidy and

[libvirt PATCH 08/10] clang-tidy: Make list of checks explicit

Preparation for next patch. Signed-off-by: Tim Wiederhake --- scripts/run-clang-tidy.py | 24 1 file changed, 20 insertions(+), 4 deletions(-) diff --git a/scripts/run-clang-tidy.py b/scripts/run-clang-tidy.py index 1d1038df0f..945a1f75d4 100755 ---

[libvirt PATCH 06/10] clang-tidy: Allow timeouts

With an empty cache, the first run of the clang-tidy scan in the CI will fail. While the instinctive reaction "press the rerun button" will eventually lead to the situation where enough files are cached that the entire scan fits within the time window, this creates friction for e.g. new

[libvirt PATCH 10/10] clang-tidy: Add CI integration

A better solution would be to have an explicit target that creates all generated files, similar to libvirt-pot-dep. Signed-off-by: Tim Wiederhake --- .gitlab-ci.yml | 88 ++ 1 file changed, 88 insertions(+) diff --git a/.gitlab-ci.yml

[libvirt PATCH 07/10] clang-tidy: Add shuffle

Randomizing the order of files to scan has no impact for local use of the script. The same holds true for use in the CI, if the amount of cached files is big enough for the entire scan to finish before timeout. If the cache is empty or not filled enough to ensure timely completion, randomizing

[libvirt PATCH 09/10] clang-tidy: Disable irrelevant and failing checks

clang-tidy's focus is on c++. Disable all checks that do not apply to the libVirt code base. Also disable all checks that are currently failing, to prevent introduction of new issues, while we work down the list of existing issues and / or decide on disabling some checks permanently.

[libvirt PATCH 03/10] clang-tidy: Filter output

GitLab's CI output is capped at a certain size. Filter out all status messages that do not add value. Signed-off-by: Tim Wiederhake --- scripts/run-clang-tidy.py | 25 +++-- 1 file changed, 23 insertions(+), 2 deletions(-) diff --git a/scripts/run-clang-tidy.py

[libvirt PATCH 05/10] clang-tidy: Add timeout

Defining a custom timeout allows the scan to finish (albeit unsuccessfully) before GitLab's 60 minute limit and thus preserve the cache of already scanned files. A successive run, e.g. when the "rerun" button in GitLab's web interface is clicked, roughly picks up where the previous run stopped,

[libvirt PATCH 02/10] clang-tidy: Run in parallel

Similar to the "official" run-clang-tidy script. Defaults to run one thread per core, making the tool more pleasant to use locally. Signed-off-by: Tim Wiederhake --- scripts/run-clang-tidy.py | 38 +- 1 file changed, 29 insertions(+), 9 deletions(-) diff

[libvirt PATCH 01/10] clang-tidy: Add a simple runner

Run clang-tidy with default configuration on all files listed in the compilation database. Note that the generated files in the build directory have to be built first. The simplest way to achieve this is to build libVirt first. Example: $ meson build && ninja -C build $

[libvirt PATCH 00/10] [RFC] clang-tidy CI integration

"clang-tidy" is a static code analysis tool for c and c++ code. See https://listman.redhat.com/archives/libvir-list/2021-January/msg01152.html for some issues found by clang-tidy and more background information. Meson has support for clang-tidy and generates target named "clang-tidy" if it finds

[libvirt PATCH 04/10] clang-tidy: Add cache

Cache the results of clang-tidy. The cache is keyed, for each file, on: * the file name, * the exact command used to compile the file to detect changes in arguments, * the hash of the preprocessor stage output to detect changes in includes. A later patch also adds the list of enabled checks to

Re: [libvirt PATCH 3/4] ci: Expose CI_USER_LOGIN as a Makefile variable for users to use

On Fri, Feb 12, 2021 at 01:27:42PM +0100, Andrea Bolognani wrote: > On Wed, 2021-02-10 at 18:00 +0100, Erik Skultety wrote: > > # We need the container process to run with current host IDs > > # so that it can access the passed in build directory > > -CI_UID = $(shell id -u) > > -CI_GID =

Re: [libvirt RFC PATCH 0/5] eliminating VIR_FREE in the *Clear() functions

On Fri, Feb 12, 2021 at 12:14:27PM +0100, Michal Privoznik wrote: > On 2/12/21 11:07 AM, Erik Skultety wrote: > > On Fri, Feb 12, 2021 at 10:43:56AM +0100, Martin Kletzander wrote: > > > On Fri, Feb 12, 2021 at 12:54:02AM -0500, Laine Stump wrote: > > > > I've looked at a few of these, and one

Re: [libvirt PATCH 4/4] ci: Drop the prepare.sh script

On Wed, 2021-02-10 at 18:00 +0100, Erik Skultety wrote: > The purpose of this script was to prepare a customized environment in > the container, but was actually never used and it required the usage of > sudo to switch the environment from root's context to a regular user's > one. > The thing is

Re: [libvirt PATCH 3/4] ci: Expose CI_USER_LOGIN as a Makefile variable for users to use

On Wed, 2021-02-10 at 18:00 +0100, Erik Skultety wrote: > # We need the container process to run with current host IDs > # so that it can access the passed in build directory > -CI_UID = $(shell id -u) > -CI_GID = $(shell id -g) > +CI_UID = $(shell id -u $(CI_USER_LOGIN)) > +CI_GID = $(shell id

Re: [PATCH 08/19] storage: Format qcow2v3 volumes by default

On Fri, Feb 12, 2021 at 12:13:58 +, Daniel P. Berrangé wrote: > On Fri, Feb 12, 2021 at 11:55:36AM +0100, Peter Krempa wrote: > > On Fri, Feb 12, 2021 at 10:49:02 +, Daniel Berrange wrote: > > > On Thu, Feb 11, 2021 at 04:37:47PM +0100, Peter Krempa wrote: > > > > Format the new volumes

Re: [PATCH 08/19] storage: Format qcow2v3 volumes by default

On Fri, Feb 12, 2021 at 11:55:36AM +0100, Peter Krempa wrote: > On Fri, Feb 12, 2021 at 10:49:02 +, Daniel Berrange wrote: > > On Thu, Feb 11, 2021 at 04:37:47PM +0100, Peter Krempa wrote: > > > Format the new volumes with 'compat=1.1' since the minimum supported > > > qemu version is now 1.5

Re: [libvirt PATCH 2/4] ci: Run podman command directly without wrapping it with prepare.sh

On Wed, 2021-02-10 at 18:00 +0100, Erik Skultety wrote: > The prepare.sh script isn't currently used and forces us to make use > of sudo to switch the user inside the container from root to $USER > which created a problem on our Debian Slim-based containers which don't > have the 'sudo' package

Re: [libvirt PATCH 1/4] ci: Specify the shebang sequence for build.sh

On Wed, 2021-02-10 at 18:00 +0100, Erik Skultety wrote: > This is necessary for the follow up patch, because the default > entrypoint for a Dockerfile is exec. > > Signed-off-by: Erik Skultety > --- > ci/build.sh | 2 ++ > 1 file changed, 2 insertions(+) Reviewed-by: Andrea Bolognani --

Re: [libvirt RFC PATCH 0/5] eliminating VIR_FREE in the *Clear() functions

On 2/12/21 11:07 AM, Erik Skultety wrote: On Fri, Feb 12, 2021 at 10:43:56AM +0100, Martin Kletzander wrote: On Fri, Feb 12, 2021 at 12:54:02AM -0500, Laine Stump wrote: I've looked at a few of these, and one thing I've found is that very often we have a function called

Re: [PATCH 08/19] storage: Format qcow2v3 volumes by default

On Fri, Feb 12, 2021 at 10:49:02 +, Daniel Berrange wrote: > On Thu, Feb 11, 2021 at 04:37:47PM +0100, Peter Krempa wrote: > > Format the new volumes with 'compat=1.1' since the minimum supported > > qemu version is now 1.5 rather the pre-historic compat=0.10. > > I understand the desire to

Re: [PATCH 08/19] storage: Format qcow2v3 volumes by default

On Thu, Feb 11, 2021 at 04:37:47PM +0100, Peter Krempa wrote: > Format the new volumes with 'compat=1.1' since the minimum supported > qemu version is now 1.5 rather the pre-historic compat=0.10. I understand the desire to do this, but this is none the less a semantic change to the behaviour of

Re: [PATCH 08/19] storage: Format qcow2v3 volumes by default

On Thu, Feb 11, 2021 at 16:37:47 +0100, Peter Krempa wrote: > Format the new volumes with 'compat=1.1' since the minimum supported > qemu version is now 1.5 rather the pre-historic compat=0.10. > > Signed-off-by: Peter Krempa > --- > src/storage/storage_util.c

Re: [PATCH 07/19] storagevolxml2argvdata: Rewrap all output files

On Thu, Feb 11, 2021 at 16:37:46 +0100, Peter Krempa wrote: > Use scripts/test-wrap-argv.py to rewrap the output files so that any > further changes don't introduce churn since we are rewrapping the output > automatically now. > > Signed-off-by: Peter Krempa > --- >

Re: [PATCH 06/19] testutils: virTestRewrapFile: Rewrap also '.argv' files

On Thu, Feb 11, 2021 at 16:37:45 +0100, Peter Krempa wrote: > The suffix is used for output files of 'storagevolxml2argvtest. > > Signed-off-by: Peter Krempa > --- > tests/testutils.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/tests/testutils.c b/tests/testutils.c > index

Re: [libvirt RFC PATCH 0/5] eliminating VIR_FREE in the *Clear() functions

On Fri, Feb 12, 2021 at 11:07:21AM +0100, Erik Skultety wrote: > On Fri, Feb 12, 2021 at 10:43:56AM +0100, Martin Kletzander wrote: > > On Fri, Feb 12, 2021 at 12:54:02AM -0500, Laine Stump wrote: > > > I've looked at a few of these, and one thing I've found is that very > > > often we have a

Re: [PATCH 05/19] qemuMigrationSrcPerformPeer2Peer3: Don't leak 'dom_xml' on cleanup

On Thu, Feb 11, 2021 at 16:37:44 +0100, Peter Krempa wrote: > Use g_autofree for 'dom_xml' to free it on some of the (unlikely) code > paths jumping to cleanup prior to the deallocation which is done right > after it's not needed any more since it's a big string. > > Noticed when running under

Re: [PATCH 04/19] virDomainMigrateVersion3Full: Don't set 'cancelled' to the same value

On Thu, Feb 11, 2021 at 16:37:43 +0100, Peter Krempa wrote: > It's already initialized to '1'. > > Signed-off-by: Peter Krempa > --- > src/libvirt-domain.c | 2 -- > 1 file changed, 2 deletions(-) > > diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c > index dba89a7d3a..e9688a15b4

Re: [PATCH 03/19] qemu: Probe whether an image is 'qcow2 v2' from query-named-block-nodes

On Thu, Feb 11, 2021 at 16:37:42 +0100, Peter Krempa wrote: > Such images don't support stuff like dirty bitmaps. Note that the > synthetic test for detecting bitmaps is used as an example to prevent > adding additional test cases. > > Signed-off-by: Peter Krempa > --- > src/qemu/qemu_monitor.h

Re: [PATCH 02/19] qemu: capabilities: Introduce QEMU_CAPS_MIGRATION_PARAM_BLOCK_BITMAP_MAPPING

On Fri, Feb 12, 2021 at 09:30:25 +0100, Peter Krempa wrote: > On Fri, Feb 12, 2021 at 08:57:08 +0100, Jiri Denemark wrote: > > On Thu, Feb 11, 2021 at 16:37:41 +0100, Peter Krempa wrote: > > > The capability represents qemu's ability to setup mappings for migrating > > > block dirty bitmaps and is

Re: [PATCH 01/19] qemucapabilitiesdata: Update test data for qemu-6.0 on x86_64

On Thu, Feb 11, 2021 at 16:37:40 +0100, Peter Krempa wrote: > Include the 'transform' member of 'block-bitmap-mapping'. > > Note that this is based on uncommited patches and will be updated once > they are merged. > --- > .../caps_6.0.0.x86_64.replies | 510 ++ >

Re: [libvirt RFC PATCH 0/5] eliminating VIR_FREE in the *Clear() functions

On Fri, Feb 12, 2021 at 10:43:56AM +0100, Martin Kletzander wrote: > On Fri, Feb 12, 2021 at 12:54:02AM -0500, Laine Stump wrote: > > I've looked at a few of these, and one thing I've found is that very > > often we have a function called somethingSomethingClear(), and: > > > > 1) The only places

Re: [libvirt RFC PATCH 0/5] eliminating VIR_FREE in the *Clear() functions

On Fri, Feb 12, 2021 at 12:54:02AM -0500, Laine Stump wrote: I've looked at a few of these, and one thing I've found is that very often we have a function called somethingSomethingClear(), and: 1) The only places it is ever called will immediately free the memory of the object as soon as they

Re: [libvirt RFC PATCH 0/5] eliminating VIR_FREE in the *Clear() functions

On 2/12/21 6:54 AM, Laine Stump wrote: I've looked at a few of these, and one thing I've found is that very often we have a function called somethingSomethingClear(), and: 1) The only places it is ever called will immediately free the memory of the object as soon as they clear it. and very

Re: [libvirt PATCH 0/8] More VIR_FREE removals

On 2/12/21 6:28 AM, Laine Stump wrote: Only 90 this time. These are all functions that behave similar to the *Free() functions, but their names don't end in "Free" so I missed them last time. Laine Stump (8): esx: replace VIR_FREE with g_free in any ESX_VI__TEMPLATE__FREE conf: convert

Re: [PATCH 02/19] qemu: capabilities: Introduce QEMU_CAPS_MIGRATION_PARAM_BLOCK_BITMAP_MAPPING

On Fri, Feb 12, 2021 at 08:57:08 +0100, Jiri Denemark wrote: > On Thu, Feb 11, 2021 at 16:37:41 +0100, Peter Krempa wrote: > > The capability represents qemu's ability to setup mappings for migrating > > block dirty bitmaps and is based on presence of the 'transform' property > > of the