[libvirt] [PATCH 00/25] Final pile of qemu_command.c changes

Finishing the adventure - certainly qemuBuildCommandLine looks much cleaner now and is much shorter (~2100 lines down to ~250 lines). The downside is plowing though the changes and the reviews. John Ferlan (25): qemu: Introduce qemuBuildClockCommandLine qemu: Introduce qemuBuildPMCommandLine qemu: Introduce qemuBuildBootCommandLine qemu: Introduce qemuBuildGlobalControllerCommandLine qemu: Introduce qemuBuildControllerDevCommandLine qemu: Introduce qemuBuildHubCommandLine qemu: Introduce qemuBuildDiskDriveCommandLine qemu: Introduce qemuBuildFSDevCommandLine qemu: Introduce qemuBuildNetCommandLine qemu: Introduce qemuBuildSmartcardCommandLine qemu: Introduce qemuBuildSerialCommandLine qemu: Introduce qemuBuildParallelsCommandLine qemu: Introduce qemuBuildChannelsCommandLine qemu: Introduce qemuBuildConsoleCommandLine qemu: Modify qemuBuildTPMCommandLine qemu: Introduce qemuBuildInputCommandLine qemu: Introduce qemuBuildVideoCommandLine qemu: Introduce qemuBuildSoundCommandLine qemu: Introduce qemuBuildWatchdogCommandLine qemu: Introduce qemuBuildRedirdevCommandLine qemu: Introduce qemuBuildHostdevCommandLine qemu: Introduce qemuBuildMemballoonCommandLine qemu: Introduce qemuBuildRNGCommandLine qemu: Introduce qemuBuildNVRAMCommandLine qemu: Introduce qemuBuildPanicCommandLine src/conf/domain_conf.c | 14 +- src/conf/domain_conf.h | 8 +- src/qemu/qemu_capabilities.c | 2 +- src/qemu/qemu_capabilities.h | 2 +- src/qemu/qemu_command.c | 7703 +++++++++++++++++++++------------------- src/qemu/qemu_command.h | 46 +- src/qemu/qemu_domain_address.c | 2 +- src/qemu/qemu_domain_address.h | 2 +- 8 files changed, 4073 insertions(+), 3706 deletions(-) -- 2.5.0

Add new function to manage adding the '-clock' options to the command line removing that task from the mainline qemuBuildCommandLine. Also includes some minor formatting cleanups. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_command.c | 300 ++++++++++++++++++++++++++---------------------- 1 file changed, 160 insertions(+), 140 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index b751f04..d3377ee 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -4609,6 +4609,7 @@ qemuBuildSgaCommandLine(virCommandPtr cmd, static char * qemuBuildClockArgStr(virDomainClockDefPtr def) { + size_t i; virBuffer buf = VIR_BUFFER_INITIALIZER; switch (def->offset) { @@ -4673,8 +4674,8 @@ qemuBuildClockArgStr(virDomainClockDefPtr def) goto error; } - /* Look for an 'rtc' timer element, and add in appropriate clock= and driftfix= */ - size_t i; + /* Look for an 'rtc' timer element, and add in appropriate + * clock= and driftfix= */ for (i = 0; i < def->ntimers; i++) { if (def->timers[i]->name == VIR_DOMAIN_TIMER_NAME_RTC) { switch (def->timers[i]->track) { @@ -4724,6 +4725,161 @@ qemuBuildClockArgStr(virDomainClockDefPtr def) return NULL; } + +/* NOTE: Building of commands can change def->clock->data.* values, so + * virDomainDef is not const here. + */ +static int +qemuBuildClockCommandLine(virCommandPtr cmd, + virDomainDefPtr def, + virQEMUCapsPtr qemuCaps) +{ + size_t i; + + if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_RTC)) { + char *rtcopt; + virCommandAddArg(cmd, "-rtc"); + if (!(rtcopt = qemuBuildClockArgStr(&def->clock))) + return -1; + virCommandAddArg(cmd, rtcopt); + VIR_FREE(rtcopt); + } else { + switch (def->clock.offset) { + case VIR_DOMAIN_CLOCK_OFFSET_LOCALTIME: + case VIR_DOMAIN_CLOCK_OFFSET_TIMEZONE: + virCommandAddArg(cmd, "-localtime"); + break; + + case VIR_DOMAIN_CLOCK_OFFSET_UTC: + /* Nothing, its the default */ + break; + + default: + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("unsupported clock offset '%s'"), + virDomainClockOffsetTypeToString(def->clock.offset)); + return -1; + } + } + + if (def->clock.offset == VIR_DOMAIN_CLOCK_OFFSET_TIMEZONE && + def->clock.data.timezone) { + virCommandAddEnvPair(cmd, "TZ", def->clock.data.timezone); + } + + for (i = 0; i < def->clock.ntimers; i++) { + switch ((virDomainTimerNameType) def->clock.timers[i]->name) { + case VIR_DOMAIN_TIMER_NAME_PLATFORM: + case VIR_DOMAIN_TIMER_NAME_TSC: + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("unsupported timer type (name) '%s'"), + virDomainTimerNameTypeToString(def->clock.timers[i]->name)); + return -1; + + case VIR_DOMAIN_TIMER_NAME_KVMCLOCK: + case VIR_DOMAIN_TIMER_NAME_HYPERVCLOCK: + /* Timers above are handled when building -cpu. */ + case VIR_DOMAIN_TIMER_NAME_LAST: + break; + + case VIR_DOMAIN_TIMER_NAME_RTC: + /* This has already been taken care of (in qemuBuildClockArgStr) + if QEMU_CAPS_RTC is set (mutually exclusive with + QEMUD_FLAG_RTC_TD_HACK) */ + if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_RTC_TD_HACK)) { + switch (def->clock.timers[i]->tickpolicy) { + case -1: + case VIR_DOMAIN_TIMER_TICKPOLICY_DELAY: + /* the default - do nothing */ + break; + case VIR_DOMAIN_TIMER_TICKPOLICY_CATCHUP: + virCommandAddArg(cmd, "-rtc-td-hack"); + break; + case VIR_DOMAIN_TIMER_TICKPOLICY_MERGE: + case VIR_DOMAIN_TIMER_TICKPOLICY_DISCARD: + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("unsupported rtc tickpolicy '%s'"), + virDomainTimerTickpolicyTypeToString(def->clock.timers[i]->tickpolicy)); + return -1; + } + } else if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_RTC) && + (def->clock.timers[i]->tickpolicy + != VIR_DOMAIN_TIMER_TICKPOLICY_DELAY) && + (def->clock.timers[i]->tickpolicy != -1)) { + /* a non-default rtc policy was given, but there is no + way to implement it in this version of qemu */ + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("unsupported rtc tickpolicy '%s'"), + virDomainTimerTickpolicyTypeToString(def->clock.timers[i]->tickpolicy)); + return -1; + } + break; + + case VIR_DOMAIN_TIMER_NAME_PIT: + switch (def->clock.timers[i]->tickpolicy) { + case -1: + case VIR_DOMAIN_TIMER_TICKPOLICY_DELAY: + /* delay is the default if we don't have kernel + (-no-kvm-pit), otherwise, the default is catchup. */ + if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_KVM_PIT_TICK_POLICY)) + virCommandAddArgList(cmd, "-global", + "kvm-pit.lost_tick_policy=discard", NULL); + else if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_NO_KVM_PIT)) + virCommandAddArg(cmd, "-no-kvm-pit-reinjection"); + break; + case VIR_DOMAIN_TIMER_TICKPOLICY_CATCHUP: + if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_NO_KVM_PIT) || + virQEMUCapsGet(qemuCaps, QEMU_CAPS_KVM_PIT_TICK_POLICY)) { + /* do nothing - this is default for kvm-pit */ + } else if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_TDF)) { + /* -tdf switches to 'catchup' with userspace pit. */ + virCommandAddArg(cmd, "-tdf"); + } else { + /* can't catchup if we have neither pit mode */ + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("unsupported pit tickpolicy '%s'"), + virDomainTimerTickpolicyTypeToString(def->clock.timers[i]->tickpolicy)); + return -1; + } + break; + case VIR_DOMAIN_TIMER_TICKPOLICY_MERGE: + case VIR_DOMAIN_TIMER_TICKPOLICY_DISCARD: + /* no way to support these modes for pit in qemu */ + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("unsupported pit tickpolicy '%s'"), + virDomainTimerTickpolicyTypeToString(def->clock.timers[i]->tickpolicy)); + return -1; + } + break; + + case VIR_DOMAIN_TIMER_NAME_HPET: + /* the only meaningful attribute for hpet is "present". If + * present is -1, that means it wasn't specified, and + * should be left at the default for the + * hypervisor. "default" when -no-hpet exists is "yes", + * and when -no-hpet doesn't exist is "no". "confusing"? + * "yes"! */ + + if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_NO_HPET)) { + if (def->clock.timers[i]->present == 0) + virCommandAddArg(cmd, "-no-hpet"); + } else { + /* no hpet timer available. The only possible action + is to raise an error if present="yes" */ + if (def->clock.timers[i]->present == 1) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + "%s", _("hpet timer is not supported")); + return -1; + } + } + break; + } + } + + return 0; +} + + static int qemuBuildCpuModelArgStr(virQEMUDriverPtr driver, const virDomainDef *def, @@ -7153,144 +7309,8 @@ qemuBuildCommandLine(virConnectPtr conn, monitor_json) < 0) goto error; - if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_RTC)) { - char *rtcopt; - virCommandAddArg(cmd, "-rtc"); - if (!(rtcopt = qemuBuildClockArgStr(&def->clock))) - goto error; - virCommandAddArg(cmd, rtcopt); - VIR_FREE(rtcopt); - } else { - switch (def->clock.offset) { - case VIR_DOMAIN_CLOCK_OFFSET_LOCALTIME: - case VIR_DOMAIN_CLOCK_OFFSET_TIMEZONE: - virCommandAddArg(cmd, "-localtime"); - break; - - case VIR_DOMAIN_CLOCK_OFFSET_UTC: - /* Nothing, its the default */ - break; - - default: - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("unsupported clock offset '%s'"), - virDomainClockOffsetTypeToString(def->clock.offset)); - goto error; - } - } - if (def->clock.offset == VIR_DOMAIN_CLOCK_OFFSET_TIMEZONE && - def->clock.data.timezone) { - virCommandAddEnvPair(cmd, "TZ", def->clock.data.timezone); - } - - for (i = 0; i < def->clock.ntimers; i++) { - switch ((virDomainTimerNameType) def->clock.timers[i]->name) { - case VIR_DOMAIN_TIMER_NAME_PLATFORM: - case VIR_DOMAIN_TIMER_NAME_TSC: - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("unsupported timer type (name) '%s'"), - virDomainTimerNameTypeToString(def->clock.timers[i]->name)); - goto error; - - case VIR_DOMAIN_TIMER_NAME_KVMCLOCK: - case VIR_DOMAIN_TIMER_NAME_HYPERVCLOCK: - /* Timers above are handled when building -cpu. */ - case VIR_DOMAIN_TIMER_NAME_LAST: - break; - - case VIR_DOMAIN_TIMER_NAME_RTC: - /* This has already been taken care of (in qemuBuildClockArgStr) - if QEMU_CAPS_RTC is set (mutually exclusive with - QEMUD_FLAG_RTC_TD_HACK) */ - if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_RTC_TD_HACK)) { - switch (def->clock.timers[i]->tickpolicy) { - case -1: - case VIR_DOMAIN_TIMER_TICKPOLICY_DELAY: - /* the default - do nothing */ - break; - case VIR_DOMAIN_TIMER_TICKPOLICY_CATCHUP: - virCommandAddArg(cmd, "-rtc-td-hack"); - break; - case VIR_DOMAIN_TIMER_TICKPOLICY_MERGE: - case VIR_DOMAIN_TIMER_TICKPOLICY_DISCARD: - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("unsupported rtc tickpolicy '%s'"), - virDomainTimerTickpolicyTypeToString(def->clock.timers[i]->tickpolicy)); - goto error; - } - } else if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_RTC) && - (def->clock.timers[i]->tickpolicy - != VIR_DOMAIN_TIMER_TICKPOLICY_DELAY) && - (def->clock.timers[i]->tickpolicy != -1)) { - /* a non-default rtc policy was given, but there is no - way to implement it in this version of qemu */ - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("unsupported rtc tickpolicy '%s'"), - virDomainTimerTickpolicyTypeToString(def->clock.timers[i]->tickpolicy)); - goto error; - } - break; - - case VIR_DOMAIN_TIMER_NAME_PIT: - switch (def->clock.timers[i]->tickpolicy) { - case -1: - case VIR_DOMAIN_TIMER_TICKPOLICY_DELAY: - /* delay is the default if we don't have kernel - (-no-kvm-pit), otherwise, the default is catchup. */ - if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_KVM_PIT_TICK_POLICY)) - virCommandAddArgList(cmd, "-global", - "kvm-pit.lost_tick_policy=discard", NULL); - else if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_NO_KVM_PIT)) - virCommandAddArg(cmd, "-no-kvm-pit-reinjection"); - break; - case VIR_DOMAIN_TIMER_TICKPOLICY_CATCHUP: - if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_NO_KVM_PIT) || - virQEMUCapsGet(qemuCaps, QEMU_CAPS_KVM_PIT_TICK_POLICY)) { - /* do nothing - this is default for kvm-pit */ - } else if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_TDF)) { - /* -tdf switches to 'catchup' with userspace pit. */ - virCommandAddArg(cmd, "-tdf"); - } else { - /* can't catchup if we have neither pit mode */ - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("unsupported pit tickpolicy '%s'"), - virDomainTimerTickpolicyTypeToString(def->clock.timers[i]->tickpolicy)); - goto error; - } - break; - case VIR_DOMAIN_TIMER_TICKPOLICY_MERGE: - case VIR_DOMAIN_TIMER_TICKPOLICY_DISCARD: - /* no way to support these modes for pit in qemu */ - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("unsupported pit tickpolicy '%s'"), - virDomainTimerTickpolicyTypeToString(def->clock.timers[i]->tickpolicy)); - goto error; - } - break; - - case VIR_DOMAIN_TIMER_NAME_HPET: - /* the only meaningful attribute for hpet is "present". If - * present is -1, that means it wasn't specified, and - * should be left at the default for the - * hypervisor. "default" when -no-hpet exists is "yes", - * and when -no-hpet doesn't exist is "no". "confusing"? - * "yes"! */ - - if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_NO_HPET)) { - if (def->clock.timers[i]->present == 0) - virCommandAddArg(cmd, "-no-hpet"); - } else { - /* no hpet timer available. The only possible action - is to raise an error if present="yes" */ - if (def->clock.timers[i]->present == 1) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - "%s", _("hpet timer is not supported")); - goto error; - } - } - break; - } - } + if (qemuBuildClockCommandLine(cmd, def, qemuCaps) < 0) + goto error; /* Only add -no-reboot option if each event destroys domain */ if (def->onReboot == VIR_DOMAIN_LIFECYCLE_DESTROY && -- 2.5.0

Add new function to manage adding the power management options to the command line removing that task from the mainline qemuBuildCommandLine. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_command.c | 136 ++++++++++++++++++++++++++---------------------- 1 file changed, 75 insertions(+), 61 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index d3377ee..302c150 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -4881,6 +4881,79 @@ qemuBuildClockCommandLine(virCommandPtr cmd, static int +qemuBuildPMCommandLine(virCommandPtr cmd, + const virDomainDef *def, + virQEMUCapsPtr qemuCaps, + bool monitor_json) +{ + bool allowReboot = true; + + /* Only add -no-reboot option if each event destroys domain */ + if (def->onReboot == VIR_DOMAIN_LIFECYCLE_DESTROY && + def->onPoweroff == VIR_DOMAIN_LIFECYCLE_DESTROY && + (def->onCrash == VIR_DOMAIN_LIFECYCLE_CRASH_DESTROY || + def->onCrash == VIR_DOMAIN_LIFECYCLE_CRASH_COREDUMP_DESTROY)) { + allowReboot = false; + virCommandAddArg(cmd, "-no-reboot"); + } + + /* If JSON monitor is enabled, we can receive an event + * when QEMU stops. If we use no-shutdown, then we can + * watch for this event and do a soft/warm reboot. + */ + if (monitor_json && allowReboot && + virQEMUCapsGet(qemuCaps, QEMU_CAPS_NO_SHUTDOWN)) { + virCommandAddArg(cmd, "-no-shutdown"); + } + + if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_NO_ACPI)) { + if (def->features[VIR_DOMAIN_FEATURE_ACPI] != VIR_TRISTATE_SWITCH_ON) + virCommandAddArg(cmd, "-no-acpi"); + } + + /* We fall back to PIIX4_PM even for q35, since it's what we did + pre-q35-pm support. QEMU starts up fine (with a warning) if + mixing PIIX PM and -M q35. Starting to reject things here + could mean we refuse to start existing configs in the wild.*/ + if (def->pm.s3) { + const char *pm_object = "PIIX4_PM"; + + if (qemuDomainMachineIsQ35(def) && + virQEMUCapsGet(qemuCaps, QEMU_CAPS_ICH9_DISABLE_S3)) { + pm_object = "ICH9-LPC"; + } else if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_PIIX_DISABLE_S3)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + "%s", _("setting ACPI S3 not supported")); + return -1; + } + + virCommandAddArg(cmd, "-global"); + virCommandAddArgFormat(cmd, "%s.disable_s3=%d", + pm_object, def->pm.s3 == VIR_TRISTATE_BOOL_NO); + } + + if (def->pm.s4) { + const char *pm_object = "PIIX4_PM"; + + if (qemuDomainMachineIsQ35(def) && + virQEMUCapsGet(qemuCaps, QEMU_CAPS_ICH9_DISABLE_S4)) { + pm_object = "ICH9-LPC"; + } else if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_PIIX_DISABLE_S4)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + "%s", _("setting ACPI S4 not supported")); + return -1; + } + + virCommandAddArg(cmd, "-global"); + virCommandAddArgFormat(cmd, "%s.disable_s4=%d", + pm_object, def->pm.s4 == VIR_TRISTATE_BOOL_NO); + } + + return 0; +} + + +static int qemuBuildCpuModelArgStr(virQEMUDriverPtr driver, const virDomainDef *def, virBufferPtr buf, @@ -7175,7 +7248,6 @@ qemuBuildCommandLine(virConnectPtr conn, bool havespice = false; int last_good_net = -1; virCommandPtr cmd = NULL; - bool allowReboot = true; bool emitBootindex = false; int usbcontroller = 0; int actualSerials = 0; @@ -7312,66 +7384,8 @@ qemuBuildCommandLine(virConnectPtr conn, if (qemuBuildClockCommandLine(cmd, def, qemuCaps) < 0) goto error; - /* Only add -no-reboot option if each event destroys domain */ - if (def->onReboot == VIR_DOMAIN_LIFECYCLE_DESTROY && - def->onPoweroff == VIR_DOMAIN_LIFECYCLE_DESTROY && - (def->onCrash == VIR_DOMAIN_LIFECYCLE_CRASH_DESTROY || - def->onCrash == VIR_DOMAIN_LIFECYCLE_CRASH_COREDUMP_DESTROY)) { - allowReboot = false; - virCommandAddArg(cmd, "-no-reboot"); - } - - /* If JSON monitor is enabled, we can receive an event - * when QEMU stops. If we use no-shutdown, then we can - * watch for this event and do a soft/warm reboot. - */ - if (monitor_json && allowReboot && - virQEMUCapsGet(qemuCaps, QEMU_CAPS_NO_SHUTDOWN)) { - virCommandAddArg(cmd, "-no-shutdown"); - } - - if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_NO_ACPI)) { - if (def->features[VIR_DOMAIN_FEATURE_ACPI] != VIR_TRISTATE_SWITCH_ON) - virCommandAddArg(cmd, "-no-acpi"); - } - - /* We fall back to PIIX4_PM even for q35, since it's what we did - pre-q35-pm support. QEMU starts up fine (with a warning) if - mixing PIIX PM and -M q35. Starting to reject things here - could mean we refuse to start existing configs in the wild.*/ - if (def->pm.s3) { - const char *pm_object = "PIIX4_PM"; - - if (qemuDomainMachineIsQ35(def) && - virQEMUCapsGet(qemuCaps, QEMU_CAPS_ICH9_DISABLE_S3)) { - pm_object = "ICH9-LPC"; - } else if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_PIIX_DISABLE_S3)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - "%s", _("setting ACPI S3 not supported")); - goto error; - } - - virCommandAddArg(cmd, "-global"); - virCommandAddArgFormat(cmd, "%s.disable_s3=%d", - pm_object, def->pm.s3 == VIR_TRISTATE_BOOL_NO); - } - - if (def->pm.s4) { - const char *pm_object = "PIIX4_PM"; - - if (qemuDomainMachineIsQ35(def) && - virQEMUCapsGet(qemuCaps, QEMU_CAPS_ICH9_DISABLE_S4)) { - pm_object = "ICH9-LPC"; - } else if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_PIIX_DISABLE_S4)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - "%s", _("setting ACPI S4 not supported")); - goto error; - } - - virCommandAddArg(cmd, "-global"); - virCommandAddArgFormat(cmd, "%s.disable_s4=%d", - pm_object, def->pm.s4 == VIR_TRISTATE_BOOL_NO); - } + if (qemuBuildPMCommandLine(cmd, def, qemuCaps, monitor_json) < 0) + goto error; /* * We prefer using explicit bootindex=N parameters for predictable -- 2.5.0

Add new function to manage adding the -boot options to the command line removing that task from the mainline qemuBuildCommandLine. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_command.c | 282 +++++++++++++++++++++++++----------------------- 1 file changed, 150 insertions(+), 132 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 302c150..57e9bc7 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -4954,6 +4954,155 @@ qemuBuildPMCommandLine(virCommandPtr cmd, static int +qemuBuildBootCommandLine(virCommandPtr cmd, + const virDomainDef *def, + virQEMUCapsPtr qemuCaps, + bool *emitBootindex) +{ + size_t i; + virBuffer boot_buf = VIR_BUFFER_INITIALIZER; + char *boot_order_str = NULL, *boot_opts_str = NULL; + + /* + * 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 (!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); + if (virBufferCheckError(&boot_buf) < 0) + goto error; + boot_order_str = virBufferContentAndReset(&boot_buf); + } + + if (def->os.bootmenu) { + if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_BOOT_MENU)) { + 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.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; + } + + virBufferAsprintf(&boot_buf, + "reboot-timeout=%d,", + def->os.bios.rt_delay); + } + + 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; + } + + virBufferAsprintf(&boot_buf, "splash-time=%u,", def->os.bm_timeout); + } + + if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_BOOT_STRICT)) + virBufferAddLit(&boot_buf, "strict=on,"); + + virBufferTrim(&boot_buf, ",", -1); + + if (virBufferCheckError(&boot_buf) < 0) + goto error; + + boot_opts_str = virBufferContentAndReset(&boot_buf); + if (boot_order_str || boot_opts_str) { + virCommandAddArg(cmd, "-boot"); + + 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_opts_str); + VIR_FREE(boot_order_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; + } + } + + return 0; + + error: + VIR_FREE(boot_order_str); + VIR_FREE(boot_opts_str); + virBufferFreeAndReset(&boot_buf); + return -1; +} + + +static int qemuBuildCpuModelArgStr(virQEMUDriverPtr driver, const virDomainDef *def, virBufferPtr buf, @@ -7277,8 +7426,6 @@ qemuBuildCommandLine(virConnectPtr conn, VIR_DOMAIN_CONTROLLER_TYPE_CCID, }; virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); - virBuffer boot_buf = VIR_BUFFER_INITIALIZER; - char *boot_order_str = NULL, *boot_opts_str = NULL; virBuffer fdc_opts = VIR_BUFFER_INITIALIZER; char *fdc_opts_str = NULL; int bootCD = 0, bootFloppy = 0, bootDisk = 0, bootHostdevNet = 0; @@ -7387,135 +7534,9 @@ qemuBuildCommandLine(virConnectPtr conn, if (qemuBuildPMCommandLine(cmd, def, qemuCaps, monitor_json) < 0) goto error; - /* - * 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 (!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); - if (virBufferCheckError(&boot_buf) < 0) - goto error; - boot_order_str = virBufferContentAndReset(&boot_buf); - } - - if (def->os.bootmenu) { - if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_BOOT_MENU)) { - 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.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; - } - - virBufferAsprintf(&boot_buf, - "reboot-timeout=%d,", - def->os.bios.rt_delay); - } - - 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; - } - - virBufferAsprintf(&boot_buf, "splash-time=%u,", def->os.bm_timeout); - } - - if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_BOOT_STRICT)) - virBufferAddLit(&boot_buf, "strict=on,"); - - virBufferTrim(&boot_buf, ",", -1); - - if (virBufferCheckError(&boot_buf) < 0) + if (qemuBuildBootCommandLine(cmd, def, qemuCaps, &emitBootindex) < 0) goto error; - boot_opts_str = virBufferContentAndReset(&boot_buf); - if (boot_order_str || boot_opts_str) { - virCommandAddArg(cmd, "-boot"); - - 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_opts_str); - VIR_FREE(boot_order_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; - } - } - for (i = 0; i < def->ncontrollers; i++) { virDomainControllerDefPtr cont = def->controllers[i]; if (cont->type == VIR_DOMAIN_CONTROLLER_TYPE_PCI && @@ -8888,9 +8909,6 @@ qemuBuildCommandLine(virConnectPtr conn, return cmd; error: - VIR_FREE(boot_order_str); - VIR_FREE(boot_opts_str); - virBufferFreeAndReset(&boot_buf); virObjectUnref(cfg); /* free up any resources in the network driver * but don't overwrite the original error */ -- 2.5.0

Add new function to manage adding the -global controller options to the command line removing that task from the mainline qemuBuildCommandLine. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_command.c | 106 +++++++++++++++++++++++++++--------------------- 1 file changed, 60 insertions(+), 46 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 57e9bc7..66f7b13 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -5103,6 +5103,64 @@ qemuBuildBootCommandLine(virCommandPtr cmd, static int +qemuBuildGlobalControllerCommandLine(virCommandPtr cmd, + const virDomainDef *def, + virQEMUCapsPtr qemuCaps) +{ + size_t i; + + for (i = 0; i < def->ncontrollers; i++) { + virDomainControllerDefPtr cont = def->controllers[i]; + if (cont->type == VIR_DOMAIN_CONTROLLER_TYPE_PCI && + cont->opts.pciopts.pcihole64) { + const char *hoststr = NULL; + bool cap = false; + bool machine = false; + + switch (cont->model) { + case VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT: + hoststr = "i440FX-pcihost"; + cap = virQEMUCapsGet(qemuCaps, QEMU_CAPS_I440FX_PCI_HOLE64_SIZE); + machine = qemuDomainMachineIsI440FX(def); + break; + + case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT: + hoststr = "q35-pcihost"; + cap = virQEMUCapsGet(qemuCaps, QEMU_CAPS_Q35_PCI_HOLE64_SIZE); + machine = qemuDomainMachineIsQ35(def); + break; + + default: + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("64-bit PCI hole setting is only for root" + " PCI controllers")); + return -1; + } + + if (!machine) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("Setting the 64-bit PCI hole size is not " + "supported for machine '%s'"), def->os.machine); + return -1; + } + if (!cap) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("64-bit PCI hole size setting is not supported " + "with this QEMU binary")); + return -1; + } + + virCommandAddArg(cmd, "-global"); + virCommandAddArgFormat(cmd, "%s.pci-hole64-size=%luK", hoststr, + cont->opts.pciopts.pcihole64size); + } + } + + return 0; +} + + +static int qemuBuildCpuModelArgStr(virQEMUDriverPtr driver, const virDomainDef *def, virBufferPtr buf, @@ -7537,52 +7595,8 @@ qemuBuildCommandLine(virConnectPtr conn, if (qemuBuildBootCommandLine(cmd, def, qemuCaps, &emitBootindex) < 0) goto error; - for (i = 0; i < def->ncontrollers; i++) { - virDomainControllerDefPtr cont = def->controllers[i]; - if (cont->type == VIR_DOMAIN_CONTROLLER_TYPE_PCI && - cont->opts.pciopts.pcihole64) { - const char *hoststr = NULL; - bool cap = false; - bool machine = false; - - switch (cont->model) { - case VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT: - hoststr = "i440FX-pcihost"; - cap = virQEMUCapsGet(qemuCaps, QEMU_CAPS_I440FX_PCI_HOLE64_SIZE); - machine = qemuDomainMachineIsI440FX(def); - break; - - case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT: - hoststr = "q35-pcihost"; - cap = virQEMUCapsGet(qemuCaps, QEMU_CAPS_Q35_PCI_HOLE64_SIZE); - machine = qemuDomainMachineIsQ35(def); - break; - - default: - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("64-bit PCI hole setting is only for root" - " PCI controllers")); - goto error; - } - - if (!machine) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("Setting the 64-bit PCI hole size is not " - "supported for machine '%s'"), def->os.machine); - goto error; - } - if (!cap) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("64-bit PCI hole size setting is not supported " - "with this QEMU binary")); - goto error; - } - - virCommandAddArg(cmd, "-global"); - virCommandAddArgFormat(cmd, "%s.pci-hole64-size=%luK", hoststr, - cont->opts.pciopts.pcihole64size); - } - } + if (qemuBuildGlobalControllerCommandLine(cmd, def, qemuCaps) < 0) + goto error; if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) { for (j = 0; j < ARRAY_CARDINALITY(contOrder); j++) { -- 2.5.0

Add new function to manage adding the controller -device options to the command line removing that task from the mainline qemuBuildCommandLine. Also adjust to using const virDomainDef instead of virDomainDefPtr. This causes collateral damage in order to modify called APIs to use the const virDomainDef instead as well. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/conf/domain_conf.c | 10 +- src/conf/domain_conf.h | 4 +- src/qemu/qemu_command.c | 224 ++++++++++++++++++++++------------------- src/qemu/qemu_command.h | 4 +- src/qemu/qemu_domain_address.c | 2 +- src/qemu/qemu_domain_address.h | 2 +- 6 files changed, 132 insertions(+), 114 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index acd58a1..a2a0255 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -13294,8 +13294,9 @@ void virDomainControllerInsertPreAlloced(virDomainDefPtr def, } int -virDomainControllerFind(virDomainDefPtr def, - int type, int idx) +virDomainControllerFind(const virDomainDef *def, + int type, + int idx) { size_t i; @@ -13323,8 +13324,9 @@ virDomainControllerFindUnusedIndex(virDomainDefPtr def, int type) const char * -virDomainControllerAliasFind(virDomainDefPtr def, - int type, int idx) +virDomainControllerAliasFind(const virDomainDef *def, + int type, + int idx) { int contIndex; const char *contTypeStr = virDomainControllerTypeToString(type); diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 1de3be3..0b2a74f 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2825,12 +2825,12 @@ int virDomainControllerInsert(virDomainDefPtr def, ATTRIBUTE_RETURN_CHECK; void virDomainControllerInsertPreAlloced(virDomainDefPtr def, virDomainControllerDefPtr controller); -int virDomainControllerFind(virDomainDefPtr def, int type, int idx); +int virDomainControllerFind(const virDomainDef *def, int type, int idx); int virDomainControllerFindByType(virDomainDefPtr def, int type); int virDomainControllerFindByPCIAddress(virDomainDefPtr def, virDevicePCIAddressPtr addr); virDomainControllerDefPtr virDomainControllerRemove(virDomainDefPtr def, size_t i); -const char *virDomainControllerAliasFind(virDomainDefPtr def, +const char *virDomainControllerAliasFind(const virDomainDef *def, int type, int idx) ATTRIBUTE_NONNULL(1); diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 66f7b13..d06974f 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -276,7 +276,7 @@ char *qemuDeviceDriveHostAlias(virDomainDiskDefPtr disk, static int qemuBuildDeviceAddressStr(virBufferPtr buf, - virDomainDefPtr domainDef, + const virDomainDef *domainDef, virDomainDeviceInfoPtr info, virQEMUCapsPtr qemuCaps) { @@ -925,7 +925,7 @@ qemuCheckDiskConfig(virDomainDiskDefPtr disk) * an error and return false; otherwise, return true. */ bool -qemuCheckCCWS390AddressSupport(virDomainDefPtr def, +qemuCheckCCWS390AddressSupport(const virDomainDef *def, virDomainDeviceInfo info, virQEMUCapsPtr qemuCaps, const char *devicename) @@ -1904,7 +1904,7 @@ qemuControllerModelUSBToCaps(int model) static int -qemuBuildUSBControllerDevStr(virDomainDefPtr domainDef, +qemuBuildUSBControllerDevStr(const virDomainDef *domainDef, virDomainControllerDefPtr def, virQEMUCapsPtr qemuCaps, virBuffer *buf) @@ -1942,7 +1942,7 @@ qemuBuildUSBControllerDevStr(virDomainDefPtr domainDef, } char * -qemuBuildControllerDevStr(virDomainDefPtr domainDef, +qemuBuildControllerDevStr(const virDomainDef *domainDef, virDomainControllerDefPtr def, virQEMUCapsPtr qemuCaps, int *nusbcontroller) @@ -2297,6 +2297,120 @@ qemuBuildControllerDevStr(virDomainDefPtr domainDef, } +static int +qemuBuildControllerDevCommandLine(virCommandPtr cmd, + const virDomainDef *def, + virQEMUCapsPtr qemuCaps) +{ + size_t i, j; + int usbcontroller = 0; + bool usblegacy = false; + int contOrder[] = { + /* + * List of controller types that we add commandline args for, + * *in the order we want to add them*. + * + * The floppy controller is implicit on PIIX4 and older Q35 + * machines. For newer Q35 machines it is added out of the + * controllers loop, after the floppy drives. + * + * We don't add PCI/PCIe root controller either, because it's + * implicit, but we do add PCI bridges and other PCI + * controllers, so we leave that in to check each + * one. Likewise, we don't do anything for the primary IDE + * controller on an i440fx machine or primary SATA on q35, but + * we do add those beyond these two exceptions. + */ + VIR_DOMAIN_CONTROLLER_TYPE_PCI, + VIR_DOMAIN_CONTROLLER_TYPE_USB, + VIR_DOMAIN_CONTROLLER_TYPE_SCSI, + VIR_DOMAIN_CONTROLLER_TYPE_IDE, + VIR_DOMAIN_CONTROLLER_TYPE_SATA, + VIR_DOMAIN_CONTROLLER_TYPE_VIRTIO_SERIAL, + VIR_DOMAIN_CONTROLLER_TYPE_CCID, + }; + + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) + goto check_add_usb; + + for (j = 0; j < ARRAY_CARDINALITY(contOrder); j++) { + for (i = 0; i < def->ncontrollers; i++) { + virDomainControllerDefPtr cont = def->controllers[i]; + char *devstr; + + if (cont->type != contOrder[j]) + continue; + + /* skip USB controllers with type none.*/ + if (cont->type == VIR_DOMAIN_CONTROLLER_TYPE_USB && + cont->model == VIR_DOMAIN_CONTROLLER_MODEL_USB_NONE) { + usbcontroller = -1; /* mark we don't want a controller */ + continue; + } + + /* skip pci-root/pcie-root */ + if (cont->type == VIR_DOMAIN_CONTROLLER_TYPE_PCI && + (cont->model == VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT || + cont->model == VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT)) + continue; + + /* first SATA controller on Q35 machines is implicit */ + if (cont->type == VIR_DOMAIN_CONTROLLER_TYPE_SATA && + cont->idx == 0 && qemuDomainMachineIsQ35(def)) + continue; + + /* first IDE controller is implicit on various machines */ + if (cont->type == VIR_DOMAIN_CONTROLLER_TYPE_IDE && + cont->idx == 0 && qemuDomainMachineHasBuiltinIDE(def)) + continue; + + if (cont->type == VIR_DOMAIN_CONTROLLER_TYPE_USB && + cont->model == -1 && + !qemuDomainMachineIsQ35(def)) { + bool need_legacy = false; + + /* We're not using legacy usb controller for q35 */ + if (ARCH_IS_PPC64(def->os.arch)) { + /* For ppc64 the legacy was OHCI */ + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_PCI_OHCI)) + need_legacy = true; + } else { + /* For anything else, we used PIIX3_USB_UHCI */ + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_PIIX3_USB_UHCI)) + need_legacy = true; + } + + if (need_legacy) { + if (usblegacy) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("Multiple legacy USB controllers are " + "not supported")); + return -1; + } + usblegacy = true; + continue; + } + } + + virCommandAddArg(cmd, "-device"); + if (!(devstr = qemuBuildControllerDevStr(def, cont, qemuCaps, + &usbcontroller))) + return -1; + virCommandAddArg(cmd, devstr); + VIR_FREE(devstr); + } + } + + check_add_usb: + if (usbcontroller == 0 && + !qemuDomainMachineIsQ35(def) && + !ARCH_IS_S390(def->os.arch)) + virCommandAddArg(cmd, "-usb"); + + return 0; +} + + /** * qemuBuildMemoryBackendStr: * @size: size of the memory device in kibibytes @@ -7456,33 +7570,7 @@ qemuBuildCommandLine(virConnectPtr conn, int last_good_net = -1; virCommandPtr cmd = NULL; bool emitBootindex = false; - int usbcontroller = 0; int actualSerials = 0; - bool usblegacy = false; - int contOrder[] = { - /* - * List of controller types that we add commandline args for, - * *in the order we want to add them*. - * - * The floppy controller is implicit on PIIX4 and older Q35 - * machines. For newer Q35 machines it is added out of the - * controllers loop, after the floppy drives. - * - * We don't add PCI/PCIe root controller either, because it's - * implicit, but we do add PCI bridges and other PCI - * controllers, so we leave that in to check each - * one. Likewise, we don't do anything for the primary IDE - * controller on an i440fx machine or primary SATA on q35, but - * we do add those beyond these two exceptions. - */ - VIR_DOMAIN_CONTROLLER_TYPE_PCI, - VIR_DOMAIN_CONTROLLER_TYPE_USB, - VIR_DOMAIN_CONTROLLER_TYPE_SCSI, - VIR_DOMAIN_CONTROLLER_TYPE_IDE, - VIR_DOMAIN_CONTROLLER_TYPE_SATA, - VIR_DOMAIN_CONTROLLER_TYPE_VIRTIO_SERIAL, - VIR_DOMAIN_CONTROLLER_TYPE_CCID, - }; virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); virBuffer fdc_opts = VIR_BUFFER_INITIALIZER; char *fdc_opts_str = NULL; @@ -7598,80 +7686,8 @@ qemuBuildCommandLine(virConnectPtr conn, if (qemuBuildGlobalControllerCommandLine(cmd, def, qemuCaps) < 0) goto error; - if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) { - for (j = 0; j < ARRAY_CARDINALITY(contOrder); j++) { - for (i = 0; i < def->ncontrollers; i++) { - virDomainControllerDefPtr cont = def->controllers[i]; - char *devstr; - - if (cont->type != contOrder[j]) - continue; - - /* skip USB controllers with type none.*/ - if (cont->type == VIR_DOMAIN_CONTROLLER_TYPE_USB && - cont->model == VIR_DOMAIN_CONTROLLER_MODEL_USB_NONE) { - usbcontroller = -1; /* mark we don't want a controller */ - continue; - } - - /* skip pci-root/pcie-root */ - if (cont->type == VIR_DOMAIN_CONTROLLER_TYPE_PCI && - (cont->model == VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT || - cont->model == VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT)) - continue; - - /* first SATA controller on Q35 machines is implicit */ - if (cont->type == VIR_DOMAIN_CONTROLLER_TYPE_SATA && - cont->idx == 0 && qemuDomainMachineIsQ35(def)) - continue; - - /* first IDE controller is implicit on various machines */ - if (cont->type == VIR_DOMAIN_CONTROLLER_TYPE_IDE && - cont->idx == 0 && qemuDomainMachineHasBuiltinIDE(def)) - continue; - - if (cont->type == VIR_DOMAIN_CONTROLLER_TYPE_USB && - cont->model == -1 && - !qemuDomainMachineIsQ35(def)) { - bool need_legacy = false; - - /* We're not using legacy usb controller for q35 */ - if (ARCH_IS_PPC64(def->os.arch)) { - /* For ppc64 the legacy was OHCI */ - if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_PCI_OHCI)) - need_legacy = true; - } else { - /* For anything else, we used PIIX3_USB_UHCI */ - if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_PIIX3_USB_UHCI)) - need_legacy = true; - } - - if (need_legacy) { - if (usblegacy) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("Multiple legacy USB controllers are " - "not supported")); - goto error; - } - usblegacy = true; - continue; - } - } - - virCommandAddArg(cmd, "-device"); - if (!(devstr = qemuBuildControllerDevStr(def, cont, qemuCaps, - &usbcontroller))) - goto error; - virCommandAddArg(cmd, devstr); - VIR_FREE(devstr); - } - } - } - - if (usbcontroller == 0 && - !qemuDomainMachineIsQ35(def) && - !ARCH_IS_S390(def->os.arch)) - virCommandAddArg(cmd, "-usb"); + if (qemuBuildControllerDevCommandLine(cmd, def, qemuCaps) < 0) + goto error; for (i = 0; i < def->nhubs; i++) { virDomainHubDefPtr hub = def->hubs[i]; diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h index fb684d0..a2a566e 100644 --- a/src/qemu/qemu_command.h +++ b/src/qemu/qemu_command.h @@ -124,7 +124,7 @@ char *qemuBuildFSDevStr(virDomainDefPtr domainDef, virDomainFSDefPtr fs, virQEMUCapsPtr qemuCaps); /* Current, best practice */ -char *qemuBuildControllerDevStr(virDomainDefPtr domainDef, +char *qemuBuildControllerDevStr(const virDomainDef *domainDef, virDomainControllerDefPtr def, virQEMUCapsPtr qemuCaps, int *nusbcontroller); @@ -220,7 +220,7 @@ int qemuCheckDiskConfig(virDomainDiskDefPtr disk); bool qemuCheckFips(void); -bool qemuCheckCCWS390AddressSupport(virDomainDefPtr def, +bool qemuCheckCCWS390AddressSupport(const virDomainDef *def, virDomainDeviceInfo info, virQEMUCapsPtr qemuCaps, const char *devicename); diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c index 6259d87..a192f94 100644 --- a/src/qemu/qemu_domain_address.c +++ b/src/qemu/qemu_domain_address.c @@ -40,7 +40,7 @@ VIR_LOG_INIT("qemu.qemu_domain_address"); int -qemuDomainSetSCSIControllerModel(virDomainDefPtr def, +qemuDomainSetSCSIControllerModel(const virDomainDef *def, virQEMUCapsPtr qemuCaps, int *model) { diff --git a/src/qemu/qemu_domain_address.h b/src/qemu/qemu_domain_address.h index 855f4e5..50019b8 100644 --- a/src/qemu/qemu_domain_address.h +++ b/src/qemu/qemu_domain_address.h @@ -27,7 +27,7 @@ # include "domain_conf.h" # include "qemu_capabilities.h" -int qemuDomainSetSCSIControllerModel(virDomainDefPtr def, +int qemuDomainSetSCSIControllerModel(const virDomainDef *def, virQEMUCapsPtr qemuCaps, int *model); -- 2.5.0

Add new function to manage adding the hub -device options to the command line removing that task from the mainline qemuBuildCommandLine. Also make qemuBuildHubDevStr static to the module since it's only used here. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_command.c | 38 ++++++++++++++++++++++++++------------ src/qemu/qemu_command.h | 3 --- 2 files changed, 26 insertions(+), 15 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index d06974f..45c6c63 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -3732,8 +3732,8 @@ qemuBuildUSBHostdevDevStr(virDomainDefPtr def, } -char * -qemuBuildHubDevStr(virDomainDefPtr def, +static char * +qemuBuildHubDevStr(const virDomainDef *def, virDomainHubDefPtr dev, virQEMUCapsPtr qemuCaps) { @@ -3768,6 +3768,28 @@ qemuBuildHubDevStr(virDomainDefPtr def, } +static int +qemuBuildHubCommandLine(virCommandPtr cmd, + const virDomainDef *def, + virQEMUCapsPtr qemuCaps) +{ + size_t i; + + for (i = 0; i < def->nhubs; i++) { + virDomainHubDefPtr hub = def->hubs[i]; + char *optstr; + + virCommandAddArg(cmd, "-device"); + if (!(optstr = qemuBuildHubDevStr(def, hub, qemuCaps))) + return -1; + virCommandAddArg(cmd, optstr); + VIR_FREE(optstr); + } + + return 0; +} + + char * qemuBuildUSBHostdevUSBDevStr(virDomainHostdevDefPtr dev) { @@ -7689,16 +7711,8 @@ qemuBuildCommandLine(virConnectPtr conn, if (qemuBuildControllerDevCommandLine(cmd, def, qemuCaps) < 0) goto error; - for (i = 0; i < def->nhubs; i++) { - virDomainHubDefPtr hub = def->hubs[i]; - char *optstr; - - virCommandAddArg(cmd, "-device"); - if (!(optstr = qemuBuildHubDevStr(def, hub, qemuCaps))) - goto error; - virCommandAddArg(cmd, optstr); - VIR_FREE(optstr); - } + if (qemuBuildHubCommandLine(cmd, def, qemuCaps) < 0) + goto error; if ((virQEMUCapsGet(qemuCaps, QEMU_CAPS_DRIVE_BOOT) || emitBootindex)) { /* bootDevs will get translated into either bootindex=N or boot=on diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h index a2a566e..2e8521c 100644 --- a/src/qemu/qemu_command.h +++ b/src/qemu/qemu_command.h @@ -202,9 +202,6 @@ char *qemuBuildSCSIHostdevDevStr(virDomainDefPtr def, virDomainHostdevDefPtr dev, virQEMUCapsPtr qemuCaps); -char *qemuBuildHubDevStr(virDomainDefPtr def, - virDomainHubDefPtr dev, - virQEMUCapsPtr qemuCaps); char *qemuBuildRedirdevDevStr(virDomainDefPtr def, virDomainRedirdevDefPtr dev, virQEMUCapsPtr qemuCaps); -- 2.5.0

Add new function to manage adding the disk -drive options to the command line removing that task from the mainline qemuBuildCommandLine. Also since using const virDomainDef in new function, that means other functions called needed to change their usage. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/conf/domain_conf.c | 4 +- src/conf/domain_conf.h | 4 +- src/qemu/qemu_command.c | 317 +++++++++++++++++++++++++----------------------- src/qemu/qemu_command.h | 2 +- 4 files changed, 173 insertions(+), 154 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index a2a0255..4fdd284 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -5738,7 +5738,7 @@ virDomainHostdevDefParseXMLCaps(xmlNodePtr node ATTRIBUTE_UNUSED, } int -virDomainDeviceFindControllerModel(virDomainDefPtr def, +virDomainDeviceFindControllerModel(const virDomainDef *def, virDomainDeviceInfoPtr info, int controllerType) { @@ -18254,7 +18254,7 @@ virDomainDefAddImplicitControllers(virDomainDefPtr def) } virDomainIOThreadIDDefPtr -virDomainIOThreadIDFind(virDomainDefPtr def, +virDomainIOThreadIDFind(const virDomainDef *def, unsigned int iothread_id) { size_t i; diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 0b2a74f..694a6cc 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2512,7 +2512,7 @@ int virDomainDiskSetDriver(virDomainDiskDefPtr def, const char *name) ATTRIBUTE_RETURN_CHECK; int virDomainDiskGetFormat(virDomainDiskDefPtr def); void virDomainDiskSetFormat(virDomainDiskDefPtr def, int format); -int virDomainDeviceFindControllerModel(virDomainDefPtr def, +int virDomainDeviceFindControllerModel(const virDomainDef *def, virDomainDeviceInfoPtr info, int controllerType); virDomainDiskDefPtr virDomainDiskFindByBusAndDst(virDomainDefPtr def, @@ -2695,7 +2695,7 @@ bool virDomainDefCheckABIStability(virDomainDefPtr src, int virDomainDefAddImplicitControllers(virDomainDefPtr def); -virDomainIOThreadIDDefPtr virDomainIOThreadIDFind(virDomainDefPtr def, +virDomainIOThreadIDDefPtr virDomainIOThreadIDFind(const virDomainDef *def, unsigned int iothread_id); virDomainIOThreadIDDefPtr virDomainIOThreadIDAdd(virDomainDefPtr def, unsigned int iothread_id); diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 45c6c63..ae478d8 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -1443,7 +1443,7 @@ qemuBuildDriveStr(virConnectPtr conn, static bool -qemuCheckIOThreads(virDomainDefPtr def, +qemuCheckIOThreads(const virDomainDef *def, virDomainDiskDefPtr disk) { /* Right "type" of disk" */ @@ -1469,7 +1469,7 @@ qemuCheckIOThreads(virDomainDefPtr def, char * -qemuBuildDriveDevStr(virDomainDefPtr def, +qemuBuildDriveDevStr(const virDomainDef *def, virDomainDiskDefPtr disk, int bootindex, virQEMUCapsPtr qemuCaps) @@ -1766,6 +1766,168 @@ qemuBuildDriveDevStr(virDomainDefPtr def, } +static int +qemuBuildDiskDriveCommandLine(virCommandPtr cmd, + virConnectPtr conn, + const virDomainDef *def, + virQEMUCapsPtr qemuCaps, + bool emitBootindex) +{ + size_t i; + int bootCD = 0, bootFloppy = 0, bootDisk = 0; + virBuffer fdc_opts = VIR_BUFFER_INITIALIZER; + char *fdc_opts_str = NULL; + + if ((virQEMUCapsGet(qemuCaps, QEMU_CAPS_DRIVE_BOOT) || emitBootindex)) { + /* bootDevs will get translated into either bootindex=N or boot=on + * depending on what qemu supports */ + for (i = 0; i < def->os.nBootDevs; i++) { + switch (def->os.bootDevs[i]) { + case VIR_DOMAIN_BOOT_CDROM: + bootCD = i + 1; + break; + case VIR_DOMAIN_BOOT_FLOPPY: + bootFloppy = i + 1; + break; + case VIR_DOMAIN_BOOT_DISK: + bootDisk = i + 1; + break; + } + } + } + + for (i = 0; i < def->ndisks; i++) { + char *optstr; + int bootindex = 0; + virDomainDiskDefPtr disk = def->disks[i]; + bool withDeviceArg = false; + bool deviceFlagMasked = false; + + /* Unless we have -device, then USB disks need special + handling */ + if ((disk->bus == VIR_DOMAIN_DISK_BUS_USB) && + !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) { + if (disk->device == VIR_DOMAIN_DISK_DEVICE_DISK) { + virCommandAddArg(cmd, "-usbdevice"); + virCommandAddArgFormat(cmd, "disk:%s", disk->src->path); + } else { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("unsupported usb disk type for '%s'"), + disk->src->path); + return -1; + } + continue; + } + + /* PowerPC pseries based VMs do not support floppy device */ + if ((disk->device == VIR_DOMAIN_DISK_DEVICE_FLOPPY) && + ARCH_IS_PPC64(def->os.arch) && + STRPREFIX(def->os.machine, "pseries")) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("PowerPC pseries machines do not support floppy device")); + return -1; + } + + switch (disk->device) { + case VIR_DOMAIN_DISK_DEVICE_CDROM: + 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; + bootDisk = 0; + break; + } + + virCommandAddArg(cmd, "-drive"); + + /* Unfortunately it is not possible to use + -device for floppies, xen PV, or SD + devices. Fortunately, those don't need + static PCI addresses, so we don't really + care that we can't use -device */ + if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) { + if (disk->bus != VIR_DOMAIN_DISK_BUS_XEN && + disk->bus != VIR_DOMAIN_DISK_BUS_SD) { + withDeviceArg = true; + } else { + virQEMUCapsClear(qemuCaps, QEMU_CAPS_DEVICE); + deviceFlagMasked = true; + } + } + optstr = qemuBuildDriveStr(conn, disk, + emitBootindex ? false : !!bootindex, + qemuCaps); + if (deviceFlagMasked) + virQEMUCapsSet(qemuCaps, QEMU_CAPS_DEVICE); + if (!optstr) + return -1; + virCommandAddArg(cmd, optstr); + VIR_FREE(optstr); + + if (!emitBootindex) + bootindex = 0; + else if (disk->info.bootIndex) + bootindex = disk->info.bootIndex; + + if (withDeviceArg) { + if (disk->bus == VIR_DOMAIN_DISK_BUS_FDC) { + if (virAsprintf(&optstr, "drive%c=drive-%s", + disk->info.addr.drive.unit ? 'B' : 'A', + disk->info.alias) < 0) + return -1; + + if (!qemuDomainMachineNeedsFDC(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=%d", + disk->info.addr.drive.unit + ? 'B' : 'A', + bootindex) < 0) + return -1; + + if (!qemuDomainMachineNeedsFDC(def)) { + virCommandAddArg(cmd, "-global"); + virCommandAddArgFormat(cmd, "isa-fdc.%s", optstr); + } else { + virBufferAsprintf(&fdc_opts, "%s,", optstr); + } + VIR_FREE(optstr); + } + } else { + virCommandAddArg(cmd, "-device"); + + if (!(optstr = qemuBuildDriveDevStr(def, disk, bootindex, + qemuCaps))) + return -1; + virCommandAddArg(cmd, optstr); + VIR_FREE(optstr); + } + } + } + /* Newer Q35 machine types require an explicit FDC controller */ + virBufferTrim(&fdc_opts, ",", -1); + if ((fdc_opts_str = virBufferContentAndReset(&fdc_opts))) { + virCommandAddArg(cmd, "-device"); + virCommandAddArgFormat(cmd, "isa-fdc,%s", fdc_opts_str); + VIR_FREE(fdc_opts_str); + } + + return 0; +} + + char *qemuBuildFSStr(virDomainFSDefPtr fs, virQEMUCapsPtr qemuCaps ATTRIBUTE_UNUSED) { @@ -7594,9 +7756,7 @@ qemuBuildCommandLine(virConnectPtr conn, bool emitBootindex = false; int actualSerials = 0; virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); - virBuffer fdc_opts = VIR_BUFFER_INITIALIZER; - char *fdc_opts_str = NULL; - int bootCD = 0, bootFloppy = 0, bootDisk = 0, bootHostdevNet = 0; + int bootHostdevNet = 0; VIR_DEBUG("conn=%p driver=%p def=%p mon=%p json=%d " @@ -7714,150 +7874,9 @@ qemuBuildCommandLine(virConnectPtr conn, if (qemuBuildHubCommandLine(cmd, def, qemuCaps) < 0) goto error; - if ((virQEMUCapsGet(qemuCaps, QEMU_CAPS_DRIVE_BOOT) || emitBootindex)) { - /* bootDevs will get translated into either bootindex=N or boot=on - * depending on what qemu supports */ - for (i = 0; i < def->os.nBootDevs; i++) { - switch (def->os.bootDevs[i]) { - case VIR_DOMAIN_BOOT_CDROM: - bootCD = i + 1; - break; - case VIR_DOMAIN_BOOT_FLOPPY: - bootFloppy = i + 1; - break; - case VIR_DOMAIN_BOOT_DISK: - bootDisk = i + 1; - break; - } - } - } - - for (i = 0; i < def->ndisks; i++) { - char *optstr; - int bootindex = 0; - virDomainDiskDefPtr disk = def->disks[i]; - bool withDeviceArg = false; - bool deviceFlagMasked = false; - - /* Unless we have -device, then USB disks need special - handling */ - if ((disk->bus == VIR_DOMAIN_DISK_BUS_USB) && - !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) { - if (disk->device == VIR_DOMAIN_DISK_DEVICE_DISK) { - virCommandAddArg(cmd, "-usbdevice"); - virCommandAddArgFormat(cmd, "disk:%s", disk->src->path); - } else { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("unsupported usb disk type for '%s'"), - disk->src->path); - goto error; - } - continue; - } - - /* PowerPC pseries based VMs do not support floppy device */ - if ((disk->device == VIR_DOMAIN_DISK_DEVICE_FLOPPY) && - ARCH_IS_PPC64(def->os.arch) && STRPREFIX(def->os.machine, "pseries")) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("PowerPC pseries machines do not support floppy device")); - goto error; - } - - switch (disk->device) { - case VIR_DOMAIN_DISK_DEVICE_CDROM: - 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; - bootDisk = 0; - break; - } - - virCommandAddArg(cmd, "-drive"); - - /* Unfortunately it is not possible to use - -device for floppies, xen PV, or SD - devices. Fortunately, those don't need - static PCI addresses, so we don't really - care that we can't use -device */ - if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) { - if (disk->bus != VIR_DOMAIN_DISK_BUS_XEN && - disk->bus != VIR_DOMAIN_DISK_BUS_SD) { - withDeviceArg = true; - } else { - virQEMUCapsClear(qemuCaps, QEMU_CAPS_DEVICE); - deviceFlagMasked = true; - } - } - optstr = qemuBuildDriveStr(conn, disk, - emitBootindex ? false : !!bootindex, - qemuCaps); - if (deviceFlagMasked) - virQEMUCapsSet(qemuCaps, QEMU_CAPS_DEVICE); - if (!optstr) - goto error; - virCommandAddArg(cmd, optstr); - VIR_FREE(optstr); - - if (!emitBootindex) - bootindex = 0; - else if (disk->info.bootIndex) - bootindex = disk->info.bootIndex; - - if (withDeviceArg) { - if (disk->bus == VIR_DOMAIN_DISK_BUS_FDC) { - if (virAsprintf(&optstr, "drive%c=drive-%s", - disk->info.addr.drive.unit ? 'B' : 'A', - disk->info.alias) < 0) - goto error; - - if (!qemuDomainMachineNeedsFDC(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=%d", - disk->info.addr.drive.unit - ? 'B' : 'A', - bootindex) < 0) - goto error; - - if (!qemuDomainMachineNeedsFDC(def)) { - virCommandAddArg(cmd, "-global"); - virCommandAddArgFormat(cmd, "isa-fdc.%s", optstr); - } else { - virBufferAsprintf(&fdc_opts, "%s,", optstr); - } - VIR_FREE(optstr); - } - } else { - virCommandAddArg(cmd, "-device"); - - if (!(optstr = qemuBuildDriveDevStr(def, disk, bootindex, - qemuCaps))) - goto error; - virCommandAddArg(cmd, optstr); - VIR_FREE(optstr); - } - } - } - /* Newer Q35 machine types require an explicit FDC controller */ - virBufferTrim(&fdc_opts, ",", -1); - if ((fdc_opts_str = virBufferContentAndReset(&fdc_opts))) { - virCommandAddArg(cmd, "-device"); - virCommandAddArgFormat(cmd, "isa-fdc,%s", fdc_opts_str); - VIR_FREE(fdc_opts_str); - } + if (qemuBuildDiskDriveCommandLine(cmd, conn, def, qemuCaps, + emitBootindex) < 0) + goto error; if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_FSDEV)) { for (i = 0; i < def->nfss; i++) { diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h index 2e8521c..9bed794 100644 --- a/src/qemu/qemu_command.h +++ b/src/qemu/qemu_command.h @@ -116,7 +116,7 @@ char *qemuBuildFSStr(virDomainFSDefPtr fs, virQEMUCapsPtr qemuCaps); /* Current, best practice */ -char *qemuBuildDriveDevStr(virDomainDefPtr def, +char *qemuBuildDriveDevStr(const virDomainDef *def, virDomainDiskDefPtr disk, int bootindex, virQEMUCapsPtr qemuCaps); -- 2.5.0

Add new function to manage adding the -fsdev options to the command line removing that task from the mainline qemuBuildCommandLine. Since both qemuBuildFSStr and qemuBuildFSDevStr are local, make them static and fix their prototypes to use the const virDomainDef as well. Make some minor formatting changes for long lines. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_command.c | 74 ++++++++++++++++++++++++++++++------------------- src/qemu/qemu_command.h | 6 +--- 2 files changed, 46 insertions(+), 34 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index ae478d8..7b25a7c 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -1928,8 +1928,9 @@ qemuBuildDiskDriveCommandLine(virCommandPtr cmd, } -char *qemuBuildFSStr(virDomainFSDefPtr fs, - virQEMUCapsPtr qemuCaps ATTRIBUTE_UNUSED) +static char * +qemuBuildFSStr(virDomainFSDefPtr fs, + virQEMUCapsPtr qemuCaps) { virBuffer opt = VIR_BUFFER_INITIALIZER; const char *driver = qemuDomainFSDriverTypeToString(fs->fsdriver); @@ -2002,8 +2003,8 @@ char *qemuBuildFSStr(virDomainFSDefPtr fs, } -char * -qemuBuildFSDevStr(virDomainDefPtr def, +static char * +qemuBuildFSDevStr(const virDomainDef *def, virDomainFSDefPtr fs, virQEMUCapsPtr qemuCaps) { @@ -2021,7 +2022,8 @@ qemuBuildFSDevStr(virDomainDefPtr def, virBufferAddLit(&opt, "virtio-9p-pci"); virBufferAsprintf(&opt, ",id=%s", fs->info.alias); - virBufferAsprintf(&opt, ",fsdev=%s%s", QEMU_FSDEV_HOST_PREFIX, fs->info.alias); + virBufferAsprintf(&opt, ",fsdev=%s%s", + QEMU_FSDEV_HOST_PREFIX, fs->info.alias); virBufferAsprintf(&opt, ",mount_tag=%s", fs->dst); if (qemuBuildDeviceAddressStr(&opt, def, &fs->info, qemuCaps) < 0) @@ -2039,6 +2041,42 @@ qemuBuildFSDevStr(virDomainDefPtr def, static int +qemuBuildFSDevCommandLine(virCommandPtr cmd, + const virDomainDef *def, + virQEMUCapsPtr qemuCaps) +{ + size_t i; + + if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_FSDEV)) { + for (i = 0; i < def->nfss; i++) { + char *optstr; + virDomainFSDefPtr fs = def->fss[i]; + + virCommandAddArg(cmd, "-fsdev"); + if (!(optstr = qemuBuildFSStr(fs, qemuCaps))) + return -1; + virCommandAddArg(cmd, optstr); + VIR_FREE(optstr); + + virCommandAddArg(cmd, "-device"); + if (!(optstr = qemuBuildFSDevStr(def, fs, qemuCaps))) + return -1; + virCommandAddArg(cmd, optstr); + VIR_FREE(optstr); + } + } else { + if (def->nfss) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("filesystem passthrough not supported by this QEMU")); + return -1; + } + } + + return 0; +} + + +static int qemuControllerModelUSBToCaps(int model) { switch (model) { @@ -7878,30 +7916,8 @@ qemuBuildCommandLine(virConnectPtr conn, emitBootindex) < 0) goto error; - if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_FSDEV)) { - for (i = 0; i < def->nfss; i++) { - char *optstr; - virDomainFSDefPtr fs = def->fss[i]; - - virCommandAddArg(cmd, "-fsdev"); - if (!(optstr = qemuBuildFSStr(fs, qemuCaps))) - goto error; - virCommandAddArg(cmd, optstr); - VIR_FREE(optstr); - - virCommandAddArg(cmd, "-device"); - if (!(optstr = qemuBuildFSDevStr(def, fs, qemuCaps))) - goto error; - virCommandAddArg(cmd, optstr); - VIR_FREE(optstr); - } - } else { - if (def->nfss) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("filesystem passthrough not supported by this QEMU")); - goto error; - } - } + if (qemuBuildFSDevCommandLine(cmd, def, qemuCaps) < 0) + goto error; if (!def->nnets) { /* If we have -device, then we set -nodefault already */ diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h index 9bed794..2a71ac6 100644 --- a/src/qemu/qemu_command.h +++ b/src/qemu/qemu_command.h @@ -112,17 +112,13 @@ char *qemuBuildDriveStr(virConnectPtr conn, virDomainDiskDefPtr disk, bool bootable, virQEMUCapsPtr qemuCaps); -char *qemuBuildFSStr(virDomainFSDefPtr fs, - virQEMUCapsPtr qemuCaps); /* Current, best practice */ char *qemuBuildDriveDevStr(const virDomainDef *def, virDomainDiskDefPtr disk, int bootindex, virQEMUCapsPtr qemuCaps); -char *qemuBuildFSDevStr(virDomainDefPtr domainDef, - virDomainFSDefPtr fs, - virQEMUCapsPtr qemuCaps); + /* Current, best practice */ char *qemuBuildControllerDevStr(const virDomainDef *domainDef, virDomainControllerDefPtr def, -- 2.5.0

Add new function to manage adding the network device options to the command line removing that task from the mainline qemuBuildCommandLine. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_command.c | 133 +++++++++++++++++++++++++++++------------------- 1 file changed, 82 insertions(+), 51 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 7b25a7c..c4d5f7e 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -7248,6 +7248,84 @@ qemuBuildInterfaceCommandLine(virCommandPtr cmd, return ret; } + +/* NOTE: Not using const virDomainDef here since eventually a call is made + * into virSecurityManagerSetTapFDLabel which calls it's driver + * API domainSetSecurityTapFDLabel that doesn't use the const format. + */ +static int +qemuBuildNetCommandLine(virCommandPtr cmd, + virQEMUDriverPtr driver, + virDomainDefPtr def, + virQEMUCapsPtr qemuCaps, + virNetDevVPortProfileOp vmop, + bool standalone, + bool emitBootindex, + size_t *nnicindexes, + int **nicindexes, + int *bootHostdevNet) +{ + size_t i; + int last_good_net = -1; + + if (!def->nnets) { + /* If we have -device, then we set -nodefault already */ + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) + virCommandAddArgList(cmd, "-net", "none", NULL); + } else { + int bootNet = 0; + + if (emitBootindex) { + /* convert <boot dev='network'/> to bootindex since we didn't emit + * -boot n + */ + for (i = 0; i < def->os.nBootDevs; i++) { + if (def->os.bootDevs[i] == VIR_DOMAIN_BOOT_NET) { + bootNet = i + 1; + break; + } + } + } + + for (i = 0; i < def->nnets; i++) { + virDomainNetDefPtr net = def->nets[i]; + int vlan; + + /* VLANs are not used with -netdev, so don't record them */ + if (qemuDomainSupportsNetdev(def, qemuCaps, net)) + vlan = -1; + else + vlan = i; + + if (qemuBuildInterfaceCommandLine(cmd, driver, def, net, + qemuCaps, vlan, bootNet, vmop, + standalone, nnicindexes, + nicindexes) < 0) + goto error; + + last_good_net = i; + /* if this interface is a type='hostdev' interface and we + * haven't yet added a "bootindex" parameter to an + * emulated network device, save the bootindex - hostdev + * interface commandlines will be built later on when we + * cycle through all the hostdevs, and we'll use it then. + */ + if (virDomainNetGetActualType(net) == VIR_DOMAIN_NET_TYPE_HOSTDEV && + *bootHostdevNet == 0) { + *bootHostdevNet = bootNet; + } + bootNet = 0; + } + } + return 0; + + error: + for (i = 0; last_good_net != -1 && i <= last_good_net; i++) + virDomainConfNWFilterTeardown(def->nets[i]); + return -1; +} + + char * qemuBuildShmemDevStr(virDomainDefPtr def, virDomainShmemDefPtr shmem, @@ -7789,7 +7867,6 @@ qemuBuildCommandLine(virConnectPtr conn, size_t i, j; char uuid[VIR_UUID_STRING_BUFLEN]; bool havespice = false; - int last_good_net = -1; virCommandPtr cmd = NULL; bool emitBootindex = false; int actualSerials = 0; @@ -7919,54 +7996,10 @@ qemuBuildCommandLine(virConnectPtr conn, if (qemuBuildFSDevCommandLine(cmd, def, qemuCaps) < 0) goto error; - if (!def->nnets) { - /* If we have -device, then we set -nodefault already */ - if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) - virCommandAddArgList(cmd, "-net", "none", NULL); - } else { - int bootNet = 0; - - if (emitBootindex) { - /* convert <boot dev='network'/> to bootindex since we didn't emit - * -boot n - */ - for (i = 0; i < def->os.nBootDevs; i++) { - if (def->os.bootDevs[i] == VIR_DOMAIN_BOOT_NET) { - bootNet = i + 1; - break; - } - } - } - - for (i = 0; i < def->nnets; i++) { - virDomainNetDefPtr net = def->nets[i]; - int vlan; - - /* VLANs are not used with -netdev, so don't record them */ - if (qemuDomainSupportsNetdev(def, qemuCaps, net)) - vlan = -1; - else - vlan = i; - - if (qemuBuildInterfaceCommandLine(cmd, driver, def, net, - qemuCaps, vlan, bootNet, vmop, - standalone, nnicindexes, nicindexes) < 0) - goto error; - - last_good_net = i; - /* if this interface is a type='hostdev' interface and we - * haven't yet added a "bootindex" parameter to an - * emulated network device, save the bootindex - hostdev - * interface commandlines will be built later on when we - * cycle through all the hostdevs, and we'll use it then. - */ - if (virDomainNetGetActualType(net) == VIR_DOMAIN_NET_TYPE_HOSTDEV && - bootHostdevNet == 0) { - bootHostdevNet = bootNet; - } - bootNet = 0; - } - } + if (qemuBuildNetCommandLine(cmd, driver, def, qemuCaps, vmop, standalone, + emitBootindex, nnicindexes, nicindexes, + &bootHostdevNet) < 0) + goto error; if (def->nsmartcards) { /* -device usb-ccid was already emitted along with other @@ -8992,8 +9025,6 @@ qemuBuildCommandLine(virConnectPtr conn, /* free up any resources in the network driver * but don't overwrite the original error */ originalError = virSaveLastError(); - for (i = 0; last_good_net != -1 && i <= last_good_net; i++) - virDomainConfNWFilterTeardown(def->nets[i]); virSetError(originalError); virFreeError(originalError); virCommandFree(cmd); -- 2.5.0

Add new function to manage adding the smartcard device options to the command line removing that task from the mainline qemuBuildCommandLine. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_command.c | 219 ++++++++++++++++++++++++++---------------------- 1 file changed, 117 insertions(+), 102 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index c4d5f7e..05b6554 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -7326,6 +7326,121 @@ qemuBuildNetCommandLine(virCommandPtr cmd, } +static int +qemuBuildSmartcardCommandLine(virCommandPtr cmd, + const virDomainDef *def, + virQEMUCapsPtr qemuCaps) +{ + size_t i; + + if (!def->nsmartcards) + return 0; + + /* -device usb-ccid was already emitted along with other + * controllers. For now, qemu handles only one smartcard. */ + virDomainSmartcardDefPtr smartcard = def->smartcards[0]; + char *devstr; + virBuffer opt = VIR_BUFFER_INITIALIZER; + const char *database; + + if (def->nsmartcards > 1 || + smartcard->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCID || + smartcard->info.addr.ccid.controller != 0 || + smartcard->info.addr.ccid.slot != 0) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("this QEMU binary lacks multiple smartcard " + "support")); + virBufferFreeAndReset(&opt); + return -1; + } + + switch (smartcard->type) { + case VIR_DOMAIN_SMARTCARD_TYPE_HOST: + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_CHARDEV) || + !virQEMUCapsGet(qemuCaps, QEMU_CAPS_CCID_EMULATED)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("this QEMU binary lacks smartcard host " + "mode support")); + return -1; + } + + virBufferAddLit(&opt, "ccid-card-emulated,backend=nss-emulated"); + break; + + case VIR_DOMAIN_SMARTCARD_TYPE_HOST_CERTIFICATES: + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_CHARDEV) || + !virQEMUCapsGet(qemuCaps, QEMU_CAPS_CCID_EMULATED)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("this QEMU binary lacks smartcard host " + "mode support")); + return -1; + } + + virBufferAddLit(&opt, "ccid-card-emulated,backend=certificates"); + for (i = 0; i < VIR_DOMAIN_SMARTCARD_NUM_CERTIFICATES; i++) { + if (strchr(smartcard->data.cert.file[i], ',')) { + virBufferFreeAndReset(&opt); + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("invalid certificate name: %s"), + smartcard->data.cert.file[i]); + return -1; + } + virBufferAsprintf(&opt, ",cert%zu=%s", i + 1, + smartcard->data.cert.file[i]); + } + if (smartcard->data.cert.database) { + if (strchr(smartcard->data.cert.database, ',')) { + virBufferFreeAndReset(&opt); + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("invalid database name: %s"), + smartcard->data.cert.database); + return -1; + } + database = smartcard->data.cert.database; + } else { + database = VIR_DOMAIN_SMARTCARD_DEFAULT_DATABASE; + } + virBufferAsprintf(&opt, ",db=%s", database); + break; + + case VIR_DOMAIN_SMARTCARD_TYPE_PASSTHROUGH: + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_CHARDEV) || + !virQEMUCapsGet(qemuCaps, QEMU_CAPS_CCID_PASSTHRU)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("this QEMU binary lacks smartcard " + "passthrough mode support")); + return -1; + } + + virCommandAddArg(cmd, "-chardev"); + if (!(devstr = qemuBuildChrChardevStr(&smartcard->data.passthru, + smartcard->info.alias, + qemuCaps))) { + virBufferFreeAndReset(&opt); + return -1; + } + virCommandAddArg(cmd, devstr); + VIR_FREE(devstr); + + virBufferAsprintf(&opt, "ccid-card-passthru,chardev=char%s", + smartcard->info.alias); + break; + + default: + virReportError(VIR_ERR_INTERNAL_ERROR, + _("unexpected smartcard type %d"), + smartcard->type); + virBufferFreeAndReset(&opt); + return -1; + } + virCommandAddArg(cmd, "-device"); + virBufferAsprintf(&opt, ",id=%s,bus=ccid0.0", smartcard->info.alias); + virCommandAddArgBuffer(cmd, &opt); + + return 0; +} + + char * qemuBuildShmemDevStr(virDomainDefPtr def, virDomainShmemDefPtr shmem, @@ -8001,108 +8116,8 @@ qemuBuildCommandLine(virConnectPtr conn, &bootHostdevNet) < 0) goto error; - if (def->nsmartcards) { - /* -device usb-ccid was already emitted along with other - * controllers. For now, qemu handles only one smartcard. */ - virDomainSmartcardDefPtr smartcard = def->smartcards[0]; - char *devstr; - virBuffer opt = VIR_BUFFER_INITIALIZER; - const char *database; - - if (def->nsmartcards > 1 || - smartcard->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCID || - smartcard->info.addr.ccid.controller != 0 || - smartcard->info.addr.ccid.slot != 0) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("this QEMU binary lacks multiple smartcard " - "support")); - virBufferFreeAndReset(&opt); - goto error; - } - - switch (smartcard->type) { - case VIR_DOMAIN_SMARTCARD_TYPE_HOST: - if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_CHARDEV) || - !virQEMUCapsGet(qemuCaps, QEMU_CAPS_CCID_EMULATED)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("this QEMU binary lacks smartcard host " - "mode support")); - goto error; - } - - virBufferAddLit(&opt, "ccid-card-emulated,backend=nss-emulated"); - break; - - case VIR_DOMAIN_SMARTCARD_TYPE_HOST_CERTIFICATES: - if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_CHARDEV) || - !virQEMUCapsGet(qemuCaps, QEMU_CAPS_CCID_EMULATED)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("this QEMU binary lacks smartcard host " - "mode support")); - goto error; - } - - virBufferAddLit(&opt, "ccid-card-emulated,backend=certificates"); - for (j = 0; j < VIR_DOMAIN_SMARTCARD_NUM_CERTIFICATES; j++) { - if (strchr(smartcard->data.cert.file[j], ',')) { - virBufferFreeAndReset(&opt); - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("invalid certificate name: %s"), - smartcard->data.cert.file[j]); - goto error; - } - virBufferAsprintf(&opt, ",cert%zu=%s", j + 1, - smartcard->data.cert.file[j]); - } - if (smartcard->data.cert.database) { - if (strchr(smartcard->data.cert.database, ',')) { - virBufferFreeAndReset(&opt); - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("invalid database name: %s"), - smartcard->data.cert.database); - goto error; - } - database = smartcard->data.cert.database; - } else { - database = VIR_DOMAIN_SMARTCARD_DEFAULT_DATABASE; - } - virBufferAsprintf(&opt, ",db=%s", database); - break; - - case VIR_DOMAIN_SMARTCARD_TYPE_PASSTHROUGH: - if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_CHARDEV) || - !virQEMUCapsGet(qemuCaps, QEMU_CAPS_CCID_PASSTHRU)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("this QEMU binary lacks smartcard " - "passthrough mode support")); - goto error; - } - - virCommandAddArg(cmd, "-chardev"); - if (!(devstr = qemuBuildChrChardevStr(&smartcard->data.passthru, - smartcard->info.alias, - qemuCaps))) { - virBufferFreeAndReset(&opt); - goto error; - } - virCommandAddArg(cmd, devstr); - VIR_FREE(devstr); - - virBufferAsprintf(&opt, "ccid-card-passthru,chardev=char%s", - smartcard->info.alias); - break; - - default: - virReportError(VIR_ERR_INTERNAL_ERROR, - _("unexpected smartcard type %d"), - smartcard->type); - virBufferFreeAndReset(&opt); - goto error; - } - virCommandAddArg(cmd, "-device"); - virBufferAsprintf(&opt, ",id=%s,bus=ccid0.0", smartcard->info.alias); - virCommandAddArgBuffer(cmd, &opt); - } + if (qemuBuildSmartcardCommandLine(cmd, def, qemuCaps) < 0) + goto error; if (def->nserials) { for (i = 0; i < def->ngraphics; i++) { -- 2.5.0

Add new function to manage adding the serial device options to the command line removing that task from the mainline qemuBuildCommandLine. Using const virDomainDef causes collateral damage in other called APIs which need to make the similar adjustment Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_capabilities.c | 2 +- src/qemu/qemu_capabilities.h | 2 +- src/qemu/qemu_command.c | 111 ++++++++++++++++++++++++------------------- src/qemu/qemu_command.h | 2 +- 4 files changed, 65 insertions(+), 52 deletions(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 3099e34..9524a21 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -3828,7 +3828,7 @@ virQEMUCapsCacheFree(virQEMUCapsCachePtr cache) bool -virQEMUCapsSupportsChardev(virDomainDefPtr def, +virQEMUCapsSupportsChardev(const virDomainDef *def, virQEMUCapsPtr qemuCaps, virDomainChrDefPtr chr) { diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index e5353de..7f259b7 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -442,7 +442,7 @@ int virQEMUCapsParseDeviceStr(virQEMUCapsPtr qemuCaps, const char *str); VIR_ENUM_DECL(virQEMUCaps); -bool virQEMUCapsSupportsChardev(virDomainDefPtr def, +bool virQEMUCapsSupportsChardev(const virDomainDef *def, virQEMUCapsPtr qemuCaps, virDomainChrDefPtr chr); diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 05b6554..44fefc3 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -4448,7 +4448,7 @@ qemuBuildMonitorCommandLine(virCommandPtr cmd, static char * -qemuBuildVirtioSerialPortDevStr(virDomainDefPtr def, +qemuBuildVirtioSerialPortDevStr(const virDomainDef *def, virDomainChrDefPtr dev, virQEMUCapsPtr qemuCaps) { @@ -7553,7 +7553,7 @@ qemuBuildShmemCommandLine(virCommandPtr cmd, static int qemuBuildChrDeviceCommandLine(virCommandPtr cmd, - virDomainDefPtr def, + const virDomainDef *def, virDomainChrDefPtr chr, virQEMUCapsPtr qemuCaps) { @@ -7567,6 +7567,60 @@ qemuBuildChrDeviceCommandLine(virCommandPtr cmd, return 0; } + +static int +qemuBuildSerialCommandLine(virCommandPtr cmd, + const virDomainDef *def, + virQEMUCapsPtr qemuCaps) +{ + size_t i; + int actualSerials = 0; + bool havespice = false; + + if (def->nserials) { + for (i = 0; i < def->ngraphics && !havespice; i++) { + if (def->graphics[i]->type == VIR_DOMAIN_GRAPHICS_TYPE_SPICE) + havespice = true; + } + } + + for (i = 0; i < def->nserials; i++) { + virDomainChrDefPtr serial = def->serials[i]; + char *devstr; + + if (serial->source.type == VIR_DOMAIN_CHR_TYPE_SPICEPORT && !havespice) + continue; + + /* Use -chardev with -device if they are available */ + if (virQEMUCapsSupportsChardev(def, qemuCaps, serial)) { + virCommandAddArg(cmd, "-chardev"); + if (!(devstr = qemuBuildChrChardevStr(&serial->source, + serial->info.alias, + qemuCaps))) + return -1; + virCommandAddArg(cmd, devstr); + VIR_FREE(devstr); + + if (qemuBuildChrDeviceCommandLine(cmd, def, serial, qemuCaps) < 0) + return -1; + } else { + virCommandAddArg(cmd, "-serial"); + if (!(devstr = qemuBuildChrArgStr(&serial->source, NULL))) + return -1; + virCommandAddArg(cmd, devstr); + VIR_FREE(devstr); + } + actualSerials++; + } + + /* If we have -device, then we set -nodefault already */ + if (!actualSerials && !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) + virCommandAddArgList(cmd, "-serial", "none", NULL); + + return 0; +} + + static int qemuBuildDomainLoaderCommandLine(virCommandPtr cmd, virDomainDefPtr def, @@ -7981,10 +8035,8 @@ qemuBuildCommandLine(virConnectPtr conn, virErrorPtr originalError = NULL; size_t i, j; char uuid[VIR_UUID_STRING_BUFLEN]; - bool havespice = false; virCommandPtr cmd = NULL; bool emitBootindex = false; - int actualSerials = 0; virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); int bootHostdevNet = 0; @@ -8119,47 +8171,8 @@ qemuBuildCommandLine(virConnectPtr conn, if (qemuBuildSmartcardCommandLine(cmd, def, qemuCaps) < 0) goto error; - if (def->nserials) { - for (i = 0; i < def->ngraphics; i++) { - if (def->graphics[i]->type == VIR_DOMAIN_GRAPHICS_TYPE_SPICE) { - havespice = true; - break; - } - } - } - - for (i = 0; i < def->nserials; i++) { - virDomainChrDefPtr serial = def->serials[i]; - char *devstr; - - if (serial->source.type == VIR_DOMAIN_CHR_TYPE_SPICEPORT && !havespice) - continue; - - /* Use -chardev with -device if they are available */ - if (virQEMUCapsSupportsChardev(def, qemuCaps, serial)) { - virCommandAddArg(cmd, "-chardev"); - if (!(devstr = qemuBuildChrChardevStr(&serial->source, - serial->info.alias, - qemuCaps))) - goto error; - virCommandAddArg(cmd, devstr); - VIR_FREE(devstr); - - if (qemuBuildChrDeviceCommandLine(cmd, def, serial, qemuCaps) < 0) - goto error; - } else { - virCommandAddArg(cmd, "-serial"); - if (!(devstr = qemuBuildChrArgStr(&serial->source, NULL))) - goto error; - virCommandAddArg(cmd, devstr); - VIR_FREE(devstr); - } - actualSerials++; - } - - /* If we have -device, then we set -nodefault already */ - if (!actualSerials && !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) - virCommandAddArgList(cmd, "-serial", "none", NULL); + if (qemuBuildSerialCommandLine(cmd, def, qemuCaps) < 0) + goto error; if (!def->nparallels) { /* If we have -device, then we set -nodefault already */ @@ -9051,7 +9064,7 @@ qemuBuildCommandLine(virConnectPtr conn, */ static int qemuBuildSerialChrDeviceStr(char **deviceStr, - virDomainDefPtr def, + const virDomainDef *def, virDomainChrDefPtr serial, virQEMUCapsPtr qemuCaps, virArch arch, @@ -9146,7 +9159,7 @@ qemuBuildParallelChrDeviceStr(char **deviceStr, static int qemuBuildChannelChrDeviceStr(char **deviceStr, - virDomainDefPtr def, + const virDomainDef *def, virDomainChrDefPtr chr, virQEMUCapsPtr qemuCaps) { @@ -9186,7 +9199,7 @@ qemuBuildChannelChrDeviceStr(char **deviceStr, static int qemuBuildConsoleChrDeviceStr(char **deviceStr, - virDomainDefPtr def, + const virDomainDef *def, virDomainChrDefPtr chr, virQEMUCapsPtr qemuCaps) { @@ -9226,7 +9239,7 @@ qemuBuildConsoleChrDeviceStr(char **deviceStr, int qemuBuildChrDeviceStr(char **deviceStr, - virDomainDefPtr vmdef, + const virDomainDef *vmdef, virDomainChrDefPtr chr, virQEMUCapsPtr qemuCaps) { diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h index 2a71ac6..be5a4c4 100644 --- a/src/qemu/qemu_command.h +++ b/src/qemu/qemu_command.h @@ -77,7 +77,7 @@ virCommandPtr qemuBuildCommandLine(virConnectPtr conn, /* Generate '-device' string for chardev device */ int qemuBuildChrDeviceStr(char **deviceStr, - virDomainDefPtr vmdef, + const virDomainDef *vmdef, virDomainChrDefPtr chr, virQEMUCapsPtr qemuCaps); -- 2.5.0

Add new function to manage adding the parallels device options to the command line removing that task from the mainline qemuBuildCommandLine. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_command.c | 77 +++++++++++++++++++++++++++++-------------------- 1 file changed, 46 insertions(+), 31 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 44fefc3..f161c4f 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -7622,6 +7622,50 @@ qemuBuildSerialCommandLine(virCommandPtr cmd, static int +qemuBuildParallelsCommandLine(virCommandPtr cmd, + const virDomainDef *def, + virQEMUCapsPtr qemuCaps) +{ + size_t i; + + if (!def->nparallels) { + /* If we have -device, then we set -nodefault already */ + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) + virCommandAddArgList(cmd, "-parallel", "none", NULL); + } else { + for (i = 0; i < def->nparallels; i++) { + virDomainChrDefPtr parallel = def->parallels[i]; + char *devstr; + + /* Use -chardev with -device if they are available */ + if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_CHARDEV) && + virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) { + virCommandAddArg(cmd, "-chardev"); + if (!(devstr = qemuBuildChrChardevStr(¶llel->source, + parallel->info.alias, + qemuCaps))) + return -1; + virCommandAddArg(cmd, devstr); + VIR_FREE(devstr); + + if (qemuBuildChrDeviceCommandLine(cmd, def, parallel, + qemuCaps) < 0) + return -1; + } else { + virCommandAddArg(cmd, "-parallel"); + if (!(devstr = qemuBuildChrArgStr(¶llel->source, NULL))) + return -1; + virCommandAddArg(cmd, devstr); + VIR_FREE(devstr); + } + } + } + + return 0; +} + + +static int qemuBuildDomainLoaderCommandLine(virCommandPtr cmd, virDomainDefPtr def, virQEMUCapsPtr qemuCaps) @@ -8174,37 +8218,8 @@ qemuBuildCommandLine(virConnectPtr conn, if (qemuBuildSerialCommandLine(cmd, def, qemuCaps) < 0) goto error; - if (!def->nparallels) { - /* If we have -device, then we set -nodefault already */ - if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) - virCommandAddArgList(cmd, "-parallel", "none", NULL); - } else { - for (i = 0; i < def->nparallels; i++) { - virDomainChrDefPtr parallel = def->parallels[i]; - char *devstr; - - /* Use -chardev with -device if they are available */ - if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_CHARDEV) && - virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) { - virCommandAddArg(cmd, "-chardev"); - if (!(devstr = qemuBuildChrChardevStr(¶llel->source, - parallel->info.alias, - qemuCaps))) - goto error; - virCommandAddArg(cmd, devstr); - VIR_FREE(devstr); - - if (qemuBuildChrDeviceCommandLine(cmd, def, parallel, qemuCaps) < 0) - goto error; - } else { - virCommandAddArg(cmd, "-parallel"); - if (!(devstr = qemuBuildChrArgStr(¶llel->source, NULL))) - goto error; - virCommandAddArg(cmd, devstr); - VIR_FREE(devstr); - } - } - } + if (qemuBuildParallelsCommandLine(cmd, def, qemuCaps) < 0) + goto error; for (i = 0; i < def->nchannels; i++) { virDomainChrDefPtr channel = def->channels[i]; -- 2.5.0

Add new function to manage adding the channel device options to the command line removing that task from the mainline qemuBuildCommandLine. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_command.c | 159 ++++++++++++++++++++++++++---------------------- 1 file changed, 87 insertions(+), 72 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index f161c4f..07328b0 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -7666,6 +7666,91 @@ qemuBuildParallelsCommandLine(virCommandPtr cmd, static int +qemuBuildChannelsCommandLine(virCommandPtr cmd, + virQEMUDriverConfigPtr cfg, + const virDomainDef *def, + virQEMUCapsPtr qemuCaps) +{ + size_t i; + + for (i = 0; i < def->nchannels; i++) { + virDomainChrDefPtr channel = def->channels[i]; + char *devstr; + + switch (channel->targetType) { + case VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_GUESTFWD: + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_CHARDEV) || + !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + "%s", _("guestfwd requires QEMU to support -chardev & -device")); + return -1; + } + + virCommandAddArg(cmd, "-chardev"); + if (!(devstr = qemuBuildChrChardevStr(&channel->source, + channel->info.alias, + qemuCaps))) + return -1; + virCommandAddArg(cmd, devstr); + VIR_FREE(devstr); + + if (qemuBuildChrDeviceStr(&devstr, def, channel, qemuCaps) < 0) + return -1; + virCommandAddArgList(cmd, "-netdev", devstr, NULL); + VIR_FREE(devstr); + break; + + case VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_VIRTIO: + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("virtio channel requires QEMU to support -device")); + return -1; + } + + /* + * TODO: Refactor so that we generate this (and onther + * things) somewhere else then where we are building the + * command line. + */ + if (channel->source.type == VIR_DOMAIN_CHR_TYPE_UNIX && + !channel->source.data.nix.path) { + if (virAsprintf(&channel->source.data.nix.path, + "%s/domain-%s/%s", + cfg->channelTargetDir, def->name, + channel->target.name ? channel->target.name + : "unknown.sock") < 0) + return -1; + + channel->source.data.nix.listen = true; + } + + if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_SPICEVMC) && + channel->source.type == VIR_DOMAIN_CHR_TYPE_SPICEVMC) { + /* spicevmc was originally introduced via a -device + * with a backend internal to qemu; although we prefer + * the newer -chardev interface. */ + ; + } else { + virCommandAddArg(cmd, "-chardev"); + if (!(devstr = qemuBuildChrChardevStr(&channel->source, + channel->info.alias, + qemuCaps))) + return -1; + virCommandAddArg(cmd, devstr); + VIR_FREE(devstr); + } + + if (qemuBuildChrDeviceCommandLine(cmd, def, channel, qemuCaps) < 0) + return -1; + break; + } + } + + return 0; +} + + +static int qemuBuildDomainLoaderCommandLine(virCommandPtr cmd, virDomainDefPtr def, virQEMUCapsPtr qemuCaps) @@ -8221,78 +8306,8 @@ qemuBuildCommandLine(virConnectPtr conn, if (qemuBuildParallelsCommandLine(cmd, def, qemuCaps) < 0) goto error; - for (i = 0; i < def->nchannels; i++) { - virDomainChrDefPtr channel = def->channels[i]; - char *devstr; - - switch (channel->targetType) { - case VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_GUESTFWD: - if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_CHARDEV) || - !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - "%s", _("guestfwd requires QEMU to support -chardev & -device")); - goto error; - } - - virCommandAddArg(cmd, "-chardev"); - if (!(devstr = qemuBuildChrChardevStr(&channel->source, - channel->info.alias, - qemuCaps))) - goto error; - virCommandAddArg(cmd, devstr); - VIR_FREE(devstr); - - if (qemuBuildChrDeviceStr(&devstr, def, channel, qemuCaps) < 0) - goto error; - virCommandAddArgList(cmd, "-netdev", devstr, NULL); - VIR_FREE(devstr); - break; - - case VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_VIRTIO: - if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("virtio channel requires QEMU to support -device")); - goto error; - } - - /* - * TODO: Refactor so that we generate this (and onther - * things) somewhere else then where we are building the - * command line. - */ - if (channel->source.type == VIR_DOMAIN_CHR_TYPE_UNIX && - !channel->source.data.nix.path) { - if (virAsprintf(&channel->source.data.nix.path, - "%s/domain-%s/%s", - cfg->channelTargetDir, def->name, - channel->target.name ? channel->target.name - : "unknown.sock") < 0) - goto error; - - channel->source.data.nix.listen = true; - } - - if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_SPICEVMC) && - channel->source.type == VIR_DOMAIN_CHR_TYPE_SPICEVMC) { - /* spicevmc was originally introduced via a -device - * with a backend internal to qemu; although we prefer - * the newer -chardev interface. */ - ; - } else { - virCommandAddArg(cmd, "-chardev"); - if (!(devstr = qemuBuildChrChardevStr(&channel->source, - channel->info.alias, - qemuCaps))) - goto error; - virCommandAddArg(cmd, devstr); - VIR_FREE(devstr); - } - - if (qemuBuildChrDeviceCommandLine(cmd, def, channel, qemuCaps) < 0) - goto error; - break; - } - } + if (qemuBuildChannelsCommandLine(cmd, cfg, def, qemuCaps) < 0) + goto error; /* Explicit console devices */ for (i = 0; i < def->nconsoles; i++) { -- 2.5.0

On 02/18/2016 12:50 PM, John Ferlan wrote:
Add new function to manage adding the channel device options to the command line removing that task from the mainline qemuBuildCommandLine.
Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_command.c | 159 ++++++++++++++++++++++++++---------------------- 1 file changed, 87 insertions(+), 72 deletions(-)
I've merged my local branch with the latest upstream taking into account Martin's commit 'a89f05ba8d' with the following adjustment: The rest of the changes merged fine. I can repost (in parts) if desired... To make it less onerous to review such a large pile.
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index f161c4f..07328b0 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -7666,6 +7666,91 @@ qemuBuildParallelsCommandLine(virCommandPtr cmd,
static int +qemuBuildChannelsCommandLine(virCommandPtr cmd, + virQEMUDriverConfigPtr cfg, + const virDomainDef *def, + virQEMUCapsPtr qemuCaps) +{ + size_t i; + + for (i = 0; i < def->nchannels; i++) { + virDomainChrDefPtr channel = def->channels[i]; + char *devstr; + + switch (channel->targetType) { + case VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_GUESTFWD: + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_CHARDEV) || + !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + "%s", _("guestfwd requires QEMU to support -chardev & -device")); + return -1; + } + + virCommandAddArg(cmd, "-chardev"); + if (!(devstr = qemuBuildChrChardevStr(&channel->source, + channel->info.alias, + qemuCaps))) + return -1; + virCommandAddArg(cmd, devstr); + VIR_FREE(devstr); + + if (qemuBuildChrDeviceStr(&devstr, def, channel, qemuCaps) < 0) + return -1; + virCommandAddArgList(cmd, "-netdev", devstr, NULL); + VIR_FREE(devstr); + break; + + case VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_VIRTIO: + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("virtio channel requires QEMU to support -device")); + return -1; + } + + /* + * TODO: Refactor so that we generate this (and onther + * things) somewhere else then where we are building the + * command line. + */ + if (channel->source.type == VIR_DOMAIN_CHR_TYPE_UNIX && + !channel->source.data.nix.path) { + if (virAsprintf(&channel->source.data.nix.path, + "%s/domain-%s/%s", + cfg->channelTargetDir, def->name,
- "%s/domain-%s/%s", - cfg->channelTargetDir, def->name, ++ "%s/%s", domainChannelTargetDir,
+ channel->target.name ? channel->target.name + : "unknown.sock") < 0) + return -1; + + channel->source.data.nix.listen = true; + } + + if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_SPICEVMC) && + channel->source.type == VIR_DOMAIN_CHR_TYPE_SPICEVMC) { + /* spicevmc was originally introduced via a -device + * with a backend internal to qemu; although we prefer + * the newer -chardev interface. */ + ; + } else { + virCommandAddArg(cmd, "-chardev"); + if (!(devstr = qemuBuildChrChardevStr(&channel->source, + channel->info.alias, + qemuCaps))) + return -1; + virCommandAddArg(cmd, devstr); + VIR_FREE(devstr); + } + + if (qemuBuildChrDeviceCommandLine(cmd, def, channel, qemuCaps) < 0) + return -1; + break; + } + } + + return 0; +} + + +static int qemuBuildDomainLoaderCommandLine(virCommandPtr cmd, virDomainDefPtr def, virQEMUCapsPtr qemuCaps) @@ -8221,78 +8306,8 @@ qemuBuildCommandLine(virConnectPtr conn, if (qemuBuildParallelsCommandLine(cmd, def, qemuCaps) < 0) goto error;
- for (i = 0; i < def->nchannels; i++) { - virDomainChrDefPtr channel = def->channels[i]; - char *devstr; - - switch (channel->targetType) { - case VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_GUESTFWD: - if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_CHARDEV) || - !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - "%s", _("guestfwd requires QEMU to support -chardev & -device")); - goto error; - } - - virCommandAddArg(cmd, "-chardev"); - if (!(devstr = qemuBuildChrChardevStr(&channel->source, - channel->info.alias, - qemuCaps))) - goto error; - virCommandAddArg(cmd, devstr); - VIR_FREE(devstr); - - if (qemuBuildChrDeviceStr(&devstr, def, channel, qemuCaps) < 0) - goto error; - virCommandAddArgList(cmd, "-netdev", devstr, NULL); - VIR_FREE(devstr); - break; - - case VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_VIRTIO: - if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("virtio channel requires QEMU to support -device")); - goto error; - } - - /* - * TODO: Refactor so that we generate this (and onther - * things) somewhere else then where we are building the - * command line. - */ - if (channel->source.type == VIR_DOMAIN_CHR_TYPE_UNIX && - !channel->source.data.nix.path) { - if (virAsprintf(&channel->source.data.nix.path, - "%s/domain-%s/%s", - cfg->channelTargetDir, def->name, - channel->target.name ? channel->target.name - : "unknown.sock") < 0) - goto error; - - channel->source.data.nix.listen = true; - } - - if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_SPICEVMC) && - channel->source.type == VIR_DOMAIN_CHR_TYPE_SPICEVMC) { - /* spicevmc was originally introduced via a -device - * with a backend internal to qemu; although we prefer - * the newer -chardev interface. */ - ; - } else { - virCommandAddArg(cmd, "-chardev"); - if (!(devstr = qemuBuildChrChardevStr(&channel->source, - channel->info.alias, - qemuCaps))) - goto error; - virCommandAddArg(cmd, devstr); - VIR_FREE(devstr); - } - - if (qemuBuildChrDeviceCommandLine(cmd, def, channel, qemuCaps) < 0) - goto error; - break; - } - } + if (qemuBuildChannelsCommandLine(cmd, cfg, def, qemuCaps) < 0) + goto error;
/* Explicit console devices */ for (i = 0; i < def->nconsoles; i++) {

On Tue, Mar 01, 2016 at 07:43:10 -0500, John Ferlan wrote:
On 02/18/2016 12:50 PM, John Ferlan wrote:
Add new function to manage adding the channel device options to the command line removing that task from the mainline qemuBuildCommandLine.
Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_command.c | 159 ++++++++++++++++++++++++++---------------------- 1 file changed, 87 insertions(+), 72 deletions(-)
I've merged my local branch with the latest upstream taking into account Martin's commit 'a89f05ba8d' with the following adjustment:
The rest of the changes merged fine. I can repost (in parts) if desired... To make it less onerous to review such a large pile.
I'd suggest you push it to a repository where it can be fetched without having to deal with conflicts while applying from email.

Add new function to manage adding the console device options to the command line removing that task from the mainline qemuBuildCommandLine. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_command.c | 134 ++++++++++++++++++++++++++---------------------- 1 file changed, 74 insertions(+), 60 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 07328b0..71eeef0 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -7751,6 +7751,78 @@ qemuBuildChannelsCommandLine(virCommandPtr cmd, static int +qemuBuildConsoleCommandLine(virCommandPtr cmd, + const virDomainDef *def, + virQEMUCapsPtr qemuCaps) +{ + size_t i; + + /* Explicit console devices */ + for (i = 0; i < def->nconsoles; i++) { + virDomainChrDefPtr console = def->consoles[i]; + char *devstr; + + switch (console->targetType) { + case VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SCLP: + case VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SCLPLM: + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("sclp console requires QEMU to support -device")); + return -1; + } + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_SCLP_S390)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("sclp console requires QEMU to support s390-sclp")); + return -1; + } + + virCommandAddArg(cmd, "-chardev"); + if (!(devstr = qemuBuildChrChardevStr(&console->source, + console->info.alias, + qemuCaps))) + return -1; + virCommandAddArg(cmd, devstr); + VIR_FREE(devstr); + + if (qemuBuildChrDeviceCommandLine(cmd, def, console, qemuCaps) < 0) + return -1; + break; + + case VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_VIRTIO: + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("virtio channel requires QEMU to support -device")); + return -1; + } + + virCommandAddArg(cmd, "-chardev"); + if (!(devstr = qemuBuildChrChardevStr(&console->source, + console->info.alias, + qemuCaps))) + return -1; + virCommandAddArg(cmd, devstr); + VIR_FREE(devstr); + + if (qemuBuildChrDeviceCommandLine(cmd, def, console, qemuCaps) < 0) + return -1; + break; + + case VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SERIAL: + break; + + default: + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("unsupported console target type %s"), + NULLSTR(virDomainChrConsoleTargetTypeToString(console->targetType))); + return -1; + } + } + + return 0; +} + + +static int qemuBuildDomainLoaderCommandLine(virCommandPtr cmd, virDomainDefPtr def, virQEMUCapsPtr qemuCaps) @@ -8309,66 +8381,8 @@ qemuBuildCommandLine(virConnectPtr conn, if (qemuBuildChannelsCommandLine(cmd, cfg, def, qemuCaps) < 0) goto error; - /* Explicit console devices */ - for (i = 0; i < def->nconsoles; i++) { - virDomainChrDefPtr console = def->consoles[i]; - char *devstr; - - switch (console->targetType) { - case VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SCLP: - case VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SCLPLM: - if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("sclp console requires QEMU to support -device")); - goto error; - } - if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_SCLP_S390)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("sclp console requires QEMU to support s390-sclp")); - goto error; - } - - virCommandAddArg(cmd, "-chardev"); - if (!(devstr = qemuBuildChrChardevStr(&console->source, - console->info.alias, - qemuCaps))) - goto error; - virCommandAddArg(cmd, devstr); - VIR_FREE(devstr); - - if (qemuBuildChrDeviceCommandLine(cmd, def, console, qemuCaps) < 0) - goto error; - break; - - case VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_VIRTIO: - if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("virtio channel requires QEMU to support -device")); - goto error; - } - - virCommandAddArg(cmd, "-chardev"); - if (!(devstr = qemuBuildChrChardevStr(&console->source, - console->info.alias, - qemuCaps))) - goto error; - virCommandAddArg(cmd, devstr); - VIR_FREE(devstr); - - if (qemuBuildChrDeviceCommandLine(cmd, def, console, qemuCaps) < 0) - goto error; - break; - - case VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SERIAL: - break; - - default: - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("unsupported console target type %s"), - NULLSTR(virDomainChrConsoleTargetTypeToString(console->targetType))); - goto error; - } - } + if (qemuBuildConsoleCommandLine(cmd, def, qemuCaps) < 0) + goto error; if (def->tpm) { if (qemuBuildTPMCommandLine(def, cmd, qemuCaps, def->emulator) < 0) -- 2.5.0

Modify the argument order and types to match other similar helpers. Also modify called functions to use the def->emulator instead of passing def->emulator and def. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_command.c | 28 +++++++++++++--------------- 1 file changed, 13 insertions(+), 15 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 71eeef0..d106f61 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -7894,8 +7894,7 @@ qemuBuildDomainLoaderCommandLine(virCommandPtr cmd, static char * qemuBuildTPMDevStr(const virDomainDef *def, - virQEMUCapsPtr qemuCaps, - const char *emulator) + virQEMUCapsPtr qemuCaps) { virBuffer buf = VIR_BUFFER_INITIALIZER; const virDomainTPMDef *tpm = def->tpm; @@ -7905,7 +7904,7 @@ qemuBuildTPMDevStr(const virDomainDef *def, virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("The QEMU executable %s does not support TPM " "model %s"), - emulator, model); + def->emulator, model); goto error; } @@ -7980,7 +7979,6 @@ static char * qemuBuildTPMBackendStr(const virDomainDef *def, virCommandPtr cmd, virQEMUCapsPtr qemuCaps, - const char *emulator, int *tpmfd, int *cancelfd) { @@ -8056,7 +8054,7 @@ qemuBuildTPMBackendStr(const virDomainDef *def, virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("The QEMU executable %s does not support TPM " "backend type %s"), - emulator, type); + def->emulator, type); error: VIR_FREE(devset); @@ -8068,17 +8066,19 @@ qemuBuildTPMBackendStr(const virDomainDef *def, static int -qemuBuildTPMCommandLine(virDomainDefPtr def, - virCommandPtr cmd, - virQEMUCapsPtr qemuCaps, - const char *emulator) +qemuBuildTPMCommandLine(virCommandPtr cmd, + const virDomainDef *def, + virQEMUCapsPtr qemuCaps) { char *optstr; int tpmfd = -1; int cancelfd = -1; char *fdset; - if (!(optstr = qemuBuildTPMBackendStr(def, cmd, qemuCaps, emulator, + if (!def->tpm) + return 0; + + if (!(optstr = qemuBuildTPMBackendStr(def, cmd, qemuCaps, &tpmfd, &cancelfd))) return -1; @@ -8103,7 +8103,7 @@ qemuBuildTPMCommandLine(virDomainDefPtr def, VIR_FREE(fdset); } - if (!(optstr = qemuBuildTPMDevStr(def, qemuCaps, emulator))) + if (!(optstr = qemuBuildTPMDevStr(def, qemuCaps))) return -1; virCommandAddArgList(cmd, "-device", optstr, NULL); @@ -8384,10 +8384,8 @@ qemuBuildCommandLine(virConnectPtr conn, if (qemuBuildConsoleCommandLine(cmd, def, qemuCaps) < 0) goto error; - if (def->tpm) { - if (qemuBuildTPMCommandLine(def, cmd, qemuCaps, def->emulator) < 0) - goto error; - } + if (qemuBuildTPMCommandLine(cmd, def, qemuCaps) < 0) + goto error; for (i = 0; i < def->ninputs; i++) { virDomainInputDefPtr input = def->inputs[i]; -- 2.5.0

Add new function to manage adding the input device options to the command line removing that task from the mainline qemuBuildCommandLine. Make qemuBuildUSBInputDevStr static since only this module calls it. Also the change to use const virDomainDef forces other changes. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_command.c | 87 +++++++++++++++++++++++++++++-------------------- src/qemu/qemu_command.h | 4 --- 2 files changed, 51 insertions(+), 40 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index d106f61..af09d8c 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -3378,7 +3378,7 @@ qemuBuildNVRAMDevStr(virDomainNVRAMDefPtr dev) } static char * -qemuBuildVirtioInputDevStr(virDomainDefPtr def, +qemuBuildVirtioInputDevStr(const virDomainDef *def, virDomainInputDefPtr dev, virQEMUCapsPtr qemuCaps) { @@ -3447,8 +3447,8 @@ qemuBuildVirtioInputDevStr(virDomainDefPtr def, return NULL; } -char * -qemuBuildUSBInputDevStr(virDomainDefPtr def, +static char * +qemuBuildUSBInputDevStr(const virDomainDef *def, virDomainInputDefPtr dev, virQEMUCapsPtr qemuCaps) { @@ -3482,6 +3482,52 @@ qemuBuildUSBInputDevStr(virDomainDefPtr def, } +static int +qemuBuildInputCommandLine(virCommandPtr cmd, + const virDomainDef *def, + virQEMUCapsPtr qemuCaps) +{ + size_t i; + + for (i = 0; i < def->ninputs; i++) { + virDomainInputDefPtr input = def->inputs[i]; + + if (input->bus == VIR_DOMAIN_INPUT_BUS_USB) { + if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) { + char *optstr; + virCommandAddArg(cmd, "-device"); + if (!(optstr = qemuBuildUSBInputDevStr(def, input, qemuCaps))) + return -1; + virCommandAddArg(cmd, optstr); + VIR_FREE(optstr); + } else { + switch (input->type) { + case VIR_DOMAIN_INPUT_TYPE_MOUSE: + virCommandAddArgList(cmd, "-usbdevice", "mouse", NULL); + break; + case VIR_DOMAIN_INPUT_TYPE_TABLET: + virCommandAddArgList(cmd, "-usbdevice", "tablet", NULL); + break; + case VIR_DOMAIN_INPUT_TYPE_KBD: + virCommandAddArgList(cmd, "-usbdevice", "keyboard", + NULL); + break; + } + } + } else if (input->bus == VIR_DOMAIN_INPUT_BUS_VIRTIO) { + char *optstr; + virCommandAddArg(cmd, "-device"); + if (!(optstr = qemuBuildVirtioInputDevStr(def, input, qemuCaps))) + return -1; + virCommandAddArg(cmd, optstr); + VIR_FREE(optstr); + } + } + + return 0; +} + + char * qemuBuildSoundDevStr(virDomainDefPtr def, virDomainSoundDefPtr sound, @@ -8387,39 +8433,8 @@ qemuBuildCommandLine(virConnectPtr conn, if (qemuBuildTPMCommandLine(cmd, def, qemuCaps) < 0) goto error; - for (i = 0; i < def->ninputs; i++) { - virDomainInputDefPtr input = def->inputs[i]; - - if (input->bus == VIR_DOMAIN_INPUT_BUS_USB) { - if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) { - char *optstr; - virCommandAddArg(cmd, "-device"); - if (!(optstr = qemuBuildUSBInputDevStr(def, input, qemuCaps))) - goto error; - virCommandAddArg(cmd, optstr); - VIR_FREE(optstr); - } else { - switch (input->type) { - case VIR_DOMAIN_INPUT_TYPE_MOUSE: - virCommandAddArgList(cmd, "-usbdevice", "mouse", NULL); - break; - case VIR_DOMAIN_INPUT_TYPE_TABLET: - virCommandAddArgList(cmd, "-usbdevice", "tablet", NULL); - break; - case VIR_DOMAIN_INPUT_TYPE_KBD: - virCommandAddArgList(cmd, "-usbdevice", "keyboard", NULL); - break; - } - } - } else if (input->bus == VIR_DOMAIN_INPUT_BUS_VIRTIO) { - char *optstr; - virCommandAddArg(cmd, "-device"); - if (!(optstr = qemuBuildVirtioInputDevStr(def, input, qemuCaps))) - goto error; - virCommandAddArg(cmd, optstr); - VIR_FREE(optstr); - } - } + if (qemuBuildInputCommandLine(cmd, def, qemuCaps) < 0) + goto error; for (i = 0; i < def->ngraphics; ++i) { if (qemuBuildGraphicsCommandLine(cfg, cmd, def, qemuCaps, diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h index be5a4c4..8a83335 100644 --- a/src/qemu/qemu_command.h +++ b/src/qemu/qemu_command.h @@ -133,10 +133,6 @@ char *qemuBuildMemballoonDevStr(virDomainDefPtr domainDef, virDomainMemballoonDefPtr dev, virQEMUCapsPtr qemuCaps); -char *qemuBuildUSBInputDevStr(virDomainDefPtr domainDef, - virDomainInputDefPtr dev, - virQEMUCapsPtr qemuCaps); - char *qemuBuildSoundDevStr(virDomainDefPtr domainDef, virDomainSoundDefPtr sound, virQEMUCapsPtr qemuCaps); -- 2.5.0

Add new function to manage adding the video device options to the command line removing that task from the mainline qemuBuildCommandLine. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_command.c | 316 +++++++++++++++++++++++++----------------------- 1 file changed, 166 insertions(+), 150 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index af09d8c..7ec1826 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -3635,7 +3635,7 @@ qemuBuildSoundCodecStr(virDomainSoundDefPtr sound, } static char * -qemuBuildDeviceVideoStr(virDomainDefPtr def, +qemuBuildDeviceVideoStr(const virDomainDef *def, virDomainVideoDefPtr video, virQEMUCapsPtr qemuCaps, bool primary) @@ -3739,6 +3739,169 @@ qemuBuildDeviceVideoStr(virDomainDefPtr def, } +static int +qemuBuildVideoCommandLine(virCommandPtr cmd, + const virDomainDef *def, + virQEMUCapsPtr qemuCaps) +{ + size_t i; + int primaryVideoType = def->videos[0]->type; + + if (!def->nvideos) { + /* If we have -device, then we set -nodefault already */ + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE) && + virQEMUCapsGet(qemuCaps, QEMU_CAPS_VGA_NONE)) + virCommandAddArgList(cmd, "-vga", "none", NULL); + return 0; + } + + if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VIDEO_PRIMARY) && + ((primaryVideoType == VIR_DOMAIN_VIDEO_TYPE_VGA && + virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VGA)) || + (primaryVideoType == VIR_DOMAIN_VIDEO_TYPE_CIRRUS && + virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_CIRRUS_VGA)) || + (primaryVideoType == VIR_DOMAIN_VIDEO_TYPE_VMVGA && + virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VMWARE_SVGA)) || + (primaryVideoType == VIR_DOMAIN_VIDEO_TYPE_QXL && + virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_QXL_VGA)) || + (primaryVideoType == VIR_DOMAIN_VIDEO_TYPE_VIRTIO && + virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VIRTIO_GPU))) + ) { + for (i = 0; i < def->nvideos; i++) { + char *str; + virCommandAddArg(cmd, "-device"); + if (!(str = qemuBuildDeviceVideoStr(def, def->videos[i], + qemuCaps, !i))) + return -1; + + virCommandAddArg(cmd, str); + VIR_FREE(str); + } + } else { + if (primaryVideoType == VIR_DOMAIN_VIDEO_TYPE_XEN) { + /* nothing - vga has no effect on Xen pvfb */ + } else { + if ((primaryVideoType == VIR_DOMAIN_VIDEO_TYPE_QXL) && + !virQEMUCapsGet(qemuCaps, QEMU_CAPS_VGA_QXL)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("This QEMU does not support QXL graphics adapters")); + return -1; + } + + const char *vgastr = qemuVideoTypeToString(primaryVideoType); + if (!vgastr || STREQ(vgastr, "")) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("video type %s is not supported with QEMU"), + virDomainVideoTypeToString(primaryVideoType)); + return -1; + } + + virCommandAddArgList(cmd, "-vga", vgastr, NULL); + + /* If we cannot use --device option to specify the video device + * in QEMU we will fallback to the old --vga option. To get the + * correct device name for the --vga option the 'qemuVideo' is + * used, but to set some device attributes we need to use the + * --global option and for that we need to specify the device + * name the same as for --device option and for that we need to + * use 'qemuDeviceVideo'. + * + * See 'Graphics Devices' section in docs/qdev-device-use.txt in + * QEMU repository. + */ + const char *dev = qemuDeviceVideoTypeToString(primaryVideoType); + + if (def->videos[0]->type == VIR_DOMAIN_VIDEO_TYPE_QXL && + (def->videos[0]->vram || def->videos[0]->ram) && + virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) { + unsigned int ram = def->videos[0]->ram; + unsigned int vram = def->videos[0]->vram; + unsigned int vgamem = def->videos[0]->vgamem; + + if (vram > (UINT_MAX / 1024)) { + virReportError(VIR_ERR_OVERFLOW, + _("value for 'vram' must be less than '%u'"), + UINT_MAX / 1024); + return -1; + } + if (ram > (UINT_MAX / 1024)) { + virReportError(VIR_ERR_OVERFLOW, + _("value for 'ram' must be less than '%u'"), + UINT_MAX / 1024); + return -1; + } + + if (ram) { + virCommandAddArg(cmd, "-global"); + virCommandAddArgFormat(cmd, "%s.ram_size=%u", + dev, ram * 1024); + } + if (vram) { + virCommandAddArg(cmd, "-global"); + virCommandAddArgFormat(cmd, "%s.vram_size=%u", + dev, vram * 1024); + } + if (vgamem && + virQEMUCapsGet(qemuCaps, QEMU_CAPS_QXL_VGA_VGAMEM)) { + virCommandAddArg(cmd, "-global"); + virCommandAddArgFormat(cmd, "%s.vgamem_mb=%u", + dev, vgamem / 1024); + } + } + + if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE) && + def->videos[0]->vram && + ((primaryVideoType == VIR_DOMAIN_VIDEO_TYPE_VGA && + virQEMUCapsGet(qemuCaps, QEMU_CAPS_VGA_VGAMEM)) || + (primaryVideoType == VIR_DOMAIN_VIDEO_TYPE_VMVGA && + virQEMUCapsGet(qemuCaps, QEMU_CAPS_VMWARE_SVGA_VGAMEM)))) { + unsigned int vram = def->videos[0]->vram; + + if (vram < 1024) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + "%s", _("value for 'vgamem' must be at " + "least 1 MiB (1024 KiB)")); + return -1; + } + + virCommandAddArg(cmd, "-global"); + virCommandAddArgFormat(cmd, "%s.vgamem_mb=%u", + dev, vram / 1024); + } + } + + if (def->nvideos > 1) { + if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) { + for (i = 1; i < def->nvideos; i++) { + char *str; + if (def->videos[i]->type != VIR_DOMAIN_VIDEO_TYPE_QXL) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("video type %s is only valid as primary video card"), + virDomainVideoTypeToString(def->videos[0]->type)); + return -1; + } + + virCommandAddArg(cmd, "-device"); + + if (!(str = qemuBuildDeviceVideoStr(def, def->videos[i], + qemuCaps, false))) + return -1; + + virCommandAddArg(cmd, str); + VIR_FREE(str); + } + } else { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("only one video card is currently supported")); + return -1; + } + } + } + + return 0; +} + + int qemuOpenPCIConfig(virDomainHostdevDefPtr dev) { @@ -8442,155 +8605,8 @@ qemuBuildCommandLine(virConnectPtr conn, goto error; } - if (def->nvideos > 0) { - int primaryVideoType = def->videos[0]->type; - if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VIDEO_PRIMARY) && - ((primaryVideoType == VIR_DOMAIN_VIDEO_TYPE_VGA && - virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VGA)) || - (primaryVideoType == VIR_DOMAIN_VIDEO_TYPE_CIRRUS && - virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_CIRRUS_VGA)) || - (primaryVideoType == VIR_DOMAIN_VIDEO_TYPE_VMVGA && - virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VMWARE_SVGA)) || - (primaryVideoType == VIR_DOMAIN_VIDEO_TYPE_QXL && - virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_QXL_VGA)) || - (primaryVideoType == VIR_DOMAIN_VIDEO_TYPE_VIRTIO && - virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VIRTIO_GPU))) - ) { - for (i = 0; i < def->nvideos; i++) { - char *str; - virCommandAddArg(cmd, "-device"); - if (!(str = qemuBuildDeviceVideoStr(def, def->videos[i], qemuCaps, !i))) - goto error; - - virCommandAddArg(cmd, str); - VIR_FREE(str); - } - } else { - if (primaryVideoType == VIR_DOMAIN_VIDEO_TYPE_XEN) { - /* nothing - vga has no effect on Xen pvfb */ - } else { - if ((primaryVideoType == VIR_DOMAIN_VIDEO_TYPE_QXL) && - !virQEMUCapsGet(qemuCaps, QEMU_CAPS_VGA_QXL)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("This QEMU does not support QXL graphics adapters")); - goto error; - } - - const char *vgastr = qemuVideoTypeToString(primaryVideoType); - if (!vgastr || STREQ(vgastr, "")) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("video type %s is not supported with QEMU"), - virDomainVideoTypeToString(primaryVideoType)); - goto error; - } - - virCommandAddArgList(cmd, "-vga", vgastr, NULL); - - /* If we cannot use --device option to specify the video device - * in QEMU we will fallback to the old --vga option. To get the - * correct device name for the --vga option the 'qemuVideo' is - * used, but to set some device attributes we need to use the - * --global option and for that we need to specify the device - * name the same as for --device option and for that we need to - * use 'qemuDeviceVideo'. - * - * See 'Graphics Devices' section in docs/qdev-device-use.txt in - * QEMU repository. - */ - const char *dev = qemuDeviceVideoTypeToString(primaryVideoType); - - if (def->videos[0]->type == VIR_DOMAIN_VIDEO_TYPE_QXL && - (def->videos[0]->vram || def->videos[0]->ram) && - virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) { - unsigned int ram = def->videos[0]->ram; - unsigned int vram = def->videos[0]->vram; - unsigned int vgamem = def->videos[0]->vgamem; - - if (vram > (UINT_MAX / 1024)) { - virReportError(VIR_ERR_OVERFLOW, - _("value for 'vram' must be less than '%u'"), - UINT_MAX / 1024); - goto error; - } - if (ram > (UINT_MAX / 1024)) { - virReportError(VIR_ERR_OVERFLOW, - _("value for 'ram' must be less than '%u'"), - UINT_MAX / 1024); - goto error; - } - - if (ram) { - virCommandAddArg(cmd, "-global"); - virCommandAddArgFormat(cmd, "%s.ram_size=%u", - dev, ram * 1024); - } - if (vram) { - virCommandAddArg(cmd, "-global"); - virCommandAddArgFormat(cmd, "%s.vram_size=%u", - dev, vram * 1024); - } - if (vgamem && - virQEMUCapsGet(qemuCaps, QEMU_CAPS_QXL_VGA_VGAMEM)) { - virCommandAddArg(cmd, "-global"); - virCommandAddArgFormat(cmd, "%s.vgamem_mb=%u", - dev, vgamem / 1024); - } - } - - if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE) && - def->videos[0]->vram && - ((primaryVideoType == VIR_DOMAIN_VIDEO_TYPE_VGA && - virQEMUCapsGet(qemuCaps, QEMU_CAPS_VGA_VGAMEM)) || - (primaryVideoType == VIR_DOMAIN_VIDEO_TYPE_VMVGA && - virQEMUCapsGet(qemuCaps, QEMU_CAPS_VMWARE_SVGA_VGAMEM)))) { - unsigned int vram = def->videos[0]->vram; - - if (vram < 1024) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - "%s", _("value for 'vgamem' must be at " - "least 1 MiB (1024 KiB)")); - goto error; - } - - virCommandAddArg(cmd, "-global"); - virCommandAddArgFormat(cmd, "%s.vgamem_mb=%u", - dev, vram / 1024); - } - } - - if (def->nvideos > 1) { - if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) { - for (i = 1; i < def->nvideos; i++) { - char *str; - if (def->videos[i]->type != VIR_DOMAIN_VIDEO_TYPE_QXL) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("video type %s is only valid as primary video card"), - virDomainVideoTypeToString(def->videos[0]->type)); - goto error; - } - - virCommandAddArg(cmd, "-device"); - - if (!(str = qemuBuildDeviceVideoStr(def, def->videos[i], qemuCaps, false))) - goto error; - - virCommandAddArg(cmd, str); - VIR_FREE(str); - } - } else { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - "%s", _("only one video card is currently supported")); - goto error; - } - } - } - - } else { - /* If we have -device, then we set -nodefault already */ - if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE) && - virQEMUCapsGet(qemuCaps, QEMU_CAPS_VGA_NONE)) - virCommandAddArgList(cmd, "-vga", "none", NULL); - } + if (qemuBuildVideoCommandLine(cmd, def, qemuCaps) < 0) + goto error; /* Add sound hardware */ if (def->nsounds) { -- 2.5.0

Add new function to manage adding the sound device options to the command line removing that task from the mainline qemuBuildCommandLine. Also since qemuBuildSoundDevStr was only local here, make it static as well as modifying the const virDomainDef. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_command.c | 188 ++++++++++++++++++++++++++---------------------- src/qemu/qemu_command.h | 4 -- 2 files changed, 104 insertions(+), 88 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 7ec1826..a89b62d 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -3528,8 +3528,8 @@ qemuBuildInputCommandLine(virCommandPtr cmd, } -char * -qemuBuildSoundDevStr(virDomainDefPtr def, +static char * +qemuBuildSoundDevStr(const virDomainDef *def, virDomainSoundDefPtr sound, virQEMUCapsPtr qemuCaps) { @@ -3634,6 +3634,105 @@ qemuBuildSoundCodecStr(virDomainSoundDefPtr sound, return NULL; } + +static int +qemuBuildSoundCommandLine(virCommandPtr cmd, + const virDomainDef *def, + virQEMUCapsPtr qemuCaps) +{ + size_t i, j; + + if (!def->nsounds) + return 0; + + if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) { + for (i = 0; i < def->nsounds; i++) { + virDomainSoundDefPtr sound = def->sounds[i]; + char *str = NULL; + + /* Sadly pcspk device doesn't use -device syntax. Fortunately + * we don't need to set any PCI address on it, so we don't + * mind too much */ + if (sound->model == VIR_DOMAIN_SOUND_MODEL_PCSPK) { + virCommandAddArgList(cmd, "-soundhw", "pcspk", NULL); + } else { + virCommandAddArg(cmd, "-device"); + if (!(str = qemuBuildSoundDevStr(def, sound, qemuCaps))) + return -1; + + virCommandAddArg(cmd, str); + VIR_FREE(str); + if (sound->model == VIR_DOMAIN_SOUND_MODEL_ICH6 || + sound->model == VIR_DOMAIN_SOUND_MODEL_ICH9) { + char *codecstr = NULL; + + for (j = 0; j < sound->ncodecs; j++) { + virCommandAddArg(cmd, "-device"); + if (!(codecstr = + qemuBuildSoundCodecStr(sound, sound->codecs[j], + qemuCaps))) { + return -1; + + } + virCommandAddArg(cmd, codecstr); + VIR_FREE(codecstr); + } + if (j == 0) { + virDomainSoundCodecDef codec = { + VIR_DOMAIN_SOUND_CODEC_TYPE_DUPLEX, + 0 + }; + virCommandAddArg(cmd, "-device"); + if (!(codecstr = + qemuBuildSoundCodecStr(sound, &codec, + qemuCaps))) { + return -1; + + } + virCommandAddArg(cmd, codecstr); + VIR_FREE(codecstr); + } + } + } + } + } else { + int size = 100; + char *modstr; + if (VIR_ALLOC_N(modstr, size+1) < 0) + return -1; + + for (i = 0; i < def->nsounds && size > 0; i++) { + virDomainSoundDefPtr sound = def->sounds[i]; + const char *model = virDomainSoundModelTypeToString(sound->model); + if (!model) { + VIR_FREE(modstr); + virReportError(VIR_ERR_INTERNAL_ERROR, + "%s", _("invalid sound model")); + return -1; + } + + if (sound->model == VIR_DOMAIN_SOUND_MODEL_ICH6 || + sound->model == VIR_DOMAIN_SOUND_MODEL_ICH9) { + VIR_FREE(modstr); + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("this QEMU binary lacks hda support")); + return -1; + } + + strncat(modstr, model, size); + size -= strlen(model); + if (i < (def->nsounds - 1)) + strncat(modstr, ",", size--); + } + virCommandAddArgList(cmd, "-soundhw", modstr, NULL); + VIR_FREE(modstr); + } + + return 0; +} + + + static char * qemuBuildDeviceVideoStr(const virDomainDef *def, virDomainVideoDefPtr video, @@ -8443,7 +8542,7 @@ qemuBuildCommandLine(virConnectPtr conn, int **nicindexes) { virErrorPtr originalError = NULL; - size_t i, j; + size_t i; char uuid[VIR_UUID_STRING_BUFLEN]; virCommandPtr cmd = NULL; bool emitBootindex = false; @@ -8608,87 +8707,8 @@ qemuBuildCommandLine(virConnectPtr conn, if (qemuBuildVideoCommandLine(cmd, def, qemuCaps) < 0) goto error; - /* Add sound hardware */ - if (def->nsounds) { - if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) { - for (i = 0; i < def->nsounds; i++) { - virDomainSoundDefPtr sound = def->sounds[i]; - char *str = NULL; - - /* Sadly pcspk device doesn't use -device syntax. Fortunately - * we don't need to set any PCI address on it, so we don't - * mind too much */ - if (sound->model == VIR_DOMAIN_SOUND_MODEL_PCSPK) { - virCommandAddArgList(cmd, "-soundhw", "pcspk", NULL); - } else { - virCommandAddArg(cmd, "-device"); - if (!(str = qemuBuildSoundDevStr(def, sound, qemuCaps))) - goto error; - - virCommandAddArg(cmd, str); - VIR_FREE(str); - if (sound->model == VIR_DOMAIN_SOUND_MODEL_ICH6 || - sound->model == VIR_DOMAIN_SOUND_MODEL_ICH9) { - char *codecstr = NULL; - - for (j = 0; j < sound->ncodecs; j++) { - virCommandAddArg(cmd, "-device"); - if (!(codecstr = qemuBuildSoundCodecStr(sound, sound->codecs[j], qemuCaps))) { - goto error; - - } - virCommandAddArg(cmd, codecstr); - VIR_FREE(codecstr); - } - if (j == 0) { - virDomainSoundCodecDef codec = { - VIR_DOMAIN_SOUND_CODEC_TYPE_DUPLEX, - 0 - }; - virCommandAddArg(cmd, "-device"); - if (!(codecstr = qemuBuildSoundCodecStr(sound, &codec, qemuCaps))) { - goto error; - - } - virCommandAddArg(cmd, codecstr); - VIR_FREE(codecstr); - } - } - } - } - } else { - int size = 100; - char *modstr; - if (VIR_ALLOC_N(modstr, size+1) < 0) - goto error; - - for (i = 0; i < def->nsounds && size > 0; i++) { - virDomainSoundDefPtr sound = def->sounds[i]; - const char *model = virDomainSoundModelTypeToString(sound->model); - if (!model) { - VIR_FREE(modstr); - virReportError(VIR_ERR_INTERNAL_ERROR, - "%s", _("invalid sound model")); - goto error; - } - - if (sound->model == VIR_DOMAIN_SOUND_MODEL_ICH6 || - sound->model == VIR_DOMAIN_SOUND_MODEL_ICH9) { - VIR_FREE(modstr); - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("this QEMU binary lacks hda support")); - goto error; - } - - strncat(modstr, model, size); - size -= strlen(model); - if (i < (def->nsounds - 1)) - strncat(modstr, ",", size--); - } - virCommandAddArgList(cmd, "-soundhw", modstr, NULL); - VIR_FREE(modstr); - } - } + if (qemuBuildSoundCommandLine(cmd, def, qemuCaps) < 0) + goto error; /* Add watchdog hardware */ if (def->watchdog) { diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h index 8a83335..a26861b 100644 --- a/src/qemu/qemu_command.h +++ b/src/qemu/qemu_command.h @@ -133,10 +133,6 @@ char *qemuBuildMemballoonDevStr(virDomainDefPtr domainDef, virDomainMemballoonDefPtr dev, virQEMUCapsPtr qemuCaps); -char *qemuBuildSoundDevStr(virDomainDefPtr domainDef, - virDomainSoundDefPtr sound, - virQEMUCapsPtr qemuCaps); - int qemuBuildMemoryBackendStr(unsigned long long size, unsigned long long pagesize, int guestNode, -- 2.5.0

Add new function to manage adding the watchdog device options to the command line removing that task from the mainline qemuBuildCommandLine. Also since qemuBuildWatchdogDevStr was only local here, make it static as well as modifying the const virDomainDef. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_command.c | 93 ++++++++++++++++++++++++++++--------------------- src/qemu/qemu_command.h | 4 --- 2 files changed, 53 insertions(+), 44 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index a89b62d..af09e1e 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -3274,8 +3274,8 @@ qemuBuildHostNetStr(virDomainNetDefPtr net, } -char * -qemuBuildWatchdogDevStr(virDomainDefPtr def, +static char * +qemuBuildWatchdogDevStr(const virDomainDef *def, virDomainWatchdogDefPtr dev, virQEMUCapsPtr qemuCaps) { @@ -3303,6 +3303,55 @@ qemuBuildWatchdogDevStr(virDomainDefPtr def, } +static int +qemuBuildWatchdogCommandLine(virCommandPtr cmd, + const virDomainDef *def, + virQEMUCapsPtr qemuCaps) +{ + virDomainWatchdogDefPtr watchdog = def->watchdog; + char *optstr; + const char *action; + + if (!def->watchdog) + return 0; + + if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) { + virCommandAddArg(cmd, "-device"); + + optstr = qemuBuildWatchdogDevStr(def, watchdog, qemuCaps); + if (!optstr) + return -1; + } else { + virCommandAddArg(cmd, "-watchdog"); + + const char *model = virDomainWatchdogModelTypeToString(watchdog->model); + if (!model) { + virReportError(VIR_ERR_INTERNAL_ERROR, + "%s", _("missing watchdog model")); + return -1; + } + + if (VIR_STRDUP(optstr, model) < 0) + return -1; + } + virCommandAddArg(cmd, optstr); + VIR_FREE(optstr); + + if (watchdog->action == VIR_DOMAIN_WATCHDOG_ACTION_DUMP) + watchdog->action = VIR_DOMAIN_WATCHDOG_ACTION_PAUSE; + + action = virDomainWatchdogActionTypeToString(watchdog->action); + if (!action) { + virReportError(VIR_ERR_INTERNAL_ERROR, + "%s", _("invalid watchdog action")); + return -1; + } + virCommandAddArgList(cmd, "-watchdog-action", action, NULL); + + return 0; +} + + char * qemuBuildMemballoonDevStr(virDomainDefPtr def, virDomainMemballoonDefPtr dev, @@ -8710,44 +8759,8 @@ qemuBuildCommandLine(virConnectPtr conn, if (qemuBuildSoundCommandLine(cmd, def, qemuCaps) < 0) goto error; - /* Add watchdog hardware */ - if (def->watchdog) { - virDomainWatchdogDefPtr watchdog = def->watchdog; - char *optstr; - - if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) { - virCommandAddArg(cmd, "-device"); - - optstr = qemuBuildWatchdogDevStr(def, watchdog, qemuCaps); - if (!optstr) - goto error; - } else { - virCommandAddArg(cmd, "-watchdog"); - - const char *model = virDomainWatchdogModelTypeToString(watchdog->model); - if (!model) { - virReportError(VIR_ERR_INTERNAL_ERROR, - "%s", _("missing watchdog model")); - goto error; - } - - if (VIR_STRDUP(optstr, model) < 0) - goto error; - } - virCommandAddArg(cmd, optstr); - VIR_FREE(optstr); - - int act = watchdog->action; - if (act == VIR_DOMAIN_WATCHDOG_ACTION_DUMP) - act = VIR_DOMAIN_WATCHDOG_ACTION_PAUSE; - const char *action = virDomainWatchdogActionTypeToString(act); - if (!action) { - virReportError(VIR_ERR_INTERNAL_ERROR, - "%s", _("invalid watchdog action")); - goto error; - } - virCommandAddArgList(cmd, "-watchdog-action", action, NULL); - } + if (qemuBuildWatchdogCommandLine(cmd, def, qemuCaps) < 0) + goto error; /* Add redirected devices */ for (i = 0; i < def->nredirdevs; i++) { diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h index a26861b..6793777 100644 --- a/src/qemu/qemu_command.h +++ b/src/qemu/qemu_command.h @@ -125,10 +125,6 @@ char *qemuBuildControllerDevStr(const virDomainDef *domainDef, virQEMUCapsPtr qemuCaps, int *nusbcontroller); -char *qemuBuildWatchdogDevStr(virDomainDefPtr domainDef, - virDomainWatchdogDefPtr dev, - virQEMUCapsPtr qemuCaps); - char *qemuBuildMemballoonDevStr(virDomainDefPtr domainDef, virDomainMemballoonDefPtr dev, virQEMUCapsPtr qemuCaps); -- 2.5.0

Add new function to manage adding the redirdev device options to the command line removing that task from the mainline qemuBuildCommandLine. Also move the qemuBuildRedirdevDevStr closer to the new function and modify to use the const virDomainDef instead of virDomainDefPtr Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_command.c | 243 +++++++++++++++++++++++++----------------------- src/qemu/qemu_command.h | 2 +- 2 files changed, 129 insertions(+), 116 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index af09e1e..0649d72 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -4166,93 +4166,6 @@ qemuBuildPCIHostdevPCIDevStr(virDomainHostdevDefPtr dev, char * -qemuBuildRedirdevDevStr(virDomainDefPtr def, - virDomainRedirdevDefPtr dev, - virQEMUCapsPtr qemuCaps) -{ - size_t i; - virBuffer buf = VIR_BUFFER_INITIALIZER; - virDomainRedirFilterDefPtr redirfilter = def->redirfilter; - - if (dev->bus != VIR_DOMAIN_REDIRDEV_BUS_USB) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("Redirection bus %s is not supported by QEMU"), - virDomainRedirdevBusTypeToString(dev->bus)); - goto error; - } - - if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_USB_REDIR)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("USB redirection is not supported " - "by this version of QEMU")); - goto error; - } - - virBufferAsprintf(&buf, "usb-redir,chardev=char%s,id=%s", - dev->info.alias, dev->info.alias); - - if (redirfilter && redirfilter->nusbdevs) { - if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_USB_REDIR_FILTER)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("USB redirection filter is not " - "supported by this version of QEMU")); - goto error; - } - - virBufferAddLit(&buf, ",filter="); - - for (i = 0; i < redirfilter->nusbdevs; i++) { - virDomainRedirFilterUSBDevDefPtr usbdev = redirfilter->usbdevs[i]; - if (usbdev->usbClass >= 0) - virBufferAsprintf(&buf, "0x%02X:", usbdev->usbClass); - else - virBufferAddLit(&buf, "-1:"); - - if (usbdev->vendor >= 0) - virBufferAsprintf(&buf, "0x%04X:", usbdev->vendor); - else - virBufferAddLit(&buf, "-1:"); - - if (usbdev->product >= 0) - virBufferAsprintf(&buf, "0x%04X:", usbdev->product); - else - virBufferAddLit(&buf, "-1:"); - - if (usbdev->version >= 0) - virBufferAsprintf(&buf, "0x%04X:", usbdev->version); - else - virBufferAddLit(&buf, "-1:"); - - virBufferAsprintf(&buf, "%u", usbdev->allow); - if (i < redirfilter->nusbdevs -1) - virBufferAddLit(&buf, "|"); - } - } - - if (dev->info.bootIndex) { - if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_USB_REDIR_BOOTINDEX)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("USB redirection booting is not " - "supported by this version of QEMU")); - goto error; - } - virBufferAsprintf(&buf, ",bootindex=%d", dev->info.bootIndex); - } - - if (qemuBuildDeviceAddressStr(&buf, def, &dev->info, qemuCaps) < 0) - goto error; - - if (virBufferCheckError(&buf) < 0) - goto error; - - return virBufferContentAndReset(&buf); - - error: - virBufferFreeAndReset(&buf); - return NULL; -} - -char * qemuBuildUSBHostdevDevStr(virDomainDefPtr def, virDomainHostdevDefPtr dev, virQEMUCapsPtr qemuCaps) @@ -8179,6 +8092,132 @@ qemuBuildConsoleCommandLine(virCommandPtr cmd, } +char * +qemuBuildRedirdevDevStr(const virDomainDef *def, + virDomainRedirdevDefPtr dev, + virQEMUCapsPtr qemuCaps) +{ + size_t i; + virBuffer buf = VIR_BUFFER_INITIALIZER; + virDomainRedirFilterDefPtr redirfilter = def->redirfilter; + + if (dev->bus != VIR_DOMAIN_REDIRDEV_BUS_USB) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("Redirection bus %s is not supported by QEMU"), + virDomainRedirdevBusTypeToString(dev->bus)); + goto error; + } + + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_USB_REDIR)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("USB redirection is not supported " + "by this version of QEMU")); + goto error; + } + + virBufferAsprintf(&buf, "usb-redir,chardev=char%s,id=%s", + dev->info.alias, dev->info.alias); + + if (redirfilter && redirfilter->nusbdevs) { + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_USB_REDIR_FILTER)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("USB redirection filter is not " + "supported by this version of QEMU")); + goto error; + } + + virBufferAddLit(&buf, ",filter="); + + for (i = 0; i < redirfilter->nusbdevs; i++) { + virDomainRedirFilterUSBDevDefPtr usbdev = redirfilter->usbdevs[i]; + if (usbdev->usbClass >= 0) + virBufferAsprintf(&buf, "0x%02X:", usbdev->usbClass); + else + virBufferAddLit(&buf, "-1:"); + + if (usbdev->vendor >= 0) + virBufferAsprintf(&buf, "0x%04X:", usbdev->vendor); + else + virBufferAddLit(&buf, "-1:"); + + if (usbdev->product >= 0) + virBufferAsprintf(&buf, "0x%04X:", usbdev->product); + else + virBufferAddLit(&buf, "-1:"); + + if (usbdev->version >= 0) + virBufferAsprintf(&buf, "0x%04X:", usbdev->version); + else + virBufferAddLit(&buf, "-1:"); + + virBufferAsprintf(&buf, "%u", usbdev->allow); + if (i < redirfilter->nusbdevs -1) + virBufferAddLit(&buf, "|"); + } + } + + if (dev->info.bootIndex) { + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_USB_REDIR_BOOTINDEX)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("USB redirection booting is not " + "supported by this version of QEMU")); + goto error; + } + virBufferAsprintf(&buf, ",bootindex=%d", dev->info.bootIndex); + } + + if (qemuBuildDeviceAddressStr(&buf, def, &dev->info, qemuCaps) < 0) + goto error; + + if (virBufferCheckError(&buf) < 0) + goto error; + + return virBufferContentAndReset(&buf); + + error: + virBufferFreeAndReset(&buf); + return NULL; +} + + +static int +qemuBuildRedirdevCommandLine(virCommandPtr cmd, + const virDomainDef *def, + virQEMUCapsPtr qemuCaps) +{ + size_t i; + + for (i = 0; i < def->nredirdevs; i++) { + virDomainRedirdevDefPtr redirdev = def->redirdevs[i]; + char *devstr; + + virCommandAddArg(cmd, "-chardev"); + if (!(devstr = qemuBuildChrChardevStr(&redirdev->source.chr, + redirdev->info.alias, + qemuCaps))) { + return -1; + } + + virCommandAddArg(cmd, devstr); + VIR_FREE(devstr); + + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("redirected devices are not supported by this QEMU")); + return -1; + } + + virCommandAddArg(cmd, "-device"); + if (!(devstr = qemuBuildRedirdevDevStr(def, redirdev, qemuCaps))) + return -1; + virCommandAddArg(cmd, devstr); + VIR_FREE(devstr); + } + + return 0; +} + + static int qemuBuildDomainLoaderCommandLine(virCommandPtr cmd, virDomainDefPtr def, @@ -8762,34 +8801,8 @@ qemuBuildCommandLine(virConnectPtr conn, if (qemuBuildWatchdogCommandLine(cmd, def, qemuCaps) < 0) goto error; - /* Add redirected devices */ - for (i = 0; i < def->nredirdevs; i++) { - virDomainRedirdevDefPtr redirdev = def->redirdevs[i]; - char *devstr; - - virCommandAddArg(cmd, "-chardev"); - if (!(devstr = qemuBuildChrChardevStr(&redirdev->source.chr, - redirdev->info.alias, - qemuCaps))) { - goto error; - } - - virCommandAddArg(cmd, devstr); - VIR_FREE(devstr); - - if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("redirected devices are not supported by this QEMU")); - goto error; - } - - - virCommandAddArg(cmd, "-device"); - if (!(devstr = qemuBuildRedirdevDevStr(def, redirdev, qemuCaps))) - goto error; - virCommandAddArg(cmd, devstr); - VIR_FREE(devstr); - } + if (qemuBuildRedirdevCommandLine(cmd, def, qemuCaps) < 0) + goto error; /* Add host passthrough hardware */ for (i = 0; i < def->nhostdevs; i++) { diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h index 6793777..4959efb 100644 --- a/src/qemu/qemu_command.h +++ b/src/qemu/qemu_command.h @@ -186,7 +186,7 @@ char *qemuBuildSCSIHostdevDevStr(virDomainDefPtr def, virDomainHostdevDefPtr dev, virQEMUCapsPtr qemuCaps); -char *qemuBuildRedirdevDevStr(virDomainDefPtr def, +char *qemuBuildRedirdevDevStr(const virDomainDef *def, virDomainRedirdevDefPtr dev, virQEMUCapsPtr qemuCaps); -- 2.5.0

Add new function to manage adding the host device options to the command line removing that task from the mainline qemuBuildCommandLine. Also modify qemuBuildPCIHostdevDevStr, qemuBuildUSBHostdevDevStr, and qemuBuildSCSIHostdevDevStr to use const virDomainDef instead of virDomainDefPtr. Make qemuBuildPCIHostdevPCIDevStr and qemuBuildUSBHostdevUSBDevStr static to the qemu_command.c. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_command.c | 343 ++++++++++++++++++++++++++---------------------- src/qemu/qemu_command.h | 11 +- 2 files changed, 186 insertions(+), 168 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 0649d72..186281f 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -4073,7 +4073,7 @@ qemuOpenPCIConfig(virDomainHostdevDefPtr dev) } char * -qemuBuildPCIHostdevDevStr(virDomainDefPtr def, +qemuBuildPCIHostdevDevStr(const virDomainDef *def, virDomainHostdevDefPtr dev, int bootIndex, /* used iff dev->info->bootIndex == 0 */ const char *configfd, @@ -4137,7 +4137,7 @@ qemuBuildPCIHostdevDevStr(virDomainDefPtr def, } -char * +static char * qemuBuildPCIHostdevPCIDevStr(virDomainHostdevDefPtr dev, virQEMUCapsPtr qemuCaps) { @@ -4166,7 +4166,7 @@ qemuBuildPCIHostdevPCIDevStr(virDomainHostdevDefPtr dev, char * -qemuBuildUSBHostdevDevStr(virDomainDefPtr def, +qemuBuildUSBHostdevDevStr(const virDomainDef *def, virDomainHostdevDefPtr dev, virQEMUCapsPtr qemuCaps) { @@ -4260,7 +4260,7 @@ qemuBuildHubCommandLine(virCommandPtr cmd, } -char * +static char * qemuBuildUSBHostdevUSBDevStr(virDomainHostdevDefPtr dev) { char *ret = NULL; @@ -4385,7 +4385,7 @@ qemuBuildSCSIHostdevDrvStr(virConnectPtr conn, } char * -qemuBuildSCSIHostdevDevStr(virDomainDefPtr def, +qemuBuildSCSIHostdevDevStr(const virDomainDef *def, virDomainHostdevDefPtr dev, virQEMUCapsPtr qemuCaps) { @@ -4581,6 +4581,181 @@ qemuBuildChrChardevStr(const virDomainChrSourceDef *dev, } +static int +qemuBuildHostdevCommandLine(virCommandPtr cmd, + virConnectPtr conn, + const virDomainDef *def, + virQEMUCapsPtr qemuCaps, + qemuBuildCommandLineCallbacksPtr callbacks, + int *bootHostdevNet) +{ + size_t i; + + for (i = 0; i < def->nhostdevs; i++) { + virDomainHostdevDefPtr hostdev = def->hostdevs[i]; + virDomainHostdevSubsysPtr subsys = &hostdev->source.subsys; + char *devstr; + + if (hostdev->info->bootIndex) { + if (hostdev->mode != VIR_DOMAIN_HOSTDEV_MODE_SUBSYS || + (subsys->type != VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI && + subsys->type != VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB && + subsys->type != VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("booting from assigned devices is only " + "supported for PCI, USB and SCSI devices")); + return -1; + } else { + if (subsys->type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI) { + if (subsys->u.pci.backend == + VIR_DOMAIN_HOSTDEV_PCI_BACKEND_VFIO) { + if (!virQEMUCapsGet(qemuCaps, + QEMU_CAPS_VFIO_PCI_BOOTINDEX)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("booting from PCI devices assigned with VFIO " + "is not supported with this version of qemu")); + return -1; + } + } else { + if (!virQEMUCapsGet(qemuCaps, + QEMU_CAPS_PCI_BOOTINDEX)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("booting from assigned PCI devices is not " + "supported with this version of qemu")); + return -1; + } + } + } + if (subsys->type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB && + !virQEMUCapsGet(qemuCaps, QEMU_CAPS_USB_HOST_BOOTINDEX)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("booting from assigned USB devices is not " + "supported with this version of qemu")); + return -1; + } + if (subsys->type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI && + !virQEMUCapsGet(qemuCaps, + QEMU_CAPS_DEVICE_SCSI_GENERIC_BOOTINDEX)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("booting from assigned SCSI devices is not" + " supported with this version of qemu")); + return -1; + } + } + } + + /* USB */ + if (hostdev->mode == VIR_DOMAIN_HOSTDEV_MODE_SUBSYS && + subsys->type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB) { + + if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) { + virCommandAddArg(cmd, "-device"); + if (!(devstr = + qemuBuildUSBHostdevDevStr(def, hostdev, qemuCaps))) + return -1; + virCommandAddArg(cmd, devstr); + VIR_FREE(devstr); + } else { + virCommandAddArg(cmd, "-usbdevice"); + if (!(devstr = qemuBuildUSBHostdevUSBDevStr(hostdev))) + return -1; + virCommandAddArg(cmd, devstr); + VIR_FREE(devstr); + } + } + + /* PCI */ + if (hostdev->mode == VIR_DOMAIN_HOSTDEV_MODE_SUBSYS && + subsys->type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI) { + int backend = subsys->u.pci.backend; + + if (backend == VIR_DOMAIN_HOSTDEV_PCI_BACKEND_VFIO) { + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VFIO_PCI)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("VFIO PCI device assignment is not " + "supported by this version of qemu")); + return -1; + } + } + + if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) { + char *configfd_name = NULL; + int bootIndex = hostdev->info->bootIndex; + + /* bootNet will be non-0 if boot order was set and no other + * net devices were encountered + */ + if (hostdev->parent.type == VIR_DOMAIN_DEVICE_NET && + bootIndex == 0) { + bootIndex = *bootHostdevNet; + *bootHostdevNet = 0; + } + if ((backend != VIR_DOMAIN_HOSTDEV_PCI_BACKEND_VFIO) && + virQEMUCapsGet(qemuCaps, QEMU_CAPS_PCI_CONFIGFD)) { + int configfd = qemuOpenPCIConfig(hostdev); + + if (configfd >= 0) { + if (virAsprintf(&configfd_name, "%d", configfd) < 0) { + VIR_FORCE_CLOSE(configfd); + return -1; + } + + virCommandPassFD(cmd, configfd, + VIR_COMMAND_PASS_FD_CLOSE_PARENT); + } + } + virCommandAddArg(cmd, "-device"); + devstr = qemuBuildPCIHostdevDevStr(def, hostdev, bootIndex, + configfd_name, qemuCaps); + VIR_FREE(configfd_name); + if (!devstr) + return -1; + virCommandAddArg(cmd, devstr); + VIR_FREE(devstr); + } else if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_PCIDEVICE)) { + virCommandAddArg(cmd, "-pcidevice"); + if (!(devstr = qemuBuildPCIHostdevPCIDevStr(hostdev, qemuCaps))) + return -1; + virCommandAddArg(cmd, devstr); + VIR_FREE(devstr); + } else { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("PCI device assignment is not supported by this version of qemu")); + return -1; + } + } + + /* SCSI */ + if (hostdev->mode == VIR_DOMAIN_HOSTDEV_MODE_SUBSYS && + subsys->type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI) { + if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE) && + virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_SCSI_GENERIC)) { + char *drvstr; + + virCommandAddArg(cmd, "-drive"); + if (!(drvstr = qemuBuildSCSIHostdevDrvStr(conn, hostdev, + qemuCaps, callbacks))) + return -1; + virCommandAddArg(cmd, drvstr); + VIR_FREE(drvstr); + + virCommandAddArg(cmd, "-device"); + if (!(devstr = qemuBuildSCSIHostdevDevStr(def, hostdev, + qemuCaps))) + return -1; + virCommandAddArg(cmd, devstr); + VIR_FREE(devstr); + } else { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("SCSI passthrough is not supported by this version of qemu")); + return -1; + } + } + } + + return 0; +} + static char * qemuBuildChrArgStr(const virDomainChrSourceDef *dev, const char *prefix) @@ -8804,161 +8979,9 @@ qemuBuildCommandLine(virConnectPtr conn, if (qemuBuildRedirdevCommandLine(cmd, def, qemuCaps) < 0) goto error; - /* Add host passthrough hardware */ - for (i = 0; i < def->nhostdevs; i++) { - virDomainHostdevDefPtr hostdev = def->hostdevs[i]; - char *devstr; - - if (hostdev->info->bootIndex) { - if (hostdev->mode != VIR_DOMAIN_HOSTDEV_MODE_SUBSYS || - (hostdev->source.subsys.type != VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI && - hostdev->source.subsys.type != VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB && - hostdev->source.subsys.type != VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("booting from assigned devices is only " - "supported for PCI, USB and SCSI devices")); - goto error; - } else { - if (hostdev->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI) { - if (hostdev->source.subsys.u.pci.backend - == VIR_DOMAIN_HOSTDEV_PCI_BACKEND_VFIO) { - if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_VFIO_PCI_BOOTINDEX)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("booting from PCI devices assigned with VFIO " - "is not supported with this version of qemu")); - goto error; - } - } else { - if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_PCI_BOOTINDEX)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("booting from assigned PCI devices is not " - "supported with this version of qemu")); - goto error; - } - } - } - if (hostdev->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB && - !virQEMUCapsGet(qemuCaps, QEMU_CAPS_USB_HOST_BOOTINDEX)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("booting from assigned USB devices is not " - "supported with this version of qemu")); - goto error; - } - if (hostdev->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI && - !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_SCSI_GENERIC_BOOTINDEX)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("booting from assigned SCSI devices is not" - " supported with this version of qemu")); - goto error; - } - } - } - - /* USB */ - if (hostdev->mode == VIR_DOMAIN_HOSTDEV_MODE_SUBSYS && - hostdev->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB) { - - if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) { - virCommandAddArg(cmd, "-device"); - if (!(devstr = qemuBuildUSBHostdevDevStr(def, hostdev, qemuCaps))) - goto error; - virCommandAddArg(cmd, devstr); - VIR_FREE(devstr); - } else { - virCommandAddArg(cmd, "-usbdevice"); - if (!(devstr = qemuBuildUSBHostdevUSBDevStr(hostdev))) - goto error; - virCommandAddArg(cmd, devstr); - VIR_FREE(devstr); - } - } - - /* PCI */ - if (hostdev->mode == VIR_DOMAIN_HOSTDEV_MODE_SUBSYS && - hostdev->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI) { - int backend = hostdev->source.subsys.u.pci.backend; - - if (backend == VIR_DOMAIN_HOSTDEV_PCI_BACKEND_VFIO) { - if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VFIO_PCI)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("VFIO PCI device assignment is not " - "supported by this version of qemu")); - goto error; - } - } - - if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) { - char *configfd_name = NULL; - int bootIndex = hostdev->info->bootIndex; - - /* bootNet will be non-0 if boot order was set and no other - * net devices were encountered - */ - if (hostdev->parent.type == VIR_DOMAIN_DEVICE_NET && - bootIndex == 0) { - bootIndex = bootHostdevNet; - bootHostdevNet = 0; - } - if ((backend != VIR_DOMAIN_HOSTDEV_PCI_BACKEND_VFIO) && - virQEMUCapsGet(qemuCaps, QEMU_CAPS_PCI_CONFIGFD)) { - int configfd = qemuOpenPCIConfig(hostdev); - - if (configfd >= 0) { - if (virAsprintf(&configfd_name, "%d", configfd) < 0) { - VIR_FORCE_CLOSE(configfd); - goto error; - } - - virCommandPassFD(cmd, configfd, - VIR_COMMAND_PASS_FD_CLOSE_PARENT); - } - } - virCommandAddArg(cmd, "-device"); - devstr = qemuBuildPCIHostdevDevStr(def, hostdev, bootIndex, - configfd_name, qemuCaps); - VIR_FREE(configfd_name); - if (!devstr) - goto error; - virCommandAddArg(cmd, devstr); - VIR_FREE(devstr); - } else if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_PCIDEVICE)) { - virCommandAddArg(cmd, "-pcidevice"); - if (!(devstr = qemuBuildPCIHostdevPCIDevStr(hostdev, qemuCaps))) - goto error; - virCommandAddArg(cmd, devstr); - VIR_FREE(devstr); - } else { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("PCI device assignment is not supported by this version of qemu")); - goto error; - } - } - - /* SCSI */ - if (hostdev->mode == VIR_DOMAIN_HOSTDEV_MODE_SUBSYS && - hostdev->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI) { - if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE) && - virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_SCSI_GENERIC)) { - char *drvstr; - - virCommandAddArg(cmd, "-drive"); - if (!(drvstr = qemuBuildSCSIHostdevDrvStr(conn, hostdev, qemuCaps, callbacks))) - goto error; - virCommandAddArg(cmd, drvstr); - VIR_FREE(drvstr); - - virCommandAddArg(cmd, "-device"); - if (!(devstr = qemuBuildSCSIHostdevDevStr(def, hostdev, qemuCaps))) - goto error; - virCommandAddArg(cmd, devstr); - VIR_FREE(devstr); - } else { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("SCSI passthrough is not supported by this version of qemu")); - goto error; - } - } - } + if (qemuBuildHostdevCommandLine(cmd, conn, def, qemuCaps, callbacks, + &bootHostdevNet) < 0) + goto error; if (migrateURI) virCommandAddArgList(cmd, "-incoming", migrateURI, NULL); diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h index 4959efb..d83ff8a 100644 --- a/src/qemu/qemu_command.h +++ b/src/qemu/qemu_command.h @@ -143,11 +143,8 @@ int qemuBuildMemoryBackendStr(unsigned long long size, char *qemuBuildMemoryDeviceStr(virDomainMemoryDefPtr mem); -/* Legacy, pre device support */ -char *qemuBuildPCIHostdevPCIDevStr(virDomainHostdevDefPtr dev, - virQEMUCapsPtr qemuCaps); /* Current, best practice */ -char *qemuBuildPCIHostdevDevStr(virDomainDefPtr def, +char *qemuBuildPCIHostdevDevStr(const virDomainDef *def, virDomainHostdevDefPtr dev, int bootIndex, const char *configfd, @@ -170,10 +167,8 @@ char *qemuBuildShmemBackendStr(virDomainShmemDefPtr shmem, int qemuOpenPCIConfig(virDomainHostdevDefPtr dev); -/* Legacy, pre device support */ -char *qemuBuildUSBHostdevUSBDevStr(virDomainHostdevDefPtr dev); /* Current, best practice */ -char *qemuBuildUSBHostdevDevStr(virDomainDefPtr def, +char *qemuBuildUSBHostdevDevStr(const virDomainDef *def, virDomainHostdevDefPtr dev, virQEMUCapsPtr qemuCaps); @@ -182,7 +177,7 @@ char *qemuBuildSCSIHostdevDrvStr(virConnectPtr conn, virQEMUCapsPtr qemuCaps, qemuBuildCommandLineCallbacksPtr callbacks) ATTRIBUTE_NONNULL(4); -char *qemuBuildSCSIHostdevDevStr(virDomainDefPtr def, +char *qemuBuildSCSIHostdevDevStr(const virDomainDef *def, virDomainHostdevDefPtr dev, virQEMUCapsPtr qemuCaps); -- 2.5.0

Add new function to manage adding the memballoon device options to the command line removing that task from the mainline qemuBuildCommandLine. Also modify the qemuBuildMemballoonDevStr to use const virDomainDef instead of virDomainDefPtr. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_command.c | 76 ++++++++++++++++++++++++++++--------------------- src/qemu/qemu_command.h | 2 +- 2 files changed, 45 insertions(+), 33 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 186281f..7876d4f 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -3353,7 +3353,7 @@ qemuBuildWatchdogCommandLine(virCommandPtr cmd, char * -qemuBuildMemballoonDevStr(virDomainDefPtr def, +qemuBuildMemballoonDevStr(const virDomainDef *def, virDomainMemballoonDefPtr dev, virQEMUCapsPtr qemuCaps) { @@ -3401,6 +3401,47 @@ qemuBuildMemballoonDevStr(virDomainDefPtr def, return NULL; } + +static int +qemuBuildMemballoonCommandLine(virCommandPtr cmd, + const virDomainDef *def, + virQEMUCapsPtr qemuCaps) +{ + /* QEMU changed its default behavior to not include the virtio balloon + * device. Explicitly request it to ensure it will be present. + * + * NB: Earlier we declared that VirtIO balloon will always be in + * slot 0x3 on bus 0x0 + */ + if (STREQLEN(def->os.machine, "s390-virtio", 10) && + virQEMUCapsGet(qemuCaps, QEMU_CAPS_VIRTIO_S390) && def->memballoon) + def->memballoon->model = VIR_DOMAIN_MEMBALLOON_MODEL_NONE; + + if (def->memballoon && + def->memballoon->model != VIR_DOMAIN_MEMBALLOON_MODEL_NONE) { + if (def->memballoon->model != VIR_DOMAIN_MEMBALLOON_MODEL_VIRTIO) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("Memory balloon device type '%s' is not supported by this version of qemu"), + virDomainMemballoonModelTypeToString(def->memballoon->model)); + return -1; + } + if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) { + char *optstr; + virCommandAddArg(cmd, "-device"); + + optstr = qemuBuildMemballoonDevStr(def, def->memballoon, qemuCaps); + if (!optstr) + return -1; + virCommandAddArg(cmd, optstr); + VIR_FREE(optstr); + } else if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_BALLOON)) { + virCommandAddArgList(cmd, "-balloon", "virtio", NULL); + } + } + return 0; +} + + static char * qemuBuildNVRAMDevStr(virDomainNVRAMDefPtr dev) { @@ -8986,37 +9027,8 @@ qemuBuildCommandLine(virConnectPtr conn, if (migrateURI) virCommandAddArgList(cmd, "-incoming", migrateURI, NULL); - /* QEMU changed its default behavior to not include the virtio balloon - * device. Explicitly request it to ensure it will be present. - * - * NB: Earlier we declared that VirtIO balloon will always be in - * slot 0x3 on bus 0x0 - */ - if (STREQLEN(def->os.machine, "s390-virtio", 10) && - virQEMUCapsGet(qemuCaps, QEMU_CAPS_VIRTIO_S390) && def->memballoon) - def->memballoon->model = VIR_DOMAIN_MEMBALLOON_MODEL_NONE; - - if (def->memballoon && - def->memballoon->model != VIR_DOMAIN_MEMBALLOON_MODEL_NONE) { - if (def->memballoon->model != VIR_DOMAIN_MEMBALLOON_MODEL_VIRTIO) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("Memory balloon device type '%s' is not supported by this version of qemu"), - virDomainMemballoonModelTypeToString(def->memballoon->model)); - goto error; - } - if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) { - char *optstr; - virCommandAddArg(cmd, "-device"); - - optstr = qemuBuildMemballoonDevStr(def, def->memballoon, qemuCaps); - if (!optstr) - goto error; - virCommandAddArg(cmd, optstr); - VIR_FREE(optstr); - } else if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_BALLOON)) { - virCommandAddArgList(cmd, "-balloon", "virtio", NULL); - } - } + if (qemuBuildMemballoonCommandLine(cmd, def, qemuCaps) < 0) + goto error; for (i = 0; i < def->nrngs; i++) { virDomainRNGDefPtr rng = def->rngs[i]; diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h index d83ff8a..fb44a3c 100644 --- a/src/qemu/qemu_command.h +++ b/src/qemu/qemu_command.h @@ -125,7 +125,7 @@ char *qemuBuildControllerDevStr(const virDomainDef *domainDef, virQEMUCapsPtr qemuCaps, int *nusbcontroller); -char *qemuBuildMemballoonDevStr(virDomainDefPtr domainDef, +char *qemuBuildMemballoonDevStr(const virDomainDef *domainDef, virDomainMemballoonDefPtr dev, virQEMUCapsPtr qemuCaps); -- 2.5.0

Add new function to manage adding the RNG device options to the command line removing that task from the mainline qemuBuildCommandLine. Also modify the qemuBuildRNGDevStr to use const virDomainDef instead of virDomainDefPtr. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_command.c | 80 +++++++++++++++++++++++++++++-------------------- src/qemu/qemu_command.h | 2 +- 2 files changed, 48 insertions(+), 34 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 7876d4f..69284b4 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -5153,7 +5153,7 @@ qemuBuildRNGBackendStr(virDomainRNGDefPtr rng, char * -qemuBuildRNGDevStr(virDomainDefPtr def, +qemuBuildRNGDevStr(const virDomainDef *def, virDomainRNGDefPtr dev, virQEMUCapsPtr qemuCaps) { @@ -5205,6 +5205,50 @@ qemuBuildRNGDevStr(virDomainDefPtr def, } +static int +qemuBuildRNGCommandLine(virCommandPtr cmd, + const virDomainDef *def, + virQEMUCapsPtr qemuCaps) +{ + size_t i; + + for (i = 0; i < def->nrngs; i++) { + virDomainRNGDefPtr rng = def->rngs[i]; + char *tmp; + + if (!rng->info.alias) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("RNG device is missing alias")); + return -1; + } + + /* possibly add character device for backend */ + if (qemuBuildRNGBackendChrdevStr(rng, qemuCaps, &tmp) < 0) + return -1; + + if (tmp) { + virCommandAddArgList(cmd, "-chardev", tmp, NULL); + VIR_FREE(tmp); + } + + /* add the RNG source backend */ + if (!(tmp = qemuBuildRNGBackendStr(rng, qemuCaps))) + return -1; + + virCommandAddArgList(cmd, "-object", tmp, NULL); + VIR_FREE(tmp); + + /* add the device */ + if (!(tmp = qemuBuildRNGDevStr(def, rng, qemuCaps))) + return -1; + virCommandAddArgList(cmd, "-device", tmp, NULL); + VIR_FREE(tmp); + } + + return 0; +} + + static char *qemuBuildSmbiosBiosStr(virSysinfoBIOSDefPtr def) { virBuffer buf = VIR_BUFFER_INITIALIZER; @@ -9030,38 +9074,8 @@ qemuBuildCommandLine(virConnectPtr conn, if (qemuBuildMemballoonCommandLine(cmd, def, qemuCaps) < 0) goto error; - for (i = 0; i < def->nrngs; i++) { - virDomainRNGDefPtr rng = def->rngs[i]; - char *tmp; - - if (!rng->info.alias) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("RNG device is missing alias")); - goto error; - } - - /* possibly add character device for backend */ - if (qemuBuildRNGBackendChrdevStr(rng, qemuCaps, &tmp) < 0) - goto error; - - if (tmp) { - virCommandAddArgList(cmd, "-chardev", tmp, NULL); - VIR_FREE(tmp); - } - - /* add the RNG source backend */ - if (!(tmp = qemuBuildRNGBackendStr(rng, qemuCaps))) - goto error; - - virCommandAddArgList(cmd, "-object", tmp, NULL); - VIR_FREE(tmp); - - /* add the device */ - if (!(tmp = qemuBuildRNGDevStr(def, rng, qemuCaps))) - goto error; - virCommandAddArgList(cmd, "-device", tmp, NULL); - VIR_FREE(tmp); - } + if (qemuBuildRNGCommandLine(cmd, def, qemuCaps) < 0) + goto error; if (def->nvram) { if (ARCH_IS_PPC64(def->os.arch) && diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h index fb44a3c..cef5d6f 100644 --- a/src/qemu/qemu_command.h +++ b/src/qemu/qemu_command.h @@ -150,7 +150,7 @@ char *qemuBuildPCIHostdevDevStr(const virDomainDef *def, const char *configfd, virQEMUCapsPtr qemuCaps); -char *qemuBuildRNGDevStr(virDomainDefPtr def, +char *qemuBuildRNGDevStr(const virDomainDef *def, virDomainRNGDefPtr dev, virQEMUCapsPtr qemuCaps); int qemuBuildRNGBackendProps(virDomainRNGDefPtr rng, -- 2.5.0

Add new function to manage adding the NVRAM device options to the command line removing that task from the mainline qemuBuildCommandLine. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_command.c | 61 ++++++++++++++++++++++++++++++------------------- 1 file changed, 38 insertions(+), 23 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 69284b4..2a6bd5e 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -3467,6 +3467,42 @@ qemuBuildNVRAMDevStr(virDomainNVRAMDefPtr dev) return NULL; } + +static int +qemuBuildNVRAMCommandLine(virCommandPtr cmd, + const virDomainDef *def, + virQEMUCapsPtr qemuCaps) +{ + if (!def->nvram) + return 0; + + if (ARCH_IS_PPC64(def->os.arch) && + STRPREFIX(def->os.machine, "pseries")) { + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_NVRAM)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("nvram device is not supported by " + "this QEMU binary")); + return -1; + } + + char *optstr; + virCommandAddArg(cmd, "-global"); + optstr = qemuBuildNVRAMDevStr(def->nvram); + if (!optstr) + return -1; + if (optstr) + virCommandAddArg(cmd, optstr); + VIR_FREE(optstr); + } else { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("nvram device is only supported for PPC64")); + return -1; + } + + return 0; +} + + static char * qemuBuildVirtioInputDevStr(const virDomainDef *def, virDomainInputDefPtr dev, @@ -9077,30 +9113,9 @@ qemuBuildCommandLine(virConnectPtr conn, if (qemuBuildRNGCommandLine(cmd, def, qemuCaps) < 0) goto error; - if (def->nvram) { - if (ARCH_IS_PPC64(def->os.arch) && - STRPREFIX(def->os.machine, "pseries")) { - if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_NVRAM)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("nvram device is not supported by " - "this QEMU binary")); - goto error; - } + if (qemuBuildNVRAMCommandLine(cmd, def, qemuCaps) < 0) + goto error; - char *optstr; - virCommandAddArg(cmd, "-global"); - optstr = qemuBuildNVRAMDevStr(def->nvram); - if (!optstr) - goto error; - if (optstr) - virCommandAddArg(cmd, optstr); - VIR_FREE(optstr); - } else { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("nvram device is only supported for PPC64")); - goto error; - } - } if (snapshot) virCommandAddArgList(cmd, "-loadvm", snapshot->def->name, NULL); -- 2.5.0

Add new function to manage adding the panic device options to the command line removing that task from the mainline qemuBuildCommandLine. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_command.c | 158 ++++++++++++++++++++++++++---------------------- 1 file changed, 87 insertions(+), 71 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 2a6bd5e..f0266db 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -8805,6 +8805,91 @@ qemuBuildTPMCommandLine(virCommandPtr cmd, } +static int +qemuBuildPanicCommandLine(virCommandPtr cmd, + const virDomainDef *def, + virQEMUCapsPtr qemuCaps) +{ + size_t i; + + for (i = 0; i < def->npanics; i++) { + switch ((virDomainPanicModel) def->panics[i]->model) { + case VIR_DOMAIN_PANIC_MODEL_HYPERV: + /* Panic with model 'hyperv' is not a device, it should + * be configured in cpu commandline. The address + * cannot be configured by the user */ + if (!ARCH_IS_X86(def->os.arch)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("only i686 and x86_64 guests support " + "panic device of model 'hyperv'")); + return -1; + } + if (def->panics[i]->info.type != + VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("setting the panic device address is not " + "supported for model 'hyperv'")); + return -1; + } + break; + + case VIR_DOMAIN_PANIC_MODEL_PSERIES: + /* For pSeries guests, the firmware provides the same + * functionality as the pvpanic device. The address + * cannot be configured by the user */ + if (!ARCH_IS_PPC64(def->os.arch) || + !STRPREFIX(def->os.machine, "pseries")) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("only pSeries guests support panic device " + "of model 'pseries'")); + return -1; + } + if (def->panics[i]->info.type != + VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("setting the panic device address is not " + "supported for model 'pseries'")); + return -1; + } + break; + + case VIR_DOMAIN_PANIC_MODEL_ISA: + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_PANIC)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("the QEMU binary does not support the " + "panic device")); + return -1; + } + + switch (def->panics[i]->info.type) { + case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_ISA: + virCommandAddArg(cmd, "-device"); + virCommandAddArgFormat(cmd, "pvpanic,ioport=%d", + def->panics[i]->info.addr.isa.iobase); + break; + + case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE: + virCommandAddArgList(cmd, "-device", "pvpanic", NULL); + break; + + default: + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("panic is supported only " + "with ISA address type")); + return -1; + } + + /* default model value was changed before in post parse */ + case VIR_DOMAIN_PANIC_MODEL_DEFAULT: + case VIR_DOMAIN_PANIC_MODEL_LAST: + break; + } + } + + return 0; +} + + /** * qemuBuildCommandLineValidate: * @@ -9142,77 +9227,8 @@ qemuBuildCommandLine(virConnectPtr conn, goto error; } - for (i = 0; i < def->npanics; i++) { - switch ((virDomainPanicModel) def->panics[i]->model) { - case VIR_DOMAIN_PANIC_MODEL_HYPERV: - /* Panic with model 'hyperv' is not a device, it should - * be configured in cpu commandline. The address - * cannot be configured by the user */ - if (!ARCH_IS_X86(def->os.arch)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("only i686 and x86_64 guests support " - "panic device of model 'hyperv'")); - goto error; - } - if (def->panics[i]->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("setting the panic device address is not " - "supported for model 'hyperv'")); - goto error; - } - break; - - case VIR_DOMAIN_PANIC_MODEL_PSERIES: - /* For pSeries guests, the firmware provides the same - * functionality as the pvpanic device. The address - * cannot be configured by the user */ - if (!ARCH_IS_PPC64(def->os.arch) || - !STRPREFIX(def->os.machine, "pseries")) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("only pSeries guests support panic device " - "of model 'pseries'")); - goto error; - } - if (def->panics[i]->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("setting the panic device address is not " - "supported for model 'pseries'")); - goto error; - } - break; - - case VIR_DOMAIN_PANIC_MODEL_ISA: - if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_PANIC)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("the QEMU binary does not support the " - "panic device")); - goto error; - } - - switch (def->panics[i]->info.type) { - case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_ISA: - virCommandAddArg(cmd, "-device"); - virCommandAddArgFormat(cmd, "pvpanic,ioport=%d", - def->panics[i]->info.addr.isa.iobase); - break; - - case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE: - virCommandAddArgList(cmd, "-device", "pvpanic", NULL); - break; - - default: - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("panic is supported only " - "with ISA address type")); - goto error; - } - - /* default model value was changed before in post parse */ - case VIR_DOMAIN_PANIC_MODEL_DEFAULT: - case VIR_DOMAIN_PANIC_MODEL_LAST: - break; - } - } + if (qemuBuildPanicCommandLine(cmd, def, qemuCaps) < 0) + goto error; for (i = 0; i < def->nshmems; i++) { if (qemuBuildShmemCommandLine(cmd, def, def->shmems[i], qemuCaps)) -- 2.5.0
participants (2)
-
John Ferlan
-
Peter Krempa