[libvirt] [PATCH 0/5] Boot options cleanup
Remove the -bootloader option from the QEMU driver, for https://bugzilla.redhat.com/show_bug.cgi?id=1176050 as it was only supported by xenner. Remove the -domid option, also xenner-only. Cleanup -boot command line generation. Ján Tomko (5): Remove bootloader option from QEMU Remove code handling the QEMU_CAPS_DOMID capability Make -boot arg generation more readable Rename boot_buf to boot_opts Use virBufferTrim when generating boot options src/qemu/qemu_command.c | 240 ++++++++++----------- src/qemu/qemu_domain.c | 6 + .../qemuxml2argvdata/qemuxml2argv-bootloader.args | 5 - tests/qemuxml2argvdata/qemuxml2argv-bootloader.xml | 27 --- tests/qemuxml2argvdata/qemuxml2argv-input-xen.args | 5 - tests/qemuxml2argvdata/qemuxml2argv-input-xen.xml | 34 --- tests/qemuxml2argvtest.c | 7 +- tests/qemuxml2xmltest.c | 2 - tests/qemuxmlnstest.c | 5 +- 9 files changed, 122 insertions(+), 209 deletions(-) delete mode 100644 tests/qemuxml2argvdata/qemuxml2argv-bootloader.args delete mode 100644 tests/qemuxml2argvdata/qemuxml2argv-bootloader.xml delete mode 100644 tests/qemuxml2argvdata/qemuxml2argv-input-xen.args delete mode 100644 tests/qemuxml2argvdata/qemuxml2argv-input-xen.xml -- 2.0.5
It was only supported by xenner, for which we removed support in commit de9be0a. Remove the code generating this command line option, refuse to parse it and delete the outdated tests. https://bugzilla.redhat.com/show_bug.cgi?id=1176050 --- src/qemu/qemu_command.c | 233 ++++++++++----------- src/qemu/qemu_domain.c | 6 + .../qemuxml2argvdata/qemuxml2argv-bootloader.args | 5 - tests/qemuxml2argvdata/qemuxml2argv-bootloader.xml | 27 --- tests/qemuxml2argvdata/qemuxml2argv-input-xen.args | 5 - tests/qemuxml2argvdata/qemuxml2argv-input-xen.xml | 34 --- tests/qemuxml2argvtest.c | 2 - tests/qemuxml2xmltest.c | 2 - 8 files changed, 120 insertions(+), 194 deletions(-) delete mode 100644 tests/qemuxml2argvdata/qemuxml2argv-bootloader.args delete mode 100644 tests/qemuxml2argvdata/qemuxml2argv-bootloader.xml delete mode 100644 tests/qemuxml2argvdata/qemuxml2argv-input-xen.args delete mode 100644 tests/qemuxml2argvdata/qemuxml2argv-input-xen.xml diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 743d6f0..b522bdc 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -8207,6 +8207,8 @@ qemuBuildCommandLine(virConnectPtr conn, }; virArch hostarch = virArchFromHost(); virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); + virBuffer boot_buf = VIR_BUFFER_INITIALIZER; + int boot_nparams = 0; VIR_DEBUG("conn=%p driver=%p def=%p mon=%p json=%d " "qemuCaps=%p migrateFrom=%s migrateFD=%d " @@ -8728,148 +8730,140 @@ qemuBuildCommandLine(virConnectPtr conn, def->pm.s4 == VIR_TRISTATE_BOOL_NO); } - 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. - * However, we can't use bootindex if boot menu was requested. + /* + * We prefer using explicit bootindex=N parameters for predictable + * results even though domain XML doesn't use per device boot elements. + * However, we can't use bootindex if boot menu was requested. + */ + if (!def->os.nBootDevs) { + /* def->os.nBootDevs is guaranteed to be > 0 unless per-device boot + * configuration is used */ - if (!def->os.nBootDevs) { - /* def->os.nBootDevs is guaranteed to be > 0 unless per-device boot - * configuration is used - */ - if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_BOOTINDEX)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("hypervisor lacks deviceboot feature")); - goto error; - } - emitBootindex = true; - } else if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_BOOTINDEX) && - (def->os.bootmenu != VIR_TRISTATE_BOOL_YES || - !virQEMUCapsGet(qemuCaps, QEMU_CAPS_BOOT_MENU))) { - emitBootindex = true; - } - - if (!emitBootindex) { - char boot[VIR_DOMAIN_BOOT_LAST+1]; - - for (i = 0; i < def->os.nBootDevs; i++) { - switch (def->os.bootDevs[i]) { - case VIR_DOMAIN_BOOT_CDROM: - boot[i] = 'd'; - break; - case VIR_DOMAIN_BOOT_FLOPPY: - boot[i] = 'a'; - break; - case VIR_DOMAIN_BOOT_DISK: - boot[i] = 'c'; - break; - case VIR_DOMAIN_BOOT_NET: - boot[i] = 'n'; - break; - default: - boot[i] = 'c'; - break; - } - } - boot[def->os.nBootDevs] = '\0'; - - virBufferAsprintf(&boot_buf, "%s", boot); - boot_nparams++; + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_BOOTINDEX)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("hypervisor lacks deviceboot feature")); + goto error; } + emitBootindex = true; + } else if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_BOOTINDEX) && + (def->os.bootmenu != VIR_TRISTATE_BOOL_YES || + !virQEMUCapsGet(qemuCaps, QEMU_CAPS_BOOT_MENU))) { + emitBootindex = true; + } - if (def->os.bootmenu) { - if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_BOOT_MENU)) { - if (boot_nparams++) - virBufferAddChar(&boot_buf, ','); + if (!emitBootindex) { + char boot[VIR_DOMAIN_BOOT_LAST+1]; - if (def->os.bootmenu == VIR_TRISTATE_BOOL_YES) - virBufferAddLit(&boot_buf, "menu=on"); - else - virBufferAddLit(&boot_buf, "menu=off"); - } else { - /* 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"); + for (i = 0; i < def->os.nBootDevs; i++) { + switch (def->os.bootDevs[i]) { + case VIR_DOMAIN_BOOT_CDROM: + boot[i] = 'd'; + break; + case VIR_DOMAIN_BOOT_FLOPPY: + boot[i] = 'a'; + break; + case VIR_DOMAIN_BOOT_DISK: + boot[i] = 'c'; + break; + case VIR_DOMAIN_BOOT_NET: + boot[i] = 'n'; + break; + default: + boot[i] = 'c'; + break; } } + boot[def->os.nBootDevs] = '\0'; - if (def->os.bios.rt_set) { - if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_REBOOT_TIMEOUT)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("reboot timeout is not supported " - "by this QEMU binary")); - virBufferFreeAndReset(&boot_buf); - goto error; - } + virBufferAsprintf(&boot_buf, "%s", boot); + boot_nparams++; + } + if (def->os.bootmenu) { + if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_BOOT_MENU)) { if (boot_nparams++) virBufferAddChar(&boot_buf, ','); - virBufferAsprintf(&boot_buf, - "reboot-timeout=%d", - def->os.bios.rt_delay); + if (def->os.bootmenu == VIR_TRISTATE_BOOL_YES) + virBufferAddLit(&boot_buf, "menu=on"); + else + virBufferAddLit(&boot_buf, "menu=off"); + } else { + /* 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"); } + } - if (def->os.bm_timeout_set) { - if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_SPLASH_TIMEOUT)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("splash timeout is not supported " - "by this QEMU binary")); - virBufferFreeAndReset(&boot_buf); - goto error; - } + if (def->os.bios.rt_set) { + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_REBOOT_TIMEOUT)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("reboot timeout is not supported " + "by this QEMU binary")); + goto error; + } - if (boot_nparams++) - virBufferAddChar(&boot_buf, ','); + if (boot_nparams++) + virBufferAddChar(&boot_buf, ','); - virBufferAsprintf(&boot_buf, "splash-time=%u", def->os.bm_timeout); - } + virBufferAsprintf(&boot_buf, + "reboot-timeout=%d", + def->os.bios.rt_delay); + } - if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_BOOT_STRICT)) { - if (boot_nparams++) - virBufferAddChar(&boot_buf, ','); - virBufferAddLit(&boot_buf, "strict=on"); + if (def->os.bm_timeout_set) { + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_SPLASH_TIMEOUT)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("splash timeout is not supported " + "by this QEMU binary")); + goto error; } - if (boot_nparams > 0) { - virCommandAddArg(cmd, "-boot"); + if (boot_nparams++) + virBufferAddChar(&boot_buf, ','); - if (virBufferCheckError(&boot_buf) < 0) - goto error; + virBufferAsprintf(&boot_buf, "splash-time=%u", def->os.bm_timeout); + } - if (boot_nparams < 2 || emitBootindex) { - virCommandAddArgBuffer(cmd, &boot_buf); - virBufferFreeAndReset(&boot_buf); - } else { - char *str = virBufferContentAndReset(&boot_buf); - virCommandAddArgFormat(cmd, - "order=%s", - str); - VIR_FREE(str); - } + if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_BOOT_STRICT)) { + if (boot_nparams++) + virBufferAddChar(&boot_buf, ','); + virBufferAddLit(&boot_buf, "strict=on"); + } + + if (boot_nparams > 0) { + virCommandAddArg(cmd, "-boot"); + + if (virBufferCheckError(&boot_buf) < 0) + goto error; + + if (boot_nparams < 2 || emitBootindex) { + virCommandAddArgBuffer(cmd, &boot_buf); + virBufferFreeAndReset(&boot_buf); + } else { + char *str = virBufferContentAndReset(&boot_buf); + virCommandAddArgFormat(cmd, + "order=%s", + str); + VIR_FREE(str); } + } - if (def->os.kernel) - virCommandAddArgList(cmd, "-kernel", def->os.kernel, NULL); - if (def->os.initrd) - virCommandAddArgList(cmd, "-initrd", def->os.initrd, NULL); - if (def->os.cmdline) - virCommandAddArgList(cmd, "-append", def->os.cmdline, NULL); - if (def->os.dtb) { - if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DTB)) { - virCommandAddArgList(cmd, "-dtb", def->os.dtb, NULL); - } else { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("dtb is not supported with this QEMU binary")); - goto error; - } + if (def->os.kernel) + virCommandAddArgList(cmd, "-kernel", def->os.kernel, NULL); + if (def->os.initrd) + virCommandAddArgList(cmd, "-initrd", def->os.initrd, NULL); + if (def->os.cmdline) + virCommandAddArgList(cmd, "-append", def->os.cmdline, NULL); + if (def->os.dtb) { + if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DTB)) { + virCommandAddArgList(cmd, "-dtb", def->os.dtb, NULL); + } else { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("dtb is not supported with this QEMU binary")); + goto error; } - } else { - virCommandAddArgList(cmd, "-bootloader", def->os.bootloader, NULL); } for (i = 0; i < def->ncontrollers; i++) { @@ -10344,6 +10338,7 @@ qemuBuildCommandLine(virConnectPtr conn, return cmd; error: + virBufferFreeAndReset(&boot_buf); virObjectUnref(cfg); /* free up any resources in the network driver * but don't overwrite the original error */ diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 99c46d4..c3838b8 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -925,6 +925,12 @@ qemuDomainDefPostParse(virDomainDefPtr def, bool addDefaultUSBKBD = false; bool addDefaultUSBMouse = false; + if (def->os.bootloader || def->os.bootloaderArgs) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("bootloader is not supported by QEMU")); + return -1; + } + /* check for emulator and create a default one if needed */ if (!def->emulator && !(def->emulator = virDomainDefGetDefaultEmulator(def, caps))) diff --git a/tests/qemuxml2argvdata/qemuxml2argv-bootloader.args b/tests/qemuxml2argvdata/qemuxml2argv-bootloader.args deleted file mode 100644 index d7c4139..0000000 --- a/tests/qemuxml2argvdata/qemuxml2argv-bootloader.args +++ /dev/null @@ -1,5 +0,0 @@ -LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test QEMU_AUDIO_DRV=none \ -/usr/bin/xenner -S \ --M xenner -m 214 -smp 1 -domid 6 -nographic -monitor unix:/tmp/test-monitor,\ -server,nowait -no-acpi -bootloader /usr/bin/pygrub -usb -cdrom /dev/cdrom -net none \ --serial none -parallel none diff --git a/tests/qemuxml2argvdata/qemuxml2argv-bootloader.xml b/tests/qemuxml2argvdata/qemuxml2argv-bootloader.xml deleted file mode 100644 index 3b4c2bd..0000000 --- a/tests/qemuxml2argvdata/qemuxml2argv-bootloader.xml +++ /dev/null @@ -1,27 +0,0 @@ -<domain type='kvm'> - <name>QEMUGuest1</name> - <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> - <memory unit='KiB'>219100</memory> - <currentMemory unit='KiB'>219100</currentMemory> - <vcpu placement='static'>1</vcpu> - <bootloader>/usr/bin/pygrub</bootloader> - <os> - <type arch='x86_64' machine='xenner'>xen</type> - </os> - <clock offset='utc'/> - <on_poweroff>destroy</on_poweroff> - <on_reboot>restart</on_reboot> - <on_crash>destroy</on_crash> - <devices> - <emulator>/usr/bin/xenner</emulator> - <disk type='block' device='cdrom'> - <source dev='/dev/cdrom'/> - <target dev='hdc' bus='ide'/> - <readonly/> - <address type='drive' controller='0' bus='1' target='0' unit='0'/> - </disk> - <controller type='usb' index='0'/> - <controller type='ide' index='0'/> - <memballoon model='virtio'/> - </devices> -</domain> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-input-xen.args b/tests/qemuxml2argvdata/qemuxml2argv-input-xen.args deleted file mode 100644 index 8c336bd..0000000 --- a/tests/qemuxml2argvdata/qemuxml2argv-input-xen.args +++ /dev/null @@ -1,5 +0,0 @@ -LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test QEMU_AUDIO_DRV=none \ -/usr/bin/xenner -S -M xenner -m 214 -smp 1 -domid 6 -monitor \ -unix:/tmp/test-monitor,server,nowait -no-acpi -bootloader /foo -usb -hda \ -/dev/HostVG/QEMUGuest1 -net none -serial none -parallel none -vnc \ -127.0.0.1:3 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-input-xen.xml b/tests/qemuxml2argvdata/qemuxml2argv-input-xen.xml deleted file mode 100644 index a19fe4f..0000000 --- a/tests/qemuxml2argvdata/qemuxml2argv-input-xen.xml +++ /dev/null @@ -1,34 +0,0 @@ -<domain type='kvm'> - <name>QEMUGuest1</name> - <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> - <memory unit='KiB'>219136</memory> - <currentMemory unit='KiB'>219136</currentMemory> - <vcpu placement='static'>1</vcpu> - <bootloader>/foo</bootloader> - <os> - <type arch='x86_64' machine='xenner'>xen</type> - </os> - <clock offset='utc'/> - <on_poweroff>destroy</on_poweroff> - <on_reboot>restart</on_reboot> - <on_crash>destroy</on_crash> - <devices> - <emulator>/usr/bin/xenner</emulator> - <disk type='block' device='disk'> - <source dev='/dev/HostVG/QEMUGuest1'/> - <target dev='hda' bus='ide'/> - <address type='drive' controller='0' bus='0' target='0' unit='0'/> - </disk> - <controller type='usb' index='0'/> - <controller type='ide' index='0'/> - <input type='mouse' bus='xen'/> - <input type='keyboard' bus='xen'/> - <graphics type='vnc' port='5903' autoport='no' listen='127.0.0.1'> - <listen type='address' address='127.0.0.1'/> - </graphics> - <video> - <model type='xen' vram='4096' heads='1'/> - </video> - <memballoon model='virtio'/> - </devices> -</domain> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 39ed66b..1071dba 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -648,7 +648,6 @@ mymain(void) QEMU_CAPS_DEVICE, QEMU_CAPS_DRIVE, QEMU_CAPS_DRIVE_BOOT, QEMU_CAPS_BOOTINDEX, QEMU_CAPS_BOOT_STRICT, QEMU_CAPS_VIRTIO_BLK_SCSI, QEMU_CAPS_VIRTIO_BLK_SG_IO); - DO_TEST("bootloader", QEMU_CAPS_DOMID, QEMU_CAPS_KVM); DO_TEST("reboot-timeout-disabled", QEMU_CAPS_REBOOT_TIMEOUT); DO_TEST("reboot-timeout-enabled", QEMU_CAPS_REBOOT_TIMEOUT); @@ -971,7 +970,6 @@ mymain(void) DO_TEST("input-usbmouse", NONE); DO_TEST("input-usbtablet", NONE); - DO_TEST("input-xen", QEMU_CAPS_DOMID, QEMU_CAPS_KVM, QEMU_CAPS_VNC); DO_TEST("misc-acpi", NONE); DO_TEST("misc-disable-s3", QEMU_CAPS_DISABLE_S3); DO_TEST("misc-disable-suspends", QEMU_CAPS_DISABLE_S3, QEMU_CAPS_DISABLE_S4); diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index 99d4629..58917c3 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -178,7 +178,6 @@ mymain(void) DO_TEST("boot-menu-disable"); DO_TEST_DIFFERENT("boot-menu-disable-with-timeout"); DO_TEST("boot-order"); - DO_TEST("bootloader"); DO_TEST("reboot-timeout-enabled"); DO_TEST("reboot-timeout-disabled"); @@ -259,7 +258,6 @@ mymain(void) DO_TEST("graphics-spice-qxl-vga"); DO_TEST("input-usbmouse"); DO_TEST("input-usbtablet"); - DO_TEST("input-xen"); DO_TEST("misc-acpi"); DO_TEST("misc-disable-s3"); DO_TEST("misc-disable-suspends"); -- 2.0.5
On 02/18/2015 11:39 AM, Ján Tomko wrote:
It was only supported by xenner, for which we removed support in commit de9be0a.
Remove the code generating this command line option, refuse to parse it and delete the outdated tests.
https://bugzilla.redhat.com/show_bug.cgi?id=1176050 --- src/qemu/qemu_command.c | 233 ++++++++++----------- src/qemu/qemu_domain.c | 6 + .../qemuxml2argvdata/qemuxml2argv-bootloader.args | 5 - tests/qemuxml2argvdata/qemuxml2argv-bootloader.xml | 27 --- tests/qemuxml2argvdata/qemuxml2argv-input-xen.args | 5 - tests/qemuxml2argvdata/qemuxml2argv-input-xen.xml | 34 --- tests/qemuxml2argvtest.c | 2 - tests/qemuxml2xmltest.c | 2 - 8 files changed, 120 insertions(+), 194 deletions(-) delete mode 100644 tests/qemuxml2argvdata/qemuxml2argv-bootloader.args delete mode 100644 tests/qemuxml2argvdata/qemuxml2argv-bootloader.xml delete mode 100644 tests/qemuxml2argvdata/qemuxml2argv-input-xen.args delete mode 100644 tests/qemuxml2argvdata/qemuxml2argv-input-xen.xml
ACK John FWIW: Just for completeness... Originally added by commit id '763a59d8'
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 743d6f0..b522bdc 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -8207,6 +8207,8 @@ qemuBuildCommandLine(virConnectPtr conn, }; virArch hostarch = virArchFromHost(); virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); + virBuffer boot_buf = VIR_BUFFER_INITIALIZER; + int boot_nparams = 0;
VIR_DEBUG("conn=%p driver=%p def=%p mon=%p json=%d " "qemuCaps=%p migrateFrom=%s migrateFD=%d " @@ -8728,148 +8730,140 @@ qemuBuildCommandLine(virConnectPtr conn, def->pm.s4 == VIR_TRISTATE_BOOL_NO); }
- 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. - * However, we can't use bootindex if boot menu was requested. + /* + * We prefer using explicit bootindex=N parameters for predictable + * results even though domain XML doesn't use per device boot elements. + * However, we can't use bootindex if boot menu was requested. + */ + if (!def->os.nBootDevs) { + /* def->os.nBootDevs is guaranteed to be > 0 unless per-device boot + * configuration is used */ - if (!def->os.nBootDevs) { - /* def->os.nBootDevs is guaranteed to be > 0 unless per-device boot - * configuration is used - */ - if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_BOOTINDEX)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("hypervisor lacks deviceboot feature")); - goto error; - } - emitBootindex = true; - } else if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_BOOTINDEX) && - (def->os.bootmenu != VIR_TRISTATE_BOOL_YES || - !virQEMUCapsGet(qemuCaps, QEMU_CAPS_BOOT_MENU))) { - emitBootindex = true; - } - - if (!emitBootindex) { - char boot[VIR_DOMAIN_BOOT_LAST+1]; - - for (i = 0; i < def->os.nBootDevs; i++) { - switch (def->os.bootDevs[i]) { - case VIR_DOMAIN_BOOT_CDROM: - boot[i] = 'd'; - break; - case VIR_DOMAIN_BOOT_FLOPPY: - boot[i] = 'a'; - break; - case VIR_DOMAIN_BOOT_DISK: - boot[i] = 'c'; - break; - case VIR_DOMAIN_BOOT_NET: - boot[i] = 'n'; - break; - default: - boot[i] = 'c'; - break; - } - } - boot[def->os.nBootDevs] = '\0'; - - virBufferAsprintf(&boot_buf, "%s", boot); - boot_nparams++; + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_BOOTINDEX)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("hypervisor lacks deviceboot feature")); + goto error; } + emitBootindex = true; + } else if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_BOOTINDEX) && + (def->os.bootmenu != VIR_TRISTATE_BOOL_YES || + !virQEMUCapsGet(qemuCaps, QEMU_CAPS_BOOT_MENU))) { + emitBootindex = true; + }
- if (def->os.bootmenu) { - if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_BOOT_MENU)) { - if (boot_nparams++) - virBufferAddChar(&boot_buf, ','); + if (!emitBootindex) { + char boot[VIR_DOMAIN_BOOT_LAST+1];
- if (def->os.bootmenu == VIR_TRISTATE_BOOL_YES) - virBufferAddLit(&boot_buf, "menu=on"); - else - virBufferAddLit(&boot_buf, "menu=off"); - } else { - /* 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"); + for (i = 0; i < def->os.nBootDevs; i++) { + switch (def->os.bootDevs[i]) { + case VIR_DOMAIN_BOOT_CDROM: + boot[i] = 'd'; + break; + case VIR_DOMAIN_BOOT_FLOPPY: + boot[i] = 'a'; + break; + case VIR_DOMAIN_BOOT_DISK: + boot[i] = 'c'; + break; + case VIR_DOMAIN_BOOT_NET: + boot[i] = 'n'; + break; + default: + boot[i] = 'c'; + break; } } + boot[def->os.nBootDevs] = '\0';
- if (def->os.bios.rt_set) { - if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_REBOOT_TIMEOUT)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("reboot timeout is not supported " - "by this QEMU binary")); - virBufferFreeAndReset(&boot_buf); - goto error; - } + virBufferAsprintf(&boot_buf, "%s", boot); + boot_nparams++; + }
+ if (def->os.bootmenu) { + if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_BOOT_MENU)) { if (boot_nparams++) virBufferAddChar(&boot_buf, ',');
- virBufferAsprintf(&boot_buf, - "reboot-timeout=%d", - def->os.bios.rt_delay); + if (def->os.bootmenu == VIR_TRISTATE_BOOL_YES) + virBufferAddLit(&boot_buf, "menu=on"); + else + virBufferAddLit(&boot_buf, "menu=off"); + } else { + /* 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"); } + }
- if (def->os.bm_timeout_set) { - if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_SPLASH_TIMEOUT)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("splash timeout is not supported " - "by this QEMU binary")); - virBufferFreeAndReset(&boot_buf); - goto error; - } + if (def->os.bios.rt_set) { + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_REBOOT_TIMEOUT)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("reboot timeout is not supported " + "by this QEMU binary")); + goto error; + }
- if (boot_nparams++) - virBufferAddChar(&boot_buf, ','); + if (boot_nparams++) + virBufferAddChar(&boot_buf, ',');
- virBufferAsprintf(&boot_buf, "splash-time=%u", def->os.bm_timeout); - } + virBufferAsprintf(&boot_buf, + "reboot-timeout=%d", + def->os.bios.rt_delay); + }
- if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_BOOT_STRICT)) { - if (boot_nparams++) - virBufferAddChar(&boot_buf, ','); - virBufferAddLit(&boot_buf, "strict=on"); + if (def->os.bm_timeout_set) { + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_SPLASH_TIMEOUT)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("splash timeout is not supported " + "by this QEMU binary")); + goto error; }
- if (boot_nparams > 0) { - virCommandAddArg(cmd, "-boot"); + if (boot_nparams++) + virBufferAddChar(&boot_buf, ',');
- if (virBufferCheckError(&boot_buf) < 0) - goto error; + virBufferAsprintf(&boot_buf, "splash-time=%u", def->os.bm_timeout); + }
- if (boot_nparams < 2 || emitBootindex) { - virCommandAddArgBuffer(cmd, &boot_buf); - virBufferFreeAndReset(&boot_buf); - } else { - char *str = virBufferContentAndReset(&boot_buf); - virCommandAddArgFormat(cmd, - "order=%s", - str); - VIR_FREE(str); - } + if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_BOOT_STRICT)) { + if (boot_nparams++) + virBufferAddChar(&boot_buf, ','); + virBufferAddLit(&boot_buf, "strict=on"); + } + + if (boot_nparams > 0) { + virCommandAddArg(cmd, "-boot"); + + if (virBufferCheckError(&boot_buf) < 0) + goto error; + + if (boot_nparams < 2 || emitBootindex) { + virCommandAddArgBuffer(cmd, &boot_buf); + virBufferFreeAndReset(&boot_buf); + } else { + char *str = virBufferContentAndReset(&boot_buf); + virCommandAddArgFormat(cmd, + "order=%s", + str); + VIR_FREE(str); } + }
- if (def->os.kernel) - virCommandAddArgList(cmd, "-kernel", def->os.kernel, NULL); - if (def->os.initrd) - virCommandAddArgList(cmd, "-initrd", def->os.initrd, NULL); - if (def->os.cmdline) - virCommandAddArgList(cmd, "-append", def->os.cmdline, NULL); - if (def->os.dtb) { - if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DTB)) { - virCommandAddArgList(cmd, "-dtb", def->os.dtb, NULL); - } else { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("dtb is not supported with this QEMU binary")); - goto error; - } + if (def->os.kernel) + virCommandAddArgList(cmd, "-kernel", def->os.kernel, NULL); + if (def->os.initrd) + virCommandAddArgList(cmd, "-initrd", def->os.initrd, NULL); + if (def->os.cmdline) + virCommandAddArgList(cmd, "-append", def->os.cmdline, NULL); + if (def->os.dtb) { + if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DTB)) { + virCommandAddArgList(cmd, "-dtb", def->os.dtb, NULL); + } else { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("dtb is not supported with this QEMU binary")); + goto error; } - } else { - virCommandAddArgList(cmd, "-bootloader", def->os.bootloader, NULL); }
for (i = 0; i < def->ncontrollers; i++) { @@ -10344,6 +10338,7 @@ qemuBuildCommandLine(virConnectPtr conn, return cmd;
error: + virBufferFreeAndReset(&boot_buf); virObjectUnref(cfg); /* free up any resources in the network driver * but don't overwrite the original error */ diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 99c46d4..c3838b8 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -925,6 +925,12 @@ qemuDomainDefPostParse(virDomainDefPtr def, bool addDefaultUSBKBD = false; bool addDefaultUSBMouse = false;
+ if (def->os.bootloader || def->os.bootloaderArgs) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("bootloader is not supported by QEMU")); + return -1; + } + /* check for emulator and create a default one if needed */ if (!def->emulator && !(def->emulator = virDomainDefGetDefaultEmulator(def, caps))) diff --git a/tests/qemuxml2argvdata/qemuxml2argv-bootloader.args b/tests/qemuxml2argvdata/qemuxml2argv-bootloader.args deleted file mode 100644 index d7c4139..0000000 --- a/tests/qemuxml2argvdata/qemuxml2argv-bootloader.args +++ /dev/null @@ -1,5 +0,0 @@ -LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test QEMU_AUDIO_DRV=none \ -/usr/bin/xenner -S \ --M xenner -m 214 -smp 1 -domid 6 -nographic -monitor unix:/tmp/test-monitor,\ -server,nowait -no-acpi -bootloader /usr/bin/pygrub -usb -cdrom /dev/cdrom -net none \ --serial none -parallel none diff --git a/tests/qemuxml2argvdata/qemuxml2argv-bootloader.xml b/tests/qemuxml2argvdata/qemuxml2argv-bootloader.xml deleted file mode 100644 index 3b4c2bd..0000000 --- a/tests/qemuxml2argvdata/qemuxml2argv-bootloader.xml +++ /dev/null @@ -1,27 +0,0 @@ -<domain type='kvm'> - <name>QEMUGuest1</name> - <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> - <memory unit='KiB'>219100</memory> - <currentMemory unit='KiB'>219100</currentMemory> - <vcpu placement='static'>1</vcpu> - <bootloader>/usr/bin/pygrub</bootloader> - <os> - <type arch='x86_64' machine='xenner'>xen</type> - </os> - <clock offset='utc'/> - <on_poweroff>destroy</on_poweroff> - <on_reboot>restart</on_reboot> - <on_crash>destroy</on_crash> - <devices> - <emulator>/usr/bin/xenner</emulator> - <disk type='block' device='cdrom'> - <source dev='/dev/cdrom'/> - <target dev='hdc' bus='ide'/> - <readonly/> - <address type='drive' controller='0' bus='1' target='0' unit='0'/> - </disk> - <controller type='usb' index='0'/> - <controller type='ide' index='0'/> - <memballoon model='virtio'/> - </devices> -</domain> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-input-xen.args b/tests/qemuxml2argvdata/qemuxml2argv-input-xen.args deleted file mode 100644 index 8c336bd..0000000 --- a/tests/qemuxml2argvdata/qemuxml2argv-input-xen.args +++ /dev/null @@ -1,5 +0,0 @@ -LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test QEMU_AUDIO_DRV=none \ -/usr/bin/xenner -S -M xenner -m 214 -smp 1 -domid 6 -monitor \ -unix:/tmp/test-monitor,server,nowait -no-acpi -bootloader /foo -usb -hda \ -/dev/HostVG/QEMUGuest1 -net none -serial none -parallel none -vnc \ -127.0.0.1:3 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-input-xen.xml b/tests/qemuxml2argvdata/qemuxml2argv-input-xen.xml deleted file mode 100644 index a19fe4f..0000000 --- a/tests/qemuxml2argvdata/qemuxml2argv-input-xen.xml +++ /dev/null @@ -1,34 +0,0 @@ -<domain type='kvm'> - <name>QEMUGuest1</name> - <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> - <memory unit='KiB'>219136</memory> - <currentMemory unit='KiB'>219136</currentMemory> - <vcpu placement='static'>1</vcpu> - <bootloader>/foo</bootloader> - <os> - <type arch='x86_64' machine='xenner'>xen</type> - </os> - <clock offset='utc'/> - <on_poweroff>destroy</on_poweroff> - <on_reboot>restart</on_reboot> - <on_crash>destroy</on_crash> - <devices> - <emulator>/usr/bin/xenner</emulator> - <disk type='block' device='disk'> - <source dev='/dev/HostVG/QEMUGuest1'/> - <target dev='hda' bus='ide'/> - <address type='drive' controller='0' bus='0' target='0' unit='0'/> - </disk> - <controller type='usb' index='0'/> - <controller type='ide' index='0'/> - <input type='mouse' bus='xen'/> - <input type='keyboard' bus='xen'/> - <graphics type='vnc' port='5903' autoport='no' listen='127.0.0.1'> - <listen type='address' address='127.0.0.1'/> - </graphics> - <video> - <model type='xen' vram='4096' heads='1'/> - </video> - <memballoon model='virtio'/> - </devices> -</domain> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 39ed66b..1071dba 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -648,7 +648,6 @@ mymain(void) QEMU_CAPS_DEVICE, QEMU_CAPS_DRIVE, QEMU_CAPS_DRIVE_BOOT, QEMU_CAPS_BOOTINDEX, QEMU_CAPS_BOOT_STRICT, QEMU_CAPS_VIRTIO_BLK_SCSI, QEMU_CAPS_VIRTIO_BLK_SG_IO); - DO_TEST("bootloader", QEMU_CAPS_DOMID, QEMU_CAPS_KVM);
DO_TEST("reboot-timeout-disabled", QEMU_CAPS_REBOOT_TIMEOUT); DO_TEST("reboot-timeout-enabled", QEMU_CAPS_REBOOT_TIMEOUT); @@ -971,7 +970,6 @@ mymain(void)
DO_TEST("input-usbmouse", NONE); DO_TEST("input-usbtablet", NONE); - DO_TEST("input-xen", QEMU_CAPS_DOMID, QEMU_CAPS_KVM, QEMU_CAPS_VNC); DO_TEST("misc-acpi", NONE); DO_TEST("misc-disable-s3", QEMU_CAPS_DISABLE_S3); DO_TEST("misc-disable-suspends", QEMU_CAPS_DISABLE_S3, QEMU_CAPS_DISABLE_S4); diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index 99d4629..58917c3 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -178,7 +178,6 @@ mymain(void) DO_TEST("boot-menu-disable"); DO_TEST_DIFFERENT("boot-menu-disable-with-timeout"); DO_TEST("boot-order"); - DO_TEST("bootloader");
DO_TEST("reboot-timeout-enabled"); DO_TEST("reboot-timeout-disabled"); @@ -259,7 +258,6 @@ mymain(void) DO_TEST("graphics-spice-qxl-vga"); DO_TEST("input-usbmouse"); DO_TEST("input-usbtablet"); - DO_TEST("input-xen"); DO_TEST("misc-acpi"); DO_TEST("misc-disable-s3"); DO_TEST("misc-disable-suspends");
This option is xenner-only, and we dropped support for xenner in commit de9be0a. --- src/qemu/qemu_command.c | 5 +---- tests/qemuxml2argvtest.c | 5 +---- tests/qemuxmlnstest.c | 5 +---- 3 files changed, 3 insertions(+), 12 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index b522bdc..3658d5f 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -8408,10 +8408,7 @@ qemuBuildCommandLine(virConnectPtr conn, if (def->virtType == VIR_DOMAIN_VIRT_XEN || STREQ(def->os.type, "xen") || STREQ(def->os.type, "linux")) { - if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DOMID)) { - virCommandAddArg(cmd, "-domid"); - virCommandAddArgFormat(cmd, "%d", def->id); - } else if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_XEN_DOMID)) { + if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_XEN_DOMID)) { virCommandAddArg(cmd, "-xen-attach"); virCommandAddArg(cmd, "-xen-domid"); virCommandAddArgFormat(cmd, "%d", def->id); diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 1071dba..ba1813c 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -289,10 +289,7 @@ static int testCompareXMLToArgvFiles(const char *xml, goto out; } - if (virQEMUCapsGet(extraFlags, QEMU_CAPS_DOMID)) - vmdef->id = 6; - else - vmdef->id = -1; + vmdef->id = -1; memset(&monitor_chr, 0, sizeof(monitor_chr)); monitor_chr.type = VIR_DOMAIN_CHR_TYPE_UNIX; diff --git a/tests/qemuxmlnstest.c b/tests/qemuxmlnstest.c index a068135..cc290cd 100644 --- a/tests/qemuxmlnstest.c +++ b/tests/qemuxmlnstest.c @@ -86,10 +86,7 @@ static int testCompareXMLToArgvFiles(const char *xml, goto fail; } - if (virQEMUCapsGet(extraFlags, QEMU_CAPS_DOMID)) - vmdef->id = 6; - else - vmdef->id = -1; + vmdef->id = -1; memset(&monitor_chr, 0, sizeof(monitor_chr)); monitor_chr.type = VIR_DOMAIN_CHR_TYPE_UNIX; -- 2.0.5
On 02/18/2015 11:39 AM, Ján Tomko wrote:
This option is xenner-only, and we dropped support for xenner in commit de9be0a. --- src/qemu/qemu_command.c | 5 +---- tests/qemuxml2argvtest.c | 5 +---- tests/qemuxmlnstest.c | 5 +---- 3 files changed, 3 insertions(+), 12 deletions(-)
ACK, John FWIW: Originally added by 'b81a7ece'
If we combine the boot order on the command line with other boot options, we prepend order= in front of it. Instead of checking if the number of added arguments is between 0 and 2, separate the buffers for boot order and options and prepend boot order only if both buffers are not empty. --- src/qemu/qemu_command.c | 37 ++++++++++++++++++++++--------------- 1 file changed, 22 insertions(+), 15 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 3658d5f..f13dbda 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -8208,6 +8208,8 @@ qemuBuildCommandLine(virConnectPtr conn, virArch hostarch = virArchFromHost(); virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); virBuffer boot_buf = VIR_BUFFER_INITIALIZER; + virBuffer boot_order = VIR_BUFFER_INITIALIZER; + char *boot_order_str = NULL, *boot_opts_str = NULL; int boot_nparams = 0; VIR_DEBUG("conn=%p driver=%p def=%p mon=%p json=%d " @@ -8772,10 +8774,12 @@ qemuBuildCommandLine(virConnectPtr conn, } boot[def->os.nBootDevs] = '\0'; - virBufferAsprintf(&boot_buf, "%s", boot); - boot_nparams++; + virBufferAsprintf(&boot_order, "%s", boot); } + if (virBufferCheckError(&boot_order) < 0) + goto error; + if (def->os.bootmenu) { if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_BOOT_MENU)) { if (boot_nparams++) @@ -8829,23 +8833,25 @@ qemuBuildCommandLine(virConnectPtr conn, virBufferAddLit(&boot_buf, "strict=on"); } - if (boot_nparams > 0) { - virCommandAddArg(cmd, "-boot"); + if (virBufferCheckError(&boot_buf) < 0) + goto error; - if (virBufferCheckError(&boot_buf) < 0) - goto error; + boot_order_str = virBufferContentAndReset(&boot_order); + boot_opts_str = virBufferContentAndReset(&boot_buf); + if (boot_order_str || boot_opts_str) { + virCommandAddArg(cmd, "-boot"); - if (boot_nparams < 2 || emitBootindex) { - virCommandAddArgBuffer(cmd, &boot_buf); - virBufferFreeAndReset(&boot_buf); - } else { - char *str = virBufferContentAndReset(&boot_buf); - virCommandAddArgFormat(cmd, - "order=%s", - str); - VIR_FREE(str); + if (boot_order_str && boot_opts_str) { + virCommandAddArgFormat(cmd, "order=%s,%s", + boot_order_str, boot_opts_str); + } else if (boot_order_str) { + virCommandAddArg(cmd, boot_order_str); + } else if (boot_opts_str) { + virCommandAddArg(cmd, boot_opts_str); } } + VIR_FREE(boot_order_str); + VIR_FREE(boot_opts_str); if (def->os.kernel) virCommandAddArgList(cmd, "-kernel", def->os.kernel, NULL); @@ -10335,6 +10341,7 @@ qemuBuildCommandLine(virConnectPtr conn, return cmd; error: + virBufferFreeAndReset(&boot_order); virBufferFreeAndReset(&boot_buf); virObjectUnref(cfg); /* free up any resources in the network driver -- 2.0.5
On 02/18/2015 11:39 AM, Ján Tomko wrote:
If we combine the boot order on the command line with other boot options, we prepend order= in front of it.
Instead of checking if the number of added arguments is between 0 and 2, separate the buffers for boot order and options and prepend boot order only if both buffers are not empty. --- src/qemu/qemu_command.c | 37 ++++++++++++++++++++++--------------- 1 file changed, 22 insertions(+), 15 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 3658d5f..f13dbda 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -8208,6 +8208,8 @@ qemuBuildCommandLine(virConnectPtr conn, virArch hostarch = virArchFromHost(); virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); virBuffer boot_buf = VIR_BUFFER_INITIALIZER; + virBuffer boot_order = VIR_BUFFER_INITIALIZER; + char *boot_order_str = NULL, *boot_opts_str = NULL; int boot_nparams = 0;
VIR_DEBUG("conn=%p driver=%p def=%p mon=%p json=%d " @@ -8772,10 +8774,12 @@ qemuBuildCommandLine(virConnectPtr conn, } boot[def->os.nBootDevs] = '\0';
- virBufferAsprintf(&boot_buf, "%s", boot); - boot_nparams++; + virBufferAsprintf(&boot_order, "%s", boot); }
+ if (virBufferCheckError(&boot_order) < 0) + goto error; + ^^^ Since the only place to add items is inside the "if (!emitBootindex)",
ACK - there's a note below which could be implemented or not. John then this check can move inside the if statement. Probably could move the other boot_order stuff inside here too including setting the boot_order_str... You could then go back to just one boot_buf, but I see in patch 4 & 5 you rename boot_buf... this is fine, just was typing and thinking as usual...
if (def->os.bootmenu) { if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_BOOT_MENU)) { if (boot_nparams++)
^^ This one cannot happen... Although I see in patch 5 it gets removed anyway...
@@ -8829,23 +8833,25 @@ qemuBuildCommandLine(virConnectPtr conn, virBufferAddLit(&boot_buf, "strict=on"); }
- if (boot_nparams > 0) { - virCommandAddArg(cmd, "-boot"); + if (virBufferCheckError(&boot_buf) < 0) + goto error;
- if (virBufferCheckError(&boot_buf) < 0) - goto error; + boot_order_str = virBufferContentAndReset(&boot_order); + boot_opts_str = virBufferContentAndReset(&boot_buf); + if (boot_order_str || boot_opts_str) { + virCommandAddArg(cmd, "-boot");
- if (boot_nparams < 2 || emitBootindex) { - virCommandAddArgBuffer(cmd, &boot_buf); - virBufferFreeAndReset(&boot_buf); - } else { - char *str = virBufferContentAndReset(&boot_buf); - virCommandAddArgFormat(cmd, - "order=%s", - str); - VIR_FREE(str); + if (boot_order_str && boot_opts_str) { + virCommandAddArgFormat(cmd, "order=%s,%s", + boot_order_str, boot_opts_str); + } else if (boot_order_str) { + virCommandAddArg(cmd, boot_order_str); + } else if (boot_opts_str) { + virCommandAddArg(cmd, boot_opts_str); } } + VIR_FREE(boot_order_str); + VIR_FREE(boot_opts_str);
if (def->os.kernel) virCommandAddArgList(cmd, "-kernel", def->os.kernel, NULL); @@ -10335,6 +10341,7 @@ qemuBuildCommandLine(virConnectPtr conn, return cmd;
error: + virBufferFreeAndReset(&boot_order); virBufferFreeAndReset(&boot_buf); virObjectUnref(cfg); /* free up any resources in the network driver
--- src/qemu/qemu_command.c | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index f13dbda..0b5fb72 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -8207,7 +8207,7 @@ qemuBuildCommandLine(virConnectPtr conn, }; virArch hostarch = virArchFromHost(); virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); - virBuffer boot_buf = VIR_BUFFER_INITIALIZER; + virBuffer boot_opts = VIR_BUFFER_INITIALIZER; virBuffer boot_order = VIR_BUFFER_INITIALIZER; char *boot_order_str = NULL, *boot_opts_str = NULL; int boot_nparams = 0; @@ -8783,12 +8783,12 @@ qemuBuildCommandLine(virConnectPtr conn, if (def->os.bootmenu) { if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_BOOT_MENU)) { if (boot_nparams++) - virBufferAddChar(&boot_buf, ','); + virBufferAddChar(&boot_opts, ','); if (def->os.bootmenu == VIR_TRISTATE_BOOL_YES) - virBufferAddLit(&boot_buf, "menu=on"); + virBufferAddLit(&boot_opts, "menu=on"); else - virBufferAddLit(&boot_buf, "menu=off"); + virBufferAddLit(&boot_opts, "menu=off"); } else { /* We cannot emit an error when bootmenu is enabled but * unsupported because of backward compatibility */ @@ -8806,9 +8806,9 @@ qemuBuildCommandLine(virConnectPtr conn, } if (boot_nparams++) - virBufferAddChar(&boot_buf, ','); + virBufferAddChar(&boot_opts, ','); - virBufferAsprintf(&boot_buf, + virBufferAsprintf(&boot_opts, "reboot-timeout=%d", def->os.bios.rt_delay); } @@ -8822,22 +8822,22 @@ qemuBuildCommandLine(virConnectPtr conn, } if (boot_nparams++) - virBufferAddChar(&boot_buf, ','); + virBufferAddChar(&boot_opts, ','); - virBufferAsprintf(&boot_buf, "splash-time=%u", def->os.bm_timeout); + virBufferAsprintf(&boot_opts, "splash-time=%u", def->os.bm_timeout); } if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_BOOT_STRICT)) { if (boot_nparams++) - virBufferAddChar(&boot_buf, ','); - virBufferAddLit(&boot_buf, "strict=on"); + virBufferAddChar(&boot_opts, ','); + virBufferAddLit(&boot_opts, "strict=on"); } - if (virBufferCheckError(&boot_buf) < 0) + if (virBufferCheckError(&boot_opts) < 0) goto error; boot_order_str = virBufferContentAndReset(&boot_order); - boot_opts_str = virBufferContentAndReset(&boot_buf); + boot_opts_str = virBufferContentAndReset(&boot_opts); if (boot_order_str || boot_opts_str) { virCommandAddArg(cmd, "-boot"); @@ -10342,7 +10342,7 @@ qemuBuildCommandLine(virConnectPtr conn, error: virBufferFreeAndReset(&boot_order); - virBufferFreeAndReset(&boot_buf); + virBufferFreeAndReset(&boot_opts); virObjectUnref(cfg); /* free up any resources in the network driver * but don't overwrite the original error */ -- 2.0.5
Instead of tracking the number of added parameters, add a comma at the end of each one unconditionally and trim the trailing one at the end. --- src/qemu/qemu_command.c | 27 ++++++++------------------- 1 file changed, 8 insertions(+), 19 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 0b5fb72..06a9e54 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -8210,7 +8210,6 @@ qemuBuildCommandLine(virConnectPtr conn, virBuffer boot_opts = VIR_BUFFER_INITIALIZER; virBuffer boot_order = VIR_BUFFER_INITIALIZER; char *boot_order_str = NULL, *boot_opts_str = NULL; - int boot_nparams = 0; VIR_DEBUG("conn=%p driver=%p def=%p mon=%p json=%d " "qemuCaps=%p migrateFrom=%s migrateFD=%d " @@ -8782,13 +8781,10 @@ qemuBuildCommandLine(virConnectPtr conn, if (def->os.bootmenu) { if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_BOOT_MENU)) { - if (boot_nparams++) - virBufferAddChar(&boot_opts, ','); - if (def->os.bootmenu == VIR_TRISTATE_BOOL_YES) - virBufferAddLit(&boot_opts, "menu=on"); + virBufferAddLit(&boot_opts, "menu=on,"); else - virBufferAddLit(&boot_opts, "menu=off"); + virBufferAddLit(&boot_opts, "menu=off,"); } else { /* We cannot emit an error when bootmenu is enabled but * unsupported because of backward compatibility */ @@ -8805,11 +8801,8 @@ qemuBuildCommandLine(virConnectPtr conn, goto error; } - if (boot_nparams++) - virBufferAddChar(&boot_opts, ','); - virBufferAsprintf(&boot_opts, - "reboot-timeout=%d", + "reboot-timeout=%d,", def->os.bios.rt_delay); } @@ -8821,17 +8814,13 @@ qemuBuildCommandLine(virConnectPtr conn, goto error; } - if (boot_nparams++) - virBufferAddChar(&boot_opts, ','); - - virBufferAsprintf(&boot_opts, "splash-time=%u", def->os.bm_timeout); + virBufferAsprintf(&boot_opts, "splash-time=%u,", def->os.bm_timeout); } - if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_BOOT_STRICT)) { - if (boot_nparams++) - virBufferAddChar(&boot_opts, ','); - virBufferAddLit(&boot_opts, "strict=on"); - } + if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_BOOT_STRICT)) + virBufferAddLit(&boot_opts, "strict=on,"); + + virBufferTrim(&boot_opts, ",", -1); if (virBufferCheckError(&boot_opts) < 0) goto error; -- 2.0.5
On 02/18/2015 11:39 AM, Ján Tomko wrote:
Instead of tracking the number of added parameters, add a comma at the end of each one unconditionally and trim the trailing one at the end. --- src/qemu/qemu_command.c | 27 ++++++++------------------- 1 file changed, 8 insertions(+), 19 deletions(-)
ACK John
participants (2)
-
John Ferlan -
Ján Tomko