Re: [libvirt] [PATCH 5/6] qemu: command: Don't format -device isa-fdc, ... twice with two floppy drives
On Thu, Aug 09, 2018 at 02:48:58PM +0200, Peter Krempa wrote:
Fix regression introduced in 42fd5a58adb. With q35 machine type which
Please wrap the commit id in <> or rephrase the sentence to have the
commit id separated by spaces.
requires the explicitly specified FDC we'd format two isa-fdc
two spaces
controllers to the command line as the code was moved to a place where
it's called per-disk.
Move the call back after formatting all disks and reiterate the disks to
find the floppy controllers.
This also moves the '-global' directive which sets up the default
ISA-FDC to the end after all the disks but since we are modifying the
properties it is safe to do so.
Signed-off-by: Peter Krempa
---
src/qemu/qemu_command.c| 100 +
tests/qemuxml2argvdata/boot-complex-bootindex.args | 2 +-
tests/qemuxml2argvdata/boot-complex.args | 2 +-
tests/qemuxml2argvdata/boot-strict.args| 2 +-
.../disk-floppy-q35-2_11.x86_64-latest.args| 2 +-
.../disk-floppy-q35-2_9.x86_64-latest.args | 3 +-
tests/qemuxml2argvdata/disk-floppy-tray.args | 2 +-
tests/qemuxml2argvdata/disk-floppy.args| 2 +-
.../disk-floppy.x86_64-latest.args | 2 +-
tests/qemuxml2argvdata/user-aliases.args | 2 +-
10 files changed, 71 insertions(+), 48 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index daf037328f..1169164a39 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -2143,50 +2143,78 @@ qemuBuildDiskDeviceStr(const virDomainDef *def,
static int
-qemuBuildFloppyCommandLineOptions(virCommandPtr cmd,
- const virDomainDef *def,
- virDomainDiskDefPtr disk,
- virQEMUCapsPtr qemuCaps,
- unsigned int bootindex)
+qemuBuildFloppyCommandLineControllerOptions(virCommandPtr cmd,
+const virDomainDef *def,
+virQEMUCapsPtr qemuCaps,
+unsigned int bootFloppy)
{
virBuffer fdc_opts = VIR_BUFFER_INITIALIZER;
+bool explicitfdc = qemuDomainNeedsFDC(def);
+bool hasfloppy = false;
+unsigned int bootindex;
char driveLetter;
char *backendAlias = NULL;
char *backendStr = NULL;
char *bootindexStr = NULL;
+size_t i;
int ret = -1;
-if (disk->info.addr.drive.unit)
-driveLetter = 'B';
-else
-driveLetter = 'A';
+virBufferAddLit(_opts, "isa-fdc,");
-if (qemuDomainDiskGetBackendAlias(disk, qemuCaps, ) < 0)
-goto cleanup;
+for (i = 0; i < def->ndisks; i++) {
+virDomainDiskDefPtr disk = def->disks[i];
-if (backendAlias &&
-virAsprintf(, "drive%c=%s", driveLetter, backendAlias) < 0)
-goto cleanup;
+if (disk->bus != VIR_DOMAIN_DISK_BUS_FDC)
+continue;
-if (bootindex &&
-virAsprintf(, "bootindex%c=%u", driveLetter, bootindex) <
0)
-goto cleanup;
+hasfloppy = true;
-if (!qemuDomainNeedsFDC(def)) {
-if (backendStr) {
-virCommandAddArg(cmd, "-global");
-virCommandAddArgFormat(cmd, "isa-fdc.%s", backendStr);
-}
+if (disk->info.bootIndex)
+bootindex = disk->info.bootIndex;
+else
+bootindex = bootFloppy;
-if (bootindexStr) {
-virCommandAddArg(cmd, "-global");
-virCommandAddArgFormat(cmd, "isa-fdc.%s", bootindexStr);
+bootFloppy = 0;
before, we only zeroed bootFloppy if we have used it, not if we used
disk->info.bootIndex
+
+if (disk->info.addr.drive.unit)
+driveLetter = 'B';
+else
+driveLetter = 'A';
+
+if (qemuDomainDiskGetBackendAlias(disk, qemuCaps, ) < 0)
+goto cleanup;
+
+if (backendAlias &&
+virAsprintf(, "drive%c=%s", driveLetter, backendAlias)
< 0)
+goto cleanup;
+
+if (bootindex &&
+virAsprintf(, "bootindex%c=%u", driveLetter, bootindex)
< 0)
+goto cleanup;
+
+if (!explicitfdc) {
+if (backendStr) {
+virCommandAddArg(cmd, "-global");
+virCommandAddArgFormat(cmd, "isa-fdc.%s", backendStr);
+}
+
+if (bootindexStr) {
+virCommandAddArg(cmd, "-global");
+virCommandAddArgFormat(cmd, "isa-fdc.%s", bootindexStr);
+}
+} else {
+virBufferStrcat(_opts, backendStr, ",", NULL);
+virBufferStrcat(_opts, bootindexStr, ",", NULL);
}
-} else {
+
+VIR_FREE(backendAlias);
+VIR_FREE(backendStr);
+VIR_FREE(bootindexStr);
+}
+
+
Two empty lines.
+if (explicitfdc && hasfloppy) {
/* Newer Q35 machine types require an explicit FDC
[libvirt] [PATCH 5/6] qemu: command: Don't format -device isa-fdc, ... twice with two floppy drives
Fix regression introduced in 42fd5a58adb. With q35 machine type which
requires the explicitly specified FDC we'd format two isa-fdc
controllers to the command line as the code was moved to a place where
it's called per-disk.
Move the call back after formatting all disks and reiterate the disks to
find the floppy controllers.
This also moves the '-global' directive which sets up the default
ISA-FDC to the end after all the disks but since we are modifying the
properties it is safe to do so.
Signed-off-by: Peter Krempa
---
src/qemu/qemu_command.c| 100 +
tests/qemuxml2argvdata/boot-complex-bootindex.args | 2 +-
tests/qemuxml2argvdata/boot-complex.args | 2 +-
tests/qemuxml2argvdata/boot-strict.args| 2 +-
.../disk-floppy-q35-2_11.x86_64-latest.args| 2 +-
.../disk-floppy-q35-2_9.x86_64-latest.args | 3 +-
tests/qemuxml2argvdata/disk-floppy-tray.args | 2 +-
tests/qemuxml2argvdata/disk-floppy.args| 2 +-
.../disk-floppy.x86_64-latest.args | 2 +-
tests/qemuxml2argvdata/user-aliases.args | 2 +-
10 files changed, 71 insertions(+), 48 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index daf037328f..1169164a39 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -2143,50 +2143,78 @@ qemuBuildDiskDeviceStr(const virDomainDef *def,
static int
-qemuBuildFloppyCommandLineOptions(virCommandPtr cmd,
- const virDomainDef *def,
- virDomainDiskDefPtr disk,
- virQEMUCapsPtr qemuCaps,
- unsigned int bootindex)
+qemuBuildFloppyCommandLineControllerOptions(virCommandPtr cmd,
+const virDomainDef *def,
+virQEMUCapsPtr qemuCaps,
+unsigned int bootFloppy)
{
virBuffer fdc_opts = VIR_BUFFER_INITIALIZER;
+bool explicitfdc = qemuDomainNeedsFDC(def);
+bool hasfloppy = false;
+unsigned int bootindex;
char driveLetter;
char *backendAlias = NULL;
char *backendStr = NULL;
char *bootindexStr = NULL;
+size_t i;
int ret = -1;
-if (disk->info.addr.drive.unit)
-driveLetter = 'B';
-else
-driveLetter = 'A';
+virBufferAddLit(_opts, "isa-fdc,");
-if (qemuDomainDiskGetBackendAlias(disk, qemuCaps, ) < 0)
-goto cleanup;
+for (i = 0; i < def->ndisks; i++) {
+virDomainDiskDefPtr disk = def->disks[i];
-if (backendAlias &&
-virAsprintf(, "drive%c=%s", driveLetter, backendAlias) < 0)
-goto cleanup;
+if (disk->bus != VIR_DOMAIN_DISK_BUS_FDC)
+continue;
-if (bootindex &&
-virAsprintf(, "bootindex%c=%u", driveLetter, bootindex) <
0)
-goto cleanup;
+hasfloppy = true;
-if (!qemuDomainNeedsFDC(def)) {
-if (backendStr) {
-virCommandAddArg(cmd, "-global");
-virCommandAddArgFormat(cmd, "isa-fdc.%s", backendStr);
-}
+if (disk->info.bootIndex)
+bootindex = disk->info.bootIndex;
+else
+bootindex = bootFloppy;
-if (bootindexStr) {
-virCommandAddArg(cmd, "-global");
-virCommandAddArgFormat(cmd, "isa-fdc.%s", bootindexStr);
+bootFloppy = 0;
+
+if (disk->info.addr.drive.unit)
+driveLetter = 'B';
+else
+driveLetter = 'A';
+
+if (qemuDomainDiskGetBackendAlias(disk, qemuCaps, ) < 0)
+goto cleanup;
+
+if (backendAlias &&
+virAsprintf(, "drive%c=%s", driveLetter, backendAlias)
< 0)
+goto cleanup;
+
+if (bootindex &&
+virAsprintf(, "bootindex%c=%u", driveLetter,
bootindex) < 0)
+goto cleanup;
+
+if (!explicitfdc) {
+if (backendStr) {
+virCommandAddArg(cmd, "-global");
+virCommandAddArgFormat(cmd, "isa-fdc.%s", backendStr);
+}
+
+if (bootindexStr) {
+virCommandAddArg(cmd, "-global");
+virCommandAddArgFormat(cmd, "isa-fdc.%s", bootindexStr);
+}
+} else {
+virBufferStrcat(_opts, backendStr, ",", NULL);
+virBufferStrcat(_opts, bootindexStr, ",", NULL);
}
-} else {
+
+VIR_FREE(backendAlias);
+VIR_FREE(backendStr);
+VIR_FREE(bootindexStr);
+}
+
+
+if (explicitfdc && hasfloppy) {
/* Newer Q35 machine types require an explicit FDC controller */
-virBufferAddLit(_opts, "isa-fdc,");
-virBufferStrcat(_opts, backendStr, ",", NULL);
-virBufferStrcat(_opts, bootindexStr, NULL);
virBufferTrim(_opts, ",", -1);
virCommandAddArg(cmd, "-device");
