Re: [Xen-devel] [PATCH] tools/hotpug: only attempt to call 'ip route' if there is valid command

[Paul Durrant <pdurrant@xxxxxxxxx>]

On Mon, 28 Oct 2019 at 12:30, Anthony PERARD <anthony.perard@xxxxxxxxxx> wrote:
>
> On Thu, Oct 24, 2019 at 03:21:03PM +0100, paul@xxxxxxx wrote:
> > From: Paul Durrant <pdurrant@xxxxxxxxxx>
> >
> > The vif-route script should only call 'ip route' when 'ipcmd' has been
> > set, otherwise it will fail due to an incorrect command string.
> >
> > Signed-off-by: Paul Durrant <pdurrant@xxxxxxxxxx>
> > ---
> > Cc: Ian Jackson <ian.jackson@xxxxxxxxxxxxx>
> > Cc: Wei Liu <wei.liu2@xxxxxxxxxx>
> >
> > This appears to have been broken forever.
>
> Or probably since c0efb8dc852805fd4d3c2691aca1f6c52f6b6ac7, 13 years
> ago.
>

Maybe. The heritage is not all that clear.

> > ---
> >  tools/hotplug/Linux/vif-route | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/tools/hotplug/Linux/vif-route b/tools/hotplug/Linux/vif-route
> > index c149ffca73..98893d90b6 100644
> > --- a/tools/hotplug/Linux/vif-route
> > +++ b/tools/hotplug/Linux/vif-route
> > @@ -35,7 +35,7 @@ case "${command}" in
> >          ;;
> >  esac
> >
> > -if [ "${ip}" ] ; then
> > +if [ "${ipcmd}" ] ; then
>
> The commit message and this new condition doesn't really explain what is
> happening.

It kind of does, but maybe not with the standard context. Looking at
the whole file it does appear to be the intention that the clause is
only invoked when ipcmd is set, and it was probably typoed. I can add
some more explanation though.

> Does that means we only need to run `ip route` when command is 'online'
> or 'offline'? Is there something else to do 'add' and 'remove'?
>

Empirically online/offline appear to be used for PV interfaces and
add/remove for emulated interfaces. I do actually have a newer version
of the script now that adds a route metric and handles add/remove such
that the emulated interface gets a higher priority... so it is used
until the guest triggers an unplug. I need to do some more testing and
I'll send that as v2 once I'm done.

  Cheers,

    Paul

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel