- User Since
- Dec 19 2016, 4:11 AM (134 w, 3 d)
Thu, Jun 27
Wed, Jun 26
Finally review please
Fix 7F to F7 in 4 places
I was kinda waiting for jhb@ on this, as it is his patch, but given the urgency I am commandering this, and I'll push a new patch up for review. I would like to ask markj and pmooney to final review that and accept it before I commit to ^/head and push for an immediate MFC, and merge to releng. I'll also need to merge r347065 which is 6 weeks past MFC date.
Ping, since 11.3RC3 is back on the table I would like to see this pushed through along with r347065 which should of already been pushed through as it is 3 weeks past original MFC.
I helped Jason Tubnor test this patch on 11.3RC2, we had to first merge r347065 to get this patch to apply, and then had to make the 0x7F to 0xF7 Markj points out to get it to work. OpenBSD now runs as a guest in 11.3RC2
Tue, Jun 25
I am working with the original poster to get him a binary so he can test this fix.
Fri, Jun 21
Thu, Jun 20
This appears to be hung up on Capsicum additions
Wed, Jun 19
In agreement on it probably needs cleaned up, but first finding out why it was done this way as a safe position to take.
Jun 13 2019
One could theoretically take advantage of this to have a stable MAC across hosts for cases like, e.g., migration, no? Although it's not clear that bhyve folk would want to maintain this kind of promise, I could see it being somewhat handy.
For the purpose of migration you take the mac with you, that is part of the guest state.
I am pulling Kyle Evans in on this as he just did a bunch of cleanup on mac generation code and there may already be existing code to reused rather than have yet another mac generator.
Jun 10 2019
Jun 8 2019
Jun 6 2019
Please document why the -2 is needed per the earlier discussion. Does this need urgent attention to get in 11.3?
Looks like diff has gotten really confused by something making this very hard to see what it is that really changed since we are seeing function
compared to a new function
pci_emul_cmd_changed and are not actually seeing what changed in
Jun 5 2019
Just questions really, no changes requested.
I agree here, but did not have a better name so opted to say nothing.
I am not really keen on adding yet another top level .mk file that sets command names, much of this is caused by scatering etc/Makefile contents and rules about the tree when it was and should of been maintained as the one place for all of this and simply called as a submake. Perhaps it makes since anymore, perhaps not. But certainly having INSTALL and INSTALL_CMD default to and 99.999% of the time be "install" is not a good idea, same for MTREE and MTREE_CMD, they are being used in mixed manners, this needs to be sorted out asap and just use
MTREE_CMD and untwist some of this MTREE overload that should really overlaod MTREE_CMD.
Jun 4 2019
This is a ping. The pr https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=229852 has had another success report. This really should get in tree and in release, what is holding it up?
May 29 2019
Quote above As Ed beat me to the comment, I had be saying: Actually look closer at that 1.1 version, at line 119 it existed, and then in 1.3 was modified at line 159 became 194.
We can ask Kirk for clarity?
May 28 2019
I would even be fine with "If the function has no local variables", I do not think the length should come into play at all. Can someone explain why we have this special case at all? I know we want a blank line between variable declarations and the start of the function, is this just an attempt to preserve that empty line when there are no declarations?
May 26 2019
May 25 2019
May 24 2019
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.
May 23 2019
May 22 2019
Minor nits, and a major win, good work!
May 21 2019
Any more little gold mines like this sitting around?
May 18 2019
Ok to commit once blank lines are added per jhb in person
May 15 2019
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?
May 10 2019
May 9 2019
This may be a fix for https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=231117, the person is testing.
May 7 2019
Thanks for the quick spin on this Patrick
May 4 2019
May 3 2019
@jhb it would be good to not have this issue in 11.3
May 2 2019
May 1 2019
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.