Hi Andres, Blake and Jason,
reading this long context trying to help avaluating the change.

Summarizing (mostly for myself) c#34 and c#35 means that you think it is "ok" 
due to the out-of-band update to have time windows where primary and secondary 
will offer different IPs.
Instead of (trying to) fix this part of the dhcp setup to onyl ever offer one 
(or at least offer the same from primary and secondary) the suggestion is to 
make iPXE aware of how to handle the NACK.
The patch in c#41 and tests in c#40 suggest this works fine for your case now.
Overall that was already a lot of work to debug it down, kudos to all of you.

Taking a look on this from the qemu POV  - probably especially since the
last ipxe upload broke all migrations :-) (due to a size change).

One thing I wondered, but think we are good are state loops.
Since we add a state transition back (that didn't exist before at all) it might 
need some sort of infinite loop protection. I tried to read all that code but 
might be wrong.
D: dhcp_state_discover; R: dhcp_state_request
init: -> D -> R
         ^---/ (this is what we add)
As I read it, this will continue as before, neiher _expiry function had a 
responsibiity to abort he overall discovery. Each state change resets it's 
timer, but that should be ok. Also a scenario where a guest repeatetly gets an 
offer but then Nacked should be invalid in general. So it should (IMHO) be ok 
to add this transition.

In addition to the review I ran some tests (on Xenial as the ppa). On
Artful sizes would now be checked on build, but the ppa is only xenial -
so I checked manually and they are ok. Further some upgrade & migration
test I ran were all without issues - ok.

The open comments/requests to you that I'd have are:
1. Trivial: current patch has (only) whitespace damage in hunk #2
2. Upstreaming: While we can't wait (or could we) with the fix for an upstream 
accept it should be forwarded
3. Formalisms in the patch header:
  3.1 After being submitted please add as usual add to the dep3 header a 
Forwarded statement
      with a link to where you sent it.
  3.2 We all know how hard backtracing in a few years can be, please add like
      Bug-Ubuntu: https://bugs.launchpad.net/bugs/1707999

Other than that I like it and would consider it clearly good enough to
go from on-bug-RFC to ipxe-upstream-RFC with the patch.

-- 
You received this bug notification because you are a member of Ubuntu
Bugs, which is subscribed to Ubuntu.
https://bugs.launchpad.net/bugs/1707999

Title:
  pod VM fails to PXE boot after receiving multiple DHCP offers, for
  different IPs, from the dhcp server

To manage notifications about this bug go to:
https://bugs.launchpad.net/maas/+bug/1707999/+subscriptions

-- 
ubuntu-bugs mailing list
[email protected]
https://lists.ubuntu.com/mailman/listinfo/ubuntu-bugs

Reply via email to