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

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

 



On Wed, Sep 20, 2017 at 03:15:23PM +0200, Michal Privoznik wrote:
> 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.

Sigh, I was planning to reply to this patch that we should model the XML
the same way as the reconnect element for chardev devices [1].  Now I
see that the documentation misses an example.  Anyway this should be the
correct XML:

<source type='unix' path='/tmp/vhost2.sock' mode='client'>
  <reconnect enabled='yes' timeout='10'/>
</source>

We should revert this commit and implement it the same way as for
chardev devices.

Using the existing structure should have been a hint that some device
already uses it.

[1] <http://libvirt.org/formatdomain.html#elementsConsole>

Pavel

Attachment: signature.asc
Description: PGP signature

--
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