Re: [libvirt] [PATCH 1/4] qemu: Cleanup boot parameter building

This patch cleans up building the "-boot" parameter and while on that fixes one inconsistency by modifying these things: - First off, there's the cleanup for all *_LAST enum values to end without comma at the end. I chose this way because if it were the other way around, the patch would have 2 times more lines. - I completed the unfinished virDomainBootMenu enum by specifying LAST, declaring it and also declaring the TypeFromString and TypeToString parameters. - Previously mentioned TypeFromString and TypeToString are used when parsing the XML. - Last, but not least, visible change is that the "-boot" parameter is built and parsed properly: - The "order=" prefix is used only when additional parameters are used (menu, etc.). - It's rewritten in a way that other parameters can be added easily in the future (used in following patch). - The "order=" parameter is properly parsed regardless to where it is placed in the string (e.g. "menu=on,order=nc"). - The "menu=" parameter (and others in the future) are created when they should be (i.e. even when bootindex is supported and used, but not when bootloader is selected). --- src/conf/domain_conf.c | 16 +++- src/conf/domain_conf.h | 63 ++++++++------- src/libvirt_private.syms | 2 + src/qemu/qemu_command.c | 93 ++++++++++++++++------ ...xml2argv-boot-menu-disable-drive-bootindex.args | 1 + 5 files changed, 118 insertions(+), 57 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 15b360a..35814fb 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -102,6 +102,11 @@ VIR_ENUM_IMPL(virDomainBoot, VIR_DOMAIN_BOOT_LAST, "hd", "network") +VIR_ENUM_IMPL(virDomainBootMenu, VIR_DOMAIN_BOOT_MENU_LAST, + "default", + "yes", + "no") + VIR_ENUM_IMPL(virDomainFeature, VIR_DOMAIN_FEATURE_LAST, "acpi", "apic", @@ -8181,10 +8186,15 @@ virDomainDefParseBootXML(xmlXPathContextPtr ctxt, bootstr = virXPathString("string(./os/bootmenu[1]/@enable)", ctxt); if (bootstr) { - if (STREQ(bootstr, "yes")) - def->os.bootmenu = VIR_DOMAIN_BOOT_MENU_ENABLED; - else + def->os.bootmenu = virDomainBootMenuTypeFromString(bootstr); + if (def->os.bootmenu <= 0) { + /* In order not to break misconfigured machines, this + * should not emit an error, but rather set the bootmenu + * to disabled */ + VIR_WARN("disabling bootmenu due to unknown option '%s'", + bootstr); def->os.bootmenu = VIR_DOMAIN_BOOT_MENU_DISABLED; + } VIR_FREE(bootstr); } diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index f0dea48..510406a 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -134,7 +134,7 @@ typedef enum { VIR_DOMAIN_DEVICE_CHR, VIR_DOMAIN_DEVICE_MEMBALLOON, - VIR_DOMAIN_DEVICE_LAST, + VIR_DOMAIN_DEVICE_LAST } virDomainDeviceType; typedef struct _virDomainDeviceDef virDomainDeviceDef; @@ -178,7 +178,7 @@ enum virDomainVirtType { VIR_DOMAIN_VIRT_PHYP, VIR_DOMAIN_VIRT_PARALLELS, - VIR_DOMAIN_VIRT_LAST, + VIR_DOMAIN_VIRT_LAST }; enum virDomainDeviceAddressType { @@ -289,7 +289,7 @@ enum virDomainSeclabelType { VIR_DOMAIN_SECLABEL_DYNAMIC, VIR_DOMAIN_SECLABEL_STATIC, - VIR_DOMAIN_SECLABEL_LAST, + VIR_DOMAIN_SECLABEL_LAST }; /* Security configuration for domain */ @@ -353,7 +353,7 @@ enum virDomainHostdevMode { VIR_DOMAIN_HOSTDEV_MODE_SUBSYS, VIR_DOMAIN_HOSTDEV_MODE_CAPABILITIES, - VIR_DOMAIN_HOSTDEV_MODE_LAST, + VIR_DOMAIN_HOSTDEV_MODE_LAST }; enum virDomainHostdevSubsysType { @@ -736,7 +736,7 @@ enum virDomainNetType { VIR_DOMAIN_NET_TYPE_DIRECT, VIR_DOMAIN_NET_TYPE_HOSTDEV, - VIR_DOMAIN_NET_TYPE_LAST, + VIR_DOMAIN_NET_TYPE_LAST }; /* the backend driver used for virtio interfaces */ @@ -745,7 +745,7 @@ enum virDomainNetBackendType { VIR_DOMAIN_NET_BACKEND_TYPE_QEMU, /* userland */ VIR_DOMAIN_NET_BACKEND_TYPE_VHOST, /* kernel */ - VIR_DOMAIN_NET_BACKEND_TYPE_LAST, + VIR_DOMAIN_NET_BACKEND_TYPE_LAST }; /* the TX algorithm used for virtio interfaces */ @@ -754,7 +754,7 @@ enum virDomainNetVirtioTxModeType { VIR_DOMAIN_NET_VIRTIO_TX_MODE_IOTHREAD, VIR_DOMAIN_NET_VIRTIO_TX_MODE_TIMER, - VIR_DOMAIN_NET_VIRTIO_TX_MODE_LAST, + VIR_DOMAIN_NET_VIRTIO_TX_MODE_LAST }; /* link interface states */ @@ -868,7 +868,7 @@ enum virDomainChrDeviceType { VIR_DOMAIN_CHR_DEVICE_TYPE_CONSOLE, VIR_DOMAIN_CHR_DEVICE_TYPE_CHANNEL, - VIR_DOMAIN_CHR_DEVICE_TYPE_LAST, + VIR_DOMAIN_CHR_DEVICE_TYPE_LAST }; enum virDomainChrChannelTargetType { @@ -876,7 +876,7 @@ enum virDomainChrChannelTargetType { VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_GUESTFWD, VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_VIRTIO, - VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_LAST, + VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_LAST }; enum virDomainChrConsoleTargetType { @@ -887,7 +887,7 @@ enum virDomainChrConsoleTargetType { VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_LXC, VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_OPENVZ, - VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_LAST, + VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_LAST }; enum virDomainChrType { @@ -903,7 +903,7 @@ enum virDomainChrType { VIR_DOMAIN_CHR_TYPE_UNIX, VIR_DOMAIN_CHR_TYPE_SPICEVMC, - VIR_DOMAIN_CHR_TYPE_LAST, + VIR_DOMAIN_CHR_TYPE_LAST }; enum virDomainChrTcpProtocol { @@ -912,7 +912,7 @@ enum virDomainChrTcpProtocol { VIR_DOMAIN_CHR_TCP_PROTOCOL_TELNETS, /* secure telnet */ VIR_DOMAIN_CHR_TCP_PROTOCOL_TLS, - VIR_DOMAIN_CHR_TCP_PROTOCOL_LAST, + VIR_DOMAIN_CHR_TCP_PROTOCOL_LAST }; enum virDomainChrSpicevmcName { @@ -920,7 +920,7 @@ enum virDomainChrSpicevmcName { VIR_DOMAIN_CHR_SPICEVMC_SMARTCARD, VIR_DOMAIN_CHR_SPICEVMC_USBREDIR, - VIR_DOMAIN_CHR_SPICEVMC_LAST, + VIR_DOMAIN_CHR_SPICEVMC_LAST }; /* The host side information for a character device. */ @@ -973,7 +973,7 @@ enum virDomainSmartcardType { VIR_DOMAIN_SMARTCARD_TYPE_HOST_CERTIFICATES, VIR_DOMAIN_SMARTCARD_TYPE_PASSTHROUGH, - VIR_DOMAIN_SMARTCARD_TYPE_LAST, + VIR_DOMAIN_SMARTCARD_TYPE_LAST }; # define VIR_DOMAIN_SMARTCARD_NUM_CERTIFICATES 3 @@ -1002,7 +1002,7 @@ enum virDomainInputType { VIR_DOMAIN_INPUT_TYPE_MOUSE, VIR_DOMAIN_INPUT_TYPE_TABLET, - VIR_DOMAIN_INPUT_TYPE_LAST, + VIR_DOMAIN_INPUT_TYPE_LAST }; enum virDomainInputBus { @@ -1110,7 +1110,7 @@ enum virDomainGraphicsType { VIR_DOMAIN_GRAPHICS_TYPE_DESKTOP, VIR_DOMAIN_GRAPHICS_TYPE_SPICE, - VIR_DOMAIN_GRAPHICS_TYPE_LAST, + VIR_DOMAIN_GRAPHICS_TYPE_LAST }; enum virDomainGraphicsAuthConnectedType { @@ -1220,13 +1220,13 @@ enum virDomainGraphicsListenType { VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_ADDRESS, VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_NETWORK, - VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_LAST, + VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_LAST }; enum virDomainHubType { VIR_DOMAIN_HUB_TYPE_USB, - VIR_DOMAIN_HUB_TYPE_LAST, + VIR_DOMAIN_HUB_TYPE_LAST }; typedef struct _virDomainGraphicsListenDef virDomainGraphicsListenDef; @@ -1352,13 +1352,15 @@ enum virDomainBootOrder { VIR_DOMAIN_BOOT_DISK, VIR_DOMAIN_BOOT_NET, - VIR_DOMAIN_BOOT_LAST, + VIR_DOMAIN_BOOT_LAST }; enum virDomainBootMenu { VIR_DOMAIN_BOOT_MENU_DEFAULT = 0, VIR_DOMAIN_BOOT_MENU_ENABLED, VIR_DOMAIN_BOOT_MENU_DISABLED, + + VIR_DOMAIN_BOOT_MENU_LAST }; enum virDomainFeature { @@ -1377,7 +1379,7 @@ enum virDomainApicEoi { VIR_DOMAIN_APIC_EOI_ON, VIR_DOMAIN_APIC_EOI_OFF, - VIR_DOMAIN_APIC_EOI_LAST, + VIR_DOMAIN_APIC_EOI_LAST }; enum virDomainLifecycleAction { @@ -1405,7 +1407,7 @@ enum virDomainPMState { VIR_DOMAIN_PM_STATE_ENABLED, VIR_DOMAIN_PM_STATE_DISABLED, - VIR_DOMAIN_PM_STATE_LAST, + VIR_DOMAIN_PM_STATE_LAST }; enum virDomainBIOSUseserial { @@ -1429,6 +1431,7 @@ struct _virDomainOSDef { char *machine; int nBootDevs; int bootDevs[VIR_DOMAIN_BOOT_LAST]; + /* enum virDomainBootMenu */ int bootmenu; char *init; char **initargv; @@ -1440,6 +1443,7 @@ struct _virDomainOSDef { char *bootloader; char *bootloaderArgs; int smbios_mode; + virDomainBIOSDef bios; }; @@ -1451,7 +1455,7 @@ enum virDomainTimerNameType { VIR_DOMAIN_TIMER_NAME_TSC, VIR_DOMAIN_TIMER_NAME_KVMCLOCK, - VIR_DOMAIN_TIMER_NAME_LAST, + VIR_DOMAIN_TIMER_NAME_LAST }; enum virDomainTimerTrackType { @@ -1459,7 +1463,7 @@ enum virDomainTimerTrackType { VIR_DOMAIN_TIMER_TRACK_GUEST, VIR_DOMAIN_TIMER_TRACK_WALL, - VIR_DOMAIN_TIMER_TRACK_LAST, + VIR_DOMAIN_TIMER_TRACK_LAST }; enum virDomainTimerTickpolicyType { @@ -1468,7 +1472,7 @@ enum virDomainTimerTickpolicyType { VIR_DOMAIN_TIMER_TICKPOLICY_MERGE, VIR_DOMAIN_TIMER_TICKPOLICY_DISCARD, - VIR_DOMAIN_TIMER_TICKPOLICY_LAST, + VIR_DOMAIN_TIMER_TICKPOLICY_LAST }; enum virDomainTimerModeType { @@ -1478,14 +1482,14 @@ enum virDomainTimerModeType { VIR_DOMAIN_TIMER_MODE_PARAVIRT, VIR_DOMAIN_TIMER_MODE_SMPSAFE, - VIR_DOMAIN_TIMER_MODE_LAST, + VIR_DOMAIN_TIMER_MODE_LAST }; enum virDomainCpuPlacementMode { VIR_DOMAIN_CPU_PLACEMENT_MODE_STATIC = 0, VIR_DOMAIN_CPU_PLACEMENT_MODE_AUTO, - VIR_DOMAIN_CPU_PLACEMENT_MODE_LAST, + VIR_DOMAIN_CPU_PLACEMENT_MODE_LAST }; enum virDomainNumatuneMemPlacementMode { @@ -1493,7 +1497,7 @@ enum virDomainNumatuneMemPlacementMode { VIR_DOMAIN_NUMATUNE_MEM_PLACEMENT_MODE_STATIC, VIR_DOMAIN_NUMATUNE_MEM_PLACEMENT_MODE_AUTO, - VIR_DOMAIN_NUMATUNE_MEM_PLACEMENT_MODE_LAST, + VIR_DOMAIN_NUMATUNE_MEM_PLACEMENT_MODE_LAST }; typedef struct _virDomainTimerCatchupDef virDomainTimerCatchupDef; @@ -1527,14 +1531,14 @@ enum virDomainClockOffsetType { VIR_DOMAIN_CLOCK_OFFSET_VARIABLE = 2, VIR_DOMAIN_CLOCK_OFFSET_TIMEZONE = 3, - VIR_DOMAIN_CLOCK_OFFSET_LAST, + VIR_DOMAIN_CLOCK_OFFSET_LAST }; enum virDomainClockBasis { VIR_DOMAIN_CLOCK_BASIS_UTC = 0, VIR_DOMAIN_CLOCK_BASIS_LOCALTIME = 1, - VIR_DOMAIN_CLOCK_BASIS_LAST, + VIR_DOMAIN_CLOCK_BASIS_LAST }; typedef struct _virDomainClockDef virDomainClockDef; @@ -2129,6 +2133,7 @@ VIR_ENUM_DECL(virDomainTaint) VIR_ENUM_DECL(virDomainVirt) VIR_ENUM_DECL(virDomainBoot) +VIR_ENUM_DECL(virDomainBootMenu) VIR_ENUM_DECL(virDomainFeature) VIR_ENUM_DECL(virDomainApicEoi) VIR_ENUM_DECL(virDomainLifecycle) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index ec2e544..be49214 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -280,6 +280,8 @@ virDomainApicEoiTypeToString; virDomainAssignDef; virDomainBlockedReasonTypeFromString; virDomainBlockedReasonTypeToString; +virDomainBootMenuTypeFromString; +virDomainBootMenuTypeToString; virDomainChrConsoleTargetTypeFromString; virDomainChrConsoleTargetTypeToString; virDomainChrDefForeach; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index cbf4aee..f8012ec 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -4879,6 +4879,8 @@ qemuBuildCommandLine(virConnectPtr conn, } if (!def->os.bootloader) { + int boot_nparams = 0; + virBuffer boot_buf = VIR_BUFFER_INITIALIZER; /* * We prefer using explicit bootindex=N parameters for predictable * results even though domain XML doesn't use per device boot elements. @@ -4901,7 +4903,6 @@ qemuBuildCommandLine(virConnectPtr conn, } if (!emitBootindex) { - virBuffer boot_buf = VIR_BUFFER_INITIALIZER; char boot[VIR_DOMAIN_BOOT_LAST+1]; for (i = 0 ; i < def->os.nBootDevs ; i++) { @@ -4925,19 +4926,38 @@ qemuBuildCommandLine(virConnectPtr conn, } boot[def->os.nBootDevs] = '\0'; - virCommandAddArg(cmd, "-boot"); + virBufferAsprintf(&boot_buf, "%s", boot); + boot_nparams++; + } + + if (def->os.bootmenu) { + if (qemuCapsGet(caps, QEMU_CAPS_BOOT_MENU)) { + if (boot_nparams++) + virBufferAddChar(&boot_buf, ','); - if (qemuCapsGet(caps, QEMU_CAPS_BOOT_MENU) && - def->os.bootmenu != VIR_DOMAIN_BOOT_MENU_DEFAULT) { if (def->os.bootmenu == VIR_DOMAIN_BOOT_MENU_ENABLED) - virBufferAsprintf(&boot_buf, "order=%s,menu=on", boot); - else if (def->os.bootmenu == VIR_DOMAIN_BOOT_MENU_DISABLED) - virBufferAsprintf(&boot_buf, "order=%s,menu=off", boot); + virBufferAsprintf(&boot_buf, "menu=on"); + else + virBufferAsprintf(&boot_buf, "menu=off"); } else { - virBufferAdd(&boot_buf, boot, -1); + /* We cannot emit an error when bootmenu is enabled but + * unsupported because of backward compatibility */ + VIR_WARN("bootmenu is enabled but not " + "supported by this QEMU binary"); } - virCommandAddArgBuffer(cmd, &boot_buf); + } + + if (boot_nparams > 0) { + virCommandAddArg(cmd, "-boot"); + + if (boot_nparams < 2 || emitBootindex) { + virCommandAddArgBuffer(cmd, &boot_buf); + } else { + virCommandAddArgFormat(cmd, + "order=%s", + virBufferContentAndReset(&boot_buf)); + } } if (def->os.kernel) @@ -7861,6 +7881,26 @@ error: } +static void +qemuParseCommandLineBootDevs(virDomainDefPtr def, const char *str) { + int n, b = 0; + + for (n = 0 ; str[n] && b < VIR_DOMAIN_BOOT_LAST ; n++) { + if (str[n] == 'a') + def->os.bootDevs[b++] = VIR_DOMAIN_BOOT_FLOPPY; + else if (str[n] == 'c') + def->os.bootDevs[b++] = VIR_DOMAIN_BOOT_DISK; + else if (str[n] == 'd') + def->os.bootDevs[b++] = VIR_DOMAIN_BOOT_CDROM; + else if (str[n] == 'n') + def->os.bootDevs[b++] = VIR_DOMAIN_BOOT_NET; + else if (str[n] == ',') + break; + } + def->os.nBootDevs = b; +} + + /* * Analyse the env and argv settings and reconstruct a * virDomainDefPtr representing these settings as closely @@ -8218,24 +8258,27 @@ virDomainDefPtr qemuParseCommandLine(virCapsPtr caps, if (!(def->os.cmdline = strdup(val))) goto no_memory; } else if (STREQ(arg, "-boot")) { - int n, b = 0; + const char *token = NULL; WANT_VALUE(); - for (n = 0 ; val[n] && b < VIR_DOMAIN_BOOT_LAST ; n++) { - if (val[n] == 'a') - def->os.bootDevs[b++] = VIR_DOMAIN_BOOT_FLOPPY; - else if (val[n] == 'c') - def->os.bootDevs[b++] = VIR_DOMAIN_BOOT_DISK; - else if (val[n] == 'd') - def->os.bootDevs[b++] = VIR_DOMAIN_BOOT_CDROM; - else if (val[n] == 'n') - def->os.bootDevs[b++] = VIR_DOMAIN_BOOT_NET; - else if (val[n] == ',') - break; - } - def->os.nBootDevs = b; - if (strstr(val, "menu=on")) - def->os.bootmenu = 1; + if (!strchr(val, ',')) + qemuParseCommandLineBootDevs(def, val); + else { + token = val; + while (token && *token) { + if (STRPREFIX(token, "order=")) { + token += strlen("order="); + qemuParseCommandLineBootDevs(def, token); + } else if (STRPREFIX(token, "menu=on")) { + def->os.bootmenu = 1; + } + token = strchr(token, ','); + /* This incrementation has to be done here in order to make it + * possible to pass the token pointer properly into the loop */ + if (token) + token++; + } + } } else if (STREQ(arg, "-name")) { char *process; WANT_VALUE(); diff --git a/tests/qemuxml2argvdata/qemuxml2argv-boot-menu-disable-drive-bootindex.args b/tests/qemuxml2argvdata/qemuxml2argv-boot-menu-disable-drive-bootindex.args index 5074e32..75b6b4c 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-boot-menu-disable-drive-bootindex.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-boot-menu-disable-drive-bootindex.args @@ -7,6 +7,7 @@ LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu \ -nodefaults \ -monitor unix:/tmp/test-monitor,server,nowait \ -no-acpi \ +-boot menu=off \ -drive file=/dev/cdrom,if=none,media=cdrom,id=drive-ide0-1-0 \ -device ide-drive,bus=ide.1,unit=0,drive=drive-ide0-1-0,id=ide0-1-0,bootindex=1 \ -usb \