8983 libbe: be_do_installboot() fails with vdev replacing

Reviews
Diff

- Download Diff

Information
Submitter:	Toomas Soome
Repository:	illumos-gate
Branch:
Bugs:	8983
Depends On:
Commit:	e90780d...
Reviewers
Groups:	general
People:

Description

8983: libbe: be_do_installboot() fails with vdev replacing

Testing Done

root@uefi-oi:/home/tsoome# beadm activate -v uefi-199
be_do_installboot: child 0 of 1 device mirror-0
be_do_installboot: child 0 of 2 device c3t0d0
  Command: "/usr/sbin/installboot -m -f //boot/pmbr //boot/gptzfsboot /dev/rdsk/c3t0d0s1"
  Errors:
bootblock version installed on /dev/rdsk/c3t0d0s1 is more recent or identical
Use -F to override or install without the -u option
be_do_installboot: child 1 of 2 device replacing-1
be_do_installboot: child 0 of 2 device c6t3d0/old
be_do_installboot: vdev /dev/dsk/c6t3d0s1/old is OFFLINE, can't install boot loader
be_do_installboot: child 1 of 2 device c6t3d0
  Command: "/usr/sbin/installboot -m -f //boot/pmbr //boot/gptzfsboot /dev/rdsk/c6t3d0s1"
  Errors:
bootblock version installed on /dev/rdsk/c6t3d0s1 is more recent or identical
Use -F to override or install without the -u option
Activated successfully
root@uefi-oi:/home/tsoome#

From the above, we have mirror with c6t3d0 being replaced, we are recursing into vdev "replacing-1", we are detecting the offline device and ignoring it, and we do have installboot being executed for new device.

Issues

0
1
0
All Issues: 1

Description	From	Last Updated
If these messages are supposed to help with tracing where we failed, shouldn't we print correct function names? i.e. use ...	Yuri Pankov	June 21, 2018, 11:45 a.m.

Summary:

-	8983: libbe: be_do_installboot() fails with vdev replacing
+	8983 libbe: be_do_installboot() fails with vdev replacing

Ship it!

Ship It!

usr/src/lib/libbe/common/be_activate.c (Diff revision 1)

Show all issues

If these messages are supposed to help with tracing where we failed, shouldn't we print correct function names? i.e. use "%s:....", __func__, ... here instead.

usr/src/lib/libbe/common/be_activate.c (Diff revision 1)

I understand that we have C99 enabled, but it looks like more of a style issue where we define all variables at the start of function.

Toomas Soome June 20, 2018, 11:05 p.m.

I am not sure really; this for was actually moved - it was already there. Also the zfs code is filled with constructs like that.

usr/src/lib/libbe/common/be_activate.c (Diff revision 1)

Should have a whitespace after -1?

Toomas Soome June 20, 2018, 11:05 p.m.

cstyle does not complain:

tsoome@beastie:/code/8983$ git pbchk
Copyrights:
usr/src/lib/libbe/common/be_activate.c: no copyright claim for current year found

tsoome@beastie:/code/8983$

Change Summary:

use func as suggested by Yuri.

Commit:

-	1f49fd9197912620ec75527e880b23744a260144
+	079240b7493bf79a36865c92c68b90413809fb66

Diff:

Revision 2 (+108 -113)

Show changes

usr/src/lib/libbe/common/be_activate.c

Ship it!

Ship It!

Ship it!

Ship It!

Change Summary:

debug mode build did complain about unused variables.

Commit:

-	079240b7493bf79a36865c92c68b90413809fb66
+	e90780dedb7a26788f0a0f774eca6e540613e78d

Diff:

Revision 3 (+109 -117)

Show changes

usr/src/lib/libbe/common/be_activate.c

Ship it!

Ship It!

You have a pending review.

Review Board 2.5.16

Screenshots

Files

Issues

Summary:

Change Summary:

Commit:

Diff:

Change Summary:

Commit:

Diff:

Status: Closed (submitted)