On 11/23/2016 02:32 PM, Nadav Har'El wrote:
On Wed, Nov 23, 2016 at 3:27 PM, Justin Cinkelj
<justin.cink...@xlab.si <mailto:justin.cink...@xlab.si>> wrote:
When cloud-init changes VM hostname, VM DNS name should be updated
too.
We try to do this by releasing DHCP lease and requesting new lease
with new name. This instructs DHCP server to perform dynamic DNS
update.
DNS update will/should succeed if
1) new name is not already claimed by some other VM/IP and
2) obviously, DHCP and DNS server need to support dynamic DNS
updates.
The dhcp_worker::init was split into two parts (::init and
::start), so
that can be discovery-resend logic can be reused.
Signed-off-by: Justin Cinkelj <justin.cink...@xlab.si
<mailto:justin.cink...@xlab.si>>
---
core/dhcp.cc | 23 ++++++++++++++++++-----
include/osv/dhcp.hh | 8 ++++++--
modules/cloud-init/cloud-init.cc | 27 +++++++++++++++++++--------
3 files changed, 43 insertions(+), 15 deletions(-)
diff --git a/core/dhcp.cc b/core/dhcp.cc
index 27d7aef..d89eb46 100644
--- a/core/dhcp.cc
+++ b/core/dhcp.cc
@@ -66,7 +66,8 @@ int dhcp_hook_rx(struct mbuf* m)
void dhcp_start(bool wait)
{
// Initialize the global DHCP worker
- net_dhcp_worker.init(wait);
+ net_dhcp_worker.init();
+ net_dhcp_worker.start(wait);
}
// Send DHCP release, for example at shutdown.
@@ -75,6 +76,12 @@ void dhcp_release()
net_dhcp_worker.release();
}
+void dhcp_restart(bool wait)
+{
+ net_dhcp_worker.release();
+ net_dhcp_worker.start(wait);
+}
+
namespace dhcp {
const char * dhcp_options_magic = "\x63\x82\x53\x63";
@@ -504,7 +511,10 @@ namespace dhcp {
void dhcp_interface_state::discover()
{
- // FIXME: send release packet in case the interface has
an address
+ // send release packet in case the interface has an address
+ if (_client_addr != ipv4_zero) {
+ release();
+ }
I don't know why this FIXME was correct, or why this extra code was
needed: In both the dhcp_start() and dhcp_restart() cases, we start
with zero address (in dhcp_restart() you call release() already). So
why do we need to check it again here?
I just noted the fixme line, and then added the if sentence for
just-in-case.
I don't see how current code could send release via this 3 lines; maybe
if at some later time
we/I forget to call release before discover;
Maybe I should just remove the fixme comment?
Justin
// Update state
_state = DHCP_DISCOVER;
@@ -665,12 +675,10 @@ namespace dhcp {
// FIXME: free packets and states
}
- void dhcp_worker::init(bool wait)
+ void dhcp_worker::init()
{
struct ifnet *ifp = nullptr;
- // FIXME: clear routing table (use case run dhclient 2nd
time)
-
// Allocate a state for each interface
IFNET_RLOCK();
TAILQ_FOREACH(ifp, &V_ifnet, if_link) {
@@ -686,7 +694,11 @@ namespace dhcp {
_dhcp_thread = sched::thread::make([&] {
dhcp_worker_fn(); });
_dhcp_thread->set_name("dhcp");
_dhcp_thread->start();
+ }
+ void dhcp_worker::start(bool wait)
+ {
+ // FIXME: clear routing table (use case run dhclient 2nd
time)
do {
// Send discover packets!
for (auto &it: _universe) {
@@ -711,6 +723,7 @@ namespace dhcp {
for (auto &it: _universe) {
it.second->release();
}
+ _have_ip = false;
// Wait a bit, so hopefully UDP release packets will be
actually put on wire.
usleep(1000);
}
diff --git a/include/osv/dhcp.hh b/include/osv/dhcp.hh
index 30ca86f..b286727 100644
--- a/include/osv/dhcp.hh
+++ b/include/osv/dhcp.hh
@@ -25,6 +25,7 @@
extern "C" {
void dhcp_start(bool wait);
void dhcp_release();
+void dhcp_restart(bool wait);
}
namespace dhcp {
@@ -249,8 +250,11 @@ namespace dhcp {
dhcp_worker();
~dhcp_worker();
- // Initializing a state per interface, sends discover packets
- void init(bool wait);
+ // Initializing a state per interface
+ void init();
+ // Send discover packets
+ void start(bool wait);
+ // Send release packet for all DHCP IPs.
void release();
void dhcp_worker_fn();
diff --git a/modules/cloud-init/cloud-init.cc
b/modules/cloud-init/cloud-init.cc
index 67f477e..e3ec8f8 100644
--- a/modules/cloud-init/cloud-init.cc
+++ b/modules/cloud-init/cloud-init.cc
@@ -16,6 +16,22 @@
#include <osv/firmware.hh>
#include <osv/hypervisor.hh>
+// we cannot include osv/dhcp.hh, hence direct declaration.
+extern "C" void dhcp_restart(bool wait);
+
+// Set the hostname to given string.
+// If hostname changes, try to propagate the change to DHCP
server too.
+void set_hostname_restart_dhcp(std::string hostname) {
+ if (hostname.length() > 0) {
+ char old_hostname[256] = "";
+ gethostname(old_hostname, sizeof(old_hostname));
+ sethostname(hostname.c_str(), hostname.length());
+ if (hostname != old_hostname) {
+ dhcp_restart(true);
+ }
+ }
+}
+
namespace init {
using namespace std;
@@ -229,9 +245,7 @@ void hostname_module::handle(const YAML::Node&
doc)
{
auto hostname = doc.as <http://doc.as><string>();
debug("cloudinit hostname: %s\n", hostname.c_str());
- if (hostname.size()) {
- sethostname(hostname.c_str(), hostname.size());
- }
+ set_hostname_restart_dhcp(hostname);
}
void osvinit::add_module(std::shared_ptr<config_module> module)
@@ -265,11 +279,8 @@ void osvinit::load_from_cloud(bool
ignore_missing_source)
try {
auto& ds = get_data_source();
- std::string hostname = ds.external_hostname();
- if (hostname.length() > 0) {
- // Set the hostname from given data source, if it exists.
- sethostname(hostname.c_str(), hostname.length());
- }
+ // Set the hostname from given data source, if it exists.
+ set_hostname_restart_dhcp(ds.external_hostname());
// Load user data.
user_data = ds.get_user_data();
--
2.9.3
--
You received this message because you are subscribed to the Google
Groups "OSv Development" group.
To unsubscribe from this group and stop receiving emails from it,
send an email to osv-dev+unsubscr...@googlegroups.com
<mailto:osv-dev%2bunsubscr...@googlegroups.com>.
For more options, visit https://groups.google.com/d/optout
<https://groups.google.com/d/optout>.
--
You received this message because you are subscribed to the Google Groups "OSv
Development" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to osv-dev+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
