This is the first step in getting rid of the assumption that
isa-serial is the default target type for serial devices.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
---
src/conf/domain_conf.c | 8 +++++---
src/conf/domain_conf.h | 3 ++-
src/qemu/qemu_command.c | 13 +++++++++++++
src/qemu/qemu_domain.c | 21 +++++++++++++++++++++
src/qemu/qemu_domain_address.c | 1 +
5 files changed, 42 insertions(+), 4 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index bd3a23c0a..23ae68b9a 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -446,6 +446,7 @@ VIR_ENUM_IMPL(virDomainChrDeviceState, VIR_DOMAIN_CHR_DEVICE_STATE_LAST,
VIR_ENUM_IMPL(virDomainChrSerialTarget,
VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_LAST,
+ "none",
"isa-serial",
"usb-serial",
"pci-serial")
@@ -4014,7 +4015,7 @@ 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_ISA;
+ def->serials[0]->targetType = VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_NONE;
def->serials[0]->target.port = 0;
} else {
/* if the console source doesn't match */
@@ -4038,7 +4039,8 @@ virDomainDefAddConsoleCompat(virDomainDefPtr def)
def->serials[0]->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_SERIAL) {
switch ((virDomainChrSerialTargetType) def->serials[0]->targetType) {
- case VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_ISA: {
+ case VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_ISA:
+ case VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_NONE: {
/* Create a stub console to match the serial port.
* console[0] either does not exist
@@ -11479,7 +11481,7 @@ virDomainChrDefaultTargetType(int devtype)
return VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_NONE;
case VIR_DOMAIN_CHR_DEVICE_TYPE_SERIAL:
- return VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_ISA;
+ return VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_NONE;
case VIR_DOMAIN_CHR_DEVICE_TYPE_PARALLEL:
case VIR_DOMAIN_CHR_DEVICE_TYPE_LAST:
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 41443a02c..645845dfc 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -1081,7 +1081,8 @@ typedef enum {
} virDomainChrDeviceType;
typedef enum {
- VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_ISA = 0,
+ VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_NONE = 0,
+ VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_ISA,
VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_USB,
VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_PCI,
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 010c2992f..60bdcfb2a 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -9143,6 +9143,14 @@ qemuChrIsPlatformDevice(const virDomainDef *def,
return true;
}
+ /* If we got all the way here and we're still stuck with the default
+ * target type for a serial device, it means we have no clue what kind of
+ * device we're talking about and we must treat it as a platform device */
+ if (chr->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_SERIAL &&
+ chr->targetType == VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_NONE) {
+ return true;
+ }
+
return false;
}
@@ -10246,7 +10254,12 @@ qemuBuildSerialChrDeviceStr(char **deviceStr,
}
break;
+ case VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_NONE:
case VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_LAST:
+ /* Except from _LAST, which is just a guard value and will never
+ * be used, all of the above are platform devices, which means
+ * qemuBuildSerialCommandLine() will have taken the appropriate
+ * branch and we will not have ended up here */
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("Invalid target type for serial device"));
goto error;
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 2d5eee01e..15ab51dbd 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -4047,6 +4047,27 @@ qemuDomainChrDefPostParse(virDomainChrDefPtr chr,
chr->targetType = VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_VIRTIO;
}
+ /* Historically, isa-serial and the default matched, so in order to
+ * maintain backwards compatibility we map them here. The actual default
+ * will be picked below based on the architecture and machine type */
+ if (chr->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_SERIAL &&
+ chr->targetType == VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_ISA) {
+ chr->targetType = VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_NONE;
+ }
+
+ /* Set the default serial type */
+ if (chr->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_SERIAL &&
+ chr->targetType == VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_NONE) {
+ if (ARCH_IS_X86(def->os.arch)) {
+ chr->targetType = VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_ISA;
+ } else if (qemuDomainIsPSeries(def)) {
+ /* Setting TYPE_ISA here is just a temporary hack to reduce test
+ * suite churn. Later on we will have a proper serial type for
+ * pSeries and this line will be updated accordingly */
+ chr->targetType = VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_ISA;
+ }
+ }
+
return 0;
}
diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c
index 7f4ac0f45..989c0e6c9 100644
--- a/src/qemu/qemu_domain_address.c
+++ b/src/qemu/qemu_domain_address.c
@@ -782,6 +782,7 @@ qemuDomainDeviceCalculatePCIConnectFlags(virDomainDeviceDefPtr dev,
case VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_ISA:
case VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_USB:
+ case VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_NONE:
case VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_LAST:
return 0;
}
--
2.13.6
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On Wed, Nov 15, 2017 at 12:50:08PM +0100, Andrea Bolognani wrote:
> This is the first step in getting rid of the assumption that
> isa-serial is the default target type for serial devices.
>
> Signed-off-by: Andrea Bolognani <abologna@redhat.com>
> ---
> src/conf/domain_conf.c | 8 +++++---
> src/conf/domain_conf.h | 3 ++-
> src/qemu/qemu_command.c | 13 +++++++++++++
> src/qemu/qemu_domain.c | 21 +++++++++++++++++++++
> src/qemu/qemu_domain_address.c | 1 +
> 5 files changed, 42 insertions(+), 4 deletions(-)
>
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index bd3a23c0a..23ae68b9a 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -446,6 +446,7 @@ VIR_ENUM_IMPL(virDomainChrDeviceState, VIR_DOMAIN_CHR_DEVICE_STATE_LAST,
>
> VIR_ENUM_IMPL(virDomainChrSerialTarget,
> VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_LAST,
> + "none",
> "isa-serial",
> "usb-serial",
> "pci-serial")
> @@ -4014,7 +4015,7 @@ 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_ISA;
> + def->serials[0]->targetType = VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_NONE;
> def->serials[0]->target.port = 0;
> } else {
> /* if the console source doesn't match */
> @@ -4038,7 +4039,8 @@ virDomainDefAddConsoleCompat(virDomainDefPtr def)
> def->serials[0]->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_SERIAL) {
>
> switch ((virDomainChrSerialTargetType) def->serials[0]->targetType) {
> - case VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_ISA: {
> + case VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_ISA:
> + case VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_NONE: {
>
> /* Create a stub console to match the serial port.
> * console[0] either does not exist
> @@ -11479,7 +11481,7 @@ virDomainChrDefaultTargetType(int devtype)
> return VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_NONE;
>
> case VIR_DOMAIN_CHR_DEVICE_TYPE_SERIAL:
> - return VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_ISA;
> + return VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_NONE;
>
> case VIR_DOMAIN_CHR_DEVICE_TYPE_PARALLEL:
> case VIR_DOMAIN_CHR_DEVICE_TYPE_LAST:
> diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
> index 41443a02c..645845dfc 100644
> --- a/src/conf/domain_conf.h
> +++ b/src/conf/domain_conf.h
> @@ -1081,7 +1081,8 @@ typedef enum {
> } virDomainChrDeviceType;
>
> typedef enum {
> - VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_ISA = 0,
> + VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_NONE = 0,
> + VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_ISA,
> VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_USB,
> VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_PCI,
>
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index 010c2992f..60bdcfb2a 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -9143,6 +9143,14 @@ qemuChrIsPlatformDevice(const virDomainDef *def,
> return true;
> }
>
> + /* If we got all the way here and we're still stuck with the default
> + * target type for a serial device, it means we have no clue what kind of
> + * device we're talking about and we must treat it as a platform device */
Missing full stop/period at the end of the sentence.
> + if (chr->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_SERIAL &&
> + chr->targetType == VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_NONE) {
> + return true;
> + }
> +
> return false;
> }
>
> @@ -10246,7 +10254,12 @@ qemuBuildSerialChrDeviceStr(char **deviceStr,
> }
> break;
>
> + case VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_NONE:
> case VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_LAST:
> + /* Except from _LAST, which is just a guard value and will never
> + * be used, all of the above are platform devices, which means
> + * qemuBuildSerialCommandLine() will have taken the appropriate
> + * branch and we will not have ended up here */
Missing full stop/period at the end of the sentence.
> virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> _("Invalid target type for serial device"));
> goto error;
> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
> index 2d5eee01e..15ab51dbd 100644
> --- a/src/qemu/qemu_domain.c
> +++ b/src/qemu/qemu_domain.c
> @@ -4047,6 +4047,27 @@ qemuDomainChrDefPostParse(virDomainChrDefPtr chr,
> chr->targetType = VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_VIRTIO;
> }
>
> + /* Historically, isa-serial and the default matched, so in order to
> + * maintain backwards compatibility we map them here. The actual default
> + * will be picked below based on the architecture and machine type */
Missing full stop/period at the end of the sentence.
> + if (chr->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_SERIAL &&
> + chr->targetType == VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_ISA) {
> + chr->targetType = VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_NONE;
> + }
> +
> + /* Set the default serial type */
> + if (chr->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_SERIAL &&
> + chr->targetType == VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_NONE) {
> + if (ARCH_IS_X86(def->os.arch)) {
> + chr->targetType = VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_ISA;
> + } else if (qemuDomainIsPSeries(def)) {
> + /* Setting TYPE_ISA here is just a temporary hack to reduce test
> + * suite churn. Later on we will have a proper serial type for
> + * pSeries and this line will be updated accordingly */
Missing full stop/period at the end of the sentence.
> + chr->targetType = VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_ISA;
> + }
> + }
> +
> return 0;
> }
>
> diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c
> index 7f4ac0f45..989c0e6c9 100644
> --- a/src/qemu/qemu_domain_address.c
> +++ b/src/qemu/qemu_domain_address.c
> @@ -782,6 +782,7 @@ qemuDomainDeviceCalculatePCIConnectFlags(virDomainDeviceDefPtr dev,
>
> case VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_ISA:
> case VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_USB:
> + case VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_NONE:
> case VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_LAST:
> return 0;
> }
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On Thu, 2017-11-16 at 12:30 +0100, Pavel Hrdina wrote: > > @@ -9143,6 +9143,14 @@ qemuChrIsPlatformDevice(const virDomainDef *def, > > return true; > > } > > > > + /* If we got all the way here and we're still stuck with the default > > + * target type for a serial device, it means we have no clue what kind of > > + * device we're talking about and we must treat it as a platform device */ > > Missing full stop/period at the end of the sentence. Is that a thing now? It seems like *not* having the full stop is way more common: $ git grep -E '\.($| \*/)' src/ | grep -iEv 'copyr|licen' | wc -l 8166 $ git grep -E '[^\.] \*/' src/ | wc -l 19772 with the former number being bigger than reality because of API documentation and such. -- Andrea Bolognani / Red Hat / Virtualization -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On Thu, Nov 16, 2017 at 02:00:24PM +0100, Andrea Bolognani wrote: > On Thu, 2017-11-16 at 12:30 +0100, Pavel Hrdina wrote: > > > @@ -9143,6 +9143,14 @@ qemuChrIsPlatformDevice(const virDomainDef *def, > > > return true; > > > } > > > > > > + /* If we got all the way here and we're still stuck with the default > > > + * target type for a serial device, it means we have no clue what kind of > > > + * device we're talking about and we must treat it as a platform device */ > > > > Missing full stop/period at the end of the sentence. > > Is that a thing now? It seems like *not* having the full stop is way > more common: > > $ git grep -E '\.($| \*/)' src/ | grep -iEv 'copyr|licen' | wc -l > 8166 > $ git grep -E '[^\.] \*/' src/ | wc -l > 19772 > > with the former number being bigger than reality because of API > documentation and such. Since when something that is more common does mean it's right? :) To make the argument even more absurd, why did you use the period at the end of other sentences or even in commit messages? I hoped that you've just missed it and wanted to point it out, not to start this silly and wasteful conversation. Pavel -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On Thu, 2017-11-16 at 14:08 +0100, Pavel Hrdina wrote: > On Thu, Nov 16, 2017 at 02:00:24PM +0100, Andrea Bolognani wrote: > > On Thu, 2017-11-16 at 12:30 +0100, Pavel Hrdina wrote: > > > > @@ -9143,6 +9143,14 @@ qemuChrIsPlatformDevice(const virDomainDef *def, > > > > return true; > > > > } > > > > > > > > + /* If we got all the way here and we're still stuck with the default > > > > + * target type for a serial device, it means we have no clue what kind of > > > > + * device we're talking about and we must treat it as a platform device */ > > > > > > Missing full stop/period at the end of the sentence. > > > > Is that a thing now? It seems like *not* having the full stop is way > > more common: > > > > $ git grep -E '\.($| \*/)' src/ | grep -iEv 'copyr|licen' | wc -l > > 8166 > > $ git grep -E '[^\.] \*/' src/ | wc -l > > 19772 > > > > with the former number being bigger than reality because of API > > documentation and such. > > Since when something that is more common does mean it's right? :) > To make the argument even more absurd, why did you use the period > at the end of other sentences or even in commit messages? Different contexts often call for different conventions. I would never willfully not capitalize the first word in a sentence or skip the full stop at the end when writing an e-mail, but I regularly do both when using IM. And I know for a fact that so do you ;) > I hoped that you've just missed it and wanted to point it out, not > to start this silly and wasteful conversation. Agreed. I'll add the missing full stops and pick up your R-b. -- Andrea Bolognani / Red Hat / Virtualization -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
© 2016 - 2025 Red Hat, Inc.