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(-)
>
ACK - there's a note below which could be implemented or not.
John
> 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)",
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
>
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list