On Tue, 2017-11-21 at 17:42 +0100, Andrea Bolognani wrote:
> +static void
> +virDomainChrConsoleTargetTypeToSerial(virDomainChrConsoleTargetType type,
> + int *retType, int *retModel)
The last two arguments should be each on a separate line.
I'd also prefer the names 'targetType' and 'targetModel'.
> @@ -4014,7 +4048,9 @@ virDomainDefAddConsoleCompat(virDomainDefPtr def,
> }
> if (def->nconsoles > 0 && def->os.type == VIR_DOMAIN_OSTYPE_HVM &&
> (def->consoles[0]->targetType == VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SERIAL ||
> - def->consoles[0]->targetType == VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_NONE)) {
> + def->consoles[0]->targetType == VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_NONE ||
> + (def->consoles[0]->targetType == VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SCLP &&
> + (parseFlags & VIR_DOMAIN_DEF_PARSE_ABI_UPDATE)))) {
VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SCLPLM should be included in the
check above, or <console/>s with <target type='sclplm'/> will not
be converted into <serial/>s.
> @@ -4027,6 +4063,8 @@ virDomainDefAddConsoleCompat(virDomainDefPtr def,
>
> /* create the serial port definition from the console definition */
> if (def->nserials == 0) {
> + virDomainChrConsoleTargetType type = def->consoles[0]->targetType;
> +
> if (VIR_APPEND_ELEMENT(def->serials,
> def->nserials,
> def->consoles[0]) < 0)
> @@ -4034,7 +4072,9 @@ virDomainDefAddConsoleCompat(virDomainDefPtr def,
>
> /* modify it to be a serial port */
> def->serials[0]->deviceType = VIR_DOMAIN_CHR_DEVICE_TYPE_SERIAL;
> - def->serials[0]->targetType = VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_NONE;
> + virDomainChrConsoleTargetTypeToSerial(type,
> + &def->serials[0]->targetType,
> + &def->serials[0]->targetModel);
Drop 'type' and just use 'def->consoles[0]->targetType' here.
Everything else looks good, so my R-b stands. I can fix all of the
above before pushing (assuming a respin won't be necessary).
--
Andrea Bolognani / Red Hat / Virtualization
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
