Comment 12 for bug 1532198

Colin Ian King (colin-king) wrote :

Following up comments on #3

 the libraries install directly to /lib, is this intentional?
- efi_use_whole_disk() leaks &efi_label via VT_ENOSPC error return no leak:

        if ((efi_label->efi_altern_lba == 1) || (efi_label->efi_altern_lba
            >= efi_label->efi_last_lba)) {
                if (efi_debug) {
                        (void) fprintf(stderr,
                            "efi_use_whole_disk: requested space not found\n");
                return (VT_ENOSPC);

- vdev_elevator_switch() broken strncmp() test
   - bug filed upstream #4507, from code clean up which caused a regression

- _finish_daemonize() if devnull is 0, 1, 2, the close(devnull) call will
  leave the program without that fd
  * upstream fix 048bb5bd4950b9cb5368ed93d273f0f36e439122
    ("Ensure zed _finish_daemonize() leaves fds 0-2 open")
    - will SRU this

- zfs_fuid_node_add() modifies zfs_fuid_info_t list without obvious
  locking -- are calls to this routine suitably locked elsewhere?
   * checked this out, agreed, can't see locking,
    -- reported upstream, #4508

- ddt_zap_update() allocates cbuf[sizeof(dde_phys)] on the stack -- which
  is an array DDT_PHYS_TYPES long of structs which themselves have an
  embedded array of structs with further embedded array of integers. How
  likely is it for this to blow out the stack depth?

dde_phys is an array of DDT_PHYS_TYPES ddt_phys_t types, and ddt_phys_t is:

typedef struct ddt_phys {
        dva_t ddp_dva[SPA_DVAS_PER_BP];
        uint64_t ddp_refcnt;
        uint64_t ddp_phys_birth;
} ddt_phys_t;

and dva_t is 2 x uint64_t, SPA_DVAS_PER_BP is 3, so ddt_phys_t is

(2 x 8 x 3) + 8 + 8, making 64 bytes per ddt_phys_t. DDT_PHYS_TYPES is 4, making the allocation (64 * 4) + 1 = 257 bytes, so no problem at all, it's an acceptable size.

- zpool_vdev_remove() includes misleading error message that top-level
  devices can be removed
   * minor error message,
    - bug filed upstream #4506

- zpool_vdev_remove() awkward error message "pool must be upgrade to
  support log removal"
   - fixed in