Xen devel - Sep 2012 - [PATCH vtpm_manager 2/2] Fixes to vtpm hotplug scripts

This patch fixes IO deadlocks in the vtpm hotplug scripts

Signed off by: Matthew Fioravante matthew.fioravante@jhuapl.edu


diff --git a/tools/hotplug/Linux/vtpm b/tools/hotplug/Linux/vtpm
--- a/tools/hotplug/Linux/vtpm
+++ b/tools/hotplug/Linux/vtpm
@@ -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
diff --git a/tools/hotplug/Linux/vtpm-common.sh
b/tools/hotplug/Linux/vtpm-common.sh
--- a/tools/hotplug/Linux/vtpm-common.sh
+++ b/tools/hotplug/Linux/vtpm-common.sh
@@ -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";                 \
diff --git a/tools/hotplug/Linux/vtpm-delete
b/tools/hotplug/Linux/vtpm-delete
--- a/tools/hotplug/Linux/vtpm-delete
+++ b/tools/hotplug/Linux/vtpm-delete
@@ -5,6 +5,8 @@
 # or
 # vtpm-delete --vmname <vm name>
 
+export PATH=$PATH:/usr/sbin:/sbin
+
 dir=$(dirname "$0")
 . "$dir/vtpm-common.sh"
 
diff --git a/tools/hotplug/Linux/vtpm-impl b/tools/hotplug/Linux/vtpm-impl
--- a/tools/hotplug/Linux/vtpm-impl
+++ b/tools/hotplug/Linux/vtpm-impl
@@ -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
diff --git a/tools/python/xen/xend/server/tpmif.py
b/tools/python/xen/xend/server/tpmif.py
--- a/tools/python/xen/xend/server/tpmif.py
+++ b/tools/python/xen/xend/server/tpmif.py
@@ -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
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

On Mon, 2012-09-17 at 22:19 +0100, Matthew Fioravante
wrote:
> 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.
> - set -e
> + local resp_hex
> + #send cmd to vtpm_manager and 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.

On 09/18/2012 03:43 AM, Ian Campbell wrote:
> On Mon, 2012-09-17 at 22:19 +0100, Matthew Fioravante wrote:
>> 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.
>
>> - set -e
>> + local resp_hex
>> + #send cmd to vtpm_manager and 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?
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.
>
> Ian.
>



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

On Tue, 2012-09-18 at 18:40 +0100, Matthew Fioravante
wrote:
> On 09/18/2012 03:43 AM, Ian Campbell wrote:
> > On Mon, 2012-09-17 at 22:19 +0100, Matthew Fioravante wrote:
> >> 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.
> >
> >> - set -e
> >> + local resp_hex
> >> + #send cmd to vtpm_manager and 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?
> 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).
>  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.
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.