答复: Re: [PATCH] vhost-user: add support reconnect forvhost-user ports

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

 



>>                <empty/>
>>              </element>
> >             <ref name="interface-options"/>
>> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
>> index cc5e79b..d121a9e 100644
>> --- a/src/conf/domain_conf.c
>> +++ b/src/conf/domain_conf.c
>> @@ -83,6 +83,13 @@ struct _virDomainXMLOption {
>>      /* Private data for save image stored in snapshot XML */
>>      virSaveCookieCallbacks saveCookie;
>>  };
>> +static int
>> +virDomainChrSourceReconnectDefParseXML(virDomainChrSourceReconnectDefPtr def,
>> +                                       xmlNodePtr node,
>> +                                       xmlXPathContextPtr ctxt);
>> +static void
>> +virDomainChrSourceReconnectDefFormat(virBufferPtr buf,
>> +                                     virDomainChrSourceReconnectDefPtr def);

>Since these functions (and the struct) are no longer used just for chr
>source, their names should be made more generic - remove the "Chr" from
>them all (maybe replace it with "Device"?)

In fact, these functions are still used to char device of  vhostuser devices. 

Changing the name may not reflect the meaning.







为了让您的VPlat虚拟机故障和docker故障得到高效的处理,请上报故障到: $VPlat技术支持。

芦志朋 luzhipeng


IT开发工程师 IT Development Engineer
操作系统产品部/中心研究院/系统产品 OS Product Dept./Central R&D Institute/System Product



四川省成都市天府大道中段800号
E: lu.zhipeng@xxxxxxxxxx
www.zte.com.cn
原始邮件
发件人: <laine@xxxxxxxxx>;
收件人: <libvir-list@xxxxxxxxxx>;
抄送人:芦志朋10108272; <mprivozn@xxxxxxxxxx>; <phrdina@xxxxxxxxxx>;
日 期 :2017年09月22日 01:45
主 题 :Re: [libvirt] [PATCH] vhost-user: add support reconnect forvhost-user ports


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

When sending a v2/v3/etc of a patch, use the "-v2" (or -v3, -v4, etc)
option to git send-email so that the subject line shows up as "[PATCH v2]".

On 09/21/2017 07:39 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.


"When OVS crashes or restarts, the QEMU process should be reconnected to
OVS"

>
> Signed-off-by: ZhiPeng Lu <lu.zhipeng@xxxxxxxxxx>
> ---
> v1->v2:
>  - modify xml format
> ---

Looks like you added the above text to your commit log message. Instead,
you can just add it in below the "---" line during git send-email, with
the "--annotate" option.


> ---
>  docs/schemas/domaincommon.rng                      | 12 ++++++++++
>  src/conf/domain_conf.c                             | 26 ++++++++++++++++++++++
>  .../qemuxml2argv-net-vhostuser-multiq.args         |  4 ++--
>  .../qemuxml2argv-net-vhostuser-multiq.xml          |  8 +++++--
>  4 files changed, 46 insertions(+), 4 deletions(-)
>
> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
> index 76852ab..bf015ca 100644
> --- a/docs/schemas/domaincommon.rng
> +++ b/docs/schemas/domaincommon.rng
> @@ -2388,6 +2388,18 @@
>                    <value>client</value>
>                  </choice>
>                </attribute>
> +              <optional>
> +                <element name="reconnect">
> +                  <attribute name="enabled">
> +                    <ref name="virYesNo"/>
> +                  </attribute>
> +                  <optional>
> +                    <attribute name="timeout">
> +                      <ref name="unsignedInt"/>
> +                    </attribute>
> +                  </optional>
> +               </element>
> +              </optional>

Rather than doing a copy-paste of this from qemucdevSrcDef, it would be
more useful to *move* it to create a new definition for something called
"reconnect":

              <define name="reconnect">
                <element name="reconnect">
                  <attribute name="enabled">
                    <ref name="virYesNo"/>
                  </attribute>
                  <optional>
                    <attribute name="timeout">
                      <ref name="unsignedInt"/>
                    </attribute>
                  </optional>
               </element>
             </define>

then reference that where it's needed (both for interface source and qemucdevSrcDef):


        <optional>
          <ref name="reconnect"/>
        </optional>

>                <empty/>
>              </element>
>              <ref name="interface-options"/>
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index cc5e79b..d121a9e 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -83,6 +83,13 @@ struct _virDomainXMLOption {
>      /* Private data for save image stored in snapshot XML */
>      virSaveCookieCallbacks saveCookie;
>  };
> +static int
> +virDomainChrSourceReconnectDefParseXML(virDomainChrSourceReconnectDefPtr def,
> +                                       xmlNodePtr node,
> +                                       xmlXPathContextPtr ctxt);
> +static void
> +virDomainChrSourceReconnectDefFormat(virBufferPtr buf,
> +                                     virDomainChrSourceReconnectDefPtr def);

Since these functions (and the struct) are no longer used just for chr
source, their names should be made more generic - remove the "Chr" from
them all (maybe replace it with "Device"?)

>  
>  #define VIR_DOMAIN_DEF_FORMAT_COMMON_FLAGS             \
>      (VIR_DOMAIN_DEF_FORMAT_SECURE |                    \
> @@ -10245,6 +10252,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)
> @@ -10331,6 +10339,8 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt,
>                  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) {
> @@ -10552,8 +10562,17 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt,
>  
>          if (STREQ(vhostuser_mode, "server")) {
>              def->data.vhostuser->data.nix.listen = true;
> +            if (reconnect.enabled != VIR_TRISTATE_BOOL_ABSENT) {
> +                virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> +                               _("'reconnect' attribute  unsupported "
> +                                 "'server' mode for <interface type='vhostuser'>"));
> +                goto error;
> +           }

Ugh. Having the reconnect info stored all the way down in a
device-type-specific part of the object makes it *really* difficult to
do proper validation (e.g. if someone adds a <reconnect> element to the
<source> of a <hostdev>, it will be ignored, then silently disappear,
rather than being reported as an error. Unfortunately we don't have a
generic "source" sub-object that is a part of all device objects
(similar to the virDomainDeviceInfo that is a part of every device no
matter the type), so there isn't a real clear place to put it. It would
be nice to be able to properly log an error when <reconnect> is used in
an inappropriate place, though (admittedly, there are infinite things
that might be added to the XML and be silently ignored, so it may be
appropriate to "punt" in this case too.)


>          } 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;
> +            reconnect.enabled = VIR_TRISTATE_BOOL_ABSENT;
>          } else {
>              virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
>                             _("Wrong <source> 'mode' attribute "
> @@ -22984,6 +23003,13 @@ virDomainNetDefFormat(virBufferPtr buf,
>                                    def->data.vhostuser->data.nix.listen ?
>                                    "server"  : "client");
>                  sourceLines++;
> +                if (def->data.vhostuser->data.nix.reconnect.enabled != VIR_TRISTATE_BOOL_ABSENT) {
> +                    virBufferAddLit(buf, ">\n");
> +                    sourceLines++;
> +                    virBufferAdjustIndent(buf, 2);
> +                    virDomainChrSourceReconnectDefFormat(buf, &def->data.vhostuser->data.nix.reconnect);
> +                    virBufferAdjustIndent(buf, -2);
> +                }
>              }
>              break;
>  
> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-net-vhostuser-multiq.args b/tests/qemuxml2argvdata/qemuxml2argv-net-vhostuser-multiq.args
> index b69ebd8..0b08f44 100644
> --- a/tests/qemuxml2argvdata/qemuxml2argv-net-vhostuser-multiq.args
> +++ b/tests/qemuxml2argvdata/qemuxml2argv-net-vhostuser-multiq.args
> @@ -25,14 +25,14 @@ server,nowait \
>  -netdev vhost-user,chardev=charnet0,id=hostnet0 \
>  -device virtio-net-pci,netdev=hostnet0,id=net0,mac=52:54:00:ee:96:6b,bus=pci.0,\
>  addr=0x3 \
> --chardev socket,id=charnet1,path=/tmp/vhost1.sock \
> +-chardev socket,id=charnet1,path=/tmp/vhost1.sock,reconnect=10 \
>  -netdev vhost-user,chardev=charnet1,id=hostnet1 \
>  -device virtio-net-pci,netdev=hostnet1,id=net1,mac=52:54:00:ee:96:6c,bus=pci.0,\
>  addr=0x4 \
>  -netdev socket,listen=:2015,id=hostnet2 \
>  -device rtl8139,netdev=hostnet2,id=net2,mac=52:54:00:95:db:c0,bus=pci.0,\
>  addr=0x5 \
> --chardev socket,id=charnet3,path=/tmp/vhost2.sock \
> +-chardev socket,id=charnet3,path=/tmp/vhost2.sock,reconnect=0 \
>  -netdev vhost-user,chardev=charnet3,queues=4,id=hostnet3 \
>  -device virtio-net-pci,mq=on,vectors=10,netdev=hostnet3,id=net3,\
>  mac=52:54:00:ee:96:6d,bus=pci.0,addr=0x6
> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-net-vhostuser-multiq.xml b/tests/qemuxml2argvdata/qemuxml2argv-net-vhostuser-multiq.xml
> index d5c42fe..2fadb1c 100644
> --- a/tests/qemuxml2argvdata/qemuxml2argv-net-vhostuser-multiq.xml
> +++ b/tests/qemuxml2argvdata/qemuxml2argv-net-vhostuser-multiq.xml
> @@ -30,7 +30,9 @@
>      </interface>
>      <interface type='vhostuser'>
>        <mac address='52:54:00:ee:96:6c'/>
> -      <source type='unix' path='/tmp/vhost1.sock' mode='client'/>
> +      <source type='unix' path='/tmp/vhost1.sock' mode='client'>
> +        <reconnect enabled='yes' timeout='10'/>
> +      </source>
>        <model type='virtio'/>
>      </interface>
>      <interface type='server'>
> @@ -40,7 +42,9 @@
>      </interface>
>      <interface type='vhostuser'>
>        <mac address='52:54:00:ee:96:6d'/>
> -      <source type='unix' path='/tmp/vhost2.sock' mode='client'/>
> +      <source type='unix' path='/tmp/vhost2.sock' mode='client'>
> +        <reconnect enabled='no'/>
> +      </source>
>        <model type='virtio'/>
>        <driver queues='4'/>
>      </interface>



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