Allow raw disks (or other GZ devices) to be used by guests #65

Open
trisk opened this Issue Feb 26, 2012 · 6 comments

Projects

None yet

5 participants

@trisk
trisk commented Feb 26, 2012

I have existing pools which I want to be managed by KVM guests running illumos and NexentaStor. On the older vmadmd in smartos-live 201109xx, I was able to add the physical disks by overloading the 'model' net-attr for devices.

For example, I have a

<device match="/dev/rdsk/c2t514d0">
    <net-attr name="model" value="none,id=sata-disk0"/>
</device>

Corresponding to 'qemu-extra-opts' contents "-device ahci,id=ahci0,bus=pci.0,multifunction=on -device ide-drive,drive=sata0-disk0,bus=ahci0.0".

This was converted into qemu arguments:
"-drive file=/dev/rdsk/c2t514d0,if=none,id=sata-disk0,index=1,media=disk"

The current vmadmd does this instead and causes qemu to choke:
"-drive file=/dev/rdsk/c2t514d0,if=undefined,index=1,media=disk"

startElement in VM.js assumes all device elements are disks but only uses the net-attr child attributes for devices with paths matching "/dev/._disk[0-9]+" when setting properties. It tries to use direct attributes for the device element as well but I don't think any are allowed by the zonecfg DTD. This results in "undefined" for the 'model' property for any devices not matching "/dev/._disk[0-9]+".

    } else if (where === 'zone.device') {
        // new disk device
        for (prop in attrs) {
            if (XML_PROPERTIES.disk.hasOwnProperty(prop)) {
                use = XML_PROPERTIES.disk[prop];
                disk[use] = attrs[prop];
            } else {
                VM.log('DEBUG', 'unknown disk prop: ' + prop);
            }
        }
        if (!obj.hasOwnProperty('devices')) {
            obj.devices = {};
        }
        obj.devices[disk.path] = disk;
        stack.push(disk.path);
    } else if (where.match(/zone\.device\.\/dev\/.*disk[0-9]+\.net-attr/)) {
        if (XML_PROPERTIES.disk.hasOwnProperty(attrs.name)) {
            use = XML_PROPERTIES.disk[attrs.name];
            obj.devices[stack[2]][use] = attrs.value;
        } else {
            VM.log('DEBUG', 'unknown disk prop: ' + attrs.name);
        }
    }

One alternative might be for vmadmd to ignore device elements without a 'model' net-attr, so invalid "-drive" arguments are not passed to qemu. This would also allow non-disk devices (audio, etc.) to be passed through.

This would leave "-drive" to be manually specified in the 'qemu-extra-opts' attr. A helpful addition might be to keep constructing "-drive" for devices with another net-attr, whose value is passed directly to qemu (as 'model' was previously).

This is obviously not a complete solution for external devices but would make my life a lot easier.

@trisk
trisk commented Feb 26, 2012

Maybe something like this as a workaround:

diff --git a/src/vm/node_modules/VM.js b/src/vm/node_modules/VM.js
--- a/src/vm/node_modules/VM.js
+++ b/src/vm/node_modules/VM.js
@@ -218,6 +218,7 @@
 var DISK_PROPS = [
     'model',
     'boot',
+    'qemu_params',
     'match',
     'zpool',
     'media',
@@ -229,7 +230,8 @@

 var UPDATABLE_DISK_PROPS = [
     'boot',
-    'model'
+    'model',
+    'qemu_params'
 ];

 // Note: this doesn't include 'state' because of 'stopping' which is a virtual
@@ -354,6 +356,7 @@
         'match': 'path',
         'model': 'model',
         'boot': 'boot',
+        'qemu-params': 'qemu_params',
         'image-size': 'image_size',
         'image-uuid': 'image_uuid',
         'image-name': 'image_name',
@@ -489,7 +492,7 @@
         }
         obj.devices[disk.path] = disk;
         stack.push(disk.path);
-    } else if (where.match(/zone\.device\.\/dev\/.*disk[0-9]+\.net-attr/)) {
+    } else if (where.match(/zone\.device\..*\.net-attr/)) {
         if (XML_PROPERTIES.disk.hasOwnProperty(attrs.name)) {
             use = XML_PROPERTIES.disk[attrs.name];
             obj.devices[stack[2]][use] = attrs.value;
@@ -3434,11 +3437,20 @@
     for (disk in vmobj.disks) {
         if (vmobj.disks.hasOwnProperty(disk)) {
             disk = vmobj.disks[disk];
+            if (!disk.model && !disk.qemu_params) {
+                continue;
+            }
             if (!disk.media) {
                 disk.media = 'disk';
             }
-            diskargs = 'file=' + disk.path + ',if=' + disk.model
-                + ',index=' + disk_idx + ',media=' + disk.media;
+            diskargs = 'file=' + disk.path;
+            if (disk.model) {
+                diskargs = diskargs + ',if=' + disk.model;
+            }
+            if (disk.qemu_params) {
+                diskargs = diskargs + ',' + disk.qemu_params;
+            }
+            diskargs = diskargs + ',index=' + disk_idx + ',media=' + disk.media;
             if (disk.boot) {
                 diskargs = diskargs + ',boot=on';
             }
@joshwilsdon joshwilsdon was assigned Feb 27, 2012
@antoinerg

+1

This feature or CIFS in kernel would be great to share files with Windows clients!

@trisk
trisk commented Sep 21, 2012

An example of a configuration using the above changes, with a regular zvol root shown for completeness:

  <attr name="qemu-extra-opts" type="string" value="LWRldmljZSBhaGNpLGlkPWFoY2kwLGJ1cz1wY2kuMCxtdWx0aWZ1bmN0aW9uPW9uIC1kZXZpY2UgaWRlLWRyaXZlLGRyaXZlPXNhdGEwLWRpc2swLGJ1cz1haGNpMC4wIC1kZXZpY2UgaWRlLWRyaXZlLGRyaXZlPXNhdGEwLWRpc2sxLGJ1cz1haGNpMC4x"/>
  <device match="/dev/zvol/rdsk/zones/f4763d2b-d656-4e4e-a3ae-0a16900bc9c6-disk0">
    <net-attr name="index" value="0"/>
    <net-attr name="boot" value="true"/>
    <net-attr name="model" value="ide"/>
    <net-attr name="size" value="20480"/>
  </device>
  <device match="/dev/rdsk/c0t0d0">
    <net-attr name="qemu-params" value="if=none,id=sata0-disk0"/>
  </device>
  <device match="/dev/rdsk/c0t0d1">
    <net-attr name="qemu-params" value="if=none,id=sata0-disk1"/>
  </device>

The base64-encoded 'qemu-extra-opts' string is: -device ahci,id=ahci0,bus=pci.0,multifunction=on -device ide-drive,drive=sata0-disk0,bus=ahci0.0 -device ide-drive,drive=sata0-disk1,bus=ahci0.1 This creates an AHCI controller (which allows more attachments than IDE) and two devices.

@mikl
mikl commented Dec 2, 2012

@trisk perhaps I'm missing something, but it seems the configuration has been changed to use JSON syntax. Any idea if this'll work with that as well?

@trisk trisk added a commit to trisk/smartos-live that referenced this issue Jul 11, 2013
@trisk trisk Closes #65 Allow raw disks (or other GZ devices) to be used by guests b760d4e
@trisk trisk added a commit to trisk/smartos-live that referenced this issue Jul 11, 2013
@trisk trisk Closes #65 Allow raw disks (or other GZ devices) to be used by guests 4f8d317
@trisk
trisk commented Jul 11, 2013

@mikl These attributes are in the zonecfg configuration and added after the fact. I don't know if you can pass them to vmadm create.

@tomww
tomww commented Oct 30, 2015

Does a more current SmartOS version allow raw disks without patching?

I would highly appreciate if this patch could be updated to work with an end-2015 SmartOS version.
The *js scripts have changed too much since 2012 as the above would be a quick task for me.

As the files reside on the ramdisk, I would patch in the changed with a SMF startscript at boot time.

Thanks for having a look into this. A plus would be to have this integrated in the regular SmartOS code one day!
@tomww

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment