On Thu, 2011-10-27 at 06:28 +0100, Jim Fehlig wrote:
> Jim Fehlig wrote:
> > Ian Campbell wrote:
> >
> >> On Wed, 2011-10-26 at 00:06 +0100, Jim Fehlig wrote:
> >>
> >>
> >>> I previously sent this from my @suse.com mail address without having
> >>> subscribed it. Sending again now that I have done so...
> >>>
> >>> I received a report that vif-bridge adds any tap interface to a bridge,
> >>> regardless if xen is running and who created the tap interface. E.g.
> >>>
> >>> # tunctl -p -t tap42
> >>>
> >>> will cause vif-bridge to be executed as per the following rule in
> >>> xen-backend.rules
> >>>
> >>>
> >> Oh dear.
> >>
> >>
> >>
> >>> SUBSYSTEM=="net", KERNEL=="tap*", ACTION=="add",
> >>> RUN+="/etc/xen/scripts/vif-setup $env{ACTION} type_if=tap"
> >>>
> >>> I'm not sure how to improve the rule to prevent execution of vif-setup
> >>> in this case. But it seems better to handle it in vif-bridge anyhow, by
> >>> not connecting the interface to a bridge if there is no corresponding
> >>> info in xenstore. Something along the lines of the attached quick
> >>> patch. Comments?
> >>>
> >>>
> >> I think overall your change is an improvement, some thoughts:
> >>
> >> For a tap device XENBUS_PATH is set in vif-common.sh:
> >> elif [ "$type_if" = tap ]; then
> >> # Check presence of compulsory args.
> >> : ${INTERFACE:?}
> >>
> >> # Get xenbus_path from device name.
> >> # The name is built like that: "tap${domid}.${devid}".
> >> dev_=${dev#tap}
> >> domid=${dev_%.*}
> >> devid=${dev_#*.}
> >>
> >> XENBUS_PATH="/local/domain/0/backend/vif/$domid/$devid"
> >> fi
> >>
> >> Could there be false positives from this?
> >>
> >
> > Hmm, yes, I think it is possible.
> >
>
> On second thought, maybe not. A false positive would mean two tap
> devices with the same name right? AFAICT, that's not permitted.
Oh right, we are given $dev aren't we.
>
> >
> >> Perhaps we should be more
> >> aggressively checking for the tapX.Y, where X and Y are integers, format
> >> as well? (that's not foolproof either though).
> >>
> >>
> >
> > Yeah, I don't think that buys us much.
> >
> >
> >> Perhaps the toolstack could write something to xenstore containing the
> >> literal tap device name which it asked qemu for? Then we can simply read
> >> it back here, e.g. /libxl/tap/0/tapX.Y -> $XENBUS_PATH (0 being the
> >> backend domain and the content being the xenbus path so we don't need to
> >> magic it up).
> >>
>
> The toolstack already writes something in xenstore, namely
> $XENBUS_PATH/bridge.
XENBUS_PATH here is really the vif backend path, not the tap path,
although they in some way are aliased so in many cases that ok. I was
just thinking it might be useful to have a backend space for the tap
device only (since the guest can see the vif backend dir).
> IMO, the problem is in vif-bridge
>
> bridge=${bridge:-}
> bridge=$(xenstore_read_default "$XENBUS_PATH/bridge" "$bridge")
>
> if [ -z "$bridge" ]
> then
> bridge=$(brctl show | cut -d "
> " -f 2 | cut -f 1)
>
> if [ -z "$bridge" ]
> then
> fatal "Could not find bridge, and none was specified"
> fi
> else
> ...
>
> If the toolstack hasn't written anything to xenstore, vif-bridge happily
> connects the tap device to the first bridge it finds. Shouldn't
> vif-bridge just exit if no bridge is specified?
I think that behaviour is historical (which isn't to say it's correct).
FWIW xl defaults to writing xenbr0. I don't know what xend does.
Ian.
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel
|
Home