On Wed, Nov 15, 2017 at 12:50:09PM +0100, Andrea Bolognani wrote:
This attribute was used to decide whether to format the type attribute of the <target> element, but the logic didn't take into account all possible cases and as such could lead to unexpected results. Moreover, it's one more thing to keep track of, and can easily fall out of sync with other attributes.
Now that we have VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_NONE, we can use that value to signal that no specific target type has been configured for the serial device and as such the attribute should not be formatted at all. All other values are now formatted.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- src/conf/domain_conf.c | 11 ++++------- src/conf/domain_conf.h | 1 - src/vz/vz_sdk.c | 3 +-- tests/qemuargv2xmldata/qemuargv2xml-console-compat.xml | 2 +- tests/qemuargv2xmldata/qemuargv2xml-serial-dev.xml | 2 +- tests/qemuargv2xmldata/qemuargv2xml-serial-file.xml | 2 +- tests/qemuargv2xmldata/qemuargv2xml-serial-many.xml | 4 ++-- tests/qemuargv2xmldata/qemuargv2xml-serial-pty.xml | 2 +- tests/qemuargv2xmldata/qemuargv2xml-serial-tcp-telnet.xml | 2 +- tests/qemuargv2xmldata/qemuargv2xml-serial-tcp.xml | 2 +- tests/qemuargv2xmldata/qemuargv2xml-serial-udp.xml | 4 ++-- tests/qemuargv2xmldata/qemuargv2xml-serial-unix.xml | 2 +- tests/qemuargv2xmldata/qemuargv2xml-serial-vc.xml | 2 +- .../qemuhotplug-console-compat-2-live+console-virtio.xml | 4 ++-- .../qemuhotplug-console-compat-2-live.xml | 4 ++-- .../qemuxml2argv-serial-tcp-tlsx509-chardev-notls.xml | 4 ++-- tests/qemuxml2argvdata/qemuxml2argv-user-aliases.xml | 4 ++-- .../qemuxml2xmlout-bios-nvram-os-interleave.xml | 2 +- tests/qemuxml2xmloutdata/qemuxml2xmlout-chardev-label.xml | 4 ++-- .../qemuxml2xmloutdata/qemuxml2xmlout-console-compat-auto.xml | 2 +- tests/qemuxml2xmloutdata/qemuxml2xmlout-console-compat.xml | 2 +- tests/qemuxml2xmloutdata/qemuxml2xmlout-console-compat2.xml | 2 +- .../qemuxml2xmloutdata/qemuxml2xmlout-console-virtio-many.xml | 2 +- tests/qemuxml2xmloutdata/qemuxml2xmlout-interface-driver.xml | 2 +- tests/qemuxml2xmloutdata/qemuxml2xmlout-interface-server.xml | 4 ++-- tests/qemuxml2xmloutdata/qemuxml2xmlout-net-bandwidth.xml | 2 +- tests/qemuxml2xmloutdata/qemuxml2xmlout-net-bandwidth2.xml | 2 +- tests/qemuxml2xmloutdata/qemuxml2xmlout-net-coalesce.xml | 2 +- tests/qemuxml2xmloutdata/qemuxml2xmlout-net-mtu.xml | 2 +- tests/qemuxml2xmloutdata/qemuxml2xmlout-panic-pseries.xml | 2 +- .../qemuxml2xmlout-pseries-cpu-compat-power9.xml | 2 +- .../qemuxml2xmloutdata/qemuxml2xmlout-pseries-cpu-compat.xml | 2 +- tests/qemuxml2xmloutdata/qemuxml2xmlout-pseries-cpu-exact.xml | 2 +- .../qemuxml2xmlout-pseries-panic-missing.xml | 2 +- .../qemuxml2xmlout-pseries-panic-no-address.xml | 2 +- .../qemuxml2xmlout-q35-virt-manager-basic.xml | 2 +- .../qemuxml2xmlout-serial-spiceport-nospice.xml | 2 +- tests/qemuxml2xmloutdata/qemuxml2xmlout-serial-spiceport.xml | 2 +- .../qemuxml2xmlout-serial-target-port-auto.xml | 6 +++--- .../qemuxml2xmlout-serial-tcp-tlsx509-chardev.xml | 4 ++-- .../qemuxml2xmloutdata/qemuxml2xmlout-tap-vhost-incorrect.xml | 2 +- tests/qemuxml2xmloutdata/qemuxml2xmlout-tap-vhost.xml | 2 +- tests/qemuxml2xmloutdata/qemuxml2xmlout-vhost_queues.xml | 2 +- 43 files changed, 56 insertions(+), 61 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 23ae68b9a..0bcfd5537 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -11493,8 +11493,7 @@ virDomainChrDefaultTargetType(int devtype) }
static int -virDomainChrTargetTypeFromString(virDomainChrDefPtr def, - int devtype, +virDomainChrTargetTypeFromString(int devtype, const char *targetType) { int ret = -1; @@ -11522,8 +11521,6 @@ virDomainChrTargetTypeFromString(virDomainChrDefPtr def, break; }
- def->targetTypeAttr = true; - return ret; }
@@ -11540,7 +11537,7 @@ virDomainChrDefParseTargetXML(virDomainChrDefPtr def, char *stateStr = NULL;
if ((def->targetType = - virDomainChrTargetTypeFromString(def, def->deviceType, + virDomainChrTargetTypeFromString(def->deviceType, targetType)) < 0) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("unknown target type '%s' specified for character device"), @@ -16460,7 +16457,7 @@ virDomainChrEquals(virDomainChrDefPtr src, break;
case VIR_DOMAIN_CHR_DEVICE_TYPE_SERIAL: - if (src->targetTypeAttr != tgt->targetTypeAttr) + if (src->targetType != tgt->targetType) return false;
ATTRIBUTE_FALLTHROUGH; @@ -24020,7 +24017,7 @@ virDomainChrDefFormat(virBufferPtr buf, break;
case VIR_DOMAIN_CHR_DEVICE_TYPE_SERIAL: - if (def->targetTypeAttr) { + if (def->targetType) {
I would prefer explicit check if it's equal to VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_NONE. It's not a bool variable.
virBufferAsprintf(buf, "<target type='%s' port='%d'/>\n", virDomainChrTargetTypeToString(def->deviceType, diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 645845dfc..9856fbc10 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1207,7 +1207,6 @@ struct _virDomainChrSourceDef { struct _virDomainChrDef { int deviceType; /* enum virDomainChrDeviceType */
- bool targetTypeAttr; int targetType; /* enum virDomainChrConsoleTargetType || enum virDomainChrChannelTargetType || enum virDomainChrSerialTargetType according to deviceType */ diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c index 819b02b1e..c9f2a13e4 100644 --- a/src/vz/vz_sdk.c +++ b/src/vz/vz_sdk.c @@ -1191,7 +1191,6 @@ prlsdkGetSerialInfo(PRL_HANDLE serialPort, virDomainChrDefPtr chr) int ret = -1;
chr->deviceType = VIR_DOMAIN_CHR_DEVICE_TYPE_SERIAL; - chr->targetTypeAttr = false; pret = PrlVmDev_GetIndex(serialPort, &serialPortIndex); prlsdkCheckRetGoto(pret, cleanup); chr->target.port = serialPortIndex; @@ -2864,7 +2863,7 @@ static int prlsdkCheckSerialUnsupportedParams(virDomainChrDefPtr chr) return -1; }
- if (chr->targetTypeAttr) { + if (chr->targetType) {
Same here.
virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("Specified character device target type is not " "supported by vz driver."));
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>