Added an option for IntelliProp RAID controllers for choosing the drive to send some ATA commands to.

[Casey Biemiller]

Added an option that would allow the choice of selecting which drive to send certain commands to behind an IntelliProp RAID controller. The commands this mainly works for are SMART, GPL Read Log, and GPL Write Log commands. Since smartctl uses these commands, this routing functionality would be necessary to be able to send smartctl commands to each individual drive behind an IntelliProp RAID controller.

[Casey Biemiller]

A diff of the atacmds.cpp

[Casey Biemiller]

A diff of atacmds.h

[Casey Biemiller]

A diff of ataprint.h

[Casey Biemiller]

A diff of Makefile.am

[Casey Biemiller]

A diff of smartctl.8.in

[Casey Biemiller]

A diff of smartctl.cpp

[Casey Biemiller]

The source of an IntelliProp specific file.

[Casey Biemiller]

The header of an IntelliProp specific file.

[Casey Biemiller]

A diff of ChangeLog that can act as another summary of changes.

[Christian Franke]

Thanks for this patch.

Some notes from a quick review:

• Please provide the patch again as one large patch file which cleanly applies to current SVN HEAD (with svn patch ... or patch < FILE.patch). The patch file should also include the new files (e.g. use svn add NEWFILES and then svn diff > FILE.patch).
• The file intelliprop.cpp contains a license info ("IntelliProp Software Products License") which is likely not compatible with GPLv2(+).
• Please explain which controller products and OS platforms benefit from this patch.

[Casey Biemiller]

I added the patch in one file as requested and I also removed the IntelliProp Software Products License from the intelliprop.cpp/h files. As for the controllers this code benefits, they are as follows: IPA-SA117-BR, IPA-SA149-BR, and IPA-SA145-BR. This change was only tested in Linux as that is the only OS it is currently desired to work with, but it may work in other OS platforms as well(there was no Linux specific code added).

[Christian Franke]

Thanks, but a closer look shows that the patch breaks the overall design decision that all controller specific issues should be handled behind the smart_device and smart_interface scenes. It should not be needed to add any code to smartctl.cpp or smartd.cpp to support new controllers. Controller and physical drives should be selected by some -d TYPE[,PARAMETERS] option.

Please provide a complete usage example (smartctl commands and resulting outputs) for your patch.

Which platforms supported by smartmontools do actually support these controllers (e.g. by existing device drivers) ?

[Casey Biemiller]

Usage example:
smartctl -H /dev/sd[a-z]
smartctl --iprop-routecmd=1 /dev/sd[a-z]
smartctl -H /dev/sd[a-z]
smartctl --iprop-routecmd=2 /dev/sd[a-z]
smartctl -H /dev/sd[a-z]
smartctl --iprop-routecmd=3 /dev/sd[a-z]
smartctl -H /dev/sd[a-z]
smartctl --iprop-routecmd=0 /dev/sd[a-z]
This can be looped and other smarctl commands can either replace the -H option or multiple smartctl can be used in between each routecmd to get information from each individual drive.

I most likely can use the -d TYPE[,PARAMETERS] option for an interface(I haven't looked at the code for that yet). Is the idea of intelliprop.cpp/.h addition against the overall design decision or is that fine to work with when working on using the above option?

So far, I have only used the controller with smartctl on the latest version of Ubuntu, but I can test it on other platforms. The controller itself has been tested on Windows 7 as well as a variety of Linux versions. No special drivers are required for the controller.

[Christian Franke]

Please rework the code as follows:

Discard any changes in smartctl.cpp and ataprint.*

Create new file dev_intelliprop.cpp which implements a wrapper class intelliprop_device. Example:

namespace intelliprop {

class intelliprop_device
: public tunnelled_device<
    /*implements*/ ata_device,
    /*by using an*/ ata_device
  >
{
public:
  intelliprop_device(smart_interface * intf, unsigned phydrive, ata_device * atadev);

  virtual bool open();

  virtual bool ata_pass_through(const ata_cmd_in & in, ata_cmd_out & out);

private:
  unsigned m_phydrive;
};

intelliprop_device::intelliprop_device(smart_interface * intf, unsigned phydrive, ata_device * atadev)
: smart_device(intf, atadev->get_dev_name(), "intelliprop", "intelliprop"),
  tunnelled_device<ata_device, ata_device>(atadev),
  m_phydrive(phydrive)
{
  set_info().info_name = strprintf("%s [intelliprop_disk_%u]", atadev->get_info_name(), phydrive);
}

bool intelliprop_device::open()
{
  ata_device * atadev = get_tunnel_dev();
  if (!atadev->open())
    return false;

  // >>>>> TODO: Add command(s) to select phydrive here <<<<<
  return true;
}

bool intelliprop_device::ata_pass_through(const ata_cmd_in & in, ata_cmd_out & out)
{
  return get_tunnel_dev()->ata_pass_through(in, out);
}

} // namespace intelliprop

ata_device * get_intelliprop_device(smart_interface * intf, unsigned phydrive, ata_device * atadev)
{
  return new intelliprop::intelliprop_device(intf, phydrive, atadev);
}

Copy required code from intelliprop.* to this file. Discard intelliprop.*.

Create new file dev_intelliprop.h which contains the prototype of get_intelliprop_device().

Include this file to dev_interface.cppand add -d intelliprop,N support to smart_interface::get_smart_device(). Example:

dev_interface.cpp

@@ -422 +422 @@
     return get_sat_device(sattype.c_str(), basedev.release()->to_scsi());
   }
 
+  else if (str_starts_with(type, "intelliprop")) {
+    int n = -1, len = strlen(type); unsigned phydrive = ~0;
+    sscanf(type, "intelliprop,%u%n", &phydrive, &n);
+    if (!(n == len && phydrive <= 3)) {
+      set_err(EINVAL, "Option '-d intelliprop,N' requires N between 0 and 3");
+      return 0;
+    }
+    // Recurse to get SAT base device
+    smart_device * basedev = get_smart_device(name, "sat");
+    if (!basedev)
+       return 0;
+    return get_intelliprop_device(this, phydrive, basedev->to_ata());
+  }
+
   else {
     set_err(EINVAL, "Unknown device type '%s'", type);
     return 0;

(This is currently limited to SATA controllers behind SAT layers which should be sufficient for Linux. We could later add -d intelliprop,N[+TYPE] to select another base device).

Adjust Makefile.am accordingly.

Test smartctl -d intelliprop,N -x /dev/sdX and report test results and platform here.

Finally note that this all adds -d intelliprop,N support for smartd daemon for free :-)

[Christian Franke]

The patch is not suitable for inclusion (see above). Set milestone back to undecided due to missing feedback from patch author.

[Casey Biemiller]

Sorry for the long delay in responding. The above code works if the Intelliprop controller is plugged in via SATA. If the controller is in the following required situation, it doesn't work. SAS HBA -> SAS/SATA Convertor -> Intelliprop Controller. Using -d sat sends command correctly, but the above code causes errors, even if I don't use any of the routing code. -d sat does show that the basedev is SCSI, but -d intelliprop shows the basedev as ATA. If I set smartctl to ignore errors when doing -a, while using -d intelliprop, I am able to get some SMART information, but many of the commands fail. Any thoughts on this? I'm still working on a solution.

[Christian Franke]

Due to lack of access to this specific hardware, I only could provide limited help. Please attach the full patch (as a single .patch file) here. You could also send it via private mail if desired.

[Casey Biemiller]

I have been able to confirm the code works for Linux via ATA directly and for Linux through a SCSI HBA <---> SCSI/ATA converter interface. The man page for smartctl has been updated as well.

[Casey Biemiller]

Single file diff of every change

[Casey Biemiller]

I included more documentation of the added -d option as well as a few other changes to the code that have been tested.

[Christian Franke]

Patch applied in r4370. Thanks.