Opened 4 months ago

Closed 3 months ago

#1413 closed defect (fixed)

Some SCSI Drives spin up when using -n option.

Reported by: Simon Fairweather Owned by: Christian Franke
Priority: minor Milestone: Release 7.3
Component: smartctl Version:
Keywords: scsi Cc:

Description

Some SCSI/SAS drives will spin up even if -n option is used.

This does not affect all drives.

I have noticed this only a SEAGATE drive at present.

Issue can be resolved by moving the following code to after the check for -n

if (supported_vpd_pages_p) {

delete supported_vpd_pages_p;
supported_vpd_pages_p = NULL;

}
supported_vpd_pages_p = new supported_vpd_pages(device);

to after the -n check code.

Change History (6)

comment:1 Changed 4 months ago by Christian Franke

Component: allsmartctl
Keywords: scsi added
Milestone: undecided

Code could also be simplified to:

  delete supported_vpd_pages_p;
  supported_vpd_pages_p = new supported_vpd_pages(device);

comment:2 Changed 4 months ago by Simon Fairweather

I have just moved the code as unsure of effect on simplification.

Patch

From 5c6b5e38d56d46fec72f3a2b3c2e2eae16e2f9fe Mon Sep 17 00:00:00 2001
From: SimonFair? <39065407+SimonFair@…>
Date: Thu, 24 Dec 2020 08:26:01 +0000
Subject: [PATCH] Move vpd_pages after -n processing

---

smartmontools/scsiprint.cpp | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/smartmontools/scsiprint.cpp b/smartmontools/scsiprint.cpp
index 55ab0fdc..46a07f3f 100644
--- a/smartmontools/scsiprint.cpp
+++ b/smartmontools/scsiprint.cpp
@@ -2305,12 +2305,6 @@ scsiPrintMain(scsi_device * device, const scsi_print_options & options)

bool is_tape;
bool any_output = options.drive_info;


  • if (supported_vpd_pages_p) {
  • delete supported_vpd_pages_p;
  • supported_vpd_pages_p = NULL;
  • }
  • supported_vpd_pages_p = new supported_vpd_pages(device);

-

Enable -n option for SCSI Drives

const char * powername = NULL;
bool powerchg = false;

@@ -2386,6 +2380,12 @@ scsiPrintMain(scsi_device * device, const scsi_print_options & options)

powername = "ACTIVE";

}


+ if (supported_vpd_pages_p) {
+ delete supported_vpd_pages_p;
+ supported_vpd_pages_p = NULL;
+ }
+ supported_vpd_pages_p = new supported_vpd_pages(device);
+

res = scsiGetDriveInfo(device, &peripheral_type, options.drive_info);
if (res) {

if (2 == res)

comment:3 Changed 3 months ago by Simon Fairweather

Hi, any updates when this will be reviewed/fixed?

comment:4 Changed 3 months ago by Christian Franke

Please include the suggested obvious simplification from above and re-test the code. This also avoids a warning about the useless assignment supported_vpd_pages_p = NULL;.
Add an entry to the ChangeLog file.

To avoid reformatting of the resulting patch, use a plain-text attachment or wiki markup.

comment:5 Changed 3 months ago by Christian Franke

Milestone: undecidedRelease 7.3
Owner: set to Christian Franke
Status: newaccepted

comment:6 Changed 3 months ago by Christian Franke

Resolution: fixed
Status: acceptedclosed

GH pull/84 (b82cb9b) applied with a few whitespace/newline only changes. Thanks.

Note: See TracTickets for help on using tickets.