oxenstored fixes -- fixes recent pvops kernel hang
PATCH 1/1 oxenstored fixes -- fixes recent pvops kernel hang
Currently PVHVM Linux guests after ddacf5ef684a "xen/pv-on-hvm kexec:
add xs_reset_watches to shutdown watches from old kernel" hang when run against oxenstored because it does not handle the unknown XS_RESET_WATCHES operation and does not reply.
The symptom of this issue is a hang during boot at this point:
cpu 1 spinlock event irq 70
CPU 1 irqstacks, hard=dec94000 soft=dec96000
Booting Node 0, Processors #1
smpboot cpu 1: start_ip = 99000
Initializing CPU#1
installing Xen timer for CPU 1
Brought up 2 CPUs
Total of 2 processors activated (9625.99 BogoMIPS).
NET: Registered protocol family 16
<HANG>
This series makes oxenstored handle unknown operations by returning an error indicating that the operation is unknown. I have not actually implemented support for XS_RESET_WATCHES.
Two other small fixlets are included.
Lastly I include a patch which I've been using for some time to enable the use of oxenstored in preference to C xenstored when available.
PATCH 1/1 oxenstored: Remove support for PQ defined operations
# HG changeset patch
# User Ian Campbell <[email protected]> # Date 1323447576 0
# Node ID beaf8eb93c36062b395bab3a30e3cf9d1f335a73 # Parent 3429972bbc6a1b998b13ba38f449fbb8251b3717
oxenstored: Remove support for PQ defined operations.
It is unused.
tools/ocaml/libs/xb/op.ml +0 -13
@@ -22,9 +22,6 @@ type operation = Debug | Directory | Rea Resume | Set_target | Restrict -(* There are two sets of XB operations: the one coming from open-source and *) -(* the one coming from our private patch queue. These operations *) -(* in two differents arrays for make easier the forward compatibility *) let operation_c_mapping = [| Debug; Directory; Read; Getperms; Watch; Unwatch; Transaction_start;
@@ -34,12 +31,6 @@ let operation_c_mapping = Resume; Set_target; Restrict |] let size = Array.length operation_c_mapping -(* [offset_pq] has to be the same as in <xen/io/xs_wire.h> *) -let offset_pq = size -let operation_c_mapping_pq = - [| |] -let size_pq = Array.length operation_c_mapping_pq - let array_search el a = let len = Array.length a in let rec search i =
@@ -50,14 +41,10 @@ let array_search el a = let of_cval i = if i >= 0 && i < size then operation_c_mapping.(i) - else if i >= offset_pq && i < offset_pq + size_pq - then operation_c_mapping_pq.(i-offset_pq) else raise Not_found let to_cval op = - try array_search op operation_c_mapping - with _ -> offset_pq + array_search op operation_c_mapping_pq let to_string ty = match ty with
tools/ocaml/libs/xb/xb.mli +0 -3
@@ -25,9 +25,6 @@ module Op : | Restrict val operation_c_mapping : operation array val size : int - val offset_pq : int - val operation_c_mapping_pq : 'a array - val size_pq : int val array_search : 'a -> 'a array -> int val of_cval : int -> operation val to_cval : operation -> int
PATCH 1/1 oxenstored: log Errors and Warnings by default
In reply to Ian Campbell# HG changeset patch
# User Ian Campbell <[email protected]> # Date 1323447576 0
# Node ID 18c0807a0504791bcb6e083866def9c4862a119a # Parent beaf8eb93c36062b395bab3a30e3cf9d1f335a73
oxenstored: log Errors and Warnings by default.
tools/ocaml/xenstored/logging.ml +1 -1
@@ -105,7 +105,7 @@ let string_of_date () = (int_of_float (1000.0 *. msec)) let xenstored_log_file = ref "/var/log/xenstored.log" -let xenstored_log_level = ref Null +let xenstored_log_level = ref Warn let xenstored_log_nb_files = ref 10 let xenstored_log_nb_lines = ref 13215 let xenstored_log_nb_chars = ref (-1)
PATCH 1/1 oxenstored: handle unknown operations by returning an error to the client
In reply to Ian Campbell# HG changeset patch
# User Ian Campbell <[email protected]>
# Date 1323448556 0
# Node ID 74f94e15bfe1dad412d0342aeabdbd895657f54f
# Parent 18c0807a0504791bcb6e083866def9c4862a119a
oxenstored: handle unknown operations by returning an error to the client
Previous an unknown operation would be decoded as a Not_found exception which would bubble all the way up to the try ... with surrounding the call to main_loop where it would be logged and ignored.
This would leave the guest hanging waiting for a response to the invalid request.
Instead introduce a specific "Invalid" operation. Higher level functionality, such as Process.process_packet, already handles operations which are not understood with an error reply due to the final wildcard entry in Process.function_of_type but explicitly handle Invalid this way to make it clear what is going on.
tools/ocaml/libs/xb/op.ml +3 -3
@@ -19,8 +19,7 @@ type operation = Debug | Directory | Rea Transaction_end | Introduce | Release | Getdomainpath | Write | Mkdir | Rm | Setperms | Watchevent | Error | Isintroduced | - Resume | Set_target - | Restrict + Resume | Set_target | Restrict | Invalid let operation_c_mapping = [| Debug; Directory; Read; Getperms;
@@ -41,7 +40,7 @@ let array_search el a = let of_cval i = if i >= 0 && i < size then operation_c_mapping.(i) - else raise Not_found + else Invalid let to_cval op = array_search op operation_c_mapping
@@ -69,3 +68,4 @@ let to_string ty = | Resume -> "RESUME" | Set_target -> "SET_TARGET" | Restrict -> "RESTRICT" + | Invalid -> "INVALID"
tools/ocaml/libs/xb/xb.mli +1 -0
@@ -23,6 +23,7 @@ module Op : | Resume | Set_target | Restrict + | Invalid (* Not a valid wire operation *) val operation_c_mapping : operation array val size : int val array_search : 'a -> 'a array -> int
tools/ocaml/xenstored/process.ml +2 -1
@@ -324,7 +324,8 @@ let function_of_type ty = | Xenbus.Xb.Op.Resume -> reply_ack do_resume | Xenbus.Xb.Op.Set_target -> reply_ack do_set_target | Xenbus.Xb.Op.Restrict -> reply_ack do_restrict - | _ -> reply_ack do_error + | Xenbus.Xb.Op.Invalid -> reply_ack do_error + | _ -> reply_ack do_error let input_handle_error ~cons ~doms ~fct ~ty ~con ~t ~rid ~data = let reply_error e =
tools/ocaml/xenstored/xenstored.ml +1 -3
@@ -43,9 +43,7 @@ let process_connection_fds store cons do debug "closing socket connection" in let process_fdset_with fds fct = - List.iter (fun fd -> - try try_fct fct (Connections.find cons fd) - with Not_found -> ()) fds + List.iter (fun fd -> try_fct fct (Connections.find cons fd)) fds in process_fdset_with rset Process.do_input; process_fdset_with wset Process.do_output
Re: [PATCH 3 of 4] oxenstored: handle unknown operations by returning an error to the client
Ian Campbell writes ("[Xen-devel] [PATCH 3 of 4] oxenstored: handle unknown operations by returning an error to the client"):
Quoted 1 line
oxenstored: handle unknown operations by returning an error to the client
This introduces a new warning:
File "logging.ml", line 157, characters 14-1067:
Warning P: this pattern-matching is not exhaustive. Here is an example of a value that is not matched:
Invalid
I have applied 1/4 and 2/4.
Ian.
Re: [PATCH 3 of 4] oxenstored: handle unknown operations by returning an error to the client
Quoted 9 lines · Ian Jackson
Ian Campbell writes ("[Xen-devel] [PATCH 3 of 4] oxenstored: handle unknown operations by returning an error to the client"):
Quoted 1 line
oxenstored: handle unknown operations by returning an error to the client
This introduces a new warning:
File "logging.ml", line 157, characters 14-1067:
Warning P: this pattern-matching is not exhaustive. Here is an example of a value that is not matched:
Invalid
Sorry, will fix and resend.
Quoted 1 line
I have applied 1/4 and 2/4.
Did you see "5/4" and "6/4"? In any case I shall include them in my resend.
Ian.
PATCH 1/1 Linux/xencommons: Use oxenstored by default when available
In reply to Ian Campbell# HG changeset patch
# User Ian Campbell <[email protected]>
# Date 1323448636 0
# Node ID 3b7ac401f144206c30440fbb41c74b13fa20b8cb # Parent 74f94e15bfe1dad412d0342aeabdbd895657f54f Linux/xencommons: Use oxenstored by default when available
oxenstored is an ocaml implementation of the xenstore daemon. It is faster, more scalable and more reliable than the C xenstored.
In particular the transaction model in oxenstored does not involve taking a complete copy of the database and aborting on any (even non-conflicting) other change.
There is a paper on the design and implementation of oxenstored at
http://gazagnaire.org/pub/GH09.pdf which includes a performance evaluation and comparison with the C daemon etc.
tools/hotplug/Linux/init.d/sysconfig.xencommons +3 -0
@@ -1,6 +1,9 @@ # Log xenconsoled messages (cf xl dmesg) XENCONSOLED_TRACE=guest +# Select xenstored implementation +#XENSTORED=[oxenstored|xenstored] + # Log xenstored messages #XENSTORED_TRACE=[yes|on|1]
tools/hotplug/Linux/init.d/xencommons +13 -2
@@ -70,8 +70,19 @@ do_start () { rm -f "$XENSTORED_ROOTDIR"/tdb* &>/dev/null test -z "$XENSTORED_TRACE" || XENSTORED_ARGS=" -T /var/log/xen/xenstored-trace.log" - echo -n Starting xenstored... - xenstored --pid-file=/var/run/xenstored.pid $XENSTORED_ARGS + if [ -n "$XENSTORED" ] ; then + echo -n Starting $XENSTORED... + $XENSTORED --pid-file /var/run/xenstored.pid $XENSTORED_ARGS + elif [ -x /usr/sbin/oxenstored ] ; then + echo -n Starting oxenstored... + /usr/sbin/oxenstored --pid-file /var/run/xenstored.pid $XENSTORED_ARGS + elif [ -x /usr/sbin/xenstored ] ; then + echo -n Starting C xenstored... + /usr/sbin/xenstored --pid-file /var/run/xenstored.pid $XENSTORED_ARGS + else + echo "No xenstored found" + exit 1 + fi # Wait for xenstored to actually come up, timing out after 30 seconds while [ $time -lt $timeout ] && ! `xenstore-read -s / >/dev/null 2>&1` ; do
Re: [PATCH 4 of 4] Linux/xencommons: Use oxenstored by default when available
Quoted 6 lines · Ian Campbell
# HG changeset patch
# User Ian Campbell <[email protected]>
# Date 1323448636 0
# Node ID3b7ac401f144206c30440fbb41c74b13fa20b8cb# Parent74f94e15bfe1dad412d0342aeabdbd895657f54fLinux/xencommons: Use oxenstored by default when available
Assuming this has been tested:
Re: [PATCH 4 of 4] Linux/xencommons: Use oxenstored by default when available
Quoted 10 lines · Olaf Hering
Quoted 6 lines · Ian Campbell
# HG changeset patch
# User Ian Campbell <[email protected]>
# Date 1323448636 0
# Node ID3b7ac401f144206c30440fbb41c74b13fa20b8cb# Parent74f94e15bfe1dad412d0342aeabdbd895657f54fLinux/xencommons: Use oxenstored by default when availableAssuming this has been tested:
I've been running with it for quite a while. Perhaps more importantly XenServer has been using it since, well, since I can't remember when but it was several years ago when they switched and they pound it pretty hard in their testing. Harder than C xenstored could cope with which is why this one got written...
Another thing I forgot to mention is that by default the database is non-persistent and held in RAM, so no need for hacks like putting the db file on a tmpfs.
Ian.
Re: [PATCH 4 of 4] Linux/xencommons: Use oxenstored by default when available
Quoted 10 lines · Ian Campbell
Quoted 9 lines · Olaf Hering
Quoted 6 lines · Ian Campbell
# HG changeset patch
# User Ian Campbell <[email protected]>
# Date 1323448636 0
# Node ID3b7ac401f144206c30440fbb41c74b13fa20b8cb# Parent74f94e15bfe1dad412d0342aeabdbd895657f54fLinux/xencommons: Use oxenstored by default when available
Quoted 7 lines
Quoted 1 line
Assuming this has been tested:
I've been running with it for quite a while. Perhaps more importantly XenServer has been using it since, well, since I can't remember when but it was several years ago when they switched and they pound it pretty hard in their testing. Harder than C xenstored could cope with which is why this one got written...
However is our version up in sync with theirs I wonder... Are we missing any important bug fix?
Re: [PATCH 4 of 4] Linux/xencommons: Use oxenstored by default when available
Quoted 13 lines · Stefano Stabellini
Quoted 12 lines · Ian Campbell
Quoted 10 lines · Olaf Hering
Quoted 6 lines · Ian Campbell
# HG changeset patch
# User Ian Campbell <[email protected]>
# Date 1323448636 0
# Node ID3b7ac401f144206c30440fbb41c74b13fa20b8cb# Parent74f94e15bfe1dad412d0342aeabdbd895657f54fLinux/xencommons: Use oxenstored by default when availableAssuming this has been tested:
Quoted 8 lines
Quoted 5 lines
I've been running with it for quite a while. Perhaps more importantly XenServer has been using it since, well, since I can't remember when but it was several years ago when they switched and they pound it pretty hard in their testing. Harder than C xenstored could cope with which is why this one got written...
However is our version up in sync with theirs I wonder... Are we missing any important bug fix?
XenServer switched to the one in the xen tree in March by the looks of things, at least that's when the copy in xen-api.git was removed.
There is nothing of interest in xen-api.git's copy from the point it was imported into xen-unstable until now, if anything xen-unstable has been the one getting fixes.
Ian.
Re: [PATCH 4 of 4] Linux/xencommons: Use oxenstored by default when available
In reply to Ian Campbell2011/12/9 Ian Campbell <[email protected]>:
Quoted 9 lines
# HG changeset patch
# User Ian Campbell <[email protected]>
# Date 1323448636 0
# Node ID3b7ac401f144206c30440fbb41c74b13fa20b8cb# Parent 74f94e15bfe1dad412d0342aeabdbd895657f54f Linux/xencommons: Use oxenstored by default when availableoxenstored is an ocaml implementation of the xenstore daemon. It is faster, more scalable and more reliable than the C xenstored.
This means that C xenstore is going the way of the Dodo? Or are we going to maintain both implementations (ocalm and C)?
_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xensource.com/xen-devel
Re: [PATCH 4 of 4] Linux/xencommons: Use oxenstored by default when available
Quoted 8 lines · Roger Pau Monné
2011/12/9 Ian Campbell <[email protected]>:
Quoted 7 lines
# HG changeset patch
# User Ian Campbell <[email protected]>
# Date 1323448636 0
# Node ID3b7ac401f144206c30440fbb41c74b13fa20b8cb# Parent74f94e15bfe1dad412d0342aeabdbd895657f54fLinux/xencommons: Use oxenstored by default when available
Quoted 5 lines
Quoted 2 lines
oxenstored is an ocaml implementation of the xenstore daemon. It is faster, more scalable and more reliable than the C xenstored.
This means that C xenstore is going the way of the Dodo? Or are we going to maintain both implementations (ocalm and C)?
I honestly don't know.
They are both reasonably low maintenance and rarely change or have bugs reported. We don't actually see many bug reports about C xenstored although it's short comings only tend to become apparent at scale and I suspect lots of people use workarounds like nuking the tdb on boot and/or putting it on a tmpfs.
On the other hand having two code bases providing the same service has its own downsides.
I expect our (or at least my) first response to a non-obvious bug report in C xenstored would be "have you tried oxenstored?".
FWIW my intention (when I eventually get to it and resurrect those patches) is for stub-xenstored to be an oxenstored only thing.
Ian.
_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xensource.com/xen-devel
Quoted 1 line · Ian Campbell
Two other small fixlets are included.
Actually, that should have been four. I'll post the other two as separate replies to this mail. They will be:
oxenstored: install configuration file
oxenstored: Always log something at start of day (if logging enabled at all)
Ian.
PATCH 1/1 oxenstored: install configuration file
# HG changeset patch
# User Ian Campbell <[email protected]> # Date 1323449332 0
# Node ID 91415c125c676d9e87d9939bfecdd1a4a0b2edfa # Parent 3b7ac401f144206c30440fbb41c74b13fa20b8cb
oxenstored: install configuration file
First though:
- Move it to /etc/xen/oxenstored.conf.
- Use /var/run/xenstored.pid as default pid file
- Disable test-eagain "Randomly failed a transaction with EAGAIN. Used for testing Xs user". Doesn't sound fun by default...
tools/ocaml/xenstored/Makefile +2 -0
@@ -52,5 +52,7 @@ bins: $(PROGRAMS) install: all $(INSTALL_DIR) $(DESTDIR)$(SBINDIR) $(INSTALL_PROG) oxenstored $(DESTDIR)$(SBINDIR) + $(INSTALL_DIR) $(DESTDIR)$(XEN_CONFIG_DIR) + $(INSTALL_DATA) oxenstored.conf $(DESTDIR)$(XEN_CONFIG_DIR) include $(OCAML_TOPLEVEL)/Makefile.rules
tools/ocaml/xenstored/define.ml +1 -1
@@ -23,7 +23,7 @@ let xenstored_proc_port = "/proc/xen/xsd let xs_daemon_socket = "/var/run/xenstored/socket" let xs_daemon_socket_ro = "/var/run/xenstored/socket_ro" -let default_config_dir = "/etc/xensource" +let default_config_dir = "/etc/xen" let maxwatch = ref (50) let maxtransaction = ref (20)
tools/ocaml/xenstored/oxenstored.conf renamed from tools/ocaml/xenstored/xenstored.conf +2 -2
@@ -1,10 +1,10 @@ # default xenstored config # Where the pid file is stored -pid-file = /var/run/xensource/xenstored.pid +pid-file = /var/run/xenstored.pid # Randomly failed a transaction with EAGAIN. Used for testing Xs user -test-eagain = true +test-eagain = false # Activate transaction merge support merge-activate = true
tools/ocaml/xenstored/xenstored.ml +1 -1
@@ -71,7 +71,7 @@ let sighup_handler _ = let config_filename cf = match cf.config_file with | Some name -> name - | None -> Define.default_config_dir ^ "/xenstored.conf" + | None -> Define.default_config_dir ^ "/oxenstored.conf" let default_pidfile = "/var/run/xenstored.pid"
PATCH 1/1 oxenstored: Always log something at start of day (if logging enabled at all)
In reply to Ian Campbell# HG changeset patch
# User Ian Campbell <[email protected]>
# Date 1323450179 0
# Node ID c2fbf133c235c023769640fdb24d84462a6b36ec
# Parent 91415c125c676d9e87d9939bfecdd1a4a0b2edfa
oxenstored: Always log something at start of day (if logging enabled at all)
Otherwise at the default level we rarely log anything at all.
A completely empty log file is a good sign, but only if you know you are looking in the right place...
tools/ocaml/xenstored/logging.ml +3 -0
@@ -117,6 +117,9 @@ let init_xenstored_log () = make_logger !xenstored_log_file !xenstored_log_nb_files !xenstored_log_nb_lines !xenstored_log_nb_chars ignore in + let date = string_of_date() in + logger.write ("[%s|%5s|%s] Xen Storage Daemon, version %d.%d") date "" "startup" + Define.xenstored_major Define.xenstored_minor; xenstored_logger := Some logger let xenstored_logging level key (fmt: (_,_,_,_) format4) =
tools/ocaml/xenstored/xenstored.ml +0 -3
@@ -284,9 +284,6 @@ let _ = Logging.init_access_log post_rotate end; - info "Xen Storage Daemon, version %d.%d" - Define.xenstored_major Define.xenstored_minor; - let spec_fds = (match rw_sock with None -> [] | Some x -> [ x ]) @ (match ro_sock with None -> [] | Some x -> [ x ]) @