Test case for this and the other cursor bug fix: https://reviews.freebsd.org/D40469
- Queries
- All Stories
- Search
- Advanced Search
- Transactions
- Transaction Logs
All Stories
Today
Revert back to function with no arguments. Fix typo and formatting.
Thank you for suggestion.
I've re-worked patch with MASTERDIR approach and shortened the patch to only angie and angie-module-geoip2 ports for ease of review process. If we can agree on this method, I'm ready to introduce all the others.
Thank you in advance!
In D40105#921197, @jhb wrote:Can you provide more description of what this change is doing? From what I can tell it uses err directly to abort quicker if it fails to restore an in-kernel structure, it inlines vm_restore_kern_struct into the loop in vm_restore_kern_structs. It renames vm_snapshot_kern_structs which is somewhat gratuitous IMO as it now no longer matches the other function names like vm_snapshot_user_devs. But I think the big change is not using lookup_struct and instead using meta->dev_name instead of meta->dev_req as the main key for the JSON for a given kernel struct? I think this means you can avoid lookup_struct because now instead of a flat array of all kernel structures they are separate objects with unique names? Can you expand on that more perhaps maybe with some examples of before/after JSON snippets? I would also suggest perhaps only doing the JSON change in this commit and not mixing in the other changes that I think obscure the real change you are making (e.g. the function rename, or inlining the function).
Nit: s/npd/ndp/ in commit log
Humm, I had originally considered this a break of the API contract FWIW that callers shouldn't try to use a cursor after advancing past the end (though perhaps I broke that contract myself). Hmm, reading swcr_encdec, it might indeed be a bit annoying to avoid calling crypto_cursor_segment for this case.
Can you provide more description of what this change is doing? From what I can tell it uses err directly to abort quicker if it fails to restore an in-kernel structure, it inlines vm_restore_kern_struct into the loop in vm_restore_kern_structs. It renames vm_snapshot_kern_structs which is somewhat gratuitous IMO as it now no longer matches the other function names like vm_snapshot_user_devs. But I think the big change is not using lookup_struct and instead using meta->dev_name instead of meta->dev_req as the main key for the JSON for a given kernel struct? I think this means you can avoid lookup_struct because now instead of a flat array of all kernel structures they are separate objects with unique names? Can you expand on that more perhaps maybe with some examples of before/after JSON snippets? I would also suggest perhaps only doing the JSON change in this commit and not mixing in the other changes that I think obscure the real change you are making (e.g. the function rename, or inlining the function).
It might be nicer to have the logic of which devices pci-gvt_d.c supports be a bit more self-contained, e.g.:
Respond to comments from @rfyu28uyeg_snkmail.com:
- fix licensing (lib/libc/db is BSD 3-clause, not 4-clause).
- remove unnecessary "dual licensing" logic for the BDB1 option.
@rfyu28uyeg_snkmail.com: thank you for the input about the licensing. You're right -- I'm updating the patch now.
Assert that the value stored in tm->tm_gmtoff will fit in a int32_t.
Looks like these are in amd64 via rG:8ba66bb849129
This seems OK as a minimal change.
This revision narrows the scope of change to only focus on FreeBSD related L2 packet handling in wpa_supplican. The previous revision updated all occurrences of the filter program, however failed to account for packet and length offsets when handling encapsulated frames. Someone else can pick up the torch and provide patches for other platforms upstream as-needed.
Make utc_offset() argument match gmt2local() and make it a static function.
This looks like a kernel with HWT_HOOKS but not HWT wouldn't link as, for example, hwt_switch_in will be missing. HWPMC handles this by adding a function pointer and protects it with an epoch.
This will sort by name not priority. Ld.lld will sort by priority automatically if you omit init_array from the linker script but for bfd you need SORT_BY_INIT_PRIORITY
@emaste : You added something similar a while back for amd64. Adding you for review.
In D40463#921097, @jrtc27 wrote:Oh also linker_file_lookup_set with a section name like that won't work on anything other than amd64, since link_elf relies on the start/stop symbols, not the section name.
a small update
Oh also linker_file_lookup_set with a section name like that won't work on anything other than amd64, since link_elf relies on the start/stop symbols, not the section name.
Is this patch targeted at FreeBSD15 head, after FreeBSD 14 is branched? Is this patch just for early review and is not planned to commit until FreeBSD 15 becomes the head?
Change from .ctors to .init_array and .dtors to .fini_array . This is what the default FreeBSD kernel toolchain currently outputs. Also noted by @jrtc27 .
Yes, I read the code, thank you for asking
Finally, I'm highly unconvinced that it makes sense to dynamically register each sysinit individually.
In D40463#921068, @jrtc27 wrote:MFC after: 1 week
There is no way this can be MFC'ed, it is a big pile of KBI break.
What do you think about this change?
MFC after: 1 week
Keep "struct __hack" to force semicolon after SYSINIT() and SYSUNINIT().
This revision is abandoned in favor of D40463 .
FYI: Here is my second take on the issue D40463 . Please have a look if you are interested.
Implemented as https://reviews.freebsd.org/D38933.
Removed some VNET related changes (not related to this change).