Re: [PATCH] Rewrite iscsi code using libiscsi

[Chris Lumens <clumens redhat com>]

Okay, I'll be the picky one.

> diff --git a/backend.py b/backend.py
> index 3008671..06f6c2b 100644
> --- a/backend.py
> +++ b/backend.py
> @@ -79,9 +79,20 @@ class AnacondaBackend:
>                  log.error("Could not copy firmware file %s: %s" % (f,  
> e.strerror))
>
>      def doPostInstall(self, anaconda):
> +        has_iscsi_disk = False
> +
> +        # See if we have an iscsi disk. If we do we rerun mkinitrd, as
> +        # the initrd might need iscsi-initiator-utils, and chances are
> +        # it was not installed yet the first time mkinitrd was run, as
> +        # mkinitrd does not require it.
> +        for disk in anaconda.id.diskset.disks.keys():
> +            if isys.driveIsIscsi(disk):
> +                has_iscsi_disk = True

Don't you want to break here after setting has_iscsi_disk=True, since
there's no need to keep scanning?

> @@ -222,27 +114,38 @@ def randomIname():
>          s += dig[random.randrange(0, 32)]
>      return s
>
> +def stabilize(intf = None):
> +    # Wait for udev to create the devices for the just added disks
> +    if intf:
> +        w = intf.waitWindow(_("Scanning iSCSI nodes"),
> +                            _("Scanning iSCSI nodes"))
> +    # It is possible when we get here the events for the new devices
> +    # are not send yet, so sleep to make sure the events are fired
> +    time.sleep(2)
> +    iutil.execWithRedirect("/sbin/udevadm", [ "settle" ],
> +                           stdout = "/dev/tty5", stderr="/dev/tty5")

In general, we don't want to specify the path to a program when we use
execWith*.  Instead, you just want to call "udevadm" and pass
searchPath=1.

> diff --git a/kickstart.py b/kickstart.py
> index eafd4d8..9305ef0 100644
> --- a/kickstart.py
> +++ b/kickstart.py
> @@ -313,9 +313,6 @@ class IscsiName(commands.iscsiname.FC6_IscsiName):
>          retval = commands.iscsiname.FC6_IscsiName.parse(self, args)
>
>          self.handler.id.iscsi.initiator = self.iscsiname
> -        self.handler.id.iscsi.startIBFT()
> -        # FIXME: flush the drive dict so we figure drives out again
> -        isys.flushDriveDict()
>          return retval
>
>  class Keyboard(commands.keyboard.FC3_Keyboard):

Are any kickstart syntax changes needed for this new iscsi stuff?

> diff --git a/rescue.py b/rescue.py
> index 2ae35e6..42e5b96 100644
> --- a/rescue.py
> +++ b/rescue.py
> @@ -205,6 +205,7 @@ def runRescue(anaconda, instClass):
>              else:
>                  break
>
> +        anaconda.intf = None
>          screen.finish()
>
>      # Early shell access with no disk access attempts

I'm not sure what the point of this one is.  Could you explain?

I'm having a little trouble following the meat of this patch, which is
probably just due to the fact that sometimes diff makes a colossal mess
of things.  Could you either post the full iscsi.py somewhere or break
it into smaller chunks?  Thanks.

- Chris