Re: [libvirt] [PATCH 10/12] Introduce support for parsing/formatting Xen xl config format

[Jim Fehlig <jfehlig@suse.com>]

John Ferlan wrote:
> On 01/10/2015 12:03 AM, Jim Fehlig wrote:
>   
>> Introduce a parser/formatter for the xl config format.  Since the
>> deprecation of xm/xend, the VM config file format has diverged as
>> new features are added to libxl.  This patch adds support for parsing
>> and formating the xl config format.  It supports the existing xm config
>> format, plus adds support for spice graphics and xl disk config syntax.
>>
>> Disk config is specified a bit differently in xl as compared to xm.  In
>> xl, disk config consists of comma-separated positional parameters and
>> keyword/value pairs separated by commas. Positional parameters are
>> specified as follows
>>
>>    target, format, vdev, access
>>
>> Supported keys for key=value options are
>>
>>   devtype, backend, backendtype, script, direct-io-safe,
>>
>> The positional paramters can also be specified in key/value form.  For
>> example the following xl disk config are equivalent
>>
>>     /dev/vg/guest-volume,,hda
>>     /dev/vg/guest-volume,raw,hda,rw
>>     format=raw, vdev=hda, access=rw, target=/dev/vg/guest-volume
>>
>> See $xen_sources/docs/misc/xl-disk-configuration.txt for more details.
>>
>> xl disk config is parsed with the help of xlu_disk_parse() from
>> libxlutil, libxl's utility library.  Although the library exists
>> in all Xen versions supported by the libxl virt driver, only
>> recently has the corresponding header file been included.  A check
>> for the header is done in configure.ac.  If not found, xlu_disk_parse()
>> is declared externally.
>>
>> Signed-off-by: Kiarie Kahurani <davidkiarie4@gmail.com>
>> Signed-off-by: Jim Fehlig <jfehlig@suse.com>
>> ---
>>  configure.ac               |   3 +
>>  po/POTFILES.in             |   1 +
>>  src/Makefile.am            |   4 +-
>>  src/libvirt_xenconfig.syms |   4 +
>>  src/xenconfig/xen_common.c |   3 +-
>>  src/xenconfig/xen_xl.c     | 492 +++++++++++++++++++++++++++++++++++++++++++++
>>  src/xenconfig/xen_xl.h     |  33 +++
>>  7 files changed, 538 insertions(+), 2 deletions(-)
>>
>>     
>
> The following is just from the Coverity check...  I don't have all the
> build environments that have proved to be problematic over the last week
> or so...
>
> I assume all you've done is take the generated code and use that rather
> than going through the problems as a result of attempting to generate
> the code.
>   

In place of the generated code I'm using libxlutil from Xen.  The
initial series copied Xen's flex parser for parsing disk config strings,
but thankfully Ian Campbell noticed this and suggested using Xen's
libxlutil instead, which is provided specifically for this purpose.

Thanks for the review.  I'll address the issues you've noted in the next
version.

Regards,
Jim