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

Qemu driver: Task customization using config arguments #1588

Closed
ramukima opened this issue Aug 15, 2016 · 9 comments
Closed

Qemu driver: Task customization using config arguments #1588

ramukima opened this issue Aug 15, 2016 · 9 comments

Comments

@ramukima
Copy link
Contributor

ramukima commented Aug 15, 2016

Nomad version

Output from nomad version
Nomad v0.4.1-dev ('08628fea9058aeff80f9320cd350453a048f6153+CHANGES')

Operating system and Environment details

Linux 4.4.0-34-generic #53-Ubuntu x86_64

Issue

VM qcow2 image with embedded boot configuration is successfully deployed but does not boot with default configuration due to the options specified to the qemu driver by default.

Reference Discussion Thread : https://groups.google.com/forum/#!topic/nomad-tool/F1ZbRtrB9zc

The idea is to allow task configuration argument as a passthrough to qemu executable instead of hard-coding that in the driver.

@ramukima
Copy link
Contributor Author

I have patched qemu.go, qemu_test.go, and updated the documentation locally. However, I am unable to create a pull request due to permission issues. Here is a diff for your cursory review -

diff --git a/client/driver/qemu.go b/client/driver/qemu.go
index 7f577df..d109f39 100644
--- a/client/driver/qemu.go
+++ b/client/driver/qemu.go
@@ -46,6 +46,7 @@ type QemuDriverConfig struct {
        ImagePath   string           `mapstructure:"image_path"`
        Accelerator string           `mapstructure:"accelerator"`
        PortMap     []map[string]int `mapstructure:"port_map"` // A map of host port labels and to guest ports.
+       Args        []string         `mapstructure:"args"`     // extra arguments to qemu executable
 }

 // qemuHandle is returned from Start/Open as a handle to the PID
@@ -82,6 +83,9 @@ func (d *QemuDriver) Validate(config map[string]interface{}) error {
                        "port_map": &fields.FieldSchema{
                                Type: fields.TypeArray,
                        },
+                       "args": &fields.FieldSchema{
+                               Type: fields.TypeArray,
+                       },
                },
        }

@@ -169,11 +173,16 @@ func (d *QemuDriver) Start(ctx *ExecContext, task *structs.Task) (DriverHandle,
                "-name", vmID,
                "-m", mem,
                "-drive", "file=" + vmPath,
-               "-nodefconfig",
-               "-nodefaults",
                "-nographic",
        }

+       // Add pass through arguments to qemu executable. A user can specify
+       // these arguments in driver task configuration. These arguments are
+       // passed directly to the qemu driver as command line options.
+       // For example, args = [ "-nodefconfig", "-nodefaults" ]
+       // This will allow a VM with embedded configuration to boot successfully.
+       args = append(args, driverConfig.Args...)
+
        // Check the Resources required Networks to add port mappings. If no resources
        // are required, we assume the VM is a purely compute job and does not require
        // the outside world to be able to reach it. VMs ran without port mappings can
diff --git a/client/driver/qemu_test.go b/client/driver/qemu_test.go
index 23dfcff..32a2e30 100644
--- a/client/driver/qemu_test.go
+++ b/client/driver/qemu_test.go
@@ -14,7 +14,6 @@ import (

 // The fingerprinter test should always pass, even if QEMU is not installed.
 func TestQemuDriver_Fingerprint(t *testing.T) {
-       t.Parallel()
        ctestutils.QemuCompatible(t)
        driverCtx, _ := testDriverContexts(&structs.Task{Name: "foo"})
        d := NewQemuDriver(driverCtx)
@@ -37,7 +36,6 @@ func TestQemuDriver_Fingerprint(t *testing.T) {
 }

 func TestQemuDriver_StartOpen_Wait(t *testing.T) {
-       t.Parallel()
        ctestutils.QemuCompatible(t)
        task := &structs.Task{
                Name: "linux",
@@ -48,6 +46,7 @@ func TestQemuDriver_StartOpen_Wait(t *testing.T) {
                                "main": 22,
                                "web":  8080,
                        }},
+                       "args": []string{"-nodefconfig", "-nodefaults"},
                },
                LogConfig: &structs.LogConfig{
                        MaxFiles:      10,
@@ -96,7 +95,6 @@ func TestQemuDriver_StartOpen_Wait(t *testing.T) {
 }

 func TestQemuDriverUser(t *testing.T) {
-       t.Parallel()
        ctestutils.QemuCompatible(t)
        task := &structs.Task{
                Name: "linux",
@@ -108,6 +106,7 @@ func TestQemuDriverUser(t *testing.T) {
                                "main": 22,
                                "web":  8080,
                        }},
+                       "args": []string{"-nodefconfig", "-nodefaults"},
                },
                LogConfig: &structs.LogConfig{
                        MaxFiles:      10,
diff --git a/website/source/docs/drivers/qemu.html.md b/website/source/docs/drivers/qemu.html.md
index c905178..5a1f449 100644
--- a/website/source/docs/drivers/qemu.html.md
+++ b/website/source/docs/drivers/qemu.html.md
@@ -40,6 +40,9 @@ The `Qemu` driver supports the following configuration in the job spec:
   `port_map { db = 6539 }` would forward the host port with label `db` to the
   guest VM's port 6539.

+* `args` - (Optional) A `[]string` that is passed to qemu as command line options.
+  For example, `args = [ "-nodefconfig", "-nodefaults" ]
+
 ## Examples

 A simple config block to run a `Qemu` image:
@@ -51,6 +54,7 @@ task "virtual" {
   config {
     image_path = "local/linux.img"
     accelerator = "kvm"
+    args = [ "-nodefaults", "-nodefconfig" ]
   }

   # Specifying an artifact is required with the "qemu"

@ramukima
Copy link
Contributor Author

Do you think I should fork and do the update to raise a pull request ?

@diptanu
Copy link
Contributor

diptanu commented Aug 15, 2016

@ramukima Yes, please fork and open a PR.

@dadgar
Copy link
Contributor

dadgar commented Aug 15, 2016

I think we should keep the current args as is for backwards compatibility and if args are specified we override them

@ramukima
Copy link
Contributor Author

@dadgar Not sure. The issue I have is "My VM with embedded default image and self configurability does not boot with config properly if I use the current qemu driver". I needed to remove "-nodefaults" option in order to make it work for my VM.

The original issue I have will not be resolved if I keep the existing defaulted options. I may have to introduce a "compatibility" flag in that case to fallback. Do you agree ?

@dadgar
Copy link
Contributor

dadgar commented Aug 16, 2016

Hmm okay good point. I would forgo the compatibility flag and ignore my previous comment

@ramukima
Copy link
Contributor Author

Here is the pull request : #1596

@dadgar
Copy link
Contributor

dadgar commented Aug 16, 2016

Closing as #1596 was merged

@dadgar dadgar closed this as completed Aug 16, 2016
@github-actions
Copy link

I'm going to lock this issue because it has been closed for 120 days ⏳. This helps our maintainers find and focus on the active issues.
If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Dec 20, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants