[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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 <pkrempa redhat com>
---
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(&fdc_opts, "isa-fdc,");

-    if (qemuDomainDiskGetBackendAlias(disk, qemuCaps, &backendAlias) < 0)
-        goto cleanup;
+    for (i = 0; i < def->ndisks; i++) {
+        virDomainDiskDefPtr disk = def->disks[i];

-    if (backendAlias &&
-        virAsprintf(&backendStr, "drive%c=%s", driveLetter, backendAlias) < 0)
-        goto cleanup;
+        if (disk->bus != VIR_DOMAIN_DISK_BUS_FDC)
+            continue;

-    if (bootindex &&
-        virAsprintf(&bootindexStr, "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, &backendAlias) < 0)
+            goto cleanup;
+
+        if (backendAlias &&
+            virAsprintf(&backendStr, "drive%c=%s", driveLetter, backendAlias) < 0)
+            goto cleanup;
+
+        if (bootindex &&
+            virAsprintf(&bootindexStr, "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(&fdc_opts, backendStr, ",", NULL);
+            virBufferStrcat(&fdc_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 controller */
-        virBufferAddLit(&fdc_opts, "isa-fdc,");
-        virBufferStrcat(&fdc_opts, backendStr, ",", NULL);
-        virBufferStrcat(&fdc_opts, bootindexStr, NULL);
        virBufferTrim(&fdc_opts, ",", -1);
        virCommandAddArg(cmd, "-device");
        virCommandAddArgBuffer(cmd, &fdc_opts);
@@ -2276,11 +2304,7 @@ qemuBuildDiskCommandLine(virCommandPtr cmd,
        return -1;

    if (!qemuDiskBusNeedsDriveArg(disk->bus)) {
-        if (disk->bus == VIR_DOMAIN_DISK_BUS_FDC) {
-            if (qemuBuildFloppyCommandLineOptions(cmd, def, disk, qemuCaps,
-                                                  bootindex) < 0)
-                return -1;
-        } else {
+        if (disk->bus != VIR_DOMAIN_DISK_BUS_FDC) {
            virCommandAddArg(cmd, "-device");

            if (!(optstr = qemuBuildDiskDeviceStr(def, disk, bootindex,
@@ -2331,10 +2355,6 @@ qemuBuildDisksCommandLine(virCommandPtr cmd,
                bootindex = bootCD;
                bootCD = 0;
                break;
-            case VIR_DOMAIN_DISK_DEVICE_FLOPPY:
-                bootindex = bootFloppy;
-                bootFloppy = 0;
-                break;
            case VIR_DOMAIN_DISK_DEVICE_DISK:
            case VIR_DOMAIN_DISK_DEVICE_LUN:
                bootindex = bootDisk;
@@ -2348,6 +2368,10 @@ qemuBuildDisksCommandLine(virCommandPtr cmd,
            return -1;
    }

+    if (qemuBuildFloppyCommandLineControllerOptions(cmd, def, qemuCaps, bootFloppy) < 0)
+        return -1;
+
+

Two empty lines.

    return 0;
}


Reviewed-by: Ján Tomko <jtomko redhat com>

Jano

Attachment: signature.asc
Description: Digital signature


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]