[libvirt] [PATCH 0/3] build: fix 3 Fedora 17 build errors
The first two of these ("missing initializer") I would expect anybody to get if they're building with -Wall on Fedora 17, but the problems have been there at least since the last libvirt release. The last error only happens if you manually specify CFLAGS with no -O (or with -O0). Although these break the build for me, none of it is new, so I'll wait for ACKs before pushing. (I do think it would be good to get these fixes into the -rc2 candidate, though).
Found when attempting to build on Fedora 17 alpha with: ./autogen.sh --system --enable-compile-warnings=error (this same build command works without problem on Fedora 16). Since the consumer of the qemuProcessReconnectData doesn't assume that the other fields of the struct are initialized (although it uses them internally), the simpler solution is to just switch to C99-style struct initialization (which doesn't require specification of all fields). --- src/qemu/qemu_process.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 0e768fe..ce3bd27 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -3255,7 +3255,7 @@ error: void qemuProcessReconnectAll(virConnectPtr conn, struct qemud_driver *driver) { - struct qemuProcessReconnectData data = {conn, driver}; + struct qemuProcessReconnectData data = {.conn = conn, .driver = driver}; virHashForEach(driver->domains.objs, qemuProcessReconnectHelper, &data); } -- 1.7.7.6
On 03/26/2012 12:53 PM, Laine Stump wrote:
Found when attempting to build on Fedora 17 alpha with:
./autogen.sh --system --enable-compile-warnings=error
(this same build command works without problem on Fedora 16). Since the consumer of the qemuProcessReconnectData doesn't assume that the other fields of the struct are initialized (although it uses them internally), the simpler solution is to just switch to C99-style struct initialization (which doesn't require specification of all fields). --- src/qemu/qemu_process.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 0e768fe..ce3bd27 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -3255,7 +3255,7 @@ error: void qemuProcessReconnectAll(virConnectPtr conn, struct qemud_driver *driver) { - struct qemuProcessReconnectData data = {conn, driver}; + struct qemuProcessReconnectData data = {.conn = conn, .driver = driver};
Both styles guarantee that the other fields are actually used 0-initialized (not uninitialized); but since only the latter shuts up the compiler: ACK. It really bothers me that gcc has some warnings that only trigger at -O2, but not at -O0 -g, but that's not your fault :) -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
Found when attempting to build on Fedora 17 alpha with: ./autogen.sh --system --enable-compile-warnings=error (this same build command works without problem on Fedora 16). All other struct initializers for this struct have the extra field filled in (almost always to 0), so the two errant ones were fixed by adding in the extra 0 field. --- tools/virsh.c | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/tools/virsh.c b/tools/virsh.c index ee6db4c..2c1f166 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -17152,7 +17152,7 @@ static const vshCmdDef domManagementCmds[] = { {"dumpxml", cmdDumpXML, opts_dumpxml, info_dumpxml, 0}, {"edit", cmdEdit, opts_edit, info_edit, 0}, {"inject-nmi", cmdInjectNMI, opts_inject_nmi, info_inject_nmi, 0}, - {"send-key", cmdSendKey, opts_send_key, info_send_key}, + {"send-key", cmdSendKey, opts_send_key, info_send_key, 0}, {"managedsave", cmdManagedSave, opts_managedsave, info_managedsave, 0}, {"managedsave-remove", cmdManagedSaveRemove, opts_managedsaveremove, info_managedsaveremove, 0}, @@ -17406,7 +17406,7 @@ static const vshCmdDef hostAndHypervisorCmds[] = { {"nodeinfo", cmdNodeinfo, NULL, info_nodeinfo, 0}, {"nodememstats", cmdNodeMemStats, opts_node_memstats, info_nodememstats, 0}, {"nodesuspend", cmdNodeSuspend, opts_node_suspend, info_nodesuspend, 0}, - {"qemu-attach", cmdQemuAttach, opts_qemu_attach, info_qemu_attach}, + {"qemu-attach", cmdQemuAttach, opts_qemu_attach, info_qemu_attach, 0}, {"qemu-monitor-command", cmdQemuMonitorCommand, opts_qemu_monitor_command, info_qemu_monitor_command, 0}, {"sysinfo", cmdSysinfo, NULL, info_sysinfo, 0}, -- 1.7.7.6
On 03/26/2012 12:53 PM, Laine Stump wrote:
Found when attempting to build on Fedora 17 alpha with:
./autogen.sh --system --enable-compile-warnings=error
(this same build command works without problem on Fedora 16). All other struct initializers for this struct have the extra field filled in (almost always to 0), so the two errant ones were fixed by adding in the extra 0 field. --- tools/virsh.c | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-)
ACK. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
libvirt always adds -Werror-frame-larger-than=4096 to the flags when it builds. When building on Fedora 17, two functions with multiple 1024 buffers declared inside if {} blocks would generate frame size errors; apparently the version of gcc on Fedora 16 will merge these multiple buffers into a single buffer even when optimization is off, but Fedora 17 won't. The fix is to declare a single 1024 buffer at the top of the two offending functions, and reuse the single buffer throughout the functions. --- src/libxl/libxl_driver.c | 5 +---- src/qemu/qemu_driver.c | 10 ++-------- 2 files changed, 3 insertions(+), 12 deletions(-) diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index eccd198..fb18948 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -833,6 +833,7 @@ libxlStartup(int privileged) { char *log_file = NULL; virCommandPtr cmd; int status, ret = 0; + char ebuf[1024]; /* Disable libxl driver if non-root */ if (!privileged) { @@ -893,25 +894,21 @@ libxlStartup(int privileged) { goto out_of_memory; if (virFileMakePath(libxl_driver->logDir) < 0) { - char ebuf[1024]; VIR_ERROR(_("Failed to create log dir '%s': %s"), libxl_driver->logDir, virStrerror(errno, ebuf, sizeof ebuf)); goto error; } if (virFileMakePath(libxl_driver->stateDir) < 0) { - char ebuf[1024]; VIR_ERROR(_("Failed to create state dir '%s': %s"), libxl_driver->stateDir, virStrerror(errno, ebuf, sizeof ebuf)); goto error; } if (virFileMakePath(libxl_driver->libDir) < 0) { - char ebuf[1024]; VIR_ERROR(_("Failed to create lib dir '%s': %s"), libxl_driver->libDir, virStrerror(errno, ebuf, sizeof ebuf)); goto error; } if (virFileMakePath(libxl_driver->saveDir) < 0) { - char ebuf[1024]; VIR_ERROR(_("Failed to create save dir '%s': %s"), libxl_driver->saveDir, virStrerror(errno, ebuf, sizeof ebuf)); goto error; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index e574126..f4aea61 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -455,6 +455,7 @@ qemudStartup(int privileged) { char *driverConf = NULL; int rc; virConnectPtr conn = NULL; + char ebuf[1024]; if (VIR_ALLOC(qemu_driver) < 0) return -1; @@ -548,37 +549,31 @@ qemudStartup(int privileged) { } if (virFileMakePath(qemu_driver->stateDir) < 0) { - char ebuf[1024]; VIR_ERROR(_("Failed to create state dir '%s': %s"), qemu_driver->stateDir, virStrerror(errno, ebuf, sizeof ebuf)); goto error; } if (virFileMakePath(qemu_driver->libDir) < 0) { - char ebuf[1024]; VIR_ERROR(_("Failed to create lib dir '%s': %s"), qemu_driver->libDir, virStrerror(errno, ebuf, sizeof ebuf)); goto error; } if (virFileMakePath(qemu_driver->cacheDir) < 0) { - char ebuf[1024]; VIR_ERROR(_("Failed to create cache dir '%s': %s"), qemu_driver->cacheDir, virStrerror(errno, ebuf, sizeof ebuf)); goto error; } if (virFileMakePath(qemu_driver->saveDir) < 0) { - char ebuf[1024]; VIR_ERROR(_("Failed to create save dir '%s': %s"), qemu_driver->saveDir, virStrerror(errno, ebuf, sizeof ebuf)); goto error; } if (virFileMakePath(qemu_driver->snapshotDir) < 0) { - char ebuf[1024]; VIR_ERROR(_("Failed to create save dir '%s': %s"), qemu_driver->snapshotDir, virStrerror(errno, ebuf, sizeof ebuf)); goto error; } if (virFileMakePath(qemu_driver->autoDumpPath) < 0) { - char ebuf[1024]; VIR_ERROR(_("Failed to create dump dir '%s': %s"), qemu_driver->autoDumpPath, virStrerror(errno, ebuf, sizeof ebuf)); goto error; @@ -596,9 +591,8 @@ qemudStartup(int privileged) { rc = virCgroupForDriver("qemu", &qemu_driver->cgroup, privileged, 1); if (rc < 0) { - char buf[1024]; VIR_INFO("Unable to create cgroup for driver: %s", - virStrerror(-rc, buf, sizeof(buf))); + virStrerror(-rc, ebuf, sizeof(ebuf))); } if (qemudLoadDriverConfig(qemu_driver, driverConf) < 0) { -- 1.7.7.6
On 03/26/2012 12:53 PM, Laine Stump wrote:
libvirt always adds -Werror-frame-larger-than=4096 to the flags when it builds. When building on Fedora 17, two functions with multiple 1024 buffers declared inside if {} blocks would generate frame size errors; apparently the version of gcc on Fedora 16 will merge these multiple buffers into a single buffer even when optimization is off, but Fedora 17 won't.
The fix is to declare a single 1024 buffer at the top of the two offending functions, and reuse the single buffer throughout the functions. --- src/libxl/libxl_driver.c | 5 +---- src/qemu/qemu_driver.c | 10 ++-------- 2 files changed, 3 insertions(+), 12 deletions(-)
ACK. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
participants (2)
-
Eric Blake -
Laine Stump