7:16 a.m.
Patches to fix the recent build failures caused by merging the mapped-ram
series. They get a green light from a CI run in my fork
https://gitlab.com/jfehlig/libvirt/-/pipelines/1727301916
I would push them as build-breakers, but the first one does raise a
question that should be answered beforehand IMO. See the individual
patches for more commentary. Also, feel free to push if the patches
are fine and it's outside of MDT timezone working hours :-).
Jim Fehlig (2):
qemu: Fix CLang build
qemu: Fix build on 32-bit platforms
src/qemu/qemu_migration_params.c | 2 +-
src/qemu/qemu_monitor.c | 4 ++--
src/qemu/qemu_process.c | 2 +-
3 files changed, 4 insertions(+), 4 deletions(-)
--
2.43.0
7:16 a.m.
New subject: [PATCH 1/2] qemu: Fix CLang build
Commit f0169f4d caused a build failure with CLang due to potentially
uninitialized variable
../src/qemu/qemu_migration_params.c:825:17: error: variable 'nchannels'
is used uninitialized whenever '&&' condition is false
[-Werror,-Wsometimes-uninitialized]
825 | if (params && virTypedParamsGetInt(params, nparams,
| ^~~~~~
../src/qemu/qemu_migration_params.c:830:17: note: uninitialized use occurs here
830 | if (nchannels < 1) {
| ^~~~~~~~~
Initialize the number of channels to 1, the default value when the
number of channels is not specified.
Fixes: f0169f4d6ce3915cf70bc3e21fa874369e22c840
Signed-off-by: Jim Fehlig <jfehlig(a)suse.com>
---
Is it valid to specify '--parallel' without '--parallel-channels'? If so,
should the number of channels default to 1?
src/qemu/qemu_migration_params.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/qemu/qemu_migration_params.c b/src/qemu/qemu_migration_params.c
index b696b0d13e..f8b9e2bd7e 100644
--- a/src/qemu/qemu_migration_params.c
+++ b/src/qemu/qemu_migration_params.c
@@ -820,7 +820,7 @@ qemuMigrationParamsForSave(virTypedParameterPtr params,
return NULL;
if (flags & VIR_DOMAIN_SAVE_PARALLEL) {
- int nchannels;
+ int nchannels = 1;
if (params && virTypedParamsGetInt(params, nparams,
VIR_DOMAIN_SAVE_PARAM_PARALLEL_CHANNELS,
--
2.43.0
8:09 a.m.
New subject: [PATCH 1/2] qemu: Fix CLang build
On Thu, Mar 20, 2025 at 04:16:59PM -0600, Jim Fehlig via Devel wrote:
Commit f0169f4d caused a build failure with CLang due to potentially
uninitialized variable
../src/qemu/qemu_migration_params.c:825:17: error: variable 'nchannels'
is used uninitialized whenever '&&' condition is false
[-Werror,-Wsometimes-uninitialized]
825 | if (params && virTypedParamsGetInt(params, nparams,
| ^~~~~~
../src/qemu/qemu_migration_params.c:830:17: note: uninitialized use occurs here
830 | if (nchannels < 1) {
| ^~~~~~~~~
Initialize the number of channels to 1, the default value when the
number of channels is not specified.
Fixes: f0169f4d6ce3915cf70bc3e21fa874369e22c840
Signed-off-by: Jim Fehlig <jfehlig(a)suse.com>
---
Is it valid to specify '--parallel' without '--parallel-channels'? If
so,
should the number of channels default to 1?
I've posted a patch series addressing this issue by removing --parallel
option completely including the VIR_DOMAIN_SAVE_PARALLEL flag.
IMHO there is no need to have --parallel option and VIR_DOMAIN_SAVE_PARALLEL flag,
for more details see the reasoning in the specific patches.
Pavel
src/qemu/qemu_migration_params.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/qemu/qemu_migration_params.c b/src/qemu/qemu_migration_params.c
index b696b0d13e..f8b9e2bd7e 100644
--- a/src/qemu/qemu_migration_params.c
+++ b/src/qemu/qemu_migration_params.c
@@ -820,7 +820,7 @@ qemuMigrationParamsForSave(virTypedParameterPtr params,
return NULL;
if (flags & VIR_DOMAIN_SAVE_PARALLEL) {
- int nchannels;
+ int nchannels = 1;
if (params && virTypedParamsGetInt(params, nparams,
VIR_DOMAIN_SAVE_PARAM_PARALLEL_CHANNELS,
--
2.43.0
7:17 a.m.
New subject: [PATCH 2/2] qemu: Fix build on 32-bit platforms
Commits c2518f7bc7 and 28a0621528 introduced build failures on 32-bit
platforms by using incorrect format specifiers with g_strdup_printf.
In one case, an 'unsigned long' format specifier is used with a
'long long int' variable. Fix by changing the format specifier to
'uintmax_t', and casting the variable likewise.
In a second case, an 'unsigned long' format specifier is used with a
'size_t' variable, which is 'unsigned int' on 32-bit systems. Fix by
changing the format specifier to use the 'z' modifier.
Fixes: c2518f7bc7dd4f8ab8655a12ec3a000e1eb5b232
Fixes: 28a06215280b99708ed8dc2d183f62ba7b34ccf8
Signed-off-by: Jim Fehlig <jfehlig(a)suse.com>
---
A lot of ways to skin this cat, but I think this would be the
preferred approach. And while adding the 2 'Fixes:' lines to the
commit message, I questioned whether this should be 2 patches.
src/qemu/qemu_monitor.c | 4 ++--
src/qemu/qemu_process.c | 2 +-
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index 7610d43e6c..6249dc8299 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -2259,8 +2259,8 @@ qemuMonitorMigrateToFdSet(virDomainObj *vm,
qemuFDPassAddFD(fdPassMigrate, directFd, "-directio-fd");
qemuFDPassTransferMonitor(fdPassMigrate, mon);
- uri = g_strdup_printf("file:%s,offset=%#lx",
- qemuFDPassGetPath(fdPassMigrate), offset);
+ uri = g_strdup_printf("file:%s,offset=%#jx",
+ qemuFDPassGetPath(fdPassMigrate), (uintmax_t)offset);
ret = qemuMonitorJSONMigrate(mon, flags, uri);
return ret;
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index c244895863..c3eeadfc3b 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -4917,7 +4917,7 @@ qemuProcessIncomingDefNew(virQEMUDriver *driver,
} else {
qemuFDPassAddFD(inc->fdPassMigrate, fd, "-buffered-fd");
}
- inc->uri = g_strdup_printf("file:%s,offset=%#lx",
+ inc->uri = g_strdup_printf("file:%s,offset=%#zx",
qemuFDPassGetPath(inc->fdPassMigrate), offset);
} else {
inc->uri = qemuMigrationDstGetURI(migrateFrom, *fd);
--
2.43.0
6:54 p.m.
New subject: [PATCH 2/2] qemu: Fix build on 32-bit platforms
On Thu, Mar 20, 2025 at 04:17:00PM -0600, Jim Fehlig via Devel wrote:
Commits c2518f7bc7 and 28a0621528 introduced build failures on
32-bit
platforms by using incorrect format specifiers with g_strdup_printf.
In one case, an 'unsigned long' format specifier is used with a
'long long int' variable. Fix by changing the format specifier to
'uintmax_t', and casting the variable likewise.
In a second case, an 'unsigned long' format specifier is used with a
'size_t' variable, which is 'unsigned int' on 32-bit systems. Fix by
changing the format specifier to use the 'z' modifier.
Fixes: c2518f7bc7dd4f8ab8655a12ec3a000e1eb5b232
Fixes: 28a06215280b99708ed8dc2d183f62ba7b34ccf8
Signed-off-by: Jim Fehlig <jfehlig(a)suse.com>
---
A lot of ways to skin this cat, but I think this would be the
preferred approach. And while adding the 2 'Fixes:' lines to the
commit message, I questioned whether this should be 2 patches.
src/qemu/qemu_monitor.c | 4 ++--
src/qemu/qemu_process.c | 2 +-
2 files changed, 3 insertions(+), 3 deletions(-)
Reviewed-by: Pavel Hrdina <phrdina(a)redhat.com>
12
days inactive
13
days old
4 comments
2 participants
participants (2)
-
Jim Fehlig -
Pavel Hrdina