Re: [PATCH] virt-aa-helper: Allow swtpm to fsync on dir
On Tue, Jul 13, 2021 at 2:42 PM Stefan Berger wrote: > > Allow swtpm (0.7.0 or later) to fsync on the directory where it writes > its state files into so that "the entry in the directory containing the > file has also reached disk" (fsync(2)). > > Signed-off-by: Stefan Berger > --- > src/security/virt-aa-helper.c | 5 - > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c > index 52cfebf6e0..e21557c810 100644 > --- a/src/security/virt-aa-helper.c > +++ b/src/security/virt-aa-helper.c > @@ -1250,8 +1250,11 @@ get_files(vahControl * ctl) > " \"%s/libvirt/qemu/swtpm/%s-swtpm.sock\" rw,\n", > RUNSTATEDIR, shortName); > /* Paths for swtpm to use: give it access to its state > - * directory, log, and PID files. > + * directory (state files and fsync on dir), log, and PID files. > */ > +virBufferAsprintf(&buf, > +" \"%s/lib/libvirt/swtpm/%s/%s/\" r,\n", > +LOCALSTATEDIR, uuidstr, tpmpath); > virBufferAsprintf(&buf, > " \"%s/lib/libvirt/swtpm/%s/%s/**\" rwk,\n", > LOCALSTATEDIR, uuidstr, tpmpath); > -- > 2.31.1 > Patch looks fine to me. Reviewed-by: Neal Gompa -- 真実はいつも一つ！/ Always, there's only one truth!
[PATCH] virt-aa-helper: Allow swtpm to fsync on dir
Allow swtpm (0.7.0 or later) to fsync on the directory where it writes its state files into so that "the entry in the directory containing the file has also reached disk" (fsync(2)). Signed-off-by: Stefan Berger --- src/security/virt-aa-helper.c | 5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c index 52cfebf6e0..e21557c810 100644 --- a/src/security/virt-aa-helper.c +++ b/src/security/virt-aa-helper.c @@ -1250,8 +1250,11 @@ get_files(vahControl * ctl) " \"%s/libvirt/qemu/swtpm/%s-swtpm.sock\" rw,\n", RUNSTATEDIR, shortName); /* Paths for swtpm to use: give it access to its state - * directory, log, and PID files. + * directory (state files and fsync on dir), log, and PID files. */ +virBufferAsprintf(&buf, +" \"%s/lib/libvirt/swtpm/%s/%s/\" r,\n", +LOCALSTATEDIR, uuidstr, tpmpath); virBufferAsprintf(&buf, " \"%s/lib/libvirt/swtpm/%s/%s/**\" rwk,\n", LOCALSTATEDIR, uuidstr, tpmpath); -- 2.31.1
[libvirt PATCH v2 09/10] qemuMonitorGetMemoryDeviceInfo: Use automatic memory management
Signed-off-by: Tim Wiederhake
---
src/qemu/qemu_monitor.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index 9b1a3ec3eb..0b5da8b71f 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -4214,7 +4214,7 @@ int
qemuMonitorGetMemoryDeviceInfo(qemuMonitor *mon,
GHashTable **info)
{
-GHashTable *hash;
+g_autoptr(GHashTable) hash = NULL;
int ret;
VIR_DEBUG("info=%p", info);
@@ -4229,7 +4229,6 @@ qemuMonitorGetMemoryDeviceInfo(qemuMonitor *mon,
if ((ret = qemuMonitorJSONGetMemoryDeviceInfo(mon, hash)) >= 0)
*info = g_steal_pointer(&hash);
-virHashFree(hash);
return ret;
}
--
2.31.1
[libvirt PATCH v2 01/10] virNWFilterCreateVarsFrom: `virHashNew` cannot return NULL
Signed-off-by: Tim Wiederhake
Reviewed-by: Peter Krempa
---
tests/nwfilterxml2firewalltest.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/tests/nwfilterxml2firewalltest.c b/tests/nwfilterxml2firewalltest.c
index 6709cc15fd..3b7190b5cd 100644
--- a/tests/nwfilterxml2firewalltest.c
+++ b/tests/nwfilterxml2firewalltest.c
@@ -149,8 +149,6 @@ virNWFilterCreateVarsFrom(GHashTable *vars1,
GHashTable *vars2)
{
GHashTable *res = virHashNew(virNWFilterVarValueHashFree);
-if (!res)
-return NULL;
if (virNWFilterHashTablePutAll(vars1, res) < 0)
goto err_exit;
--
2.31.1
[libvirt PATCH v2 07/10] iptablesPrivateChainCreate: Remove superfluous `goto`s
Signed-off-by: Tim Wiederhake
Reviewed-by: Peter Krempa
---
src/util/viriptables.c | 9 +++--
1 file changed, 3 insertions(+), 6 deletions(-)
diff --git a/src/util/viriptables.c b/src/util/viriptables.c
index 847af9b9d7..721e1eeae7 100644
--- a/src/util/viriptables.c
+++ b/src/util/viriptables.c
@@ -73,14 +73,13 @@ iptablesPrivateChainCreate(virFirewall *fw,
g_autoptr(GHashTable) chains = virHashNew(NULL);
g_autoptr(GHashTable) links = virHashNew(NULL);
const char *const *tmp;
-int ret = -1;
size_t i;
tmp = lines;
while (tmp && *tmp) {
if (STRPREFIX(*tmp, "-N ")) { /* eg "-N LIBVIRT_INP" */
if (virHashUpdateEntry(chains, *tmp + 3, (void *)0x1) < 0)
-goto cleanup;
+return -1;
} else if (STRPREFIX(*tmp, "-A ")) { /* eg "-A INPUT -j LIBVIRT_INP" */
char *sep = strchr(*tmp + 3, ' ');
if (sep) {
@@ -88,7 +87,7 @@ iptablesPrivateChainCreate(virFirewall *fw,
if (STRPREFIX(sep + 1, "-j ")) {
if (virHashUpdateEntry(links, sep + 4,
(char *)*tmp + 3) < 0)
-goto cleanup;
+return -1;
}
}
}
@@ -112,9 +111,7 @@ iptablesPrivateChainCreate(virFirewall *fw,
"--jump", data->chains[i].child, NULL);
}
-ret = 0;
- cleanup:
-return ret;
+return 0;
}
--
2.31.1
[libvirt PATCH v2 10/10] qemuMonitorGetMemoryDeviceInfo: `virHashNew` cannot return NULL
Signed-off-by: Tim Wiederhake
Reviewed-by: Peter Krempa
---
src/qemu/qemu_monitor.c | 5 +
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index 0b5da8b71f..f66a3457c1 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -4214,7 +4214,7 @@ int
qemuMonitorGetMemoryDeviceInfo(qemuMonitor *mon,
GHashTable **info)
{
-g_autoptr(GHashTable) hash = NULL;
+g_autoptr(GHashTable) hash = virHashNew(g_free);
int ret;
VIR_DEBUG("info=%p", info);
@@ -4223,9 +4223,6 @@ qemuMonitorGetMemoryDeviceInfo(qemuMonitor *mon,
QEMU_CHECK_MONITOR(mon);
-if (!(hash = virHashNew(g_free)))
-return -1;
-
if ((ret = qemuMonitorJSONGetMemoryDeviceInfo(mon, hash)) >= 0)
*info = g_steal_pointer(&hash);
--
2.31.1
[libvirt PATCH v2 08/10] qemuMonitorGetMemoryDeviceInfo: Assign hash table only on success
Signed-off-by: Tim Wiederhake
---
src/qemu/qemu_monitor.c | 10 +-
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index a2df1a6ec3..9b1a3ec3eb 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -4214,6 +4214,7 @@ int
qemuMonitorGetMemoryDeviceInfo(qemuMonitor *mon,
GHashTable **info)
{
+GHashTable *hash;
int ret;
VIR_DEBUG("info=%p", info);
@@ -4222,14 +4223,13 @@ qemuMonitorGetMemoryDeviceInfo(qemuMonitor *mon,
QEMU_CHECK_MONITOR(mon);
-if (!(*info = virHashNew(g_free)))
+if (!(hash = virHashNew(g_free)))
return -1;
-if ((ret = qemuMonitorJSONGetMemoryDeviceInfo(mon, *info)) < 0) {
-virHashFree(*info);
-*info = NULL;
-}
+if ((ret = qemuMonitorJSONGetMemoryDeviceInfo(mon, hash)) >= 0)
+*info = g_steal_pointer(&hash);
+virHashFree(hash);
return ret;
}
--
2.31.1
[libvirt PATCH v2 06/10] iptablesPrivateChainCreate: Use automatic memory management
Signed-off-by: Tim Wiederhake
Reviewed-by: Peter Krempa
---
src/util/viriptables.c | 6 ++
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/src/util/viriptables.c b/src/util/viriptables.c
index 198ece3d71..847af9b9d7 100644
--- a/src/util/viriptables.c
+++ b/src/util/viriptables.c
@@ -70,8 +70,8 @@ iptablesPrivateChainCreate(virFirewall *fw,
void *opaque)
{
iptablesGlobalChainData *data = opaque;
-GHashTable *chains = virHashNew(NULL);
-GHashTable *links = virHashNew(NULL);
+g_autoptr(GHashTable) chains = virHashNew(NULL);
+g_autoptr(GHashTable) links = virHashNew(NULL);
const char *const *tmp;
int ret = -1;
size_t i;
@@ -114,8 +114,6 @@ iptablesPrivateChainCreate(virFirewall *fw,
ret = 0;
cleanup:
-virHashFree(chains);
-virHashFree(links);
return ret;
}
--
2.31.1
[libvirt PATCH v2 03/10] virNWFilterCreateVarsFrom: Remove superfluous `goto`s
Signed-off-by: Tim Wiederhake Reviewed-by: Peter Krempa --- tests/nwfilterxml2firewalltest.c | 7 ++- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/tests/nwfilterxml2firewalltest.c b/tests/nwfilterxml2firewalltest.c index 26d4a936ad..bdfe858185 100644 --- a/tests/nwfilterxml2firewalltest.c +++ b/tests/nwfilterxml2firewalltest.c @@ -151,15 +151,12 @@ virNWFilterCreateVarsFrom(GHashTable *vars1, g_autoptr(GHashTable) res = virHashNew(virNWFilterVarValueHashFree); if (virNWFilterHashTablePutAll(vars1, res) < 0) -goto err_exit; +return NULL; if (virNWFilterHashTablePutAll(vars2, res) < 0) -goto err_exit; +return NULL; return g_steal_pointer(&res); - - err_exit: -return NULL; } -- 2.31.1
[libvirt PATCH v2 02/10] virNWFilterCreateVarsFrom: Use automatic memory management
Signed-off-by: Tim Wiederhake
Reviewed-by: Peter Krempa
---
tests/nwfilterxml2firewalltest.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/tests/nwfilterxml2firewalltest.c b/tests/nwfilterxml2firewalltest.c
index 3b7190b5cd..26d4a936ad 100644
--- a/tests/nwfilterxml2firewalltest.c
+++ b/tests/nwfilterxml2firewalltest.c
@@ -148,7 +148,7 @@ static GHashTable *
virNWFilterCreateVarsFrom(GHashTable *vars1,
GHashTable *vars2)
{
-GHashTable *res = virHashNew(virNWFilterVarValueHashFree);
+g_autoptr(GHashTable) res = virHashNew(virNWFilterVarValueHashFree);
if (virNWFilterHashTablePutAll(vars1, res) < 0)
goto err_exit;
@@ -156,10 +156,9 @@ virNWFilterCreateVarsFrom(GHashTable *vars1,
if (virNWFilterHashTablePutAll(vars2, res) < 0)
goto err_exit;
-return res;
+return g_steal_pointer(&res);
err_exit:
-virHashFree(res);
return NULL;
}
--
2.31.1
[libvirt PATCH v2 05/10] iptablesPrivateChainCreate: `virHashNew` cannot return NULL
Signed-off-by: Tim Wiederhake
Reviewed-by: Peter Krempa
---
src/util/viriptables.c | 9 ++---
1 file changed, 2 insertions(+), 7 deletions(-)
diff --git a/src/util/viriptables.c b/src/util/viriptables.c
index 4189578245..198ece3d71 100644
--- a/src/util/viriptables.c
+++ b/src/util/viriptables.c
@@ -70,17 +70,12 @@ iptablesPrivateChainCreate(virFirewall *fw,
void *opaque)
{
iptablesGlobalChainData *data = opaque;
-GHashTable *chains = NULL;
-GHashTable *links = NULL;
+GHashTable *chains = virHashNew(NULL);
+GHashTable *links = virHashNew(NULL);
const char *const *tmp;
int ret = -1;
size_t i;
-if (!(chains = virHashNew(NULL)))
-goto cleanup;
-if (!(links = virHashNew(NULL)))
-goto cleanup;
-
tmp = lines;
while (tmp && *tmp) {
if (STRPREFIX(*tmp, "-N ")) { /* eg "-N LIBVIRT_INP" */
--
2.31.1
[libvirt PATCH v2 04/10] virNWFilterRuleDefToRuleInst: `virHashNew` cannot return NULL
Signed-off-by: Tim Wiederhake Reviewed-by: Peter Krempa --- tests/nwfilterxml2firewalltest.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/nwfilterxml2firewalltest.c b/tests/nwfilterxml2firewalltest.c index bdfe858185..75a70e9972 100644 --- a/tests/nwfilterxml2firewalltest.c +++ b/tests/nwfilterxml2firewalltest.c @@ -208,8 +208,8 @@ virNWFilterRuleDefToRuleInst(virNWFilterDef *def, ruleinst->chainPriority = def->chainPriority; ruleinst->def = rule; ruleinst->priority = rule->priority; -if (!(ruleinst->vars = virHashNew(virNWFilterVarValueHashFree))) -goto cleanup; +ruleinst->vars = virHashNew(virNWFilterVarValueHashFree); + if (virNWFilterHashTablePutAll(vars, ruleinst->vars) < 0) goto cleanup; -- 2.31.1
[libvirt PATCH v2 00/10] virHashNew refactorings - part II
"virHashNew" cannot return NULL, yet we check for NULL in various places. V1: https://listman.redhat.com/archives/libvir-list/2021-July/msg00188.html Changes since V1: * Moved the inversion of the `if` condition in patch #9 to patch #8 * Only patches #8 and #9 are missing review Tim Wiederhake (10): virNWFilterCreateVarsFrom: `virHashNew` cannot return NULL virNWFilterCreateVarsFrom: Use automatic memory management virNWFilterCreateVarsFrom: Remove superfluous `goto`s virNWFilterRuleDefToRuleInst: `virHashNew` cannot return NULL iptablesPrivateChainCreate: `virHashNew` cannot return NULL iptablesPrivateChainCreate: Use automatic memory management iptablesPrivateChainCreate: Remove superfluous `goto`s qemuMonitorGetMemoryDeviceInfo: Assign hash table only on success qemuMonitorGetMemoryDeviceInfo: Use automatic memory management qemuMonitorGetMemoryDeviceInfo: `virHashNew` cannot return NULL src/qemu/qemu_monitor.c | 10 +++--- src/util/viriptables.c | 20 +--- tests/nwfilterxml2firewalltest.c | 18 ++ 3 files changed, 14 insertions(+), 34 deletions(-) -- 2.31.1
[libvirt PATCH v2 1/1] virThreadPoolNewFull: Prevent expanding worker pool by zero
On libvirtd startup, the list of priority worker threads is uninitialized (`pool->prioWorkers` is NULL), and then "expanded" to zero (`prioWorkers`) entries. This causes `virThreadPoolExpand` to call `VIR_EXPAND_N` on a null pointer and an increment of zero. The zero increment triggers `virReallocN` to not actually allocate any memory and leave the pointer NULL, which, eventually, causes `memset(NULL, 0, 0)` to be called in `virExpandN`. `memset` is declared `__attribute__ ((__nonnull__ 1))`, which triggers the following warning when libvirt is compiled with address sanitizing enabled: $ meson -Dbuildtype=debug -Db_lundef=false -Db_sanitize=address,undefined build && ninja -C build $ ./build/run build/src/libvirtd src/util/viralloc.c:82:5: runtime error: null pointer passed as argument 1, which is declared to never be null Signed-off-by: Tim Wiederhake --- src/util/virthreadpool.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/util/virthreadpool.c b/src/util/virthreadpool.c index 9ddd86a679..92b7cac286 100644 --- a/src/util/virthreadpool.c +++ b/src/util/virthreadpool.c @@ -247,10 +247,10 @@ virThreadPoolNewFull(size_t minWorkers, pool->maxWorkers = maxWorkers; pool->maxPrioWorkers = prioWorkers; -if (virThreadPoolExpand(pool, minWorkers, false) < 0) +if ((minWorkers > 0) && virThreadPoolExpand(pool, minWorkers, false) < 0) goto error; -if (virThreadPoolExpand(pool, prioWorkers, true) < 0) +if ((prioWorkers > 0) && virThreadPoolExpand(pool, prioWorkers, true) < 0) goto error; return pool; -- 2.31.1
[libvirt PATCH v2 0/1] Prevent expanding worker pool by zero
V1: https://listman.redhat.com/archives/libvir-list/2021-July/msg00217.html Changes since V1: * Fix this not in virThreadPoolExpand, but in virThreadPoolNewFull. * Expanded the commit message. Tim Wiederhake (1): virThreadPoolNewFull: Prevent expanding worker pool by zero src/util/virthreadpool.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) -- 2.31.1
[libvirt PATCH 07/10] virNWFilterBindingDefForNet: Remove superfluous `goto`s
Signed-off-by: Tim Wiederhake --- src/conf/domain_nwfilter.c | 5 + 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/conf/domain_nwfilter.c b/src/conf/domain_nwfilter.c index cb35221a59..0a67b6765e 100644 --- a/src/conf/domain_nwfilter.c +++ b/src/conf/domain_nwfilter.c @@ -61,12 +61,9 @@ virNWFilterBindingDefForNet(const char *vmname, if (net->filterparams && virNWFilterHashTablePutAll(net->filterparams, ret->filterparams) < 0) -goto error; +return NULL; return g_steal_pointer(&ret); - - error: -return NULL; } -- 2.31.1
[libvirt PATCH 05/10] virNWFilterBindingDefForNet: `virHashNew` cannot return NULL
Signed-off-by: Tim Wiederhake --- src/conf/domain_nwfilter.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/conf/domain_nwfilter.c b/src/conf/domain_nwfilter.c index 1c62353fa1..5024d5fa03 100644 --- a/src/conf/domain_nwfilter.c +++ b/src/conf/domain_nwfilter.c @@ -59,8 +59,7 @@ virNWFilterBindingDefForNet(const char *vmname, ret->filter = g_strdup(net->filter); -if (!(ret->filterparams = virHashNew(virNWFilterVarValueHashFree))) -goto error; +ret->filterparams = virHashNew(virNWFilterVarValueHashFree); if (net->filterparams && virNWFilterHashTablePutAll(net->filterparams, ret->filterparams) < 0) -- 2.31.1
[libvirt PATCH 10/10] virNWFilterDHCPSnoopInit: `virHashNew` cannot return NULL
Signed-off-by: Tim Wiederhake --- src/nwfilter/nwfilter_dhcpsnoop.c | 17 - 1 file changed, 17 deletions(-) diff --git a/src/nwfilter/nwfilter_dhcpsnoop.c b/src/nwfilter/nwfilter_dhcpsnoop.c index 2481ea5371..4b62a7b661 100644 --- a/src/nwfilter/nwfilter_dhcpsnoop.c +++ b/src/nwfilter/nwfilter_dhcpsnoop.c @@ -1998,27 +1998,10 @@ virNWFilterDHCPSnoopInit(void) virNWFilterSnoopState.snoopReqs = virHashNew(virNWFilterSnoopReqRelease); -if (!virNWFilterSnoopState.ifnameToKey || -!virNWFilterSnoopState.snoopReqs || -!virNWFilterSnoopState.active) -goto error; - virNWFilterSnoopLeaseFileLoad(); virNWFilterSnoopLeaseFileOpen(); return 0; - - error: -virHashFree(virNWFilterSnoopState.ifnameToKey); -virNWFilterSnoopState.ifnameToKey = NULL; - -virHashFree(virNWFilterSnoopState.snoopReqs); -virNWFilterSnoopState.snoopReqs = NULL; - -virHashFree(virNWFilterSnoopState.active); -virNWFilterSnoopState.active = NULL; - -return -1; } /** -- 2.31.1
[libvirt PATCH 08/10] virNWFilterBindingObjListNew: `virHashNew` cannot return NULL
Signed-off-by: Tim Wiederhake
---
src/conf/virnwfilterbindingobjlist.c | 5 +
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/src/conf/virnwfilterbindingobjlist.c
b/src/conf/virnwfilterbindingobjlist.c
index 1f19e27eb0..470a30ca90 100644
--- a/src/conf/virnwfilterbindingobjlist.c
+++ b/src/conf/virnwfilterbindingobjlist.c
@@ -66,10 +66,7 @@ virNWFilterBindingObjListNew(void)
if (!(bindings = virObjectRWLockableNew(virNWFilterBindingObjListClass)))
return NULL;
-if (!(bindings->objs = virHashNew(virObjectFreeHashData))) {
-virObjectUnref(bindings);
-return NULL;
-}
+bindings->objs = virHashNew(virObjectFreeHashData);
return bindings;
}
--
2.31.1
[libvirt PATCH 09/10] virNWFilterBuildAll: `virHashNew` cannot return NULL
Signed-off-by: Tim Wiederhake
---
src/nwfilter/nwfilter_gentech_driver.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/src/nwfilter/nwfilter_gentech_driver.c
b/src/nwfilter/nwfilter_gentech_driver.c
index 8aa1db23d3..da4f71daf1 100644
--- a/src/nwfilter/nwfilter_gentech_driver.c
+++ b/src/nwfilter/nwfilter_gentech_driver.c
@@ -1007,8 +1007,7 @@ virNWFilterBuildAll(virNWFilterDriverState *driver,
VIR_DEBUG("Build all filters newFilters=%d", newFilters);
if (newFilters) {
-if (!(data.skipInterfaces = virHashNew(NULL)))
-return -1;
+data.skipInterfaces = virHashNew(NULL);
data.step = STEP_APPLY_NEW;
if (virNWFilterBindingObjListForEach(driver->bindings,
--
2.31.1
[libvirt PATCH 02/10] virNWFilterBindingDefCopy: `virHashNew` cannot return NULL
Signed-off-by: Tim Wiederhake --- src/conf/virnwfilterbindingdef.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/conf/virnwfilterbindingdef.c b/src/conf/virnwfilterbindingdef.c index 98df1f750a..9704e1bebb 100644 --- a/src/conf/virnwfilterbindingdef.c +++ b/src/conf/virnwfilterbindingdef.c @@ -65,8 +65,7 @@ virNWFilterBindingDefCopy(virNWFilterBindingDef *src) ret->filter = g_strdup(src->filter); -if (!(ret->filterparams = virHashNew(virNWFilterVarValueHashFree))) -goto error; +ret->filterparams = virHashNew(virNWFilterVarValueHashFree); if (virNWFilterHashTablePutAll(src->filterparams, ret->filterparams) < 0) goto error; -- 2.31.1
[libvirt PATCH 04/10] virNWFilterBindingDefCopy: Remove superfluous `goto`s
Signed-off-by: Tim Wiederhake --- src/conf/virnwfilterbindingdef.c | 5 + 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/conf/virnwfilterbindingdef.c b/src/conf/virnwfilterbindingdef.c index 4ed3763efd..22ecf7b828 100644 --- a/src/conf/virnwfilterbindingdef.c +++ b/src/conf/virnwfilterbindingdef.c @@ -66,12 +66,9 @@ virNWFilterBindingDefCopy(virNWFilterBindingDef *src) ret->filterparams = virHashNew(virNWFilterVarValueHashFree); if (virNWFilterHashTablePutAll(src->filterparams, ret->filterparams) < 0) -goto error; +return NULL; return g_steal_pointer(&ret); - - error: -return NULL; } -- 2.31.1
[libvirt PATCH 01/10] conf: Add AUTOPTR_CLEANUP_FUNC for virNWFilterBindingDef
Signed-off-by: Tim Wiederhake
---
src/conf/virnwfilterbindingdef.h | 2 ++
1 file changed, 2 insertions(+)
diff --git a/src/conf/virnwfilterbindingdef.h b/src/conf/virnwfilterbindingdef.h
index 0e52789332..68d531b75d 100644
--- a/src/conf/virnwfilterbindingdef.h
+++ b/src/conf/virnwfilterbindingdef.h
@@ -41,6 +41,8 @@ struct _virNWFilterBindingDef {
void
virNWFilterBindingDefFree(virNWFilterBindingDef *binding);
+G_DEFINE_AUTOPTR_CLEANUP_FUNC(virNWFilterBindingDef,
virNWFilterBindingDefFree);
+
virNWFilterBindingDef *
virNWFilterBindingDefCopy(virNWFilterBindingDef *src);
--
2.31.1
[libvirt PATCH 03/10] virNWFilterBindingDefCopy: Use automatic memory management
Signed-off-by: Tim Wiederhake
---
src/conf/virnwfilterbindingdef.c | 7 ++-
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/src/conf/virnwfilterbindingdef.c b/src/conf/virnwfilterbindingdef.c
index 9704e1bebb..4ed3763efd 100644
--- a/src/conf/virnwfilterbindingdef.c
+++ b/src/conf/virnwfilterbindingdef.c
@@ -49,9 +49,7 @@ virNWFilterBindingDefFree(virNWFilterBindingDef *def)
virNWFilterBindingDef *
virNWFilterBindingDefCopy(virNWFilterBindingDef *src)
{
-virNWFilterBindingDef *ret;
-
-ret = g_new0(virNWFilterBindingDef, 1);
+g_autoptr(virNWFilterBindingDef) ret = g_new0(virNWFilterBindingDef, 1);
ret->ownername = g_strdup(src->ownername);
@@ -70,10 +68,9 @@ virNWFilterBindingDefCopy(virNWFilterBindingDef *src)
if (virNWFilterHashTablePutAll(src->filterparams, ret->filterparams) < 0)
goto error;
-return ret;
+return g_steal_pointer(&ret);
error:
-virNWFilterBindingDefFree(ret);
return NULL;
}
--
2.31.1
[libvirt PATCH 06/10] virNWFilterBindingDefForNet: Use automatic memory management
Signed-off-by: Tim Wiederhake
---
src/conf/domain_nwfilter.c | 7 ++-
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/src/conf/domain_nwfilter.c b/src/conf/domain_nwfilter.c
index 5024d5fa03..cb35221a59 100644
--- a/src/conf/domain_nwfilter.c
+++ b/src/conf/domain_nwfilter.c
@@ -42,9 +42,7 @@ virNWFilterBindingDefForNet(const char *vmname,
const unsigned char *vmuuid,
virDomainNetDef *net)
{
-virNWFilterBindingDef *ret;
-
-ret = g_new0(virNWFilterBindingDef, 1);
+g_autoptr(virNWFilterBindingDef) ret = g_new0(virNWFilterBindingDef, 1);
ret->ownername = g_strdup(vmname);
@@ -65,10 +63,9 @@ virNWFilterBindingDefForNet(const char *vmname,
virNWFilterHashTablePutAll(net->filterparams, ret->filterparams) < 0)
goto error;
-return ret;
+return g_steal_pointer(&ret);
error:
-virNWFilterBindingDefFree(ret);
return NULL;
}
--
2.31.1
[libvirt PATCH 00/10] virHashNew refactorings - part IV
"virHashNew" cannot return NULL, yet we check for NULL in various places. See https://listman.redhat.com/archives/libvir-list/2021-July/msg00074.html. Tim Wiederhake (10): conf: Add AUTOPTR_CLEANUP_FUNC for virNWFilterBindingDef virNWFilterBindingDefCopy: `virHashNew` cannot return NULL virNWFilterBindingDefCopy: Use automatic memory management virNWFilterBindingDefCopy: Remove superfluous `goto`s virNWFilterBindingDefForNet: `virHashNew` cannot return NULL virNWFilterBindingDefForNet: Use automatic memory management virNWFilterBindingDefForNet: Remove superfluous `goto`s virNWFilterBindingObjListNew: `virHashNew` cannot return NULL virNWFilterBuildAll: `virHashNew` cannot return NULL virNWFilterDHCPSnoopInit: `virHashNew` cannot return NULL src/conf/domain_nwfilter.c | 15 --- src/conf/virnwfilterbindingdef.c | 15 --- src/conf/virnwfilterbindingdef.h | 2 ++ src/conf/virnwfilterbindingobjlist.c | 5 + src/nwfilter/nwfilter_dhcpsnoop.c | 17 - src/nwfilter/nwfilter_gentech_driver.c | 3 +-- 6 files changed, 12 insertions(+), 45 deletions(-) -- 2.31.1
[PATCH v1] apparmor: Allow /usr/libexec for private xen-tools binaries
This is a followup for commit e906c4d02bdcddf141b4d124afd68c8ee10134fe
("apparmor: Allow /usr/libexec for libxl-save-helper and pygrub"):
In recent rpm versions --libexecdir changed from /usr/lib64 to
/usr/libexec. A plain rpmbuild %configure in xen.git will install all
files, including the private copies of qemu, into /usr/libexec/xen/bin.
Expand the existing pattern to cover also this libexecdir variant.
Signed-off-by: Olaf Hering
---
src/security/apparmor/usr.sbin.libvirtd.in | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/security/apparmor/usr.sbin.libvirtd.in
b/src/security/apparmor/usr.sbin.libvirtd.in
index 928782b709..f2ab6ff2aa 100644
--- a/src/security/apparmor/usr.sbin.libvirtd.in
+++ b/src/security/apparmor/usr.sbin.libvirtd.in
@@ -88,7 +88,7 @@ profile libvirtd @sbindir@/libvirtd
flags=(attach_disconnected) {
@sbindir@/* PUx,
/{usr/,}lib/udev/scsi_id PUx,
/usr/{lib,lib64}/xen-common/bin/xen-toolstack PUx,
- /usr/{lib,lib64}/xen/bin/* Ux,
+ /usr/{lib,lib64,libexec}/xen/bin/* Ux,
/usr/{lib,libexec}/xen-*/bin/libxl-save-helper PUx,
/usr/{lib,libexec}/xen-*/bin/pygrub PUx,
/usr/{lib,lib64,lib/qemu,libexec}/vhost-user-gpu PUx,
Re: [libvirt PATCH v2 00/11] virHashNew refactorings
On 7/13/21 9:36 AM, Michal Prívozník wrote: > On 7/6/21 2:37 PM, Tim Wiederhake wrote: >> "virHashNew" cannot return NULL, yet we check for NULL in various places. >> >> This series is the first of several that remove these checks. Where >> applicable, the functions are refactored to use automatic memory management >> by means of g_autoptr etc. as well. >> >> v1: https://listman.redhat.com/archives/libvir-list/2021-July/msg00074.html >> >> Changes since v1: >> * Fixed a memory leak that was introduced in patch #3 and fixed in patch #4 >> * Split up patches 3 and 4 into three patches >> >> Regards, >> Tim >> >> Tim Wiederhake (11): >> qemuMonitorGetAllBlockStatsInfo: Clean up line break >> qemuMonitorGetAllBlockStatsInfo: Remove superfluous variable >> initialization >> qemuMonitorGetAllBlockStatsInfo: Assign hash table only on success >> qemuMonitorGetAllBlockStatsInfo: Use automatic memory management >> qemuMonitorGetAllBlockStatsInfo: `virHashNew` cannot return NULL >> qemuMonitorGetBlockInfo: Remove superfluous variable "ret" >> qemuMonitorGetBlockInfo: Use automatic memory management >> qemuMonitorGetBlockInfo: `virHashNew` cannot return NULL >> qemuMonitorGetChardevInfo: Remove superfluous variable "ret" >> qemuMonitorGetChardevInfo: Use automatic memory management >> qemuMonitorGetChardevInfo: `virHashNew` cannot return NULL >> >> src/qemu/qemu_monitor.c | 53 - >> 1 file changed, 15 insertions(+), 38 deletions(-) >> > > Looks good to me. Please let me know if you agree with my suggestion in > 10/11. I can fix that before pushing. > > Reviewed-by: Michal Privoznik Pushed now. Michal
[PATCH] test_driver: Implement virDomainGetControlInfo and add test
As test driver won't have real background job running, in order to get
all possible states, the time is used here to decide which state to be
returned. The default time will get `ok` as return value.
Note that using `virsh domtime fc4 200` won't take effect for the test
driver, to get other states, you have to enter virsh interactive
terminal and set time.
Signed-off-by: Luke Yue
---
src/test/test_driver.c | 63 ++
tests/virshtest.c | 11
2 files changed, 74 insertions(+)
diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index ef0ddab54d..892dc978f2 100644
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -2080,6 +2080,68 @@ testDomainGetState(virDomainPtr domain,
return 0;
}
+static int
+testDomainGetControlInfo(virDomainPtr dom,
+ virDomainControlInfoPtr info,
+ unsigned int flags)
+{
+virDomainObj *vm;
+testDomainObjPrivate *priv;
+int ret = -1;
+
+virCheckFlags(0, -1);
+
+if (!(vm = testDomObjFromDomain(dom)))
+goto cleanup;
+
+if (virDomainObjCheckActive(vm) < 0)
+goto cleanup;
+
+priv = vm->privateData;
+
+memset(info, 0, sizeof(*info));
+
+if (priv->seconds > 0 && priv->seconds < 1) {
+info->state = VIR_DOMAIN_CONTROL_JOB;
+info->stateTime = priv->seconds;
+} else if (priv->seconds < 3 && priv->seconds >= 1) {
+info->state = VIR_DOMAIN_CONTROL_OCCUPIED;
+info->stateTime = priv->seconds - 1;
+} else if (priv->seconds < 6 && priv->seconds >= 3) {
+info->state = VIR_DOMAIN_CONTROL_ERROR;
+switch (priv->seconds % 4) {
+case 0:
+info->details = VIR_DOMAIN_CONTROL_ERROR_REASON_NONE;
+break;
+
+case 1:
+info->details = VIR_DOMAIN_CONTROL_ERROR_REASON_UNKNOWN;
+break;
+
+case 2:
+info->details = VIR_DOMAIN_CONTROL_ERROR_REASON_MONITOR;
+break;
+
+case 3:
+info->details = VIR_DOMAIN_CONTROL_ERROR_REASON_INTERNAL;
+break;
+
+default:
+info->details = VIR_DOMAIN_CONTROL_ERROR_REASON_NONE;
+break;
+}
+info->stateTime = priv->seconds - 3;
+} else {
+info->state = VIR_DOMAIN_CONTROL_OK;
+}
+
+ret = 0;
+
+ cleanup:
+virDomainObjEndAPI(&vm);
+return ret;
+}
+
static int
testDomainGetTime(virDomainPtr dom,
long long *seconds,
@@ -9335,6 +9397,7 @@ static virHypervisorDriver testHypervisorDriver = {
.domainGetHostname = testDomainGetHostname, /* 5.5.0 */
.domainGetInfo = testDomainGetInfo, /* 0.1.1 */
.domainGetState = testDomainGetState, /* 0.9.2 */
+.domainGetControlInfo = testDomainGetControlInfo, /* 7.6.0 */
.domainGetTime = testDomainGetTime, /* 5.4.0 */
.domainSetTime = testDomainSetTime, /* 5.7.0 */
.domainSave = testDomainSave, /* 0.3.2 */
diff --git a/tests/virshtest.c b/tests/virshtest.c
index c1974c46cb..fe0c420958 100644
--- a/tests/virshtest.c
+++ b/tests/virshtest.c
@@ -250,6 +250,13 @@ static int testCompareDomstateByName(const void *data
G_GNUC_UNUSED)
return testCompareOutputLit(exp, NULL, argv);
}
+static int testCompareDomControlInfoByName(const void *data G_GNUC_UNUSED)
+{
+const char *const argv[] = { VIRSH_CUSTOM, "domcontrol", "fc4", NULL };
+const char *exp = "ok\n\n";
+return testCompareOutputLit(exp, NULL, argv);
+}
+
struct testInfo {
const char *const *argv;
const char *result;
@@ -334,6 +341,10 @@ mymain(void)
testCompareDomstateByName, NULL) != 0)
ret = -1;
+if (virTestRun("virsh domcontrol (by name)",
+ testCompareDomControlInfoByName, NULL) != 0)
+ret = -1;
+
/* It's a bit awkward listing result before argument, but that's a
* limitation of C99 vararg macros. */
# define DO_TEST(i, result, ...) \
--
2.32.0
Re: [PATCH] Use postfix increment
On 7/13/21 11:11 AM, Peter Krempa wrote: > On Tue, Jul 13, 2021 at 08:44:06 +0200, Michal Privoznik wrote: >> We document that our coding style is to use postfix increment in >> for() loops rather than prefix. This change was generated by the >> following sed script: >> >> for i in $(git grep -l "for (.*; ++.*)"); do \ >> sed -i 's/\(for (.*; \)++\(.*\))/\1\2++)/' $i; \ >> done >> >> Signed-off-by: Michal Privoznik >> --- > > NACK this is mostly pointless churn. > Fair enough. I just wanted us to be compliant with our coding style guide. But I can live with this patch unmerged. Michal
Re: [PATCH] virnettlshelpers: Update private key
On 7/13/21 11:25 AM, Peter Krempa wrote: > On Tue, Jul 13, 2021 at 08:57:30 +0200, Michal Privoznik wrote: >> In not so distant past (v6.5.0~3) I've updated the private key we >> use for virnettls* tests. Back then I was driven by Fedora 33 >> change which deprecated RSA-1024 which we used back then. I >> generated an EC-384 key which was fine as it was considered >> strong enough until RHEL-9 came along. RHEL-9 no longer considers >> any of EC keys strong enough (for key exchange) and thus we're >> back to RSA, but this time with 2048 bits. Generated by this cmd >> line: > > I'd go for 4096 bits to stay ahead a bit. > >> >> openssl genpkey -algorithm RSA -out key.pem -pkeyopt rsa_keygen_bits:2048 >> >> Signed-off-by: Michal Privoznik >> --- > > My quick google search yielded just some JDK changes for improving the > implementation of EC algorithms: > > https://bugs.openjdk.java.net/browse/JDK-8208698 > > but nothing that would state it's no longer secure or anything. > > Either way. > > Reviewed-by: Peter Krempa > Alright, so after more debugging this turned out to be a bug in crypto-policies package in RHEL-9. It's fixed by the following commit: https://gitlab.com/redhat-crypto/fedora-crypto-policies/-/commit/a5e64bb9a4afcd67965218ba41e28a6839aa9a12 And I can confirm that with that commit the virnettlssessiontest passes again. Thus I think this patch can be discarded. Michal
Tuning tool
Hi all, I have recently started a virt-tuner tool using the python3 binding. The goal of this tool is to help users tune their domains using a few templates. https://github.com/SUSE/virt-tuner As of today I only wrote a template for a giant VM taking almost all the host resources and mapping the host CPU topology. Feel free to submit pull requests to submit other templates that would be of interest to you. Kind regards, -- Cédric
Re: [libvirt PATCH 00/10] virHashNew refactorings - part III
On Mon, Jul 12, 2021 at 11:34:07 +0200, Tim Wiederhake wrote: > "virHashNew" cannot return NULL, yet we check for NULL in various places. > > See https://listman.redhat.com/archives/libvir-list/2021-July/msg00074.html. > Series: Reviewed-by: Peter Krempa
Re: [libvirt PATCH 01/10] testCompareXMLToArgvFiles: `virHashNew` cannot return NULL
The function name in the subject is unfortunately ambiguous. Please prepend it with the test file name or something. On Mon, Jul 12, 2021 at 11:34:08 +0200, Tim Wiederhake wrote: > Signed-off-by: Tim Wiederhake > --- > tests/nwfilterxml2firewalltest.c | 3 --- > 1 file changed, 3 deletions(-)
Re: [libvirt PATCH 00/10] virHashNew refactorings - part II
On Fri, Jul 09, 2021 at 10:27:29 +0200, Tim Wiederhake wrote: > "virHashNew" cannot return NULL, yet we check for NULL in various places. > > See https://listman.redhat.com/archives/libvir-list/2021-July/msg00074.html. > > Tim Wiederhake (10): > virNWFilterCreateVarsFrom: `virHashNew` cannot return NULL > virNWFilterCreateVarsFrom: Use automatic memory management > virNWFilterCreateVarsFrom: Remove superfluous `goto`s > virNWFilterRuleDefToRuleInst: `virHashNew` cannot return NULL > iptablesPrivateChainCreate: `virHashNew` cannot return NULL > iptablesPrivateChainCreate: Use automatic memory management > iptablesPrivateChainCreate: Remove superfluous `goto`s > qemuMonitorGetMemoryDeviceInfo: Assign hash table only on success > qemuMonitorGetMemoryDeviceInfo: Use automatic memory management > qemuMonitorGetMemoryDeviceInfo: `virHashNew` cannot return NULL Series: Reviewed-by: Peter Krempa
Re: [libvirt PATCH 09/10] qemuMonitorGetMemoryDeviceInfo: Use automatic memory management
On Fri, Jul 09, 2021 at 10:27:38 +0200, Tim Wiederhake wrote:
> Signed-off-by: Tim Wiederhake
> ---
> src/qemu/qemu_monitor.c | 7 +++
> 1 file changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
> index 8646efe9c4..3a56ed8ef9 100644
> --- a/src/qemu/qemu_monitor.c
> +++ b/src/qemu/qemu_monitor.c
> @@ -4238,7 +4238,7 @@ int
> qemuMonitorGetMemoryDeviceInfo(qemuMonitor *mon,
> GHashTable **info)
> {
> -GHashTable *hash;
> +g_autoptr(GHashTable) hash = NULL;
> int ret;
>
> VIR_DEBUG("info=%p", info);
> @@ -4250,11 +4250,10 @@ qemuMonitorGetMemoryDeviceInfo(qemuMonitor *mon,
> if (!(hash = virHashNew(g_free)))
> return -1;
>
> -if ((ret = qemuMonitorJSONGetMemoryDeviceInfo(mon, hash)) < 0) {
> -virHashFree(hash);
> +if ((ret = qemuMonitorJSONGetMemoryDeviceInfo(mon, hash)) >= 0) {
> +*info = g_steal_pointer(&hash);
> }
>
> -*info = hash;
> return ret
This would probably look better when squashed into the previous commit.
Re: [libvirt PATCH v2 10/11] qemuMonitorGetChardevInfo: Use automatic memory management
On Tue, 2021-07-13 at 09:36 +0200, Michal Prívozník wrote:
> On 7/6/21 2:37 PM, Tim Wiederhake wrote:
> > Signed-off-by: Tim Wiederhake
> > ---
> > src/qemu/qemu_monitor.c | 16 ++--
> > 1 file changed, 6 insertions(+), 10 deletions(-)
> >
> > diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
> > index cb59fc7b7b..4489b809f4 100644
> > --- a/src/qemu/qemu_monitor.c
> > +++ b/src/qemu/qemu_monitor.c
> > @@ -2884,25 +2884,21 @@ int
> > qemuMonitorGetChardevInfo(qemuMonitor *mon,
> > GHashTable **retinfo)
> > {
> > - GHashTable *info = NULL;
> > + g_autoptr(GHashTable) info = NULL;
> >
> > VIR_DEBUG("retinfo=%p", retinfo);
> >
> > - QEMU_CHECK_MONITOR_GOTO(mon, error);
> > + QEMU_CHECK_MONITOR(mon);
> >
> > + *retinfo = NULL;
>
> This feels redundant. In previous patches you changed the code so
> that
> the output argument is set only in case of success. I think this line
> should be removed.
>
Previous behavior was to set *retinfo to NULL explicitly ...
> > if (!(info = virHashNew(qemuMonitorChardevInfoFree)))
> > - goto error;
> > + return -1;
> >
> > if (qemuMonitorJSONGetChardevInfo(mon, info) < 0)
> > - goto error;
> > + return -1;
> >
> > - *retinfo = info;
> > + *retinfo = g_steal_pointer(&info);
> > return 0;
> > -
> > - error:
> > - virHashFree(info);
> > - *retinfo = NULL;
... here, if an error occured. From what I can tell, this is not really
neccessary, feel free to merge with or without the explicit "*retinfo =
NULL;".
Thanks,
Tim
> > - return -1;
> > }
> >
> >
> >
>
> Michal
>
Re: [PATCH] virnettlshelpers: Update private key
On Tue, Jul 13, 2021 at 08:57:30 +0200, Michal Privoznik wrote: > In not so distant past (v6.5.0~3) I've updated the private key we > use for virnettls* tests. Back then I was driven by Fedora 33 > change which deprecated RSA-1024 which we used back then. I > generated an EC-384 key which was fine as it was considered > strong enough until RHEL-9 came along. RHEL-9 no longer considers > any of EC keys strong enough (for key exchange) and thus we're > back to RSA, but this time with 2048 bits. Generated by this cmd > line: I'd go for 4096 bits to stay ahead a bit. > > openssl genpkey -algorithm RSA -out key.pem -pkeyopt rsa_keygen_bits:2048 > > Signed-off-by: Michal Privoznik > --- My quick google search yielded just some JDK changes for improving the implementation of EC algorithms: https://bugs.openjdk.java.net/browse/JDK-8208698 but nothing that would state it's no longer secure or anything. Either way. Reviewed-by: Peter Krempa
Re: [PATCH] Use postfix increment
On Tue, Jul 13, 2021 at 08:44:06 +0200, Michal Privoznik wrote: > We document that our coding style is to use postfix increment in > for() loops rather than prefix. This change was generated by the > following sed script: > > for i in $(git grep -l "for (.*; ++.*)"); do \ > sed -i 's/\(for (.*; \)++\(.*\))/\1\2++)/' $i; \ > done > > Signed-off-by: Michal Privoznik > --- NACK this is mostly pointless churn.
Re: [PATCH] virsh: Fix the order of format arguments in doDump
On Tue, Jul 13, 2021 at 16:04:12 +0800, Han Han wrote:
> According to definition of virDomainCoreDumpFormat, the "elf" should be
> the first argument in VIR_ENUM_*.
>
> Fixes: 84cc4543be
>
> Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1981625
>
> Signed-off-by: Han Han
> ---
> tools/virsh-domain.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
Reviewed-by: Peter Krempa
and will be pushed shortly.
>
> diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
> index f2a5fb03a4..29748b0257 100644
> --- a/tools/virsh-domain.c
> +++ b/tools/virsh-domain.c
> @@ -5412,10 +5412,10 @@ static const vshCmdOptDef opts_dump[] = {
>
> VIR_ENUM_IMPL(virDomainCoreDumpFormat,
Not a problem with this patch, but the virsh macro definition should
start with 'virsh' or 'vsh' prefix instead of 'vir' which is reserved
for the internal library functions.
>VIR_DOMAIN_CORE_DUMP_FORMAT_LAST,
> + "elf",
>"kdump-zlib",
>"kdump-lzo",
>"kdump-snappy",
> - "elf",
>"win-dmp");
>
> static void
> --
> 2.31.1
>
[PATCH] virsh: Fix the order of format arguments in doDump
According to definition of virDomainCoreDumpFormat, the "elf" should be
the first argument in VIR_ENUM_*.
Fixes: 84cc4543be
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1981625
Signed-off-by: Han Han
---
tools/virsh-domain.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index f2a5fb03a4..29748b0257 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -5412,10 +5412,10 @@ static const vshCmdOptDef opts_dump[] = {
VIR_ENUM_IMPL(virDomainCoreDumpFormat,
VIR_DOMAIN_CORE_DUMP_FORMAT_LAST,
+ "elf",
"kdump-zlib",
"kdump-lzo",
"kdump-snappy",
- "elf",
"win-dmp");
static void
--
2.31.1
Re: [libvirt PATCH v2 10/11] qemuMonitorGetChardevInfo: Use automatic memory management
On 7/6/21 2:37 PM, Tim Wiederhake wrote:
> Signed-off-by: Tim Wiederhake
> ---
> src/qemu/qemu_monitor.c | 16 ++--
> 1 file changed, 6 insertions(+), 10 deletions(-)
>
> diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
> index cb59fc7b7b..4489b809f4 100644
> --- a/src/qemu/qemu_monitor.c
> +++ b/src/qemu/qemu_monitor.c
> @@ -2884,25 +2884,21 @@ int
> qemuMonitorGetChardevInfo(qemuMonitor *mon,
>GHashTable **retinfo)
> {
> -GHashTable *info = NULL;
> +g_autoptr(GHashTable) info = NULL;
>
> VIR_DEBUG("retinfo=%p", retinfo);
>
> -QEMU_CHECK_MONITOR_GOTO(mon, error);
> +QEMU_CHECK_MONITOR(mon);
>
> +*retinfo = NULL;
This feels redundant. In previous patches you changed the code so that
the output argument is set only in case of success. I think this line
should be removed.
> if (!(info = virHashNew(qemuMonitorChardevInfoFree)))
> -goto error;
> +return -1;
>
> if (qemuMonitorJSONGetChardevInfo(mon, info) < 0)
> -goto error;
> +return -1;
>
> -*retinfo = info;
> +*retinfo = g_steal_pointer(&info);
> return 0;
> -
> - error:
> -virHashFree(info);
> -*retinfo = NULL;
> -return -1;
> }
>
>
>
Michal
Re: [libvirt PATCH v2 00/11] virHashNew refactorings
On 7/6/21 2:37 PM, Tim Wiederhake wrote: > "virHashNew" cannot return NULL, yet we check for NULL in various places. > > This series is the first of several that remove these checks. Where > applicable, the functions are refactored to use automatic memory management > by means of g_autoptr etc. as well. > > v1: https://listman.redhat.com/archives/libvir-list/2021-July/msg00074.html > > Changes since v1: > * Fixed a memory leak that was introduced in patch #3 and fixed in patch #4 > * Split up patches 3 and 4 into three patches > > Regards, > Tim > > Tim Wiederhake (11): > qemuMonitorGetAllBlockStatsInfo: Clean up line break > qemuMonitorGetAllBlockStatsInfo: Remove superfluous variable > initialization > qemuMonitorGetAllBlockStatsInfo: Assign hash table only on success > qemuMonitorGetAllBlockStatsInfo: Use automatic memory management > qemuMonitorGetAllBlockStatsInfo: `virHashNew` cannot return NULL > qemuMonitorGetBlockInfo: Remove superfluous variable "ret" > qemuMonitorGetBlockInfo: Use automatic memory management > qemuMonitorGetBlockInfo: `virHashNew` cannot return NULL > qemuMonitorGetChardevInfo: Remove superfluous variable "ret" > qemuMonitorGetChardevInfo: Use automatic memory management > qemuMonitorGetChardevInfo: `virHashNew` cannot return NULL > > src/qemu/qemu_monitor.c | 53 - > 1 file changed, 15 insertions(+), 38 deletions(-) > Looks good to me. Please let me know if you agree with my suggestion in 10/11. I can fix that before pushing. Reviewed-by: Michal Privoznik Michal
