[v2,2/2] Add dynamic generation of module_block.h

To simplify the addition of new block modules, add a script that generates
include/qemu/module_block.h automatically from the modules' source code.

This script assumes that the QEMU coding style rules are followed.

Signed-off-by: Marc Marí <markmb@redhat.com>
---
 .gitignore                      |   1 +
 Makefile                        |  10 ++-
 include/qemu/module_block.h     |  90 ---------------------------
 scripts/modules/module_block.py | 134 ++++++++++++++++++++++++++++++++++++++++
 4 files changed, 142 insertions(+), 93 deletions(-)
 delete mode 100644 include/qemu/module_block.h
 create mode 100755 scripts/modules/module_block.py

On Tue, 09/08 15:53, Marc Marí wrote:
> diff --git a/include/qemu/module_block.h b/include/qemu/module_block.h
> deleted file mode 100644
> index d725db8..0000000
> --- a/include/qemu/module_block.h
> +++ /dev/null
> @@ -1,90 +0,0 @@
> -/* AUTOMATICALLY GENERATED, DO NOT MODIFY */
> -/*
> - * QEMU Block Module Infrastructure
> - *
> - * Copyright Red Hat, Inc. 2015
> - *
> - * Authors:
> - *  Marc Mari       <markmb@redhat.com>
> - *
> - * This work is licensed under the terms of the GNU GPL, version 2.  See
> - * the COPYING file in the top-level directory.
> - *
> - */
> -

Could you reorder the patches so you don't need to add them remove the
generated header?

> diff --git a/scripts/modules/module_block.py b/scripts/modules/module_block.py
> new file mode 100755
> index 0000000..0846362
> --- /dev/null
> +++ b/scripts/modules/module_block.py
> @@ -0,0 +1,134 @@
> +#!/usr/bin/python
> +#
> +# Module information generator
> +#
> +# Copyright Red Hat, Inc. 2015
> +#
> +# Authors:
> +#  Marc Mari <markmb@redhat.com>
> +#
> +# This work is licensed under the terms of the GNU GPL, version 2.
> +# See the COPYING file in the top-level directory.
> +
> +from __future__ import print_function
> +import sys
> +import os
> +
> +def get_string_struct(line):
> +    data = line.split()
> +
> +    # data[0] -> struct element name
> +    # data[1] -> =
> +    # data[2] -> value
> +
> +    return data[2].replace('"', '')[:-1]
> +
> +def add_module(fhader, library, format_name, protocol_name,
> +                probe, probe_device):
> +    lines = []
> +    lines.append('.library_name = "' + library + '",')
> +    if format_name != "":
> +        lines.append('.format_name = "' + format_name + '",')
> +    if protocol_name != "":
> +        lines.append('.protocol_name = "' + protocol_name + '",')
> +    if probe:
> +        lines.append('.has_probe = true,')
> +    if probe_device:
> +        lines.append('.has_probe_device = true,')
> +
> +    text = '\n\t'.join(lines)
> +    fheader.write('\n\t{\n\t' + text + '\n\t},')
> +
> +def process_file(fheader, filename):
> +    # This parser assumes the coding style rules are being followed
> +    with open(filename, "r") as cfile:
> +        found_something = False
> +        found_start = False
> +        library, _ = os.path.splitext(os.path.basename(filename))
> +        for line in cfile:
> +            if found_start:
> +                line = line.replace('\n', '')
> +                if line.find(".format_name") != -1:
> +                    format_name = get_string_struct(line)
> +                elif line.find(".protocol_name") != -1:
> +                    protocol_name = get_string_struct(line)
> +                elif line.find(".bdrv_probe") != -1:
> +                    probe = True
> +                elif line.find(".bdrv_probe_device") != -1:
> +                    probe_device = True
> +                elif line == "};":
> +                    add_module(fheader, library, format_name, protocol_name,
> +                                probe, probe_device)
> +                    found_start = False
> +            elif line.find("static BlockDriver") != -1:
> +                found_something = True
> +                found_start = True
> +                format_name = ""
> +                protocol_name = ""
> +                probe = False
> +                probe_device = False
> +
> +        if not found_something:
> +            print("No BlockDriver struct found in " + filename + ". \
> +                    Is this really a module?", file=sys.stderr)
> +            sys.exit(1)
> +
> +def print_top(fheader):
> +    fheader.write('''/* AUTOMATICALLY GENERATED, DO NOT MODIFY */
> +/*
> + * QEMU Block Module Infrastructure
> + *
> + * Copyright Red Hat, Inc. 2015
> + *
> + * Authors:
> + *  Marc Mari       <markmb@redhat.com>
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2.  See
> + * the COPYING file in the top-level directory.
> + *
> + */
> +
> +''')
> +
> +    fheader.write('''#ifndef QEMU_MODULE_BLOCK_H
> +#define QEMU_MODULE_BLOCK_H
> +
> +#include "qemu-common.h"
> +
> +static const struct {
> +    const char *format_name;
> +    const char *protocol_name;
> +    const char *library_name;
> +    bool has_probe;
> +    bool has_probe_device;
> +} block_driver_modules[] = {''')
> +
> +def print_bottom(fheader):
> +    fheader.write('''
> +};
> +
> +#endif
> +''')
> +
> +# First argument: output folder
> +# All other arguments: modules source files (.c)
> +output_dir = sys.argv[1]
> +if not os.path.isdir(output_dir):
> +    print("Folder " + output_dir + " does not exist", file=sys.stderr)
> +    sys.exit(1)
> +
> +path = output_dir + 'module_block.h'
> +
> +with open(path, 'w') as fheader:
> +    print_top(fheader)
> +
> +    for filename in sys.argv[2:]:
> +        if os.path.isfile(filename):
> +            process_file(fheader, filename)
> +        else:
> +            print("File " + filename + " does not exist.", file=sys.stderr)
> +            sys.exit(1)
> +
> +    print_bottom(fheader)
> +
> +sys.exit(0)

If we decide to parse c sources, I really want the parser as simple as
possible, while keeping the syntax explicit:

in block_int.h:

    /* In order to allow dynamical loading, block modules should specify which
     * protocols and formats are supported with one or more of following
     * macros, which are parsed by code generator to create mappings from
     * format or protocol name to module name.
     */
    #define BLOCK_PROTOCOL_MODULE(protocol, has_probe)
    #define BLOCK_FORMAT_MODULE(format, has_probe)

in curl.c:
    BLOCK_PROTOCOL_MODULE(http, false)
    BLOCK_PROTOCOL_MODULE(https, false)
    BLOCK_PROTOCOL_MODULE(ftp, false)
    BLOCK_PROTOCOL_MODULE(ftps, false)
    BLOCK_PROTOCOL_MODULE(tftp, false)

in dmg.c:
    BLOCK_FORMAT_MODULE(dmg, true)

In generator.py, you can grep for "BLOCK_PROTOCOL_MODULE(*)" and
"BLOCK_FORMAT_MODULE(*)".

Fam

On Wed, 9 Sep 2015 10:27:13 +0800
Fam Zheng <famz@redhat.com> wrote:

> On Tue, 09/08 15:53, Marc Marí wrote:
> > diff --git a/include/qemu/module_block.h
> > b/include/qemu/module_block.h deleted file mode 100644
> > index d725db8..0000000
> > --- a/include/qemu/module_block.h
> > +++ /dev/null
> > @@ -1,90 +0,0 @@
> > -/* AUTOMATICALLY GENERATED, DO NOT MODIFY */
> > -/*
> > - * QEMU Block Module Infrastructure
> > - *
> > - * Copyright Red Hat, Inc. 2015
> > - *
> > - * Authors:
> > - *  Marc Mari       <markmb@redhat.com>
> > - *
> > - * This work is licensed under the terms of the GNU GPL, version
> > 2.  See
> > - * the COPYING file in the top-level directory.
> > - *
> > - */
> > -
> 
> Could you reorder the patches so you don't need to add them remove the
> generated header?
> 
> > diff --git a/scripts/modules/module_block.py
> > b/scripts/modules/module_block.py new file mode 100755
> > index 0000000..0846362
> > --- /dev/null
> > +++ b/scripts/modules/module_block.py
> > @@ -0,0 +1,134 @@
> > +#!/usr/bin/python
> > +#
> > +# Module information generator
> > +#
> > +# Copyright Red Hat, Inc. 2015
> > +#
> > +# Authors:
> > +#  Marc Mari <markmb@redhat.com>
> > +#
> > +# This work is licensed under the terms of the GNU GPL, version 2.
> > +# See the COPYING file in the top-level directory.
> > +
> > +from __future__ import print_function
> > +import sys
> > +import os
> > +
> > +def get_string_struct(line):
> > +    data = line.split()
> > +
> > +    # data[0] -> struct element name
> > +    # data[1] -> =
> > +    # data[2] -> value
> > +
> > +    return data[2].replace('"', '')[:-1]
> > +
> > +def add_module(fhader, library, format_name, protocol_name,
> > +                probe, probe_device):
> > +    lines = []
> > +    lines.append('.library_name = "' + library + '",')
> > +    if format_name != "":
> > +        lines.append('.format_name = "' + format_name + '",')
> > +    if protocol_name != "":
> > +        lines.append('.protocol_name = "' + protocol_name + '",')
> > +    if probe:
> > +        lines.append('.has_probe = true,')
> > +    if probe_device:
> > +        lines.append('.has_probe_device = true,')
> > +
> > +    text = '\n\t'.join(lines)
> > +    fheader.write('\n\t{\n\t' + text + '\n\t},')
> > +
> > +def process_file(fheader, filename):
> > +    # This parser assumes the coding style rules are being followed
> > +    with open(filename, "r") as cfile:
> > +        found_something = False
> > +        found_start = False
> > +        library, _ = os.path.splitext(os.path.basename(filename))
> > +        for line in cfile:
> > +            if found_start:
> > +                line = line.replace('\n', '')
> > +                if line.find(".format_name") != -1:
> > +                    format_name = get_string_struct(line)
> > +                elif line.find(".protocol_name") != -1:
> > +                    protocol_name = get_string_struct(line)
> > +                elif line.find(".bdrv_probe") != -1:
> > +                    probe = True
> > +                elif line.find(".bdrv_probe_device") != -1:
> > +                    probe_device = True
> > +                elif line == "};":
> > +                    add_module(fheader, library, format_name,
> > protocol_name,
> > +                                probe, probe_device)
> > +                    found_start = False
> > +            elif line.find("static BlockDriver") != -1:
> > +                found_something = True
> > +                found_start = True
> > +                format_name = ""
> > +                protocol_name = ""
> > +                probe = False
> > +                probe_device = False
> > +
> > +        if not found_something:
> > +            print("No BlockDriver struct found in " + filename +
> > ". \
> > +                    Is this really a module?", file=sys.stderr)
> > +            sys.exit(1)
> > +
> > +def print_top(fheader):
> > +    fheader.write('''/* AUTOMATICALLY GENERATED, DO NOT MODIFY */
> > +/*
> > + * QEMU Block Module Infrastructure
> > + *
> > + * Copyright Red Hat, Inc. 2015
> > + *
> > + * Authors:
> > + *  Marc Mari       <markmb@redhat.com>
> > + *
> > + * This work is licensed under the terms of the GNU GPL, version
> > 2.  See
> > + * the COPYING file in the top-level directory.
> > + *
> > + */
> > +
> > +''')
> > +
> > +    fheader.write('''#ifndef QEMU_MODULE_BLOCK_H
> > +#define QEMU_MODULE_BLOCK_H
> > +
> > +#include "qemu-common.h"
> > +
> > +static const struct {
> > +    const char *format_name;
> > +    const char *protocol_name;
> > +    const char *library_name;
> > +    bool has_probe;
> > +    bool has_probe_device;
> > +} block_driver_modules[] = {''')
> > +
> > +def print_bottom(fheader):
> > +    fheader.write('''
> > +};
> > +
> > +#endif
> > +''')
> > +
> > +# First argument: output folder
> > +# All other arguments: modules source files (.c)
> > +output_dir = sys.argv[1]
> > +if not os.path.isdir(output_dir):
> > +    print("Folder " + output_dir + " does not exist",
> > file=sys.stderr)
> > +    sys.exit(1)
> > +
> > +path = output_dir + 'module_block.h'
> > +
> > +with open(path, 'w') as fheader:
> > +    print_top(fheader)
> > +
> > +    for filename in sys.argv[2:]:
> > +        if os.path.isfile(filename):
> > +            process_file(fheader, filename)
> > +        else:
> > +            print("File " + filename + " does not exist.",
> > file=sys.stderr)
> > +            sys.exit(1)
> > +
> > +    print_bottom(fheader)
> > +
> > +sys.exit(0)
> 
> If we decide to parse c sources, I really want the parser as simple as
> possible, while keeping the syntax explicit:
> 
> in block_int.h:
> 
>     /* In order to allow dynamical loading, block modules should
> specify which
>      * protocols and formats are supported with one or more of
> following
>      * macros, which are parsed by code generator to create mappings
> from
>      * format or protocol name to module name.
>      */
>     #define BLOCK_PROTOCOL_MODULE(protocol, has_probe)
>     #define BLOCK_FORMAT_MODULE(format, has_probe)
> 
> in curl.c:
>     BLOCK_PROTOCOL_MODULE(http, false)
>     BLOCK_PROTOCOL_MODULE(https, false)
>     BLOCK_PROTOCOL_MODULE(ftp, false)
>     BLOCK_PROTOCOL_MODULE(ftps, false)
>     BLOCK_PROTOCOL_MODULE(tftp, false)
> 
> in dmg.c:
>     BLOCK_FORMAT_MODULE(dmg, true)
> 
> In generator.py, you can grep for "BLOCK_PROTOCOL_MODULE(*)" and
> "BLOCK_FORMAT_MODULE(*)".
> 

Yes, that makes it simpler.

The only drawback I see is that all drivers that want to be converted
into modules will have to be touched, and the information from the
struct duplicated. It is not a big drawback because this will make the
parser really simple.

Thanks
Marc