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, 8: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, 8: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, 8: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)