> Re: [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
