On Wed, 2013-05-15 at 17:36 -0700, Andy Grover wrote:
> Currently we write APTPL state to /var/target/pr ourself. It's
> preferable to avoid this if possible, by relying on a user helper
> to save the state for us. But if it's not present, still do it the
> old way for backwards-compatibility.
>
> Signed-off-by: Andy Grover <agrover@xxxxxxxxxx>
> ---
A big NAK here..
Please don't add user-space upcalls when they don't provide a real
benefit. It just makes things needlessly more complex, and adds
pointless user-space dependencies for existing code.
--nab
> drivers/target/target_core_pr.c | 48 ++++++++++++++++++++++++++++++++++++--
> 1 files changed, 45 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/target/target_core_pr.c b/drivers/target/target_core_pr.c
> index ac34401..c719bdf 100644
> --- a/drivers/target/target_core_pr.c
> +++ b/drivers/target/target_core_pr.c
> @@ -28,6 +28,7 @@
> #include <linux/spinlock.h>
> #include <linux/list.h>
> #include <linux/file.h>
> +#include <linux/kmod.h>
> #include <scsi/scsi.h>
> #include <scsi/scsi_cmnd.h>
> #include <asm/unaligned.h>
> @@ -1916,7 +1917,33 @@ out:
> return ret;
> }
>
> -static int __core_scsi3_write_aptpl_to_file(
> +static char target_upcall_path[] = "/sbin/target_notify";
> +
> +static int core_scsi3_aptpl_upcall(struct se_device *dev, unsigned char *buf)
> +{
> + int ret;
> + char *argv[2], *envp[4];
> + /* space for wwn + 'APTPL=' + '\0' */
> + char aptpl_env[INQUIRY_VPD_SERIAL_LEN+7];
> +
> + snprintf(aptpl_env, sizeof(aptpl_env), "APTPL=%s", dev->t10_wwn.unit_serial);
> +
> + argv[0] = target_upcall_path;
> + argv[1] = NULL;
> +
> + envp[0] = "HOME=/";
> + envp[1] = "PATH=/sbin:/bin:/usr/sbin:/usr/bin";
> + envp[2] = aptpl_env;
> + envp[3] = NULL;
> +
> + ret = call_usermodehelper(argv[0], argv, envp, UMH_WAIT_PROC);
> + if (ret < 0)
> + pr_debug("target: upcall failed\n");
> +
> + return ret;
> +}
> +
> +static int core_scsi3_write_aptpl_to_file(
> struct se_device *dev,
> unsigned char *buf)
> {
> @@ -1954,6 +1981,21 @@ static int __core_scsi3_write_aptpl_to_file(
> return ret ? -EIO : 0;
> }
>
> +static int core_scsi3_save_aptpl(struct se_device *dev, unsigned char *buf)
> +{
> + int ret;
> +
> + /*
> + * Assume if user helper exists, it saved aptpl state.
> + * Otherwise, write it to the fs ourselves.
> + */
> + ret = core_scsi3_aptpl_upcall(dev, buf);
> + if (ret < 0)
> + ret = core_scsi3_write_aptpl_to_file(dev, buf);
> +
> + return ret;
> +}
> +
> /*
> * Clear the APTPL metadata if APTPL has been disabled, otherwise
> * write out the updated metadata to struct file for this SCSI device.
> @@ -1965,7 +2007,7 @@ static int core_scsi3_update_and_write_aptpl(struct se_device *dev, bool aptpl)
> if (!aptpl) {
> char *null_buf = "No Registrations or Reservations\n";
>
> - ret = __core_scsi3_write_aptpl_to_file(dev, null_buf);
> + ret = core_scsi3_save_aptpl(dev, null_buf);
> dev->t10_pr.pr_aptpl_active = 0;
> pr_debug("SPC-3 PR: Set APTPL Bit Deactivated\n");
> } else {
> @@ -1982,7 +2024,7 @@ static int core_scsi3_update_and_write_aptpl(struct se_device *dev, bool aptpl)
> return ret;
> }
>
> - ret = __core_scsi3_write_aptpl_to_file(dev, buf);
> + ret = core_scsi3_save_aptpl(dev, buf);
> if (ret < 0) {
> pr_err("SPC-3 PR: Could not update APTPL\n");
> } else {
--
To unsubscribe from this list: send the line "unsubscribe target-devel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
