- User Since
- Dec 19 2016, 4:11 AM (127 w, 3 h)
Sat, May 25
Fri, May 24
Getting way above my pay grade
@allanjude I know your work load is huge as well, if you need me to push a new diff I can do that, but then you have to agree to commit it or I have to pester bde/phk for an approval.
Concur, but not sure how to force it closed.
@jhb, reopening a commited phab review messes with things, as now when you make a second commit the default top patch in this review well be the one you commit, and one has to go digging in the review history to find the prior patch that was in the first commit. It is fine to reopen a review for a revert, but I think one should start a new review for a new change, which is what this is.
Thu, May 23
Wed, May 22
Minor nits, and a major win, good work!
Tue, May 21
Any more little gold mines like this sitting around?
Sat, May 18
Ok to commit once blank lines are added per jhb in person
Wed, May 15
I thought this had already been committed?
I would like to see this in next weeks 11.3-PRERELEASE build, this looks to be a low risk barrier addition, @jhb does that seem to be a reasonable target, or would you like to see a longer ^head test cycle? Does anyone see any risk in this change?
Fri, May 10
Thu, May 9
This may be a fix for https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=231117, the person is testing.
Tue, May 7
Thanks for the quick spin on this Patrick
Sat, May 4
Fri, May 3
@jhb it would be good to not have this issue in 11.3
Thu, May 2
Wed, May 1
You can mark all my comments as done, I am fine with your answers, I added bz and requested his review on the one change.
Do we have to add _user_ to the variables, I only see a rename here, and not a second value used in place of where _users_ went in, or am I totally missing something?
I have made one quick pass over this, now that I have read all the bits I would like to make a second pass, my comments now are nits mostly, and can be safely ignored. However I do, as others, have some pretty big concerns about sharing the rcvif with tags, is there some great cost in not doing that?
Well this in any way help with the fact we do not apply any pressure to the zfs arc_max and end up in a memory shortfall that causes high paging? I suspect not as most people do not wire there VM's in memory unless they know that is what they need to do to keep zfs from killing your box with OOM
Apr 27 2019
Some of it is, but we meet on a cyclic basis to have high bandwidth low delay discussions and I was just asking for the opportunity to allow that discussion to occur. Or if you referring to the in person option that was offered up "if you wish".
Apr 26 2019
I can see both sides of this, can I ask that the bhyve developers group which meets every 2 weeks be given a chance to discuss this and provide feedback on it? Thanks. Many of us well be at BSDCan a day early for bhyvecon if you wish to discuss it in person.
Apr 25 2019
This makes it much clearer to me, thanks jhb
Apr 20 2019
We only need to maintain "backwards" compatibility in the stable/ branches (12 and 11) as far as I am concerned head can just nuke the compatibilty link, and have a 13.0 release notes item that is "fuse has been renamed to fusefs".
A question to @jhb, should the order of the enum and the code updated to be in alphabetic order, in a seperate review, it would only obfuscate the change to do it here.
Apr 15 2019
Apr 14 2019
Looks good, other than a lot of extra blank lines.
Apr 12 2019
Apr 7 2019
You can mark all my comments as done, you have addressed them to my satisfaction.
Apr 6 2019
Apr 5 2019
I have performed a visual only review.
Thank you, that is 100x clearer as to what is in the constant
Apr 3 2019
Confirmation of a working Windows 2019 boot after this patch: https://lists.freebsd.org/pipermail/freebsd-virtualization/2019-April/007333.html
Apr 2 2019
Can we get a review to cause bhyve -b to emit a deprecation warning notice (gone_in(13)) if someone tries to use it? That could be merged to stable/12, then the code in head killed.
This would also apply to the bvmconsole driver, is there someway to emit a warning during config(8) when someone uses a feature being deprecated? Doing this provides us feedback
sooner rather than later that some feature was in use some place.
Looks like someone may of removed a condition at some time in the past and not re-dented the code to produce a minimal diff.
Does this variable get used by default? I saw this here and just ignored it as I thought it would only be invoked if I asked for a DEBUG build from a level above. If we are always adding DEBUG_FLAGS to our compiles perhaps it should be called something else?
Mar 25 2019
Mar 19 2019
Please have a discussion on arch@ or filesystems@, this review needs more discussion
Can I get some final accept on this, I updated the open issues
Mar 15 2019
Mar 14 2019
Mar 11 2019
I like it, just nits, that for the most part you can ignore, other than $FreeBSD: vs $FreeBSD$
Mar 9 2019
This appears to be used to deal with a core hang on specific CPU's, we need to see that we have mitigated that in the host, and the decide what the appropriate action here is, I am uncomfortable with just making this silently disappear. Deferning to jhb.
Though this does fix your issue by silencing the log, it does little to improve the situation. I think what we may want to do is to add yet another option to the bhyve command that along with -w to ignore invalid MSR access to do so silently so that we do not have to add code like this everything a new msr comes into play. John, Patrick your thoughts on this approach?
Mar 8 2019
I believe what your wanting is web server re-write rules, that would rewrite the URL in and out of the web pages to have the preffered form. Beyond my area of expertise to implement, but I know it can be done and that is probably the easiest place to do it (ie, no modifying bugzilla should be required.)
Mar 7 2019
I removed a redundant "to" in the description. Patch looks good to me. Copyright over to jhb@
Mar 6 2019
I'll review if you ack your are done updating John