Page MenuHomeFreeBSD

rew (Rob Wing)
User

Projects

User Details

User Since
Nov 14 2018, 8:11 PM (237 w, 4 d)

Recent Activity

Fri, May 19

rew added a comment to D40132: taclib: 0 (not set) is a valid auth type in authorization requests..

looks like this is value is stated in RFC8907 section 6.1 under the authen_type header

Fri, May 19, 7:00 PM
rew accepted D40132: taclib: 0 (not set) is a valid auth type in authorization requests..
Fri, May 19, 6:56 PM

Tue, May 16

rew accepted D40109: bhyve: [snapshot] Use pci_next() to save/restore pci devices.

Thanks

Tue, May 16, 4:27 PM · bhyve
rew added a comment to D40108: bhyve: [snapshot] Add .pe_snapshot method for PCI 'hostbridge'.

At least it saves pdi.pi_cfgdata. Do you think it is waste doing and .pi_cfgdata can not be changed by guest OS or be changed between bhyve versions ?

Tue, May 16, 3:46 PM · bhyve

Mon, May 15

rew added a comment to D40109: bhyve: [snapshot] Use pci_next() to save/restore pci devices.

small nits - other than that, looks good

Mon, May 15, 5:57 PM · bhyve
rew accepted D40108: bhyve: [snapshot] Add .pe_snapshot method for PCI 'hostbridge'.

not sure that devices should define a snapshot handler when not supported by the device.

Mon, May 15, 5:39 PM · bhyve
rew accepted D40105: bhyve: [snapshot] Simplify restore kernel structs.
Mon, May 15, 5:33 PM · bhyve
rew accepted D40104: bhyve: [snapshot] Rename 'struct' key with 'kern_struct'.
Mon, May 15, 5:24 PM · bhyve
rew accepted D40106: bhyve: [snapshot] Rename 'user_dev' with 'devices'.
Mon, May 15, 5:23 PM · bhyve

Fri, May 12

rew added inline comments to D40076: bhyve: error out if fwcfg user file isn't read completely.
Fri, May 12, 9:33 PM
rew added inline comments to D40076: bhyve: error out if fwcfg user file isn't read completely.
Fri, May 12, 6:10 AM
rew added inline comments to D40076: bhyve: error out if fwcfg user file isn't read completely.
Fri, May 12, 5:55 AM

May 5 2023

rew added a comment to D39979: RFC: bhyve: add config option to load ACPI tables into memory.

Do you think we should add this option now or should we wait until someone has some issues or a feature requires it?

It might be helpful for debugging purposes too.

May 5 2023, 5:28 PM

Apr 25 2023

rew accepted D39805: vmm: Dynamically allocate a couple of per-CPU state save areas.

Would anyone be willing to test this on an AMD system?

Apr 25 2023, 7:24 PM

Mar 31 2023

rew added a comment to D39319: bhyve: introduce acpi_device_emul struct.

so I assume some ACPI devices will have different emulation than others?

Mar 31 2023, 6:14 AM

Mar 30 2023

rew accepted D39338: bhyve: return EEXIST when adding a fwcfg item twice.
Mar 30 2023, 2:36 PM

Mar 29 2023

rew added a comment to D39316: bhyve/fwcfg: add QEMU_FWCFG_INDEX_NB_CPUS item.

the comments from D39315 apply to this review as well

Mar 29 2023, 5:20 PM
rew added inline comments to D39315: bhyve/fwcfg: add QEMU_FWCFG_INDEX_MAX_CPUS item.
Mar 29 2023, 3:17 PM

Mar 28 2023

rew added a comment to D39290: bhyve: export passthru softc.

Seems a bit odd making this public? as opposed to keeping a reference of the softc in gpu_passthru and passing that off to pci?

Mar 28 2023, 4:17 PM

Mar 27 2023

rew added inline comments to D39284: bhyve: add helper to create a bootorder.
Mar 27 2023, 5:40 PM

Mar 15 2023

rew closed D1888: Extend the unit test to fix the bug caught in r277925.

committed in rG2dfd9979d827b36a9ed6ef2d5911a755091216c7 or SVN r279342

Mar 15 2023, 5:28 PM

Mar 14 2023

rew accepted D38441: ssh: fix leak and apply style(9) to hostname canonicalization.
Mar 14 2023, 4:56 PM

Mar 10 2023

rew resigned from D38858: bhyve: Use directory `fd` with checkpoint.
Mar 10 2023, 11:28 PM · bhyve
rew added a comment to D35590: bhyve: multiple devices support for suspend/checkpoint/resume.

Let's move forward and all useful things will be added :)

Mar 10 2023, 9:39 PM · bhyve

Mar 9 2023

rew added a comment to D35590: bhyve: multiple devices support for suspend/checkpoint/resume.

That will work, but this is not easier than current review's approach.

Mar 9 2023, 9:13 PM · bhyve

Mar 8 2023

rew added a comment to D38860: bhyve: Enable Capsicum for snapshots.
In D38860#887381, @rew wrote:

https://reviews.freebsd.org/D38858 also needs to be addressed before this patch is committed.

Mar 8 2023, 8:16 PM · capsicum, bhyve
rew accepted D38858: bhyve: Use directory `fd` with checkpoint.

I withdraw my request for changes.

Mar 8 2023, 8:11 PM · bhyve
rew added a comment to D38860: bhyve: Enable Capsicum for snapshots.

Looks like this patch needs to be rebased.

Mar 8 2023, 4:37 PM · capsicum, bhyve
rew added a comment to D35590: bhyve: multiple devices support for suspend/checkpoint/resume.
Mar 8 2023, 4:33 PM · bhyve

Mar 7 2023

rew added a comment to D35590: bhyve: multiple devices support for suspend/checkpoint/resume.

The main idea of this review - introduce struct snapshot_dev and it will be used by the following nvlist changes.

Mar 7 2023, 4:44 PM · bhyve

Mar 6 2023

rew added a comment to D35590: bhyve: multiple devices support for suspend/checkpoint/resume.
Mar 6 2023, 9:26 PM · bhyve

Mar 4 2023

rew added a comment to D35590: bhyve: multiple devices support for suspend/checkpoint/resume.

@rew This is stopper for following up nvlist changes. Please review and possibly approve. Thanks.

Mar 4 2023, 6:05 PM · bhyve

Mar 3 2023

rew requested changes to D38858: bhyve: Use directory `fd` with checkpoint.
In D38858#885635, @rew wrote:

nvlist implementation will use single file for all:

  • config
  • vram
  • kernel data
  • devices data

So you are right, using multiple files is not reasonable and is hard to operate.

In which case, the file descriptor change needs to happen after bhyve starts using a single file for snapshots.

I don't think so. It is not dependent thing. Current state of snapshot/resume is using multiple files. And this work is about adding support Capsicum. It is not related to the single file format at all.

So this review and https://reviews.freebsd.org/D38860 will have to wait till the above work is merged.

Why do you think single format work (nvlist) should be *before* enabling Capsicum? nvlist work will have a lot of changes and we can stuck if discussing any useful thing eats a lot of time and risk is not to move forward for the months. Just note, multiple device review is 7-8 months old. It is crazy for the code that is under #ifdef BHYVE_SNAPSHOT. We need speedup this work. For now it is about 2 years when Snapshot/Resume was added and there no significant progress.

If you have questions or notes about the code, please write here. Otherwise please accept the reviews.

Mar 3 2023, 11:15 PM · bhyve
rew added a comment to D38858: bhyve: Use directory `fd` with checkpoint.
In D38858#885604, @rew wrote:

..

see the original commit message that brought the snapshot code in

The file format also does not currently support
    versioning of individual chunks of state.  As a result, the current
    file format is not a fixed binary format and future revisions to save
    and restore will break binary compatiblity of snapshot files.  The
    goal is to move to a more flexible format that adds versioning,
    etc. and at that point to commit to providing a reasonable level of
    compatibility.

If I understand right, all things could be achieved with nvlist implementation. Thanks.

Why does there need to be multiple files for a single snapshot?

nvlist implementation will use single file for all:

  • config
  • vram
  • kernel data
  • devices data

So you are right, using multiple files is not reasonable and is hard to operate.

Mar 3 2023, 10:22 PM · bhyve
rew added a comment to D38858: bhyve: Use directory `fd` with checkpoint.
In D38858#885508, @rew wrote:

! In D38858#884878, @gusev.vitaliy_gmail.com wrote:
Did you mean about format of the image files or something another? If format, I am going to resolve it with coming nvlist changes.

you plan on converting the snapshot file format to an nvlist?

Plain is:

  1. Capsicum (has review)
  2. Multiple devices support (has review)
  3. nvlist review. Not yet created and waits for 1-2 finish. I will describe and propose file format with nvlist implementation.

Sorry, which advice did you mean? I don't see any comment from jhb in https://reviews.freebsd.org/D35454

see the original commit message that brought the snapshot code in

The file format also does not currently support
    versioning of individual chunks of state.  As a result, the current
    file format is not a fixed binary format and future revisions to save
    and restore will break binary compatiblity of snapshot files.  The
    goal is to move to a more flexible format that adds versioning,
    etc. and at that point to commit to providing a reasonable level of
    compatibility.

If I understand right, all things could be achieved with nvlist implementation. Thanks.

Mar 3 2023, 9:38 PM · bhyve
rew added a comment to D38858: bhyve: Use directory `fd` with checkpoint.

! In D38858#884878, @gusev.vitaliy_gmail.com wrote:
Did you mean about format of the image files or something another? If format, I am going to resolve it with coming nvlist changes.

Mar 3 2023, 7:16 PM · bhyve
rew added inline comments to D38858: bhyve: Use directory `fd` with checkpoint.
Mar 3 2023, 7:10 PM · bhyve

Mar 2 2023

rew added a comment to D38858: bhyve: Use directory `fd` with checkpoint.

Did you mean about format of the image files or something another? If format, I am going to resolve it with coming nvlist changes.

Mar 2 2023, 8:15 PM · bhyve
rew added a comment to D38858: bhyve: Use directory `fd` with checkpoint.

I still don't understand why you haven't addressed my comments that I raised in the original review. Do you feel it's not worth addressing or is that I'm out of line?

Mar 2 2023, 6:58 PM · bhyve

Mar 1 2023

rew closed D38836: bhyve: Remove excessive 'err' variable.
Mar 1 2023, 4:53 PM · bhyve
rew committed rG956171d5bc29: bhyve: remove redundant variable (authored by gusev.vitaliy_gmail.com).
bhyve: remove redundant variable
Mar 1 2023, 4:53 PM
rew accepted D38840: bhyveload: Address compiler warnings and bump WARNS.
Mar 1 2023, 4:37 PM
rew accepted D38839: bhyvectl: Address compiler warnings and bump WARNS.
Mar 1 2023, 4:35 PM

Feb 28 2023

rew added a comment to D35454: bhyve: snapshot capsicum support.

I still hold the opinion that the snapshot data should be stored in a single file and the file descriptor for that file should be passed to bhyve.

Feb 28 2023, 10:42 PM · bhyve

Feb 25 2023

rew committed rGc5f019807059: stand: fix buffer overflow in getrootmount() (authored by rew).
stand: fix buffer overflow in getrootmount()
Feb 25 2023, 6:43 PM
rew closed D38734: stand: fix buffer overflow in getrootmount().
Feb 25 2023, 6:43 PM

Feb 24 2023

rew accepted D38746: time: s/ppsratecheck/eventratecheck.
Feb 24 2023, 5:59 PM

Feb 23 2023

rew added a comment to D38746: time: s/ppsratecheck/eventratecheck.

spelling nit

Feb 23 2023, 6:34 PM

Feb 22 2023

rew added reviewers for D38734: stand: fix buffer overflow in getrootmount(): imp, kevans.
Feb 22 2023, 6:56 PM
rew requested review of D38734: stand: fix buffer overflow in getrootmount().
Feb 22 2023, 6:55 PM
rew accepted D38600: loader: Add support for booting from a ZFS snapshot.

Tested - works as expected for me,

Feb 22 2023, 4:30 PM

Feb 17 2023

rew abandoned D38647: zfs: don't verify recycled vnode for zfs control directory.

https://github.com/openzfs/zfs/pull/14501

Feb 17 2023, 12:16 AM

Feb 16 2023

rew requested review of D38647: zfs: don't verify recycled vnode for zfs control directory.
Feb 16 2023, 9:42 PM

Feb 7 2023

rew added inline comments to D38332: bhyve: add emulation for the qemu fwcfg selector port.
Feb 7 2023, 6:42 PM

Feb 6 2023

rew added inline comments to D38332: bhyve: add emulation for the qemu fwcfg selector port.
Feb 6 2023, 10:20 PM
rew added inline comments to D38327: bhyve: add helper struct for acpi device handling.
Feb 6 2023, 9:41 PM

Jan 30 2023

rew accepted D38285: vtd: Increase DRHD_MAX_UNITS.

I don't see any issues with this offhand - be nice to have this fixed for 13.2

Jan 30 2023, 10:23 PM

Jan 27 2023

rew added a comment to D38231: zfs: Disable -Wunused-variable for environ in main in zfs_main.c..

would it be worth the effort to fix this upstream?

I prefer that if someone's willing to shepherd it in, but in the meantime adding the warning flag is sensible.

Jan 27 2023, 10:37 PM
rew added a comment to D38231: zfs: Disable -Wunused-variable for environ in main in zfs_main.c..

would it be worth the effort to fix this upstream? something like:

Jan 27 2023, 9:23 PM

Jan 26 2023

rew added a comment to D38211: loader: Fix md device name parsing.

After a second look, perhaps md's devsw shouldn't even define dv_fmtdev or dv_parsedev and use the defaults instead.

Jan 26 2023, 9:31 PM
rew added a comment to D38211: loader: Fix md device name parsing.

it looked something like: https://reviews.freebsd.org/differential/diff/115789/

Jan 26 2023, 5:25 PM
rew added a comment to D38211: loader: Fix md device name parsing.

My local workaround to fix this was to add md_parsedev() and md_formatdev() and have it only understand md0.

Jan 26 2023, 5:09 PM

Jan 20 2023

rew committed rG27029bc08f1d: vmm: fix use after free in ppt_detach() (authored by rew).
vmm: fix use after free in ppt_detach()
Jan 20 2023, 8:26 PM
rew closed D38072: vmm: fix use after free in ppt_detach().
Jan 20 2023, 8:25 PM
rew committed rGc668e8173a8f: vmm: take exclusive mem_segs_lock in vm_cleanup() (authored by rew).
vmm: take exclusive mem_segs_lock in vm_cleanup()
Jan 20 2023, 8:11 PM
rew closed D38071: vmm: take exclusive mem_segs_lock in vm_cleanup().
Jan 20 2023, 8:11 PM
rew committed rGccf32a68f821: vmm: take exclusive mem_segs_lock when (un)assigning ppt dev (authored by rew).
vmm: take exclusive mem_segs_lock when (un)assigning ppt dev
Jan 20 2023, 7:05 PM
rew closed D37962: vmm: take exclusive mem_segs_lock when (un)assigning ppt dev.
Jan 20 2023, 7:05 PM

Jan 16 2023

rew added inline comments to D38071: vmm: take exclusive mem_segs_lock in vm_cleanup().
Jan 16 2023, 7:00 PM
rew added a comment to D38072: vmm: fix use after free in ppt_detach().

Does the bug occur because the ppt module is unloaded after the vmm module when vmm.ko is unloaded?

Jan 16 2023, 5:47 PM
rew updated the diff for D38072: vmm: fix use after free in ppt_detach().

previous diff was incorrect

Jan 16 2023, 12:02 AM

Jan 15 2023

rew updated the summary of D38072: vmm: fix use after free in ppt_detach().
Jan 15 2023, 10:42 PM
rew requested review of D38072: vmm: fix use after free in ppt_detach().
Jan 15 2023, 10:40 PM
rew added a comment to D37972: bhyve: help rendezvous request when waiting for IDLE vcpu state.

I applied your patch to -CURRENT, but unfortunately I get a panic in vm_iommu_modify(): "vm mem_segs not locked "

That doesn't seem related to this change.

Jan 15 2023, 10:25 PM
rew requested review of D38071: vmm: take exclusive mem_segs_lock in vm_cleanup().
Jan 15 2023, 10:22 PM
rew added a comment to D37972: bhyve: help rendezvous request when waiting for IDLE vcpu state.

I applied your patch to -CURRENT, but unfortunately I get a panic in vm_iommu_modify(): "vm mem_segs not locked @ /usr/src/sys/amd64/vmm/vmm.c:1189"

Jan 15 2023, 9:48 PM

Jan 6 2023

rew requested review of D37962: vmm: take exclusive mem_segs_lock when (un)assigning ppt dev.
Jan 6 2023, 10:14 AM

Dec 5 2022

rew closed D37274: bhyveload: open guest boot disk image O_RDWR.
Dec 5 2022, 5:24 PM
rew committed rG5a023bd2a53a: bhyveload: open guest boot disk image O_RDWR (authored by rew).
bhyveload: open guest boot disk image O_RDWR
Dec 5 2022, 5:24 PM
rew closed D37600: config: remove LOCK_PROFILING_FAST.
Dec 5 2022, 5:05 PM
rew committed rGdacbe4d533b7: config: remove LOCK_PROFILING_FAST (authored by rew).
config: remove LOCK_PROFILING_FAST
Dec 5 2022, 5:05 PM

Dec 4 2022

rew requested review of D37600: config: remove LOCK_PROFILING_FAST.
Dec 4 2022, 8:25 PM

Nov 7 2022

rew added a comment to D37296: bhyve: Use the default compiler warnings.

Disabling this warning will lead to even more unaligned accesses. If you're working on removing all unaligned accesses and this commit is inteded to increase the warn level early, it's fine but please mention it in the commit message.

Nov 7 2022, 5:24 PM
rew added a comment to D37295: bhyve: Disable thread safety analysis by the compiler.

Even if it only finds very simple bugs, it's a good option. Simple bugs happen. Setting up a VM to find a bug requires some work and is error prone. So, it's much better if the compiler complains about your bug.
Note that if we disable the option, it'll get even worse.

Nov 7 2022, 4:37 PM

Nov 6 2022

rew added a comment to D37296: bhyve: Use the default compiler warnings.

Not sure if I missed a step while applying this stack of patches but, I got the following error while build testing. I've provided a diff for context that allowed the build to complete, https://reviews.freebsd.org/differential/diff/112707/

Nov 6 2022, 10:23 PM

Nov 4 2022

rew requested review of D37274: bhyveload: open guest boot disk image O_RDWR.
Nov 4 2022, 10:17 PM

Oct 27 2022

rew added inline comments to D37146: vmm: Simplify saving of absolute TSC values in snapshots..
Oct 27 2022, 7:09 PM

Oct 26 2022

rew added inline comments to D37117: bhyve: Make sure that the VNC version is initialized.
Oct 26 2022, 8:49 PM

Sep 13 2022

rew added a comment to D35454: bhyve: snapshot capsicum support.

I haven't fully reviewed in detail, but it seems that this is an incremental step forward, and it makes sense to go ahead with this change [

Sep 13 2022, 7:46 PM · bhyve

Aug 23 2022

rew added a comment to D36279: vlan_output(): use parents context when calling if_output().
In D36279#824118, @kp wrote:
In D36279#824072, @kp wrote:

Hmm. This *looks* sane to me, but this patch breaks the if_vlan tests, so clearly I'm missing something. A ping over a vlan interface (on top of epair, in a vnet jail) results in ping: sendto: Invalid argument, and I'm not seeing why at the moment.

It looks like what's happening here is that we fail the arp lookup because we're trying to do a lookup on the underlying interface, not the vlan interface.

Aug 23 2022, 6:58 AM

Aug 21 2022

rew updated the summary of D36279: vlan_output(): use parents context when calling if_output().
Aug 21 2022, 7:22 PM
rew requested review of D36279: vlan_output(): use parents context when calling if_output().
Aug 21 2022, 7:19 PM
rew closed D36205: kqueue: retire knlist_init_rw_reader().
Aug 21 2022, 5:24 AM
rew committed rG3454a7caa053: kqueue: retire knlist_init_rw_reader() (authored by rew).
kqueue: retire knlist_init_rw_reader()
Aug 21 2022, 5:24 AM

Aug 15 2022

rew added reviewers for D36205: kqueue: retire knlist_init_rw_reader(): melifaro, kib.
Aug 15 2022, 7:46 AM
rew requested review of D36205: kqueue: retire knlist_init_rw_reader().
Aug 15 2022, 7:41 AM

Aug 14 2022

rew abandoned D33401: zfs: add kqfilter support for zvol's with volmode=dev.

I've submitted a pull request upstream:

Aug 14 2022, 6:46 AM
rew accepted D36188: libbe: bail out early if the zfs kmod isn't loaded.
Aug 14 2022, 3:38 AM

Aug 8 2022

rew abandoned D33400: kqueue: add knlist_init_sx() for shared/exclusive locks.
In D33400#818953, @kib wrote:

I meant something different. There is a very little need to add knlist_init_sx(9), when we only have single consumer. Put the code into ZFS OS-specific file.

Aug 8 2022, 4:18 PM

Aug 7 2022

rew added a comment to D33400: kqueue: add knlist_init_sx() for shared/exclusive locks.
In D33400#818876, @kib wrote:

Is it for single consumer?

Aug 7 2022, 8:47 PM