Re: [libvirt] [PATCH 8/9] qemu: command: Refactor floppy controller command formatting

On Tue, Jul 10, 2018 at 10:44:48AM +0200, Peter Krempa wrote: > Aggregate the code for the two separate formats used according to the > machine type and add some supporting code so that the function is > actually readable. > > Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; > --- > src/qemu/qemu_command.c | 58 ++++++++++++++++++++++++------------------------- > 1 file changed, 28 insertions(+), 30 deletions(-) > > diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c > index f756cc7112..2e5920e859 100644 > --- a/src/qemu/qemu_command.c > +++ b/src/qemu/qemu_command.c > @@ -2148,55 +2148,53 @@ qemuBuildFloppyCommandLineOptions(virCommandPtr cmd, > > { > virBuffer fdc_opts = VIR_BUFFER_INITIALIZER; > - char *fdc_opts_str = NULL; > - char *optstr; > + char driveLetter; > char *backendAlias = NULL; > + char *backendStr = NULL; > + char *bootindexStr = NULL; > int ret = -1; > > + if (disk->info.addr.drive.unit) > + driveLetter = 'B'; > + else > + driveLetter = 'A'; > + > if (!(backendAlias = qemuAliasDiskDriveFromDisk(disk))) > return -1; > > - if (virAsprintf(&optstr, "drive%c=%s", > - disk->info.addr.drive.unit ? 'B' : 'A', > - backendAlias) < 0) > + if (virAsprintf(&backendStr, "drive%c=%s", driveLetter, backendAlias) < 0) > goto cleanup; > > - if (!qemuDomainNeedsFDC(def)) { > - virCommandAddArg(cmd, "-global"); > - virCommandAddArgFormat(cmd, "isa-fdc.%s", optstr); > - } else { > - virBufferAsprintf(&fdc_opts, "%s,", optstr); > - } > - VIR_FREE(optstr); > - > - if (bootindex) { > - if (virAsprintf(&optstr, "bootindex%c=%u", > - disk->info.addr.drive.unit > - ? 'B' : 'A', > - bootindex) < 0) > - goto cleanup; > + if (bootindex && > + virAsprintf(&bootindexStr, "bootindex%c=%u", driveLetter, bootindex) < 0) > + goto cleanup; > > - if (!qemuDomainNeedsFDC(def)) { > + if (!qemuDomainNeedsFDC(def)) { > + if (backendStr) { backendStr is filled in unconditionally, this condition is not necessary.

> virCommandAddArg(cmd, "-global"); > - virCommandAddArgFormat(cmd, "isa-fdc.%s", optstr); > - } else { > - virBufferAsprintf(&fdc_opts, "%s,", optstr); > + virCommandAddArgFormat(cmd, "isa-fdc.%s", backendStr); Reviewed-by: Ján Tomko <jtomko(a)redhat.com&gt; Jano