On Wed, 2011-07-06 at 10:45 -0700, Joe Eykholt wrote: > On 7/6/11 10:17 AM, Robert Love wrote: > > These options require arguments. This patch validates that > > the arguments are both valid interfaces and that there is a > > fcoe connection on them before proceeding to issue a command > > to fcoemon. > > > > This changes the error message to the user. Previously the > > command would be sent to fcoemon, which would fail and fcoeadm > > would return "Internal Error." Now that validation is occuring > > and fcoemon is not involved we return a more appropriate, "No > > connection created on<ifname>" message. > > > > Signed-off-by: Robert Love<[email protected]> > > Tested-by: Ross Brattain<[email protected]> > > --- > > fcoeadm.c | 22 +++++++++++++++++++--- > > 1 files changed, 19 insertions(+), 3 deletions(-) > > > > diff --git a/fcoeadm.c b/fcoeadm.c > > index 61a84a9..1685a42 100644 > > --- a/fcoeadm.c > > +++ b/fcoeadm.c > > @@ -217,12 +217,24 @@ int main(int argc, char *argv[]) > > switch (opt) { > > case 'c': > > cmd = CLIF_CREATE_CMD; > > + /* fall through */ > > case 'd': > > if (cmd == CLIF_NONE) > > cmd = CLIF_DESTROY_CMD; > > + > > + if (argc> 3) { > > + rc = EBADNUMARGS; > > + break; > > + } > > Hi Rob, > > This isn't a problem with this patch, but with the existing code. > > I wonder if it might be a good idea to check that there > aren't conflicting options like both -c and -d or multiple > -d options. As it is, if you say -c eth0 -d eth1 you'll > end up *creating* eth1 since cmd will remain CLIF_CREATE_CMD > after creating eth0. The same sort of problem can happen with > -r and -S. > Yeah, that's a problem for sure.
> Preferably, all options would be checked before any action > was taken. I would just set cmd and ifname during the options > handling loop, and make sure cmd isn't already set. Then at > the end of the loop switch on the cmd. > Makes sense to me. I was hoping to take a stab at it before replying, but I haven't found time. I'll see what I can do. //Rob _______________________________________________ devel mailing list [email protected] https://lists.open-fcoe.org/mailman/listinfo/devel
