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

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

 



On 09/20/2017 03:59 PM, Pavel Hrdina wrote:
> 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>

I was thinking about this too. Problem with this is that <source/> for
<interface> has slightly different meaning than for <channel/>. For
instance, we allow:

    <interface type='ethernet'>
      <mac address='00:16:3e:0f:ef:8a'/>
      <source>
        <ip address='192.168.122.12' family='ipv4' prefix='24' peer='192.168.122.1'/>
        <ip address='192.168.122.13' family='ipv4' prefix='24'/>
        <route family='ipv4' address='0.0.0.0' gateway='192.168.122.1'/>
        <route family='ipv4' address='192.168.124.0' prefix='24' gateway='192.168.124.1'/>
      </source>
      <ip address='192.168.122.1' family='ipv4' prefix='32' peer='192.168.122.12'/>
      <guest dev='eth2'/>
    </interface>

But now that I think about it, sure. If you think that having it as a
separate element instead of an attribute is better do provide patches.

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