[libvirt] [PATCH 0/2] Add virFirmware object for use by driver config

[Jim Fehlig]

Hi All,

Any comments to this approach of enabling some code sharing between qemu and libxl?

Regards,
Jim

On 05/18/2016 10:38 AM, Jim Fehlig wrote:
> While refreshing and testing my series to add domain capabilities
> and UEFI support to the libxl driver [1], I realized it would be
> useful to support the '--with-loader-nvram' configure option as
> well. This would allow users/packagers to specify system-provided
> firmwares instead of those provided by the Xen installation.
>
> Instead of copying code from the qemu driver, I decided to create
> a virFirmware object containing the firmware name (loader) and
> associated nvram, along with a few operations on the object. Patch1
> introduces the object in src/util/virfirmware.[ch] and switches
> the qemu driver to use. The patch could be split along those lines,
> but I thought it would be review-friendly to see the movement of
> code in a single patch. But it can be split if preferred.
>
> Patch2 adds virFirmware to the libxl driver config. It is unused
> in this series, but if this approach is aggreable it will be used
> when rebasing the domain capabilities series.
>
> Note that currently Xen does not support storage for non-volitile
> variables for UEFI firmware (OVMF), so 'nvram' will be unused for
> the time being. Adding a 'nvram' option to libxl.conf can be
> deferred until support is added to Xen. But even with this
> limitation, folks are using UEFI + HVM guests with xl/libxl and
> would like to do the same with libvirt.
>
> Patch1 is a lot of code movement to avoid copying
> virQEMUDriverConfigLoaderNVRAMParse (and eventually
> virQEMUDriverConfigNVRAMParse). But I've complained about copy/paste
> in some of my recent reviews and shouldn't be doing the same in
> my own patches :-).
>
> [1] https://www.redhat.com/archives/libvir-list/2016-April/msg01358.html
>
> Jim Fehlig (2):
>   driver config: Introduce virFirmware object
>   libxl: add default firmwares to driver config object
>
>  po/POTFILES.in               |   1 +
>  src/Makefile.am              |   1 +
>  src/libvirt_private.syms     |   6 ++
>  src/libxl/libxl_conf.c       |  21 +++++++
>  src/libxl/libxl_conf.h       |   4 ++
>  src/qemu/qemu_capabilities.c |  22 +++----
>  src/qemu/qemu_capabilities.h |   5 +-
>  src/qemu/qemu_conf.c         | 127 ++++++----------------------------------
>  src/qemu/qemu_conf.h         |   7 +--
>  src/qemu/qemu_driver.c       |   2 +-
>  src/qemu/qemu_process.c      |   6 +-
>  src/util/virfirmware.c       | 135 +++++++++++++++++++++++++++++++++++++++++++
>  src/util/virfirmware.h       |  51 ++++++++++++++++
>  tests/domaincapstest.c       |   3 +-
>  14 files changed, 260 insertions(+), 131 deletions(-)
>  create mode 100644 src/util/virfirmware.c
>  create mode 100644 src/util/virfirmware.h
>