* [PATCH 01/13 V6] remus: add libnl3 dependency to autoconf scripts
2014-01-21 9:05 [PATCH 00/13 V6] Remus/Libxl: Network buffering support Lai Jiangshan
@ 2014-01-21 9:05 ` Lai Jiangshan
2014-01-21 9:05 ` [PATCH 02/13 V6] remus: implement network buffering hotplug scripts Lai Jiangshan
` (12 subsequent siblings)
13 siblings, 0 replies; 26+ messages in thread
From: Lai Jiangshan @ 2014-01-21 9:05 UTC (permalink / raw)
To: xen-devel
Cc: Ian Campbell, FNST-Wen Congyang, Stefano Stabellini,
Andrew Cooper, Jiang Yunhong, Ian Jackson, Dong Eddie,
Shriram Rajagopalan, Roger Pau Monne
From: Shriram Rajagopalan <rshriram@cs.ubc.ca>
Libnl3 is required for controlling Remus network buffering.
This patch adds dependency on libnl3 (>= 3.2.8) to autoconf scripts.
Also provide ability to configure tools without libnl3 support, that
is without network buffering support.
Signed-off-by: Shriram Rajagopalan <rshriram@cs.ubc.ca>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
---
README | 4 ++++
config/Tools.mk.in | 3 +++
tools/configure.ac | 15 +++++++++++++++
tools/libxl/Makefile | 2 ++
tools/remus/README | 6 ++++++
5 files changed, 30 insertions(+)
diff --git a/README b/README
index 4148a26..7bb25fb 100644
--- a/README
+++ b/README
@@ -72,6 +72,10 @@ disabled at compile time:
* cmake (if building vtpm stub domains)
* markdown
* figlet (for generating the traditional Xen start of day banner)
+ * Development install of libnl3 (e.g., libnl-3-200,
+ libnl-3-dev, etc). Required if network buffering is desired
+ when using Remus with libxl. See tools/remus/README for detailed
+ information.
Second, you need to acquire a suitable kernel for use in domain 0. If
possible you should use a kernel provided by your OS distributor. If
diff --git a/config/Tools.mk.in b/config/Tools.mk.in
index d9d3239..81802b3 100644
--- a/config/Tools.mk.in
+++ b/config/Tools.mk.in
@@ -38,6 +38,8 @@ PTHREAD_LIBS := @PTHREAD_LIBS@
PTYFUNCS_LIBS := @PTYFUNCS_LIBS@
+LIBNL3_LIBS := @LIBNL3_LIBS@
+LIBNL3_CFLAGS := @LIBNL3_CFLAGS@
# Download GIT repositories via HTTP or GIT's own protocol?
# GIT's protocol is faster and more robust, when it works at all (firewalls
# may block it). We make it the default, but if your GIT repository downloads
@@ -56,6 +58,7 @@ CONFIG_QEMU_TRAD := @qemu_traditional@
CONFIG_QEMU_XEN := @qemu_xen@
CONFIG_XEND := @xend@
CONFIG_BLKTAP1 := @blktap1@
+CONFIG_REMUS_NETBUF := @remus_netbuf@
#System options
ZLIB := @zlib@
diff --git a/tools/configure.ac b/tools/configure.ac
index 0754f0e..f95956d 100644
--- a/tools/configure.ac
+++ b/tools/configure.ac
@@ -236,6 +236,21 @@ esac
# Checks for header files.
AC_CHECK_HEADERS([yajl/yajl_version.h sys/eventfd.h])
+# Check for libnl3 >=3.2.8. If present enable remus network buffering.
+PKG_CHECK_MODULES(LIBNL3, [libnl-3.0 >= 3.2.8 libnl-route-3.0 >= 3.2.8],
+ [libnl3_lib="y"], [libnl3_lib="n"])
+
+AS_IF([test "x$libnl3_lib" = "xn" ], [
+ AC_MSG_WARN([Disabling support for Remus network buffering.
+ Please install libnl3 libraries, command line tools and devel
+ headers - version 3.2.8 or higher])
+ AC_SUBST(remus_netbuf, [n])
+ ],[
+ AC_SUBST(LIBNL3_LIBS)
+ AC_SUBST(LIBNL3_CFLAGS)
+ AC_SUBST(remus_netbuf, [y])
+])
+
AC_OUTPUT()
AS_IF([test "x$xend" = "xy" ], [
diff --git a/tools/libxl/Makefile b/tools/libxl/Makefile
index d8495bb..da27c84 100644
--- a/tools/libxl/Makefile
+++ b/tools/libxl/Makefile
@@ -21,11 +21,13 @@ endif
LIBXL_LIBS =
LIBXL_LIBS = $(LDLIBS_libxenctrl) $(LDLIBS_libxenguest) $(LDLIBS_libxenstore) $(LDLIBS_libblktapctl) $(PTYFUNCS_LIBS) $(LIBUUID_LIBS)
+LIBXL_LIBS += $(LIBNL3_LIBS)
CFLAGS_LIBXL += $(CFLAGS_libxenctrl)
CFLAGS_LIBXL += $(CFLAGS_libxenguest)
CFLAGS_LIBXL += $(CFLAGS_libxenstore)
CFLAGS_LIBXL += $(CFLAGS_libblktapctl)
+CFLAGS_LIBXL += $(LIBNL3_CFLAGS)
CFLAGS_LIBXL += -Wshadow
LIBXL_LIBS-$(CONFIG_ARM) += -lfdt
diff --git a/tools/remus/README b/tools/remus/README
index 9e8140b..4736252 100644
--- a/tools/remus/README
+++ b/tools/remus/README
@@ -2,3 +2,9 @@ Remus provides fault tolerance for virtual machines by sending continuous
checkpoints to a backup, which will activate if the target VM fails.
See the website at http://nss.cs.ubc.ca/remus/ for details.
+
+Using Remus with libxl on Xen 4.4 and higher:
+ To enable network buffering, you need libnl 3.2.8
+ or higher along with the development headers and command line utilities.
+ If your distro does not have the appropriate libnl3 version, you can find
+ the latest source tarball of libnl3 at http://www.carisma.slowglass.com/~tgr/libnl/
--
1.8.4.2
^ permalink raw reply related [flat|nested] 26+ messages in thread* [PATCH 02/13 V6] remus: implement network buffering hotplug scripts
2014-01-21 9:05 [PATCH 00/13 V6] Remus/Libxl: Network buffering support Lai Jiangshan
2014-01-21 9:05 ` [PATCH 01/13 V6] remus: add libnl3 dependency to autoconf scripts Lai Jiangshan
@ 2014-01-21 9:05 ` Lai Jiangshan
2014-01-26 22:27 ` Shriram Rajagopalan
2014-01-21 9:05 ` [PATCH 03/13 V6] tools/libxl: update libxl_domain_remus_info Lai Jiangshan
` (11 subsequent siblings)
13 siblings, 1 reply; 26+ messages in thread
From: Lai Jiangshan @ 2014-01-21 9:05 UTC (permalink / raw)
To: xen-devel
Cc: Ian Campbell, FNST-Wen Congyang, Stefano Stabellini,
Andrew Cooper, Jiang Yunhong, Ian Jackson, Lai Jiangshan,
Dong Eddie, Shriram Rajagopalan, Roger Pau Monne
From: Shriram Rajagopalan <rshriram@cs.ubc.ca>
This patch introduces remus-netbuf-setup hotplug script responsible for
setting up and tearing down the necessary infrastructure required for
network output buffering in Remus. This script is intended to be invoked
by libxl for each guest interface, when starting or stopping Remus.
Apart from returning success/failure indication via the usual hotplug
entries in xenstore, this script also writes to xenstore, the name of
the IFB device to be used to control the vif's network output.
The script relies on libnl3 command line utilities to perform various
setup/teardown functions. The script is confined to Linux platforms only
since NetBSD does not seem to have libnl3.
Signed-off-by: Shriram Rajagopalan <rshriram@cs.ubc.ca>
Signed-off-by: Lai Jiangshan <laijs@cn.fujitsu.com>
Reviewed-by: Wen Congyang <wency@cn.fujitsu.com>
---
tools/hotplug/Linux/Makefile | 1 +
tools/hotplug/Linux/remus-netbuf-setup | 183 +++++++++++++++++++++++++++++++++
2 files changed, 184 insertions(+)
create mode 100644 tools/hotplug/Linux/remus-netbuf-setup
diff --git a/tools/hotplug/Linux/Makefile b/tools/hotplug/Linux/Makefile
index 47655f6..6139c1f 100644
--- a/tools/hotplug/Linux/Makefile
+++ b/tools/hotplug/Linux/Makefile
@@ -16,6 +16,7 @@ XEN_SCRIPTS += network-nat vif-nat
XEN_SCRIPTS += vif-openvswitch
XEN_SCRIPTS += vif2
XEN_SCRIPTS += vif-setup
+XEN_SCRIPTS-$(CONFIG_REMUS_NETBUF) += remus-netbuf-setup
XEN_SCRIPTS += block
XEN_SCRIPTS += block-enbd block-nbd
XEN_SCRIPTS-$(CONFIG_BLKTAP1) += blktap
diff --git a/tools/hotplug/Linux/remus-netbuf-setup b/tools/hotplug/Linux/remus-netbuf-setup
new file mode 100644
index 0000000..3467db2
--- /dev/null
+++ b/tools/hotplug/Linux/remus-netbuf-setup
@@ -0,0 +1,183 @@
+#!/bin/bash
+#============================================================================
+# ${XEN_SCRIPT_DIR}/remus-netbuf-setup
+#
+# Script for attaching a network buffer to the specified vif (in any mode).
+# The hotplugging system will call this script when starting remus via libxl
+# API, libxl_domain_remus_start.
+#
+# Usage:
+# remus-netbuf-setup (setup|teardown)
+#
+# Environment vars:
+# vifname vif interface name (required).
+# XENBUS_PATH path in Xenstore, where the IFB device details will be stored
+# or read from (required).
+# (libxl passes /libxl/<domid>/remus/netbuf/<devid>)
+# IFB ifb interface to be cleaned up (required). [for teardown op only]
+
+# Written to the store: (setup operation)
+# XENBUS_PATH/ifb=<ifbdevName> the IFB device serving
+# as the intermediate buffer through which the interface's network output
+# can be controlled.
+#
+# To install a network buffer on a guest vif (vif1.0) using ifb (ifb0)
+# we need to do the following
+#
+# ip link set dev ifb0 up
+# tc qdisc add dev vif1.0 ingress
+# tc filter add dev vif1.0 parent ffff: proto ip \
+# prio 10 u32 match u32 0 0 action mirred egress redirect dev ifb0
+# nl-qdisc-add --dev=ifb0 --parent root plug
+# nl-qdisc-add --dev=ifb0 --parent root --update plug --limit=10000000
+# (10MB limit on buffer)
+#
+# So order of operations when installing a network buffer on vif1.0
+# 1. find a free ifb and bring up the device
+# 2. redirect traffic from vif1.0 to ifb:
+# 2.1 add ingress qdisc to vif1.0 (to capture outgoing packets from guest)
+# 2.2 use tc filter command with actions mirred egress + redirect
+# 3. install plug_qdisc on ifb device, with which we can buffer/release
+# guest's network output from vif1.0
+#
+#
+
+#============================================================================
+
+# Unlike other vif scripts, vif-common is not needed here as it executes vif
+#specific setup code such as renaming.
+dir=$(dirname "$0")
+. "$dir/xen-hotplug-common.sh"
+
+findCommand "$@"
+
+if [ "$command" != "setup" -a "$command" != "teardown" ]
+then
+ echo "Invalid command: $command"
+ log err "Invalid command: $command"
+ exit 1
+fi
+
+evalVariables "$@"
+
+: ${vifname:?}
+: ${XENBUS_PATH:?}
+
+check_libnl_tools() {
+ if ! command -v nl-qdisc-list > /dev/null 2>&1; then
+ fatal "Unable to find nl-qdisc-list tool"
+ fi
+ if ! command -v nl-qdisc-add > /dev/null 2>&1; then
+ fatal "Unable to find nl-qdisc-add tool"
+ fi
+ if ! command -v nl-qdisc-delete > /dev/null 2>&1; then
+ fatal "Unable to find nl-qdisc-delete tool"
+ fi
+}
+
+# We only check for modules. We don't load them.
+# User/Admin is supposed to load ifb during boot time,
+# ensuring that there are enough free ifbs in the system.
+# Other modules will be loaded automatically by tc commands.
+check_modules() {
+ for m in ifb sch_plug sch_ingress act_mirred cls_u32
+ do
+ if ! modinfo $m > /dev/null 2>&1; then
+ fatal "Unable to find $m kernel module"
+ fi
+ done
+}
+
+setup_ifb() {
+
+ for ifb in `ifconfig -a -s|egrep ^ifb|cut -d ' ' -f1`
+ do
+ local installed=`nl-qdisc-list -d $ifb`
+ [ -n "$installed" ] && continue
+ IFB="$ifb"
+ break
+ done
+
+ if [ -z "$IFB" ]
+ then
+ fatal "Unable to find a free IFB device for $vifname"
+ fi
+
+ do_or_die ip link set dev "$IFB" up
+}
+
+redirect_vif_traffic() {
+ local vif=$1
+ local ifb=$2
+
+ do_or_die tc qdisc add dev "$vif" ingress
+
+ tc filter add dev "$vif" parent ffff: proto ip prio 10 \
+ u32 match u32 0 0 action mirred egress redirect dev "$ifb" >/dev/null 2>&1
+
+ if [ $? -ne 0 ]
+ then
+ do_without_error tc qdisc del dev "$vif" ingress
+ fatal "Failed to redirect traffic from $vif to $ifb"
+ fi
+}
+
+add_plug_qdisc() {
+ local vif=$1
+ local ifb=$2
+
+ nl-qdisc-add --dev="$ifb" --parent root plug >/dev/null 2>&1
+ if [ $? -ne 0 ]
+ then
+ do_without_error tc qdisc del dev "$vif" ingress
+ fatal "Failed to add plug qdisc to $ifb"
+ fi
+
+ #set ifb buffering limit in bytes. Its okay if this command fails
+ nl-qdisc-add --dev="$ifb" --parent root \
+ --update plug --limit=10000000 >/dev/null 2>&1
+}
+
+teardown_netbuf() {
+ local vif=$1
+ local ifb=$2
+
+ if [ "$ifb" ]; then
+ do_without_error ip link set dev "$ifb" down
+ do_without_error nl-qdisc-delete --dev="$ifb" --parent root plug >/dev/null 2>&1
+ xenstore-rm -t "$XENBUS_PATH/ifb" 2>/dev/null || true
+ fi
+ do_without_error tc qdisc del dev "$vif" ingress
+ xenstore-rm -t "$XENBUS_PATH/hotplug-status" 2>/dev/null || true
+}
+
+xs_write_failed() {
+ local vif=$1
+ local ifb=$2
+ teardown_netbuf "$vifname" "$IFB"
+ fatal "failed to write ifb name to xenstore"
+}
+
+case "$command" in
+ setup)
+ check_libnl_tools
+ check_modules
+
+ claim_lock "pickifb"
+ setup_ifb
+ redirect_vif_traffic "$vifname" "$IFB"
+ add_plug_qdisc "$vifname" "$IFB"
+ release_lock "pickifb"
+
+ #not using xenstore_write that automatically exits on error
+ #because we need to cleanup
+ _xenstore_write "$XENBUS_PATH/ifb" "$IFB" || xs_write_failed "$vifname" "$IFB"
+ success
+ ;;
+ teardown)
+ : ${IFB:?}
+ teardown_netbuf "$vifname" "$IFB"
+ ;;
+esac
+
+log debug "Successful remus-netbuf-setup $command for $vifname, ifb $IFB."
--
1.8.4.2
^ permalink raw reply related [flat|nested] 26+ messages in thread* Re: [PATCH 02/13 V6] remus: implement network buffering hotplug scripts
2014-01-21 9:05 ` [PATCH 02/13 V6] remus: implement network buffering hotplug scripts Lai Jiangshan
@ 2014-01-26 22:27 ` Shriram Rajagopalan
2014-01-27 7:06 ` Wen Congyang
0 siblings, 1 reply; 26+ messages in thread
From: Shriram Rajagopalan @ 2014-01-26 22:27 UTC (permalink / raw)
To: Lai Jiangshan
Cc: Ian Campbell, FNST-Wen Congyang, Stefano Stabellini,
Andrew Cooper, Jiang Yunhong, Ian Jackson,
xen-devel@lists.xen.org, Dong Eddie, Roger Pau Monne
[-- Attachment #1.1: Type: text/plain, Size: 1400 bytes --]
On Tue, Jan 21, 2014 at 1:05 AM, Lai Jiangshan <laijs@cn.fujitsu.com> wrote:
> From: Shriram Rajagopalan <rshriram@cs.ubc.ca>
>
> This patch introduces remus-netbuf-setup hotplug script responsible for
> setting up and tearing down the necessary infrastructure required for
> network output buffering in Remus. This script is intended to be invoked
> by libxl for each guest interface, when starting or stopping Remus.
>
> Apart from returning success/failure indication via the usual hotplug
> entries in xenstore, this script also writes to xenstore, the name of
> the IFB device to be used to control the vif's network output.
>
> The script relies on libnl3 command line utilities to perform various
> setup/teardown functions. The script is confined to Linux platforms only
> since NetBSD does not seem to have libnl3.
>
> Signed-off-by: Shriram Rajagopalan <rshriram@cs.ubc.ca>
> Signed-off-by: Lai Jiangshan <laijs@cn.fujitsu.com>
> Reviewed-by: Wen Congyang <wency@cn.fujitsu.com>
> ---
> tools/hotplug/Linux/Makefile | 1 +
> tools/hotplug/Linux/remus-netbuf-setup | 183
> +++++++++++++++++++++++++++++++++
> 2 files changed, 184 insertions(+)
> create mode 100644 tools/hotplug/Linux/remus-netbuf-setup
>
>
The last time I posted this script, the feedback was that the script and
the code invoking
the script should be in a single patch. So I would suggest doing the same.
[-- Attachment #1.2: Type: text/html, Size: 2063 bytes --]
[-- Attachment #2: Type: text/plain, Size: 126 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 02/13 V6] remus: implement network buffering hotplug scripts
2014-01-26 22:27 ` Shriram Rajagopalan
@ 2014-01-27 7:06 ` Wen Congyang
2014-01-27 18:05 ` Shriram Rajagopalan
0 siblings, 1 reply; 26+ messages in thread
From: Wen Congyang @ 2014-01-27 7:06 UTC (permalink / raw)
To: rshriram, Lai Jiangshan
Cc: Ian Campbell, Stefano Stabellini, Andrew Cooper, Jiang Yunhong,
Ian Jackson, xen-devel@lists.xen.org, Dong Eddie, Roger Pau Monne
At 01/27/2014 06:27 AM, Shriram Rajagopalan Wrote:
> On Tue, Jan 21, 2014 at 1:05 AM, Lai Jiangshan <laijs@cn.fujitsu.com> wrote:
>
>> From: Shriram Rajagopalan <rshriram@cs.ubc.ca>
>>
>> This patch introduces remus-netbuf-setup hotplug script responsible for
>> setting up and tearing down the necessary infrastructure required for
>> network output buffering in Remus. This script is intended to be invoked
>> by libxl for each guest interface, when starting or stopping Remus.
>>
>> Apart from returning success/failure indication via the usual hotplug
>> entries in xenstore, this script also writes to xenstore, the name of
>> the IFB device to be used to control the vif's network output.
>>
>> The script relies on libnl3 command line utilities to perform various
>> setup/teardown functions. The script is confined to Linux platforms only
>> since NetBSD does not seem to have libnl3.
>>
>> Signed-off-by: Shriram Rajagopalan <rshriram@cs.ubc.ca>
>> Signed-off-by: Lai Jiangshan <laijs@cn.fujitsu.com>
>> Reviewed-by: Wen Congyang <wency@cn.fujitsu.com>
>> ---
>> tools/hotplug/Linux/Makefile | 1 +
>> tools/hotplug/Linux/remus-netbuf-setup | 183
>> +++++++++++++++++++++++++++++++++
>> 2 files changed, 184 insertions(+)
>> create mode 100644 tools/hotplug/Linux/remus-netbuf-setup
>>
>>
> The last time I posted this script, the feedback was that the script and
> the code invoking
> the script should be in a single patch. So I would suggest doing the same.
We use the script in patch6. It adds 479 lines. These two patches are big
patches(add more than 100 lines), so why put them into a single patch?
Thanks
Wen Congyang
>
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel
>
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 02/13 V6] remus: implement network buffering hotplug scripts
2014-01-27 7:06 ` Wen Congyang
@ 2014-01-27 18:05 ` Shriram Rajagopalan
2014-01-28 9:29 ` Ian Campbell
0 siblings, 1 reply; 26+ messages in thread
From: Shriram Rajagopalan @ 2014-01-27 18:05 UTC (permalink / raw)
To: Ian Jackson
Cc: Lai Jiangshan, Wen Congyang, Stefano Stabellini, Andrew Cooper,
Jiang Yunhong, Dong Eddie, xen-devel@lists.xen.org,
Roger Pau Monne, Ian Campbell
[-- Attachment #1.1: Type: text/plain, Size: 1903 bytes --]
On Sun, Jan 26, 2014 at 11:06 PM, Wen Congyang <wency@cn.fujitsu.com> wrote:
> At 01/27/2014 06:27 AM, Shriram Rajagopalan Wrote:
> > On Tue, Jan 21, 2014 at 1:05 AM, Lai Jiangshan <laijs@cn.fujitsu.com>
> wrote:
> >
> >> From: Shriram Rajagopalan <rshriram@cs.ubc.ca>
> >>
> >> This patch introduces remus-netbuf-setup hotplug script responsible for
> >> setting up and tearing down the necessary infrastructure required for
> >> network output buffering in Remus. This script is intended to be
> invoked
> >> by libxl for each guest interface, when starting or stopping Remus.
> >>
> >> Apart from returning success/failure indication via the usual hotplug
> >> entries in xenstore, this script also writes to xenstore, the name of
> >> the IFB device to be used to control the vif's network output.
> >>
> >> The script relies on libnl3 command line utilities to perform various
> >> setup/teardown functions. The script is confined to Linux platforms only
> >> since NetBSD does not seem to have libnl3.
> >>
> >> Signed-off-by: Shriram Rajagopalan <rshriram@cs.ubc.ca>
> >> Signed-off-by: Lai Jiangshan <laijs@cn.fujitsu.com>
> >> Reviewed-by: Wen Congyang <wency@cn.fujitsu.com>
> >> ---
> >> tools/hotplug/Linux/Makefile | 1 +
> >> tools/hotplug/Linux/remus-netbuf-setup | 183
> >> +++++++++++++++++++++++++++++++++
> >> 2 files changed, 184 insertions(+)
> >> create mode 100644 tools/hotplug/Linux/remus-netbuf-setup
> >>
> >>
> > The last time I posted this script, the feedback was that the script and
> > the code invoking
> > the script should be in a single patch. So I would suggest doing the
> same.
>
> We use the script in patch6. It adds 479 lines. These two patches are big
> patches(add more than 100 lines), so why put them into a single patch?
>
>
That is a valid question. IIRC, IanJ was the one who wanted the code and the
script together. IanJ, any thoughts?
[-- Attachment #1.2: Type: text/html, Size: 2790 bytes --]
[-- Attachment #2: Type: text/plain, Size: 126 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 02/13 V6] remus: implement network buffering hotplug scripts
2014-01-27 18:05 ` Shriram Rajagopalan
@ 2014-01-28 9:29 ` Ian Campbell
2014-02-10 2:29 ` Lai Jiangshan
0 siblings, 1 reply; 26+ messages in thread
From: Ian Campbell @ 2014-01-28 9:29 UTC (permalink / raw)
To: rshriram
Cc: Lai Jiangshan, Wen Congyang, Stefano Stabellini, Andrew Cooper,
Jiang Yunhong, Ian Jackson, xen-devel@lists.xen.org, Dong Eddie,
Roger Pau Monne
On Mon, 2014-01-27 at 10:05 -0800, Shriram Rajagopalan wrote:
> On Sun, Jan 26, 2014 at 11:06 PM, Wen Congyang <wency@cn.fujitsu.com>
> wrote:
> > The last time I posted this script, the feedback was that
> the script and> the code invoking > the script should be in a
> single patch. So I would suggest doing the same.
>
>
> We use the script in patch6. It adds 479 lines. These two
> patches are big patches(add more than 100 lines), so why put
> them into a single patch?
>
> That is a valid question. IIRC, IanJ was the one who wanted the code
> and the script together. IanJ, any thoughts?
Unless the patches are so big they won't get past the mailing list
filters (which are 100s of Kb I think) the important thing is the
logical separation of functionality into separate patches, not the
individual line count of each patch.
Ian.
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 02/13 V6] remus: implement network buffering hotplug scripts
2014-01-28 9:29 ` Ian Campbell
@ 2014-02-10 2:29 ` Lai Jiangshan
0 siblings, 0 replies; 26+ messages in thread
From: Lai Jiangshan @ 2014-02-10 2:29 UTC (permalink / raw)
To: Ian Campbell
Cc: Dong Eddie, Wen Congyang, Stefano Stabellini, Andrew Cooper,
Jiang Yunhong, Ian Jackson, xen-devel@lists.xen.org, rshriram,
Roger Pau Monne
On 01/28/2014 05:29 PM, Ian Campbell wrote:
> On Mon, 2014-01-27 at 10:05 -0800, Shriram Rajagopalan wrote:
>> On Sun, Jan 26, 2014 at 11:06 PM, Wen Congyang <wency@cn.fujitsu.com>
>> wrote:
>> > The last time I posted this script, the feedback was that
>> the script and> the code invoking > the script should be in a
>> single patch. So I would suggest doing the same.
>>
>>
>> We use the script in patch6. It adds 479 lines. These two
>> patches are big patches(add more than 100 lines), so why put
>> them into a single patch?
>
>>
>> That is a valid question. IIRC, IanJ was the one who wanted the code
>> and the script together. IanJ, any thoughts?
>
> Unless the patches are so big they won't get past the mailing list
> filters (which are 100s of Kb I think) the important thing is the
> logical separation of functionality into separate patches, not the
> individual line count of each patch.
>
We did the logically separate, patches are split by functionalities, ^_^.
They are a little more fine-grain-separation than previous versions.
fine-grain-separation patches are much convenient to be reviewed in the
mail-list and in the future changlog.
If any one insist the original way, we will change it back.
thx,
Lai
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH 03/13 V6] tools/libxl: update libxl_domain_remus_info
2014-01-21 9:05 [PATCH 00/13 V6] Remus/Libxl: Network buffering support Lai Jiangshan
2014-01-21 9:05 ` [PATCH 01/13 V6] remus: add libnl3 dependency to autoconf scripts Lai Jiangshan
2014-01-21 9:05 ` [PATCH 02/13 V6] remus: implement network buffering hotplug scripts Lai Jiangshan
@ 2014-01-21 9:05 ` Lai Jiangshan
2014-01-21 9:05 ` [PATCH 04/13 V6] tools/libxl: introduce a new structure libxl__remus_state Lai Jiangshan
` (10 subsequent siblings)
13 siblings, 0 replies; 26+ messages in thread
From: Lai Jiangshan @ 2014-01-21 9:05 UTC (permalink / raw)
To: xen-devel
Cc: Ian Campbell, FNST-Wen Congyang, Stefano Stabellini,
Andrew Cooper, Jiang Yunhong, Ian Jackson, Lai Jiangshan,
Dong Eddie, Shriram Rajagopalan, Roger Pau Monne
From: Shriram Rajagopalan <rshriram@cs.ubc.ca>
Add two members:
1. netbuf: whether netbuf is enabled
2. netbufscript: the path of the script which will be run to setup
and tear down the guest's interface.
Signed-off-by: Shriram Rajagopalan <rshriram@cs.ubc.ca>
Signed-off-by: Lai Jiangshan <laijs@cn.fujitsu.com>
Reviewed-by: Wen Congyang <wency@cn.fujitsu.com>
---
tools/libxl/libxl.h | 13 +++++++++++++
tools/libxl/libxl_types.idl | 2 ++
2 files changed, 15 insertions(+)
diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
index 12d6c31..d89ad0a 100644
--- a/tools/libxl/libxl.h
+++ b/tools/libxl/libxl.h
@@ -409,6 +409,19 @@
*/
#define LIBXL_HAVE_DRIVER_DOMAIN_CREATION 1
+/*
+ * LIBXL_HAVE_REMUS_NETBUF 1
+ *
+ * If this is defined, then the libxl_domain_remus_info structure will
+ * have a boolean field (netbuf) and a string field (netbufscript).
+ *
+ * netbuf, if true, indicates that network buffering should be enabled.
+ *
+ * netbufscript, if set, indicates the path to the hotplug script to
+ * setup or teardown network buffers.
+ */
+#define LIBXL_HAVE_REMUS_NETBUF 1
+
/* Functions annotated with LIBXL_EXTERNAL_CALLERS_ONLY may not be
* called from within libxl itself. Callers outside libxl, who
* do not #include libxl_internal.h, are fine. */
diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
index 649ce50..e49945a 100644
--- a/tools/libxl/libxl_types.idl
+++ b/tools/libxl/libxl_types.idl
@@ -561,6 +561,8 @@ libxl_domain_remus_info = Struct("domain_remus_info",[
("interval", integer),
("blackhole", bool),
("compression", bool),
+ ("netbuf", bool),
+ ("netbufscript", string),
])
libxl_event_type = Enumeration("event_type", [
--
1.8.4.2
^ permalink raw reply related [flat|nested] 26+ messages in thread* [PATCH 04/13 V6] tools/libxl: introduce a new structure libxl__remus_state
2014-01-21 9:05 [PATCH 00/13 V6] Remus/Libxl: Network buffering support Lai Jiangshan
` (2 preceding siblings ...)
2014-01-21 9:05 ` [PATCH 03/13 V6] tools/libxl: update libxl_domain_remus_info Lai Jiangshan
@ 2014-01-21 9:05 ` Lai Jiangshan
2014-01-21 9:05 ` [PATCH 05/13 V6] remus: introduce a function to check whether network buffering is enabled Lai Jiangshan
` (9 subsequent siblings)
13 siblings, 0 replies; 26+ messages in thread
From: Lai Jiangshan @ 2014-01-21 9:05 UTC (permalink / raw)
To: xen-devel
Cc: Ian Campbell, FNST-Wen Congyang, Stefano Stabellini,
Andrew Cooper, Jiang Yunhong, Ian Jackson, Lai Jiangshan,
Dong Eddie, Shriram Rajagopalan, Roger Pau Monne
From: Shriram Rajagopalan <rshriram@cs.ubc.ca>
libxl_domain_remus_info only contains the argument of the command
'xl remus'. So introduce a new structure libxl__remus_state to save
the remus state.
Signed-off-by: Shriram Rajagopalan <rshriram@cs.ubc.ca>
Signed-off-by: Lai Jiangshan <laijs@cn.fujitsu.com>
Reviewed-by: Wen Congyang <wency@cn.fujitsu.com>
---
tools/libxl/libxl.c | 25 +++++++++++++++++++++++--
tools/libxl/libxl_dom.c | 12 ++++--------
tools/libxl/libxl_internal.h | 22 ++++++++++++++++++++--
3 files changed, 47 insertions(+), 12 deletions(-)
diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index 2845ca4..25af816 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -729,11 +729,32 @@ int libxl_domain_remus_start(libxl_ctx *ctx, libxl_domain_remus_info *info,
dss->type = type;
dss->live = 1;
dss->debug = 0;
- dss->remus = info;
assert(info);
- /* TBD: Remus setup - i.e. attach qdisc, enable disk buffering, etc */
+ GCNEW(dss->remus_state);
+
+ /* convenience shorthand */
+ libxl__remus_state *remus_state = dss->remus_state;
+ remus_state->blackhole = info->blackhole;
+ remus_state->interval = info->interval;
+ remus_state->compression = info->compression;
+ remus_state->dss = dss;
+ libxl__ev_child_init(&remus_state->child);
+
+ /* TODO: enable disk buffering */
+
+ /* Setup network buffering */
+ if (info->netbuf) {
+ if (info->netbufscript) {
+ remus_state->netbufscript =
+ libxl__strdup(gc, info->netbufscript);
+ } else {
+ remus_state->netbufscript =
+ GCSPRINTF("%s/remus-netbuf-setup",
+ libxl__xen_script_dir_path());
+ }
+ }
/* Point of no return */
libxl__domain_suspend(egc, dss);
diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c
index 55f74b2..8d63f90 100644
--- a/tools/libxl/libxl_dom.c
+++ b/tools/libxl/libxl_dom.c
@@ -1290,7 +1290,7 @@ static void remus_checkpoint_dm_saved(libxl__egc *egc,
/* REMUS TODO: Wait for disk and memory ack, release network buffer */
/* REMUS TODO: make this asynchronous */
assert(!rc); /* REMUS TODO handle this error properly */
- usleep(dss->interval * 1000);
+ usleep(dss->remus_state->interval * 1000);
libxl__xc_domain_saverestore_async_callback_done(egc, &dss->shs, 1);
}
@@ -1308,7 +1308,6 @@ void libxl__domain_suspend(libxl__egc *egc, libxl__domain_suspend_state *dss)
const libxl_domain_type type = dss->type;
const int live = dss->live;
const int debug = dss->debug;
- const libxl_domain_remus_info *const r_info = dss->remus;
libxl__srm_save_autogen_callbacks *const callbacks =
&dss->shs.callbacks.save.a;
@@ -1343,11 +1342,8 @@ void libxl__domain_suspend(libxl__egc *egc, libxl__domain_suspend_state *dss)
dss->guest_responded = 0;
dss->dm_savefile = libxl__device_model_savefile(gc, domid);
- if (r_info != NULL) {
- dss->interval = r_info->interval;
- if (r_info->compression)
- dss->xcflags |= XCFLAGS_CHECKPOINT_COMPRESS;
- }
+ if (dss->remus_state && dss->remus_state->compression)
+ dss->xcflags |= XCFLAGS_CHECKPOINT_COMPRESS;
dss->xce = xc_evtchn_open(NULL, 0);
if (dss->xce == NULL)
@@ -1366,7 +1362,7 @@ void libxl__domain_suspend(libxl__egc *egc, libxl__domain_suspend_state *dss)
}
memset(callbacks, 0, sizeof(*callbacks));
- if (r_info != NULL) {
+ if (dss->remus_state != NULL) {
callbacks->suspend = libxl__remus_domain_suspend_callback;
callbacks->postcopy = libxl__remus_domain_resume_callback;
callbacks->checkpoint = libxl__remus_domain_checkpoint_callback;
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 1bd23ff..9970780 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -2292,6 +2292,25 @@ typedef struct libxl__logdirty_switch {
libxl__ev_time timeout;
} libxl__logdirty_switch;
+typedef struct libxl__remus_state {
+ /* filled by the user */
+ /* checkpoint interval */
+ int interval;
+ int blackhole;
+ int compression;
+ /* Script to setup/teardown network buffers */
+ const char *netbufscript;
+ libxl__domain_suspend_state *dss;
+
+ /* private */
+ int saved_rc;
+ int dev_id;
+ /* Opaque context containing network buffer related stuff */
+ void *netbuf_state;
+ libxl__ev_time timeout;
+ libxl__ev_child child;
+} libxl__remus_state;
+
struct libxl__domain_suspend_state {
/* set by caller of libxl__domain_suspend */
libxl__ao *ao;
@@ -2302,7 +2321,7 @@ struct libxl__domain_suspend_state {
libxl_domain_type type;
int live;
int debug;
- const libxl_domain_remus_info *remus;
+ libxl__remus_state *remus_state;
/* private */
xc_evtchn *xce; /* event channel handle */
int suspend_eventchn;
@@ -2310,7 +2329,6 @@ struct libxl__domain_suspend_state {
int xcflags;
int guest_responded;
const char *dm_savefile;
- int interval; /* checkpoint interval (for Remus) */
libxl__save_helper_state shs;
libxl__logdirty_switch logdirty;
/* private for libxl__domain_save_device_model */
--
1.8.4.2
^ permalink raw reply related [flat|nested] 26+ messages in thread* [PATCH 05/13 V6] remus: introduce a function to check whether network buffering is enabled
2014-01-21 9:05 [PATCH 00/13 V6] Remus/Libxl: Network buffering support Lai Jiangshan
` (3 preceding siblings ...)
2014-01-21 9:05 ` [PATCH 04/13 V6] tools/libxl: introduce a new structure libxl__remus_state Lai Jiangshan
@ 2014-01-21 9:05 ` Lai Jiangshan
2014-01-21 9:05 ` [PATCH 06/13 V6] remus: implement the API to setup network buffering Lai Jiangshan
` (8 subsequent siblings)
13 siblings, 0 replies; 26+ messages in thread
From: Lai Jiangshan @ 2014-01-21 9:05 UTC (permalink / raw)
To: xen-devel
Cc: Ian Campbell, FNST-Wen Congyang, Stefano Stabellini,
Andrew Cooper, Jiang Yunhong, Ian Jackson, Lai Jiangshan,
Dong Eddie, Shriram Rajagopalan, Roger Pau Monne
From: Shriram Rajagopalan <rshriram@cs.ubc.ca>
libxl__netbuffer_enabled() returns 1 when network buffering is compiled,
or returns 0 when network buffering is not compiled.
If network buffering is not compiled, and the user wants to use it, report
a error and exit.
Signed-off-by: Shriram Rajagopalan <rshriram@cs.ubc.ca>
Signed-off-by: Lai Jiangshan <laijs@cn.fujitsu.com>
Reviewed-by: Wen Congyang <wency@cn.fujitsu.com>
---
tools/libxl/Makefile | 7 +++++++
tools/libxl/libxl.c | 5 +++++
tools/libxl/libxl_internal.h | 2 ++
tools/libxl/libxl_netbuffer.c | 31 +++++++++++++++++++++++++++++++
tools/libxl/libxl_nonetbuffer.c | 31 +++++++++++++++++++++++++++++++
5 files changed, 76 insertions(+)
create mode 100644 tools/libxl/libxl_netbuffer.c
create mode 100644 tools/libxl/libxl_nonetbuffer.c
diff --git a/tools/libxl/Makefile b/tools/libxl/Makefile
index da27c84..84a467c 100644
--- a/tools/libxl/Makefile
+++ b/tools/libxl/Makefile
@@ -45,6 +45,13 @@ LIBXL_OBJS-y += libxl_blktap2.o
else
LIBXL_OBJS-y += libxl_noblktap2.o
endif
+
+ifeq ($(CONFIG_REMUS_NETBUF),y)
+LIBXL_OBJS-y += libxl_netbuffer.o
+else
+LIBXL_OBJS-y += libxl_nonetbuffer.o
+endif
+
LIBXL_OBJS-$(CONFIG_X86) += libxl_cpuid.o libxl_x86.o
LIBXL_OBJS-$(CONFIG_ARM) += libxl_nocpuid.o libxl_arm.o
diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index 25af816..026206a 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -746,6 +746,11 @@ int libxl_domain_remus_start(libxl_ctx *ctx, libxl_domain_remus_info *info,
/* Setup network buffering */
if (info->netbuf) {
+ if (!libxl__netbuffer_enabled(gc)) {
+ LOG(ERROR, "Remus: No support for network buffering");
+ goto out;
+ }
+
if (info->netbufscript) {
remus_state->netbufscript =
libxl__strdup(gc, info->netbufscript);
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 9970780..2f64382 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -2311,6 +2311,8 @@ typedef struct libxl__remus_state {
libxl__ev_child child;
} libxl__remus_state;
+_hidden int libxl__netbuffer_enabled(libxl__gc *gc);
+
struct libxl__domain_suspend_state {
/* set by caller of libxl__domain_suspend */
libxl__ao *ao;
diff --git a/tools/libxl/libxl_netbuffer.c b/tools/libxl/libxl_netbuffer.c
new file mode 100644
index 0000000..8e23d75
--- /dev/null
+++ b/tools/libxl/libxl_netbuffer.c
@@ -0,0 +1,31 @@
+/*
+ * Copyright (C) 2013
+ * Author Shriram Rajagopalan <rshriram@cs.ubc.ca>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU Lesser General Public License as published
+ * by the Free Software Foundation; version 2.1 only. with the special
+ * exception on linking described in file LICENSE.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU Lesser General Public License for more details.
+ */
+
+#include "libxl_osdeps.h" /* must come before any other headers */
+
+#include "libxl_internal.h"
+
+int libxl__netbuffer_enabled(libxl__gc *gc)
+{
+ return 1;
+}
+
+/*
+ * Local variables:
+ * mode: C
+ * c-basic-offset: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
diff --git a/tools/libxl/libxl_nonetbuffer.c b/tools/libxl/libxl_nonetbuffer.c
new file mode 100644
index 0000000..6aa4bf1
--- /dev/null
+++ b/tools/libxl/libxl_nonetbuffer.c
@@ -0,0 +1,31 @@
+/*
+ * Copyright (C) 2013
+ * Author Shriram Rajagopalan <rshriram@cs.ubc.ca>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU Lesser General Public License as published
+ * by the Free Software Foundation; version 2.1 only. with the special
+ * exception on linking described in file LICENSE.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU Lesser General Public License for more details.
+ */
+
+#include "libxl_osdeps.h" /* must come before any other headers */
+
+#include "libxl_internal.h"
+
+int libxl__netbuffer_enabled(libxl__gc *gc)
+{
+ return 0;
+}
+
+/*
+ * Local variables:
+ * mode: C
+ * c-basic-offset: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
--
1.8.4.2
^ permalink raw reply related [flat|nested] 26+ messages in thread* [PATCH 06/13 V6] remus: implement the API to setup network buffering
2014-01-21 9:05 [PATCH 00/13 V6] Remus/Libxl: Network buffering support Lai Jiangshan
` (4 preceding siblings ...)
2014-01-21 9:05 ` [PATCH 05/13 V6] remus: introduce a function to check whether network buffering is enabled Lai Jiangshan
@ 2014-01-21 9:05 ` Lai Jiangshan
2014-01-26 22:30 ` Shriram Rajagopalan
2014-01-21 9:05 ` [PATCH 07/13 V6] remus: implement the API to buffer/release packages Lai Jiangshan
` (7 subsequent siblings)
13 siblings, 1 reply; 26+ messages in thread
From: Lai Jiangshan @ 2014-01-21 9:05 UTC (permalink / raw)
To: xen-devel
Cc: Ian Campbell, FNST-Wen Congyang, Stefano Stabellini,
Andrew Cooper, Jiang Yunhong, Ian Jackson, Lai Jiangshan,
Dong Eddie, Shriram Rajagopalan, Roger Pau Monne
From: Shriram Rajagopalan <rshriram@cs.ubc.ca>
The following steps are taken during setup:
a) call the hotplug script for each vif to setup its network buffer
b) establish a dedicated remus context containing libnl related
state (netlink sockets, qdisc caches, etc.,)
c) Obtain handles to plug qdiscs installed on the IFB devices
chosen by the hotplug scripts.
Signed-off-by: Shriram Rajagopalan <rshriram@cs.ubc.ca>
Signed-off-by: Lai Jiangshan <laijs@cn.fujitsu.com>
Reviewed-by: Wen Congyang <wency@cn.fujitsu.com>
---
docs/misc/xenstore-paths.markdown | 4 +
tools/libxl/Makefile | 2 +
tools/libxl/libxl_dom.c | 7 +-
tools/libxl/libxl_internal.h | 11 +
tools/libxl/libxl_netbuffer.c | 419 ++++++++++++++++++++++++++++++++++++++
tools/libxl/libxl_nonetbuffer.c | 6 +
tools/libxl/libxl_remus.c | 35 ++++
7 files changed, 479 insertions(+), 5 deletions(-)
create mode 100644 tools/libxl/libxl_remus.c
diff --git a/docs/misc/xenstore-paths.markdown b/docs/misc/xenstore-paths.markdown
index 70ab7f4..7a0d2c9 100644
--- a/docs/misc/xenstore-paths.markdown
+++ b/docs/misc/xenstore-paths.markdown
@@ -385,6 +385,10 @@ The guest's virtual time offset from UTC in seconds.
The device model version for a domain.
+#### /libxl/$DOMID/remus/netbuf/$DEVID/ifb = STRING [n,INTERNAL]
+
+IFB device used by Remus to buffer network output from the associated vif.
+
[BLKIF]: http://xenbits.xen.org/docs/unstable/hypercall/include,public,io,blkif.h.html
[FBIF]: http://xenbits.xen.org/docs/unstable/hypercall/include,public,io,fbif.h.html
[HVMPARAMS]: http://xenbits.xen.org/docs/unstable/hypercall/include,public,hvm,params.h.html
diff --git a/tools/libxl/Makefile b/tools/libxl/Makefile
index 84a467c..218f55e 100644
--- a/tools/libxl/Makefile
+++ b/tools/libxl/Makefile
@@ -52,6 +52,8 @@ else
LIBXL_OBJS-y += libxl_nonetbuffer.o
endif
+LIBXL_OBJS-y += libxl_remus.o
+
LIBXL_OBJS-$(CONFIG_X86) += libxl_cpuid.o libxl_x86.o
LIBXL_OBJS-$(CONFIG_ARM) += libxl_nocpuid.o libxl_arm.o
diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c
index 8d63f90..e3e9f6f 100644
--- a/tools/libxl/libxl_dom.c
+++ b/tools/libxl/libxl_dom.c
@@ -753,9 +753,6 @@ int libxl__toolstack_restore(uint32_t domid, const uint8_t *buf,
/*==================== Domain suspend (save) ====================*/
-static void domain_suspend_done(libxl__egc *egc,
- libxl__domain_suspend_state *dss, int rc);
-
/*----- complicated callback, called by xc_domain_save -----*/
/*
@@ -1508,8 +1505,8 @@ static void save_device_model_datacopier_done(libxl__egc *egc,
dss->save_dm_callback(egc, dss, our_rc);
}
-static void domain_suspend_done(libxl__egc *egc,
- libxl__domain_suspend_state *dss, int rc)
+void domain_suspend_done(libxl__egc *egc,
+ libxl__domain_suspend_state *dss, int rc)
{
STATE_AO_GC(dss->ao);
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 2f64382..0430307 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -2313,6 +2313,17 @@ typedef struct libxl__remus_state {
_hidden int libxl__netbuffer_enabled(libxl__gc *gc);
+_hidden void domain_suspend_done(libxl__egc *egc,
+ libxl__domain_suspend_state *dss,
+ int rc);
+
+_hidden void libxl__remus_setup_done(libxl__egc *egc,
+ libxl__domain_suspend_state *dss,
+ int rc);
+
+_hidden void libxl__remus_netbuf_setup(libxl__egc *egc,
+ libxl__domain_suspend_state *dss);
+
struct libxl__domain_suspend_state {
/* set by caller of libxl__domain_suspend */
libxl__ao *ao;
diff --git a/tools/libxl/libxl_netbuffer.c b/tools/libxl/libxl_netbuffer.c
index 8e23d75..0be876c 100644
--- a/tools/libxl/libxl_netbuffer.c
+++ b/tools/libxl/libxl_netbuffer.c
@@ -17,11 +17,430 @@
#include "libxl_internal.h"
+#include <netlink/cache.h>
+#include <netlink/socket.h>
+#include <netlink/attr.h>
+#include <netlink/route/link.h>
+#include <netlink/route/route.h>
+#include <netlink/route/qdisc.h>
+#include <netlink/route/qdisc/plug.h>
+
+typedef struct libxl__remus_netbuf_state {
+ struct rtnl_qdisc **netbuf_qdisc_list;
+ struct nl_sock *nlsock;
+ struct nl_cache *qdisc_cache;
+ const char **vif_list;
+ const char **ifb_list;
+ uint32_t num_netbufs;
+ uint32_t unused;
+} libxl__remus_netbuf_state;
+
int libxl__netbuffer_enabled(libxl__gc *gc)
{
return 1;
}
+/* If the device has a vifname, then use that instead of
+ * the vifX.Y format.
+ */
+static const char *get_vifname(libxl__gc *gc, uint32_t domid,
+ libxl_device_nic *nic)
+{
+ const char *vifname = NULL;
+ const char *path;
+ int rc;
+
+ path = libxl__sprintf(gc, "%s/backend/vif/%d/%d/vifname",
+ libxl__xs_get_dompath(gc, 0), domid, nic->devid);
+ rc = libxl__xs_read_checked(gc, XBT_NULL, path, &vifname);
+ if (rc < 0) {
+ /* use the default name */
+ vifname = libxl__device_nic_devname(gc, domid,
+ nic->devid,
+ nic->nictype);
+ }
+
+ return vifname;
+}
+
+static const char **get_guest_vif_list(libxl__gc *gc, uint32_t domid,
+ int *num_vifs)
+{
+ libxl_device_nic *nics = NULL;
+ int nb, i = 0;
+ const char **vif_list = NULL;
+
+ *num_vifs = 0;
+ nics = libxl_device_nic_list(CTX, domid, &nb);
+ if (!nics)
+ return NULL;
+
+ /* Ensure that none of the vifs are backed by driver domains */
+ for (i = 0; i < nb; i++) {
+ if (nics[i].backend_domid != LIBXL_TOOLSTACK_DOMID) {
+ LOG(ERROR, "vif %s has driver domain (%u) as its backend. "
+ "Network buffering is not supported with driver domains",
+ get_vifname(gc, domid, &nics[i]), nics[i].backend_domid);
+ *num_vifs = -1;
+ goto out;
+ }
+ }
+
+ GCNEW_ARRAY(vif_list, nb);
+ for (i = 0; i < nb; ++i) {
+ vif_list[i] = get_vifname(gc, domid, &nics[i]);
+ if (!vif_list[i]) {
+ vif_list = NULL;
+ goto out;
+ }
+ }
+ *num_vifs = nb;
+
+ out:
+ for (i = 0; i < nb; i++)
+ libxl_device_nic_dispose(&nics[i]);
+ free(nics);
+ return vif_list;
+}
+
+static void free_qdiscs(libxl__remus_netbuf_state *netbuf_state)
+{
+ int i;
+ struct rtnl_qdisc *qdisc = NULL;
+
+ /* free qdiscs */
+ for (i = 0; i < netbuf_state->num_netbufs; i++) {
+ qdisc = netbuf_state->netbuf_qdisc_list[i];
+ if (!qdisc)
+ break;
+
+ nl_object_put((struct nl_object *)qdisc);
+ }
+
+ /* free qdisc cache */
+ nl_cache_clear(netbuf_state->qdisc_cache);
+ nl_cache_free(netbuf_state->qdisc_cache);
+
+ /* close nlsock */
+ nl_close(netbuf_state->nlsock);
+
+ /* free nlsock */
+ nl_socket_free(netbuf_state->nlsock);
+}
+
+static int init_qdiscs(libxl__gc *gc,
+ libxl__remus_state *remus_state)
+{
+ int i, ret, ifindex;
+ struct rtnl_link *ifb = NULL;
+ struct rtnl_qdisc *qdisc = NULL;
+
+ /* Convenience aliases */
+ libxl__remus_netbuf_state * const netbuf_state = remus_state->netbuf_state;
+ const int num_netbufs = netbuf_state->num_netbufs;
+ const char ** const ifb_list = netbuf_state->ifb_list;
+
+ /* Now that we have brought up IFB devices with plug qdisc for
+ * each vif, lets get a netlink handle on the plug qdisc for use
+ * during checkpointing.
+ */
+ netbuf_state->nlsock = nl_socket_alloc();
+ if (!netbuf_state->nlsock) {
+ LOG(ERROR, "cannot allocate nl socket");
+ goto out;
+ }
+
+ ret = nl_connect(netbuf_state->nlsock, NETLINK_ROUTE);
+ if (ret) {
+ LOG(ERROR, "failed to open netlink socket: %s",
+ nl_geterror(ret));
+ goto out;
+ }
+
+ /* get list of all qdiscs installed on network devs. */
+ ret = rtnl_qdisc_alloc_cache(netbuf_state->nlsock,
+ &netbuf_state->qdisc_cache);
+ if (ret) {
+ LOG(ERROR, "failed to allocate qdisc cache: %s",
+ nl_geterror(ret));
+ goto out;
+ }
+
+ /* list of handles to plug qdiscs */
+ GCNEW_ARRAY(netbuf_state->netbuf_qdisc_list, num_netbufs);
+
+ for (i = 0; i < num_netbufs; ++i) {
+
+ /* get a handle to the IFB interface */
+ ifb = NULL;
+ ret = rtnl_link_get_kernel(netbuf_state->nlsock, 0,
+ ifb_list[i], &ifb);
+ if (ret) {
+ LOG(ERROR, "cannot obtain handle for %s: %s", ifb_list[i],
+ nl_geterror(ret));
+ goto out;
+ }
+
+ ifindex = rtnl_link_get_ifindex(ifb);
+ if (!ifindex) {
+ LOG(ERROR, "interface %s has no index", ifb_list[i]);
+ goto out;
+ }
+
+ /* Get a reference to the root qdisc installed on the IFB, by
+ * querying the qdisc list we obtained earlier. The netbufscript
+ * sets up the plug qdisc as the root qdisc, so we don't have to
+ * search the entire qdisc tree on the IFB dev.
+
+ * There is no need to explicitly free this qdisc as its just a
+ * reference from the qdisc cache we allocated earlier.
+ */
+ qdisc = rtnl_qdisc_get_by_parent(netbuf_state->qdisc_cache, ifindex,
+ TC_H_ROOT);
+
+ if (qdisc) {
+ const char *tc_kind = rtnl_tc_get_kind(TC_CAST(qdisc));
+ /* Sanity check: Ensure that the root qdisc is a plug qdisc. */
+ if (!tc_kind || strcmp(tc_kind, "plug")) {
+ nl_object_put((struct nl_object *)qdisc);
+ LOG(ERROR, "plug qdisc is not installed on %s", ifb_list[i]);
+ goto out;
+ }
+ netbuf_state->netbuf_qdisc_list[i] = qdisc;
+ } else {
+ LOG(ERROR, "Cannot get qdisc handle from ifb %s", ifb_list[i]);
+ goto out;
+ }
+ rtnl_link_put(ifb);
+ }
+
+ return 0;
+
+ out:
+ if (ifb)
+ rtnl_link_put(ifb);
+ free_qdiscs(netbuf_state);
+ return ERROR_FAIL;
+}
+
+static void netbuf_setup_timeout_cb(libxl__egc *egc,
+ libxl__ev_time *ev,
+ const struct timeval *requested_abs)
+{
+ libxl__remus_state *remus_state = CONTAINER_OF(ev, *remus_state, timeout);
+
+ /* Convenience aliases */
+ const int devid = remus_state->dev_id;
+ libxl__remus_netbuf_state *const netbuf_state = remus_state->netbuf_state;
+ const char *const vif = netbuf_state->vif_list[devid];
+
+ STATE_AO_GC(remus_state->dss->ao);
+
+ libxl__ev_time_deregister(gc, &remus_state->timeout);
+ assert(libxl__ev_child_inuse(&remus_state->child));
+
+ LOG(DEBUG, "killing hotplug script %s (on vif %s) because of timeout",
+ remus_state->netbufscript, vif);
+
+ if (kill(remus_state->child.pid, SIGKILL)) {
+ LOGEV(ERROR, errno, "unable to kill hotplug script %s [%ld]",
+ remus_state->netbufscript,
+ (unsigned long)remus_state->child.pid);
+ }
+
+ return;
+}
+
+/* the script needs the following env & args
+ * $vifname
+ * $XENBUS_PATH (/libxl/<domid>/remus/netbuf/<devid>/)
+ * $IFB (for teardown)
+ * setup/teardown as command line arg.
+ * In return, the script writes the name of IFB device (during setup) to be
+ * used for output buffering into XENBUS_PATH/ifb
+ */
+static int exec_netbuf_script(libxl__gc *gc, libxl__remus_state *remus_state,
+ char *op, libxl__ev_child_callback *death)
+{
+ int arraysize = 7, nr = 0;
+ char **env = NULL, **args = NULL;
+ pid_t pid;
+
+ /* Convenience aliases */
+ libxl__ev_child *const child = &remus_state->child;
+ libxl__ev_time *const timeout = &remus_state->timeout;
+ char *const script = libxl__strdup(gc, remus_state->netbufscript);
+ const uint32_t domid = remus_state->dss->domid;
+ const int devid = remus_state->dev_id;
+ libxl__remus_netbuf_state *const netbuf_state = remus_state->netbuf_state;
+ const char *const vif = netbuf_state->vif_list[devid];
+ const char *const ifb = netbuf_state->ifb_list[devid];
+
+ GCNEW_ARRAY(env, arraysize);
+ env[nr++] = "vifname";
+ env[nr++] = libxl__strdup(gc, vif);
+ env[nr++] = "XENBUS_PATH";
+ env[nr++] = GCSPRINTF("%s/remus/netbuf/%d",
+ libxl__xs_libxl_path(gc, domid), devid);
+ if (!strcmp(op, "teardown")) {
+ env[nr++] = "IFB";
+ env[nr++] = libxl__strdup(gc, ifb);
+ }
+ env[nr++] = NULL;
+ assert(nr <= arraysize);
+
+ arraysize = 3; nr = 0;
+ GCNEW_ARRAY(args, arraysize);
+ args[nr++] = script;
+ args[nr++] = op;
+ args[nr++] = NULL;
+ assert(nr == arraysize);
+
+ /* Set hotplug timeout */
+ if (libxl__ev_time_register_rel(gc, timeout,
+ netbuf_setup_timeout_cb,
+ LIBXL_HOTPLUG_TIMEOUT * 1000)) {
+ LOG(ERROR, "unable to register timeout for "
+ "netbuf setup script %s on vif %s", script, vif);
+ return ERROR_FAIL;
+ }
+
+ LOG(DEBUG, "Calling netbuf script: %s %s on vif %s",
+ script, op, vif);
+
+ /* Fork and exec netbuf script */
+ pid = libxl__ev_child_fork(gc, child, death);
+ if (pid == -1) {
+ LOG(ERROR, "unable to fork netbuf script %s", script);
+ return ERROR_FAIL;
+ }
+
+ if (!pid) {
+ /* child: Launch netbuf script */
+ libxl__exec(gc, -1, -1, -1, args[0], args, env);
+ /* notreached */
+ abort();
+ }
+
+ return 0;
+}
+
+static void netbuf_setup_script_cb(libxl__egc *egc,
+ libxl__ev_child *child,
+ pid_t pid, int status)
+{
+ libxl__remus_state *remus_state = CONTAINER_OF(child, *remus_state, child);
+ const char *out_path_base, *hotplug_error = NULL;
+ int rc = ERROR_FAIL;
+
+ /* Convenience aliases */
+ const uint32_t domid = remus_state->dss->domid;
+ const int devid = remus_state->dev_id;
+ libxl__remus_netbuf_state *const netbuf_state = remus_state->netbuf_state;
+ const char *const vif = netbuf_state->vif_list[devid];
+ const char **const ifb = &netbuf_state->ifb_list[devid];
+
+ STATE_AO_GC(remus_state->dss->ao);
+
+ libxl__ev_time_deregister(gc, &remus_state->timeout);
+
+ out_path_base = GCSPRINTF("%s/remus/netbuf/%d",
+ libxl__xs_libxl_path(gc, domid), devid);
+
+ rc = libxl__xs_read_checked(gc, XBT_NULL,
+ GCSPRINTF("%s/hotplug-error", out_path_base),
+ &hotplug_error);
+ if (rc)
+ goto out;
+
+ if (hotplug_error) {
+ LOG(ERROR, "netbuf script %s setup failed for vif %s: %s",
+ remus_state->netbufscript,
+ netbuf_state->vif_list[devid], hotplug_error);
+ rc = ERROR_FAIL;
+ goto out;
+ }
+
+ if (status) {
+ libxl_report_child_exitstatus(CTX, LIBXL__LOG_ERROR,
+ remus_state->netbufscript,
+ pid, status);
+ rc = ERROR_FAIL;
+ goto out;
+ }
+
+ rc = libxl__xs_read_checked(gc, XBT_NULL,
+ GCSPRINTF("%s/remus/netbuf/%d/ifb",
+ libxl__xs_libxl_path(gc, domid),
+ devid),
+ ifb);
+ if (rc)
+ goto out;
+
+ if (!(*ifb)) {
+ LOG(ERROR, "Cannot get ifb dev name for domain %u dev %s",
+ domid, vif);
+ rc = ERROR_FAIL;
+ goto out;
+ }
+
+ LOG(DEBUG, "%s will buffer packets from vif %s", *ifb, vif);
+ remus_state->dev_id++;
+ if (remus_state->dev_id < netbuf_state->num_netbufs) {
+ rc = exec_netbuf_script(gc, remus_state,
+ "setup", netbuf_setup_script_cb);
+ if (rc)
+ goto out;
+
+ return;
+ }
+
+ rc = init_qdiscs(gc, remus_state);
+ out:
+ libxl__remus_setup_done(egc, remus_state->dss, rc);
+}
+
+/* Scan through the list of vifs belonging to domid and
+ * invoke the netbufscript to setup the IFB device & plug qdisc
+ * for each vif. Then scan through the list of IFB devices to obtain
+ * a handle on the plug qdisc installed on these IFB devices.
+ * Network output buffering is controlled via these qdiscs.
+ */
+void libxl__remus_netbuf_setup(libxl__egc *egc,
+ libxl__domain_suspend_state *dss)
+{
+ libxl__remus_netbuf_state *netbuf_state = NULL;
+ int num_netbufs = 0;
+ int rc = ERROR_FAIL;
+
+ /* Convenience aliases */
+ const uint32_t domid = dss->domid;
+ libxl__remus_state *const remus_state = dss->remus_state;
+
+ STATE_AO_GC(dss->ao);
+
+ GCNEW(netbuf_state);
+ netbuf_state->vif_list = get_guest_vif_list(gc, domid, &num_netbufs);
+ if (!num_netbufs) {
+ rc = 0;
+ goto out;
+ }
+
+ if (num_netbufs < 0) goto out;
+
+ GCNEW_ARRAY(netbuf_state->ifb_list, num_netbufs);
+ netbuf_state->num_netbufs = num_netbufs;
+ remus_state->netbuf_state = netbuf_state;
+ remus_state->dev_id = 0;
+ if (exec_netbuf_script(gc, remus_state, "setup",
+ netbuf_setup_script_cb))
+ goto out;
+ return;
+
+ out:
+ libxl__remus_setup_done(egc, dss, rc);
+}
+
/*
* Local variables:
* mode: C
diff --git a/tools/libxl/libxl_nonetbuffer.c b/tools/libxl/libxl_nonetbuffer.c
index 6aa4bf1..acfa534 100644
--- a/tools/libxl/libxl_nonetbuffer.c
+++ b/tools/libxl/libxl_nonetbuffer.c
@@ -22,6 +22,12 @@ int libxl__netbuffer_enabled(libxl__gc *gc)
return 0;
}
+/* Remus network buffer related stubs */
+void libxl__remus_netbuf_setup(libxl__egc *egc,
+ libxl__domain_suspend_state *dss)
+{
+}
+
/*
* Local variables:
* mode: C
diff --git a/tools/libxl/libxl_remus.c b/tools/libxl/libxl_remus.c
new file mode 100644
index 0000000..b3342b3
--- /dev/null
+++ b/tools/libxl/libxl_remus.c
@@ -0,0 +1,35 @@
+/*
+ * Copyright (C) 2014
+ * Author Shriram Rajagopalan <rshriram@cs.ubc.ca>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU Lesser General Public License as published
+ * by the Free Software Foundation; version 2.1 only. with the special
+ * exception on linking described in file LICENSE.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU Lesser General Public License for more details.
+ */
+
+#include "libxl_osdeps.h" /* must come before any other headers */
+
+#include "libxl_internal.h"
+
+/*----- remus setup/teardown code -----*/
+
+void libxl__remus_setup_done(libxl__egc *egc,
+ libxl__domain_suspend_state *dss,
+ int rc)
+{
+ STATE_AO_GC(dss->ao);
+ if (!rc) {
+ libxl__domain_suspend(egc, dss);
+ return;
+ }
+
+ LOG(ERROR, "Remus: failed to setup network buffering"
+ " for guest with domid %u", dss->domid);
+ domain_suspend_done(egc, dss, rc);
+}
--
1.8.4.2
^ permalink raw reply related [flat|nested] 26+ messages in thread* Re: [PATCH 06/13 V6] remus: implement the API to setup network buffering
2014-01-21 9:05 ` [PATCH 06/13 V6] remus: implement the API to setup network buffering Lai Jiangshan
@ 2014-01-26 22:30 ` Shriram Rajagopalan
2014-01-27 6:57 ` Wen Congyang
0 siblings, 1 reply; 26+ messages in thread
From: Shriram Rajagopalan @ 2014-01-26 22:30 UTC (permalink / raw)
To: Lai Jiangshan
Cc: Ian Campbell, FNST-Wen Congyang, Stefano Stabellini,
Andrew Cooper, Jiang Yunhong, Ian Jackson,
xen-devel@lists.xen.org, Dong Eddie, Roger Pau Monne
[-- Attachment #1.1: Type: text/plain, Size: 15889 bytes --]
On Tue, Jan 21, 2014 at 1:05 AM, Lai Jiangshan <laijs@cn.fujitsu.com> wrote:
> From: Shriram Rajagopalan <rshriram@cs.ubc.ca>
>
> The following steps are taken during setup:
> a) call the hotplug script for each vif to setup its network buffer
>
> b) establish a dedicated remus context containing libnl related
> state (netlink sockets, qdisc caches, etc.,)
>
> c) Obtain handles to plug qdiscs installed on the IFB devices
> chosen by the hotplug scripts.
>
> Signed-off-by: Shriram Rajagopalan <rshriram@cs.ubc.ca>
> Signed-off-by: Lai Jiangshan <laijs@cn.fujitsu.com>
> Reviewed-by: Wen Congyang <wency@cn.fujitsu.com>
> ---
> docs/misc/xenstore-paths.markdown | 4 +
> tools/libxl/Makefile | 2 +
> tools/libxl/libxl_dom.c | 7 +-
> tools/libxl/libxl_internal.h | 11 +
> tools/libxl/libxl_netbuffer.c | 419
> ++++++++++++++++++++++++++++++++++++++
> tools/libxl/libxl_nonetbuffer.c | 6 +
> tools/libxl/libxl_remus.c | 35 ++++
> 7 files changed, 479 insertions(+), 5 deletions(-)
> create mode 100644 tools/libxl/libxl_remus.c
>
> diff --git a/docs/misc/xenstore-paths.markdown
> b/docs/misc/xenstore-paths.markdown
> index 70ab7f4..7a0d2c9 100644
> --- a/docs/misc/xenstore-paths.markdown
> +++ b/docs/misc/xenstore-paths.markdown
> @@ -385,6 +385,10 @@ The guest's virtual time offset from UTC in seconds.
>
> The device model version for a domain.
>
> +#### /libxl/$DOMID/remus/netbuf/$DEVID/ifb = STRING [n,INTERNAL]
> +
> +IFB device used by Remus to buffer network output from the associated vif.
> +
> [BLKIF]:
> http://xenbits.xen.org/docs/unstable/hypercall/include,public,io,blkif.h.html
> [FBIF]:
> http://xenbits.xen.org/docs/unstable/hypercall/include,public,io,fbif.h.html
> [HVMPARAMS]:
> http://xenbits.xen.org/docs/unstable/hypercall/include,public,hvm,params.h.html
> diff --git a/tools/libxl/Makefile b/tools/libxl/Makefile
> index 84a467c..218f55e 100644
> --- a/tools/libxl/Makefile
> +++ b/tools/libxl/Makefile
> @@ -52,6 +52,8 @@ else
> LIBXL_OBJS-y += libxl_nonetbuffer.o
> endif
>
> +LIBXL_OBJS-y += libxl_remus.o
> +
> LIBXL_OBJS-$(CONFIG_X86) += libxl_cpuid.o libxl_x86.o
> LIBXL_OBJS-$(CONFIG_ARM) += libxl_nocpuid.o libxl_arm.o
>
> diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c
> index 8d63f90..e3e9f6f 100644
> --- a/tools/libxl/libxl_dom.c
> +++ b/tools/libxl/libxl_dom.c
> @@ -753,9 +753,6 @@ int libxl__toolstack_restore(uint32_t domid, const
> uint8_t *buf,
>
> /*==================== Domain suspend (save) ====================*/
>
> -static void domain_suspend_done(libxl__egc *egc,
> - libxl__domain_suspend_state *dss, int rc);
> -
> /*----- complicated callback, called by xc_domain_save -----*/
>
> /*
> @@ -1508,8 +1505,8 @@ static void
> save_device_model_datacopier_done(libxl__egc *egc,
> dss->save_dm_callback(egc, dss, our_rc);
> }
>
> -static void domain_suspend_done(libxl__egc *egc,
> - libxl__domain_suspend_state *dss, int rc)
> +void domain_suspend_done(libxl__egc *egc,
> + libxl__domain_suspend_state *dss, int rc)
> {
> STATE_AO_GC(dss->ao);
>
> diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
> index 2f64382..0430307 100644
> --- a/tools/libxl/libxl_internal.h
> +++ b/tools/libxl/libxl_internal.h
> @@ -2313,6 +2313,17 @@ typedef struct libxl__remus_state {
>
> _hidden int libxl__netbuffer_enabled(libxl__gc *gc);
>
> +_hidden void domain_suspend_done(libxl__egc *egc,
> + libxl__domain_suspend_state *dss,
> + int rc);
> +
> +_hidden void libxl__remus_setup_done(libxl__egc *egc,
> + libxl__domain_suspend_state *dss,
> + int rc);
> +
> +_hidden void libxl__remus_netbuf_setup(libxl__egc *egc,
> + libxl__domain_suspend_state *dss);
> +
> struct libxl__domain_suspend_state {
> /* set by caller of libxl__domain_suspend */
> libxl__ao *ao;
> diff --git a/tools/libxl/libxl_netbuffer.c b/tools/libxl/libxl_netbuffer.c
> index 8e23d75..0be876c 100644
> --- a/tools/libxl/libxl_netbuffer.c
> +++ b/tools/libxl/libxl_netbuffer.c
> @@ -17,11 +17,430 @@
>
> #include "libxl_internal.h"
>
> +#include <netlink/cache.h>
> +#include <netlink/socket.h>
> +#include <netlink/attr.h>
> +#include <netlink/route/link.h>
> +#include <netlink/route/route.h>
> +#include <netlink/route/qdisc.h>
> +#include <netlink/route/qdisc/plug.h>
> +
> +typedef struct libxl__remus_netbuf_state {
> + struct rtnl_qdisc **netbuf_qdisc_list;
> + struct nl_sock *nlsock;
> + struct nl_cache *qdisc_cache;
> + const char **vif_list;
> + const char **ifb_list;
> + uint32_t num_netbufs;
> + uint32_t unused;
> +} libxl__remus_netbuf_state;
> +
> int libxl__netbuffer_enabled(libxl__gc *gc)
> {
> return 1;
> }
>
> +/* If the device has a vifname, then use that instead of
> + * the vifX.Y format.
> + */
> +static const char *get_vifname(libxl__gc *gc, uint32_t domid,
> + libxl_device_nic *nic)
> +{
> + const char *vifname = NULL;
> + const char *path;
> + int rc;
> +
> + path = libxl__sprintf(gc, "%s/backend/vif/%d/%d/vifname",
> + libxl__xs_get_dompath(gc, 0), domid,
> nic->devid);
> + rc = libxl__xs_read_checked(gc, XBT_NULL, path, &vifname);
> + if (rc < 0) {
> + /* use the default name */
> + vifname = libxl__device_nic_devname(gc, domid,
> + nic->devid,
> + nic->nictype);
> + }
> +
> + return vifname;
>
IanJ's feedback last time was that the error code rc needs to be checked.
If its a failure, then return NULL to the caller. If its a ENOENT, then use
the
default name.
+}
> +
> +static const char **get_guest_vif_list(libxl__gc *gc, uint32_t domid,
> + int *num_vifs)
> +{
> + libxl_device_nic *nics = NULL;
> + int nb, i = 0;
> + const char **vif_list = NULL;
> +
> + *num_vifs = 0;
> + nics = libxl_device_nic_list(CTX, domid, &nb);
> + if (!nics)
> + return NULL;
> +
> + /* Ensure that none of the vifs are backed by driver domains */
> + for (i = 0; i < nb; i++) {
> + if (nics[i].backend_domid != LIBXL_TOOLSTACK_DOMID) {
> + LOG(ERROR, "vif %s has driver domain (%u) as its backend. "
> + "Network buffering is not supported with driver domains",
> + get_vifname(gc, domid, &nics[i]), nics[i].backend_domid);
>
And if the previous feedback were to be incorporated, then get_vifname's
return
value should be assigned to a variable and checked before printing or using
it for
other purposes.
> + *num_vifs = -1;
> + goto out;
> + }
> + }
> +
> + GCNEW_ARRAY(vif_list, nb);
> + for (i = 0; i < nb; ++i) {
> + vif_list[i] = get_vifname(gc, domid, &nics[i]);
> + if (!vif_list[i]) {
> + vif_list = NULL;
> + goto out;
> + }
> + }
> + *num_vifs = nb;
> +
> + out:
> + for (i = 0; i < nb; i++)
> + libxl_device_nic_dispose(&nics[i]);
> + free(nics);
> + return vif_list;
> +}
> +
> +static void free_qdiscs(libxl__remus_netbuf_state *netbuf_state)
> +{
> + int i;
> + struct rtnl_qdisc *qdisc = NULL;
> +
> + /* free qdiscs */
> + for (i = 0; i < netbuf_state->num_netbufs; i++) {
> + qdisc = netbuf_state->netbuf_qdisc_list[i];
> + if (!qdisc)
> + break;
> +
> + nl_object_put((struct nl_object *)qdisc);
> + }
> +
> + /* free qdisc cache */
> + nl_cache_clear(netbuf_state->qdisc_cache);
> + nl_cache_free(netbuf_state->qdisc_cache);
> +
> + /* close nlsock */
> + nl_close(netbuf_state->nlsock);
> +
> + /* free nlsock */
> + nl_socket_free(netbuf_state->nlsock);
> +}
> +
>
This code (free_qdiscs) is new. Have you tested it?
While the control flow looks pretty sane, libnl has been evolving
a bit ever since the 3.* series.
If init_qdisc fails, it calls free_qdisc(). If any other setup stage after
network buffering fails, it would invoke the teardown code, which also
calls free_qdisc(). This may end up in a segfault.
I suggest adding a simple check to see if nlsock/qdisc_cache are NULL
before attempting to execute the rest of the function. And after you free
the
qdisc_cache & nlsock, set them to NULL.
> +static int init_qdiscs(libxl__gc *gc,
> + libxl__remus_state *remus_state)
> +{
> + int i, ret, ifindex;
> + struct rtnl_link *ifb = NULL;
> + struct rtnl_qdisc *qdisc = NULL;
> +
> + /* Convenience aliases */
> + libxl__remus_netbuf_state * const netbuf_state =
> remus_state->netbuf_state;
> + const int num_netbufs = netbuf_state->num_netbufs;
> + const char ** const ifb_list = netbuf_state->ifb_list;
> +
> + /* Now that we have brought up IFB devices with plug qdisc for
> + * each vif, lets get a netlink handle on the plug qdisc for use
> + * during checkpointing.
> + */
> + netbuf_state->nlsock = nl_socket_alloc();
> + if (!netbuf_state->nlsock) {
> + LOG(ERROR, "cannot allocate nl socket");
> + goto out;
> + }
> +
> + ret = nl_connect(netbuf_state->nlsock, NETLINK_ROUTE);
> + if (ret) {
> + LOG(ERROR, "failed to open netlink socket: %s",
> + nl_geterror(ret));
> + goto out;
> + }
> +
> + /* get list of all qdiscs installed on network devs. */
> + ret = rtnl_qdisc_alloc_cache(netbuf_state->nlsock,
> + &netbuf_state->qdisc_cache);
> + if (ret) {
> + LOG(ERROR, "failed to allocate qdisc cache: %s",
> + nl_geterror(ret));
> + goto out;
> + }
> +
> + /* list of handles to plug qdiscs */
> + GCNEW_ARRAY(netbuf_state->netbuf_qdisc_list, num_netbufs);
> +
> + for (i = 0; i < num_netbufs; ++i) {
> +
> + /* get a handle to the IFB interface */
> + ifb = NULL;
> + ret = rtnl_link_get_kernel(netbuf_state->nlsock, 0,
> + ifb_list[i], &ifb);
> + if (ret) {
> + LOG(ERROR, "cannot obtain handle for %s: %s", ifb_list[i],
> + nl_geterror(ret));
> + goto out;
> + }
> +
> + ifindex = rtnl_link_get_ifindex(ifb);
> + if (!ifindex) {
> + LOG(ERROR, "interface %s has no index", ifb_list[i]);
> + goto out;
> + }
> +
> + /* Get a reference to the root qdisc installed on the IFB, by
> + * querying the qdisc list we obtained earlier. The netbufscript
> + * sets up the plug qdisc as the root qdisc, so we don't have to
> + * search the entire qdisc tree on the IFB dev.
> +
> + * There is no need to explicitly free this qdisc as its just a
> + * reference from the qdisc cache we allocated earlier.
> + */
> + qdisc = rtnl_qdisc_get_by_parent(netbuf_state->qdisc_cache,
> ifindex,
> + TC_H_ROOT);
> +
> + if (qdisc) {
> + const char *tc_kind = rtnl_tc_get_kind(TC_CAST(qdisc));
> + /* Sanity check: Ensure that the root qdisc is a plug qdisc.
> */
> + if (!tc_kind || strcmp(tc_kind, "plug")) {
> + nl_object_put((struct nl_object *)qdisc);
> + LOG(ERROR, "plug qdisc is not installed on %s",
> ifb_list[i]);
> + goto out;
> + }
> + netbuf_state->netbuf_qdisc_list[i] = qdisc;
> + } else {
> + LOG(ERROR, "Cannot get qdisc handle from ifb %s",
> ifb_list[i]);
> + goto out;
> + }
> + rtnl_link_put(ifb);
> + }
> +
> + return 0;
> +
> + out:
> + if (ifb)
> + rtnl_link_put(ifb);
> + free_qdiscs(netbuf_state);
> + return ERROR_FAIL;
> +}
> +
> +static void netbuf_setup_timeout_cb(libxl__egc *egc,
> + libxl__ev_time *ev,
> + const struct timeval *requested_abs)
> +{
> + libxl__remus_state *remus_state = CONTAINER_OF(ev, *remus_state,
> timeout);
> +
> + /* Convenience aliases */
> + const int devid = remus_state->dev_id;
> + libxl__remus_netbuf_state *const netbuf_state =
> remus_state->netbuf_state;
> + const char *const vif = netbuf_state->vif_list[devid];
> +
> + STATE_AO_GC(remus_state->dss->ao);
> +
> + libxl__ev_time_deregister(gc, &remus_state->timeout);
> + assert(libxl__ev_child_inuse(&remus_state->child));
> +
> + LOG(DEBUG, "killing hotplug script %s (on vif %s) because of timeout",
> + remus_state->netbufscript, vif);
> +
> + if (kill(remus_state->child.pid, SIGKILL)) {
> + LOGEV(ERROR, errno, "unable to kill hotplug script %s [%ld]",
> + remus_state->netbufscript,
> + (unsigned long)remus_state->child.pid);
> + }
> +
> + return;
> +}
> +
> +/* the script needs the following env & args
> + * $vifname
> + * $XENBUS_PATH (/libxl/<domid>/remus/netbuf/<devid>/)
> + * $IFB (for teardown)
> + * setup/teardown as command line arg.
> + * In return, the script writes the name of IFB device (during setup) to
> be
> + * used for output buffering into XENBUS_PATH/ifb
> + */
> +static int exec_netbuf_script(libxl__gc *gc, libxl__remus_state
> *remus_state,
> + char *op, libxl__ev_child_callback *death)
> +{
> + int arraysize = 7, nr = 0;
> + char **env = NULL, **args = NULL;
> + pid_t pid;
> +
> + /* Convenience aliases */
> + libxl__ev_child *const child = &remus_state->child;
> + libxl__ev_time *const timeout = &remus_state->timeout;
> + char *const script = libxl__strdup(gc, remus_state->netbufscript);
> + const uint32_t domid = remus_state->dss->domid;
> + const int devid = remus_state->dev_id;
> + libxl__remus_netbuf_state *const netbuf_state =
> remus_state->netbuf_state;
> + const char *const vif = netbuf_state->vif_list[devid];
> + const char *const ifb = netbuf_state->ifb_list[devid];
> +
>
Please set arraysize to 7 here, instead of the beginning of the function.
Its more readable that way.
+ GCNEW_ARRAY(env, arraysize);
> + env[nr++] = "vifname";
> + env[nr++] = libxl__strdup(gc, vif);
> + env[nr++] = "XENBUS_PATH";
> + env[nr++] = GCSPRINTF("%s/remus/netbuf/%d",
> + libxl__xs_libxl_path(gc, domid), devid);
> + if (!strcmp(op, "teardown")) {
> + env[nr++] = "IFB";
> + env[nr++] = libxl__strdup(gc, ifb);
> + }
> + env[nr++] = NULL;
> + assert(nr <= arraysize);
> +
> + arraysize = 3; nr = 0;
> + GCNEW_ARRAY(args, arraysize);
> + args[nr++] = script;
> + args[nr++] = op;
> + args[nr++] = NULL;
> + assert(nr == arraysize);
> +
> + /* Set hotplug timeout */
> + if (libxl__ev_time_register_rel(gc, timeout,
> + netbuf_setup_timeout_cb,
> + LIBXL_HOTPLUG_TIMEOUT * 1000)) {
> + LOG(ERROR, "unable to register timeout for "
> + "netbuf setup script %s on vif %s", script, vif);
> + return ERROR_FAIL;
> + }
> +
> + LOG(DEBUG, "Calling netbuf script: %s %s on vif %s",
> + script, op, vif);
> +
> + /* Fork and exec netbuf script */
> + pid = libxl__ev_child_fork(gc, child, death);
> + if (pid == -1) {
> + LOG(ERROR, "unable to fork netbuf script %s", script);
> + return ERROR_FAIL;
> + }
> +
> + if (!pid) {
> + /* child: Launch netbuf script */
> + libxl__exec(gc, -1, -1, -1, args[0], args, env);
> + /* notreached */
> + abort();
> + }
> +
> + return 0;
> +}
> +
>
>
[-- Attachment #1.2: Type: text/html, Size: 19677 bytes --]
[-- Attachment #2: Type: text/plain, Size: 126 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [PATCH 06/13 V6] remus: implement the API to setup network buffering
2014-01-26 22:30 ` Shriram Rajagopalan
@ 2014-01-27 6:57 ` Wen Congyang
0 siblings, 0 replies; 26+ messages in thread
From: Wen Congyang @ 2014-01-27 6:57 UTC (permalink / raw)
To: rshriram, Lai Jiangshan
Cc: Ian Campbell, Stefano Stabellini, Andrew Cooper, Jiang Yunhong,
Ian Jackson, xen-devel@lists.xen.org, Dong Eddie, Roger Pau Monne
At 01/27/2014 06:30 AM, Shriram Rajagopalan Wrote:
> On Tue, Jan 21, 2014 at 1:05 AM, Lai Jiangshan <laijs@cn.fujitsu.com> wrote:
>
>> From: Shriram Rajagopalan <rshriram@cs.ubc.ca>
>>
>> The following steps are taken during setup:
>> a) call the hotplug script for each vif to setup its network buffer
>>
>> b) establish a dedicated remus context containing libnl related
>> state (netlink sockets, qdisc caches, etc.,)
>>
>> c) Obtain handles to plug qdiscs installed on the IFB devices
>> chosen by the hotplug scripts.
>>
>> Signed-off-by: Shriram Rajagopalan <rshriram@cs.ubc.ca>
>> Signed-off-by: Lai Jiangshan <laijs@cn.fujitsu.com>
>> Reviewed-by: Wen Congyang <wency@cn.fujitsu.com>
>> ---
>> docs/misc/xenstore-paths.markdown | 4 +
>> tools/libxl/Makefile | 2 +
>> tools/libxl/libxl_dom.c | 7 +-
>> tools/libxl/libxl_internal.h | 11 +
>> tools/libxl/libxl_netbuffer.c | 419
>> ++++++++++++++++++++++++++++++++++++++
>> tools/libxl/libxl_nonetbuffer.c | 6 +
>> tools/libxl/libxl_remus.c | 35 ++++
>> 7 files changed, 479 insertions(+), 5 deletions(-)
>> create mode 100644 tools/libxl/libxl_remus.c
>>
>> diff --git a/docs/misc/xenstore-paths.markdown
>> b/docs/misc/xenstore-paths.markdown
>> index 70ab7f4..7a0d2c9 100644
>> --- a/docs/misc/xenstore-paths.markdown
>> +++ b/docs/misc/xenstore-paths.markdown
>> @@ -385,6 +385,10 @@ The guest's virtual time offset from UTC in seconds.
>>
>> The device model version for a domain.
>>
>> +#### /libxl/$DOMID/remus/netbuf/$DEVID/ifb = STRING [n,INTERNAL]
>> +
>> +IFB device used by Remus to buffer network output from the associated vif.
>> +
>> [BLKIF]:
>> http://xenbits.xen.org/docs/unstable/hypercall/include,public,io,blkif.h.html
>> [FBIF]:
>> http://xenbits.xen.org/docs/unstable/hypercall/include,public,io,fbif.h.html
>> [HVMPARAMS]:
>> http://xenbits.xen.org/docs/unstable/hypercall/include,public,hvm,params.h.html
>> diff --git a/tools/libxl/Makefile b/tools/libxl/Makefile
>> index 84a467c..218f55e 100644
>> --- a/tools/libxl/Makefile
>> +++ b/tools/libxl/Makefile
>> @@ -52,6 +52,8 @@ else
>> LIBXL_OBJS-y += libxl_nonetbuffer.o
>> endif
>>
>> +LIBXL_OBJS-y += libxl_remus.o
>> +
>> LIBXL_OBJS-$(CONFIG_X86) += libxl_cpuid.o libxl_x86.o
>> LIBXL_OBJS-$(CONFIG_ARM) += libxl_nocpuid.o libxl_arm.o
>>
>> diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c
>> index 8d63f90..e3e9f6f 100644
>> --- a/tools/libxl/libxl_dom.c
>> +++ b/tools/libxl/libxl_dom.c
>> @@ -753,9 +753,6 @@ int libxl__toolstack_restore(uint32_t domid, const
>> uint8_t *buf,
>>
>> /*==================== Domain suspend (save) ====================*/
>>
>> -static void domain_suspend_done(libxl__egc *egc,
>> - libxl__domain_suspend_state *dss, int rc);
>> -
>> /*----- complicated callback, called by xc_domain_save -----*/
>>
>> /*
>> @@ -1508,8 +1505,8 @@ static void
>> save_device_model_datacopier_done(libxl__egc *egc,
>> dss->save_dm_callback(egc, dss, our_rc);
>> }
>>
>> -static void domain_suspend_done(libxl__egc *egc,
>> - libxl__domain_suspend_state *dss, int rc)
>> +void domain_suspend_done(libxl__egc *egc,
>> + libxl__domain_suspend_state *dss, int rc)
>> {
>> STATE_AO_GC(dss->ao);
>>
>> diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
>> index 2f64382..0430307 100644
>> --- a/tools/libxl/libxl_internal.h
>> +++ b/tools/libxl/libxl_internal.h
>> @@ -2313,6 +2313,17 @@ typedef struct libxl__remus_state {
>>
>> _hidden int libxl__netbuffer_enabled(libxl__gc *gc);
>>
>> +_hidden void domain_suspend_done(libxl__egc *egc,
>> + libxl__domain_suspend_state *dss,
>> + int rc);
>> +
>> +_hidden void libxl__remus_setup_done(libxl__egc *egc,
>> + libxl__domain_suspend_state *dss,
>> + int rc);
>> +
>> +_hidden void libxl__remus_netbuf_setup(libxl__egc *egc,
>> + libxl__domain_suspend_state *dss);
>> +
>> struct libxl__domain_suspend_state {
>> /* set by caller of libxl__domain_suspend */
>> libxl__ao *ao;
>> diff --git a/tools/libxl/libxl_netbuffer.c b/tools/libxl/libxl_netbuffer.c
>> index 8e23d75..0be876c 100644
>> --- a/tools/libxl/libxl_netbuffer.c
>> +++ b/tools/libxl/libxl_netbuffer.c
>> @@ -17,11 +17,430 @@
>>
>> #include "libxl_internal.h"
>>
>> +#include <netlink/cache.h>
>> +#include <netlink/socket.h>
>> +#include <netlink/attr.h>
>> +#include <netlink/route/link.h>
>> +#include <netlink/route/route.h>
>> +#include <netlink/route/qdisc.h>
>> +#include <netlink/route/qdisc/plug.h>
>> +
>> +typedef struct libxl__remus_netbuf_state {
>> + struct rtnl_qdisc **netbuf_qdisc_list;
>> + struct nl_sock *nlsock;
>> + struct nl_cache *qdisc_cache;
>> + const char **vif_list;
>> + const char **ifb_list;
>> + uint32_t num_netbufs;
>> + uint32_t unused;
>> +} libxl__remus_netbuf_state;
>> +
>> int libxl__netbuffer_enabled(libxl__gc *gc)
>> {
>> return 1;
>> }
>>
>> +/* If the device has a vifname, then use that instead of
>> + * the vifX.Y format.
>> + */
>> +static const char *get_vifname(libxl__gc *gc, uint32_t domid,
>> + libxl_device_nic *nic)
>> +{
>> + const char *vifname = NULL;
>> + const char *path;
>> + int rc;
>> +
>> + path = libxl__sprintf(gc, "%s/backend/vif/%d/%d/vifname",
>> + libxl__xs_get_dompath(gc, 0), domid,
>> nic->devid);
>> + rc = libxl__xs_read_checked(gc, XBT_NULL, path, &vifname);
>> + if (rc < 0) {
>> + /* use the default name */
>> + vifname = libxl__device_nic_devname(gc, domid,
>> + nic->devid,
>> + nic->nictype);
>> + }
>> +
>> + return vifname;
>>
>
> IanJ's feedback last time was that the error code rc needs to be checked.
> If its a failure, then return NULL to the caller. If its a ENOENT, then use
> the
> default name.
OK. This should be
if (!rc && !vifname) {
/* use the default name */
....
}
>
> +}
>> +
>> +static const char **get_guest_vif_list(libxl__gc *gc, uint32_t domid,
>> + int *num_vifs)
>> +{
>> + libxl_device_nic *nics = NULL;
>> + int nb, i = 0;
>> + const char **vif_list = NULL;
>> +
>> + *num_vifs = 0;
>> + nics = libxl_device_nic_list(CTX, domid, &nb);
>> + if (!nics)
>> + return NULL;
>> +
>> + /* Ensure that none of the vifs are backed by driver domains */
>> + for (i = 0; i < nb; i++) {
>> + if (nics[i].backend_domid != LIBXL_TOOLSTACK_DOMID) {
>> + LOG(ERROR, "vif %s has driver domain (%u) as its backend. "
>> + "Network buffering is not supported with driver domains",
>> + get_vifname(gc, domid, &nics[i]), nics[i].backend_domid);
>>
>
> And if the previous feedback were to be incorporated, then get_vifname's
> return
> value should be assigned to a variable and checked before printing or using
> it for
> other purposes.
OK
>
>
>> + *num_vifs = -1;
>> + goto out;
>> + }
>> + }
>> +
>> + GCNEW_ARRAY(vif_list, nb);
>> + for (i = 0; i < nb; ++i) {
>> + vif_list[i] = get_vifname(gc, domid, &nics[i]);
>> + if (!vif_list[i]) {
>> + vif_list = NULL;
>> + goto out;
>> + }
>> + }
>> + *num_vifs = nb;
>> +
>> + out:
>> + for (i = 0; i < nb; i++)
>> + libxl_device_nic_dispose(&nics[i]);
>> + free(nics);
>> + return vif_list;
>> +}
>> +
>> +static void free_qdiscs(libxl__remus_netbuf_state *netbuf_state)
>> +{
>> + int i;
>> + struct rtnl_qdisc *qdisc = NULL;
>> +
>> + /* free qdiscs */
>> + for (i = 0; i < netbuf_state->num_netbufs; i++) {
>> + qdisc = netbuf_state->netbuf_qdisc_list[i];
>> + if (!qdisc)
>> + break;
>> +
>> + nl_object_put((struct nl_object *)qdisc);
>> + }
>> +
>> + /* free qdisc cache */
>> + nl_cache_clear(netbuf_state->qdisc_cache);
>> + nl_cache_free(netbuf_state->qdisc_cache);
>> +
>> + /* close nlsock */
>> + nl_close(netbuf_state->nlsock);
>> +
>> + /* free nlsock */
>> + nl_socket_free(netbuf_state->nlsock);
>> +}
>> +
>>
>
> This code (free_qdiscs) is new. Have you tested it?
> While the control flow looks pretty sane, libnl has been evolving
> a bit ever since the 3.* series.
>
> If init_qdisc fails, it calls free_qdisc(). If any other setup stage after
> network buffering fails, it would invoke the teardown code, which also
> calls free_qdisc(). This may end up in a segfault.
>
> I suggest adding a simple check to see if nlsock/qdisc_cache are NULL
> before attempting to execute the rest of the function. And after you free
> the
> qdisc_cache & nlsock, set them to NULL.
Yes, free_qdisc() may be called twice. Will fix it in the next version.
>
>
>> +static int init_qdiscs(libxl__gc *gc,
>> + libxl__remus_state *remus_state)
>> +{
>> + int i, ret, ifindex;
>> + struct rtnl_link *ifb = NULL;
>> + struct rtnl_qdisc *qdisc = NULL;
>> +
>> + /* Convenience aliases */
>> + libxl__remus_netbuf_state * const netbuf_state =
>> remus_state->netbuf_state;
>> + const int num_netbufs = netbuf_state->num_netbufs;
>> + const char ** const ifb_list = netbuf_state->ifb_list;
>> +
>> + /* Now that we have brought up IFB devices with plug qdisc for
>> + * each vif, lets get a netlink handle on the plug qdisc for use
>> + * during checkpointing.
>> + */
>> + netbuf_state->nlsock = nl_socket_alloc();
>> + if (!netbuf_state->nlsock) {
>> + LOG(ERROR, "cannot allocate nl socket");
>> + goto out;
>> + }
>> +
>> + ret = nl_connect(netbuf_state->nlsock, NETLINK_ROUTE);
>> + if (ret) {
>> + LOG(ERROR, "failed to open netlink socket: %s",
>> + nl_geterror(ret));
>> + goto out;
>> + }
>> +
>> + /* get list of all qdiscs installed on network devs. */
>> + ret = rtnl_qdisc_alloc_cache(netbuf_state->nlsock,
>> + &netbuf_state->qdisc_cache);
>> + if (ret) {
>> + LOG(ERROR, "failed to allocate qdisc cache: %s",
>> + nl_geterror(ret));
>> + goto out;
>> + }
>> +
>> + /* list of handles to plug qdiscs */
>> + GCNEW_ARRAY(netbuf_state->netbuf_qdisc_list, num_netbufs);
>> +
>> + for (i = 0; i < num_netbufs; ++i) {
>> +
>> + /* get a handle to the IFB interface */
>> + ifb = NULL;
>> + ret = rtnl_link_get_kernel(netbuf_state->nlsock, 0,
>> + ifb_list[i], &ifb);
>> + if (ret) {
>> + LOG(ERROR, "cannot obtain handle for %s: %s", ifb_list[i],
>> + nl_geterror(ret));
>> + goto out;
>> + }
>> +
>> + ifindex = rtnl_link_get_ifindex(ifb);
>> + if (!ifindex) {
>> + LOG(ERROR, "interface %s has no index", ifb_list[i]);
>> + goto out;
>> + }
>> +
>> + /* Get a reference to the root qdisc installed on the IFB, by
>> + * querying the qdisc list we obtained earlier. The netbufscript
>> + * sets up the plug qdisc as the root qdisc, so we don't have to
>> + * search the entire qdisc tree on the IFB dev.
>> +
>> + * There is no need to explicitly free this qdisc as its just a
>> + * reference from the qdisc cache we allocated earlier.
>> + */
>> + qdisc = rtnl_qdisc_get_by_parent(netbuf_state->qdisc_cache,
>> ifindex,
>> + TC_H_ROOT);
>> +
>> + if (qdisc) {
>> + const char *tc_kind = rtnl_tc_get_kind(TC_CAST(qdisc));
>> + /* Sanity check: Ensure that the root qdisc is a plug qdisc.
>> */
>> + if (!tc_kind || strcmp(tc_kind, "plug")) {
>> + nl_object_put((struct nl_object *)qdisc);
>> + LOG(ERROR, "plug qdisc is not installed on %s",
>> ifb_list[i]);
>> + goto out;
>> + }
>> + netbuf_state->netbuf_qdisc_list[i] = qdisc;
>> + } else {
>> + LOG(ERROR, "Cannot get qdisc handle from ifb %s",
>> ifb_list[i]);
>> + goto out;
>> + }
>> + rtnl_link_put(ifb);
>> + }
>> +
>> + return 0;
>> +
>> + out:
>> + if (ifb)
>> + rtnl_link_put(ifb);
>> + free_qdiscs(netbuf_state);
>> + return ERROR_FAIL;
>> +}
>> +
>> +static void netbuf_setup_timeout_cb(libxl__egc *egc,
>> + libxl__ev_time *ev,
>> + const struct timeval *requested_abs)
>> +{
>> + libxl__remus_state *remus_state = CONTAINER_OF(ev, *remus_state,
>> timeout);
>> +
>> + /* Convenience aliases */
>> + const int devid = remus_state->dev_id;
>> + libxl__remus_netbuf_state *const netbuf_state =
>> remus_state->netbuf_state;
>> + const char *const vif = netbuf_state->vif_list[devid];
>> +
>> + STATE_AO_GC(remus_state->dss->ao);
>> +
>> + libxl__ev_time_deregister(gc, &remus_state->timeout);
>> + assert(libxl__ev_child_inuse(&remus_state->child));
>> +
>> + LOG(DEBUG, "killing hotplug script %s (on vif %s) because of timeout",
>> + remus_state->netbufscript, vif);
>> +
>> + if (kill(remus_state->child.pid, SIGKILL)) {
>> + LOGEV(ERROR, errno, "unable to kill hotplug script %s [%ld]",
>> + remus_state->netbufscript,
>> + (unsigned long)remus_state->child.pid);
>> + }
>> +
>> + return;
>> +}
>> +
>> +/* the script needs the following env & args
>> + * $vifname
>> + * $XENBUS_PATH (/libxl/<domid>/remus/netbuf/<devid>/)
>> + * $IFB (for teardown)
>> + * setup/teardown as command line arg.
>> + * In return, the script writes the name of IFB device (during setup) to
>> be
>> + * used for output buffering into XENBUS_PATH/ifb
>> + */
>> +static int exec_netbuf_script(libxl__gc *gc, libxl__remus_state
>> *remus_state,
>> + char *op, libxl__ev_child_callback *death)
>> +{
>> + int arraysize = 7, nr = 0;
>> + char **env = NULL, **args = NULL;
>> + pid_t pid;
>> +
>> + /* Convenience aliases */
>> + libxl__ev_child *const child = &remus_state->child;
>> + libxl__ev_time *const timeout = &remus_state->timeout;
>> + char *const script = libxl__strdup(gc, remus_state->netbufscript);
>> + const uint32_t domid = remus_state->dss->domid;
>> + const int devid = remus_state->dev_id;
>> + libxl__remus_netbuf_state *const netbuf_state =
>> remus_state->netbuf_state;
>> + const char *const vif = netbuf_state->vif_list[devid];
>> + const char *const ifb = netbuf_state->ifb_list[devid];
>> +
>>
>
> Please set arraysize to 7 here, instead of the beginning of the function.
> Its more readable that way.
OK.
Thanks
Wen Congyang
>
> + GCNEW_ARRAY(env, arraysize);
>> + env[nr++] = "vifname";
>> + env[nr++] = libxl__strdup(gc, vif);
>> + env[nr++] = "XENBUS_PATH";
>> + env[nr++] = GCSPRINTF("%s/remus/netbuf/%d",
>> + libxl__xs_libxl_path(gc, domid), devid);
>> + if (!strcmp(op, "teardown")) {
>> + env[nr++] = "IFB";
>> + env[nr++] = libxl__strdup(gc, ifb);
>> + }
>> + env[nr++] = NULL;
>> + assert(nr <= arraysize);
>> +
>> + arraysize = 3; nr = 0;
>> + GCNEW_ARRAY(args, arraysize);
>> + args[nr++] = script;
>> + args[nr++] = op;
>> + args[nr++] = NULL;
>> + assert(nr == arraysize);
>> +
>> + /* Set hotplug timeout */
>> + if (libxl__ev_time_register_rel(gc, timeout,
>> + netbuf_setup_timeout_cb,
>> + LIBXL_HOTPLUG_TIMEOUT * 1000)) {
>> + LOG(ERROR, "unable to register timeout for "
>> + "netbuf setup script %s on vif %s", script, vif);
>> + return ERROR_FAIL;
>> + }
>> +
>> + LOG(DEBUG, "Calling netbuf script: %s %s on vif %s",
>> + script, op, vif);
>> +
>> + /* Fork and exec netbuf script */
>> + pid = libxl__ev_child_fork(gc, child, death);
>> + if (pid == -1) {
>> + LOG(ERROR, "unable to fork netbuf script %s", script);
>> + return ERROR_FAIL;
>> + }
>> +
>> + if (!pid) {
>> + /* child: Launch netbuf script */
>> + libxl__exec(gc, -1, -1, -1, args[0], args, env);
>> + /* notreached */
>> + abort();
>> + }
>> +
>> + return 0;
>> +}
>> +
>>
>>
>
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel
>
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH 07/13 V6] remus: implement the API to buffer/release packages
2014-01-21 9:05 [PATCH 00/13 V6] Remus/Libxl: Network buffering support Lai Jiangshan
` (5 preceding siblings ...)
2014-01-21 9:05 ` [PATCH 06/13 V6] remus: implement the API to setup network buffering Lai Jiangshan
@ 2014-01-21 9:05 ` Lai Jiangshan
2014-01-21 9:05 ` [PATCH 08/13 V6] remus: implement the API to teardown network buffering Lai Jiangshan
` (6 subsequent siblings)
13 siblings, 0 replies; 26+ messages in thread
From: Lai Jiangshan @ 2014-01-21 9:05 UTC (permalink / raw)
To: xen-devel
Cc: Ian Campbell, FNST-Wen Congyang, Stefano Stabellini,
Andrew Cooper, Jiang Yunhong, Ian Jackson, Lai Jiangshan,
Dong Eddie, Shriram Rajagopalan, Roger Pau Monne
From: Shriram Rajagopalan <rshriram@cs.ubc.ca>
This patch implements two APIs:
1. libxl__remus_netbuf_start_new_epoch()
It marks a new epoch. The packages before this epoch will
be flushed, and the packages after this epoch will be buffered.
It will be called after the guest is suspended.
2. libxl__remus_netbuf_release_prev_epoch()
It flushes the buffered packages to client, and it will be
called when a checkpoint finishes.
Signed-off-by: Shriram Rajagopalan <rshriram@cs.ubc.ca>
Signed-off-by: Lai Jiangshan <laijs@cn.fujitsu.com>
Reviewed-by: Wen Congyang <wency@cn.fujitsu.com>
---
tools/libxl/libxl_internal.h | 6 +++++
tools/libxl/libxl_netbuffer.c | 49 +++++++++++++++++++++++++++++++++++++++++
tools/libxl/libxl_nonetbuffer.c | 14 ++++++++++++
3 files changed, 69 insertions(+)
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 0430307..7216f89 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -2324,6 +2324,12 @@ _hidden void libxl__remus_setup_done(libxl__egc *egc,
_hidden void libxl__remus_netbuf_setup(libxl__egc *egc,
libxl__domain_suspend_state *dss);
+_hidden int libxl__remus_netbuf_start_new_epoch(libxl__gc *gc, uint32_t domid,
+ libxl__remus_state *remus_state);
+
+_hidden int libxl__remus_netbuf_release_prev_epoch(libxl__gc *gc, uint32_t domid,
+ libxl__remus_state *remus_state);
+
struct libxl__domain_suspend_state {
/* set by caller of libxl__domain_suspend */
libxl__ao *ao;
diff --git a/tools/libxl/libxl_netbuffer.c b/tools/libxl/libxl_netbuffer.c
index 0be876c..1b61597 100644
--- a/tools/libxl/libxl_netbuffer.c
+++ b/tools/libxl/libxl_netbuffer.c
@@ -441,6 +441,55 @@ void libxl__remus_netbuf_setup(libxl__egc *egc,
libxl__remus_setup_done(egc, dss, rc);
}
+/* The buffer_op's value, not the value passed to kernel */
+enum {
+ tc_buffer_start,
+ tc_buffer_release
+};
+
+static int remus_netbuf_op(libxl__gc *gc, uint32_t domid,
+ libxl__remus_state *remus_state,
+ int buffer_op)
+{
+ int i, ret;
+
+ /* Convenience aliases */
+ libxl__remus_netbuf_state *const netbuf_state = remus_state->netbuf_state;
+
+ for (i = 0; i < netbuf_state->num_netbufs; ++i) {
+ if (buffer_op == tc_buffer_start)
+ ret = rtnl_qdisc_plug_buffer(netbuf_state->netbuf_qdisc_list[i]);
+ else
+ ret = rtnl_qdisc_plug_release_one(netbuf_state->netbuf_qdisc_list[i]);
+
+ if (!ret)
+ ret = rtnl_qdisc_add(netbuf_state->nlsock,
+ netbuf_state->netbuf_qdisc_list[i],
+ NLM_F_REQUEST);
+ if (ret) {
+ LOG(ERROR, "Remus: cannot do netbuf op %s on %s:%s",
+ ((buffer_op == tc_buffer_start) ?
+ "start_new_epoch" : "release_prev_epoch"),
+ netbuf_state->ifb_list[i], nl_geterror(ret));
+ return ERROR_FAIL;
+ }
+ }
+
+ return 0;
+}
+
+int libxl__remus_netbuf_start_new_epoch(libxl__gc *gc, uint32_t domid,
+ libxl__remus_state *remus_state)
+{
+ return remus_netbuf_op(gc, domid, remus_state, tc_buffer_start);
+}
+
+int libxl__remus_netbuf_release_prev_epoch(libxl__gc *gc, uint32_t domid,
+ libxl__remus_state *remus_state)
+{
+ return remus_netbuf_op(gc, domid, remus_state, tc_buffer_release);
+}
+
/*
* Local variables:
* mode: C
diff --git a/tools/libxl/libxl_nonetbuffer.c b/tools/libxl/libxl_nonetbuffer.c
index acfa534..a3e3f5c 100644
--- a/tools/libxl/libxl_nonetbuffer.c
+++ b/tools/libxl/libxl_nonetbuffer.c
@@ -28,6 +28,20 @@ void libxl__remus_netbuf_setup(libxl__egc *egc,
{
}
+int libxl__remus_netbuf_start_new_epoch(libxl__gc *gc, uint32_t domid,
+ libxl__remus_state *remus_state)
+{
+ LOG(ERROR, "Remus: No support for network buffering");
+ return ERROR_FAIL;
+}
+
+int libxl__remus_netbuf_release_prev_epoch(libxl__gc *gc, uint32_t domid,
+ libxl__remus_state *remus_state)
+{
+ LOG(ERROR, "Remus: No support for network buffering");
+ return ERROR_FAIL;
+}
+
/*
* Local variables:
* mode: C
--
1.8.4.2
^ permalink raw reply related [flat|nested] 26+ messages in thread* [PATCH 08/13 V6] remus: implement the API to teardown network buffering
2014-01-21 9:05 [PATCH 00/13 V6] Remus/Libxl: Network buffering support Lai Jiangshan
` (6 preceding siblings ...)
2014-01-21 9:05 ` [PATCH 07/13 V6] remus: implement the API to buffer/release packages Lai Jiangshan
@ 2014-01-21 9:05 ` Lai Jiangshan
2014-01-21 9:05 ` [PATCH 09/13 V6] remus: use the API to setup " Lai Jiangshan
` (5 subsequent siblings)
13 siblings, 0 replies; 26+ messages in thread
From: Lai Jiangshan @ 2014-01-21 9:05 UTC (permalink / raw)
To: xen-devel
Cc: Ian Campbell, FNST-Wen Congyang, Stefano Stabellini,
Andrew Cooper, Jiang Yunhong, Ian Jackson, Lai Jiangshan,
Dong Eddie, Shriram Rajagopalan, Roger Pau Monne
From: Shriram Rajagopalan <rshriram@cs.ubc.ca>
During teardown, the netlink resources are released, followed by
invocation of hotplug scripts to remove the IFB devices.
Signed-off-by: Shriram Rajagopalan <rshriram@cs.ubc.ca>
Signed-off-by: Lai Jiangshan <laijs@cn.fujitsu.com>
Reviewed-by: Wen Congyang <wency@cn.fujitsu.com>
---
tools/libxl/libxl_internal.h | 6 +++++
tools/libxl/libxl_netbuffer.c | 52 +++++++++++++++++++++++++++++++++++++++++
tools/libxl/libxl_nonetbuffer.c | 5 ++++
tools/libxl/libxl_remus.c | 6 +++++
4 files changed, 69 insertions(+)
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 7216f89..358590b 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -2330,6 +2330,12 @@ _hidden int libxl__remus_netbuf_start_new_epoch(libxl__gc *gc, uint32_t domid,
_hidden int libxl__remus_netbuf_release_prev_epoch(libxl__gc *gc, uint32_t domid,
libxl__remus_state *remus_state);
+_hidden void libxl__remus_teardown_done(libxl__egc *egc,
+ libxl__domain_suspend_state *dss);
+
+_hidden void libxl__remus_netbuf_teardown(libxl__egc *egc,
+ libxl__domain_suspend_state *dss);
+
struct libxl__domain_suspend_state {
/* set by caller of libxl__domain_suspend */
libxl__ao *ao;
diff --git a/tools/libxl/libxl_netbuffer.c b/tools/libxl/libxl_netbuffer.c
index 1b61597..686101b 100644
--- a/tools/libxl/libxl_netbuffer.c
+++ b/tools/libxl/libxl_netbuffer.c
@@ -490,6 +490,58 @@ int libxl__remus_netbuf_release_prev_epoch(libxl__gc *gc, uint32_t domid,
return remus_netbuf_op(gc, domid, remus_state, tc_buffer_release);
}
+static void netbuf_teardown_script_cb(libxl__egc *egc,
+ libxl__ev_child *child,
+ pid_t pid, int status)
+{
+ libxl__remus_state *remus_state = CONTAINER_OF(child, *remus_state, child);
+
+ /* Convenience aliases */
+ libxl__remus_netbuf_state *const netbuf_state = remus_state->netbuf_state;
+
+ STATE_AO_GC(remus_state->dss->ao);
+
+ libxl__ev_time_deregister(gc, &remus_state->timeout);
+
+ if (status) {
+ libxl_report_child_exitstatus(CTX, LIBXL__LOG_ERROR,
+ remus_state->netbufscript,
+ pid, status);
+ }
+
+ remus_state->dev_id++;
+ if (remus_state->dev_id < netbuf_state->num_netbufs) {
+ if (exec_netbuf_script(gc, remus_state,
+ "teardown", netbuf_teardown_script_cb))
+ goto out;
+ return;
+ }
+
+ out:
+ libxl__remus_teardown_done(egc, remus_state->dss);
+}
+
+/* Note: This function will be called in the same gc context as
+ * libxl__remus_netbuf_setup, created during the libxl_domain_remus_start
+ * API call.
+ */
+void libxl__remus_netbuf_teardown(libxl__egc *egc,
+ libxl__domain_suspend_state *dss)
+{
+ /* Convenience aliases */
+ libxl__remus_state *const remus_state = dss->remus_state;
+ libxl__remus_netbuf_state *const netbuf_state = remus_state->netbuf_state;
+
+ STATE_AO_GC(dss->ao);
+
+ free_qdiscs(netbuf_state);
+
+ remus_state->dev_id = 0;
+ if (exec_netbuf_script(gc, remus_state, "teardown",
+ netbuf_teardown_script_cb))
+ libxl__remus_teardown_done(egc, dss);
+}
+
/*
* Local variables:
* mode: C
diff --git a/tools/libxl/libxl_nonetbuffer.c b/tools/libxl/libxl_nonetbuffer.c
index a3e3f5c..ef7b513 100644
--- a/tools/libxl/libxl_nonetbuffer.c
+++ b/tools/libxl/libxl_nonetbuffer.c
@@ -42,6 +42,11 @@ int libxl__remus_netbuf_release_prev_epoch(libxl__gc *gc, uint32_t domid,
return ERROR_FAIL;
}
+void libxl__remus_netbuf_teardown(libxl__egc *egc,
+ libxl__domain_suspend_state *dss)
+{
+}
+
/*
* Local variables:
* mode: C
diff --git a/tools/libxl/libxl_remus.c b/tools/libxl/libxl_remus.c
index b3342b3..6790c61 100644
--- a/tools/libxl/libxl_remus.c
+++ b/tools/libxl/libxl_remus.c
@@ -33,3 +33,9 @@ void libxl__remus_setup_done(libxl__egc *egc,
" for guest with domid %u", dss->domid);
domain_suspend_done(egc, dss, rc);
}
+
+void libxl__remus_teardown_done(libxl__egc *egc,
+ libxl__domain_suspend_state *dss)
+{
+ dss->callback(egc, dss, dss->remus_state->saved_rc);
+}
--
1.8.4.2
^ permalink raw reply related [flat|nested] 26+ messages in thread* [PATCH 09/13 V6] remus: use the API to setup network buffering
2014-01-21 9:05 [PATCH 00/13 V6] Remus/Libxl: Network buffering support Lai Jiangshan
` (7 preceding siblings ...)
2014-01-21 9:05 ` [PATCH 08/13 V6] remus: implement the API to teardown network buffering Lai Jiangshan
@ 2014-01-21 9:05 ` Lai Jiangshan
2014-01-21 9:05 ` [PATCH 10/13 V6] remus: use the API to teardown " Lai Jiangshan
` (4 subsequent siblings)
13 siblings, 0 replies; 26+ messages in thread
From: Lai Jiangshan @ 2014-01-21 9:05 UTC (permalink / raw)
To: xen-devel
Cc: Ian Campbell, FNST-Wen Congyang, Stefano Stabellini,
Andrew Cooper, Jiang Yunhong, Ian Jackson, Lai Jiangshan,
Dong Eddie, Shriram Rajagopalan, Roger Pau Monne
From: Shriram Rajagopalan <rshriram@cs.ubc.ca>
If there is network buffering hotplug scripts, call
libxl__remus_netbuf_setup() to setup the network
buffering.
Signed-off-by: Shriram Rajagopalan <rshriram@cs.ubc.ca>
Signed-off-by: Lai Jiangshan <laijs@cn.fujitsu.com>
Reviewed-by: Wen Congyang <wency@cn.fujitsu.com>
---
tools/libxl/libxl.c | 2 +-
tools/libxl/libxl_internal.h | 3 +++
tools/libxl/libxl_remus.c | 10 ++++++++++
3 files changed, 14 insertions(+), 1 deletion(-)
diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index 026206a..f45fd74 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -762,7 +762,7 @@ int libxl_domain_remus_start(libxl_ctx *ctx, libxl_domain_remus_info *info,
}
/* Point of no return */
- libxl__domain_suspend(egc, dss);
+ libxl__remus_setup_initiate(egc, dss);
return AO_INPROGRESS;
out:
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 358590b..657cfc4 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -2336,6 +2336,9 @@ _hidden void libxl__remus_teardown_done(libxl__egc *egc,
_hidden void libxl__remus_netbuf_teardown(libxl__egc *egc,
libxl__domain_suspend_state *dss);
+_hidden void libxl__remus_setup_initiate(libxl__egc *egc,
+ libxl__domain_suspend_state *dss);
+
struct libxl__domain_suspend_state {
/* set by caller of libxl__domain_suspend */
libxl__ao *ao;
diff --git a/tools/libxl/libxl_remus.c b/tools/libxl/libxl_remus.c
index 6790c61..38776e1 100644
--- a/tools/libxl/libxl_remus.c
+++ b/tools/libxl/libxl_remus.c
@@ -19,6 +19,16 @@
/*----- remus setup/teardown code -----*/
+void libxl__remus_setup_initiate(libxl__egc *egc,
+ libxl__domain_suspend_state *dss)
+{
+ libxl__ev_time_init(&dss->remus_state->timeout);
+ if (!dss->remus_state->netbufscript)
+ libxl__remus_setup_done(egc, dss, 0);
+ else
+ libxl__remus_netbuf_setup(egc, dss);
+}
+
void libxl__remus_setup_done(libxl__egc *egc,
libxl__domain_suspend_state *dss,
int rc)
--
1.8.4.2
^ permalink raw reply related [flat|nested] 26+ messages in thread* [PATCH 10/13 V6] remus: use the API to teardown network buffering
2014-01-21 9:05 [PATCH 00/13 V6] Remus/Libxl: Network buffering support Lai Jiangshan
` (8 preceding siblings ...)
2014-01-21 9:05 ` [PATCH 09/13 V6] remus: use the API to setup " Lai Jiangshan
@ 2014-01-21 9:05 ` Lai Jiangshan
2014-01-21 9:05 ` [PATCH 11/13 V6] remus: rename remus_failover_cb() to remus_replication_failure_cb() Lai Jiangshan
` (3 subsequent siblings)
13 siblings, 0 replies; 26+ messages in thread
From: Lai Jiangshan @ 2014-01-21 9:05 UTC (permalink / raw)
To: xen-devel
Cc: Ian Campbell, FNST-Wen Congyang, Stefano Stabellini,
Andrew Cooper, Jiang Yunhong, Ian Jackson, Lai Jiangshan,
Dong Eddie, Shriram Rajagopalan, Roger Pau Monne
From: Shriram Rajagopalan <rshriram@cs.ubc.ca>
If there is network buffering hotplug scripts, call
libxl__remus_netbuf_teardown() to teardown network
buffering.
Signed-off-by: Shriram Rajagopalan <rshriram@cs.ubc.ca>
Signed-off-by: Lai Jiangshan <laijs@cn.fujitsu.com>
Reviewed-by: Wen Congyang <wency@cn.fujitsu.com>
---
tools/libxl/libxl.c | 4 ----
tools/libxl/libxl_dom.c | 11 +++++++++++
tools/libxl/libxl_internal.h | 4 ++++
tools/libxl/libxl_remus.c | 13 +++++++++++++
4 files changed, 28 insertions(+), 4 deletions(-)
diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index f45fd74..83d3772 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -778,10 +778,6 @@ static void remus_failover_cb(libxl__egc *egc,
* backup died or some network error occurred preventing us
* from sending checkpoints.
*/
-
- /* TBD: Remus cleanup - i.e. detach qdisc, release other
- * resources.
- */
libxl__ao_complete(egc, ao, rc);
}
diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c
index e3e9f6f..912a6e4 100644
--- a/tools/libxl/libxl_dom.c
+++ b/tools/libxl/libxl_dom.c
@@ -1519,6 +1519,17 @@ void domain_suspend_done(libxl__egc *egc,
if (dss->xce != NULL)
xc_evtchn_close(dss->xce);
+ if (dss->remus_state) {
+ /*
+ * With Remus, if we reach this point, it means either
+ * backup died or some network error occurred preventing us
+ * from sending checkpoints. Teardown the network buffers and
+ * release netlink resources. This is an async op.
+ */
+ libxl__remus_teardown_initiate(egc, dss, rc);
+ return;
+ }
+
dss->callback(egc, dss, rc);
}
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 657cfc4..f818916 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -2339,6 +2339,10 @@ _hidden void libxl__remus_netbuf_teardown(libxl__egc *egc,
_hidden void libxl__remus_setup_initiate(libxl__egc *egc,
libxl__domain_suspend_state *dss);
+_hidden void libxl__remus_teardown_initiate(libxl__egc *egc,
+ libxl__domain_suspend_state *dss,
+ int rc);
+
struct libxl__domain_suspend_state {
/* set by caller of libxl__domain_suspend */
libxl__ao *ao;
diff --git a/tools/libxl/libxl_remus.c b/tools/libxl/libxl_remus.c
index 38776e1..0d281a0 100644
--- a/tools/libxl/libxl_remus.c
+++ b/tools/libxl/libxl_remus.c
@@ -44,6 +44,19 @@ void libxl__remus_setup_done(libxl__egc *egc,
domain_suspend_done(egc, dss, rc);
}
+void libxl__remus_teardown_initiate(libxl__egc *egc,
+ libxl__domain_suspend_state *dss,
+ int rc)
+{
+ /* stash rc somewhere before invoking teardown ops. */
+ dss->remus_state->saved_rc = rc;
+
+ if (!dss->remus_state->netbuf_state)
+ libxl__remus_teardown_done(egc, dss);
+ else
+ libxl__remus_netbuf_teardown(egc, dss);
+}
+
void libxl__remus_teardown_done(libxl__egc *egc,
libxl__domain_suspend_state *dss)
{
--
1.8.4.2
^ permalink raw reply related [flat|nested] 26+ messages in thread* [PATCH 11/13 V6] remus: rename remus_failover_cb() to remus_replication_failure_cb()
2014-01-21 9:05 [PATCH 00/13 V6] Remus/Libxl: Network buffering support Lai Jiangshan
` (9 preceding siblings ...)
2014-01-21 9:05 ` [PATCH 10/13 V6] remus: use the API to teardown " Lai Jiangshan
@ 2014-01-21 9:05 ` Lai Jiangshan
2014-01-21 9:05 ` [PATCH 12/13 V6] tools/libxl: control network buffering in remus callbacks Lai Jiangshan
` (2 subsequent siblings)
13 siblings, 0 replies; 26+ messages in thread
From: Lai Jiangshan @ 2014-01-21 9:05 UTC (permalink / raw)
To: xen-devel
Cc: Ian Campbell, FNST-Wen Congyang, Stefano Stabellini,
Andrew Cooper, Jiang Yunhong, Ian Jackson, Lai Jiangshan,
Dong Eddie, Shriram Rajagopalan, Roger Pau Monne
From: Shriram Rajagopalan <rshriram@cs.ubc.ca>
Failover means that: the machine on which primary vm is running is
down, and we need to start the secondary vm to take over the primary
vm. remus_failover_cb() is called when remus fails, not when we need
to do failover. So rename it to remus_replication_failure_cb()
Signed-off-by: Shriram Rajagopalan <rshriram@cs.ubc.ca>
Signed-off-by: Lai Jiangshan <laijs@cn.fujitsu.com>
Reviewed-by: Wen Congyang <wency@cn.fujitsu.com>
---
tools/libxl/libxl.c | 12 +++++++-----
1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index 83d3772..70e34c0 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -702,8 +702,9 @@ out:
return ptr;
}
-static void remus_failover_cb(libxl__egc *egc,
- libxl__domain_suspend_state *dss, int rc);
+static void remus_replication_failure_cb(libxl__egc *egc,
+ libxl__domain_suspend_state *dss,
+ int rc);
/* TODO: Explicit Checkpoint acknowledgements via recv_fd. */
int libxl_domain_remus_start(libxl_ctx *ctx, libxl_domain_remus_info *info,
@@ -722,7 +723,7 @@ int libxl_domain_remus_start(libxl_ctx *ctx, libxl_domain_remus_info *info,
GCNEW(dss);
dss->ao = ao;
- dss->callback = remus_failover_cb;
+ dss->callback = remus_replication_failure_cb;
dss->domid = domid;
dss->fd = send_fd;
/* TODO do something with recv_fd */
@@ -769,8 +770,9 @@ int libxl_domain_remus_start(libxl_ctx *ctx, libxl_domain_remus_info *info,
return AO_ABORT(rc);
}
-static void remus_failover_cb(libxl__egc *egc,
- libxl__domain_suspend_state *dss, int rc)
+static void remus_replication_failure_cb(libxl__egc *egc,
+ libxl__domain_suspend_state *dss,
+ int rc)
{
STATE_AO_GC(dss->ao);
/*
--
1.8.4.2
^ permalink raw reply related [flat|nested] 26+ messages in thread* [PATCH 12/13 V6] tools/libxl: control network buffering in remus callbacks
2014-01-21 9:05 [PATCH 00/13 V6] Remus/Libxl: Network buffering support Lai Jiangshan
` (10 preceding siblings ...)
2014-01-21 9:05 ` [PATCH 11/13 V6] remus: rename remus_failover_cb() to remus_replication_failure_cb() Lai Jiangshan
@ 2014-01-21 9:05 ` Lai Jiangshan
2014-01-21 9:05 ` [PATCH 13/13 V6] tools/libxl: network buffering cmdline switch Lai Jiangshan
2014-01-24 10:14 ` [PATCH 00/13 V6] Remus/Libxl: Network buffering support Lai Jiangshan
13 siblings, 0 replies; 26+ messages in thread
From: Lai Jiangshan @ 2014-01-21 9:05 UTC (permalink / raw)
To: xen-devel
Cc: Ian Campbell, FNST-Wen Congyang, Stefano Stabellini,
Andrew Cooper, Jiang Yunhong, Ian Jackson, Lai Jiangshan,
Dong Eddie, Shriram Rajagopalan, Roger Pau Monne
From: Shriram Rajagopalan <rshriram@cs.ubc.ca>
This patch constitutes the core network buffering logic.
and does the following:
a) create a new network buffer when the domain is suspended
(remus_domain_suspend_callback)
b) release the previous network buffer pertaining to the
committed checkpoint (remus_domain_checkpoint_dm_saved)
Signed-off-by: Shriram Rajagopalan <rshriram@cs.ubc.ca>
Signed-off-by: Lai Jiangshan <laijs@cn.fujitsu.com>
Reviewed-by: Wen Congyang <wency@cn.fujitsu.com>
---
tools/libxl/libxl_dom.c | 90 ++++++++++++++++++++++++++++++++++++++++++++-----
1 file changed, 82 insertions(+), 8 deletions(-)
diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c
index 912a6e4..a4ffdfd 100644
--- a/tools/libxl/libxl_dom.c
+++ b/tools/libxl/libxl_dom.c
@@ -1243,8 +1243,30 @@ int libxl__toolstack_save(uint32_t domid, uint8_t **buf,
static int libxl__remus_domain_suspend_callback(void *data)
{
- /* REMUS TODO: Issue disk and network checkpoint reqs. */
- return libxl__domain_suspend_common_callback(data);
+ libxl__save_helper_state *shs = data;
+ libxl__domain_suspend_state *dss = CONTAINER_OF(shs, *dss, shs);
+
+ /* Convenience aliases */
+ libxl__remus_state *const remus_state = dss->remus_state;
+
+ STATE_AO_GC(dss->ao);
+
+ /* REMUS TODO: Issue disk checkpoint reqs. */
+ int ok = libxl__domain_suspend_common_callback(data);
+
+ if (!remus_state->netbuf_state || !ok) goto out;
+
+ /* The domain was suspended successfully. Start a new network
+ * buffer for the next epoch. If this operation fails, then act
+ * as though domain suspend failed -- libxc exits its infinite
+ * loop and ultimately, the replication stops.
+ */
+ if (libxl__remus_netbuf_start_new_epoch(gc, dss->domid,
+ remus_state))
+ ok = 0;
+
+ out:
+ return ok;
}
static int libxl__remus_domain_resume_callback(void *data)
@@ -1257,7 +1279,7 @@ static int libxl__remus_domain_resume_callback(void *data)
if (libxl__domain_resume(gc, dss->domid, /* Fast Suspend */1))
return 0;
- /* REMUS TODO: Deal with disk. Start a new network output buffer */
+ /* REMUS TODO: Deal with disk. */
return 1;
}
@@ -1266,11 +1288,17 @@ static int libxl__remus_domain_resume_callback(void *data)
static void remus_checkpoint_dm_saved(libxl__egc *egc,
libxl__domain_suspend_state *dss, int rc);
+static void remus_next_checkpoint(libxl__egc *egc, libxl__ev_time *ev,
+ const struct timeval *requested_abs);
+
static void libxl__remus_domain_checkpoint_callback(void *data)
{
libxl__save_helper_state *shs = data;
libxl__domain_suspend_state *dss = CONTAINER_OF(shs, *dss, shs);
- libxl__egc *egc = dss->shs.egc;
+
+ /* Convenience aliases */
+ libxl__egc *const egc = dss->shs.egc;
+
STATE_AO_GC(dss->ao);
/* This would go into tailbuf. */
@@ -1284,10 +1312,56 @@ static void libxl__remus_domain_checkpoint_callback(void *data)
static void remus_checkpoint_dm_saved(libxl__egc *egc,
libxl__domain_suspend_state *dss, int rc)
{
- /* REMUS TODO: Wait for disk and memory ack, release network buffer */
- /* REMUS TODO: make this asynchronous */
- assert(!rc); /* REMUS TODO handle this error properly */
- usleep(dss->remus_state->interval * 1000);
+ /* Convenience aliases */
+ /*
+ * REMUS TODO: Wait for disk and explicit memory ack (through restore
+ * callback from remote) before releasing network buffer.
+ */
+ libxl__remus_state *const remus_state = dss->remus_state;
+
+ STATE_AO_GC(dss->ao);
+
+ if (rc) {
+ LOG(ERROR, "Failed to save device model. Terminating Remus..");
+ goto out;
+ }
+
+ if (remus_state->netbuf_state) {
+ rc = libxl__remus_netbuf_release_prev_epoch(gc, dss->domid,
+ remus_state);
+ if (rc) {
+ LOG(ERROR, "Failed to release network buffer."
+ " Terminating Remus..");
+ goto out;
+ }
+ }
+
+ /* Set checkpoint interval timeout */
+ rc = libxl__ev_time_register_rel(gc, &remus_state->timeout,
+ remus_next_checkpoint,
+ dss->remus_state->interval);
+ if (rc) {
+ LOG(ERROR, "unable to register timeout for next epoch."
+ " Terminating Remus..");
+ goto out;
+ }
+ return;
+
+ out:
+ libxl__xc_domain_saverestore_async_callback_done(egc, &dss->shs, 0);
+}
+
+static void remus_next_checkpoint(libxl__egc *egc, libxl__ev_time *ev,
+ const struct timeval *requested_abs)
+{
+ libxl__remus_state *remus_state = CONTAINER_OF(ev, *remus_state, timeout);
+
+ /* Convenience aliases */
+ libxl__domain_suspend_state *const dss = remus_state->dss;
+
+ STATE_AO_GC(dss->ao);
+
+ libxl__ev_time_deregister(gc, &remus_state->timeout);
libxl__xc_domain_saverestore_async_callback_done(egc, &dss->shs, 1);
}
--
1.8.4.2
^ permalink raw reply related [flat|nested] 26+ messages in thread* [PATCH 13/13 V6] tools/libxl: network buffering cmdline switch
2014-01-21 9:05 [PATCH 00/13 V6] Remus/Libxl: Network buffering support Lai Jiangshan
` (11 preceding siblings ...)
2014-01-21 9:05 ` [PATCH 12/13 V6] tools/libxl: control network buffering in remus callbacks Lai Jiangshan
@ 2014-01-21 9:05 ` Lai Jiangshan
2014-01-26 22:31 ` Shriram Rajagopalan
2014-01-24 10:14 ` [PATCH 00/13 V6] Remus/Libxl: Network buffering support Lai Jiangshan
13 siblings, 1 reply; 26+ messages in thread
From: Lai Jiangshan @ 2014-01-21 9:05 UTC (permalink / raw)
To: xen-devel
Cc: Ian Campbell, FNST-Wen Congyang, Stefano Stabellini,
Andrew Cooper, Jiang Yunhong, Ian Jackson, Lai Jiangshan,
Dong Eddie, Shriram Rajagopalan, Roger Pau Monne
From: Shriram Rajagopalan <rshriram@cs.ubc.ca>
Command line switch to 'xl remus' command, to enable network buffering.
Pass on this flag to libxl so that it can act accordingly.
Also update man pages to reflect the addition of a new option to
'xl remus' command.
Note: the network buffering is enabled as default. If you want to
disable it, please use -n option.
Signed-off-by: Shriram Rajagopalan <rshriram@cs.ubc.ca>
Signed-off-by: Lai Jiangshan <laijs@cn.fujitsu.com>
Reviewed-by: Wen Congyang <wency@cn.fujitsu.com>
---
docs/man/xl.conf.pod.5 | 6 ++++++
docs/man/xl.pod.1 | 11 ++++++++++-
tools/libxl/xl.c | 4 ++++
tools/libxl/xl.h | 1 +
tools/libxl/xl_cmdimpl.c | 28 ++++++++++++++++++++++------
tools/libxl/xl_cmdtable.c | 3 +++
6 files changed, 46 insertions(+), 7 deletions(-)
diff --git a/docs/man/xl.conf.pod.5 b/docs/man/xl.conf.pod.5
index 7c43bde..8ae19bb 100644
--- a/docs/man/xl.conf.pod.5
+++ b/docs/man/xl.conf.pod.5
@@ -105,6 +105,12 @@ Configures the default gateway device to set for virtual network devices.
Default: C<None>
+=item B<remus.default.netbufscript="PATH">
+
+Configures the default script used by Remus to setup network buffering.
+
+Default: C</etc/xen/scripts/remus-netbuf-setup>
+
=item B<output_format="json|sxp">
Configures the default output format used by xl when printing "machine
diff --git a/docs/man/xl.pod.1 b/docs/man/xl.pod.1
index e7b9de2..3c5f246 100644
--- a/docs/man/xl.pod.1
+++ b/docs/man/xl.pod.1
@@ -399,7 +399,7 @@ Enable Remus HA for domain. By default B<xl> relies on ssh as a transport
mechanism between the two hosts.
N.B: Remus support in xl is still in experimental (proof-of-concept) phase.
- There is no support for network or disk buffering at the moment.
+ There is no support for disk buffering at the moment.
B<OPTIONS>
@@ -418,6 +418,15 @@ Generally useful for debugging.
Disable memory checkpoint compression.
+=item B<-n>
+
+Disable network output buffering.
+
+=item B<-N> I<netbufscript>
+
+Use <netbufscript> to setup network buffering instead of the instead of
+the default (/etc/xen/scripts/remus-netbuf-setup).
+
=item B<-s> I<sshcommand>
Use <sshcommand> instead of ssh. String will be passed to sh.
diff --git a/tools/libxl/xl.c b/tools/libxl/xl.c
index 657610b..e02a618 100644
--- a/tools/libxl/xl.c
+++ b/tools/libxl/xl.c
@@ -46,6 +46,7 @@ char *default_vifscript = NULL;
char *default_bridge = NULL;
char *default_gatewaydev = NULL;
char *default_vifbackend = NULL;
+char *default_remus_netbufscript = NULL;
enum output_format default_output_format = OUTPUT_FORMAT_JSON;
int claim_mode = 1;
@@ -177,6 +178,9 @@ static void parse_global_config(const char *configfile,
if (!xlu_cfg_get_long (config, "claim_mode", &l, 0))
claim_mode = l;
+ xlu_cfg_replace_string (config, "remus.default.netbufscript",
+ &default_remus_netbufscript, 0);
+
xlu_cfg_destroy(config);
}
diff --git a/tools/libxl/xl.h b/tools/libxl/xl.h
index c876a33..d991fd3 100644
--- a/tools/libxl/xl.h
+++ b/tools/libxl/xl.h
@@ -153,6 +153,7 @@ extern char *default_vifscript;
extern char *default_bridge;
extern char *default_gatewaydev;
extern char *default_vifbackend;
+extern char *default_remus_netbufscript;
extern char *blkdev_start;
enum output_format {
diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index d93e01b..4145543 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -7263,8 +7263,9 @@ int main_remus(int argc, char **argv)
r_info.interval = 200;
r_info.blackhole = 0;
r_info.compression = 1;
+ r_info.netbuf = 1;
- SWITCH_FOREACH_OPT(opt, "bui:s:e", NULL, "remus", 2) {
+ SWITCH_FOREACH_OPT(opt, "buni:s:N:e", NULL, "remus", 2) {
case 'i':
r_info.interval = atoi(optarg);
break;
@@ -7274,6 +7275,12 @@ int main_remus(int argc, char **argv)
case 'u':
r_info.compression = 0;
break;
+ case 'n':
+ r_info.netbuf = 0;
+ break;
+ case 'N':
+ r_info.netbufscript = optarg;
+ break;
case 's':
ssh_command = optarg;
break;
@@ -7285,6 +7292,9 @@ int main_remus(int argc, char **argv)
domid = find_domain(argv[optind]);
host = argv[optind + 1];
+ if(!r_info.netbufscript)
+ r_info.netbufscript = default_remus_netbufscript;
+
if (r_info.blackhole) {
send_fd = open("/dev/null", O_RDWR, 0644);
if (send_fd < 0) {
@@ -7322,13 +7332,19 @@ int main_remus(int argc, char **argv)
/* Point of no return */
rc = libxl_domain_remus_start(ctx, &r_info, domid, send_fd, recv_fd, 0);
- /* If we are here, it means backup has failed/domain suspend failed.
- * Try to resume the domain and exit gracefully.
- * TODO: Split-Brain check.
+ /* check if the domain exists. User may have xl destroyed the
+ * domain to force failover
*/
- fprintf(stderr, "remus sender: libxl_domain_suspend failed"
- " (rc=%d)\n", rc);
+ if (libxl_domain_info(ctx, 0, domid)) {
+ fprintf(stderr, "Remus: Primary domain has been destroyed.\n");
+ close(send_fd);
+ return 0;
+ }
+ /* If we are here, it means remus setup/domain suspend/backup has
+ * failed. Try to resume the domain and exit gracefully.
+ * TODO: Split-Brain check.
+ */
if (rc == ERROR_GUEST_TIMEDOUT)
fprintf(stderr, "Failed to suspend domain at primary.\n");
else {
diff --git a/tools/libxl/xl_cmdtable.c b/tools/libxl/xl_cmdtable.c
index ebe0220..f05e07b 100644
--- a/tools/libxl/xl_cmdtable.c
+++ b/tools/libxl/xl_cmdtable.c
@@ -481,6 +481,9 @@ struct cmd_spec cmd_table[] = {
"-i MS Checkpoint domain memory every MS milliseconds (def. 200ms).\n"
"-b Replicate memory checkpoints to /dev/null (blackhole)\n"
"-u Disable memory checkpoint compression.\n"
+ "-n Enable network output buffering.\n"
+ "-N <netbufscript> Use netbufscript to setup network buffering instead of the\n"
+ " instead of the default (/etc/xen/scripts/remus-netbuf-setup).\n"
"-s <sshcommand> Use <sshcommand> instead of ssh. String will be passed\n"
" to sh. If empty, run <host> instead of \n"
" ssh <host> xl migrate-receive -r [-e]\n"
--
1.8.4.2
^ permalink raw reply related [flat|nested] 26+ messages in thread* Re: [PATCH 13/13 V6] tools/libxl: network buffering cmdline switch
2014-01-21 9:05 ` [PATCH 13/13 V6] tools/libxl: network buffering cmdline switch Lai Jiangshan
@ 2014-01-26 22:31 ` Shriram Rajagopalan
2014-01-27 6:58 ` Wen Congyang
0 siblings, 1 reply; 26+ messages in thread
From: Shriram Rajagopalan @ 2014-01-26 22:31 UTC (permalink / raw)
To: Lai Jiangshan
Cc: Ian Campbell, FNST-Wen Congyang, Stefano Stabellini,
Andrew Cooper, Jiang Yunhong, Ian Jackson,
xen-devel@lists.xen.org, Dong Eddie, Roger Pau Monne
[-- Attachment #1.1: Type: text/plain, Size: 1119 bytes --]
On Tue, Jan 21, 2014 at 1:05 AM, Lai Jiangshan <laijs@cn.fujitsu.com> wrote:
> From: Shriram Rajagopalan <rshriram@cs.ubc.ca>
>
> Command line switch to 'xl remus' command, to enable network buffering.
> Pass on this flag to libxl so that it can act accordingly.
> Also update man pages to reflect the addition of a new option to
> 'xl remus' command.
>
> Note: the network buffering is enabled as default. If you want to
> disable it, please use -n option.
>
>
> diff --git a/tools/libxl/xl_cmdtable.c b/tools/libxl/xl_cmdtable.c
> index ebe0220..f05e07b 100644
> --- a/tools/libxl/xl_cmdtable.c
> +++ b/tools/libxl/xl_cmdtable.c
> @@ -481,6 +481,9 @@ struct cmd_spec cmd_table[] = {
> "-i MS Checkpoint domain memory every MS
> milliseconds (def. 200ms).\n"
> "-b Replicate memory checkpoints to /dev/null
> (blackhole)\n"
> "-u Disable memory checkpoint compression.\n"
> + "-n Enable network output buffering.\n"
>
Since network buffering is enabled by default, this should be "Disable
network output..".
[-- Attachment #1.2: Type: text/html, Size: 1761 bytes --]
[-- Attachment #2: Type: text/plain, Size: 126 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [PATCH 13/13 V6] tools/libxl: network buffering cmdline switch
2014-01-26 22:31 ` Shriram Rajagopalan
@ 2014-01-27 6:58 ` Wen Congyang
0 siblings, 0 replies; 26+ messages in thread
From: Wen Congyang @ 2014-01-27 6:58 UTC (permalink / raw)
To: rshriram, Lai Jiangshan
Cc: Ian Campbell, Stefano Stabellini, Andrew Cooper, Jiang Yunhong,
Ian Jackson, xen-devel@lists.xen.org, Dong Eddie, Roger Pau Monne
At 01/27/2014 06:31 AM, Shriram Rajagopalan Wrote:
> On Tue, Jan 21, 2014 at 1:05 AM, Lai Jiangshan <laijs@cn.fujitsu.com> wrote:
>
>> From: Shriram Rajagopalan <rshriram@cs.ubc.ca>
>>
>> Command line switch to 'xl remus' command, to enable network buffering.
>> Pass on this flag to libxl so that it can act accordingly.
>> Also update man pages to reflect the addition of a new option to
>> 'xl remus' command.
>>
>> Note: the network buffering is enabled as default. If you want to
>> disable it, please use -n option.
>>
>>
>
>> diff --git a/tools/libxl/xl_cmdtable.c b/tools/libxl/xl_cmdtable.c
>> index ebe0220..f05e07b 100644
>> --- a/tools/libxl/xl_cmdtable.c
>> +++ b/tools/libxl/xl_cmdtable.c
>> @@ -481,6 +481,9 @@ struct cmd_spec cmd_table[] = {
>> "-i MS Checkpoint domain memory every MS
>> milliseconds (def. 200ms).\n"
>> "-b Replicate memory checkpoints to /dev/null
>> (blackhole)\n"
>> "-u Disable memory checkpoint compression.\n"
>> + "-n Enable network output buffering.\n"
>>
>
> Since network buffering is enabled by default, this should be "Disable
> network output..".
Yes, we forgot to update this description.
Thanks
Wen Congyang
>
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel
>
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 00/13 V6] Remus/Libxl: Network buffering support
2014-01-21 9:05 [PATCH 00/13 V6] Remus/Libxl: Network buffering support Lai Jiangshan
` (12 preceding siblings ...)
2014-01-21 9:05 ` [PATCH 13/13 V6] tools/libxl: network buffering cmdline switch Lai Jiangshan
@ 2014-01-24 10:14 ` Lai Jiangshan
2014-01-24 10:20 ` Ian Campbell
13 siblings, 1 reply; 26+ messages in thread
From: Lai Jiangshan @ 2014-01-24 10:14 UTC (permalink / raw)
To: Lai Jiangshan
Cc: Ian Campbell, FNST-Wen Congyang, Stefano Stabellini,
Andrew Cooper, Jiang Yunhong, Ian Jackson, xen-devel, Dong Eddie,
Shriram Rajagopalan, Roger Pau Monne
On 01/21/2014 05:05 PM, Lai Jiangshan wrote:
>>
>
> Changes in V6:
> Applied Ian Jackson's comments of V5 series.
> the [PATCH 2/4 V5] is split by small functionalities.
>
> [PATCH 4/4 V5] --> [PATCH 13/13] netbuffer is default enabled.
>
Ping!
Ian Jackson, any suggestion?
Shriram, could you review it?
thanks,
Lai
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [PATCH 00/13 V6] Remus/Libxl: Network buffering support
2014-01-24 10:14 ` [PATCH 00/13 V6] Remus/Libxl: Network buffering support Lai Jiangshan
@ 2014-01-24 10:20 ` Ian Campbell
2014-01-26 22:26 ` Shriram Rajagopalan
0 siblings, 1 reply; 26+ messages in thread
From: Ian Campbell @ 2014-01-24 10:20 UTC (permalink / raw)
To: Lai Jiangshan
Cc: Dong Eddie, FNST-Wen Congyang, Stefano Stabellini, Andrew Cooper,
Jiang Yunhong, Ian Jackson, xen-devel, Shriram Rajagopalan,
Roger Pau Monne
On Fri, 2014-01-24 at 18:14 +0800, Lai Jiangshan wrote:
> On 01/21/2014 05:05 PM, Lai Jiangshan wrote:
>
> >>
> >
> > Changes in V6:
> > Applied Ian Jackson's comments of V5 series.
> > the [PATCH 2/4 V5] is split by small functionalities.
> >
> > [PATCH 4/4 V5] --> [PATCH 13/13] netbuffer is default enabled.
> >
>
> Ping!
This is targeting 4.5 I think? I'm afraid that this means it is likely
to be queued behind anything targeting 4.4 as far as review bandwidth
goes.
Ian.
>
> Ian Jackson, any suggestion?
>
> Shriram, could you review it?
>
> thanks,
> Lai
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 00/13 V6] Remus/Libxl: Network buffering support
2014-01-24 10:20 ` Ian Campbell
@ 2014-01-26 22:26 ` Shriram Rajagopalan
0 siblings, 0 replies; 26+ messages in thread
From: Shriram Rajagopalan @ 2014-01-26 22:26 UTC (permalink / raw)
To: Lai Jiangshan
Cc: Ian Campbell, FNST-Wen Congyang, Stefano Stabellini,
Andrew Cooper, Jiang Yunhong, Ian Jackson,
xen-devel@lists.xen.org, Dong Eddie, Roger Pau Monne
[-- Attachment #1.1: Type: text/plain, Size: 894 bytes --]
On Fri, Jan 24, 2014 at 2:20 AM, Ian Campbell <Ian.Campbell@citrix.com>wrote:
> On Fri, 2014-01-24 at 18:14 +0800, Lai Jiangshan wrote:
> > On 01/21/2014 05:05 PM, Lai Jiangshan wrote:
> >
> > >>
> > >
> > > Changes in V6:
> > > Applied Ian Jackson's comments of V5 series.
> > > the [PATCH 2/4 V5] is split by small functionalities.
> > >
> > > [PATCH 4/4 V5] --> [PATCH 13/13] netbuffer is default enabled.
> > >
> >
> > Ping!
>
> This is targeting 4.5 I think? I'm afraid that this means it is likely
> to be queued behind anything targeting 4.4 as far as review bandwidth
> goes.
>
> Ian.
>
> >
> > Ian Jackson, any suggestion?
> >
> > Shriram, could you review it?
> >
> > thanks,
> > Lai
>
>
>
Hi
Thanks for splitting up the patch into smaller ones and incorporating all
the feedback.
Most of the series is fine. I have some minor feedback related to patches
2, 6 & 13.
Shriram
[-- Attachment #1.2: Type: text/html, Size: 1541 bytes --]
[-- Attachment #2: Type: text/plain, Size: 126 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 26+ messages in thread