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

[Michal Privoznik <mprivozn@xxxxxxxxxx>]

On 09/08/2017 11:12 AM, ZhiPeng Lu wrote:
> For vhost-user ports, Open vSwitch acts as the server and QEMU the client.
> When OVS crashed or restart, QEMU shoule be reconnect to OVS.
> 
> Signed-off-by: ZhiPeng Lu <lu.zhipeng@xxxxxxxxxx>
> ---
>  docs/formatdomain.html.in                          |  6 +++--
>  docs/schemas/domaincommon.rng                      |  5 ++++
>  src/conf/domain_conf.c                             | 28 ++++++++++++++++++++--
>  .../qemuxml2argv-net-vhostuser-multiq.args         |  2 +-
>  .../qemuxml2argv-net-vhostuser-multiq.xml          |  2 +-
>  5 files changed, 37 insertions(+), 6 deletions(-)
> 
> diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
> index 8ca7637..ffe45c2 100644
> --- a/docs/formatdomain.html.in
> +++ b/docs/formatdomain.html.in
> @@ -5660,7 +5660,7 @@ 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' reconnect='10'/&gt;
>      &lt;model type='virtio'/&gt;
>      &lt;driver queues='5'/&gt;
>    &lt;/interface&gt;
> @@ -5675,7 +5675,9 @@ qemu-kvm -net nic,model=? /dev/null
>        Both <code>mode='server'</code> and <code>mode='client'</code>
>        are supported.
>        vhost-user requires the virtio model type, thus the
> -      <code>&lt;model&gt;</code> element is mandatory.
> +      <code>&lt;model&gt;</code> element is mandatory. <code>reconnect</code>
> +      is an optional element,which configures reconnect timeout if the
> +      connection is lost.

This is missing "Since 3.7.0".

>      </p>
>  
>      <h5><a id="elementNwfilter">Traffic filtering with NWFilter</a></h5>
> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
> index 06c5a91..82f30ae 100644
> --- a/docs/schemas/domaincommon.rng
> +++ b/docs/schemas/domaincommon.rng
> @@ -2383,6 +2383,11 @@
>                      <value>client</value>
>                    </choice>
>                  </attribute>
> +                <optional>
> +                  <attribute name="reconnect">
> +                    <ref name='positiveInteger'/>
> +                  </attribute>
> +                </optional>
>                  <empty/>
>                </element>
>              <ref name="interface-options"/>
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 2fc1fc3..f9c3b35 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -10176,6 +10176,7 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt,
>      char *vhostuser_mode = NULL;
>      char *vhostuser_path = NULL;
>      char *vhostuser_type = NULL;
> +    char *vhostuser_reconnect = NULL;
>      char *trustGuestRxFilters = NULL;
>      char *vhost_path = NULL;
>      virNWFilterHashTablePtr filterparams = NULL;
> @@ -10262,11 +10263,12 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt,
>                      goto error;
>                  }
>              } else if (!vhostuser_path && !vhostuser_mode && !vhostuser_type
> -                       && def->type == VIR_DOMAIN_NET_TYPE_VHOSTUSER &&
> -                       virXMLNodeNameEqual(cur, "source")) {
> +                       && !vhostuser_reconnect && def->type == VIR_DOMAIN_NET_TYPE_VHOSTUSER
> +                       && virXMLNodeNameEqual(cur, "source")) {
>                  vhostuser_type = virXMLPropString(cur, "type");
>                  vhostuser_path = virXMLPropString(cur, "path");
>                  vhostuser_mode = virXMLPropString(cur, "mode");
> +                vhostuser_reconnect = virXMLPropString(cur, "reconnect");
>              } else if (!def->virtPortProfile
>                         && virXMLNodeNameEqual(cur, "virtualport")) {
>                  if (def->type == VIR_DOMAIN_NET_TYPE_NETWORK) {
> @@ -10478,6 +10480,11 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt,
>                               "type='vhostuser'/>"));
>              goto error;
>          }
> +        if (vhostuser_reconnect != NULL && STREQ(vhostuser_mode, "server")) {
> +            virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> +                               _("'reconnect' attribute  unsupported "
> +                                 "'server' mode for <interface type='vhostuser'>"));
> +        }

This can be checked later, when we validate vhostuser_mode.

>  
>          if (VIR_ALLOC(def->data.vhostuser) < 0)
>              goto error;
> @@ -10490,6 +10497,17 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt,
>              def->data.vhostuser->data.nix.listen = true;
>          } else if (STREQ(vhostuser_mode, "client")) {
>              def->data.vhostuser->data.nix.listen = false;
> +            if (vhostuser_reconnect != NULL) {
> +                def->data.vhostuser->data.nix.reconnect.enabled = true;
> +                if (virStrToLong_ui(vhostuser_reconnect, NULL, 0,
> +                                   &def->data.vhostuser->data.nix.reconnect.timeout) < 0) {
> +                    virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +                                 _("vhost-user reconnect attribute is invalid"));
> +                    vhostuser_reconnect = NULL;
> +                    def->data.vhostuser->data.nix.reconnect.enabled = false;

There are two problems with this. Setting vhostuser_reconnect to NULL
leaks it when the control jumps to the error label. Secondly, there's no
need to set reconnect.enabled to false. Oh, yes and we know what base
the reconnect number is in. Therefore s/0/10/. I guess we don't want to
allow:

  reconnect="0x50"

I've fixed all these problems, ACKed and pushed.

Michal

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