[libvirt] [PATCH 0/2] Fix build with musl libc
From: Carlos Santos <casantos@redhat.com> - Do not use 'stderr' as a field name, since it's a preprocessor macro in stdio.h. - Include paths.h for _PATH_MOUNTED, if it's not defined in mntent.h. Found while porting libvirt to Buildroot (https://buildroot.org/). Carlos Santos (2): qemu: fix build with musl libc storage: fix build with musl libc src/qemu/qemu_process.c | 8 ++++---- src/qemu/qemu_process.h | 2 +- src/storage/storage_backend_fs.c | 3 +++ src/storage/storage_backend_vstorage.c | 3 +++ 4 files changed, 11 insertions(+), 5 deletions(-) -- 2.18.1
From: Carlos Santos <casantos@redhat.com> On musl libc "stderr" is a preprocessor macro whose expansion leads to compilation errors: In file included from qemu/qemu_process.c:66: qemu/qemu_process.c: In function 'qemuProcessQMPFree': qemu/qemu_process.c:8418:21: error: expected identifier before '(' token VIR_FREE((proc->stderr)); ^~~~~~ Prevent this by renaming the homonymous field in the _qemuProcessQMP struct to "stdErr". Signed-off-by: Carlos Santos <casantos@redhat.com> --- src/qemu/qemu_process.c | 8 ++++---- src/qemu/qemu_process.h | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 9c50c4a1d8..e7a8d74455 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -8448,7 +8448,7 @@ qemuProcessQMPFree(qemuProcessQMPPtr proc) VIR_FREE(proc->monpath); VIR_FREE(proc->monarg); VIR_FREE(proc->pidfile); - VIR_FREE(proc->stderr); + VIR_FREE(proc->stdErr); VIR_FREE(proc); } @@ -8601,7 +8601,7 @@ qemuProcessQMPLaunch(qemuProcessQMPPtr proc) virCommandSetGID(proc->cmd, proc->runGid); virCommandSetUID(proc->cmd, proc->runUid); - virCommandSetErrorBuffer(proc->cmd, &(proc->stderr)); + virCommandSetErrorBuffer(proc->cmd, &(proc->stdErr)); if (virCommandRun(proc->cmd, &status) < 0) goto cleanup; @@ -8611,7 +8611,7 @@ qemuProcessQMPLaunch(qemuProcessQMPPtr proc) virReportError(VIR_ERR_INTERNAL_ERROR, _("Failed to start QEMU binary %s for probing: %s"), proc->binary, - proc->stderr ? proc->stderr : _("unknown error")); + proc->stdErr ? proc->stdErr : _("unknown error")); goto cleanup; } @@ -8690,7 +8690,7 @@ qemuProcessQMPConnectMonitor(qemuProcessQMPPtr proc) * ** Send QMP Queries to QEMU using monitor (proc->mon) ** * qemuProcessQMPFree(proc); * - * Process error output (proc->stderr) remains available in qemuProcessQMP + * Process error output (proc->stdErr) remains available in qemuProcessQMP * struct until qemuProcessQMPFree is called. */ int diff --git a/src/qemu/qemu_process.h b/src/qemu/qemu_process.h index 1d62319092..9af9f967fd 100644 --- a/src/qemu/qemu_process.h +++ b/src/qemu/qemu_process.h @@ -212,7 +212,7 @@ struct _qemuProcessQMP { char *libDir; uid_t runUid; gid_t runGid; - char *stderr; + char *stdErr; char *monarg; char *monpath; char *pidfile; -- 2.18.1
On 10/16/19 1:22 PM, casantos@redhat.com wrote:
From: Carlos Santos <casantos@redhat.com>
On musl libc "stderr" is a preprocessor macro whose expansion leads to compilation errors:
Indeed, from their stdio.h: #define stdin (stdin) #define stdout (stdout) #define stderr (stderr) https://git.musl-libc.org/cgit/musl/tree/include/stdio.h#n66 I've came accross some other musl related bugs lately and I like it less and less. Michal
On Thu, Oct 17, 2019 at 11:06:25AM +0200, Michal Privoznik wrote:
On 10/16/19 1:22 PM, casantos@redhat.com wrote:
From: Carlos Santos <casantos@redhat.com>
On musl libc "stderr" is a preprocessor macro whose expansion leads to compilation errors:
Indeed, from their stdio.h:
#define stdin (stdin) #define stdout (stdout) #define stderr (stderr)
https://git.musl-libc.org/cgit/musl/tree/include/stdio.h#n66
I've came accross some other musl related bugs lately and I like it less and less.
Ultimately we don't explicitly support musl right, so expectation should be that it is broken and/or will be broken in future. If we want to be serious about supporting builds with musl, then we need it integrated into one of our Jenkins/GitLab/Travis CI systems via the libvirt-jenkins-ci git repo 'lcitool'. Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
On Thu, Oct 17, 2019 at 6:13 AM Daniel P. Berrangé <berrange@redhat.com> wrote:
On Thu, Oct 17, 2019 at 11:06:25AM +0200, Michal Privoznik wrote:
On 10/16/19 1:22 PM, casantos@redhat.com wrote:
From: Carlos Santos <casantos@redhat.com>
On musl libc "stderr" is a preprocessor macro whose expansion leads to compilation errors:
Indeed, from their stdio.h:
#define stdin (stdin) #define stdout (stdout) #define stderr (stderr)
https://git.musl-libc.org/cgit/musl/tree/include/stdio.h#n66
I've came accross some other musl related bugs lately and I like it less and less.
Ultimately we don't explicitly support musl right, so expectation should be that it is broken and/or will be broken in future.
If we want to be serious about supporting builds with musl, then we need it integrated into one of our Jenkins/GitLab/Travis CI systems via the libvirt-jenkins-ci git repo 'lcitool'.
Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
I'm porting libvirt to Buildroot (https://buildroot.org/) as a pet project and musl is one of the supported C libraries. This was the only compilation error I found with musl. So far all my run-time tests pass without failure but I'm enabling a very limited number of features. Other problems may arise as I enable and test more features. I'm testing with uClibc-ng too. I will keep you informed about the progress. -- Carlos Santos Senior Software Maintenance Engineer Red Hat casantos@redhat.com T: +55-11-3534-6186
On Thu, Oct 17, 2019 at 09:02:08AM -0300, Carlos Santos wrote:
On Thu, Oct 17, 2019 at 6:13 AM Daniel P. Berrangé <berrange@redhat.com> wrote:
On Thu, Oct 17, 2019 at 11:06:25AM +0200, Michal Privoznik wrote:
On 10/16/19 1:22 PM, casantos@redhat.com wrote:
From: Carlos Santos <casantos@redhat.com>
On musl libc "stderr" is a preprocessor macro whose expansion leads to compilation errors:
Indeed, from their stdio.h:
#define stdin (stdin) #define stdout (stdout) #define stderr (stderr)
https://git.musl-libc.org/cgit/musl/tree/include/stdio.h#n66
I've came accross some other musl related bugs lately and I like it less and less.
Ultimately we don't explicitly support musl right, so expectation should be that it is broken and/or will be broken in future.
If we want to be serious about supporting builds with musl, then we need it integrated into one of our Jenkins/GitLab/Travis CI systems via the libvirt-jenkins-ci git repo 'lcitool'.
Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
I'm porting libvirt to Buildroot (https://buildroot.org/) as a pet project and musl is one of the supported C libraries. This was the only compilation error I found with musl.
So far all my run-time tests pass without failure but I'm enabling a very limited number of features. Other problems may arise as I enable and test more features.
I'm testing with uClibc-ng too. I will keep you informed about the progress.
FYI, we currently use GNULIB which papers over portability issues in a lot of cases. We're working to eliminate it, however, using GLib for portability, but in doing this there's a strong risk we'll make mistakes and break things that formerly worked on platforms that we don't have CI coverage for. So don't be surprised if you find new problems introduced over the next few months. We're happy to see patches / bug reports of course. Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
From: Carlos Santos <casantos@redhat.com> On musl _PATH_MOUNTED is defined in paths.h, not in mntent.h, which causes compilation errors: storage/storage_backend_fs.c: In function 'virStorageBackendFileSystemIsMounted': storage/storage_backend_fs.c:255:23: error: '_PATH_MOUNTED' undeclared (first use in this function); did you mean 'XPATH_POINT'? if ((mtab = fopen(_PATH_MOUNTED, "r")) == NULL) { ^~~~~~~~~~~~~ XPATH_POINT Fix this including paths.h if _PATH_MOUNTED is still not defined after including mntent.h. This also works with glibc and uClibc-ng. Signed-off-by: Carlos Santos <casantos@redhat.com> --- src/storage/storage_backend_fs.c | 3 +++ src/storage/storage_backend_vstorage.c | 3 +++ 2 files changed, 6 insertions(+) diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c index ed677058ed..fafe2745cc 100644 --- a/src/storage/storage_backend_fs.c +++ b/src/storage/storage_backend_fs.c @@ -42,6 +42,9 @@ VIR_LOG_INIT("storage.storage_backend_fs"); #if WITH_STORAGE_FS # include <mntent.h> +#ifndef _PATH_MOUNTED +# include <paths.h> +#endif struct _virNetfsDiscoverState { const char *host; diff --git a/src/storage/storage_backend_vstorage.c b/src/storage/storage_backend_vstorage.c index cec21dccbf..685f78a22f 100644 --- a/src/storage/storage_backend_vstorage.c +++ b/src/storage/storage_backend_vstorage.c @@ -7,6 +7,9 @@ #include "virlog.h" #include "virstring.h" #include <mntent.h> +#ifndef _PATH_MOUNTED +#include <paths.h> +#endif #include <pwd.h> #include <grp.h> #include "storage_util.h" -- 2.18.1
On 10/16/19 1:22 PM, casantos@redhat.com wrote:
From: Carlos Santos <casantos@redhat.com>
On musl _PATH_MOUNTED is defined in paths.h, not in mntent.h, which causes compilation errors:
storage/storage_backend_fs.c: In function 'virStorageBackendFileSystemIsMounted': storage/storage_backend_fs.c:255:23: error: '_PATH_MOUNTED' undeclared (first use in this function); did you mean 'XPATH_POINT'? if ((mtab = fopen(_PATH_MOUNTED, "r")) == NULL) { ^~~~~~~~~~~~~ XPATH_POINT
Fix this including paths.h if _PATH_MOUNTED is still not defined after including mntent.h. This also works with glibc and uClibc-ng.
Signed-off-by: Carlos Santos <casantos@redhat.com> --- src/storage/storage_backend_fs.c | 3 +++ src/storage/storage_backend_vstorage.c | 3 +++ 2 files changed, 6 insertions(+)
diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c index ed677058ed..fafe2745cc 100644 --- a/src/storage/storage_backend_fs.c +++ b/src/storage/storage_backend_fs.c @@ -42,6 +42,9 @@ VIR_LOG_INIT("storage.storage_backend_fs"); #if WITH_STORAGE_FS
# include <mntent.h> +#ifndef _PATH_MOUNTED +# include <paths.h> +#endif
Well, in fact glibc has the _PATH_MOUNTED defined in paths.h too but only thanks to mntent.h including paths.h (to define deprecated MOUNTED macro) we are not hitting error there. Therefore, we can and should always include paths.h. IOW, those ifndef-s can be dropped.
struct _virNetfsDiscoverState { const char *host; diff --git a/src/storage/storage_backend_vstorage.c b/src/storage/storage_backend_vstorage.c index cec21dccbf..685f78a22f 100644 --- a/src/storage/storage_backend_vstorage.c +++ b/src/storage/storage_backend_vstorage.c @@ -7,6 +7,9 @@ #include "virlog.h" #include "virstring.h" #include <mntent.h> +#ifndef _PATH_MOUNTED +#include <paths.h> +#endif #include <pwd.h> #include <grp.h> #include "storage_util.h"
Michal
On 10/16/19 1:22 PM, casantos@redhat.com wrote:
From: Carlos Santos <casantos@redhat.com>
- Do not use 'stderr' as a field name, since it's a preprocessor macro in stdio.h. - Include paths.h for _PATH_MOUNTED, if it's not defined in mntent.h.
Found while porting libvirt to Buildroot (https://buildroot.org/).
Carlos Santos (2): qemu: fix build with musl libc storage: fix build with musl libc
src/qemu/qemu_process.c | 8 ++++---- src/qemu/qemu_process.h | 2 +- src/storage/storage_backend_fs.c | 3 +++ src/storage/storage_backend_vstorage.c | 3 +++ 4 files changed, 11 insertions(+), 5 deletions(-)
I've fixed 2/2 as suggested, and pushed. Reviewed-by: Michal Privoznik <mprivozn@redhat.com> Congratulations on your first libvirt contribution! Michal
participants (4)
-
Carlos Santos -
casantos＠redhat.com
-
Daniel P. Berrangé -
Michal Privoznik