[libvirt] [PATCH V3 0/4] Xen-xl parser

This is V3 of David's series to add support for parsing and formating Xen's xl config format https://www.redhat.com/archives/libvir-list/2014-September/msg00587.html Patch 1 has been rebased, but otherwise unchanged. In patch 2, I was able to get automake and flex play together using LEX_OUTPUT_ROOT and AM_LFLAGS. Files generated from flex are placed in a separate convenience lib to avoid compilation with WARN_FLAGS. The generated files are also excluded from syntax-check. I switched the order of patches 3 and 4 since config parsing/ formating tests should come right after the implementation. I also added more diskspecs to the test configs. Kiarie Kahurani (4): src/xenconfig: Export helper functions src/xenconfig: Xen-xl parser tests: Tests for the xen-xl parser libxl: Add support for parsing/formating Xen XL config .gitignore | 1 + cfg.mk | 3 +- configure.ac | 1 + po/POTFILES.in | 1 + src/Makefile.am | 25 +- src/libvirt_xenconfig.syms | 4 + src/libxl/libxl_driver.c | 32 ++- src/xenconfig/xen_common.c | 149 +++++------ src/xenconfig/xen_common.h | 24 +- src/xenconfig/xen_xl.c | 499 +++++++++++++++++++++++++++++++++++ src/xenconfig/xen_xl.h | 33 +++ src/xenconfig/xen_xl_disk.l | 256 ++++++++++++++++++ src/xenconfig/xen_xl_disk_i.h | 39 +++ tests/Makefile.am | 9 +- tests/testutilsxen.c | 50 ++++ tests/testutilsxen.h | 9 +- tests/xlconfigdata/test-new-disk.cfg | 26 ++ tests/xlconfigdata/test-new-disk.xml | 51 ++++ tests/xlconfigdata/test-spice.cfg | 32 +++ tests/xlconfigdata/test-spice.xml | 45 ++++ tests/xlconfigtest.c | 224 ++++++++++++++++ 21 files changed, 1421 insertions(+), 92 deletions(-) create mode 100644 src/xenconfig/xen_xl.c create mode 100644 src/xenconfig/xen_xl.h create mode 100644 src/xenconfig/xen_xl_disk.l create mode 100644 src/xenconfig/xen_xl_disk_i.h create mode 100644 tests/xlconfigdata/test-new-disk.cfg create mode 100644 tests/xlconfigdata/test-new-disk.xml create mode 100644 tests/xlconfigdata/test-spice.cfg create mode 100644 tests/xlconfigdata/test-spice.xml create mode 100644 tests/xlconfigtest.c -- 1.8.4.5

From: Kiarie Kahurani <davidkiarie4@gmail.com> Export helper functions for reuse in getting values from a virConfPtr object Signed-off-by: Kiarie Kahurani <davidkiarie4@gmail.com> Signed-off-by: Jim Fehlig <jfehlig@suse.com> --- src/xenconfig/xen_common.c | 146 ++++++++++++++++++++++----------------------- src/xenconfig/xen_common.h | 24 ++++++-- 2 files changed, 92 insertions(+), 78 deletions(-) diff --git a/src/xenconfig/xen_common.c b/src/xenconfig/xen_common.c index 7f4ec89..8ff10a0 100644 --- a/src/xenconfig/xen_common.c +++ b/src/xenconfig/xen_common.c @@ -43,7 +43,7 @@ /* * Convenience method to grab a long int from the config file object */ -static int +int xenConfigGetBool(virConfPtr conf, const char *name, int *value, @@ -73,7 +73,7 @@ xenConfigGetBool(virConfPtr conf, /* * Convenience method to grab a int from the config file object */ -static int +int xenConfigGetULong(virConfPtr conf, const char *name, unsigned long *value, @@ -179,7 +179,7 @@ xenConfigCopyString(virConfPtr conf, const char *name, char **value) } -static int +int xenConfigCopyStringOpt(virConfPtr conf, const char *name, char **value) { return xenConfigCopyStringInternal(conf, name, value, 1); @@ -262,8 +262,8 @@ xenConfigGetString(virConfPtr conf, } -static int -xenXMConfigSetInt(virConfPtr conf, const char *setting, long long l) +int +xenConfigSetInt(virConfPtr conf, const char *setting, long long l) { virConfValuePtr value = NULL; @@ -283,8 +283,8 @@ xenXMConfigSetInt(virConfPtr conf, const char *setting, long long l) } -static int -xenXMConfigSetString(virConfPtr conf, const char *setting, const char *str) +int +xenConfigSetString(virConfPtr conf, const char *setting, const char *str) { virConfValuePtr value = NULL; @@ -1384,11 +1384,11 @@ xenFormatGeneralMeta(virConfPtr conf, virDomainDefPtr def) { char uuid[VIR_UUID_STRING_BUFLEN]; - if (xenXMConfigSetString(conf, "name", def->name) < 0) + if (xenConfigSetString(conf, "name", def->name) < 0) return -1; virUUIDFormat(def->uuid, uuid); - if (xenXMConfigSetString(conf, "uuid", uuid) < 0) + if (xenConfigSetString(conf, "uuid", uuid) < 0) return -1; return 0; @@ -1398,12 +1398,12 @@ xenFormatGeneralMeta(virConfPtr conf, virDomainDefPtr def) static int xenFormatMem(virConfPtr conf, virDomainDefPtr def) { - if (xenXMConfigSetInt(conf, "maxmem", - VIR_DIV_UP(def->mem.max_balloon, 1024)) < 0) + if (xenConfigSetInt(conf, "maxmem", + VIR_DIV_UP(def->mem.max_balloon, 1024)) < 0) return -1; - if (xenXMConfigSetInt(conf, "memory", - VIR_DIV_UP(def->mem.cur_balloon, 1024)) < 0) + if (xenConfigSetInt(conf, "memory", + VIR_DIV_UP(def->mem.cur_balloon, 1024)) < 0) return -1; return 0; @@ -1465,7 +1465,7 @@ xenFormatTimeOffset(virConfPtr conf, virDomainDefPtr def, int xendConfigVersion) virDomainClockOffsetTypeToString(def->clock.offset)); return -1; } - if (xenXMConfigSetInt(conf, "rtc_timeoffset", rtc_timeoffset) < 0) + if (xenConfigSetInt(conf, "rtc_timeoffset", rtc_timeoffset) < 0) return -1; } else { @@ -1486,7 +1486,7 @@ xenFormatTimeOffset(virConfPtr conf, virDomainDefPtr def, int xendConfigVersion) } /* !hvm */ } - if (xenXMConfigSetInt(conf, "localtime", vmlocaltime) < 0) + if (xenConfigSetInt(conf, "localtime", vmlocaltime) < 0) return -1; return 0; @@ -1503,7 +1503,7 @@ xenFormatEventActions(virConfPtr conf, virDomainDefPtr def) _("unexpected lifecycle action %d"), def->onPoweroff); return -1; } - if (xenXMConfigSetString(conf, "on_poweroff", lifecycle) < 0) + if (xenConfigSetString(conf, "on_poweroff", lifecycle) < 0) return -1; @@ -1512,7 +1512,7 @@ xenFormatEventActions(virConfPtr conf, virDomainDefPtr def) _("unexpected lifecycle action %d"), def->onReboot); return -1; } - if (xenXMConfigSetString(conf, "on_reboot", lifecycle) < 0) + if (xenConfigSetString(conf, "on_reboot", lifecycle) < 0) return -1; @@ -1521,7 +1521,7 @@ xenFormatEventActions(virConfPtr conf, virDomainDefPtr def) _("unexpected lifecycle action %d"), def->onCrash); return -1; } - if (xenXMConfigSetString(conf, "on_crash", lifecycle) < 0) + if (xenConfigSetString(conf, "on_crash", lifecycle) < 0) return -1; return 0; @@ -1542,12 +1542,12 @@ xenFormatCharDev(virConfPtr conf, virDomainDefPtr def) ret = xenFormatSxprChr(def->parallels[0], &buf); str = virBufferContentAndReset(&buf); if (ret == 0) - ret = xenXMConfigSetString(conf, "parallel", str); + ret = xenConfigSetString(conf, "parallel", str); VIR_FREE(str); if (ret < 0) return -1; } else { - if (xenXMConfigSetString(conf, "parallel", "none") < 0) + if (xenConfigSetString(conf, "parallel", "none") < 0) return -1; } @@ -1560,7 +1560,7 @@ xenFormatCharDev(virConfPtr conf, virDomainDefPtr def) ret = xenFormatSxprChr(def->serials[0], &buf); str = virBufferContentAndReset(&buf); if (ret == 0) - ret = xenXMConfigSetString(conf, "serial", str); + ret = xenConfigSetString(conf, "serial", str); VIR_FREE(str); if (ret < 0) return -1; @@ -1605,7 +1605,7 @@ xenFormatCharDev(virConfPtr conf, virDomainDefPtr def) VIR_FREE(serialVal); } } else { - if (xenXMConfigSetString(conf, "serial", "none") < 0) + if (xenConfigSetString(conf, "serial", "none") < 0) return -1; } } @@ -1620,13 +1620,13 @@ xenFormatCPUAllocation(virConfPtr conf, virDomainDefPtr def) int ret = -1; char *cpus = NULL; - if (xenXMConfigSetInt(conf, "vcpus", def->maxvcpus) < 0) + if (xenConfigSetInt(conf, "vcpus", def->maxvcpus) < 0) goto cleanup; /* Computing the vcpu_avail bitmask works because MAX_VIRT_CPUS is either 32, or 64 on a platform where long is big enough. */ if (def->vcpus < def->maxvcpus && - xenXMConfigSetInt(conf, "vcpu_avail", (1UL << def->vcpus) - 1) < 0) + xenConfigSetInt(conf, "vcpu_avail", (1UL << def->vcpus) - 1) < 0) goto cleanup; if ((def->cpumask != NULL) && @@ -1635,7 +1635,7 @@ xenFormatCPUAllocation(virConfPtr conf, virDomainDefPtr def) } if (cpus && - xenXMConfigSetString(conf, "cpus", cpus) < 0) + xenConfigSetString(conf, "cpus", cpus) < 0) goto cleanup; ret = 0; @@ -1652,37 +1652,37 @@ xenFormatCPUFeatures(virConfPtr conf, virDomainDefPtr def, int xendConfigVersion size_t i; if (STREQ(def->os.type, "hvm")) { - if (xenXMConfigSetInt(conf, "pae", - (def->features[VIR_DOMAIN_FEATURE_PAE] == - VIR_TRISTATE_SWITCH_ON) ? 1 : 0) < 0) + if (xenConfigSetInt(conf, "pae", + (def->features[VIR_DOMAIN_FEATURE_PAE] == + VIR_TRISTATE_SWITCH_ON) ? 1 : 0) < 0) return -1; - if (xenXMConfigSetInt(conf, "acpi", - (def->features[VIR_DOMAIN_FEATURE_ACPI] == - VIR_TRISTATE_SWITCH_ON) ? 1 : 0) < 0) + if (xenConfigSetInt(conf, "acpi", + (def->features[VIR_DOMAIN_FEATURE_ACPI] == + VIR_TRISTATE_SWITCH_ON) ? 1 : 0) < 0) return -1; - if (xenXMConfigSetInt(conf, "apic", - (def->features[VIR_DOMAIN_FEATURE_APIC] == - VIR_TRISTATE_SWITCH_ON) ? 1 : 0) < 0) + if (xenConfigSetInt(conf, "apic", + (def->features[VIR_DOMAIN_FEATURE_APIC] == + VIR_TRISTATE_SWITCH_ON) ? 1 : 0) < 0) return -1; if (xendConfigVersion >= XEND_CONFIG_VERSION_3_0_4) { - if (xenXMConfigSetInt(conf, "hap", - (def->features[VIR_DOMAIN_FEATURE_HAP] == - VIR_TRISTATE_SWITCH_ON) ? 1 : 0) < 0) + if (xenConfigSetInt(conf, "hap", + (def->features[VIR_DOMAIN_FEATURE_HAP] == + VIR_TRISTATE_SWITCH_ON) ? 1 : 0) < 0) return -1; - if (xenXMConfigSetInt(conf, "viridian", - (def->features[VIR_DOMAIN_FEATURE_VIRIDIAN] == - VIR_TRISTATE_SWITCH_ON) ? 1 : 0) < 0) + if (xenConfigSetInt(conf, "viridian", + (def->features[VIR_DOMAIN_FEATURE_VIRIDIAN] == + VIR_TRISTATE_SWITCH_ON) ? 1 : 0) < 0) return -1; } for (i = 0; i < def->clock.ntimers; i++) { if (def->clock.timers[i]->name == VIR_DOMAIN_TIMER_NAME_HPET && def->clock.timers[i]->present != -1 && - xenXMConfigSetInt(conf, "hpet", def->clock.timers[i]->present) < 0) + xenConfigSetInt(conf, "hpet", def->clock.timers[i]->present) < 0) return -1; } } @@ -1695,7 +1695,7 @@ static int xenFormatEmulator(virConfPtr conf, virDomainDefPtr def) { if (def->emulator && - xenXMConfigSetString(conf, "device_model", def->emulator) < 0) + xenConfigSetString(conf, "device_model", def->emulator) < 0) return -1; return 0; @@ -1714,8 +1714,8 @@ xenFormatCDROM(virConfPtr conf, virDomainDefPtr def, int xendConfigVersion) def->disks[i]->dst && STREQ(def->disks[i]->dst, "hdc") && virDomainDiskGetSource(def->disks[i])) { - if (xenXMConfigSetString(conf, "cdrom", - virDomainDiskGetSource(def->disks[i])) < 0) + if (xenConfigSetString(conf, "cdrom", + virDomainDiskGetSource(def->disks[i])) < 0) return -1; break; } @@ -1734,11 +1734,11 @@ xenFormatOS(virConfPtr conf, virDomainDefPtr def) if (STREQ(def->os.type, "hvm")) { char boot[VIR_DOMAIN_BOOT_LAST+1]; - if (xenXMConfigSetString(conf, "builder", "hvm") < 0) + if (xenConfigSetString(conf, "builder", "hvm") < 0) return -1; if (def->os.loader && def->os.loader->path && - xenXMConfigSetString(conf, "kernel", def->os.loader->path) < 0) + xenConfigSetString(conf, "kernel", def->os.loader->path) < 0) return -1; for (i = 0; i < def->os.nBootDevs; i++) { @@ -1766,29 +1766,29 @@ xenFormatOS(virConfPtr conf, virDomainDefPtr def) boot[def->os.nBootDevs] = '\0'; } - if (xenXMConfigSetString(conf, "boot", boot) < 0) + if (xenConfigSetString(conf, "boot", boot) < 0) return -1; /* XXX floppy disks */ } else { if (def->os.bootloader && - xenXMConfigSetString(conf, "bootloader", def->os.bootloader) < 0) + xenConfigSetString(conf, "bootloader", def->os.bootloader) < 0) return -1; if (def->os.bootloaderArgs && - xenXMConfigSetString(conf, "bootargs", def->os.bootloaderArgs) < 0) + xenConfigSetString(conf, "bootargs", def->os.bootloaderArgs) < 0) return -1; if (def->os.kernel && - xenXMConfigSetString(conf, "kernel", def->os.kernel) < 0) + xenConfigSetString(conf, "kernel", def->os.kernel) < 0) return -1; if (def->os.initrd && - xenXMConfigSetString(conf, "ramdisk", def->os.initrd) < 0) + xenConfigSetString(conf, "ramdisk", def->os.initrd) < 0) return -1; if (def->os.cmdline && - xenXMConfigSetString(conf, "extra", def->os.cmdline) < 0) + xenConfigSetString(conf, "extra", def->os.cmdline) < 0) return -1; } /* !hvm */ @@ -1804,52 +1804,52 @@ xenFormatVfb(virConfPtr conf, virDomainDefPtr def, int xendConfigVersion) if (def->ngraphics == 1) { if (hvm || (xendConfigVersion < XEND_CONFIG_MIN_VERS_PVFB_NEWCONF)) { if (def->graphics[0]->type == VIR_DOMAIN_GRAPHICS_TYPE_SDL) { - if (xenXMConfigSetInt(conf, "sdl", 1) < 0) + if (xenConfigSetInt(conf, "sdl", 1) < 0) return -1; - if (xenXMConfigSetInt(conf, "vnc", 0) < 0) + if (xenConfigSetInt(conf, "vnc", 0) < 0) return -1; if (def->graphics[0]->data.sdl.display && - xenXMConfigSetString(conf, "display", - def->graphics[0]->data.sdl.display) < 0) + xenConfigSetString(conf, "display", + def->graphics[0]->data.sdl.display) < 0) return -1; if (def->graphics[0]->data.sdl.xauth && - xenXMConfigSetString(conf, "xauthority", - def->graphics[0]->data.sdl.xauth) < 0) + xenConfigSetString(conf, "xauthority", + def->graphics[0]->data.sdl.xauth) < 0) return -1; } else { const char *listenAddr; - if (xenXMConfigSetInt(conf, "sdl", 0) < 0) + if (xenConfigSetInt(conf, "sdl", 0) < 0) return -1; - if (xenXMConfigSetInt(conf, "vnc", 1) < 0) + if (xenConfigSetInt(conf, "vnc", 1) < 0) return -1; - if (xenXMConfigSetInt(conf, "vncunused", + if (xenConfigSetInt(conf, "vncunused", def->graphics[0]->data.vnc.autoport ? 1 : 0) < 0) return -1; if (!def->graphics[0]->data.vnc.autoport && - xenXMConfigSetInt(conf, "vncdisplay", - def->graphics[0]->data.vnc.port - 5900) < 0) + xenConfigSetInt(conf, "vncdisplay", + def->graphics[0]->data.vnc.port - 5900) < 0) return -1; listenAddr = virDomainGraphicsListenGetAddress(def->graphics[0], 0); if (listenAddr && - xenXMConfigSetString(conf, "vnclisten", listenAddr) < 0) + xenConfigSetString(conf, "vnclisten", listenAddr) < 0) return -1; if (def->graphics[0]->data.vnc.auth.passwd && - xenXMConfigSetString(conf, "vncpasswd", - def->graphics[0]->data.vnc.auth.passwd) < 0) + xenConfigSetString(conf, "vncpasswd", + def->graphics[0]->data.vnc.auth.passwd) < 0) return -1; if (def->graphics[0]->data.vnc.keymap && - xenXMConfigSetString(conf, "keymap", - def->graphics[0]->data.vnc.keymap) < 0) + xenConfigSetString(conf, "keymap", + def->graphics[0]->data.vnc.keymap) < 0) return -1; } } else { @@ -1925,7 +1925,7 @@ xenFormatSound(virConfPtr conf, virDomainDefPtr def) str = virBufferContentAndReset(&buf); if (ret == 0) - ret = xenXMConfigSetString(conf, "soundhw", str); + ret = xenConfigSetString(conf, "soundhw", str); VIR_FREE(str); if (ret < 0) @@ -1945,22 +1945,22 @@ xenFormatInputDevs(virConfPtr conf, virDomainDefPtr def) if (STREQ(def->os.type, "hvm")) { for (i = 0; i < def->ninputs; i++) { if (def->inputs[i]->bus == VIR_DOMAIN_INPUT_BUS_USB) { - if (xenXMConfigSetInt(conf, "usb", 1) < 0) + if (xenConfigSetInt(conf, "usb", 1) < 0) return -1; switch (def->inputs[i]->type) { case VIR_DOMAIN_INPUT_TYPE_MOUSE: - if (xenXMConfigSetString(conf, "usbdevice", "mouse") < 0) + if (xenConfigSetString(conf, "usbdevice", "mouse") < 0) return -1; break; case VIR_DOMAIN_INPUT_TYPE_TABLET: - if (xenXMConfigSetString(conf, "usbdevice", "tablet") < 0) + if (xenConfigSetString(conf, "usbdevice", "tablet") < 0) return -1; break; case VIR_DOMAIN_INPUT_TYPE_KBD: - if (xenXMConfigSetString(conf, "usbdevice", "keyboard") < 0) + if (xenConfigSetString(conf, "usbdevice", "keyboard") < 0) return -1; break; diff --git a/src/xenconfig/xen_common.h b/src/xenconfig/xen_common.h index 9f50aef..e993b21 100644 --- a/src/xenconfig/xen_common.h +++ b/src/xenconfig/xen_common.h @@ -27,11 +27,25 @@ # include "virconf.h" # include "domain_conf.h" -int -xenConfigGetString(virConfPtr conf, - const char *name, - const char **value, - const char *def); +int xenConfigGetString(virConfPtr conf, + const char *name, + const char **value, + const char *def); + +int xenConfigGetBool(virConfPtr conf, const char *name, int *value, int def); + +int xenConfigSetInt(virConfPtr conf, const char *name, long long value); + +int xenConfigSetString(virConfPtr conf, const char *setting, const char *value); + +int xenConfigGetULong(virConfPtr conf, + const char *name, + unsigned long *value, + unsigned long def); + +int xenConfigCopyStringOpt(virConfPtr conf, + const char *name, + char **value); int xenParseConfigCommon(virConfPtr conf, virDomainDefPtr def, -- 1.8.4.5

On 16.12.2014 05:30, Jim Fehlig wrote:
From: Kiarie Kahurani <davidkiarie4@gmail.com>
Export helper functions for reuse in getting values from a virConfPtr object
Signed-off-by: Kiarie Kahurani <davidkiarie4@gmail.com> Signed-off-by: Jim Fehlig <jfehlig@suse.com> --- src/xenconfig/xen_common.c | 146 ++++++++++++++++++++++----------------------- src/xenconfig/xen_common.h | 24 ++++++-- 2 files changed, 92 insertions(+), 78 deletions(-)
ACK Michal

From: Kiarie Kahurani <davidkiarie4@gmail.com> Introduce a Xen xl parser This parser allows for users to convert the new xl disk format and spice graphics config to libvirt xml format and vice versa. Regarding the spice graphics config, the code is pretty much straight forward. For the disk {formating, parsing}, this parser takes care of the new xl format which include positional parameters and key/value parameters. In xl format disk config a <diskspec> consists of parameters separated by commas. If the parameters do not contain an '=' they are automatically assigned to certain options following the order below target, format, vdev, access The above are the only mandatory parameters in the <diskspec> but there are many more disk config options. These options can be specified as key=value pairs. This takes care of the rest of the options such as devtype, backend, backendtype, script, direct-io-safe, The positional paramters can also be specified in key/value form for example /dev/vg/guest-volume,,hda /dev/vg/guest-volume,raw,hda,rw format=raw, vdev=hda, access=rw, target=/dev/vg/guest-volume are interpleted to one config. In xm format, the above diskspec would be written as phy:/dev/vg/guest-volume,hda,w The disk parser is based on the same parser used successfully by the Xen project for several years now. Ian Jackson authored the scanner, which is used by this commit with mimimal changes. Only the PREFIX option is changed, to produce function and file names more consistent with libvirt's convention. Signed-off-by: Kiarie Kahurani <davidkiarie4@gmail.com> Signed-off-by: Jim Fehlig <jfehlig@suse.com> --- .gitignore | 1 + cfg.mk | 3 +- configure.ac | 1 + po/POTFILES.in | 1 + src/Makefile.am | 25 ++- src/libvirt_xenconfig.syms | 4 + src/xenconfig/xen_common.c | 3 +- src/xenconfig/xen_xl.c | 499 ++++++++++++++++++++++++++++++++++++++++++ src/xenconfig/xen_xl.h | 33 +++ src/xenconfig/xen_xl_disk.l | 256 ++++++++++++++++++++++ src/xenconfig/xen_xl_disk_i.h | 39 ++++ 11 files changed, 861 insertions(+), 4 deletions(-) diff --git a/.gitignore b/.gitignore index 9d09709..eac2203 100644 --- a/.gitignore +++ b/.gitignore @@ -140,6 +140,7 @@ /src/remote/*_protocol.[ch] /src/rpc/virkeepaliveprotocol.[ch] /src/rpc/virnetprotocol.[ch] +/src/xenconfig/xen_xl_disk.[ch] /src/test_libvirt*.aug /src/test_virtlockd.aug /src/util/virkeymaps.h diff --git a/cfg.mk b/cfg.mk index 21f83c3..3df3dcb 100644 --- a/cfg.mk +++ b/cfg.mk @@ -89,8 +89,9 @@ distdir: sc_vulnerable_makefile_CVE-2012-3386.z endif # Files that should never cause syntax check failures. +# (^(HACKING|docs/(news\.html\.in|.*\.patch))|\.(po|fig|gif|ico|png))$$ VC_LIST_ALWAYS_EXCLUDE_REGEX = \ - (^(HACKING|docs/(news\.html\.in|.*\.patch))|\.(po|fig|gif|ico|png))$$ + (^(HACKING|docs/(news\.html\.in|.*\.patch)|src/xenconfig/xen_xl_disk.[chl])|\.(po|fig|gif|ico|png))$$ # Functions like free() that are no-ops on NULL arguments. useless_free_options = \ diff --git a/configure.ac b/configure.ac index 9fd44b2..777367e 100644 --- a/configure.ac +++ b/configure.ac @@ -146,6 +146,7 @@ m4_ifndef([LT_INIT], [ ]) AM_PROG_CC_C_O AM_PROG_LD +AM_PROG_LEX AC_MSG_CHECKING([for how to mark DSO non-deletable at runtime]) LIBVIRT_NODELETE= diff --git a/po/POTFILES.in b/po/POTFILES.in index e7cb2cc..094c8e3 100644 --- a/po/POTFILES.in +++ b/po/POTFILES.in @@ -247,6 +247,7 @@ src/xenapi/xenapi_driver.c src/xenapi/xenapi_utils.c src/xenconfig/xen_common.c src/xenconfig/xen_sxpr.c +src/xenconfig/xen_xl.c src/xenconfig/xen_xm.c tests/virpolkittest.c tools/libvirt-guests.sh.in diff --git a/src/Makefile.am b/src/Makefile.am index b6c1701..23c433d 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -999,11 +999,22 @@ CPU_SOURCES = \ VMX_SOURCES = \ vmx/vmx.c vmx/vmx.h +AM_LFLAGS = -Pxl_disk_ --header-file=../$*.h +LEX_OUTPUT_ROOT = lex.xl_disk_ +BUILT_SOURCES += xenconfig/xen_xl_disk.c xenconfig/xen_xl_disk.h +# Generated header file is not implicitly added to dist +EXTRA_DIST += xenconfig/xen_xl_disk.h +CLEANFILES += xenconfig/xen_xl_disk.h xenconfig/xen_xl_disk.c + +XENXLDISKPARSER_SOURCES = xenconfig/xen_xl_disk.l + XENCONFIG_SOURCES = \ xenconfig/xenxs_private.h \ - xenconfig/xen_common.c xenconfig/xen_common.h \ + xenconfig/xen_common.c xenconfig/xen_common.h \ xenconfig/xen_sxpr.c xenconfig/xen_sxpr.h \ - xenconfig/xen_xm.c xenconfig/xen_xm.h + xenconfig/xen_xm.c xenconfig/xen_xm.h \ + xenconfig/xen_xl.c xenconfig/xen_xl.h \ + xenconfig/xen_xl_disk_i.h pkgdata_DATA = cpu/cpu_map.xml @@ -1058,10 +1069,19 @@ libvirt_vmx_la_SOURCES = $(VMX_SOURCES) endif WITH_VMX if WITH_XENCONFIG +# Flex generated XL disk parser needs to be compiled without WARN_FLAGS +# 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 +libvirt_xenxldiskparser_la_SOURCES = \ + $(XENXLDISKPARSER_SOURCES) + noinst_LTLIBRARIES += libvirt_xenconfig.la libvirt_la_BUILT_LIBADD += libvirt_xenconfig.la libvirt_xenconfig_la_CFLAGS = \ -I$(top_srcdir)/src/conf $(AM_CFLAGS) +libvirt_xenconfig_la_LIBADD = libvirt_xenxldiskparser.la libvirt_xenconfig_la_SOURCES = $(XENCONFIG_SOURCES) endif WITH_XENCONFIG @@ -1823,6 +1843,7 @@ EXTRA_DIST += \ $(VBOX_DRIVER_EXTRA_DIST) \ $(VMWARE_DRIVER_SOURCES) \ $(XENCONFIG_SOURCES) \ + $(XENXLDISKPARSER_SOURCES) \ $(ACCESS_DRIVER_POLKIT_POLICY) check-local: check-augeas diff --git a/src/libvirt_xenconfig.syms b/src/libvirt_xenconfig.syms index 6541685..3e2e5d6 100644 --- a/src/libvirt_xenconfig.syms +++ b/src/libvirt_xenconfig.syms @@ -16,6 +16,10 @@ xenParseSxprChar; xenParseSxprSound; xenParseSxprString; +#xenconfig/xen_xl.h +xenFormatXL; +xenParseXL; + # xenconfig/xen_xm.h xenFormatXM; xenParseXM; diff --git a/src/xenconfig/xen_common.c b/src/xenconfig/xen_common.c index 8ff10a0..b94b5db 100644 --- a/src/xenconfig/xen_common.c +++ b/src/xenconfig/xen_common.c @@ -1801,7 +1801,8 @@ xenFormatVfb(virConfPtr conf, virDomainDefPtr def, int xendConfigVersion) { int hvm = STREQ(def->os.type, "hvm") ? 1 : 0; - if (def->ngraphics == 1) { + if (def->ngraphics == 1 && + def->graphics[0]->type != VIR_DOMAIN_GRAPHICS_TYPE_SPICE) { if (hvm || (xendConfigVersion < XEND_CONFIG_MIN_VERS_PVFB_NEWCONF)) { if (def->graphics[0]->type == VIR_DOMAIN_GRAPHICS_TYPE_SDL) { if (xenConfigSetInt(conf, "sdl", 1) < 0) diff --git a/src/xenconfig/xen_xl.c b/src/xenconfig/xen_xl.c new file mode 100644 index 0000000..8d1d2a7 --- /dev/null +++ b/src/xenconfig/xen_xl.c @@ -0,0 +1,499 @@ +/* + * xen_xl.c: Xen XL parsing functions + * + * This library 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; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library 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. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library. If not, see + * <http://www.gnu.org/licenses/>. + * + * Author: Kiarie Kahurani <davidkiarie4@gmail.com> + */ + +#include <config.h> + +#include "virconf.h" +#include "virerror.h" +#include "domain_conf.h" +#include "viralloc.h" +#include "virstring.h" +#include "xen_xl.h" +#include "xen_xl_disk.h" +#include "xen_xl_disk_i.h" + +#define VIR_FROM_THIS VIR_FROM_NONE + + +static int +xenParseXLSpice(virConfPtr conf, virDomainDefPtr def) +{ + virDomainGraphicsDefPtr graphics = NULL; + unsigned long port; + char *listenAddr = NULL; + int val; + + if (STREQ(def->os.type, "hvm")) { + if (xenConfigGetBool(conf, "spice", &val, 0) < 0) + return -1; + + if (val) { + if (VIR_ALLOC(graphics) < 0) + return -1; + + graphics->type = VIR_DOMAIN_GRAPHICS_TYPE_SPICE; + if (xenConfigCopyStringOpt(conf, "spicehost", &listenAddr) < 0) + goto cleanup; + if (listenAddr && + virDomainGraphicsListenSetAddress(graphics, 0, listenAddr, + -1, true) < 0) { + goto cleanup; + } + VIR_FREE(listenAddr); + + if (xenConfigGetULong(conf, "spicetls_port", &port, 0) < 0) + goto cleanup; + graphics->data.spice.tlsPort = (int)port; + + if (xenConfigGetULong(conf, "spiceport", &port, 0) < 0) + goto cleanup; + + graphics->data.spice.port = (int)port; + + if (!graphics->data.spice.tlsPort && + !graphics->data.spice.port) + graphics->data.spice.autoport = 1; + + if (xenConfigGetBool(conf, "spicedisable_ticketing", &val, 0) < 0) + goto cleanup; + if (val) { + if (xenConfigCopyStringOpt(conf, "spicepasswd", + &graphics->data.spice.auth.passwd) < 0) + goto cleanup; + } + + if (xenConfigGetBool(conf, "spiceagent_mouse", + &graphics->data.spice.mousemode, 0) < 0) + goto cleanup; + if (xenConfigGetBool(conf, "spicedvagent", &val, 0) < 0) + goto cleanup; + if (val) { + if (xenConfigGetBool(conf, "spice_clipboard_sharing", + &graphics->data.spice.copypaste, + 0) < 0) + goto cleanup; + } + + if (VIR_ALLOC_N(def->graphics, 1) < 0) + goto cleanup; + def->graphics[0] = graphics; + def->ngraphics = 1; + } + } + + return 0; + + cleanup: + virDomainGraphicsDefFree(graphics); + return -1; +} + + +void +xenXLDiskParserError(xenXLDiskParserContext *dpc, + const char *erroneous, + const char *message) +{ + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("disk config %s not supported: %s"), + erroneous, message); + + if (!dpc->err) + dpc->err = EINVAL; +} + + +static int +xenXLDiskParserPrep(xenXLDiskParserContext *dpc, + const char *spec, + virDomainDiskDefPtr disk) +{ + int err; + + dpc->spec = spec; + dpc->disk = disk; + dpc->access_set = 0; + + err = xl_disk_lex_init_extra(dpc, &dpc->scanner); + if (err) + goto fail; + + dpc->buf = xl_disk__scan_bytes(spec, strlen(spec), dpc->scanner); + if (!dpc->buf) { + err = ENOMEM; + goto fail; + } + + return 0; + + fail: + virReportSystemError(errno, "%s", + _("failed to initialize disk configuration parser")); + return err; +} + + +static void +xenXLDiskParserCleanup(xenXLDiskParserContext *dpc) +{ + if (dpc->buf) { + xl_disk__delete_buffer(dpc->buf, dpc->scanner); + dpc->buf = NULL; + } + + if (dpc->scanner) { + xl_disk_lex_destroy(dpc->scanner); + dpc->scanner = NULL; + } +} + + +/* + * positional parameters + * (If the <diskspec> strings are not separated by "=" + * the string is split following ',' and assigned to + * the following options in the following order) + * target,format,vdev,access + * ================================================================ + * + * The parameters below cannot be specified as positional parameters: + * + * other parameters + * devtype = <devtype> + * backendtype = <backend-type> + * parameters not taken care of + * backend = <domain-name> + * script = <script> + * direct-io-safe + * + * ================================================================ + * The parser does not take any deprecated parameters + * + * For more information refer to /xen/docs/misc/xl-disk-configuration.txt + */ +static int +xenParseXLDisk(virConfPtr conf, virDomainDefPtr def) +{ + virConfValuePtr list = virConfGetValue(conf, "disk"); + xenXLDiskParserContext dpc; + virDomainDiskDefPtr disk; + + memset(&dpc, 0, sizeof(dpc)); + + if (list && list->type == VIR_CONF_LIST) { + list = list->list; + while (list) { + char *disk_spec = list->str; + const char *driver; + + if ((list->type != VIR_CONF_STRING) || (list->str == NULL)) + goto skipdisk; + + if (!(disk = virDomainDiskDefNew())) + return -1; + + disk->src->readonly = 0; + disk->src->format = VIR_STORAGE_FILE_LAST; + + if (xenXLDiskParserPrep(&dpc, disk_spec, disk)) + goto fail; + + xl_disk_lex(dpc.scanner); + + if (dpc.err) + goto fail; + + if (disk->src->format == VIR_STORAGE_FILE_LAST) + disk->src->format = VIR_STORAGE_FILE_RAW; + + if (disk->device == VIR_DOMAIN_DISK_DEVICE_CDROM) { + disk->removable = true; + disk->src->readonly = true; + if (virDomainDiskSetDriver(disk, "qemu") < 0) + goto fail; + + virDomainDiskSetType(disk, VIR_STORAGE_TYPE_FILE); + if (!disk->src->path || STREQ(disk->src->path, "")) + disk->src->format = VIR_STORAGE_FILE_NONE; + } + + if (STRPREFIX(disk->dst, "xvd") || !STREQ(def->os.type, "hvm")) + disk->bus = VIR_DOMAIN_DISK_BUS_XEN; + else if (STRPREFIX(disk->dst, "sd")) + disk->bus = VIR_DOMAIN_DISK_BUS_SCSI; + else + disk->bus = VIR_DOMAIN_DISK_BUS_IDE; + + driver = virDomainDiskGetDriver(disk); + if (!driver) { + switch (disk->src->format) { + case VIR_STORAGE_FILE_QCOW: + case VIR_STORAGE_FILE_QCOW2: + case VIR_STORAGE_FILE_VHD: + driver = "qemu"; + if (virDomainDiskSetDriver(disk, "qemu") < 0) + goto fail; + break; + default: + driver = "phy"; + if (virDomainDiskSetDriver(disk, "phy") < 0) + goto fail; + } + } + + if (STREQ(driver, "phy")) + virDomainDiskSetType(disk, VIR_STORAGE_TYPE_BLOCK); + else + virDomainDiskSetType(disk, VIR_STORAGE_TYPE_FILE); + + if (VIR_APPEND_ELEMENT(def->disks, def->ndisks, disk) < 0) + goto fail; + + skipdisk: + list = list->next; + xenXLDiskParserCleanup(&dpc); + } + } + return 0; + + fail: + xenXLDiskParserCleanup(&dpc); + virDomainDiskDefFree(disk); + return -1; +} + + +virDomainDefPtr +xenParseXL(virConfPtr conf, virCapsPtr caps, int xendConfigVersion) +{ + virDomainDefPtr def = NULL; + + if (VIR_ALLOC(def) < 0) + return NULL; + + def->virtType = VIR_DOMAIN_VIRT_XEN; + def->id = -1; + + if (xenParseConfigCommon(conf, def, caps, xendConfigVersion) < 0) + goto cleanup; + + if (xenParseXLDisk(conf, def) < 0) + goto cleanup; + + if (xenParseXLSpice(conf, def) < 0) + goto cleanup; + + return def; + + cleanup: + virDomainDefFree(def); + return NULL; +} + + +static int +xenFormatXLDisk(virConfValuePtr list, virDomainDiskDefPtr disk) +{ + virBuffer buf = VIR_BUFFER_INITIALIZER; + virConfValuePtr val, tmp; + const char *src = virDomainDiskGetSource(disk); + int format = virDomainDiskGetFormat(disk); + + /* target */ + virBufferAsprintf(&buf, "%s,", src); + /* format */ + switch (format) { + case VIR_STORAGE_FILE_RAW: + virBufferAddLit(&buf, "raw,"); + break; + case VIR_STORAGE_FILE_VHD: + virBufferAddLit(&buf, "xvhd,"); + break; + case VIR_STORAGE_FILE_QCOW: + virBufferAddLit(&buf, "qcow,"); + break; + case VIR_STORAGE_FILE_QCOW2: + virBufferAddLit(&buf, "qcow2,"); + break; + /* set default */ + default: + virBufferAddLit(&buf, "raw,"); + } + + /* device */ + virBufferAdd(&buf, disk->dst, -1); + + virBufferAddLit(&buf, ","); + + if (disk->src->readonly) + virBufferAddLit(&buf, "r,"); + else if (disk->src->shared) + virBufferAddLit(&buf, "!,"); + else + virBufferAddLit(&buf, "w,"); + if (disk->transient) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("transient disks not supported yet")); + goto cleanup; + } + + if (disk->device == VIR_DOMAIN_DISK_DEVICE_CDROM) + virBufferAddLit(&buf, "devtype=cdrom"); + + if (virBufferCheckError(&buf) < 0) + goto cleanup; + + if (VIR_ALLOC(val) < 0) + goto cleanup; + + val->type = VIR_CONF_STRING; + val->str = virBufferContentAndReset(&buf); + tmp = list->list; + while (tmp && tmp->next) + tmp = tmp->next; + if (tmp) + tmp->next = val; + else + list->list = val; + return 0; + + cleanup: + virBufferFreeAndReset(&buf); + return -1; +} + + +static int +xenFormatXLDomainDisks(virConfPtr conf, virDomainDefPtr def) +{ + virConfValuePtr diskVal = NULL; + size_t i = 0; + + if (VIR_ALLOC(diskVal) < 0) + return -1; + + diskVal->type = VIR_CONF_LIST; + diskVal->list = NULL; + + for (i = 0; i < def->ndisks; i++) { + if (def->disks[i]->device == VIR_DOMAIN_DISK_DEVICE_FLOPPY) + continue; + if (xenFormatXLDisk(diskVal, def->disks[i]) < 0) + + goto cleanup; + } + + if (diskVal->list != NULL) { + int ret = virConfSetValue(conf, "disk", diskVal); + diskVal = NULL; + if (ret < 0) + goto cleanup; + } + + return 0; + + cleanup: + virConfFreeValue(diskVal); + return 0; +} + + +static int +xenFormatXLSpice(virConfPtr conf, virDomainDefPtr def) +{ + const char *listenAddr = NULL; + + if (STREQ(def->os.type, "hvm")) { + if (def->graphics[0]->type == VIR_DOMAIN_GRAPHICS_TYPE_SPICE) { + /* set others to false but may not be necessary */ + if (xenConfigSetInt(conf, "sdl", 0) < 0) + return -1; + + if (xenConfigSetInt(conf, "vnc", 0) < 0) + return -1; + + if (xenConfigSetInt(conf, "spice", 1) < 0) + return -1; + + if (xenConfigSetInt(conf, "spiceport", + def->graphics[0]->data.spice.port) < 0) + return -1; + + if (xenConfigSetInt(conf, "spicetls_port", + def->graphics[0]->data.spice.tlsPort) < 0) + return -1; + + if (def->graphics[0]->data.spice.auth.passwd) { + if (xenConfigSetInt(conf, "spicedisable_ticketing", 1) < 0) + return -1; + + if (def->graphics[0]->data.spice.auth.passwd && + xenConfigSetString(conf, "spicepasswd", + def->graphics[0]->data.spice.auth.passwd) < 0) + return -1; + } + + listenAddr = virDomainGraphicsListenGetAddress(def->graphics[0], 0); + if (listenAddr && + xenConfigSetString(conf, "spicehost", listenAddr) < 0) + return -1; + + if (xenConfigSetInt(conf, "spicemouse_mouse", + def->graphics[0]->data.spice.mousemode) < 0) + return -1; + + if (def->graphics[0]->data.spice.copypaste) { + if (xenConfigSetInt(conf, "spicedvagent", 1) < 0) + return -1; + if (xenConfigSetInt(conf, "spice_clipboard_sharing", + def->graphics[0]->data.spice.copypaste) < 0) + return -1; + } + } + } + + return 0; +} + + +virConfPtr +xenFormatXL(virDomainDefPtr def, virConnectPtr conn, int xendConfigVersion) +{ + virConfPtr conf = NULL; + + if (!(conf = virConfNew())) + goto cleanup; + + if (xenFormatConfigCommon(conf, def, conn, xendConfigVersion) < 0) + goto cleanup; + + if (xenFormatXLDomainDisks(conf, def) < 0) + goto cleanup; + + if (xenFormatXLSpice(conf, def) < 0) + goto cleanup; + + return conf; + + cleanup: + if (conf) + virConfFree(conf); + return NULL; +} diff --git a/src/xenconfig/xen_xl.h b/src/xenconfig/xen_xl.h new file mode 100644 index 0000000..536e9b7 --- /dev/null +++ b/src/xenconfig/xen_xl.h @@ -0,0 +1,33 @@ +/* + * xen_xl.h: Xen XL parsing functions + * + * This library 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; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library 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. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library. If not, see + * <http://www.gnu.org/licenses/>. + * + * Author: Kiarie Kahurani<davidkiarie4@gmail.com> + */ + +#ifndef __VIR_XEN_XL_H__ +# define __VIR_XEN_XL_H__ + +# include "virconf.h" +# include "domain_conf.h" +# include "xen_common.h" + +virDomainDefPtr xenParseXL(virConfPtr conn, virCapsPtr caps, + int xendConfigVersion); +virConfPtr xenFormatXL(virDomainDefPtr def, + virConnectPtr, int xendConfigVersion); + +#endif /* __VIR_XEN_XL_H__ */ 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@eu.citrix.com> + * + * 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" +# include "virstring.h" +# include "domain_conf.h" +# include "xen_xl.h" +# include "xen_xl_disk_i.h" + +#define YY_NO_INPUT +#define VIR_FROM_THIS VIR_FROM_NONE + +/* Some versions of flex have a bug (Fedora bugzilla 612465) which causes + * it to fail to declare these functions, which it defines. So declare + * them ourselves. Hopefully we won't have to simultaneously support + * a flex version which declares these differently somehow. */ +int xl_disk_lexget_column(yyscan_t yyscanner); +void xl_disk_lexset_column(int column_no, yyscan_t yyscanner); + + +/*----- useful macros and functions used in actions ----- + * we use macros in the actual rules to keep the actions short + * and particularly to avoid repeating boilerplate values such as + * DPC->disk, yytext, etc. */ + +/* For actions whose patterns contain '=', finds the start of the value */ +#define FROMEQUALS (strchr(yytext,'=')+1) + +/* Chops the delimiter off, modifying yytext and yyleng. */ +#define STRIP(delim) do{ \ + if (yyleng>0 && yytext[yyleng-1]==(delim)) \ + yytext[--yyleng] = 0; \ + }while(0) + +/* Sets a string value, checking it hasn't been set already. */ +#define SAVESTRING(what,loc,val) do{ \ + savestring(DPC, what " respecified", &DPC->disk->loc, (val)); \ + }while(0) + + +static void +savestring(xenXLDiskParserContext *dpc, + const char *what_respecified, + char **update, + const char *value) +{ + if (*update) { + if (**update) { + xenXLDiskParserError(dpc, value, what_respecified); + return; + } + + VIR_FREE(*update); /* do not complain about overwriting empty strings */ + } + + ignore_value(VIR_STRDUP(*update, value)); +} + +#define DPC dpc /* our convention in lexer helper functions */ + +/* Sets ->readwrite from the string. */ +static void +setaccess(xenXLDiskParserContext *dpc, const char *str) +{ + if (STREQ(str, "rw") || STREQ(str, "w")) { + dpc->disk->src->readonly = 0; + } else if (STREQ(str, "r") || STREQ(str, "ro")) { + dpc->disk->src->readonly = 1; + } else if (STREQ(str, "w!") || STREQ(str, "!")) { + dpc->disk->src->readonly = 0; + dpc->disk->src->shared = 1; + } else { + xenXLDiskParserError(dpc, str, "unknown value for access"); + } + dpc->access_set = 1; +} + +/* Sets ->format from the string. IDL should provide something for this. */ +static void +setformat(xenXLDiskParserContext *dpc, const char *str) +{ + if (STREQ(str, "") || STREQ(str, "raw")) + virDomainDiskSetFormat(dpc->disk, VIR_STORAGE_FILE_RAW); + else if (STREQ(str, "qcow")) + virDomainDiskSetFormat(dpc->disk, VIR_STORAGE_FILE_QCOW); + else if (STREQ(str, "qcow2")) + virDomainDiskSetFormat(dpc->disk, VIR_STORAGE_FILE_QCOW2); + else if (STREQ(str, "vhd")) + virDomainDiskSetFormat(dpc->disk, VIR_STORAGE_FILE_VHD); + else + xenXLDiskParserError(dpc, str, "unknown value for format"); +} + + +/* Sets ->backend from the string. IDL should provide something for this. */ +static void +setdrivertype(xenXLDiskParserContext *dpc, const char *str) +{ + if (STREQ(str, "phy")) + ignore_value(virDomainDiskSetDriver(dpc->disk, "phy")); + else if (STREQ(str, "tap")) + ignore_value(virDomainDiskSetDriver(dpc->disk, "tap")); + else if (STREQ(str, "file") || STREQ(str, "")) + ignore_value(virDomainDiskSetDriver(dpc->disk, "qemu")); + else + xenXLDiskParserError(dpc, str, "unknown value for backendtype"); +} + + +/* Handles a vdev positional parameter which includes a devtype. */ +static int +vdev_and_devtype(xenXLDiskParserContext *dpc, char *str) +{ + /* returns 1 if it was <vdev>:<devtype>, 0 (doing nothing) otherwise */ + char *colon = strrchr(str, ':'); + if (!colon) + return 0; + + *colon++ = 0; + SAVESTRING("vdev", dst, str); + + if (STREQ(colon,"cdrom")) { + DPC->disk->device = VIR_DOMAIN_DISK_DEVICE_CDROM; + } else if (STREQ(colon, "disk")) { + DPC->disk->device = VIR_DOMAIN_DISK_DEVICE_DISK; + } else { + xenXLDiskParserError(DPC, colon, "unknown deprecated type"); + } + return 1; +} + +#undef DPC /* needs to be defined differently the actual lexer */ +#define DPC ((xenXLDiskParserContext*)yyextra) + +%} + +%option warn +%option nodefault +%option batch +%option 8bit +%option noyywrap +%option reentrant +%option nounput + +%x LEXERR + +%% + + /*----- the scanner rules which do the parsing -----*/ + +[ \t\n]+/([^ \t\n].*)? { /* ignore whitespace before parameters */ } + + /* ordinary parameters setting enums or strings */ + +format=[^,]*,? { STRIP(','); setformat(DPC, FROMEQUALS); } + +cdrom,? { DPC->disk->device = VIR_DOMAIN_DISK_DEVICE_CDROM; } +devtype=cdrom,? { DPC->disk->device = VIR_DOMAIN_DISK_DEVICE_CDROM; } +devtype=disk,? { DPC->disk->device = VIR_DOMAIN_DISK_DEVICE_DISK; } +devtype=[^,]*,? { xenXLDiskParserError(DPC, yytext,"unknown value for type"); } + +access=[^,]*,? { STRIP(','); setaccess(DPC, FROMEQUALS); } +backendtype=[^,]*,? { STRIP(','); setdrivertype(DPC, FROMEQUALS); } + +vdev=[^,]*,? { STRIP(','); SAVESTRING("vdev", dst, FROMEQUALS); } + + /* the target magic parameter, eats the rest of the string */ + +target=.* { STRIP(','); SAVESTRING("target", src->path, FROMEQUALS); } + + /* unknown parameters */ + +[a-z][-a-z0-9]*=[^,],? { xenXLDiskParserError(DPC, yytext, "unknown parameter"); } + + /* the "/.*" in these patterns ensures that they count as if they + * matched the whole string, so these patterns take precedence */ + +(raw|qcow2?|vhd):/.* { + STRIP(':'); + DPC->had_depr_prefix=1; + setformat(DPC, yytext); + } + +tapdisk:/.* { DPC->had_depr_prefix=1; } +tap2?:/.* { DPC->had_depr_prefix=1; } +aio:/.* { DPC->had_depr_prefix=1; } +ioemu:/.* { DPC->had_depr_prefix=1; } +file:/.* { DPC->had_depr_prefix=1; } +phy:/.* { DPC->had_depr_prefix=1; } +[a-z][a-z0-9]*:/([^a-z0-9].*)? { + xenXLDiskParserError(DPC, yytext, "unknown deprecated disk prefix"); + return 0; + } + + /* positional parameters */ + +[^=,]*,|[^=,]+,? { + STRIP(','); + + if (DPC->err) { + /* previous errors may just lead to subsequent ones */ + } else if (!DPC->disk->src->path) { + SAVESTRING("target", src->path, yytext); + } else if (DPC->disk->src->format == VIR_STORAGE_FILE_LAST){ + setformat(DPC, yytext); + } + else if (!DPC->disk->dst) { + if (!vdev_and_devtype(DPC, yytext)) + SAVESTRING("vdev", dst, yytext); + } else if (!DPC->access_set) { + DPC->access_set = 1; + setaccess(DPC, yytext); + } else { + xenXLDiskParserError(DPC, yytext, "too many positional parameters"); + return 0; /* don't print any more errors */ + } +} + +. { + BEGIN(LEXERR); + yymore(); +} +<LEXERR>.* { + xenXLDiskParserError(DPC, yytext, "bad disk syntax"); + return 0; +} diff --git a/src/xenconfig/xen_xl_disk_i.h b/src/xenconfig/xen_xl_disk_i.h new file mode 100644 index 0000000..063dedf --- /dev/null +++ b/src/xenconfig/xen_xl_disk_i.h @@ -0,0 +1,39 @@ +/* + * xen_xl_disk_i.h - common header for disk spec parser + * + * Copyright (C) 2011 Citrix Ltd. + * Author Ian Jackson <ian.jackson@eu.citrix.com> + * + * 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. + */ + +#ifndef __VIR_XEN_XL_DISK_I_H__ +# define __VIR_XEN_XL_DISK_I_H__ + +# include "virconf.h" +# include "domain_conf.h" + + +typedef struct { + int err; + void *scanner; + YY_BUFFER_STATE buf; + virDomainDiskDefPtr disk; + int access_set; + int had_depr_prefix; + const char *spec; +} xenXLDiskParserContext; + +void xenXLDiskParserError(xenXLDiskParserContext *dpc, + const char *erroneous, + const char *message); + +#endif /* __VIR_XEN_XL_DISK_I_H__ */ -- 1.8.4.5

On 16.12.2014 05:30, Jim Fehlig wrote:
From: Kiarie Kahurani <davidkiarie4@gmail.com>
Introduce a Xen xl parser
This parser allows for users to convert the new xl disk format and spice graphics config to libvirt xml format and vice versa. Regarding the spice graphics config, the code is pretty much straight forward. For the disk {formating, parsing}, this parser takes care of the new xl format which include positional parameters and key/value parameters. In xl format disk config a <diskspec> consists of parameters separated by commas. If the parameters do not contain an '=' they are automatically assigned to certain options following the order below
target, format, vdev, access
The above are the only mandatory parameters in the <diskspec> but there are many more disk config options. These options can be specified as key=value pairs. This takes care of the rest of the options such as
devtype, backend, backendtype, script, direct-io-safe,
The positional paramters can also be specified in key/value form for example
/dev/vg/guest-volume,,hda /dev/vg/guest-volume,raw,hda,rw format=raw, vdev=hda, access=rw, target=/dev/vg/guest-volume
are interpleted to one config.
In xm format, the above diskspec would be written as
phy:/dev/vg/guest-volume,hda,w
The disk parser is based on the same parser used successfully by the Xen project for several years now. Ian Jackson authored the scanner, which is used by this commit with mimimal changes. Only the PREFIX option is changed, to produce function and file names more consistent with libvirt's convention.
Signed-off-by: Kiarie Kahurani <davidkiarie4@gmail.com> Signed-off-by: Jim Fehlig <jfehlig@suse.com> --- .gitignore | 1 + cfg.mk | 3 +- configure.ac | 1 + po/POTFILES.in | 1 + src/Makefile.am | 25 ++- src/libvirt_xenconfig.syms | 4 + src/xenconfig/xen_common.c | 3 +- src/xenconfig/xen_xl.c | 499 ++++++++++++++++++++++++++++++++++++++++++ src/xenconfig/xen_xl.h | 33 +++ src/xenconfig/xen_xl_disk.l | 256 ++++++++++++++++++++++ src/xenconfig/xen_xl_disk_i.h | 39 ++++ 11 files changed, 861 insertions(+), 4 deletions(-)
diff --git a/.gitignore b/.gitignore index 9d09709..eac2203 100644 --- a/.gitignore +++ b/.gitignore @@ -140,6 +140,7 @@ /src/remote/*_protocol.[ch] /src/rpc/virkeepaliveprotocol.[ch] /src/rpc/virnetprotocol.[ch] +/src/xenconfig/xen_xl_disk.[ch] /src/test_libvirt*.aug /src/test_virtlockd.aug /src/util/virkeymaps.h diff --git a/cfg.mk b/cfg.mk index 21f83c3..3df3dcb 100644 --- a/cfg.mk +++ b/cfg.mk @@ -89,8 +89,9 @@ distdir: sc_vulnerable_makefile_CVE-2012-3386.z endif
# Files that should never cause syntax check failures. +# (^(HACKING|docs/(news\.html\.in|.*\.patch))|\.(po|fig|gif|ico|png))$$ VC_LIST_ALWAYS_EXCLUDE_REGEX = \ - (^(HACKING|docs/(news\.html\.in|.*\.patch))|\.(po|fig|gif|ico|png))$$ + (^(HACKING|docs/(news\.html\.in|.*\.patch)|src/xenconfig/xen_xl_disk.[chl])|\.(po|fig|gif|ico|png))$$
# Functions like free() that are no-ops on NULL arguments. useless_free_options = \ diff --git a/configure.ac b/configure.ac index 9fd44b2..777367e 100644 --- a/configure.ac +++ b/configure.ac @@ -146,6 +146,7 @@ m4_ifndef([LT_INIT], [ ]) AM_PROG_CC_C_O AM_PROG_LD +AM_PROG_LEX
AC_MSG_CHECKING([for how to mark DSO non-deletable at runtime]) LIBVIRT_NODELETE= diff --git a/po/POTFILES.in b/po/POTFILES.in index e7cb2cc..094c8e3 100644 --- a/po/POTFILES.in +++ b/po/POTFILES.in @@ -247,6 +247,7 @@ src/xenapi/xenapi_driver.c src/xenapi/xenapi_utils.c src/xenconfig/xen_common.c src/xenconfig/xen_sxpr.c +src/xenconfig/xen_xl.c src/xenconfig/xen_xm.c tests/virpolkittest.c tools/libvirt-guests.sh.in diff --git a/src/Makefile.am b/src/Makefile.am index b6c1701..23c433d 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -999,11 +999,22 @@ CPU_SOURCES = \ VMX_SOURCES = \ vmx/vmx.c vmx/vmx.h
+AM_LFLAGS = -Pxl_disk_ --header-file=../$*.h +LEX_OUTPUT_ROOT = lex.xl_disk_ +BUILT_SOURCES += xenconfig/xen_xl_disk.c xenconfig/xen_xl_disk.h +# Generated header file is not implicitly added to dist +EXTRA_DIST += xenconfig/xen_xl_disk.h +CLEANFILES += xenconfig/xen_xl_disk.h xenconfig/xen_xl_disk.c + +XENXLDISKPARSER_SOURCES = xenconfig/xen_xl_disk.l + XENCONFIG_SOURCES = \ xenconfig/xenxs_private.h \ - xenconfig/xen_common.c xenconfig/xen_common.h \ + xenconfig/xen_common.c xenconfig/xen_common.h \ xenconfig/xen_sxpr.c xenconfig/xen_sxpr.h \ - xenconfig/xen_xm.c xenconfig/xen_xm.h + xenconfig/xen_xm.c xenconfig/xen_xm.h \ + xenconfig/xen_xl.c xenconfig/xen_xl.h \ + xenconfig/xen_xl_disk_i.h
pkgdata_DATA = cpu/cpu_map.xml
@@ -1058,10 +1069,19 @@ libvirt_vmx_la_SOURCES = $(VMX_SOURCES) endif WITH_VMX
if WITH_XENCONFIG +# Flex generated XL disk parser needs to be compiled without WARN_FLAGS +# 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 +libvirt_xenxldiskparser_la_SOURCES = \ + $(XENXLDISKPARSER_SOURCES) + noinst_LTLIBRARIES += libvirt_xenconfig.la libvirt_la_BUILT_LIBADD += libvirt_xenconfig.la libvirt_xenconfig_la_CFLAGS = \ -I$(top_srcdir)/src/conf $(AM_CFLAGS) +libvirt_xenconfig_la_LIBADD = libvirt_xenxldiskparser.la libvirt_xenconfig_la_SOURCES = $(XENCONFIG_SOURCES) endif WITH_XENCONFIG
@@ -1823,6 +1843,7 @@ EXTRA_DIST += \ $(VBOX_DRIVER_EXTRA_DIST) \ $(VMWARE_DRIVER_SOURCES) \ $(XENCONFIG_SOURCES) \ + $(XENXLDISKPARSER_SOURCES) \ $(ACCESS_DRIVER_POLKIT_POLICY)
check-local: check-augeas diff --git a/src/libvirt_xenconfig.syms b/src/libvirt_xenconfig.syms index 6541685..3e2e5d6 100644 --- a/src/libvirt_xenconfig.syms +++ b/src/libvirt_xenconfig.syms @@ -16,6 +16,10 @@ xenParseSxprChar; xenParseSxprSound; xenParseSxprString;
+#xenconfig/xen_xl.h +xenFormatXL; +xenParseXL; + # xenconfig/xen_xm.h xenFormatXM; xenParseXM; diff --git a/src/xenconfig/xen_common.c b/src/xenconfig/xen_common.c index 8ff10a0..b94b5db 100644 --- a/src/xenconfig/xen_common.c +++ b/src/xenconfig/xen_common.c @@ -1801,7 +1801,8 @@ xenFormatVfb(virConfPtr conf, virDomainDefPtr def, int xendConfigVersion) { int hvm = STREQ(def->os.type, "hvm") ? 1 : 0;
- if (def->ngraphics == 1) { + if (def->ngraphics == 1 && + def->graphics[0]->type != VIR_DOMAIN_GRAPHICS_TYPE_SPICE) { if (hvm || (xendConfigVersion < XEND_CONFIG_MIN_VERS_PVFB_NEWCONF)) { if (def->graphics[0]->type == VIR_DOMAIN_GRAPHICS_TYPE_SDL) { if (xenConfigSetInt(conf, "sdl", 1) < 0) diff --git a/src/xenconfig/xen_xl.c b/src/xenconfig/xen_xl.c new file mode 100644 index 0000000..8d1d2a7 --- /dev/null +++ b/src/xenconfig/xen_xl.c @@ -0,0 +1,499 @@ +/* + * xen_xl.c: Xen XL parsing functions + * + * This library 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; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library 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. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library. If not, see + * <http://www.gnu.org/licenses/>. + * + * Author: Kiarie Kahurani <davidkiarie4@gmail.com> + */ + +#include <config.h> + +#include "virconf.h" +#include "virerror.h" +#include "domain_conf.h" +#include "viralloc.h" +#include "virstring.h" +#include "xen_xl.h" +#include "xen_xl_disk.h" +#include "xen_xl_disk_i.h" + +#define VIR_FROM_THIS VIR_FROM_NONE + + +static int +xenParseXLSpice(virConfPtr conf, virDomainDefPtr def) +{ + virDomainGraphicsDefPtr graphics = NULL; + unsigned long port; + char *listenAddr = NULL; + int val; + + if (STREQ(def->os.type, "hvm")) { + if (xenConfigGetBool(conf, "spice", &val, 0) < 0) + return -1; + + if (val) { + if (VIR_ALLOC(graphics) < 0) + return -1; + + graphics->type = VIR_DOMAIN_GRAPHICS_TYPE_SPICE; + if (xenConfigCopyStringOpt(conf, "spicehost", &listenAddr) < 0) + goto cleanup; + if (listenAddr && + virDomainGraphicsListenSetAddress(graphics, 0, listenAddr, + -1, true) < 0) { + goto cleanup; + } + VIR_FREE(listenAddr); + + if (xenConfigGetULong(conf, "spicetls_port", &port, 0) < 0) + goto cleanup; + graphics->data.spice.tlsPort = (int)port; + + if (xenConfigGetULong(conf, "spiceport", &port, 0) < 0) + goto cleanup; + + graphics->data.spice.port = (int)port; + + if (!graphics->data.spice.tlsPort && + !graphics->data.spice.port) + graphics->data.spice.autoport = 1; + + if (xenConfigGetBool(conf, "spicedisable_ticketing", &val, 0) < 0) + goto cleanup; + if (val) { + if (xenConfigCopyStringOpt(conf, "spicepasswd", + &graphics->data.spice.auth.passwd) < 0) + goto cleanup; + } + + if (xenConfigGetBool(conf, "spiceagent_mouse", + &graphics->data.spice.mousemode, 0) < 0) + goto cleanup; + if (xenConfigGetBool(conf, "spicedvagent", &val, 0) < 0) + goto cleanup; + if (val) { + if (xenConfigGetBool(conf, "spice_clipboard_sharing", + &graphics->data.spice.copypaste, + 0) < 0) + goto cleanup; + } + + if (VIR_ALLOC_N(def->graphics, 1) < 0) + goto cleanup; + def->graphics[0] = graphics; + def->ngraphics = 1; + } + } + + return 0; + + cleanup: + virDomainGraphicsDefFree(graphics); + return -1; +} + + +void +xenXLDiskParserError(xenXLDiskParserContext *dpc, + const char *erroneous, + const char *message) +{ + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("disk config %s not supported: %s"), + erroneous, message); + + if (!dpc->err) + dpc->err = EINVAL; +} + + +static int +xenXLDiskParserPrep(xenXLDiskParserContext *dpc, + const char *spec, + virDomainDiskDefPtr disk) +{ + int err; + + dpc->spec = spec; + dpc->disk = disk; + dpc->access_set = 0; + + err = xl_disk_lex_init_extra(dpc, &dpc->scanner); + if (err) + goto fail; + + dpc->buf = xl_disk__scan_bytes(spec, strlen(spec), dpc->scanner); + if (!dpc->buf) { + err = ENOMEM; + goto fail; + } + + return 0; + + fail: + virReportSystemError(errno, "%s", + _("failed to initialize disk configuration parser")); + return err; +} + + +static void +xenXLDiskParserCleanup(xenXLDiskParserContext *dpc) +{ + if (dpc->buf) { + xl_disk__delete_buffer(dpc->buf, dpc->scanner); + dpc->buf = NULL; + } + + if (dpc->scanner) { + xl_disk_lex_destroy(dpc->scanner); + dpc->scanner = NULL; + } +} + + +/* + * positional parameters + * (If the <diskspec> strings are not separated by "=" + * the string is split following ',' and assigned to + * the following options in the following order) + * target,format,vdev,access + * ================================================================ + * + * The parameters below cannot be specified as positional parameters: + * + * other parameters + * devtype = <devtype> + * backendtype = <backend-type> + * parameters not taken care of + * backend = <domain-name> + * script = <script> + * direct-io-safe + * + * ================================================================ + * The parser does not take any deprecated parameters + * + * For more information refer to /xen/docs/misc/xl-disk-configuration.txt + */ +static int +xenParseXLDisk(virConfPtr conf, virDomainDefPtr def) +{ + virConfValuePtr list = virConfGetValue(conf, "disk"); + xenXLDiskParserContext dpc; + virDomainDiskDefPtr disk; + + memset(&dpc, 0, sizeof(dpc)); + + if (list && list->type == VIR_CONF_LIST) { + list = list->list; + while (list) { + char *disk_spec = list->str; + const char *driver; + + if ((list->type != VIR_CONF_STRING) || (list->str == NULL)) + goto skipdisk; + + if (!(disk = virDomainDiskDefNew())) + return -1; + + disk->src->readonly = 0; + disk->src->format = VIR_STORAGE_FILE_LAST; + + if (xenXLDiskParserPrep(&dpc, disk_spec, disk)) + goto fail; + + xl_disk_lex(dpc.scanner); + + if (dpc.err) + goto fail; + + if (disk->src->format == VIR_STORAGE_FILE_LAST) + disk->src->format = VIR_STORAGE_FILE_RAW; + + if (disk->device == VIR_DOMAIN_DISK_DEVICE_CDROM) { + disk->removable = true; + disk->src->readonly = true; + if (virDomainDiskSetDriver(disk, "qemu") < 0) + goto fail; + + virDomainDiskSetType(disk, VIR_STORAGE_TYPE_FILE); + if (!disk->src->path || STREQ(disk->src->path, "")) + disk->src->format = VIR_STORAGE_FILE_NONE; + } + + if (STRPREFIX(disk->dst, "xvd") || !STREQ(def->os.type, "hvm")) + disk->bus = VIR_DOMAIN_DISK_BUS_XEN; + else if (STRPREFIX(disk->dst, "sd")) + disk->bus = VIR_DOMAIN_DISK_BUS_SCSI; + else + disk->bus = VIR_DOMAIN_DISK_BUS_IDE; + + driver = virDomainDiskGetDriver(disk); + if (!driver) { + switch (disk->src->format) { + case VIR_STORAGE_FILE_QCOW: + case VIR_STORAGE_FILE_QCOW2: + case VIR_STORAGE_FILE_VHD: + driver = "qemu"; + if (virDomainDiskSetDriver(disk, "qemu") < 0) + goto fail; + break; + default: + driver = "phy"; + if (virDomainDiskSetDriver(disk, "phy") < 0) + goto fail; + } + } + + if (STREQ(driver, "phy")) + virDomainDiskSetType(disk, VIR_STORAGE_TYPE_BLOCK); + else + virDomainDiskSetType(disk, VIR_STORAGE_TYPE_FILE); + + if (VIR_APPEND_ELEMENT(def->disks, def->ndisks, disk) < 0) + goto fail; + + skipdisk: + list = list->next; + xenXLDiskParserCleanup(&dpc); + } + } + return 0; + + fail: + xenXLDiskParserCleanup(&dpc); + virDomainDiskDefFree(disk); + return -1; +} + + +virDomainDefPtr +xenParseXL(virConfPtr conf, virCapsPtr caps, int xendConfigVersion) +{ + virDomainDefPtr def = NULL; + + if (VIR_ALLOC(def) < 0) + return NULL; + + def->virtType = VIR_DOMAIN_VIRT_XEN; + def->id = -1; + + if (xenParseConfigCommon(conf, def, caps, xendConfigVersion) < 0) + goto cleanup; + + if (xenParseXLDisk(conf, def) < 0) + goto cleanup; + + if (xenParseXLSpice(conf, def) < 0) + goto cleanup; + + return def; + + cleanup: + virDomainDefFree(def); + return NULL; +} + + +static int +xenFormatXLDisk(virConfValuePtr list, virDomainDiskDefPtr disk) +{ + virBuffer buf = VIR_BUFFER_INITIALIZER; + virConfValuePtr val, tmp; + const char *src = virDomainDiskGetSource(disk); + int format = virDomainDiskGetFormat(disk); + + /* target */ + virBufferAsprintf(&buf, "%s,", src); + /* format */ + switch (format) { + case VIR_STORAGE_FILE_RAW: + virBufferAddLit(&buf, "raw,"); + break; + case VIR_STORAGE_FILE_VHD: + virBufferAddLit(&buf, "xvhd,"); + break; + case VIR_STORAGE_FILE_QCOW: + virBufferAddLit(&buf, "qcow,"); + break; + case VIR_STORAGE_FILE_QCOW2: + virBufferAddLit(&buf, "qcow2,"); + break; + /* set default */ + default: + virBufferAddLit(&buf, "raw,"); + } + + /* device */ + virBufferAdd(&buf, disk->dst, -1); + + virBufferAddLit(&buf, ","); + + if (disk->src->readonly) + virBufferAddLit(&buf, "r,"); + else if (disk->src->shared) + virBufferAddLit(&buf, "!,"); + else + virBufferAddLit(&buf, "w,"); + if (disk->transient) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("transient disks not supported yet")); + goto cleanup; + } + + if (disk->device == VIR_DOMAIN_DISK_DEVICE_CDROM) + virBufferAddLit(&buf, "devtype=cdrom"); + + if (virBufferCheckError(&buf) < 0) + goto cleanup; + + if (VIR_ALLOC(val) < 0) + goto cleanup; + + val->type = VIR_CONF_STRING; + val->str = virBufferContentAndReset(&buf); + tmp = list->list; + while (tmp && tmp->next) + tmp = tmp->next; + if (tmp) + tmp->next = val; + else + list->list = val; + return 0; + + cleanup: + virBufferFreeAndReset(&buf); + return -1; +} + + +static int +xenFormatXLDomainDisks(virConfPtr conf, virDomainDefPtr def) +{ + virConfValuePtr diskVal = NULL; + size_t i = 0; + + if (VIR_ALLOC(diskVal) < 0) + return -1; + + diskVal->type = VIR_CONF_LIST; + diskVal->list = NULL; + + for (i = 0; i < def->ndisks; i++) { + if (def->disks[i]->device == VIR_DOMAIN_DISK_DEVICE_FLOPPY) + continue; + if (xenFormatXLDisk(diskVal, def->disks[i]) < 0) + + goto cleanup; + } + + if (diskVal->list != NULL) { + int ret = virConfSetValue(conf, "disk", diskVal); + diskVal = NULL; + if (ret < 0) + goto cleanup; + } + + return 0; + + cleanup: + virConfFreeValue(diskVal); + return 0; +} + + +static int +xenFormatXLSpice(virConfPtr conf, virDomainDefPtr def) +{ + const char *listenAddr = NULL; + + if (STREQ(def->os.type, "hvm")) { + if (def->graphics[0]->type == VIR_DOMAIN_GRAPHICS_TYPE_SPICE) { + /* set others to false but may not be necessary */ + if (xenConfigSetInt(conf, "sdl", 0) < 0) + return -1; + + if (xenConfigSetInt(conf, "vnc", 0) < 0) + return -1; + + if (xenConfigSetInt(conf, "spice", 1) < 0) + return -1; + + if (xenConfigSetInt(conf, "spiceport", + def->graphics[0]->data.spice.port) < 0) + return -1; + + if (xenConfigSetInt(conf, "spicetls_port", + def->graphics[0]->data.spice.tlsPort) < 0) + return -1; + + if (def->graphics[0]->data.spice.auth.passwd) { + if (xenConfigSetInt(conf, "spicedisable_ticketing", 1) < 0) + return -1; + + if (def->graphics[0]->data.spice.auth.passwd && + xenConfigSetString(conf, "spicepasswd", + def->graphics[0]->data.spice.auth.passwd) < 0) + return -1; + } + + listenAddr = virDomainGraphicsListenGetAddress(def->graphics[0], 0); + if (listenAddr && + xenConfigSetString(conf, "spicehost", listenAddr) < 0) + return -1; + + if (xenConfigSetInt(conf, "spicemouse_mouse", + def->graphics[0]->data.spice.mousemode) < 0) + return -1; + + if (def->graphics[0]->data.spice.copypaste) { + if (xenConfigSetInt(conf, "spicedvagent", 1) < 0) + return -1; + if (xenConfigSetInt(conf, "spice_clipboard_sharing", + def->graphics[0]->data.spice.copypaste) < 0) + return -1; + } + } + } + + return 0; +} + + +virConfPtr +xenFormatXL(virDomainDefPtr def, virConnectPtr conn, int xendConfigVersion) +{ + virConfPtr conf = NULL; + + if (!(conf = virConfNew())) + goto cleanup; + + if (xenFormatConfigCommon(conf, def, conn, xendConfigVersion) < 0) + goto cleanup; + + if (xenFormatXLDomainDisks(conf, def) < 0) + goto cleanup; + + if (xenFormatXLSpice(conf, def) < 0) + goto cleanup; + + return conf; + + cleanup: + if (conf) + virConfFree(conf); + return NULL; +} diff --git a/src/xenconfig/xen_xl.h b/src/xenconfig/xen_xl.h new file mode 100644 index 0000000..536e9b7 --- /dev/null +++ b/src/xenconfig/xen_xl.h @@ -0,0 +1,33 @@ +/* + * xen_xl.h: Xen XL parsing functions + * + * This library 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; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library 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. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library. If not, see + * <http://www.gnu.org/licenses/>. + * + * Author: Kiarie Kahurani<davidkiarie4@gmail.com> + */ + +#ifndef __VIR_XEN_XL_H__ +# define __VIR_XEN_XL_H__ + +# include "virconf.h" +# include "domain_conf.h" +# include "xen_common.h" + +virDomainDefPtr xenParseXL(virConfPtr conn, virCapsPtr caps, + int xendConfigVersion); +virConfPtr xenFormatXL(virDomainDefPtr def, + virConnectPtr, int xendConfigVersion); + +#endif /* __VIR_XEN_XL_H__ */ 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@eu.citrix.com> + * + * 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 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. Michal

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@eu.citrix.com> + * + * 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

On Sun, Jan 4, 2015 at 9:00 PM, Jim Fehlig <jfehlig@suse.com> 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@eu.citrix.com> + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU Lesser General Public License as
Michal Privoznik wrote: 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 :-).
Well, that was close - close is fun :D
Regards, Jim

On 12/15/2014 11:30 PM, Jim Fehlig wrote:
From: Kiarie Kahurani <davidkiarie4@gmail.com>
Introduce a Xen xl parser
This parser allows for users to convert the new xl disk format and spice graphics config to libvirt xml format and vice versa. Regarding the spice graphics config, the code is pretty much straight forward. For the disk {formating, parsing}, this parser takes care of the new xl format which include positional parameters and key/value parameters. In xl format disk config a <diskspec> consists of parameters separated by commas. If the parameters do not contain an '=' they are automatically assigned to certain options following the order below
target, format, vdev, access
The above are the only mandatory parameters in the <diskspec> but there are many more disk config options. These options can be specified as key=value pairs. This takes care of the rest of the options such as
devtype, backend, backendtype, script, direct-io-safe,
The positional paramters can also be specified in key/value form for example
/dev/vg/guest-volume,,hda /dev/vg/guest-volume,raw,hda,rw format=raw, vdev=hda, access=rw, target=/dev/vg/guest-volume
are interpleted to one config.
In xm format, the above diskspec would be written as
phy:/dev/vg/guest-volume,hda,w
The disk parser is based on the same parser used successfully by the Xen project for several years now. Ian Jackson authored the scanner, which is used by this commit with mimimal changes. Only the PREFIX option is changed, to produce function and file names more consistent with libvirt's convention.
Signed-off-by: Kiarie Kahurani <davidkiarie4@gmail.com> Signed-off-by: Jim Fehlig <jfehlig@suse.com> --- .gitignore | 1 + cfg.mk | 3 +- configure.ac | 1 + po/POTFILES.in | 1 + src/Makefile.am | 25 ++- src/libvirt_xenconfig.syms | 4 + src/xenconfig/xen_common.c | 3 +- src/xenconfig/xen_xl.c | 499 ++++++++++++++++++++++++++++++++++++++++++ src/xenconfig/xen_xl.h | 33 +++ src/xenconfig/xen_xl_disk.l | 256 ++++++++++++++++++++++ src/xenconfig/xen_xl_disk_i.h | 39 ++++ 11 files changed, 861 insertions(+), 4 deletions(-)
In addition to the build issues - it seems the generated "xen_xl_disk.c" has numerous issues found by Coverity. Since it's not clear to me how this is all put together - I'll cut-n-paste from my Coverity output and provide some basic analysis - hope it all makes sense... Coverity Error: FORWARD_NULL (4 times - same root cause) Routines: xl_disk_restart, xl_disk__switch_to_buffer, xl_disk_push_buffer_state xl_disk_lex NOTE: While the generated .c file indicates YY_CURRENT_BUFFER_LVALUE is the same as YY_CURRENT_BUFFER, but useful when we know the buffer stack is not NULL or when we need an lvalue. As it turns out the yy_set_interactive and yy_set_bol macros will check if ( ! YY_CURRENT_BUFFER ), but then follow that up with a YY_CURRENT_BUFFER_LVALUE access both within the if statement and after the if statement. So it seems the YY_CURRENT_BUFFER_LVALUE needs a similar yyg->yy_buffer_stack check to prevent a possible bad access. The following output is just from xl_disk_restart, but is similar to others... 1782 */ 1783 void xl_disk_restart (FILE * input_file , yyscan_t yyscanner) 1784 { 1785 struct yyguts_t * yyg = (struct yyguts_t*)yyscanner; 1786 (1) Event cond_false: Condition "yyg->yy_buffer_stack", taking false branch (2) Event var_compare_op: Comparing "yyg->yy_buffer_stack" to null implies that "yyg->yy_buffer_stack" might be null. (3) Event cond_true: Condition "!(yyg->yy_buffer_stack ? yyg->yy_buffer_stack[yyg->yy_buffer_stack_top] : NULL)", taking true branch Also see events: [var_deref_op] 1787 if ( ! YY_CURRENT_BUFFER ){ 1788 xl_disk_ensure_buffer_stack (yyscanner); (4) Event var_deref_op: Dereferencing null pointer "yyg->yy_buffer_stack". Also see events: [var_compare_op] 1789 YY_CURRENT_BUFFER_LVALUE = 1790 xl_disk__create_buffer(yyin,YY_BUF_SIZE ,yyscanner); 1791 } Coverity Error: NESTING_INDENT_MISMATCH Routine: xl_disk_lex NOTE: Based on what I see, it seems the if statement at line 1118 needs an open parenthesis "{" followed by a close parenthesis at line 1140 1105 /*----- the scanner rules which do the parsing -----*/ 1106 1107 #line 1108 "xenconfig/xen_xl_disk.c" 1108 1109 if ( !yyg->yy_init ) 1110 { 1111 yyg->yy_init = 1; 1112 1113 #ifdef YY_USER_INIT 1114 YY_USER_INIT; 1115 #endif 1116 1117 /* Create the reject buffer large enough to save one state per allowed character. */ (1) Event parent: This 'if' statement is the parent, indented to column 9. Also see events: [nephew][uncle] 1118 if ( ! yyg->yy_state_buf ) (2) Event nephew: This statement is nested within its parent, indented to column 13. Also see events: [parent][uncle] 1119 yyg->yy_state_buf = (yy_state_type *)xl_disk_alloc(YY_STATE_BUF_SIZE ,yyscanner); (3) Event uncle: This 'if' statement is indented to column 13, as if it were nested within the preceding parent statement, but it is not. Also see events: [parent][nephew] 1120 if ( ! yyg->yy_state_buf ) 1121 YY_FATAL_ERROR( "out of dynamic memory in xl_disk_lex()" ); 1122 1123 if ( ! yyg->yy_start ) 1124 yyg->yy_start = 1; /* first start state */ ... 1137 1138 xl_disk__load_buffer_state(yyscanner ); 1139 } 1140 1141 while ( 1 ) /* loops until end-of-file is reached */ Coverity Error: REVERSE_INULL Routine: xl_disk_pop_buffer_state NOTE: Similar to the first issue, except that in this case we have the check/return at lines 1989 and 1990, but then later on we check it again at line 1997, which causes the (2) event below. 1986 void xl_disk_pop_buffer_state (yyscan_t yyscanner) 1987 { 1988 struct yyguts_t * yyg = (struct yyguts_t*)yyscanner; 1989 if (!YY_CURRENT_BUFFER) 1990 return; 1991 1992 xl_disk__delete_buffer(YY_CURRENT_BUFFER ,yyscanner); (1) Event deref_ptr: Directly dereferencing pointer "yyg->yy_buffer_stack". Also see events: [check_after_deref] 1993 YY_CURRENT_BUFFER_LVALUE = NULL; 1994 if (yyg->yy_buffer_stack_top > 0) 1995 --yyg->yy_buffer_stack_top; 1996 (2) Event check_after_deref: Null-checking "yyg->yy_buffer_stack" suggests that it may be null, but it has already been dereferenced on all paths leading to the check. Also see events: [deref_ptr] 1997 if (YY_CURRENT_BUFFER) { 1998 xl_disk__load_buffer_state(yyscanner ); Coverity Error: UNUSED_VALUE Routine: xl_disk_lex NOTE: This one I'm not sure about - usually it's the unconditional setting at (1) that ends up being the issue because a setting at (2) does it for something specific. But in this case, I'm not sure what the exact complaint is. 1179 1180 yy_find_action: (1) Event value_overwrite: Value from "yy_get_previous_state(yyscanner)" is overwritten with value from "*--yyg->yy_state_ptr". Also see events: [returned_value] 1181 yy_current_state = *--yyg->yy_state_ptr; 1182 yyg->yy_lp = yy_accept[yy_current_state]; 1183 find_rule: /* we branch to this label when backing up */ ... 1517 case EOB_ACT_LAST_MATCH: 1518 yyg->yy_c_buf_p = 1519 &YY_CURRENT_BUFFER_LVALUE->yy_ch_buf[yyg->yy_n_chars]; 1520 (2) Event returned_value: Value from "yy_get_previous_state(yyscanner)" is assigned to "yy_current_state" here, but that stored value is not used before it is overwritten. Also see events: [value_overwrite] 1521 yy_current_state = yy_get_previous_state( yyscanner ); 1522 1523 yy_cp = yyg->yy_c_buf_p; 1524 yy_bp = yyg->yytext_ptr + YY_MORE_ADJ;
diff --git a/.gitignore b/.gitignore index 9d09709..eac2203 100644 --- a/.gitignore +++ b/.gitignore @@ -140,6 +140,7 @@ /src/remote/*_protocol.[ch] /src/rpc/virkeepaliveprotocol.[ch] /src/rpc/virnetprotocol.[ch] +/src/xenconfig/xen_xl_disk.[ch] /src/test_libvirt*.aug /src/test_virtlockd.aug /src/util/virkeymaps.h diff --git a/cfg.mk b/cfg.mk index 21f83c3..3df3dcb 100644 --- a/cfg.mk +++ b/cfg.mk @@ -89,8 +89,9 @@ distdir: sc_vulnerable_makefile_CVE-2012-3386.z endif
# Files that should never cause syntax check failures. +# (^(HACKING|docs/(news\.html\.in|.*\.patch))|\.(po|fig|gif|ico|png))$$ VC_LIST_ALWAYS_EXCLUDE_REGEX = \ - (^(HACKING|docs/(news\.html\.in|.*\.patch))|\.(po|fig|gif|ico|png))$$ + (^(HACKING|docs/(news\.html\.in|.*\.patch)|src/xenconfig/xen_xl_disk.[chl])|\.(po|fig|gif|ico|png))$$
# Functions like free() that are no-ops on NULL arguments. useless_free_options = \ diff --git a/configure.ac b/configure.ac index 9fd44b2..777367e 100644 --- a/configure.ac +++ b/configure.ac @@ -146,6 +146,7 @@ m4_ifndef([LT_INIT], [ ]) AM_PROG_CC_C_O AM_PROG_LD +AM_PROG_LEX
AC_MSG_CHECKING([for how to mark DSO non-deletable at runtime]) LIBVIRT_NODELETE= diff --git a/po/POTFILES.in b/po/POTFILES.in index e7cb2cc..094c8e3 100644 --- a/po/POTFILES.in +++ b/po/POTFILES.in @@ -247,6 +247,7 @@ src/xenapi/xenapi_driver.c src/xenapi/xenapi_utils.c src/xenconfig/xen_common.c src/xenconfig/xen_sxpr.c +src/xenconfig/xen_xl.c src/xenconfig/xen_xm.c tests/virpolkittest.c tools/libvirt-guests.sh.in diff --git a/src/Makefile.am b/src/Makefile.am index b6c1701..23c433d 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -999,11 +999,22 @@ CPU_SOURCES = \ VMX_SOURCES = \ vmx/vmx.c vmx/vmx.h
+AM_LFLAGS = -Pxl_disk_ --header-file=../$*.h +LEX_OUTPUT_ROOT = lex.xl_disk_ +BUILT_SOURCES += xenconfig/xen_xl_disk.c xenconfig/xen_xl_disk.h +# Generated header file is not implicitly added to dist +EXTRA_DIST += xenconfig/xen_xl_disk.h +CLEANFILES += xenconfig/xen_xl_disk.h xenconfig/xen_xl_disk.c + +XENXLDISKPARSER_SOURCES = xenconfig/xen_xl_disk.l + XENCONFIG_SOURCES = \ xenconfig/xenxs_private.h \ - xenconfig/xen_common.c xenconfig/xen_common.h \ + xenconfig/xen_common.c xenconfig/xen_common.h \ xenconfig/xen_sxpr.c xenconfig/xen_sxpr.h \ - xenconfig/xen_xm.c xenconfig/xen_xm.h + xenconfig/xen_xm.c xenconfig/xen_xm.h \ + xenconfig/xen_xl.c xenconfig/xen_xl.h \ + xenconfig/xen_xl_disk_i.h
pkgdata_DATA = cpu/cpu_map.xml
@@ -1058,10 +1069,19 @@ libvirt_vmx_la_SOURCES = $(VMX_SOURCES) endif WITH_VMX
if WITH_XENCONFIG +# Flex generated XL disk parser needs to be compiled without WARN_FLAGS +# 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 +libvirt_xenxldiskparser_la_SOURCES = \ + $(XENXLDISKPARSER_SOURCES) + noinst_LTLIBRARIES += libvirt_xenconfig.la libvirt_la_BUILT_LIBADD += libvirt_xenconfig.la libvirt_xenconfig_la_CFLAGS = \ -I$(top_srcdir)/src/conf $(AM_CFLAGS) +libvirt_xenconfig_la_LIBADD = libvirt_xenxldiskparser.la libvirt_xenconfig_la_SOURCES = $(XENCONFIG_SOURCES) endif WITH_XENCONFIG
@@ -1823,6 +1843,7 @@ EXTRA_DIST += \ $(VBOX_DRIVER_EXTRA_DIST) \ $(VMWARE_DRIVER_SOURCES) \ $(XENCONFIG_SOURCES) \ + $(XENXLDISKPARSER_SOURCES) \ $(ACCESS_DRIVER_POLKIT_POLICY)
check-local: check-augeas diff --git a/src/libvirt_xenconfig.syms b/src/libvirt_xenconfig.syms index 6541685..3e2e5d6 100644 --- a/src/libvirt_xenconfig.syms +++ b/src/libvirt_xenconfig.syms @@ -16,6 +16,10 @@ xenParseSxprChar; xenParseSxprSound; xenParseSxprString;
+#xenconfig/xen_xl.h +xenFormatXL; +xenParseXL; + # xenconfig/xen_xm.h xenFormatXM; xenParseXM; diff --git a/src/xenconfig/xen_common.c b/src/xenconfig/xen_common.c index 8ff10a0..b94b5db 100644 --- a/src/xenconfig/xen_common.c +++ b/src/xenconfig/xen_common.c @@ -1801,7 +1801,8 @@ xenFormatVfb(virConfPtr conf, virDomainDefPtr def, int xendConfigVersion) { int hvm = STREQ(def->os.type, "hvm") ? 1 : 0;
- if (def->ngraphics == 1) { + if (def->ngraphics == 1 && + def->graphics[0]->type != VIR_DOMAIN_GRAPHICS_TYPE_SPICE) { if (hvm || (xendConfigVersion < XEND_CONFIG_MIN_VERS_PVFB_NEWCONF)) { if (def->graphics[0]->type == VIR_DOMAIN_GRAPHICS_TYPE_SDL) { if (xenConfigSetInt(conf, "sdl", 1) < 0) diff --git a/src/xenconfig/xen_xl.c b/src/xenconfig/xen_xl.c new file mode 100644 index 0000000..8d1d2a7 --- /dev/null +++ b/src/xenconfig/xen_xl.c @@ -0,0 +1,499 @@ +/* + * xen_xl.c: Xen XL parsing functions + * + * This library 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; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library 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. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library. If not, see + * <http://www.gnu.org/licenses/>. + * + * Author: Kiarie Kahurani <davidkiarie4@gmail.com> + */ + +#include <config.h> + +#include "virconf.h" +#include "virerror.h" +#include "domain_conf.h" +#include "viralloc.h" +#include "virstring.h" +#include "xen_xl.h" +#include "xen_xl_disk.h" +#include "xen_xl_disk_i.h" + +#define VIR_FROM_THIS VIR_FROM_NONE + + +static int +xenParseXLSpice(virConfPtr conf, virDomainDefPtr def) +{ + virDomainGraphicsDefPtr graphics = NULL; + unsigned long port; + char *listenAddr = NULL; + int val; + + if (STREQ(def->os.type, "hvm")) { + if (xenConfigGetBool(conf, "spice", &val, 0) < 0) + return -1; + + if (val) { + if (VIR_ALLOC(graphics) < 0) + return -1; + + graphics->type = VIR_DOMAIN_GRAPHICS_TYPE_SPICE; + if (xenConfigCopyStringOpt(conf, "spicehost", &listenAddr) < 0) + goto cleanup; + if (listenAddr && + virDomainGraphicsListenSetAddress(graphics, 0, listenAddr, + -1, true) < 0) { + goto cleanup; + } + VIR_FREE(listenAddr); + + if (xenConfigGetULong(conf, "spicetls_port", &port, 0) < 0) + goto cleanup; + graphics->data.spice.tlsPort = (int)port; + + if (xenConfigGetULong(conf, "spiceport", &port, 0) < 0) + goto cleanup; + + graphics->data.spice.port = (int)port; + + if (!graphics->data.spice.tlsPort && + !graphics->data.spice.port) + graphics->data.spice.autoport = 1; + + if (xenConfigGetBool(conf, "spicedisable_ticketing", &val, 0) < 0) + goto cleanup; + if (val) { + if (xenConfigCopyStringOpt(conf, "spicepasswd", + &graphics->data.spice.auth.passwd) < 0) + goto cleanup; + } + + if (xenConfigGetBool(conf, "spiceagent_mouse", + &graphics->data.spice.mousemode, 0) < 0) + goto cleanup; + if (xenConfigGetBool(conf, "spicedvagent", &val, 0) < 0) + goto cleanup; + if (val) { + if (xenConfigGetBool(conf, "spice_clipboard_sharing", + &graphics->data.spice.copypaste, + 0) < 0) + goto cleanup; + } + + if (VIR_ALLOC_N(def->graphics, 1) < 0) + goto cleanup; + def->graphics[0] = graphics; + def->ngraphics = 1; + } + } + + return 0; + + cleanup: + virDomainGraphicsDefFree(graphics); + return -1; +} + + +void +xenXLDiskParserError(xenXLDiskParserContext *dpc, + const char *erroneous, + const char *message) +{ + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("disk config %s not supported: %s"), + erroneous, message); + + if (!dpc->err) + dpc->err = EINVAL; +} + + +static int +xenXLDiskParserPrep(xenXLDiskParserContext *dpc, + const char *spec, + virDomainDiskDefPtr disk) +{ + int err; + + dpc->spec = spec; + dpc->disk = disk; + dpc->access_set = 0; + + err = xl_disk_lex_init_extra(dpc, &dpc->scanner); + if (err) + goto fail; + + dpc->buf = xl_disk__scan_bytes(spec, strlen(spec), dpc->scanner); + if (!dpc->buf) { + err = ENOMEM; + goto fail; + } + + return 0; + + fail: + virReportSystemError(errno, "%s", + _("failed to initialize disk configuration parser")); + return err; +} + + +static void +xenXLDiskParserCleanup(xenXLDiskParserContext *dpc) +{ + if (dpc->buf) { + xl_disk__delete_buffer(dpc->buf, dpc->scanner); + dpc->buf = NULL; + } + + if (dpc->scanner) { + xl_disk_lex_destroy(dpc->scanner); + dpc->scanner = NULL; + } +} + + +/* + * positional parameters + * (If the <diskspec> strings are not separated by "=" + * the string is split following ',' and assigned to + * the following options in the following order) + * target,format,vdev,access + * ================================================================ + * + * The parameters below cannot be specified as positional parameters: + * + * other parameters + * devtype = <devtype> + * backendtype = <backend-type> + * parameters not taken care of + * backend = <domain-name> + * script = <script> + * direct-io-safe + * + * ================================================================ + * The parser does not take any deprecated parameters + * + * For more information refer to /xen/docs/misc/xl-disk-configuration.txt + */ +static int +xenParseXLDisk(virConfPtr conf, virDomainDefPtr def) +{ + virConfValuePtr list = virConfGetValue(conf, "disk"); + xenXLDiskParserContext dpc; + virDomainDiskDefPtr disk; + + memset(&dpc, 0, sizeof(dpc)); + + if (list && list->type == VIR_CONF_LIST) { + list = list->list; + while (list) { + char *disk_spec = list->str; + const char *driver; + + if ((list->type != VIR_CONF_STRING) || (list->str == NULL)) + goto skipdisk; + + if (!(disk = virDomainDiskDefNew())) + return -1; + + disk->src->readonly = 0; + disk->src->format = VIR_STORAGE_FILE_LAST; + + if (xenXLDiskParserPrep(&dpc, disk_spec, disk)) + goto fail; + + xl_disk_lex(dpc.scanner); + + if (dpc.err) + goto fail; + + if (disk->src->format == VIR_STORAGE_FILE_LAST) + disk->src->format = VIR_STORAGE_FILE_RAW; + + if (disk->device == VIR_DOMAIN_DISK_DEVICE_CDROM) { + disk->removable = true; + disk->src->readonly = true; + if (virDomainDiskSetDriver(disk, "qemu") < 0) + goto fail; + + virDomainDiskSetType(disk, VIR_STORAGE_TYPE_FILE); + if (!disk->src->path || STREQ(disk->src->path, "")) + disk->src->format = VIR_STORAGE_FILE_NONE; + } + + if (STRPREFIX(disk->dst, "xvd") || !STREQ(def->os.type, "hvm")) + disk->bus = VIR_DOMAIN_DISK_BUS_XEN; + else if (STRPREFIX(disk->dst, "sd")) + disk->bus = VIR_DOMAIN_DISK_BUS_SCSI; + else + disk->bus = VIR_DOMAIN_DISK_BUS_IDE; + + driver = virDomainDiskGetDriver(disk); + if (!driver) { + switch (disk->src->format) { + case VIR_STORAGE_FILE_QCOW: + case VIR_STORAGE_FILE_QCOW2: + case VIR_STORAGE_FILE_VHD: + driver = "qemu"; + if (virDomainDiskSetDriver(disk, "qemu") < 0) + goto fail; + break; + default: + driver = "phy"; + if (virDomainDiskSetDriver(disk, "phy") < 0) + goto fail; + } + } + + if (STREQ(driver, "phy")) + virDomainDiskSetType(disk, VIR_STORAGE_TYPE_BLOCK); + else + virDomainDiskSetType(disk, VIR_STORAGE_TYPE_FILE); + + if (VIR_APPEND_ELEMENT(def->disks, def->ndisks, disk) < 0) + goto fail; + + skipdisk: + list = list->next; + xenXLDiskParserCleanup(&dpc); + } + } + return 0; + + fail: + xenXLDiskParserCleanup(&dpc); + virDomainDiskDefFree(disk); + return -1; +} + + +virDomainDefPtr +xenParseXL(virConfPtr conf, virCapsPtr caps, int xendConfigVersion) +{ + virDomainDefPtr def = NULL; + + if (VIR_ALLOC(def) < 0) + return NULL; + + def->virtType = VIR_DOMAIN_VIRT_XEN; + def->id = -1; + + if (xenParseConfigCommon(conf, def, caps, xendConfigVersion) < 0) + goto cleanup; + + if (xenParseXLDisk(conf, def) < 0) + goto cleanup; + + if (xenParseXLSpice(conf, def) < 0) + goto cleanup; + + return def; + + cleanup: + virDomainDefFree(def); + return NULL; +} + + +static int +xenFormatXLDisk(virConfValuePtr list, virDomainDiskDefPtr disk) +{ + virBuffer buf = VIR_BUFFER_INITIALIZER; + virConfValuePtr val, tmp; + const char *src = virDomainDiskGetSource(disk); + int format = virDomainDiskGetFormat(disk); + + /* target */ + virBufferAsprintf(&buf, "%s,", src); + /* format */ + switch (format) { + case VIR_STORAGE_FILE_RAW: + virBufferAddLit(&buf, "raw,"); + break; + case VIR_STORAGE_FILE_VHD: + virBufferAddLit(&buf, "xvhd,"); + break; + case VIR_STORAGE_FILE_QCOW: + virBufferAddLit(&buf, "qcow,"); + break; + case VIR_STORAGE_FILE_QCOW2: + virBufferAddLit(&buf, "qcow2,"); + break; + /* set default */ + default: + virBufferAddLit(&buf, "raw,"); + } + + /* device */ + virBufferAdd(&buf, disk->dst, -1); + + virBufferAddLit(&buf, ","); + + if (disk->src->readonly) + virBufferAddLit(&buf, "r,"); + else if (disk->src->shared) + virBufferAddLit(&buf, "!,"); + else + virBufferAddLit(&buf, "w,"); + if (disk->transient) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("transient disks not supported yet")); + goto cleanup; + } + + if (disk->device == VIR_DOMAIN_DISK_DEVICE_CDROM) + virBufferAddLit(&buf, "devtype=cdrom"); + + if (virBufferCheckError(&buf) < 0) + goto cleanup; + + if (VIR_ALLOC(val) < 0) + goto cleanup; + + val->type = VIR_CONF_STRING; + val->str = virBufferContentAndReset(&buf); + tmp = list->list; + while (tmp && tmp->next) + tmp = tmp->next; + if (tmp) + tmp->next = val; + else + list->list = val; + return 0; + + cleanup: + virBufferFreeAndReset(&buf); + return -1; +} + + +static int +xenFormatXLDomainDisks(virConfPtr conf, virDomainDefPtr def) +{ + virConfValuePtr diskVal = NULL; + size_t i = 0; + + if (VIR_ALLOC(diskVal) < 0) + return -1; + + diskVal->type = VIR_CONF_LIST; + diskVal->list = NULL; + + for (i = 0; i < def->ndisks; i++) { + if (def->disks[i]->device == VIR_DOMAIN_DISK_DEVICE_FLOPPY) + continue; + if (xenFormatXLDisk(diskVal, def->disks[i]) < 0) + + goto cleanup; + } + + if (diskVal->list != NULL) { + int ret = virConfSetValue(conf, "disk", diskVal); + diskVal = NULL; + if (ret < 0) + goto cleanup; + } + + return 0; + + cleanup: + virConfFreeValue(diskVal); + return 0; +} + + +static int +xenFormatXLSpice(virConfPtr conf, virDomainDefPtr def) +{ + const char *listenAddr = NULL; + + if (STREQ(def->os.type, "hvm")) { + if (def->graphics[0]->type == VIR_DOMAIN_GRAPHICS_TYPE_SPICE) { + /* set others to false but may not be necessary */ + if (xenConfigSetInt(conf, "sdl", 0) < 0) + return -1; + + if (xenConfigSetInt(conf, "vnc", 0) < 0) + return -1; + + if (xenConfigSetInt(conf, "spice", 1) < 0) + return -1; + + if (xenConfigSetInt(conf, "spiceport", + def->graphics[0]->data.spice.port) < 0) + return -1; + + if (xenConfigSetInt(conf, "spicetls_port", + def->graphics[0]->data.spice.tlsPort) < 0) + return -1; + + if (def->graphics[0]->data.spice.auth.passwd) { + if (xenConfigSetInt(conf, "spicedisable_ticketing", 1) < 0) + return -1; + + if (def->graphics[0]->data.spice.auth.passwd && + xenConfigSetString(conf, "spicepasswd", + def->graphics[0]->data.spice.auth.passwd) < 0) + return -1; + } + + listenAddr = virDomainGraphicsListenGetAddress(def->graphics[0], 0); + if (listenAddr && + xenConfigSetString(conf, "spicehost", listenAddr) < 0) + return -1; + + if (xenConfigSetInt(conf, "spicemouse_mouse", + def->graphics[0]->data.spice.mousemode) < 0) + return -1; + + if (def->graphics[0]->data.spice.copypaste) { + if (xenConfigSetInt(conf, "spicedvagent", 1) < 0) + return -1; + if (xenConfigSetInt(conf, "spice_clipboard_sharing", + def->graphics[0]->data.spice.copypaste) < 0) + return -1; + } + } + } + + return 0; +} + + +virConfPtr +xenFormatXL(virDomainDefPtr def, virConnectPtr conn, int xendConfigVersion) +{ + virConfPtr conf = NULL; + + if (!(conf = virConfNew())) + goto cleanup; + + if (xenFormatConfigCommon(conf, def, conn, xendConfigVersion) < 0) + goto cleanup; + + if (xenFormatXLDomainDisks(conf, def) < 0) + goto cleanup; + + if (xenFormatXLSpice(conf, def) < 0) + goto cleanup; + + return conf; + + cleanup: + if (conf) + virConfFree(conf); + return NULL; +} diff --git a/src/xenconfig/xen_xl.h b/src/xenconfig/xen_xl.h new file mode 100644 index 0000000..536e9b7 --- /dev/null +++ b/src/xenconfig/xen_xl.h @@ -0,0 +1,33 @@ +/* + * xen_xl.h: Xen XL parsing functions + * + * This library 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; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library 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. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library. If not, see + * <http://www.gnu.org/licenses/>. + * + * Author: Kiarie Kahurani<davidkiarie4@gmail.com> + */ + +#ifndef __VIR_XEN_XL_H__ +# define __VIR_XEN_XL_H__ + +# include "virconf.h" +# include "domain_conf.h" +# include "xen_common.h" + +virDomainDefPtr xenParseXL(virConfPtr conn, virCapsPtr caps, + int xendConfigVersion); +virConfPtr xenFormatXL(virDomainDefPtr def, + virConnectPtr, int xendConfigVersion); + +#endif /* __VIR_XEN_XL_H__ */ 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@eu.citrix.com> + * + * 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" +# include "virstring.h" +# include "domain_conf.h" +# include "xen_xl.h" +# include "xen_xl_disk_i.h" + +#define YY_NO_INPUT +#define VIR_FROM_THIS VIR_FROM_NONE + +/* Some versions of flex have a bug (Fedora bugzilla 612465) which causes + * it to fail to declare these functions, which it defines. So declare + * them ourselves. Hopefully we won't have to simultaneously support + * a flex version which declares these differently somehow. */ +int xl_disk_lexget_column(yyscan_t yyscanner); +void xl_disk_lexset_column(int column_no, yyscan_t yyscanner); + + +/*----- useful macros and functions used in actions ----- + * we use macros in the actual rules to keep the actions short + * and particularly to avoid repeating boilerplate values such as + * DPC->disk, yytext, etc. */ + +/* For actions whose patterns contain '=', finds the start of the value */ +#define FROMEQUALS (strchr(yytext,'=')+1) + +/* Chops the delimiter off, modifying yytext and yyleng. */ +#define STRIP(delim) do{ \ + if (yyleng>0 && yytext[yyleng-1]==(delim)) \ + yytext[--yyleng] = 0; \ + }while(0) + +/* Sets a string value, checking it hasn't been set already. */ +#define SAVESTRING(what,loc,val) do{ \ + savestring(DPC, what " respecified", &DPC->disk->loc, (val)); \ + }while(0) + + +static void +savestring(xenXLDiskParserContext *dpc, + const char *what_respecified, + char **update, + const char *value) +{ + if (*update) { + if (**update) { + xenXLDiskParserError(dpc, value, what_respecified); + return; + } + + VIR_FREE(*update); /* do not complain about overwriting empty strings */ + } + + ignore_value(VIR_STRDUP(*update, value)); +} + +#define DPC dpc /* our convention in lexer helper functions */ + +/* Sets ->readwrite from the string. */ +static void +setaccess(xenXLDiskParserContext *dpc, const char *str) +{ + if (STREQ(str, "rw") || STREQ(str, "w")) { + dpc->disk->src->readonly = 0; + } else if (STREQ(str, "r") || STREQ(str, "ro")) { + dpc->disk->src->readonly = 1; + } else if (STREQ(str, "w!") || STREQ(str, "!")) { + dpc->disk->src->readonly = 0; + dpc->disk->src->shared = 1; + } else { + xenXLDiskParserError(dpc, str, "unknown value for access"); + } + dpc->access_set = 1; +} + +/* Sets ->format from the string. IDL should provide something for this. */ +static void +setformat(xenXLDiskParserContext *dpc, const char *str) +{ + if (STREQ(str, "") || STREQ(str, "raw")) + virDomainDiskSetFormat(dpc->disk, VIR_STORAGE_FILE_RAW); + else if (STREQ(str, "qcow")) + virDomainDiskSetFormat(dpc->disk, VIR_STORAGE_FILE_QCOW); + else if (STREQ(str, "qcow2")) + virDomainDiskSetFormat(dpc->disk, VIR_STORAGE_FILE_QCOW2); + else if (STREQ(str, "vhd")) + virDomainDiskSetFormat(dpc->disk, VIR_STORAGE_FILE_VHD); + else + xenXLDiskParserError(dpc, str, "unknown value for format"); +} + + +/* Sets ->backend from the string. IDL should provide something for this. */ +static void +setdrivertype(xenXLDiskParserContext *dpc, const char *str) +{ + if (STREQ(str, "phy")) + ignore_value(virDomainDiskSetDriver(dpc->disk, "phy")); + else if (STREQ(str, "tap")) + ignore_value(virDomainDiskSetDriver(dpc->disk, "tap")); + else if (STREQ(str, "file") || STREQ(str, "")) + ignore_value(virDomainDiskSetDriver(dpc->disk, "qemu")); + else + xenXLDiskParserError(dpc, str, "unknown value for backendtype"); +} + + +/* Handles a vdev positional parameter which includes a devtype. */ +static int +vdev_and_devtype(xenXLDiskParserContext *dpc, char *str) +{ + /* returns 1 if it was <vdev>:<devtype>, 0 (doing nothing) otherwise */ + char *colon = strrchr(str, ':'); + if (!colon) + return 0; + + *colon++ = 0; + SAVESTRING("vdev", dst, str); + + if (STREQ(colon,"cdrom")) { + DPC->disk->device = VIR_DOMAIN_DISK_DEVICE_CDROM; + } else if (STREQ(colon, "disk")) { + DPC->disk->device = VIR_DOMAIN_DISK_DEVICE_DISK; + } else { + xenXLDiskParserError(DPC, colon, "unknown deprecated type"); + } + return 1; +} + +#undef DPC /* needs to be defined differently the actual lexer */ +#define DPC ((xenXLDiskParserContext*)yyextra) + +%} + +%option warn +%option nodefault +%option batch +%option 8bit +%option noyywrap +%option reentrant +%option nounput + +%x LEXERR + +%% + + /*----- the scanner rules which do the parsing -----*/ + +[ \t\n]+/([^ \t\n].*)? { /* ignore whitespace before parameters */ } + + /* ordinary parameters setting enums or strings */ + +format=[^,]*,? { STRIP(','); setformat(DPC, FROMEQUALS); } + +cdrom,? { DPC->disk->device = VIR_DOMAIN_DISK_DEVICE_CDROM; } +devtype=cdrom,? { DPC->disk->device = VIR_DOMAIN_DISK_DEVICE_CDROM; } +devtype=disk,? { DPC->disk->device = VIR_DOMAIN_DISK_DEVICE_DISK; } +devtype=[^,]*,? { xenXLDiskParserError(DPC, yytext,"unknown value for type"); } + +access=[^,]*,? { STRIP(','); setaccess(DPC, FROMEQUALS); } +backendtype=[^,]*,? { STRIP(','); setdrivertype(DPC, FROMEQUALS); } + +vdev=[^,]*,? { STRIP(','); SAVESTRING("vdev", dst, FROMEQUALS); } + + /* the target magic parameter, eats the rest of the string */ + +target=.* { STRIP(','); SAVESTRING("target", src->path, FROMEQUALS); } + + /* unknown parameters */ + +[a-z][-a-z0-9]*=[^,],? { xenXLDiskParserError(DPC, yytext, "unknown parameter"); } + + /* the "/.*" in these patterns ensures that they count as if they + * matched the whole string, so these patterns take precedence */ + +(raw|qcow2?|vhd):/.* { + STRIP(':'); + DPC->had_depr_prefix=1; + setformat(DPC, yytext); + } + +tapdisk:/.* { DPC->had_depr_prefix=1; } +tap2?:/.* { DPC->had_depr_prefix=1; } +aio:/.* { DPC->had_depr_prefix=1; } +ioemu:/.* { DPC->had_depr_prefix=1; } +file:/.* { DPC->had_depr_prefix=1; } +phy:/.* { DPC->had_depr_prefix=1; } +[a-z][a-z0-9]*:/([^a-z0-9].*)? { + xenXLDiskParserError(DPC, yytext, "unknown deprecated disk prefix"); + return 0; + } + + /* positional parameters */ + +[^=,]*,|[^=,]+,? { + STRIP(','); + + if (DPC->err) { + /* previous errors may just lead to subsequent ones */ + } else if (!DPC->disk->src->path) { + SAVESTRING("target", src->path, yytext); + } else if (DPC->disk->src->format == VIR_STORAGE_FILE_LAST){ + setformat(DPC, yytext); + } + else if (!DPC->disk->dst) { + if (!vdev_and_devtype(DPC, yytext)) + SAVESTRING("vdev", dst, yytext); + } else if (!DPC->access_set) { + DPC->access_set = 1; + setaccess(DPC, yytext); + } else { + xenXLDiskParserError(DPC, yytext, "too many positional parameters"); + return 0; /* don't print any more errors */ + } +} + +. { + BEGIN(LEXERR); + yymore(); +} +<LEXERR>.* { + xenXLDiskParserError(DPC, yytext, "bad disk syntax"); + return 0; +} diff --git a/src/xenconfig/xen_xl_disk_i.h b/src/xenconfig/xen_xl_disk_i.h new file mode 100644 index 0000000..063dedf --- /dev/null +++ b/src/xenconfig/xen_xl_disk_i.h @@ -0,0 +1,39 @@ +/* + * xen_xl_disk_i.h - common header for disk spec parser + * + * Copyright (C) 2011 Citrix Ltd. + * Author Ian Jackson <ian.jackson@eu.citrix.com> + * + * 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. + */ + +#ifndef __VIR_XEN_XL_DISK_I_H__ +# define __VIR_XEN_XL_DISK_I_H__ + +# include "virconf.h" +# include "domain_conf.h" + + +typedef struct { + int err; + void *scanner; + YY_BUFFER_STATE buf; + virDomainDiskDefPtr disk; + int access_set; + int had_depr_prefix; + const char *spec; +} xenXLDiskParserContext; + +void xenXLDiskParserError(xenXLDiskParserContext *dpc, + const char *erroneous, + const char *message); + +#endif /* __VIR_XEN_XL_DISK_I_H__ */

John Ferlan wrote:
On 12/15/2014 11:30 PM, Jim Fehlig wrote:
From: Kiarie Kahurani <davidkiarie4@gmail.com>
Introduce a Xen xl parser
This parser allows for users to convert the new xl disk format and spice graphics config to libvirt xml format and vice versa. Regarding the spice graphics config, the code is pretty much straight forward. For the disk {formating, parsing}, this parser takes care of the new xl format which include positional parameters and key/value parameters. In xl format disk config a <diskspec> consists of parameters separated by commas. If the parameters do not contain an '=' they are automatically assigned to certain options following the order below
target, format, vdev, access
The above are the only mandatory parameters in the <diskspec> but there are many more disk config options. These options can be specified as key=value pairs. This takes care of the rest of the options such as
devtype, backend, backendtype, script, direct-io-safe,
The positional paramters can also be specified in key/value form for example
/dev/vg/guest-volume,,hda /dev/vg/guest-volume,raw,hda,rw format=raw, vdev=hda, access=rw, target=/dev/vg/guest-volume
are interpleted to one config.
In xm format, the above diskspec would be written as
phy:/dev/vg/guest-volume,hda,w
The disk parser is based on the same parser used successfully by the Xen project for several years now. Ian Jackson authored the scanner, which is used by this commit with mimimal changes. Only the PREFIX option is changed, to produce function and file names more consistent with libvirt's convention.
Signed-off-by: Kiarie Kahurani <davidkiarie4@gmail.com> Signed-off-by: Jim Fehlig <jfehlig@suse.com> --- .gitignore | 1 + cfg.mk | 3 +- configure.ac | 1 + po/POTFILES.in | 1 + src/Makefile.am | 25 ++- src/libvirt_xenconfig.syms | 4 + src/xenconfig/xen_common.c | 3 +- src/xenconfig/xen_xl.c | 499 ++++++++++++++++++++++++++++++++++++++++++ src/xenconfig/xen_xl.h | 33 +++ src/xenconfig/xen_xl_disk.l | 256 ++++++++++++++++++++++ src/xenconfig/xen_xl_disk_i.h | 39 ++++ 11 files changed, 861 insertions(+), 4 deletions(-)
In addition to the build issues - it seems the generated "xen_xl_disk.c" has numerous issues found by Coverity. Since it's not clear to me how this is all put together - I'll cut-n-paste from my Coverity output and provide some basic analysis - hope it all makes sense...
Resolved by my series to revert the flex-based parser, which will remove the generated file https://www.redhat.com/archives/libvir-list/2015-January/msg00268.html Regards, Jim

From: Kiarie Kahurani <davidkiarie4@gmail.com> add tests for the xen_xl config parser Signed-off-by: Kiarie Kahurani <davidkiarie4@gmail.com> Signed-off-by: Jim Fehlig <jfehlig@suse.com> --- tests/Makefile.am | 9 +- tests/testutilsxen.c | 50 ++++++++ tests/testutilsxen.h | 9 +- tests/xlconfigdata/test-new-disk.cfg | 26 ++++ tests/xlconfigdata/test-new-disk.xml | 51 ++++++++ tests/xlconfigdata/test-spice.cfg | 32 +++++ tests/xlconfigdata/test-spice.xml | 45 +++++++ tests/xlconfigtest.c | 224 +++++++++++++++++++++++++++++++++++ 8 files changed, 444 insertions(+), 2 deletions(-) diff --git a/tests/Makefile.am b/tests/Makefile.am index e9418ea..af85335 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -138,6 +138,7 @@ EXTRA_DIST = \ vmx2xmldata \ xencapsdata \ xmconfigdata \ + xlconfigdata \ xml2sexprdata \ xml2vmxdata \ vmwareverdata \ @@ -225,7 +226,8 @@ ssh_LDADD = $(COVERAGE_LDFLAGS) if WITH_XEN test_programs += xml2sexprtest sexpr2xmltest \ - xmconfigtest xencapstest statstest reconnect + xmconfigtest xencapstest statstest reconnect \ + xlconfigtest endif WITH_XEN if WITH_QEMU test_programs += qemuxml2argvtest qemuxml2xmltest qemuxmlnstest \ @@ -477,6 +479,11 @@ sexpr2xmltest_SOURCES = \ testutils.c testutils.h sexpr2xmltest_LDADD = $(xen_LDADDS) +xlconfigtest_SOURCES = \ + xlconfigtest.c testutilsxen.c testutilsxen.h \ + testutils.c testutils.h +xlconfigtest_LDADD =$(xen_LDADDS) + xmconfigtest_SOURCES = \ xmconfigtest.c testutilsxen.c testutilsxen.h \ testutils.c testutils.h diff --git a/tests/testutilsxen.c b/tests/testutilsxen.c index a50a8a2..df1d124 100644 --- a/tests/testutilsxen.c +++ b/tests/testutilsxen.c @@ -69,3 +69,53 @@ virCapsPtr testXenCapsInit(void) virObjectUnref(caps); return NULL; } + + +virCapsPtr +testXLInitCaps(void) +{ + virCapsPtr caps; + virCapsGuestPtr guest; + virCapsGuestMachinePtr *machines; + int nmachines; + static const char *const x86_machines[] = { + "xenfv" + }; + static const char *const xen_machines[] = { + "xenpv" + }; + + if ((caps = virCapabilitiesNew(virArchFromHost(), + false, false)) == NULL) + return NULL; + nmachines = ARRAY_CARDINALITY(x86_machines); + if ((machines = virCapabilitiesAllocMachines(x86_machines, nmachines)) == NULL) + goto cleanup; + if ((guest = virCapabilitiesAddGuest(caps, "hvm", VIR_ARCH_X86_64, + "/usr/lib/xen/bin/qemu-dm", NULL, + nmachines, machines)) == NULL) + goto cleanup; + machines = NULL; + if (virCapabilitiesAddGuestDomain(guest, "xen", NULL, + NULL, 0, NULL) == NULL) + goto cleanup; + nmachines = ARRAY_CARDINALITY(xen_machines); + if ((machines = virCapabilitiesAllocMachines(xen_machines, nmachines)) == NULL) + goto cleanup; + + if ((guest = virCapabilitiesAddGuest(caps, "xen", VIR_ARCH_X86_64, + "/usr/lib/xen/bin/qemu-dm", NULL, + nmachines, machines)) == NULL) + goto cleanup; + machines = NULL; + + if (virCapabilitiesAddGuestDomain(guest, "xen", NULL, + NULL, 0, NULL) == NULL) + goto cleanup; + return caps; + + cleanup: + virCapabilitiesFreeMachines(machines, nmachines); + virObjectUnref(caps); + return NULL; +} diff --git a/tests/testutilsxen.h b/tests/testutilsxen.h index 54155e5..c78350d 100644 --- a/tests/testutilsxen.h +++ b/tests/testutilsxen.h @@ -1,3 +1,10 @@ -#include "capabilities.h" +#ifndef _TESTUTILSXEN_H_ +# define _TESTUTILSXEN_H_ + +# include "capabilities.h" virCapsPtr testXenCapsInit(void); + +virCapsPtr testXLInitCaps(void); + +#endif /* _TESTUTILSXEN_H_ */ diff --git a/tests/xlconfigdata/test-new-disk.cfg b/tests/xlconfigdata/test-new-disk.cfg new file mode 100644 index 0000000..afc0a46 --- /dev/null +++ b/tests/xlconfigdata/test-new-disk.cfg @@ -0,0 +1,26 @@ +name = "XenGuest2" +uuid = "c7a5fdb2-cdaf-9455-926a-d65c16db1809" +maxmem = 579 +memory = 394 +vcpus = 1 +builder = "hvm" +kernel = "/usr/lib/xen/boot/hvmloader" +boot = "d" +pae = 1 +acpi = 1 +apic = 1 +hap = 0 +viridian = 0 +localtime = 0 +on_poweroff = "destroy" +on_reboot = "restart" +on_crash = "restart" +device_model = "/usr/lib/xen/bin/qemu-dm" +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,type=ioemu" ] +parallel = "none" +serial = "none" +disk = [ "/dev/HostVG/XenGuest2,raw,hda,w,", "/var/lib/libvirt/images/XenGuest2-home,qcow2,hdb,w,", "/root/boot.iso,raw,hdc,r,devtype=cdrom" ] diff --git a/tests/xlconfigdata/test-new-disk.xml b/tests/xlconfigdata/test-new-disk.xml new file mode 100644 index 0000000..1c96a62 --- /dev/null +++ b/tests/xlconfigdata/test-new-disk.xml @@ -0,0 +1,51 @@ +<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 type='rom'>/usr/lib/xen/boot/hvmloader</loader> + <boot dev='cdrom'/> + </os> + <features> + <acpi/> + <apic/> + <pae/> + </features> + <clock offset='utc' adjustment='reset'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>restart</on_crash> + <devices> + <emulator>/usr/lib/xen/bin/qemu-dm</emulator> + <disk type='block' device='disk'> + <driver name='phy' type='raw'/> + <source dev='/dev/HostVG/XenGuest2'/> + <target dev='hda' bus='ide'/> + </disk> + <disk type='file' device='disk'> + <driver name='qemu' type='qcow2'/> + <source file='/var/lib/libvirt/images/XenGuest2-home'/> + <target dev='hdb' bus='ide'/> + </disk> + <disk type='file' device='cdrom'> + <driver name='qemu' type='raw'/> + <source file='/root/boot.iso'/> + <target dev='hdc' bus='ide'/> + <readonly/> + </disk> + <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> + </devices> +</domain> diff --git a/tests/xlconfigdata/test-spice.cfg b/tests/xlconfigdata/test-spice.cfg new file mode 100644 index 0000000..f7aa55c --- /dev/null +++ b/tests/xlconfigdata/test-spice.cfg @@ -0,0 +1,32 @@ +name = "XenGuest2" +uuid = "c7a5fdb2-cdaf-9455-926a-d65c16db1809" +maxmem = 579 +memory = 394 +vcpus = 1 +builder = "hvm" +kernel = "/usr/lib/xen/boot/hvmloader" +boot = "d" +pae = 1 +acpi = 1 +apic = 1 +hap = 0 +viridian = 0 +rtc_timeoffset = 0 +localtime = 1 +on_poweroff = "destroy" +on_reboot = "restart" +on_crash = "restart" +device_model = "/usr/lib/xen/bin/qemu-dm" +vif = [ "mac=00:16:3e:66:92:9c,bridge=xenbr1,script=vif-bridge,model=e1000" ] +parallel = "none" +serial = "none" +disk = [ "/dev/HostVG/XenGuest2,raw,hda,w,", "/root/boot.iso,raw,hdc,w," ] +sdl = 0 +vnc = 0 +spice = 1 +spicehost = "127.0.0.1" +spiceport = 590 +spicetls_port = 500 +spicedisable_ticketing = 1 +spicepasswd = "thebeast" +spiceagent_mouse = 0 diff --git a/tests/xlconfigdata/test-spice.xml b/tests/xlconfigdata/test-spice.xml new file mode 100644 index 0000000..1e3f78d --- /dev/null +++ b/tests/xlconfigdata/test-spice.xml @@ -0,0 +1,45 @@ +<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>/usr/lib/xen/boot/hvmloader</loader> + <boot dev='cdrom'/> + </os> + <features> + <acpi/> + <apic/> + <pae/> + </features> + <clock offset='variable' adjustment='0' basis='localtime'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>restart</on_crash> + <devices> + <emulator>/usr/lib/xen/bin/qemu-dm</emulator> + <disk type='block' device='disk'> + <driver name='phy' type='raw'/> + <source dev='/dev/HostVG/XenGuest2'/> + <target dev='hda' bus='ide'/> + </disk> + <disk type='block' device='disk'> + <driver name='phy' type='raw'/> + <source dev='/root/boot.iso'/> + <target dev='hdc' bus='ide'/> + </disk> + <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='spice' port='590' tlsPort='500' autoport='no' listen='127.0.0.1' passwd='thebeast'> + <listen type='address' address='127.0.0.1'/> + </graphics> + </devices> +</domain> diff --git a/tests/xlconfigtest.c b/tests/xlconfigtest.c new file mode 100644 index 0000000..b966c15 --- /dev/null +++ b/tests/xlconfigtest.c @@ -0,0 +1,224 @@ +/* + * xlconfigtest.c: Test backend for xl_internal config file handling + * + * Copyright (C) 2007, 2010-2011, 2014 Red Hat, Inc. + * + * This library 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; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library 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. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library. If not, see + * <http://www.gnu.org/licenses/>. + * + * Author: Daniel P. Berrange <berrange@redhat.com> + * Author: Kiarie Kahurani <davidkiarie4@gmail.com> + * + */ + +#include <config.h> + +#include <stdio.h> +#include <string.h> +#include <unistd.h> + +#include "internal.h" +#include "datatypes.h" +#include "xenconfig/xen_xl.h" +#include "viralloc.h" +#include "virstring.h" +#include "testutils.h" +#include "testutilsxen.h" +#include "xen/xen_driver.h" + +#define VIR_FROM_THIS VIR_FROM_NONE + +static virCapsPtr caps; +static virDomainXMLOptionPtr xmlopt; +/* + * parses the xml, creates a domain def and compare with equivalent xm config + */ +static int +testCompareParseXML(const char *xmcfg, const char *xml, int xendConfigVersion) +{ + char *xmlData = NULL; + char *xmcfgData = NULL; + char *gotxmcfgData = NULL; + virConfPtr conf = NULL; + virConnectPtr conn = NULL; + int wrote = 4096; + int ret = -1; + virDomainDefPtr def = NULL; + + if (VIR_ALLOC_N(gotxmcfgData, wrote) < 0) + goto fail; + + conn = virGetConnect(); + if (!conn) goto fail; + + if (virtTestLoadFile(xml, &xmlData) < 0) + goto fail; + + if (virtTestLoadFile(xmcfg, &xmcfgData) < 0) + goto fail; + + if (!(def = virDomainDefParseString(xmlData, caps, xmlopt, + 1 << VIR_DOMAIN_VIRT_XEN, + VIR_DOMAIN_XML_INACTIVE))) + goto fail; + + if (!virDomainDefCheckABIStability(def, def)) { + fprintf(stderr, "ABI stability check failed on %s", xml); + goto fail; + } + + if (!(conf = xenFormatXL(def, conn, xendConfigVersion))) + goto fail; + + if (virConfWriteMem(gotxmcfgData, &wrote, conf) < 0) + goto fail; + gotxmcfgData[wrote] = '\0'; + + if (STRNEQ(xmcfgData, gotxmcfgData)) { + virtTestDifference(stderr, xmcfgData, gotxmcfgData); + goto fail; + } + + ret = 0; + + fail: + VIR_FREE(xmlData); + VIR_FREE(xmcfgData); + VIR_FREE(gotxmcfgData); + if (conf) + virConfFree(conf); + virDomainDefFree(def); + virObjectUnref(conn); + + return ret; +} +/* + * parses the xl config, develops domain def and compares with equivalent xm config + */ +static int +testCompareFormatXML(const char *xmcfg, const char *xml, int xendConfigVersion) +{ + char *xmlData = NULL; + char *xmcfgData = NULL; + char *gotxml = NULL; + virConfPtr conf = NULL; + int ret = -1; + virConnectPtr conn; + virDomainDefPtr def = NULL; + + conn = virGetConnect(); + if (!conn) goto fail; + + if (virtTestLoadFile(xml, &xmlData) < 0) + goto fail; + + if (virtTestLoadFile(xmcfg, &xmcfgData) < 0) + goto fail; + + if (!(conf = virConfReadMem(xmcfgData, strlen(xmcfgData), 0))) + goto fail; + + if (!(def = xenParseXL(conf, caps, xendConfigVersion))) + goto fail; + + if (!(gotxml = virDomainDefFormat(def, VIR_DOMAIN_XML_INACTIVE | + VIR_DOMAIN_XML_SECURE))) + goto fail; + + if (STRNEQ(xmlData, gotxml)) { + virtTestDifference(stderr, xmlData, gotxml); + goto fail; + } + + ret = 0; + + fail: + if (conf) + virConfFree(conf); + VIR_FREE(xmlData); + VIR_FREE(xmcfgData); + VIR_FREE(gotxml); + virDomainDefFree(def); + virObjectUnref(conn); + + return ret; +} + + +struct testInfo { + const char *name; + int version; + int mode; +}; + +static int +testCompareHelper(const void *data) +{ + int result = -1; + const struct testInfo *info = data; + char *xml = NULL; + char *cfg = NULL; + + if (virAsprintf(&xml, "%s/xlconfigdata/test-%s.xml", + abs_srcdir, info->name) < 0 || + virAsprintf(&cfg, "%s/xlconfigdata/test-%s.cfg", + abs_srcdir, info->name) < 0) + goto cleanup; + + if (info->mode == 0) + result = testCompareParseXML(cfg, xml, info->version); + else + result = testCompareFormatXML(cfg, xml, info->version); + + cleanup: + VIR_FREE(xml); + VIR_FREE(cfg); + + return result; +} + + +static int +mymain(void) +{ + int ret = 0; + + if (!(caps = testXLInitCaps())) + return EXIT_FAILURE; + + if (!(xmlopt = xenDomainXMLConfInit())) + return EXIT_FAILURE; + +#define DO_TEST(name, version) \ + do { \ + struct testInfo info0 = { name, version, 0 }; \ + struct testInfo info1 = { name, version, 1 }; \ + if (virtTestRun("Xen XM-2-XML Parse " name, \ + testCompareHelper, &info0) < 0) \ + ret = -1; \ + if (virtTestRun("Xen XM-2-XML Format " name, \ + testCompareHelper, &info1) < 0) \ + ret = -1; \ + } while (0) + + DO_TEST("new-disk", 3); +// DO_TEST("spice", 3); + + virObjectUnref(caps); + virObjectUnref(xmlopt); + + return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE; +} + +VIRT_TEST_MAIN(mymain) -- 1.8.4.5

On 16.12.2014 05:30, Jim Fehlig wrote:
From: Kiarie Kahurani <davidkiarie4@gmail.com>
add tests for the xen_xl config parser
Signed-off-by: Kiarie Kahurani <davidkiarie4@gmail.com> Signed-off-by: Jim Fehlig <jfehlig@suse.com> --- tests/Makefile.am | 9 +- tests/testutilsxen.c | 50 ++++++++ tests/testutilsxen.h | 9 +- tests/xlconfigdata/test-new-disk.cfg | 26 ++++ tests/xlconfigdata/test-new-disk.xml | 51 ++++++++ tests/xlconfigdata/test-spice.cfg | 32 +++++ tests/xlconfigdata/test-spice.xml | 45 +++++++ tests/xlconfigtest.c | 224 +++++++++++++++++++++++++++++++++++ 8 files changed, 444 insertions(+), 2 deletions(-)
ACK Michal

From: Kiarie Kahurani <davidkiarie4@gmail.com> Now that xenconfig supports parsing and formatting Xen's XL config format, integrate it into the libxl driver's connectDomainXML{From,To}Native functions. Signed-off-by: Kiarie Kahurani <davidkiarie4@gmail.com> Signed-off-by: Jim Fehlig <jfehlig@suse.com> --- src/libxl/libxl_driver.c | 32 ++++++++++++++++++++++++-------- 1 file changed, 24 insertions(+), 8 deletions(-) diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index 53c87ce..4135670 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -48,6 +48,7 @@ #include "libxl_migration.h" #include "xen_xm.h" #include "xen_sxpr.h" +#include "xen_xl.h" #include "virtypedparam.h" #include "viruri.h" #include "virstring.h" @@ -67,6 +68,7 @@ VIR_LOG_INIT("libxl.libxl_driver"); #define LIBXL_DOM_REQ_CRASH 3 #define LIBXL_DOM_REQ_HALT 4 +#define LIBXL_CONFIG_FORMAT_XL "xen-xl" #define LIBXL_CONFIG_FORMAT_XM "xen-xm" #define LIBXL_CONFIG_FORMAT_SEXPR "xen-sxpr" @@ -2214,7 +2216,17 @@ libxlConnectDomainXMLFromNative(virConnectPtr conn, if (virConnectDomainXMLFromNativeEnsureACL(conn) < 0) goto cleanup; - if (STREQ(nativeFormat, LIBXL_CONFIG_FORMAT_XM)) { + if (STREQ(nativeFormat, LIBXL_CONFIG_FORMAT_XL)) { + if (!(conf = virConfReadMem(nativeConfig, strlen(nativeConfig), 0))) + goto cleanup; + if (!(def = xenParseXL(conf, + cfg->caps, + cfg->verInfo->xen_version_major))) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("parsing xl config failed")); + goto cleanup; + } + } else if (STREQ(nativeFormat, LIBXL_CONFIG_FORMAT_XM)) { if (!(conf = virConfReadMem(nativeConfig, strlen(nativeConfig), 0))) goto cleanup; @@ -2269,20 +2281,24 @@ libxlConnectDomainXMLToNative(virConnectPtr conn, const char * nativeFormat, if (virConnectDomainXMLToNativeEnsureACL(conn) < 0) goto cleanup; - if (STRNEQ(nativeFormat, LIBXL_CONFIG_FORMAT_XM)) { - virReportError(VIR_ERR_INVALID_ARG, - _("unsupported config type %s"), nativeFormat); - goto cleanup; - } - if (!(def = virDomainDefParseString(domainXml, cfg->caps, driver->xmlopt, 1 << VIR_DOMAIN_VIRT_XEN, VIR_DOMAIN_XML_INACTIVE))) goto cleanup; - if (!(conf = xenFormatXM(conn, def, cfg->verInfo->xen_version_major))) + if (STREQ(nativeFormat, LIBXL_CONFIG_FORMAT_XL)) { + if (!(conf = xenFormatXL(def, conn, cfg->verInfo->xen_version_major))) + goto cleanup; + } else if (STREQ(nativeFormat, LIBXL_CONFIG_FORMAT_XM)) { + if (!(conf = xenFormatXM(conn, def, cfg->verInfo->xen_version_major))) + goto cleanup; + } else { + + virReportError(VIR_ERR_INVALID_ARG, + _("unsupported config type %s"), nativeFormat); goto cleanup; + } if (VIR_ALLOC_N(ret, len) < 0) goto cleanup; -- 1.8.4.5

On 16.12.2014 05:30, Jim Fehlig wrote:
From: Kiarie Kahurani <davidkiarie4@gmail.com>
Now that xenconfig supports parsing and formatting Xen's XL config format, integrate it into the libxl driver's connectDomainXML{From,To}Native functions.
Signed-off-by: Kiarie Kahurani <davidkiarie4@gmail.com> Signed-off-by: Jim Fehlig <jfehlig@suse.com> --- src/libxl/libxl_driver.c | 32 ++++++++++++++++++++++++-------- 1 file changed, 24 insertions(+), 8 deletions(-)
ACK Michal
participants (4)
-
David kiarie
-
Jim Fehlig
-
John Ferlan
-
Michal Privoznik