On Wed, Oct 21, 2020 at 22:31:09 -0400, Laine Stump wrote:
> On 10/21/20 5:50 PM, Jonathon Jongsma wrote:
> > Coverity reported a potential resource leak. While it's probably not
> > a real-world scenario, the code could technically jump to cleanup
> > between the time that vdpafd is opened and whe
Glib provides a hash table implementation called GHashTable.
In this part of the series we'll refactor two instances of code which
use non-string keys for hashtable to use GHashTable directly which
simplifies the code (glib provides int hashing functions).
Since GHashTable is not a direct replace
virHashCreate will be removed in upcoming patches. This change has an
impact on ordering of the blockjob entries in one of the status XML->XML
tests.
Signed-off-by: Peter Krempa
---
src/qemu/qemu_domain.c| 2 +-
.../blockjob-blockdev-in.xml | 110 ++
Export the freeing function rather than having a wrapper for the hash
creation function.
Signed-off-by: Peter Krempa
---
src/conf/domain_nwfilter.c | 2 +-
src/conf/nwfilter_ipaddrmap.c | 2 +-
src/conf/nwfilter_params.c | 12 +++-
src/conf/nwfilter_par
Remove 'cleanup' label and simplify remembering of the returned value
from the callback.
Signed-off-by: Peter Krempa
---
src/util/vircgroup.c | 16
1 file changed, 4 insertions(+), 12 deletions(-)
diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c
index b74ec1a8fa..dafc6c
Rewrite using GHashTable which already has interfaces for using a number
as hash key.
Signed-off-by: Peter Krempa
---
src/conf/domain_addr.c | 123 +
src/conf/domain_addr.h | 4 +-
2 files changed, 27 insertions(+), 100 deletions(-)
diff --git a/src/con
virCgroupKillRecursive sneakily initializes 'ret' to 0 rather than the
usual -1. 401030499bf moved an error condition but didn't actually
modify 'ret' return the proper error code.
Fixes: 401030499bf
Signed-off-by: Peter Krempa
---
src/util/vircgroup.c | 1 +
1 file changed, 1 insertion(+)
diff
It doesn't make much sense to configure the bucket count in the hash
table for each case specifically. Replace all calls of virHashCreate
with virHashNew which has a pre-set size and remove virHashCreate
completely.
Signed-off-by: Peter Krempa
---
src/conf/domain_addr.c| 2 +
Rewrite using GHashTable which already has interfaces for using a number
as hash key. Glib's implementation doesn't copy the key by default, so
we need to allocate it, but overal the interface is more suited for this
case.
Signed-off-by: Peter Krempa
---
src/util/vircgroup.c| 61
It's used only in one place in tests which isn't even automatically
evaluated.
Signed-off-by: Peter Krempa
---
src/libvirt_private.syms | 1 -
src/util/virhash.c | 17 -
src/util/virhash.h | 1 -
tests/virhashtest.c | 6 --
4 files changed, 25 deletions(-)
Most callers pass a random number. We have virHashNew which doesn't give
the callers the option to configure the table. Since we are going to
switch to virHashNew replace it in tests and remove multiple instances
of the 'testHashGrow' case as it doesn't make sense with the new
semantics.
Signed-of
The only place we call it is in virHashNew. Move the code to virHashNew
and remove virHashCreateFull.
Code wishing to use non-strings as hash table keys will be better off
using glib's GHashTable directly.
Signed-off-by: Peter Krempa
---
src/util/virhash.c | 53 +
Use 'virHashNew' internally which uses a default size.
Signed-off-by: Peter Krempa
---
src/qemu/qemu_migration.c | 2 +-
src/util/virhash.c| 5 ++---
src/util/virhash.h| 3 +--
3 files changed, 4 insertions(+), 6 deletions(-)
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qem
Since we use virHashTable for string-keyed values only, we can remove
all the callbacks which allowed universal keys.
Code which wishes to use non-string keys should use glib's GHashTable.
Signed-off-by: Peter Krempa
---
src/util/virhash.c | 68 +++---
sr
All users of virHashTable pass strings as the name/key of the entry.
Make this an official requirement by turning the variables to 'const
char *'.
For any other case it's better to use glib's GHashTable.
Signed-off-by: Peter Krempa
---
src/conf/nwfilter_params.c | 2 +-
src/conf/vi
Use 'g_free' directly.
Signed-off-by: Peter Krempa
---
src/conf/domain_addr.c | 2 +-
src/hypervisor/virclosecallbacks.c | 2 +-
src/libvirt_private.syms| 1 -
src/nwfilter/nwfilter_learnipaddr.c | 2 +-
src/qemu/qemu_interop_config.c | 2 +-
src/qemu/qemu_migratio
Nobody uses the return value.
Signed-off-by: Peter Krempa
---
src/util/virhash.c | 8 ++--
src/util/virhash.h | 2 +-
2 files changed, 3 insertions(+), 7 deletions(-)
diff --git a/src/util/virhash.c b/src/util/virhash.c
index 7a20b28379..301e485e69 100644
--- a/src/util/virhash.c
+++ b/src/
On Thu, Oct 22, 2020 at 11:34:52AM +0200, Peter Krempa wrote:
> virCgroupKillRecursive sneakily initializes 'ret' to 0 rather than the
> usual -1. 401030499bf moved an error condition but didn't actually
> modify 'ret' return the proper error code.
>
> Fixes: 401030499bf
> Signed-off-by: Peter Kre
On Thu, Oct 22, 2020 at 11:34:55AM +0200, Peter Krempa wrote:
> Remove 'cleanup' label and simplify remembering of the returned value
> from the callback.
>
> Signed-off-by: Peter Krempa
> ---
> src/util/vircgroup.c | 16
> 1 file changed, 4 insertions(+), 12 deletions(-)
>
> d
On Thu, Oct 22, 2020 at 11:50:01 +0200, Pavel Hrdina wrote:
> On Thu, Oct 22, 2020 at 11:34:55AM +0200, Peter Krempa wrote:
> > Remove 'cleanup' label and simplify remembering of the returned value
> > from the callback.
> >
> > Signed-off-by: Peter Krempa
> > ---
> > src/util/vircgroup.c | 16 +
On Thu, Oct 22, 2020 at 11:55:06AM +0200, Peter Krempa wrote:
> On Thu, Oct 22, 2020 at 11:50:01 +0200, Pavel Hrdina wrote:
> > On Thu, Oct 22, 2020 at 11:34:55AM +0200, Peter Krempa wrote:
> > > Remove 'cleanup' label and simplify remembering of the returned value
> > > from the callback.
> > >
>
On Thu, Oct 22, 2020 at 11:34:53AM +0200, Peter Krempa wrote:
> It's used only in one place in tests which isn't even automatically
> evaluated.
>
> Signed-off-by: Peter Krempa
> ---
> src/libvirt_private.syms | 1 -
> src/util/virhash.c | 17 -
> src/util/virhash.h
The @canonical_path variable holds canonicalized path passed as
argv[1]. The canonicalized path is obtained either via
virFileResolveLink() or plain g_strdup(). Nevertheless, in both
cases it must be freed.
Signed-off-by: Michal Privoznik
---
src/storage/parthelper.c | 2 +-
1 file changed, 1 in
To observe the error fixed in 1/2 apply the following patch and run
parthelper under valgrind, e.g. like this:
valgrind libvirt_parthelper /dev/sda -g
diff --git i/src/util/virdevmapper.c w/src/util/virdevmapper.c
index 4d27c9f104..fad76f35a1 100644
--- i/src/util/virdevmapper.c
+++ w/src/util
Some functions called from parthelper can report an error. But
that means that the error object must be initialized otherwise
virResetError() (which happens as a part of virReportError())
will free random pointers.
Reported-by: Katerina Koukiou
Signed-off-by: Michal Privoznik
---
src/storage/pa
On a Thursday in 2020, Michal Privoznik wrote:
To observe the error fixed in 1/2 apply the following patch and run
parthelper under valgrind, e.g. like this:
valgrind libvirt_parthelper /dev/sda -g
diff --git i/src/util/virdevmapper.c w/src/util/virdevmapper.c
index 4d27c9f104..fad76f35a1 100
On Thu, Oct 22, 2020 at 11:34:56AM +0200, Peter Krempa wrote:
> Rewrite using GHashTable which already has interfaces for using a number
> as hash key.
>
> Signed-off-by: Peter Krempa
> ---
> src/conf/domain_addr.c | 123 +
> src/conf/domain_addr.h | 4 +
On Thu, Oct 22, 2020 at 11:34:57AM +0200, Peter Krempa wrote:
> Use 'virHashNew' internally which uses a default size.
>
> Signed-off-by: Peter Krempa
> ---
> src/qemu/qemu_migration.c | 2 +-
> src/util/virhash.c| 5 ++---
> src/util/virhash.h| 3 +--
> 3 files changed, 4 insert
On Thu, Oct 22, 2020 at 11:34:58AM +0200, Peter Krempa wrote:
> Export the freeing function rather than having a wrapper for the hash
> creation function.
>
> Signed-off-by: Peter Krempa
> ---
> src/conf/domain_nwfilter.c | 2 +-
> src/conf/nwfilter_ipaddrmap.c | 2 +-
> s
On Thu, Oct 22, 2020 at 11:34:59AM +0200, Peter Krempa wrote:
> Most callers pass a random number. We have virHashNew which doesn't give
> the callers the option to configure the table. Since we are going to
> switch to virHashNew replace it in tests and remove multiple instances
> of the 'testHash
On Thu, Oct 22, 2020 at 11:35:00AM +0200, Peter Krempa wrote:
> virHashCreate will be removed in upcoming patches. This change has an
> impact on ordering of the blockjob entries in one of the status XML->XML
> tests.
>
> Signed-off-by: Peter Krempa
> ---
> src/qemu/qemu_domain.c
On Thu, Oct 22, 2020 at 11:35:01AM +0200, Peter Krempa wrote:
> It doesn't make much sense to configure the bucket count in the hash
> table for each case specifically. Replace all calls of virHashCreate
> with virHashNew which has a pre-set size and remove virHashCreate
> completely.
>
> Signed-o
Peter Krempa [2020-10-22, 11:34AM +0200]:
> Rewrite using GHashTable which already has interfaces for using a number
> as hash key.
>
> Signed-off-by: Peter Krempa
> ---
> src/conf/domain_addr.c | 123 +
> src/conf/domain_addr.h | 4 +-
> 2 files change
On Thu, Oct 22, 2020 at 11:35:02AM +0200, Peter Krempa wrote:
> Use 'g_free' directly.
>
> Signed-off-by: Peter Krempa
> ---
> src/conf/domain_addr.c | 2 +-
> src/hypervisor/virclosecallbacks.c | 2 +-
> src/libvirt_private.syms| 1 -
> src/nwfilter/nwfilter_learnipadd
On 21.10.2020 23:43, Andrey Shinkevich wrote:
On 14.10.2020 18:22, Vladimir Sementsov-Ogievskiy wrote:
14.10.2020 15:51, Max Reitz wrote:
On 12.10.20 19:43, Andrey Shinkevich wrote:
If the flag BDRV_REQ_PREFETCH was set, pass it further to the
COR-driver to skip unneeded reading. It can be t
On Thu, Oct 22, 2020 at 11:35:03AM +0200, Peter Krempa wrote:
> The only place we call it is in virHashNew. Move the code to virHashNew
> and remove virHashCreateFull.
>
> Code wishing to use non-strings as hash table keys will be better off
> using glib's GHashTable directly.
>
> Signed-off-by:
On Thu, Oct 22, 2020 at 11:35:04AM +0200, Peter Krempa wrote:
> All users of virHashTable pass strings as the name/key of the entry.
> Make this an official requirement by turning the variables to 'const
> char *'.
>
> For any other case it's better to use glib's GHashTable.
>
> Signed-off-by: Pe
On Thu, Oct 22, 2020 at 11:35:05AM +0200, Peter Krempa wrote:
> Since we use virHashTable for string-keyed values only, we can remove
> all the callbacks which allowed universal keys.
>
> Code which wishes to use non-string keys should use glib's GHashTable.
>
> Signed-off-by: Peter Krempa
> ---
On Thu, Oct 22, 2020 at 13:44:10 +0200, Bjoern Walk wrote:
> Peter Krempa [2020-10-22, 11:34AM +0200]:
> > Rewrite using GHashTable which already has interfaces for using a number
> > as hash key.
> >
> > Signed-off-by: Peter Krempa
> > ---
> > src/conf/domain_addr.c | 123 +
On Thu, Oct 22, 2020 at 11:35:06AM +0200, Peter Krempa wrote:
> Nobody uses the return value.
>
> Signed-off-by: Peter Krempa
> ---
> src/util/virhash.c | 8 ++--
> src/util/virhash.h | 2 +-
> 2 files changed, 3 insertions(+), 7 deletions(-)
Reviewed-by: Pavel Hrdina
signature.asc
Descr
On Tue, Oct 20, 2020 at 16:44:07 +0300, Nikolay Shirokovskiy wrote:
> Currently in qemuProcessHandleBlockJob we either offload blockjob event
> processing to the worker thread or notify another thread that waits for
> blockjob event and that thread processes the event. But sometimes after event
> i
On Tue, Oct 20, 2020 at 16:44:09 +0300, Nikolay Shirokovskiy wrote:
> Some mgmt still use polling for block job completion. After job completion the
> job failure/success is infered by inspecting domain xml. With legacy block job
> processing this does not always work.
So, fix your app? :)
> The
Remove 'cleanup' label and simplify remembering of the returned value
from the callback.
Signed-off-by: Peter Krempa
---
src/util/vircgroup.c | 21 ++---
1 file changed, 10 insertions(+), 11 deletions(-)
diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c
index b74ec1a8fa..
On Thu, Oct 22, 2020 at 02:29:49PM +0200, Peter Krempa wrote:
> Remove 'cleanup' label and simplify remembering of the returned value
> from the callback.
>
> Signed-off-by: Peter Krempa
> ---
> src/util/vircgroup.c | 21 ++---
> 1 file changed, 10 insertions(+), 11 deletions(-)
22.10.2020 10:50, Andrey Shinkevich wrote:
On 21.10.2020 23:43, Andrey Shinkevich wrote:
On 14.10.2020 18:22, Vladimir Sementsov-Ogievskiy wrote:
14.10.2020 15:51, Max Reitz wrote:
On 12.10.20 19:43, Andrey Shinkevich wrote:
If the flag BDRV_REQ_PREFETCH was set, pass it further to the
COR-d
On Thu, Oct 22, 2020 at 11:34:54AM +0200, Peter Krempa wrote:
> Rewrite using GHashTable which already has interfaces for using a number
> as hash key. Glib's implementation doesn't copy the key by default, so
> we need to allocate it, but overal the interface is more suited for this
> case.
>
> S
On Thu, Oct 22, 2020 at 03:11:59PM +0200, Pavel Hrdina wrote:
> On Thu, Oct 22, 2020 at 11:34:54AM +0200, Peter Krempa wrote:
> > Rewrite using GHashTable which already has interfaces for using a number
> > as hash key. Glib's implementation doesn't copy the key by default, so
> > we need to alloca
On Thu, Oct 22, 2020 at 14:17:01 +0100, Daniel Berrange wrote:
> On Thu, Oct 22, 2020 at 03:11:59PM +0200, Pavel Hrdina wrote:
> > On Thu, Oct 22, 2020 at 11:34:54AM +0200, Peter Krempa wrote:
> > > Rewrite using GHashTable which already has interfaces for using a number
> > > as hash key. Glib's i
On Thu, Oct 22, 2020 at 02:17:01PM +0100, Daniel P. Berrangé wrote:
> On Thu, Oct 22, 2020 at 03:11:59PM +0200, Pavel Hrdina wrote:
> > On Thu, Oct 22, 2020 at 11:34:54AM +0200, Peter Krempa wrote:
> > > Rewrite using GHashTable which already has interfaces for using a number
> > > as hash key. Gli
On 10/21/20 6:17 AM, Daniel P. Berrangé wrote:
Claiming QEMU is FIPS compliant without using libgcrypt is a
bit of joke since we don't do any self-tests of ciphers, hence
this deprecation notice is warning people that libgcrypt is
going to be mandatory if you care about FIPS.
FWIW this is my m
Some completers for libvirt related tools might want to list
domain IDs only. Just like the one I've implemented for
virt-viewer [1]. I've worked around it using some awk magic,
but if it was possible to just 'virsh list --id' then I could
drop awk.
1: https://www.redhat.com/archives/virt-tools-li
On 10/14/20 12:37 PM, zhenwei pi wrote:
v2->v3:
Rework patches to make each patch could be compiled,
v1->v2:
Seperate a 'all in one' patch into 4 patches.
Use a 'flags' with bit definition instead of 'action_required'
& 'recursive' for extention.
Queue event directly
On 10/14/20 12:37 PM, zhenwei pi wrote:
Since QEMU 5.2 (commit-77b285f7f6), QEMU supports 'memory failure'
event, posts event to monitor if hitting a hardware memory error.
Fully support this feature for QEMU.
Test with commit 'libvirt: support memory failure event', build a
little complex envir
On 10/14/20 12:37 PM, zhenwei pi wrote:
Introduce memory failure event. Libvirt should monitor domain's
event, then posts it to uplayer. According to the hardware memory
corrupted message, a cloud scheduler could migrate domain to another
health physical server.
Several changes in this patch:
pu
[Please don't CC random people on patches until asked to, we are all
subscribed to the list]
On 10/22/20 4:58 PM, Christian Schoenebeck wrote:
Guests should be allowed to create hard links on mounted pathes, since
many applications rely on this functionality and would error on guest
with curren
Greetings Peter,
> Sent: Tuesday, October 20, 2020 at 2:54 PM
> From: "daggs"
> To: "daggs"
> Cc: "Peter Krempa" , libvir-list@redhat.com
> Subject: Re: [PATCH 0/6] qemu: Fix cdrom as SCSI hostdev via -blockdev
>
> Greetings Peter,
>
> so apparently, the os I'm using in the vm comes with scsi vi
> On Oct 21, 2020, at 12:11 PM, Matt Coleman wrote:
>
>> On Oct 21, 2020, at 11:44 AM, Michal Privoznik wrote:
>>
>> The more I look into the code the more things to fix I find. Well, here
>> are some fixes.
>
> Thanks! This whole patchset looks good.
Reviewed-by: Matt Coleman
--
Matt
On 22.10.2020 15:12, Peter Krempa wrote:
> On Tue, Oct 20, 2020 at 16:44:07 +0300, Nikolay Shirokovskiy wrote:
>> Currently in qemuProcessHandleBlockJob we either offload blockjob event
>> processing to the worker thread or notify another thread that waits for
>> blockjob event and that thread p
On Thu, 22 Oct 2020 09:01:13 +0200
Peter Krempa wrote:
> On Wed, Oct 21, 2020 at 22:31:09 -0400, Laine Stump wrote:
> > On 10/21/20 5:50 PM, Jonathon Jongsma wrote:
> > > Coverity reported a potential resource leak. While it's probably
> > > not a real-world scenario, the code could technically
Guests should be allowed to create hard links on mounted pathes, since
many applications rely on this functionality and would error on guest
with current "rw" AppArmor permission with 9pfs.
Signed-off-by: Christian Schoenebeck
---
src/security/virt-aa-helper.c | 2 +-
1 file changed, 1 insertion
I'm suggesting to add the AppArmor permission "l" on libvirt export pathes
to allow guests creating hard links, which is currently a problem for 9pfs
mounts.
Since the suggested patch would affect virtiofs as well, I would ask David
and Stefan for feedback. If necessary I would change the patch to
Here's a draft GitLab MR if you'd prefer to review the changes there:
https://gitlab.com/iammattcoleman/libvirt/-/merge_requests/3/diffs
Matt Coleman (6):
hyperv: reformat WQL query strings
hyperv: remove duplicate function hypervGetVSSDFromUUID()
hyperv: remove duplicate function hypervGetM
Signed-off-by: Matt Coleman
---
src/hyperv/hyperv_driver.c | 50 +-
src/hyperv/hyperv_wmi.c| 29 +++---
2 files changed, 36 insertions(+), 43 deletions(-)
diff --git a/src/hyperv/hyperv_driver.c b/src/hyperv/hyperv_driver.c
index 002434c56a
Signed-off-by: Matt Coleman
---
src/hyperv/hyperv_driver.c | 42 --
1 file changed, 9 insertions(+), 33 deletions(-)
diff --git a/src/hyperv/hyperv_driver.c b/src/hyperv/hyperv_driver.c
index dbc864b6fa..3935320ea9 100644
--- a/src/hyperv/hyperv_driver.c
+++ b
Signed-off-by: Matt Coleman
---
src/hyperv/hyperv_driver.c | 26 +-
src/hyperv/hyperv_wmi.c| 36 ++--
src/hyperv/hyperv_wmi.h| 4
3 files changed, 27 insertions(+), 39 deletions(-)
diff --git a/src/hyperv/hyperv_driver.c b/src
Signed-off-by: Matt Coleman
---
src/hyperv/hyperv_driver.c | 39 ++
1 file changed, 6 insertions(+), 33 deletions(-)
diff --git a/src/hyperv/hyperv_driver.c b/src/hyperv/hyperv_driver.c
index 3935320ea9..6d5e899535 100644
--- a/src/hyperv/hyperv_driver.c
+++ b
Signed-off-by: Matt Coleman
---
src/hyperv/hyperv_driver.c | 14 +++---
1 file changed, 3 insertions(+), 11 deletions(-)
diff --git a/src/hyperv/hyperv_driver.c b/src/hyperv/hyperv_driver.c
index fba1e355db..a71d0d6261 100644
--- a/src/hyperv/hyperv_driver.c
+++ b/src/hyperv/hyperv_drive
Signed-off-by: Matt Coleman
---
src/hyperv/hyperv_driver.c | 6 ++
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/src/hyperv/hyperv_driver.c b/src/hyperv/hyperv_driver.c
index 6d5e899535..68835cad91 100644
--- a/src/hyperv/hyperv_driver.c
+++ b/src/hyperv/hyperv_driver.c
@@ -10
On 10/23/20 1:00 AM, Michal Privoznik wrote:
On 10/14/20 12:37 PM, zhenwei pi wrote:
v2->v3:
Rework patches to make each patch could be compiled,
v1->v2:
Seperate a 'all in one' patch into 4 patches.
Use a 'flags' with bit definition instead of 'action_required'
& 'recur
On 10/22/20 3:01 AM, Peter Krempa wrote:
On Wed, Oct 21, 2020 at 22:31:09 -0400, Laine Stump wrote:
On 10/21/20 5:50 PM, Jonathon Jongsma wrote:
Coverity reported a potential resource leak. While it's probably not
a real-world scenario, the code could technically jump to cleanup
between the tim
On Thu, Oct 22, 2020 at 23:20:20 -0400, Laine Stump wrote:
> On 10/22/20 3:01 AM, Peter Krempa wrote:
> > On Wed, Oct 21, 2020 at 22:31:09 -0400, Laine Stump wrote:
> > > On 10/21/20 5:50 PM, Jonathon Jongsma wrote:
> > > > Coverity reported a potential resource leak. While it's probably not
> > >
On Thu, Oct 22, 2020 at 14:08:51 -0500, Jonathon Jongsma wrote:
> On Thu, 22 Oct 2020 09:01:13 +0200
> Peter Krempa wrote:
>
> > On Wed, Oct 21, 2020 at 22:31:09 -0400, Laine Stump wrote:
> > > On 10/21/20 5:50 PM, Jonathon Jongsma wrote:
> > > > Coverity reported a potential resource leak. Whi
72 matches
Mail list logo