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(a)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(a)redhat.com>