zipl/src: Fix problems when target parameters are specified by user

Steps to reproduce: Prepare some disk for IPL, specifying its
parameters via zipl "target options", and an image IMAGE_NAME
located on another disk. Don't specify "-a" option.

Actual result: Installation succeeded (resulting in unbootable setup)
Expected result: "Error: Could not add image file 'IMAGE_NAME': File
is not on target device"

The problem is in incorrect evaluation of device number (dev_t)
of the base disk where the image is located by the function
add_component_file_range() in case when target parameters are
specified by user.

Fixup: Retrieve info of the underlying disk without any user hints,
passing zeroed structure job_target_data

This is an improved version of fb0b6263d that was reverted by
63ff07ba3 afterwards. The shortcoming of the original ( fb0b6263d )
fix: it fails to prepare for IPL mounted qcow2 images.
The difference from the original fix: when making sure that boot
files are located on the target disk: in case of failed auto-detection
of disk parameters skip the check with warnings instead of aborting
the whole installation session.

Acked-by: Jan Höppner <hoeppner@linux.ibm.com>
Signed-off-by: Eduard Shishkin <edward6@linux.ibm.com>
Signed-off-by: Jan Höppner <hoeppner@linux.ibm.com>
This commit is contained in:
Eduard Shishkin
2024-06-26 20:22:07 +02:00
committed by Jan Höppner
parent 5218c243f4
commit c0f02d2f68
4 changed files with 39 additions and 24 deletions

View File

@@ -152,6 +152,7 @@ static inline int target_parameters_are_set(struct job_target_data *td)
int job_get(int argc, char* argv[], struct job_data** data); int job_get(int argc, char* argv[], struct job_data** data);
void job_free(struct job_data* job); void job_free(struct job_data* job);
void free_target_data(struct job_target_data *td);
int type_from_target(char *target, disk_type_t *type); int type_from_target(char *target, disk_type_t *type);
int check_job_dump_images(struct job_dump_data* dump, char* name); int check_job_dump_images(struct job_dump_data* dump, char* name);
int check_job_images_ngdump(struct job_dump_data* dump, char* name); int check_job_images_ngdump(struct job_dump_data* dump, char* name);

View File

@@ -304,7 +304,6 @@ static int add_component_file_range(struct install_set *bis,
address_t load_address, address_t load_address,
size_t trailer, void *component, size_t trailer, void *component,
int add_files, int add_files,
struct job_target_data *target,
int comp_id, int menu_idx, int comp_id, int menu_idx,
int program_table_id) int program_table_id)
{ {
@@ -339,11 +338,28 @@ static int add_component_file_range(struct install_set *bis,
return -1; return -1;
} }
} else { } else {
/* Make sure file is on correct device */ /*
rc = disk_get_info_from_file(filename, target, &file_info); * Make sure that file is on target device.
if (rc) * For this, retrieve info of the underlying disk without
return -1; * any user hints
if (file_info->device != bis->info->device) { */
struct job_target_data tmp = {.source = source_unknown};
rc = disk_get_info_from_file(filename, &tmp, &file_info);
free_target_data(&tmp);
if (rc) {
/*
* In some cases it is impossible to auto-detect
* disk parameters (e.g. when the file is on a
* mounted qcow2 image).
* Skip the check with warnings.
*/
fprintf(stderr,
"Warning: Could not auto-detect disk parameters for %s\n",
filename);
fprintf(stderr,
"Warning: Preparing a logical device for boot might fail\n");
} else if (file_info->device != bis->info->device) {
disk_free_info(file_info); disk_free_info(file_info);
error_reason("File is not on target device"); error_reason("File is not on target device");
return -1; return -1;
@@ -377,12 +393,11 @@ write_segment_table:
static int add_component_file(struct install_set *bis, const char *filename, static int add_component_file(struct install_set *bis, const char *filename,
address_t load_address, size_t trailer, address_t load_address, size_t trailer,
void *component, int add_files, void *component, int add_files,
struct job_target_data *target, int comp_id, int comp_id, int menu_idx, int program_table_id)
int menu_idx, int program_table_id)
{ {
return add_component_file_range(bis, filename, NULL, load_address, return add_component_file_range(bis, filename, NULL, load_address,
trailer, component, add_files, trailer, component, add_files,
target, comp_id, menu_idx, comp_id, menu_idx,
program_table_id); program_table_id);
} }
@@ -544,8 +559,7 @@ static int add_ipl_program(struct install_set *bis, char *filename,
bool add_envblk, struct job_envblk_data *envblk, bool add_envblk, struct job_envblk_data *envblk,
struct job_ipl_data *ipl, disk_blockptr_t *program, struct job_ipl_data *ipl, disk_blockptr_t *program,
int verbose, int add_files, component_header_type type, int verbose, int add_files, component_header_type type,
struct job_target_data *target, int is_secure, int is_secure, int menu_idx, int program_table_id)
int menu_idx, int program_table_id)
{ {
struct signature_header sig_head; struct signature_header sig_head;
size_t ramdisk_size, image_size; size_t ramdisk_size, image_size;
@@ -667,7 +681,7 @@ static int add_ipl_program(struct install_set *bis, char *filename,
/* Add stage 3 loader to bootmap */ /* Add stage 3 loader to bootmap */
rc = add_component_file(bis, ZIPL_STAGE3_PATH, STAGE3_LOAD_ADDRESS, rc = add_component_file(bis, ZIPL_STAGE3_PATH, STAGE3_LOAD_ADDRESS,
signature_size, VOID_ADD(table, offset), 1, signature_size, VOID_ADD(table, offset), 1,
target, COMPONENT_ID_LOADER, menu_idx, COMPONENT_ID_LOADER, menu_idx,
program_table_id); program_table_id);
if (rc) { if (rc) {
error_text("Could not add internal loader file '%s'", error_text("Could not add internal loader file '%s'",
@@ -745,7 +759,7 @@ static int add_ipl_program(struct install_set *bis, char *filename,
rc = add_component_file(bis, ipl->common.image, ipl->common.image_addr, rc = add_component_file(bis, ipl->common.image, ipl->common.image_addr,
signature_size, VOID_ADD(table, offset), signature_size, VOID_ADD(table, offset),
add_files, target, COMPONENT_ID_KERNEL_IMAGE, add_files, COMPONENT_ID_KERNEL_IMAGE,
menu_idx, program_table_id); menu_idx, program_table_id);
if (rc) { if (rc) {
error_text("Could not add image file '%s'", ipl->common.image); error_text("Could not add image file '%s'", ipl->common.image);
@@ -805,7 +819,7 @@ static int add_ipl_program(struct install_set *bis, char *filename,
ipl->common.ramdisk_addr, ipl->common.ramdisk_addr,
signature_size, signature_size,
VOID_ADD(table, offset), VOID_ADD(table, offset),
add_files, target, COMPONENT_ID_RAMDISK, add_files, COMPONENT_ID_RAMDISK,
menu_idx, program_table_id); menu_idx, program_table_id);
if (rc) { if (rc) {
error_text("Could not add ramdisk '%s'", error_text("Could not add ramdisk '%s'",
@@ -861,7 +875,7 @@ static int add_ipl_program(struct install_set *bis, char *filename,
rc = add_component_file_range(bis, filename, &reg, rc = add_component_file_range(bis, filename, &reg,
ipl->envblk_addr, 0, ipl->envblk_addr, 0,
VOID_ADD(table, offset), VOID_ADD(table, offset),
0, target, 0,
COMPONENT_ID_ENVBLK, COMPONENT_ID_ENVBLK,
menu_idx, menu_idx,
program_table_id); program_table_id);
@@ -893,7 +907,6 @@ static int add_segment_program(struct install_set *bis,
struct job_segment_data *segment, struct job_segment_data *segment,
disk_blockptr_t *program, int verbose, disk_blockptr_t *program, int verbose,
int add_files, component_header_type type, int add_files, component_header_type type,
struct job_target_data *target,
int program_table_id) int program_table_id)
{ {
void *table; void *table;
@@ -913,7 +926,7 @@ static int add_segment_program(struct install_set *bis,
printf(" segment file......: %s\n", segment->segment); printf(" segment file......: %s\n", segment->segment);
rc = add_component_file(bis, segment->segment, segment->segment_addr, 0, rc = add_component_file(bis, segment->segment, segment->segment_addr, 0,
VOID_ADD(table, offset), add_files, target, VOID_ADD(table, offset), add_files,
COMPONENT_ID_SEGMENT_FILE, 0 /* menu_idx */, COMPONENT_ID_SEGMENT_FILE, 0 /* menu_idx */,
program_table_id); program_table_id);
if (rc) { if (rc) {
@@ -996,7 +1009,7 @@ static int add_dump_program(struct install_set *bis, struct job_data *job,
ipl.common.parmline = dump->common.parmline; ipl.common.parmline = dump->common.parmline;
ipl.common.parm_addr = dump->common.parm_addr; ipl.common.parm_addr = dump->common.parm_addr;
return add_ipl_program(bis, NULL, false, NULL, &ipl, program, return add_ipl_program(bis, NULL, false, NULL, &ipl, program,
verbose, 1, type, target, SECURE_BOOT_DISABLED, verbose, 1, type, SECURE_BOOT_DISABLED,
0 /* menu_idx */, program_table_id); 0 /* menu_idx */, program_table_id);
} }
@@ -1041,8 +1054,7 @@ static int build_program_table(struct job_data *job,
true, &job->envblk, &job->data.ipl, true, &job->envblk, &job->data.ipl,
&table[0], verbose || job->command_line, &table[0], verbose || job->command_line,
job->add_files, component_header, job->add_files, component_header,
&job->target, job->is_secure, 0, job->is_secure, 0, program_table_id);
program_table_id);
break; break;
case job_segment: case job_segment:
if (bis->print_details) { if (bis->print_details) {
@@ -1055,7 +1067,7 @@ static int build_program_table(struct job_data *job,
rc = add_segment_program(bis, &job->data.segment, &table[0], rc = add_segment_program(bis, &job->data.segment, &table[0],
verbose || job->command_line, verbose || job->command_line,
job->add_files, COMPONENT_HEADER_IPL, job->add_files, COMPONENT_HEADER_IPL,
&job->target, program_table_id); program_table_id);
break; break;
case job_dump_partition: case job_dump_partition:
/* Only useful for a partition dump that uses a dump kernel*/ /* Only useful for a partition dump that uses a dump kernel*/
@@ -1114,7 +1126,7 @@ static int build_program_table(struct job_data *job,
&table[job->data.menu.entry[i].pos], &table[job->data.menu.entry[i].pos],
verbose || job->command_line, verbose || job->command_line,
job->add_files, component_header, job->add_files, component_header,
&job->target, is_secure, i, is_secure, i,
program_table_id); program_table_id);
break; break;
case job_print_usage: case job_print_usage:

View File

@@ -528,6 +528,9 @@ static int disk_set_info_complete(struct job_target_data *td,
* config file, or special "target options" of zipl tool. * config file, or special "target options" of zipl tool.
* If target parameters were specified by user, then the step 1 above * If target parameters were specified by user, then the step 1 above
* is skipped. * is skipped.
* To exclude any user assumptions about the DEVICE, this function
* should be called with TD pointing to a zeroed structure.
* *
* DEVICE: logical, or physical device, optionally formated with a * DEVICE: logical, or physical device, optionally formated with a
* file system. * file system.

View File

@@ -370,8 +370,7 @@ get_command_line(int argc, char* argv[], struct command_line* line)
} }
static void void free_target_data(struct job_target_data *data)
free_target_data(struct job_target_data* data)
{ {
free(data->bootmap_dir); free(data->bootmap_dir);
free(data->targetbase); free(data->targetbase);