7:20 a.m.
Pavel Hrdina (2):
src/Makefile: move the new xen_xl_disk parser code at the correct
place
src/Makefile: Fix parallel build after xen_xl_disk parser introduction
src/Makefile.am | 45 ++++++++++++++++++++++++++++-----------------
1 file changed, 28 insertions(+), 17 deletions(-)
--
2.0.5
7:20 a.m.
New subject: [libvirt] [PATCH v4 1/2] src/Makefile: move the new xen_xl_disk parser code at the correct place
Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com>
---
src/Makefile.am | 34 +++++++++++++++++-----------------
1 file changed, 17 insertions(+), 17 deletions(-)
diff --git a/src/Makefile.am b/src/Makefile.am
index f970d60..97253e0 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -1000,23 +1000,6 @@ CPU_SOURCES = \
VMX_SOURCES = \
vmx/vmx.c vmx/vmx.h
-AM_LFLAGS = -Pxl_disk_ --header-file=../$*.h
-LEX_OUTPUT_ROOT = lex.xl_disk_
-BUILT_SOURCES += xenconfig/xen_xl_disk.c xenconfig/xen_xl_disk.h
-# Generated header file is not implicitly added to dist
-EXTRA_DIST += xenconfig/xen_xl_disk.h
-CLEANFILES += xenconfig/xen_xl_disk.h xenconfig/xen_xl_disk.c
-
-XENXLDISKPARSER_SOURCES = xenconfig/xen_xl_disk.l
-
-XENCONFIG_SOURCES = \
- xenconfig/xenxs_private.h \
- xenconfig/xen_common.c xenconfig/xen_common.h \
- xenconfig/xen_sxpr.c xenconfig/xen_sxpr.h \
- xenconfig/xen_xm.c xenconfig/xen_xm.h \
- xenconfig/xen_xl.c xenconfig/xen_xl.h \
- xenconfig/xen_xl_disk.h
-
pkgdata_DATA = cpu/cpu_map.xml
EXTRA_DIST += $(pkgdata_DATA)
@@ -1070,6 +1053,23 @@ libvirt_vmx_la_SOURCES = $(VMX_SOURCES)
endif WITH_VMX
if WITH_XENCONFIG
+AM_LFLAGS = -Pxl_disk_ --header-file=../$*.h
+LEX_OUTPUT_ROOT = lex.xl_disk_
+BUILT_SOURCES += xenconfig/xen_xl_disk.c xenconfig/xen_xl_disk.h
+# Generated header file is not implicitly added to dist
+EXTRA_DIST += xenconfig/xen_xl_disk.h
+CLEANFILES += xenconfig/xen_xl_disk.h xenconfig/xen_xl_disk.c
+
+XENXLDISKPARSER_SOURCES = xenconfig/xen_xl_disk.l
+
+XENCONFIG_SOURCES = \
+ xenconfig/xenxs_private.h \
+ xenconfig/xen_common.c xenconfig/xen_common.h \
+ xenconfig/xen_sxpr.c xenconfig/xen_sxpr.h \
+ xenconfig/xen_xm.c xenconfig/xen_xm.h \
+ xenconfig/xen_xl.c xenconfig/xen_xl.h \
+ xenconfig/xen_xl_disk.h
+
# Flex generated XL disk parser needs to be compiled without WARN_FLAGS
# Add the generated object to its own library to control CFLAGS
noinst_LTLIBRARIES += libvirt_xenxldiskparser.la
--
2.0.5
7:28 a.m.
New subject: [libvirt] [PATCH v4 1/2] src/Makefile: move the new xen_xl_disk parser code at the correct place
On Thu, Jan 08, 2015 at 02:20:24PM +0100, Pavel Hrdina wrote:
Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com>
---
src/Makefile.am | 34 +++++++++++++++++-----------------
1 file changed, 17 insertions(+), 17 deletions(-)
ACK, trivial
Regards,
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
5:06 p.m.
New subject: [libvirt] [PATCH v4 1/2] src/Makefile: move the new xen_xl_disk parser code at the correct place
On 01/08/2015 06:20 AM, Pavel Hrdina wrote:
Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com>
---
src/Makefile.am | 34 +++++++++++++++++-----------------
1 file changed, 17 insertions(+), 17 deletions(-)
if WITH_XENCONFIG
+AM_LFLAGS = -Pxl_disk_ --header-file=../$*.h
Uggh. Not your fault (this patch was just code motion), but it
highlights a portability problem: RHEL 5 ships with flex 2.5.4, which
lacks --header-file, and which spells --outfile only as -o. The build
is failing with:
flex -Pxl_disk_
--header-file=/home/dummy/libvirt/src/xenconfig/xen_xl_disk.h
--outfile=/home/dummy/libvirt/src/xenconfig/xen_xl_disk.c
xenconfig/xen_xl_disk.l
flex: unknown flag '-'. For usage, try
flex --help
As far as I know, we are still aiming to allow for out-of-the-box
compilation on RHEL 5 hosts, so I'm looking at fixing this.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
5:43 p.m.
New subject: [libvirt] [PATCH v4 1/2] src/Makefile: move the new xen_xl_disk parser code at the correct place
Eric Blake wrote:
On 01/08/2015 06:20 AM, Pavel Hrdina wrote:
> Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com>
> ---
> src/Makefile.am | 34 +++++++++++++++++-----------------
> 1 file changed, 17 insertions(+), 17 deletions(-)
>
>
> if WITH_XENCONFIG
> +AM_LFLAGS = -Pxl_disk_ --header-file=../$*.h
>
Uggh. Not your fault (this patch was just code motion), but it
highlights a portability problem: RHEL 5 ships with flex 2.5.4, which
lacks --header-file, and which spells --outfile only as -o. The build
is failing with:
flex -Pxl_disk_
--header-file=/home/dummy/libvirt/src/xenconfig/xen_xl_disk.h
--outfile=/home/dummy/libvirt/src/xenconfig/xen_xl_disk.c
xenconfig/xen_xl_disk.l
flex: unknown flag '-'. For usage, try
flex --help
Sigh... Sorry for all the grief this has caused. I wasted a huge
amount of time trying to get all of this working right, including some
of the approaches in the follow-up patches, but always hit some sort of
problem. I thought I finally had everything worked out with the V3
patches that Michal ACKed. But given all the problems I encountered
along the way, I suppose I shouldn't be surprised by the problems folks
are seeing. So now I'm causing others to waste time :-(. Again, I'm
sorry for that.
And of course this has rippled to the Xen push gate tests too. I
responded to one such test failure today on xen-devel, to which Ian
Campbell replied that the parsing code in Xen was placed in libxlutil to
avoid libxl apps needing to create their own parsing code
http://lists.xen.org/archives/html/xen-devel/2015-01/msg00657.html
My immediate thought was cool, I can use that and remove all the flex
nonsense before the next libvirt release. But alas, the related header
file (libxlutil.h) has never been installed and thus not included in any
distro's xen-devel package
http://lists.xen.org/archives/html/xen-devel/2015-01/msg00690.html
I suppose it is possible to include some minimal header in libvirt (or
an extern declaration?) for the xlu_disk_parse() function, and actually
link against the libxlutil.so which is installed. I can work on such an
approach if folks think it is worthwhile.
Regards,
Jim
3:56 a.m.
New subject: [libvirt] [PATCH v4 1/2] src/Makefile: move the new xen_xl_disk parser code at the correct place
On Thu, 2015-01-08 at 16:43 -0700, Jim Fehlig wrote:
Eric Blake wrote:
> On 01/08/2015 06:20 AM, Pavel Hrdina wrote:
>
>> Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com>
>> ---
>> src/Makefile.am | 34 +++++++++++++++++-----------------
>> 1 file changed, 17 insertions(+), 17 deletions(-)
>>
>
>
>>
>> if WITH_XENCONFIG
>> +AM_LFLAGS = -Pxl_disk_ --header-file=../$*.h
>>
>
> Uggh. Not your fault (this patch was just code motion), but it
> highlights a portability problem: RHEL 5 ships with flex 2.5.4, which
> lacks --header-file, and which spells --outfile only as -o. The build
> is failing with:
>
> flex -Pxl_disk_
> --header-file=/home/dummy/libvirt/src/xenconfig/xen_xl_disk.h
> --outfile=/home/dummy/libvirt/src/xenconfig/xen_xl_disk.c
> xenconfig/xen_xl_disk.l
> flex: unknown flag '-'. For usage, try
> flex --help
>
Sigh... Sorry for all the grief this has caused. I wasted a huge
amount of time trying to get all of this working right, including some
of the approaches in the follow-up patches, but always hit some sort of
problem. I thought I finally had everything worked out with the V3
patches that Michal ACKed. But given all the problems I encountered
along the way, I suppose I shouldn't be surprised by the problems folks
are seeing. So now I'm causing others to waste time :-(. Again, I'm
sorry for that.
And of course this has rippled to the Xen push gate tests too.
NB only our libvirt push gate, which isn't really gating anything, the
result just becomes the baseline for next time and becomes what is used
for testing !libvirt.
So it's not currently impacting our testing of anything other than new
versions of libvirt. Testing of xen-unstable and stable will keep using
the old/working version of libvirt until it is fixed.
I suppose it is possible to include some minimal header in libvirt
(or
an extern declaration?) for the xlu_disk_parse() function, and actually
link against the libxlutil.so which is installed. I can work on such an
approach if folks think it is worthwhile.
FWIW MHO is that sticking the hack in an #ifndef HAVE_LIBXLUTIL_H (from
AC_CHECK_HEADER) isn't so terrible
(http://lists.xen.org/archives/html/xen-devel/2015-01/msg00715.html)
Ian.
10:59 a.m.
New subject: [libvirt] [PATCH v4 1/2] src/Makefile: move the new xen_xl_disk parser code at the correct place
On Thu, Jan 08, 2015 at 04:43:36PM -0700, Jim Fehlig wrote:
Eric Blake wrote:
> On 01/08/2015 06:20 AM, Pavel Hrdina wrote:
>
>> Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com>
>> ---
>> src/Makefile.am | 34 +++++++++++++++++-----------------
>> 1 file changed, 17 insertions(+), 17 deletions(-)
>>
>
>
>>
>> if WITH_XENCONFIG
>> +AM_LFLAGS = -Pxl_disk_ --header-file=../$*.h
>>
>
> Uggh. Not your fault (this patch was just code motion), but it
> highlights a portability problem: RHEL 5 ships with flex 2.5.4, which
> lacks --header-file, and which spells --outfile only as -o. The build
> is failing with:
>
> flex -Pxl_disk_
> --header-file=/home/dummy/libvirt/src/xenconfig/xen_xl_disk.h
> --outfile=/home/dummy/libvirt/src/xenconfig/xen_xl_disk.c
> xenconfig/xen_xl_disk.l
> flex: unknown flag '-'. For usage, try
> flex --help
>
Sigh... Sorry for all the grief this has caused. I wasted a huge
amount of time trying to get all of this working right, including some
of the approaches in the follow-up patches, but always hit some sort of
problem. I thought I finally had everything worked out with the V3
patches that Michal ACKed. But given all the problems I encountered
along the way, I suppose I shouldn't be surprised by the problems folks
are seeing. So now I'm causing others to waste time :-(. Again, I'm
sorry for that.
And of course this has rippled to the Xen push gate tests too. I
responded to one such test failure today on xen-devel, to which Ian
Campbell replied that the parsing code in Xen was placed in libxlutil to
avoid libxl apps needing to create their own parsing code
http://lists.xen.org/archives/html/xen-devel/2015-01/msg00657.html
My immediate thought was cool, I can use that and remove all the flex
nonsense before the next libvirt release. But alas, the related header
file (libxlutil.h) has never been installed and thus not included in any
distro's xen-devel package
http://lists.xen.org/archives/html/xen-devel/2015-01/msg00690.html
I suppose it is possible to include some minimal header in libvirt (or
an extern declaration?) for the xlu_disk_parse() function, and actually
link against the libxlutil.so which is installed. I can work on such an
approach if folks think it is worthwhile.
To repeat what I said on IRC. I don't have a strong opinion one way or
the other. Whatever the libvirt xen maintainers think is the best
approach is fine by me . So it sounds like using libxlutil is probably
a winner right now, with a little #ifdef to cope with the missing
header file on old versions.
Regards,
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
7:20 a.m.
New subject: [libvirt] [PATCH v4 2/2] src/Makefile: Fix parallel build after xen_xl_disk parser introduction
Well, the parallel build doesn't work as there are not dependencies
set correctly. When running 'make -j' I see this error:
make[2]: Entering directory '/home/zippy/work/libvirt/libvirt.git/src'
GEN util/virkeymaps.h
GEN locking/lock_protocol.h
make[2]: *** No rule to make target 'xenconfig/xen_xl_disk.h', needed by
'all'. Stop.
make[2]: *** Waiting for unfinished jobs....
GEN lxc/lxc_controller_dispatch.h
The fix is to correctly set dependencies by letting make know that .c
and .h are to be generated from .l. Moreover, the section is moved
closer to the other section which uses it.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com>
---
src/Makefile.am | 21 ++++++++++++++++-----
1 file changed, 16 insertions(+), 5 deletions(-)
diff --git a/src/Makefile.am b/src/Makefile.am
index 97253e0..ce6182c 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -1053,12 +1053,21 @@ libvirt_vmx_la_SOURCES = $(VMX_SOURCES)
endif WITH_VMX
if WITH_XENCONFIG
-AM_LFLAGS = -Pxl_disk_ --header-file=../$*.h
-LEX_OUTPUT_ROOT = lex.xl_disk_
-BUILT_SOURCES += xenconfig/xen_xl_disk.c xenconfig/xen_xl_disk.h
+# Disable the default rule for lex files because we need to generate the
+# xen_xl_disk files into srcdir instead of builddir.
+.l.c:
+
+$(XENXLDISKPARSER_GENERATED): $(XENXLDISKPARSER_SOURCES)
+ $(AM_V_LEX) $(LEXCOMPILE) $<
+
+AM_LFLAGS = -Pxl_disk_ --header-file=$(abs_srcdir)/xenconfig/xen_xl_disk.h \
+ --outfile=$(abs_srcdir)/xenconfig/xen_xl_disk.c
+XENXLDISKPARSER_GENERATED = xenconfig/xen_xl_disk.c xenconfig/xen_xl_disk.h
+
+BUILT_SOURCES += $(XENXLDISKPARSER_GENERATED)
# Generated header file is not implicitly added to dist
-EXTRA_DIST += xenconfig/xen_xl_disk.h
-CLEANFILES += xenconfig/xen_xl_disk.h xenconfig/xen_xl_disk.c
+EXTRA_DIST += $(XENXLDISKPARSER_GENERATED)
+MAINTAINERCLEANFILES += $(XENXLDISKPARSER_GENERATED)
XENXLDISKPARSER_SOURCES = xenconfig/xen_xl_disk.l
@@ -1078,6 +1087,8 @@ libvirt_xenxldiskparser_la_CFLAGS = \
-I$(top_srcdir)/src/conf $(AM_CFLAGS) -Wno-unused-parameter
libvirt_xenxldiskparser_la_SOURCES = \
$(XENXLDISKPARSER_SOURCES)
+libvirt_xenxldiskparser_la_DEPENDENCIES = \
+ $(XENXLDISKPARSER_GENERATED)
noinst_LTLIBRARIES += libvirt_xenconfig.la
libvirt_la_BUILT_LIBADD += libvirt_xenconfig.la
--
2.0.5
7:31 a.m.
New subject: [libvirt] [PATCH v4 2/2] src/Makefile: Fix parallel build after xen_xl_disk parser introduction
On Thu, Jan 08, 2015 at 02:20:25PM +0100, Pavel Hrdina wrote:
Well, the parallel build doesn't work as there are not
dependencies
set correctly. When running 'make -j' I see this error:
make[2]: Entering directory '/home/zippy/work/libvirt/libvirt.git/src'
GEN util/virkeymaps.h
GEN locking/lock_protocol.h
make[2]: *** No rule to make target 'xenconfig/xen_xl_disk.h', needed by
'all'. Stop.
make[2]: *** Waiting for unfinished jobs....
GEN lxc/lxc_controller_dispatch.h
The fix is to correctly set dependencies by letting make know that .c
and .h are to be generated from .l. Moreover, the section is moved
closer to the other section which uses it.
The second sentance can be removed, since the movement is in the
previous patch now.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com>
---
src/Makefile.am | 21 ++++++++++++++++-----
1 file changed, 16 insertions(+), 5 deletions(-)
ACK assuming you tested both VPATH & non-VPATH builds.
Regards,
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
7:46 a.m.
New subject: [libvirt] [PATCH v4 2/2] src/Makefile: Fix parallel build after xen_xl_disk parser introduction
On 01/08/2015 02:31 PM, Daniel P. Berrange wrote:
On Thu, Jan 08, 2015 at 02:20:25PM +0100, Pavel Hrdina wrote:
> Well, the parallel build doesn't work as there are not dependencies
> set correctly. When running 'make -j' I see this error:
>
> make[2]: Entering directory '/home/zippy/work/libvirt/libvirt.git/src'
> GEN util/virkeymaps.h
> GEN locking/lock_protocol.h
> make[2]: *** No rule to make target 'xenconfig/xen_xl_disk.h', needed by
'all'. Stop.
> make[2]: *** Waiting for unfinished jobs....
> GEN lxc/lxc_controller_dispatch.h
>
> The fix is to correctly set dependencies by letting make know that .c
> and .h are to be generated from .l. Moreover, the section is moved
> closer to the other section which uses it.
The second sentance can be removed, since the movement is in the
previous patch now.
> Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
> Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com>
> ---
> src/Makefile.am | 21 ++++++++++++++++-----
> 1 file changed, 16 insertions(+), 5 deletions(-)
ACK assuming you tested both VPATH & non-VPATH builds.
Regards,
Daniel
Yes I've tested both builds, thanks for review.
Pavel
4:49 p.m.
New subject: [libvirt] [PATCH v4 2/2] src/Makefile: Fix parallel build after xen_xl_disk parser introduction
On 01/08/2015 06:46 AM, Pavel Hrdina wrote:
On 01/08/2015 02:31 PM, Daniel P. Berrange wrote:
> On Thu, Jan 08, 2015 at 02:20:25PM +0100, Pavel Hrdina wrote:
>> Well, the parallel build doesn't work as there are not dependencies
>> set correctly. When running 'make -j' I see this error:
>>
>> make[2]: Entering directory '/home/zippy/work/libvirt/libvirt.git/src'
>> GEN util/virkeymaps.h
>> GEN locking/lock_protocol.h
>> make[2]: *** No rule to make target 'xenconfig/xen_xl_disk.h', needed
>> by 'all'. Stop.
>> make[2]: *** Waiting for unfinished jobs....
>> GEN lxc/lxc_controller_dispatch.h
>>
>> The fix is to correctly set dependencies by letting make know that .c
>> and .h are to be generated from .l. Moreover, the section is moved
>> closer to the other section which uses it.
>
> The second sentance can be removed, since the movement is in the
> previous patch now.
>
>> Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
>> Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com>
>> ---
>> src/Makefile.am | 21 ++++++++++++++++-----
>> 1 file changed, 16 insertions(+), 5 deletions(-)
>
> ACK assuming you tested both VPATH & non-VPATH builds.
I'm now seeing a 'make distcheck' failure. Remember, during 'make
distcheck', automake arranges to change the source directory tree to
read-only, then does a VPATH build of that location. If any make rule
tries to write into the source directory, it is a sign that the tarball
has incorrect timestamps. And sure enough:
make[3]: Entering directory
'/home/eblake/libvirt-tmp/build/libvirt-1.2.12/_build/src'
LEX xenconfig/xen_xl_disk.h
flex: could not create
/home/eblake/libvirt-tmp/build/libvirt-1.2.12/src/xenconfig/xen_xl_disk.c
Makefile:10402: recipe for target 'xenconfig/xen_xl_disk.h' failed
make[3]: *** [xenconfig/xen_xl_disk.h] Error 1
although the failure details aren't mentioned, it is because this is the
phase of the build where srcdir is read-only.
We're making progress, but it looks like we still need more patches in
this area to make sure things are correct (possibly as simple as
distributing the witness timestamp file, so that make from a fresh
tarball sees no reason to re-run flex?).
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
7:43 a.m.
On 01/08/2015 02:20 PM, Pavel Hrdina wrote:
Pavel Hrdina (2):
src/Makefile: move the new xen_xl_disk parser code at the correct
place
src/Makefile: Fix parallel build after xen_xl_disk parser introduction
src/Makefile.am | 45 ++++++++++++++++++++++++++++-----------------
1 file changed, 28 insertions(+), 17 deletions(-)
Also consider this revert of 703ef966 as 0/2 because it was my first
attempt to fix this issue but unfortunately I didn't have clean
repository and the xen_xl_disk.{c,h} has been present. But it's
completely wrong and it has to be reverted.
diff --git a/src/Makefile.am b/src/Makefile.am
index f970d60..557c703 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -1015,7 +1015,7 @@ XENCONFIG_SOURCES =
\
xenconfig/xen_sxpr.c xenconfig/xen_sxpr.h \
xenconfig/xen_xm.c xenconfig/xen_xm.h \
xenconfig/xen_xl.c xenconfig/xen_xl.h \
- xenconfig/xen_xl_disk.h
+ xenconfig/xen_xl_disk_i.h
pkgdata_DATA = cpu/cpu_map.xml
3608
days inactive
3609
days old
11 comments
5 participants
participants (5)
-
Daniel P. Berrange -
Eric Blake -
Ian Campbell -
Jim Fehlig -
Pavel Hrdina