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 <brueck...@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 <brueck...@linux.vnet.ibm.com> Reviewed-by: Viktor Mihajlovski <mihaj...@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 <brueck...@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 <brueck...@linux.vnet.ibm.com> Reviewed-by: Viktor Mihajlovski <mihaj...@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 <brueck...@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 <brueck...@linux.vnet.ibm.com> Reviewed-by: Viktor Mihajlovski <mihaj...@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