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@xxxxxxxxxx>
> ---
> 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@xxxxxxxxxx>
Attachment:
signature.asc
Description: PGP signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
