[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH 06/14] conf: Drop virDomainChrDeviceType.targetTypeAttr



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>

Attachment: signature.asc
Description: PGP signature


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]