Fixes to vtpm hotplug scripts
PATCH 2/2 Fixes to vtpm hotplug scripts
This patch fixes IO deadlocks in the vtpm hotplug scripts
Signed off by: Matthew Fioravante [email protected]
tools/hotplug/Linux/vtpm +4 -8
@@ -1,22 +1,18 @@ #!/bin/bash +export PATH=$PATH:/usr/sbin:/sbin + dir=$(dirname "$0") . "$dir/vtpm-hotplug-common.sh" -vtpm_fatal_error=0 - case "$command" in add) vtpm_create_instance + success ;; remove) vtpm_remove_instance + success ;; esac -if [ $vtpm_fatal_error -eq 0 ]; then - log debug "Successful vTPM operation '$command'." - success -else - fatal "Error while executing vTPM operation '$command'." -fi
(quoted diff) +0 -0
b/tools/hotplug/Linux/vtpm-common.sh
tools/hotplug/Linux/vtpm-common.sh +11 -14
@@ -98,7 +98,7 @@ function vtpmdb_is_free_instancenum () { avail=0 else instances=$(cat $VTPMDB | \ - awk \ + gawk \ '{ \ if (1 != index($1,"#")) { \ printf("%s ",$2); \
@@ -120,7 +120,7 @@ function vtpmdb_is_free_instancenum () { function vtpmdb_get_free_instancenum () { local ctr instances don found instances=$(cat $VTPMDB | \ - awk \ + gawk \ '{ \ if (1 != index($1,"#")) { \ printf("%s ",$2); \
@@ -174,7 +174,7 @@ function vtpmdb_validate_entry () { inst=$2 res=$(cat $VTPMDB | \ - awk -vvmname=$vmname \ + gawk -vvmname=$vmname \ -vinst=$inst \ '{ \ if ( 1 == index($1,"#")) {\
@@ -209,7 +209,7 @@ function vtpmdb_remove_entry () { VTPMDB_TMP="$VTPMDB".tmp $(cat $VTPMDB | \ - awk -vvmname=$vmname \ + gawk -vvmname=$vmname \ '{ \ if ( $1 != vmname ) { \ print $0; \
@@ -276,12 +276,10 @@ function vtpm_create_instance () { vtpm_create $instance - if [ $vtpm_fatal_error -eq 0 ]; then - if [ "$uuid" != "" ]; then - vtpmdb_add_instance $uuid $instance - else - vtpmdb_add_instance $domname $instance - fi + if [ "$uuid" != "" ]; then + vtpmdb_add_instance $uuid $instance + else + vtpmdb_add_instance $domname $instance fi else if [ "$reason" == "resume" ]; then
@@ -290,7 +288,6 @@ function vtpm_create_instance () { vtpm_start $instance fi fi - release_lock vtpmdb xenstore_write $XENBUS_PATH/instance $instance
@@ -322,8 +319,8 @@ function vtpm_remove_instance () { if [ "$instance" != "0" ]; then vtpm_suspend $instance fi - release_lock vtpmdb + }
@@ -350,13 +347,13 @@ function vtpm_delete_instance () { function vtpm_isLocalAddress() { local addr res addr=$(ping $1 -c 1 | \ - awk '{ print substr($3,2,length($3)-2); exit }') + gawk '{ print substr($3,2,length($3)-2); exit }') if [ "$addr" == "" ]; then echo "-1" return fi res=$(ifconfig | grep "inet addr" | \ - awk -vaddr=$addr \ + gawk -vaddr=$addr \ '{ \ if ( addr == substr($2, 6)) {\ print "1"; \
(quoted diff) +0 -0
b/tools/hotplug/Linux/vtpm-delete
tools/hotplug/Linux/vtpm-delete +2 -0
@@ -5,6 +5,8 @@ # or # vtpm-delete --vmname <vm name> +export PATH=$PATH:/usr/sbin:/sbin + dir=$(dirname "$0") . "$dir/vtpm-common.sh"
tools/hotplug/Linux/vtpm-impl +0 -1
@@ -32,14 +32,16 @@ # OF THE POSSIBILITY OF SUCH DAMAGE. # =================================================================== -# | SRC | TAG | CMD SIZE |
ORD |mtype|strt
-TPM_CMD_OPEN=\\x00\\x00\\x00\\x00\\x01\\xc1\\x00\\x00\\x00\\x11\\x01\\x00\\x00\\x01\\x01\\x01 -TPM_CMD_RESM=\\x00\\x00\\x00\\x00\\x01\\xc1\\x00\\x00\\x00\\x11\\x01\\x00\\x00\\x01\\x01\\x02 -TPM_CMD_CLOS=\\x00\\x00\\x00\\x00\\x01\\xc1\\x00\\x00\\x00\\x0e\\x01\\x00\\x00\\x02 -TPM_CMD_DELE=\\x00\\x00\\x00\\x00\\x01\\xc1\\x00\\x00\\x00\\x0e\\x01\\x00\\x00\\x03 +export PATH=$PATH:/usr/sbin:/sbin
-TPM_TYPE_PVM=\\x01
-TPM_TYPE_HVM=\\x02
+# | SRC |TAG| CMD SZ|| ORD |mtype|strt
+TPM_CMD_OPEN="0000000001C100000011010000010101"
+TPM_CMD_RESM="0000000001C100000011010000010102"
+TPM_CMD_CLOS="0000000001C10000000E01000002"
+TPM_CMD_DELE="0000000001C10000000E01000003"
+
+TPM_TYPE_PVM=01
+TPM_TYPE_HVM=02
TPM_SUCCESS=00000000
@@ -70,24 +72,19 @@ function vtpm_manager_cmd() {
local inst=$2;
local inst_bin=$(hex32_to_bin $inst);
- claim_lock vtpm_mgr
-
- #send cmd to vtpm_manager
- printf "$cmd$inst_bin" > $TX_VTPM_MANAGER
-
- #recv response
- set +e
- local resp_hex=dd skip=10 bs=1 count=4 if=$RX_VTPM_MANAGER 2> /dev/null | xxd -ps
- set -e
+ local resp_hex
+ #send cmd to vtpm_manager and get response
+ if ! resp_hex=echo "$cmd$(str_to_hex32 $inst)" | vtpmmgrtalk ; then
+ release_lock vtpmdb
+ fatal "Error communicating with vTPM Manager"
+ fi
- release_lock vtpm_mgr
+ resp_hex=echo $resp_hex | cut -b 21-
#return whether the command was successful
- if [ $resp_hex -ne $TPM_SUCCESS ]; then
- vtpm_fatal_error=1
- false
- else
- true
+ if [ "$resp_hex" != "$TPM_SUCCESS" ]; then
+ release_lock vtpmdb
+ fatal "vTPM Manager returned failure code $resp_hex"
fi
}
@@ -142,13 +139,8 @@ function vtpm_suspend() {
function vtpm_delete() {
local inst=$1
- if $(vtpm_manager_cmd $TPM_CMD_DELE $inst); then
- rm -f /var/vtpm/vtpm_dm_$1.data
- true
- else
- vtpm_fatal_error=1
- false
- fi
+ $(vtpm_manager_cmd $TPM_CMD_DELE $inst)
+ rm -f /var/vtpm/vtpm_dm_$1.data
}
# Perform a migration step. This function differentiates between migration
(quoted diff) +0 -0
b/tools/python/xen/xend/server/tpmif.py
tools/python/xen/xend/server/tpmif.py +16 -0
@@ -44,6 +44,22 @@ class TPMifController(DevController): DevController.__init__(self, vm) + def createDevice(self, config): + #Disable hotplug scripts if backend is not dom0 + import xen.xend.XendDomain + xd = xen.xend.XendDomain.instance() + backdom_name = config.get('backend') + if backdom_name is None: + backdom = xen.xend.XendDomain.DOM0_ID + else: + bd = xd.domain_lookup_nr(backdom_name) + backdom = bd.getDomid() + + if backdom != xen.xend.XendDomain.DOM0_ID: + self.hotplug = False + + return DevController.createDevice(self, config) + def getDeviceDetails(self, config): """@see DevController.getDeviceDetails"""
_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xen.org/xen-devel
Quoted 1 line · Matthew Fioravante
This patch fixes IO deadlocks in the vtpm hotplug scripts
Is the switch from awk to gawk a part of this? How?
IIRC we deliberately switched from using gawk specific features to allow e.g. mawk to be used. If there is some gawk specific construct used here it would be better to recast it in more portable terms IMHO.
Quoted 4 lines
- set -e
+ localresp_hex
+ #send cmd tovtpm_managerand get response
+ if ! resp_hex=echo "$cmd$(str_to_hex32 $inst)" | vtpmmgrtalk; then
Is this really the command/control interface provided by VTPM? Can we not work with upstream to define something more usable?
Ian.
Quoted 7 lines · Ian Campbell
Quoted 1 line · Matthew Fioravante
This patch fixes IO deadlocks in the vtpm hotplug scripts
Is the switch from awk to gawk a part of this? How?
IIRC we deliberately switched from using gawk specific features to allow e.g. mawk to be used. If there is some gawk specific construct used here it would be better to recast it in more portable terms IMHO.
I'll test and get back to you on this.
Quoted 7 lines
Quoted 5 lines
- set -e
+ localresp_hex
+ #send cmd tovtpm_managerand get response
+ if ! resp_hex=echo "$cmd$(str_to_hex32 $inst)" | vtpmmgrtalk; thenIs this really the command/control interface provided by VTPM? Can we not work with upstream to define something more usable?
Unfortunately xen itself is the upstream for vtpm_manager. The vtpm_manager code is full of bugs and actually needs a complete rewrite. The amount of careless memory leaks and other bugs I found when trying to use it were pretty astounding. The first manager patch fixes most of the ones I could find to get the manager to work properly.
The managers current form in the xen tree is actually pretty unusable. I don't have time to rewrite the manager, but what I'm offering here is at least a huge improvement over the old code.
Quoted 3 lines
Ian.
_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xen.org/xen-devel
Quoted 10 lines · Matthew Fioravante
Quoted 7 lines · Ian Campbell
Quoted 1 line · Matthew Fioravante
This patch fixes IO deadlocks in the vtpm hotplug scripts
Is the switch from awk to gawk a part of this? How?
IIRC we deliberately switched from using gawk specific features to allow e.g. mawk to be used. If there is some gawk specific construct used here it would be better to recast it in more portable terms IMHO.
I'll test and get back to you on this.
Quoted 1 line
Quoted 7 lines
Quoted 6 lines
Quoted 4 lines
- set -e
+ localresp_hex
+ #send cmd tovtpm_managerand get response
+ if ! resp_hex=echo "$cmd$(str_to_hex32 $inst)" | vtpmmgrtalk; thenIs this really the command/control interface provided by VTPM? Can we not work with upstream to define something more usable?
Unfortunately xen itself is the upstream for
vtpm_manager.
Do you suppose the berlios project might be interested in subsuming this functionality too? (I'll be honest, I have only the vaguest of ideas what each of these components does).
Quoted 9 lines
The
vtpm_managercode is full of bugs and actually needs a complete rewrite. The amount of careless memory leaks and other bugs I found when trying to use it were pretty astounding. The first manager patch fixes most of the ones I could find to get the manager to work properly.The managers current form in the xen tree is actually pretty unusable. I don't have time to rewrite the manager, but what I'm offering here is at least a huge improvement over the old code.
That doesn't surprise me, it doesn't seem to have been especially actively maintained for a very long time.
I don't suppose you want to be maintainer do you? If so you could send a patch against the MAINTINERS file.
Ian.