Re: [PATCHv4 RESEND] vhost-user: add support reconnect for vhost-user ports

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

 



On 01/17/2018 05:14 PM, ZhiPeng Lu wrote:
> For vhost-user ports, Open vSwitch acts as the server and QEMU the client.
> When OVS crashes or restarts, the QEMU process should be reconnected to
> OVS.
> 
> Signed-off-by: ZhiPeng Lu <lu.zhipeng@xxxxxxxxxx>
> Signed-off-by: Michal Privoznik <mprivozn@xxxxxxxxxx>
> ---
> v1->v2:
>  - modify xml format
> v2->v3:
>  - fix commit message syntax
>  - reimplemente functions and the struct about reconnect
> v3->v4:
>  - revert reimplemente functions and the struct about reconnect
> ---
>  docs/formatdomain.html.in                        |   7 +-
>  docs/schemas/domaincommon.rng                    |  26 ++--
>  src/conf/domain_conf.c                           | 158 +++++++++++++----------
>  tests/qemuxml2argvdata/net-vhostuser-multiq.args |  12 +-
>  tests/qemuxml2argvdata/net-vhostuser-multiq.xml  |  11 +-
>  5 files changed, 127 insertions(+), 87 deletions(-)
> 
> diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
> index d272cc1..f0f9f4b 100644
> --- a/docs/formatdomain.html.in
> +++ b/docs/formatdomain.html.in
> @@ -5832,7 +5832,9 @@ qemu-kvm -net nic,model=? /dev/null
>    &lt;/interface&gt;
>    &lt;interface type='vhostuser'&gt;
>      &lt;mac address='52:54:00:3b:83:1b'/&gt;
> -    &lt;source type='unix' path='/tmp/vhost2.sock' mode='client'/&gt;
> +    &lt;source type='unix' path='/tmp/vhost2.sock' mode='client'&gt;
> +      &lt;reconnect enabled='yes' timeout='10'/&gt;
> +    &lt;/source&gt; 

Extra space at EOL.

>      &lt;model type='virtio'/&gt;
>      &lt;driver queues='5'/&gt;
>    &lt;/interface&gt;
> @@ -5848,6 +5850,9 @@ qemu-kvm -net nic,model=? /dev/null
>        are supported.
>        vhost-user requires the virtio model type, thus the
>        <code>&lt;model&gt;</code> element is mandatory.
> +      <span class="since">Since 3.10.0</span> the element has an optional

Since 4.1.0

> +      attribute <code>reconnect</code> which configures reconnect timeout

Not attribute. Child element.

> +      (in seconds) if the connection is lost.
>      </p>
>  
>      <h5><a id="elementNwfilter">Traffic filtering with NWFilter</a></h5>
> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
> index f22c932..9258c7d 100644
> --- a/docs/schemas/domaincommon.rng
> +++ b/docs/schemas/domaincommon.rng
> @@ -2399,6 +2399,18 @@
>        </attribute>
>      </optional>
>    </define>
> +  <define name="reconnect">
> +    <element name="reconnect">
> +      <attribute name="enabled">
> +        <ref name="virYesNo"/>
> +      </attribute>
> +      <optional>
> +        <attribute name="timeout">
> +          <ref name="unsignedInt"/>
> +        </attribute>
> +      </optional>
> +    </element>
> +  </define>
>  
>    <!--
>        An interface description can either be of type bridge in which case
> @@ -2460,6 +2472,9 @@
>                    <value>client</value>
>                  </choice>
>                </attribute>
> +              <optional>
> +                 <ref name="reconnect"/>
> +              </optional>
>                <empty/>
>              </element>
>              <ref name="interface-options"/>
> @@ -3728,16 +3743,7 @@
>            </attribute>
>          </optional>
>          <optional>
> -          <element name="reconnect">
> -            <attribute name="enabled">
> -              <ref name="virYesNo"/>
> -            </attribute>
> -            <optional>
> -              <attribute name="timeout">
> -                <ref name="unsignedInt"/>
> -              </attribute>
> -            </optional>
> -          </element>
> +          <ref name="reconnect"/>
>          </optional>
>          <zeroOrMore>
>            <ref name='devSeclabel'/>
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index a1c2506..ccc8ff7 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -10695,6 +10695,56 @@ virDomainNetAppendIPAddress(virDomainNetDefPtr def,
>      return -1;
>  }
>  
> +static int
> +virDomainChrSourceReconnectDefParseXML(virDomainChrSourceReconnectDefPtr def,
> +                                       xmlNodePtr node,
> +                                       xmlXPathContextPtr ctxt)
> +{
> +    int ret = -1;
> +    int tmpVal;
> +    char *tmp = NULL;
> +    xmlNodePtr saveNode = ctxt->node;
> +    xmlNodePtr cur;
> +
> +    ctxt->node = node;
> +
> +    if ((cur = virXPathNode("./reconnect", ctxt))) {
> +        if ((tmp = virXMLPropString(cur, "enabled"))) {
> +            if ((tmpVal = virTristateBoolTypeFromString(tmp)) < 0) {
> +                virReportError(VIR_ERR_XML_ERROR,
> +                               _("invalid reconnect enabled value: '%s'"),
> +                               tmp);
> +                goto cleanup;
> +            }
> +            def->enabled = tmpVal;
> +            VIR_FREE(tmp);
> +        }
> +
> +        if (def->enabled == VIR_TRISTATE_BOOL_YES) {
> +            if ((tmp = virXMLPropString(cur, "timeout"))) {
> +                if (virStrToLong_ui(tmp, NULL, 10, &def->timeout) < 0) {
> +                    virReportError(VIR_ERR_XML_ERROR,
> +                                   _("invalid reconnect timeout value: '%s'"),
> +                                   tmp);
> +                    goto cleanup;
> +                }
> +                VIR_FREE(tmp);
> +            } else {
> +                virReportError(VIR_ERR_XML_ERROR, "%s",
> +                               _("missing timeout for chardev with "
> +                                 "reconnect enabled"));
> +                goto cleanup;
> +            }
> +        }
> +    }
> +
> +    ret = 0;
> + cleanup:
> +    ctxt->node = saveNode;
> +    VIR_FREE(tmp);
> +    return ret;
> +}
> +
>  /* Parse the XML definition for a network interface
>   * @param node XML nodeset to parse for net definition
>   * @return 0 on success, -1 on failure
> @@ -10749,6 +10799,7 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt,
>      virNWFilterHashTablePtr filterparams = NULL;
>      virDomainActualNetDefPtr actual = NULL;
>      xmlNodePtr oldnode = ctxt->node;
> +    virDomainChrSourceReconnectDef reconnect = {0};
>      int rv, val;
>  
>      if (VIR_ALLOC(def) < 0)
> @@ -10830,11 +10881,14 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt,
>                      goto error;
>                  }
>              } else if (!vhostuser_path && !vhostuser_mode && !vhostuser_type
> -                       && def->type == VIR_DOMAIN_NET_TYPE_VHOSTUSER &&
> -                       virXMLNodeNameEqual(cur, "source")) {
> +                       && def->type == VIR_DOMAIN_NET_TYPE_VHOSTUSER
> +                       && virXMLNodeNameEqual(cur, "source")) {

In fact we like logical operands at the end of previous line.

>                  vhostuser_type = virXMLPropString(cur, "type");
>                  vhostuser_path = virXMLPropString(cur, "path");
>                  vhostuser_mode = virXMLPropString(cur, "mode");
> +                if (virDomainChrSourceReconnectDefParseXML(&reconnect, cur, ctxt) < 0)
> +                    goto error;
> +
>              } else if (!def->virtPortProfile
>                         && virXMLNodeNameEqual(cur, "virtualport")) {
>                  if (def->type == VIR_DOMAIN_NET_TYPE_NETWORK) {
> @@ -11056,8 +11110,16 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt,
>  
>          if (STREQ(vhostuser_mode, "server")) {
>              def->data.vhostuser->data.nix.listen = true;
> +            if (reconnect.enabled != VIR_TRISTATE_BOOL_ABSENT) {

We can accept BOOL_NO. Therefore the check needs to be:

  if (reconnect.enabled == VIR_TRISTATE_BOOL_YES) { ... }

> +                virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> +                               _("'reconnect' attribute  unsupported "
> +                                 "'server' mode for <interface type='vhostuser'>"));
> +                goto error;
> +           }
>          } else if (STREQ(vhostuser_mode, "client")) {
>              def->data.vhostuser->data.nix.listen = false;
> +            def->data.vhostuser->data.nix.reconnect.enabled = reconnect.enabled;
> +            def->data.vhostuser->data.nix.reconnect.timeout = reconnect.timeout;

Or just ->data.nix.reconnect = reconnect;

>          } else {
>              virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
>                             _("Wrong <source> 'mode' attribute "
> @@ -11763,57 +11825,6 @@ virDomainChrDefParseTargetXML(virDomainChrDefPtr def,
>      return ret;
>  }

There are some other small nits. Anyway, I'm fixing all of the raised
points, ACKing and pushing.

Michal

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list



[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]
  Powered by Linux