Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Handle VM with no network configuration and restrict ethernet channels on Linux #3

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

aegiap
Copy link
Contributor

@aegiap aegiap commented Apr 30, 2018

Restrict virtio channel configuration to Linux.

Handle VM configuration JSON with no network interface.

Signed-off-by: aegiap <aegiap@gandi.net>
Signed-off-by: aegiap <aegiap@gandi.net>
@grigouze
Copy link

+1

@kalou
Copy link
Contributor

kalou commented Apr 30, 2018

Would moving the "vif" key check to network_setup_check make sense ?

That would extend the function meaning to "we want it, and we can" -- but also remove a bit of complexity down in the main function if you want to make virtio config depend on that as well

@aegiap
Copy link
Contributor Author

aegiap commented May 4, 2018

@kalou somehing like:

diff --git a/swap_init.py b/swap_init.py
index dfcd01b..587515d 100755
--- a/swap_init.py
+++ b/swap_init.py
@@ -259,14 +259,17 @@ def network_disable_dhcp(vif_list, default_file):
     file(default_file, 'w').write(''.join(new_entries))
 
 
-def network_setup_check(default_file):
+def network_setup_check(default_file, vm_conf):
     """ Check if the system admin wants to have the network auto-configured.
     """
     for entry in file(default_file).readlines():
         if entry.startswith('CONFIG_NETWORK=0') or \
            entry.startswith('CONFIG_NETWORK = 0'):
             return False
-    return True
+    if vm_conf.get('vif', []):
+        return True
+    else:
+        return False
 
 
 def nameserver_setup_check(default_file):
@@ -616,12 +619,10 @@ if __name__ == '__main__':
         nameservers = conf.get('nameservers', [])
         resolver_setup(nameservers)
 
-    if network_setup_check(default_file):
+    if network_setup_check(default_file, conf):
         vm_hostname = conf.get('vm_hostname', '')
-        vif = conf.get('vif', [])
-        if vif:
-            network_setup(vm_hostname, vif)
-            network_enable(vif)
+        network_setup(vm_hostname, vif)
+        network_enable(vif)
     try:
         if os.path.exists('/sys/module/virtio_net'):
             vif = conf.get('vif', [])

@kalou
Copy link
Contributor

kalou commented May 4, 2018

Yeah, would the virtio config also be a good thing to put under the network config check ?
So you can also get rid of the vif presence check there

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants