Bug#815918: dasdfmt: detect format status and improve handling
Package: s390-dasd
Version: 0.0.34
Severity: normal
Tags: d-i patch
Hi,
below you can find three patches to improve the low-level formatting
of DASDs. After an DASD has been enabled (set online), the status
sysfs attribute can be used to check if a DASD is formatted. Depending
on the value of the status (n/f for non-formatted), the priority and
default answer for the ""Format the DASD" question is properly set.
The dasdfmt program fails if the DASD to be formatted is already in use.
This might happen if the partitioner detected a physial volume which
becomes part of an mapped LVM device. In such a case, the error handling
is improved.
Also the "Go back" button was not correctly detected because the code
tested for a wrong return code. This has been corrected too.
Kind regards,
Hendrik
>From 74df2f8fe686d29e19b937332127f514531b5d33 Mon Sep 17 00:00:00 2001
From: Hendrik Brueckner <brueckner@linux.vnet.ibm.com>
Date: Thu, 11 Feb 2016 18:18:48 +0100
Subject: [PATCH 1/4] dasdfmt: report user error if disk is in use
Display an error message if the disk to be low-level formatted is in use.
This might happen if disk was or is used as physical volume for LVM. If
the DASD is already mapped, for example, from going back from the
partitioner, the DASD cannot be low-level formatted.
Signed-off-by: Hendrik Brueckner <brueckner@linux.vnet.ibm.com>
Reviewed-by: Viktor Mihajlovski <mihajlov@linux.vnet.ibm.com>
---
dasd-config.c | 18 ++++++++++++++++++
debian/s390-dasd.templates | 7 +++++++
2 files changed, 25 insertions(+)
diff --git a/dasd-config.c b/dasd-config.c
index 37a0fec..b8d9203 100644
--- a/dasd-config.c
+++ b/dasd-config.c
@@ -326,7 +326,25 @@ static enum state_wanted format (void)
debconf_progress_stop (client);
if (ret)
+ {
+ if (di_exec_mangle_status (ret) == 2)
+ {
+ /*
+ * dasdfmt failed because the DASD is in use. For example,
+ * it is mapped as part of an LVM.
+ */
+ debconf_subst (client, TEMPLATE_PREFIX "format_disk_in_use",
+ "device", channel_current->name);
+ debconf_input (client, "critical",
+ TEMPLATE_PREFIX "format_disk_in_use");
+ debconf_capb (client);
+ ret = debconf_go (client);
+ debconf_capb (client, "backup");
+
+ return WANT_BACKUP;
+ }
return WANT_ERROR;
+ }
return WANT_NEXT;
}
diff --git a/debian/s390-dasd.templates b/debian/s390-dasd.templates
index 2c307be..8d443c0 100644
--- a/debian/s390-dasd.templates
+++ b/debian/s390-dasd.templates
@@ -36,6 +36,13 @@ _Description: Format the device?
If you are sure the device has already been correctly formatted, you don't
need to do so again.
+Template: s390-dasd/format_disk_in_use
+Type: error
+_Description: The DASD ${device} is in use
+ Could not low-level format the DASD ${device} because the DASD
+ is in use. For example, the DASD could be a member of a mapped device in
+ an LVM volume group.
+
Template: s390-dasd/formatting
Type: text
# :sl5:
--
2.7.0
>From aa69e1cd3390444f4b88ef6eaa6eafff07e71aeb Mon Sep 17 00:00:00 2001
From: Hendrik Brueckner <brueckner@linux.vnet.ibm.com>
Date: Fri, 12 Feb 2016 11:04:17 +0100
Subject: [PATCH 2/4] dasdfmt: detect and properly handle already formatted
DASDs
When the DASD has been set online, the device driver provides basic
information about the status of the DASD. The status can be used
to detect whether a DASD is low-level formatted.
If the DASD is already low-level formatted, properly set the default
and reduce the priority for asking the user to low-level format the
DASD.
Signed-off-by: Hendrik Brueckner <brueckner@linux.vnet.ibm.com>
Reviewed-by: Viktor Mihajlovski <mihajlov@linux.vnet.ibm.com>
---
dasd-config.c | 72 +++++++++++++++++++++++++++++++++++++++++++---
debian/s390-dasd.templates | 16 +++++++----
2 files changed, 79 insertions(+), 9 deletions(-)
diff --git a/dasd-config.c b/dasd-config.c
index b8d9203..a25d992 100644
--- a/dasd-config.c
+++ b/dasd-config.c
@@ -33,6 +33,7 @@ struct channel
char devtype[SYSFS_NAME_LEN];
bool configured;
bool online;
+ bool formatted;
enum channel_type type;
};
@@ -293,15 +294,75 @@ static int format_handler (const char *buf, size_t len, void *user_data __attrib
return 0;
}
+static bool dasd_is_formatted (const char *name)
+{
+ int tries;
+ bool formatted;
+ struct sysfs_device *device;
+ struct sysfs_attribute *status;
+
+ formatted = false;
+ device = sysfs_open_device ("ccw", name);
+ if (!device)
+ {
+ di_warning ("s390-dasd: could not open device %s", name);
+ return false;
+ }
+
+ /* Examine the status of the DASD to retrieve information whether
+ * the DASD is already low-level formatted.
+ */
+ status = sysfs_get_device_attr (device, "status");
+ if (!status)
+ {
+ di_warning ("s390-dasd: could not find status attribute for %s", name);
+ sysfs_close_device (device);
+ return false;
+ }
+
+ tries = 5;
+ while (tries--)
+ {
+ sysfs_read_attribute (status);
+
+ /* DASD is low-level formatted and in the online state ? */
+ if (!strcmp (status->value, "online\n"))
+ {
+ formatted = true;
+ break;
+ }
+
+ /* DASD is not low-level formatted ? */
+ if (!strcmp (status->value, "unformatted\n"))
+ break;
+
+ /* Give the DASD device driver some time to complete
+ * DASD online processing.
+ */
+ usleep (250000);
+ }
+
+ sysfs_close_device (device);
+ return formatted;
+}
+
static enum state_wanted format (void)
{
- char buf[256], dev[128], *ptr;
+ char buf[256], dev[128], *ptr, *template;
int fd, ret;
struct hd_geometry drive_geo;
- debconf_subst (client, TEMPLATE_PREFIX "format", "device", channel_current->name);
- debconf_set (client, TEMPLATE_PREFIX "format", "false");
- ret = my_debconf_input ("critical", TEMPLATE_PREFIX "format", &ptr);
+ /* Retrieve information about the format status of the DASD */
+ channel_current->formatted = dasd_is_formatted (channel_current->name);
+ template = channel_current->formatted ? TEMPLATE_PREFIX "format"
+ : TEMPLATE_PREFIX "format_required";
+ /*
+ * Depending on whether the DASD is already formatted,
+ * use the respective template and priority.
+ */
+ debconf_subst (client, template, "device", channel_current->name);
+ ret = my_debconf_input (channel_current->formatted ? "low" : "critical",
+ template, &ptr);
if (ret == 10)
return WANT_BACKUP;
@@ -344,6 +405,9 @@ static enum state_wanted format (void)
return WANT_BACKUP;
}
return WANT_ERROR;
+ } else {
+ /* DASD successfully low-level formatted */
+ channel_current->formatted = true;
}
return WANT_NEXT;
diff --git a/debian/s390-dasd.templates b/debian/s390-dasd.templates
index 8d443c0..38226e0 100644
--- a/debian/s390-dasd.templates
+++ b/debian/s390-dasd.templates
@@ -27,14 +27,20 @@ _Description: Invalid device
Template: s390-dasd/format
Type: boolean
+Default: false
# :sl5:
_Description: Format the device?
- The installer is unable to detect if the device ${device} has already
- been formatted or not. Devices need to be formatted before you can
- create partitions.
+ The DASD ${device} is already low-level formatted.
.
- If you are sure the device has already been correctly formatted, you don't
- need to do so again.
+ Select "Yes" to format again and remove any data on the DASD again.
+ Note that formatting a DASD might take a long time.
+
+Template: s390-dasd/format_required
+Type: boolean
+Default: true
+_Description: Format the device?
+ The DASD ${device} is not low-level formatted. DASDs must be formatted
+ before you can create partitions.
Template: s390-dasd/format_disk_in_use
Type: error
--
2.7.0
>From 686d0c0cf2ee621508283148588d3f2a9c360ae8 Mon Sep 17 00:00:00 2001
From: Hendrik Brueckner <brueckner@linux.vnet.ibm.com>
Date: Fri, 12 Feb 2016 16:55:00 +0100
Subject: [PATCH 3/4] dasdfmt: correct and improve <Go back> handling
Selecting <Go back> for the DASD format question was ignored because
of checking a different return value. Correct this behavior to handle
<Go back> properly and, thus, add the FORMAT state to the state machine
to return the DASD selection state.
Because the installer knows about whether the DASD is formatted or not,
the module can be more restrictive in handling the selected options.
Ensure that the question is always reset to its default value and let
the user do not configure DASDs that are not formatted.
Signed-off-by: Hendrik Brueckner <brueckner@linux.vnet.ibm.com>
Reviewed-by: Viktor Mihajlovski <mihajlov@linux.vnet.ibm.com>
---
dasd-config.c | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/dasd-config.c b/dasd-config.c
index a25d992..6194da7 100644
--- a/dasd-config.c
+++ b/dasd-config.c
@@ -358,13 +358,17 @@ static enum state_wanted format (void)
: TEMPLATE_PREFIX "format_required";
/*
* Depending on whether the DASD is already formatted,
- * use the respective template and priority.
+ * use the respective template, its default, and priority.
*/
+ debconf_reset (client, template);
debconf_subst (client, template, "device", channel_current->name);
ret = my_debconf_input (channel_current->formatted ? "low" : "critical",
template, &ptr);
- if (ret == 10)
+ if (ret == CMD_GOBACK)
+ return WANT_BACKUP;
+ /* Prohibit to configure a DASD that is not formatted */
+ if (strcmp (ptr, "true") && !channel_current->formatted)
return WANT_BACKUP;
if (strcmp (ptr, "true"))
return WANT_NEXT;
@@ -506,6 +510,7 @@ int main ()
case GET_CHANNEL:
state = BACKUP;
break;
+ case FORMAT:
case WRITE:
state = GET_CHANNEL;
break;
--
2.7.0
Reply to: