Re: [libvirt] [PATCH V3 2/4] src/xenconfig: Xen-xl parser

Michal Privoznik wrote: > On 16.12.2014 05:30, Jim Fehlig wrote: >> diff --git a/src/xenconfig/xen_xl_disk.l b/src/xenconfig/xen_xl_disk.l >> new file mode 100644 >> index 0000000..164aa32 >> --- /dev/null >> +++ b/src/xenconfig/xen_xl_disk.l >> @@ -0,0 +1,256 @@ >> +/* >> + * xen_xl_disk.l - parser for disk specification strings >> + * >> + * Copyright (C) 2011 Citrix Ltd. >> + * Author Ian Jackson <ian.jackson(a)eu.citrix.com&gt; >> + * >> + * This program is free software; you can redistribute it and/or modify >> + * it under the terms of the GNU Lesser General Public License as published >> + * by the Free Software Foundation; version 2.1 only. with the special >> + * exception on linking described in file LICENSE. >> + * >> + * This program is distributed in the hope that it will be useful, >> + * but WITHOUT ANY WARRANTY; without even the implied warranty of >> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the >> + * GNU Lesser General Public License for more details. >> + */ >> + >> +/* >> + * Parsing the old xm/xend/xl-4.1 disk specs is a tricky problem, >> + * because the target string might in theory contain "," which is the >> + * delimiter we use for stripping off things on the RHS, and ":", >> + * which is the delimiter we use for stripping off things on the LHS. >> + * >> + * In this parser we do not support such target strings in the old >> + * syntax; if the target string has to contain "," or ":" the new >> + * syntax's "target=" should be used. >> + */ >> +%{ >> +# include <config.h> >> + >> +# include <stdio.h> >> + >> +# include "viralloc.h" >> +# include "virstoragefile.h" >> > > With this, you need to -I$(LIBXML_CFLAGS), otherwise you'll get an compile error: > > CC xenconfig/libvirt_xenxldiskparser_la-xen_xl_disk.lo > In file included from ../src/util/virstoragefile.h:29:0, > from xenconfig/xen_xl_disk.l:34: > ../src/util/virstorageencryption.h:30:26: fatal error: libxml/tree.h: No such file or directory > # include <libxml/tree.h> > ^ > > However, that alone is not enough: > make[3]: Entering directory '/home/zippy/work/libvirt/libvirt.git/src' > CC xenconfig/libvirt_xenxldiskparser_la-xen_xl_disk.lo > xenconfig/xen_xl_disk.c: In function 'yy_fatal_error': > xenconfig/xen_xl_disk.c:2143:58: error: unused parameter 'yyscanner' [-Werror=unused-parameter] > static void yy_fatal_error (yyconst char* msg , yyscan_t yyscanner) > ^ > xenconfig/xen_xl_disk.c: In function 'xl_disk_alloc': > xenconfig/xen_xl_disk.c:2471:49: error: unused parameter 'yyscanner' [-Werror=unused-parameter] > void *xl_disk_alloc (yy_size_t size , yyscan_t yyscanner) > ^ > xenconfig/xen_xl_disk.c: In function 'xl_disk_realloc': > xenconfig/xen_xl_disk.c:2476:64: error: unused parameter 'yyscanner' [-Werror=unused-parameter] > void *xl_disk_realloc (void * ptr, yy_size_t size , yyscan_t yyscanner) > ^ > xenconfig/xen_xl_disk.c: In function 'xl_disk_free': > xenconfig/xen_xl_disk.c:2488:42: error: unused parameter 'yyscanner' [-Werror=unused-parameter] > void xl_disk_free (void * ptr , yyscan_t yyscanner) > ^ > cc1: all warnings being treated as errors > Odd that I didn't see these errors in my test builds... > So we are aiming at this diff: > > diff --git a/src/Makefile.am b/src/Makefile.am > index 3eb9a18..8ccc273 100644 > --- a/src/Makefile.am > +++ b/src/Makefile.am > @@ -1074,7 +1074,7 @@ if WITH_XENCONFIG > # Add the generated object to its own library to control CFLAGS > noinst_LTLIBRARIES += libvirt_xenxldiskparser.la > libvirt_xenxldiskparser_la_CFLAGS = \ > - -I$(top_srcdir)/src/conf > + -I$(top_srcdir)/src/conf $(AM_CFLAGS) -Wno-unused-parameter > libvirt_xenxldiskparser_la_SOURCES = \ > $(XENXLDISKPARSER_SOURCES) > > > ACK with that squashed in. > I've added your change and pushed the series. Thanks for the review! David, thank you for working on this, and your patience while we sorted out the flex/autotools integration :-).

Regards, Jim