[ovs-dev] [PATCH 2/2] xenserver: Support network names with spaces

Ian Campbell Ian.Campbell at citrix.com
Tue Mar 2 17:41:15 UTC 2010


On Tue, 2010-03-02 at 17:09 +0000, Justin Pettit wrote: 
> On Mar 2, 2010, at 5:50 AM, Ian Campbell wrote:

> > What about adding an option to ovs-vsctl which takes a xenstore root and
> > pulls the extra keys out itself? Since ovs-vsctl is in a "proper"
> > language(*) it ought to be trivial to get the quoting correct. Something
> > like "set interface vif${DOMID}.${DEVID} xs-keys=${PRIVATE}".
> > 
> > It's a bit skanky to build the XS specific knowledge into ovs-vsctl
> > though. At the risk of complete over engineering things what about
> > allowing ovs-vsctl to call out to one or more hooks to provide this sort
> > of additional data for various commands?
> 
> I'll see how things are looking today. If we have time, we'll try to
> come up with a more permanent solution than my original patch that
> makes us all happy.

If we get rid of the building the command line in a variable thing
(which seems unnecessary anyway) I think it's trivial to quote
correctly, like the untested patch below.

I guess it relies on ovs-vsctl handing blank values for the set command,
although that should be unusual since we'd always expect to get
something back from xenstore for each of these variables.

Ian.

diff -r 0ede0363a33a scripts/vif
--- a/scripts/vif	Thu Feb 25 16:07:59 2010 +0000
+++ b/scripts/vif	Tue Mar 02 17:37:58 2010 +0000
@@ -66,40 +66,17 @@
     fi
 }
 
-set_vif_external_id()
-{
-    local key=$1
-    local value=$2
-
-    logger -t scripts-vif "vif${DOMID}.${DEVID} external-ids:\"${key}\"=\"${value}\""
-
-    echo "-- set interface vif${DOMID}.${DEVID} external-ids:\"${key}\"=\"${value}\""
-}
-
 handle_vswitch_vif_details()
 {
-    local vif_details=
-    local net_uuid=$(xenstore-read "${PRIVATE}/network-uuid" 2>/dev/null)
-    if [ -n "${net_uuid}" ] ; then
-	set_vif_external_id "xs-network-uuid" "${net_uuid}"
-    fi
+    XS_NETWORK_UUID=$(xenstore-read "${PRIVATE}/network-uuid" 2>/dev/null)
 
-    local address=$(xenstore-read "/local/domain/$DOMID/device/vif/$DEVID/mac" 2>/dev/null)
-    if [ -n "${address}" ] ; then
-	set_vif_external_id "xs-vif-mac" "${address}"
-    fi
+    XS_VIF_MAC=$(xenstore-read "/local/domain/$DOMID/device/vif/$DEVID/mac" 2>/dev/null)
 
-    local vif_uuid=$(xenstore-read "${PRIVATE}/vif-uuid" 2>/dev/null)
-    if [ -n "${vif_uuid}" ] ; then
-	set_vif_external_id "xs-vif-uuid" "${vif_uuid}"
-    fi
+    XS_VIF_UUID=$(xenstore-read "${PRIVATE}/vif-uuid" 2>/dev/null)
 
     local vm=$(xenstore-read "/local/domain/$DOMID/vm" 2>/dev/null)
     if [ $? -eq 0 -a -n "${vm}" ] ; then
-	local vm_uuid=$(xenstore-read "$vm/uuid" 2>/dev/null)
-    fi
-    if [ -n "${vm_uuid}" ] ; then
-	set_vif_external_id "xs-vm-uuid" "${vm_uuid}"
+	XS_VM_UUID=$(xenstore-read "$vm/uuid" 2>/dev/null)
     fi
 }
 
@@ -129,11 +106,19 @@
 	    ${BRCTL} addif "${bridge}" "${dev}"                 || logger -t scripts-vif "Failed to brctl addif ${bridge} ${dev}"
 	    ;;
 	vswitch)
-	    if [ "$TYPE" = "vif" ] ; then
-		local vif_details=$(handle_vswitch_vif_details)
-	    fi
-
-	    $vsctl add-port $bridge $dev $vif_details
+	    case "$TYPE" in
+	        vif)
+		    handle_vswitch_vif_details
+	            $vsctl add-port $bridge $dev \
+                        -- set interface vif${DOMID}.${DEVID} external-ids:xs-network-uuid="$XS_NETWORK_UUID" \
+                        -- set interface vif${DOMID}.${DEVID} external-ids:xs-vif-mac="$XS_VIF_MAC" \
+                        -- set interface vif${DOMID}.${DEVID} external-ids:xs-vif-uuid="$XS_VIF_UUID" \
+                        -- set interface vif${DOMID}.${DEVID} external-ids:xs-vm-uuid="$XS_VM_UUID"
+	            ;;
+	        tap)
+		    $vsctl add-port $bridge $dev
+	            ;;
+            esac
 	    ;;
     esac
 	    






More information about the dev mailing list