Bug#666399: s390-dasd fails to work with >20 devices visible (mostly in LPAR mode)
Control: tags -1 + patch
On Tue, Feb 16, 2016 at 06:22:30PM +0100, Hendrik Brueckner wrote:
> On Tue, Feb 16, 2016 at 05:34:02PM +0100, Holger Wansing wrote:
> > Hendrik Brueckner <brueckner@linux.vnet.ibm.com> wrote:
> > > On Sun, Feb 14, 2016 at 05:46:39PM +0100, Philipp Kern wrote:
> > > > > For example,
> > > > >
> > > > > cio_ignore=all,!ipldev,!condev,!0.0.da00-0.0.da10
> > > > >
> > > > > ignores all devices except the console device, the IPL device, and the
> > > > > range of devices from 0.0.da00 to 0.0.da10 that might be DASDs or any
> > > > > other devices. Note that you can change the cio_ignore settings at runtime,
> > > > > so you can later make additional devices visible.
> > > > >
> > > > > With this solution, there is no hardcoded limit necessary and the user
> > > > > can still see the list of DASDs to be configured.
> > > > >
> > > > > What do you think?
> > > >
> > > > it'd be nice to see that in the installation manual, I think, so that
> > > > it's at least documented.
>
> Actually, this is already documented in the installation manual:
>
> https://d-i.debian.org/manual/en.s390x/ch05s01.html
Because this is already documented, below a patch that removes the
get_channel() function to handle more than 20 DASDs. If there are
numerous DASDs, the cio_ignore could be used to limit the list to
those that are required for the Linux instance / installer.
The patch also corrects a stack overflow for displaying DASDs which
similar to the one in s390-netdevice which was corrected few weeks
ago.
Thanks and kind regards,
Hendrik
>From 04236b0f262476dfdc84fc751f72dd65519be83f Mon Sep 17 00:00:00 2001
From: Hendrik Brueckner <brueckner@linux.vnet.ibm.com>
Date: Thu, 14 Jan 2016 17:04:36 +0100
Subject: [PATCH] dasd-config: always list available DASDs for selection and configuration
Previously, the DASD configuration module prompted the user to specify
DASDs if there were more than 20 DASDs available. If this dialog is
shown, the user can configure DASD devices; but can only return to the
installer through the <Go Back> button. The <Go Back> triggers a special
debconf return code and the "hardware-detection" dependency for this is
not satisfied. Later the partitioner will complain and the intermediate
configuration file might not be copied to the target system.
The second disadvantage with prompt is that the user does not see which
DASDs are already online and configured. See also Debian bug 666399 for
more details.
This commit removes the prompt to manually enter DASDs. The panel to
select particular DASDs is displayed instead.
Of course, there can be the case where LPARs have access to (almost)
every device. For this case, the solution is to use the "cio_ignore"
kernel parameter. With this kernel parameter you can control the
devices that are visible to the Linux instance.
For example,
cio_ignore=all,!ipldev,!condev,!0.0.da00-0.0.da10
ignores all devices except the console device, the IPL device, and the
range of devices from 0.0.da00 to 0.0.da10 that might be DASDs or any
other devices. Note that you can change the cio_ignore settings at
runtime, so you can later make additional devices visible.
With this solution, there is no hardcoded limit necessary and the user
can still see the list of DASDs to be configured.
Signed-off-by: Hendrik Brueckner <brueckner@linux.vnet.ibm.com>
---
dasd-config.c | 71 ++++++++++++++++++++++++++++-----------------------------
1 files changed, 35 insertions(+), 36 deletions(-)
diff --git a/dasd-config.c b/dasd-config.c
index e085534..daa53a4 100644
--- a/dasd-config.c
+++ b/dasd-config.c
@@ -18,6 +18,7 @@
#define SYSCONFIG_DIR "/etc/sysconfig/hardware/"
#define TEMPLATE_PREFIX "s390-dasd/"
+#define DASD_ENTRY_SIZE 32
static struct debconfclient *client;
@@ -173,62 +174,62 @@ static enum state_wanted detect_channels (void)
return WANT_NEXT;
}
-static enum state_wanted get_channel_input (void)
+struct buffer_desc
{
- int ret, dev;
- char *ptr;
-
- while (1)
- {
- ret = my_debconf_input ("critical", TEMPLATE_PREFIX "choose", &ptr);
- if (ret == 30)
- return WANT_BACKUP;
-
- dev = channel_device (ptr);
- if (dev >= 0)
- {
- channel_current = di_tree_lookup (channels, &dev);
- if (channel_current)
- return WANT_NEXT;
- }
-
- ret = my_debconf_input ("critical", TEMPLATE_PREFIX "choose_invalid", &ptr);
- if (ret == 30)
- return WANT_BACKUP;
- }
-}
+ char *buf;
+ size_t size;
+};
static di_hfunc get_channel_select_append;
static void get_channel_select_append (void *key __attribute__ ((unused)), void *value, void *user_data)
{
struct channel *channel = value;
- char *buf = user_data;
- if (buf[0])
- strncat (buf, ", ", 1024);
- strncat (buf, channel->name, 1024);
+ struct buffer_desc *bd = user_data;
+
+ if (bd->buf[0])
+ di_snprintfcat (bd->buf, bd->size, ", ");
+ di_snprintfcat(bd->buf, bd->size, channel->name);
if (channel->configured)
- strncat (buf, " (configured)", 1024);
+ di_snprintfcat (bd->buf, bd->size, " (configured)");
else if (channel->online)
- strncat (buf, " (online)", 1024);
+ di_snprintfcat (bd->buf, bd->size, " (online)");
}
static enum state_wanted get_channel_select (void)
{
- char buf[1024], *ptr;
+ struct buffer_desc bd;
int ret, dev;
+ char *ptr;
- buf[0] = '\0';
- di_tree_foreach (channels, get_channel_select_append, buf);
+ /*
+ * Allocate memory to store DASD bus-IDs along with their
+ * online or configured state information.
+ */
+ bd.size = DASD_ENTRY_SIZE * di_tree_size (channels);
+ bd.buf = di_malloc0 (bd.size);
+ if (bd.buf == NULL)
+ {
+ di_warning ("Could not allocate memory for DASD list\n");
+ return WANT_ERROR;
+ }
+ di_tree_foreach (channels, get_channel_select_append, &bd);
- debconf_subst (client, TEMPLATE_PREFIX "choose_select", "choices", buf);
+ debconf_subst (client, TEMPLATE_PREFIX "choose_select", "choices", bd.buf);
ret = my_debconf_input ("critical", TEMPLATE_PREFIX "choose_select", &ptr);
if (ret == 30)
+ {
+ di_free (bd.buf);
return WANT_BACKUP;
+ }
if (!strcmp (ptr, "Finish"))
+ {
+ di_free (bd.buf);
return WANT_FINISH;
+ }
dev = channel_device (ptr);
+ di_free (bd.buf);
channel_current = di_tree_lookup (channels, &dev);
if (channel_current)
return WANT_NEXT;
@@ -238,9 +239,7 @@ static enum state_wanted get_channel_select (void)
static enum state_wanted get_channel (void)
{
- if (di_tree_size (channels) > 20)
- return get_channel_input ();
- else if (di_tree_size (channels) > 0)
+ if (di_tree_size (channels) > 0)
return get_channel_select ();
di_info("s390-dasd: no channel found");
return WANT_FINISH;
--
1.7.1
Reply to: