6:22 a.m.
From: Carlos Santos <casantos(a)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
6:22 a.m.
New subject: [libvirt] [PATCH 1/2] qemu: fix build with musl libc
From: Carlos Santos <casantos(a)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(a)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
4:06 a.m.
New subject: [libvirt] [PATCH 1/2] qemu: fix build with musl libc
On 10/16/19 1:22 PM, casantos(a)redhat.com wrote:
From: Carlos Santos <casantos(a)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
4:13 a.m.
New subject: [libvirt] [PATCH 1/2] qemu: fix build with musl libc
On Thu, Oct 17, 2019 at 11:06:25AM +0200, Michal Privoznik wrote:
On 10/16/19 1:22 PM, casantos(a)redhat.com wrote:
> From: Carlos Santos <casantos(a)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 :|
7:02 a.m.
New subject: [libvirt] [PATCH 1/2] qemu: fix build with musl libc
On Thu, Oct 17, 2019 at 6:13 AM Daniel P. Berrangé <berrange(a)redhat.com> wrote:
On Thu, Oct 17, 2019 at 11:06:25AM +0200, Michal Privoznik wrote:
> On 10/16/19 1:22 PM, casantos(a)redhat.com wrote:
> > From: Carlos Santos <casantos(a)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(a)redhat.com T: +55-11-3534-6186
7:40 a.m.
New subject: [libvirt] [PATCH 1/2] qemu: fix build with musl libc
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(a)redhat.com> wrote:
>
> On Thu, Oct 17, 2019 at 11:06:25AM +0200, Michal Privoznik wrote:
> > On 10/16/19 1:22 PM, casantos(a)redhat.com wrote:
> > > From: Carlos Santos <casantos(a)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 :|
6:22 a.m.
New subject: [libvirt] [PATCH 2/2] storage: fix build with musl libc
From: Carlos Santos <casantos(a)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(a)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
4:06 a.m.
New subject: [libvirt] [PATCH 2/2] storage: fix build with musl libc
On 10/16/19 1:22 PM, casantos(a)redhat.com wrote:
From: Carlos Santos <casantos(a)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(a)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
4:06 a.m.
On 10/16/19 1:22 PM, casantos(a)redhat.com wrote:
From: Carlos Santos <casantos(a)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(a)redhat.com>
Congratulations on your first libvirt contribution!
Michal
1863
days inactive
1864
days old
8 comments
4 participants
participants (4)
-
Carlos Santos -
casantos＠redhat.com
-
Daniel P. Berrangé -
Michal Privoznik