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
> > </interface>
> > <interface type='vhostuser'>
> > <mac address='52:54:00:3b:83:1b'/>
> > - <source type='unix' path='/tmp/vhost2.sock' mode='client'/>
> > + <source type='unix' path='/tmp/vhost2.sock' mode='client' reconnect='10'/>
> > <model type='virtio'/>
> > <driver queues='5'/>
> > </interface>
> > @@ -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><model></code> element is mandatory.
> > + <code><model></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
