Re: [PATCH 2.6.11.7] ATA Over Ethernet Root

From: randy_dunlap (rdunlap_at_xenotime.net)
Date: 05/13/05

  • Next message: Michael H. Warfield: "Re: Sync option destroys flash!"
    Date:	Fri, 13 May 2005 10:48:37 -0700
    To: "McMullan, Jason" <jason.mcmullan@timesys.com>
    
    

    On Fri, 13 May 2005 13:19:55 -0400 McMullan, Jason wrote:

    | This patch allows you to use ATA Over Ethernet as your root device,
    | with 'root=/dev/etherd/eX.Y/disc'
    |
    | Limited testing, just for ya'lls review.
    or y'alls

    Please make patches against 2.6.12-rcX, not against 2.6.11.z.
    (probably doesn't matter in this case)

    +struct aoedev *
    +aoedev_bymajor_minor(ulong major, ulong minor)
    +{

    Kernel preferred style is not to split the function definition line.

    +#ifdef CONFIG_ATA_OVER_ETH_ROOT
    +#include <linux/delay.h>
    +#include <linux/rtnetlink.h>
    +#include <linux/netdevice.h>
    +#include <net/sock.h>
    +#endif

    Lose the ifdef/endif. Don't make #includes conditional.

    +printk("Bring up loopback...\n");
    + if (dev_change_flags(&loopback_dev, loopback_dev.flags | IFF_UP) < 0)
    + printk(KERN_ERR "AOE Root: Failed to open %s\n", loopback_dev.name);
    +
    + /* Setup all network devices */
    + for (dev = dev_base; dev ; dev = dev->next) {
    + if (dev == &loopback_dev)
    + continue;
    +printk("Bring up %s...\n",dev->name);

    Did you mean to leave these printk()s here?
    I think not, but if so, they need a KERN_ level, like KERN_INFO.

    It would help readability if you would add some spaces around
    '=', after commas, etc., in places like these:

    + for (d=devlist; d; d=d->next)
    and
    + } while (!aoedev_bymajor_minor(CONFIG_ATA_OVER_ETH_ROOT_SHELF,CONFIG_ATA_OVER_ETH_ROOT_SLOT));
    and
    + aoe_root(CONFIG_ATA_OVER_ETH_ROOT_SHELF,CONFIG_ATA_OVER_ETH_ROOT_SLOT);

    Thanks.

    ---
    ~Randy
    -
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at  http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at  http://www.tux.org/lkml/
    

  • Next message: Michael H. Warfield: "Re: Sync option destroys flash!"

    Relevant Pages

    • Re: Linux 2.4.28-pre4
      ... > and if substantial amount of users will benefit from it. ... I would appreciate a review of the driver very much. ... send the line "unsubscribe linux-kernel" in ...
      (Linux-Kernel)
    • Re: rootdelay
      ... >> boot time because it wakes up the boot process right when the ... Since I saw your patch included, ... that mounting the root device happens exactly when it needs to, ... send the line "unsubscribe linux-kernel" in ...
      (Linux-Kernel)
    • Re: 2.6.5-aa1
      ... > intradiff for review, plus it merges some nice lowlatency improvement ... access the net server game browser in ET to play online! ... send the line "unsubscribe linux-kernel" in ...
      (Linux-Kernel)
    • Re: M7101
      ... > Any chance we could get the PCI folks to review the code and push it ... send the line "unsubscribe linux-kernel" in ...
      (Linux-Kernel)
    • Re: Linux 2.6.12-rc3
      ... > I think I have a sane splitup of that stuff. ... I did some review and the compiler liked it as well, ... Unfortunately due to various personal reasons I won't have time to test it on ... send the line "unsubscribe linux-kernel" in ...
      (Linux-Kernel)