Re: [libvirt] [PATCH V3 5/6] xenconfig: support bios=ovmf xl.cfg

On 11.06.2016 00:34, Jim Fehlig wrote: > Add support to xenconfig for conversion of xl.cfg(5) bios config > to/from libvirt domXml <loader> config. SeaBIOS is the default > for HVM guests using upstream QEMU. ROMBIOS is the default when > using the old qemu-dm. This patch allows specifying OVMF as an > alternate firmware. > > Example xl.cfg: > bios = "ovmf" > > Example domXML: > <os> > ... > <loader readonly='yes' type='pflash'>/usr/lib/xen/boot/ovmf.bin</loader> > </os> > > Note that currently Xen does not support a separate nvram for > non-volatile variables. > > Signed-off-by: Jim Fehlig <jfehlig(a)suse.com&gt; > --- > > V3: > - Rework test code to account for discovery of LIBXL_FIRMWARE_DIR > at build time > - Drop code that relies on Xen patches that are not yet committed > to xen.git master > > src/Makefile.am | 2 +- > src/xenconfig/xen_xl.c | 37 ++++++++++--- > tests/testutils.c | 16 ++++++ > tests/testutils.h | 2 + > tests/xlconfigdata/test-fullvirt-ovmf.cfg | 26 +++++++++ > tests/xlconfigdata/test-fullvirt-ovmf.xml | 58 ++++++++++++++++++++ > tests/xlconfigtest.c | 90 +++++++++++++++++++++++-------- > 7 files changed, 202 insertions(+), 29 deletions(-) > > diff --git a/src/Makefile.am b/src/Makefile.am > index 7004d14..1aff57d 100644 > --- a/src/Makefile.am > +++ b/src/Makefile.am > @@ -1151,7 +1151,7 @@ noinst_LTLIBRARIES += libvirt_xenconfig.la > libvirt_la_BUILT_LIBADD += libvirt_xenconfig.la > libvirt_xenconfig_la_LIBADD = $(LIBXL_LIBS) > libvirt_xenconfig_la_CFLAGS = \ > - -I$(srcdir)/conf $(AM_CFLAGS) > + -I$(srcdir)/conf -I$(srcdir)/libxl $(AM_CFLAGS) > libvirt_xenconfig_la_SOURCES = $(XENCONFIG_SOURCES) > endif WITH_XENCONFIG > > diff --git a/src/xenconfig/xen_xl.c b/src/xenconfig/xen_xl.c > index 9b8306f..dc54a73 100644 > --- a/src/xenconfig/xen_xl.c > +++ b/src/xenconfig/xen_xl.c > @@ -33,6 +33,7 @@ > #include "virstring.h" > #include "virstoragefile.h" > #include "xen_xl.h" > +#include "libxl_capabilities.h" > > #define VIR_FROM_THIS VIR_FROM_XENXL > > @@ -103,15 +104,31 @@ xenParseXLOS(virConfPtr conf, virDomainDefPtr def, virCapsPtr caps) > size_t i; > > if (def->os.type == VIR_DOMAIN_OSTYPE_HVM) { > + const char *bios; > const char *boot; > > - for (i = 0; i < caps->nguests; i++) { > - if (caps->guests[i]->ostype == VIR_DOMAIN_OSTYPE_HVM && > - caps->guests[i]->arch.id == def->os.arch) { > - if (VIR_ALLOC(def->os.loader) < 0 || > - VIR_STRDUP(def->os.loader->path, > - caps->guests[i]->arch.defaultInfo.loader) < 0) > - return -1; > + if (xenConfigGetString(conf, "bios", &bios, NULL) < 0) > + return -1; > + > + if (bios && STREQ(bios, "ovmf")) { > + if (VIR_ALLOC(def->os.loader) < 0) > + return -1; > + > + def->os.loader->type = VIR_DOMAIN_LOADER_TYPE_PFLASH; > + def->os.loader->readonly = VIR_TRISTATE_BOOL_YES; > + > + if (VIR_STRDUP(def->os.loader->path, > + LIBXL_FIRMWARE_DIR "/ovmf.bin") < 0) > + return -1; > + } else { > + for (i = 0; i < caps->nguests; i++) { > + if (caps->guests[i]->ostype == VIR_DOMAIN_OSTYPE_HVM && > + caps->guests[i]->arch.id == def->os.arch) { > + if (VIR_ALLOC(def->os.loader) < 0 || > + VIR_STRDUP(def->os.loader->path, > + caps->guests[i]->arch.defaultInfo.loader) < 0) > + return -1; > + } > } > } > > @@ -535,6 +552,12 @@ xenFormatXLOS(virConfPtr conf, virDomainDefPtr def) > if (xenConfigSetString(conf, "builder", "hvm") < 0) > return -1; > > + if (def->os.loader && > + def->os.loader->type == VIR_DOMAIN_LOADER_TYPE_PFLASH) { > + if (xenConfigSetString(conf, "bios", "ovmf") < 0) > + return -1; > + } > + > #ifdef LIBXL_HAVE_BUILDINFO_KERNEL > if (def->os.kernel && > xenConfigSetString(conf, "kernel", def->os.kernel) < 0) > diff --git a/tests/testutils.c b/tests/testutils.c > index e2dd2fe..54adab2 100644 > --- a/tests/testutils.c > +++ b/tests/testutils.c > @@ -718,6 +718,22 @@ virTestCompareToFile(const char *strcontent, > return ret; > } > > +/* > + * @param strcontent: String input content > + * @param strsrc: String source to compare strcontent against > + */ > +int > +virTestCompareToString(const char *strcontent, > + const char *strsrc) > +{ > + if (STRNEQ_NULLABLE(strcontent, strsrc)) { > + virTestDifference(stderr, strcontent, strsrc); > + return -1; > + } > + > + return 0; > +} > + > static void > virTestErrorFuncQuiet(void *data ATTRIBUTE_UNUSED, > virErrorPtr err ATTRIBUTE_UNUSED) > diff --git a/tests/testutils.h b/tests/testutils.h > index e3881e8..c7c641c 100644 > --- a/tests/testutils.h > +++ b/tests/testutils.h > @@ -75,6 +75,8 @@ int virTestDifferenceBin(FILE *stream, > size_t length); > int virTestCompareToFile(const char *strcontent, > const char *filename); > +int virTestCompareToString(const char *strcontent, > + const char *strsrc); > > unsigned int virTestGetDebug(void); > unsigned int virTestGetVerbose(void); > diff --git a/tests/xlconfigdata/test-fullvirt-ovmf.cfg b/tests/xlconfigdata/test-fullvirt-ovmf.cfg > new file mode 100644 > index 0000000..b08e7fd > --- /dev/null > +++ b/tests/xlconfigdata/test-fullvirt-ovmf.cfg > @@ -0,0 +1,26 @@ > +name = "XenGuest2" > +uuid = "c7a5fdb2-cdaf-9455-926a-d65c16db1809" > +maxmem = 579 > +memory = 394 > +vcpus = 1 > +pae = 1 > +acpi = 1 > +apic = 1 > +viridian = 0 > +rtc_timeoffset = 0 > +localtime = 0 > +on_poweroff = "destroy" > +on_reboot = "restart" > +on_crash = "restart" > +device_model = "/usr/lib/xen/bin/qemu-system-i386" > +sdl = 0 > +vnc = 1 > +vncunused = 1 > +vnclisten = "127.0.0.1" > +vif = [ "mac=00:16:3e:66:92:9c,bridge=xenbr1,script=vif-bridge,model=e1000" ] > +parallel = "none" > +serial = "none" > +builder = "hvm" > +bios = "ovmf" > +boot = "d" > +disk = [ "format=raw,vdev=hda,access=rw,backendtype=phy,target=/dev/HostVG/XenGuest2", "format=qcow2,vdev=hdb,access=rw,backendtype=qdisk,target=/var/lib/libvirt/images/XenGuest2-home", "format=raw,vdev=hdc,access=ro,backendtype=qdisk,devtype=cdrom,target=/root/boot.iso" ] > diff --git a/tests/xlconfigdata/test-fullvirt-ovmf.xml b/tests/xlconfigdata/test-fullvirt-ovmf.xml > new file mode 100644 > index 0000000..ca902e6 > --- /dev/null > +++ b/tests/xlconfigdata/test-fullvirt-ovmf.xml > @@ -0,0 +1,58 @@ > +<domain type='xen'> > + <name>XenGuest2</name> > + <uuid>c7a5fdb2-cdaf-9455-926a-d65c16db1809</uuid> > + <memory unit='KiB'>592896</memory> > + <currentMemory unit='KiB'>403456</currentMemory> > + <vcpu placement='static'>1</vcpu> > + <os> > + <type arch='x86_64' machine='xenfv'>hvm</type> > + <loader readonly='yes' type='pflash'>/LIBXL_FIRMWARE_DIR/ovmf.bin</loader> > + <boot dev='cdrom'/> > + </os> > + <features> > + <acpi/> > + <apic/> > + <pae/> > + </features> > + <clock offset='variable' adjustment='0' basis='utc'/> > + <on_poweroff>destroy</on_poweroff> > + <on_reboot>restart</on_reboot> > + <on_crash>restart</on_crash> > + <devices> > + <emulator>/usr/lib/xen/bin/qemu-system-i386</emulator> > + <disk type='block' device='disk'> > + <driver name='phy' type='raw'/> > + <source dev='/dev/HostVG/XenGuest2'/> > + <target dev='hda' bus='ide'/> > + <address type='drive' controller='0' bus='0' target='0' unit='0'/> > + </disk> > + <disk type='file' device='disk'> > + <driver name='qemu' type='qcow2'/> > + <source file='/var/lib/libvirt/images/XenGuest2-home'/> > + <target dev='hdb' bus='ide'/> > + <address type='drive' controller='0' bus='0' target='0' unit='1'/> > + </disk> > + <disk type='file' device='cdrom'> > + <driver name='qemu' type='raw'/> > + <source file='/root/boot.iso'/> > + <target dev='hdc' bus='ide'/> > + <readonly/> > + <address type='drive' controller='0' bus='1' target='0' unit='0'/> > + </disk> > + <controller type='ide' index='0'/> > + <interface type='bridge'> > + <mac address='00:16:3e:66:92:9c'/> > + <source bridge='xenbr1'/> > + <script path='vif-bridge'/> > + <model type='e1000'/> > + </interface> > + <input type='mouse' bus='ps2'/> > + <input type='keyboard' bus='ps2'/> > + <graphics type='vnc' port='-1' autoport='yes' listen='127.0.0.1'> > + <listen type='address' address='127.0.0.1'/> > + </graphics> > + <video> > + <model type='cirrus' vram='8192' heads='1' primary='yes'/> > + </video> > + </devices> > +</domain> > diff --git a/tests/xlconfigtest.c b/tests/xlconfigtest.c > index 62bb144..55c756c 100644 > --- a/tests/xlconfigtest.c > +++ b/tests/xlconfigtest.c > @@ -43,12 +43,33 @@ > static virCapsPtr caps; > static virDomainXMLOptionPtr xmlopt; > > + > +/* > + * This function provides a mechanism to replace variables in test > + * data files whose values are discovered at built time. > + */ > +static char * > +testReplaceVarsXML(const char *xml) > +{ > + char *xmlcfgData; > + char *replacedXML; > + > + if (virTestLoadFile(xml, &xmlcfgData) < 0) > + return NULL; > + > + replacedXML = virStringReplace(xmlcfgData, "/LIBXL_FIRMWARE_DIR", > + LIBXL_FIRMWARE_DIR); I think we want to replace just LIBXL_FW_DIR without the leading slash. That's what would have been done if we had .in files anyway.