[Libvir] [PATCH] Add bus attribute to disk target definition

Soren Hansen soren at ubuntu.com
Tue Apr 29 07:56:13 UTC 2008


Hi!

I'd like to propose that the following patch gets applied against
libvirt. It adds the option of putting a bus attribute on a disk target.
To acommodate this, it also changes the way drives are defined for kvm
from the old "-hda /path/to/file -boot c" style to the new "-drive
file=/path/to/file,if=ide,boot=on". This makes it possible to specify
virtio, scsi, and ide disks.


=== modified file 'src/qemu_conf.c'
--- src/qemu_conf.c	2008-04-28 15:14:59 +0000
+++ src/qemu_conf.c	2008-04-29 07:43:11 +0000
@@ -568,6 +568,7 @@
     xmlChar *source = NULL;
     xmlChar *target = NULL;
     xmlChar *type = NULL;
+    xmlChar *bus = NULL;
     int typ = 0;
 
     type = xmlGetProp(node, BAD_CAST "type");
@@ -598,6 +599,7 @@
             } else if ((target == NULL) &&
                        (xmlStrEqual(cur->name, BAD_CAST "target"))) {
                 target = xmlGetProp(cur, BAD_CAST "dev");
+                bus = xmlGetProp(cur, BAD_CAST "bus");
             } else if (xmlStrEqual(cur->name, BAD_CAST "readonly")) {
                 disk->readonly = 1;
             }
@@ -646,7 +648,9 @@
         strcmp((const char *)target, "hda") &&
         strcmp((const char *)target, "hdb") &&
         strcmp((const char *)target, "hdc") &&
-        strcmp((const char *)target, "hdd")) {
+        strcmp((const char *)target, "hdd") &&
+        strcmp((const char *)target, "hdd") &&
+        strncmp((const char *)target, "vd", 2)) {
         qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
                          _("Invalid harddisk device name: %s"), target);
         goto error;
@@ -673,6 +677,20 @@
         goto error;
     }
 
+    if (!bus)
+        disk->bus = QEMUD_DISK_BUS_IDE;
+    else if (!strcmp((const char *)bus, "ide"))
+        disk->bus = QEMUD_DISK_BUS_IDE;
+    else if (!strcmp((const char *)bus, "scsi"))
+        disk->bus = QEMUD_DISK_BUS_SCSI;
+    else if (!strcmp((const char *)bus, "virtio"))
+        disk->bus = QEMUD_DISK_BUS_VIRTIO;
+    else {
+        qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, "Invalid bus type: %s", bus);
+        goto error;
+    }
+
+    xmlFree(bus);
     xmlFree(device);
     xmlFree(target);
     xmlFree(source);
@@ -688,6 +706,8 @@
         xmlFree(source);
     if (device)
         xmlFree(device);
+    if (bus)
+        xmlFree(bus);
     return -1;
 }
 
@@ -1350,6 +1370,68 @@
     return -1;
 }
 
+static int qemudDiskCompare(const void *aptr, const void *bptr) {
+    struct qemud_vm_disk_def *a = (struct qemud_vm_disk_def *) aptr;
+    struct qemud_vm_disk_def *b = (struct qemud_vm_disk_def *) bptr;
+    if (a->device == b->device)
+        return virDiskNameToIndex(a->dst) - virDiskNameToIndex(b->dst);
+    else
+        return a->device - b->device;
+}
+
+static const char *qemudBusIdToName(int busId) {
+    const char *busnames[] = { "ide",
+                                "scsi",
+                                "virtio" };
+
+	if (busId >= 0 && busId < 3)
+	    return busnames[busId];
+	else
+		return 0;
+}
+
+static char *qemudDriveOpt(struct qemud_vm_disk_def *disk, int boot)
+{
+    char opt[PATH_MAX];
+
+    switch (disk->device) {
+        case QEMUD_DISK_CDROM:
+            snprintf(opt, PATH_MAX, "file=%s,if=ide,media=cdrom%s",
+                          disk->src, boot ? ",boot=on" : "");
+        break;
+        case QEMUD_DISK_FLOPPY:
+            snprintf(opt, PATH_MAX, "file=%s,if=floppy%s",
+                          disk->src, boot ? ",boot=on" : "");
+        break;
+        case QEMUD_DISK_DISK:
+            snprintf(opt, PATH_MAX, "file=%s,if=%s%s",
+                          disk->src, qemudBusIdToName(disk->bus), boot ? ",boot=on" : "");
+        break;
+        default:
+            return 0;
+    }
+    return strdup(opt);
+}
+
+static char *qemudAddBootDrive(virConnectPtr conn,
+                                struct qemud_vm_def *def,
+                            	char *handledDisks,
+                            	int type) {
+    int j = 0;
+    struct qemud_vm_disk_def *disk = def->disks;
+
+    while (disk) {
+        if (!handledDisks[j] && disk->device == type) {
+            handledDisks[j] = 1;
+            return qemudDriveOpt(disk, 1);
+        }
+        j++;
+        disk = disk->next;
+    }
+    qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
+                         "Requested boot device type %d, but no such device defined.", type);
+    return 0;
+}
 
 /*
  * Parses a libvirt XML definition of a guest, and populates the
@@ -1739,7 +1821,6 @@
     obj = xmlXPathEval(BAD_CAST "/domain/devices/disk", ctxt);
     if ((obj != NULL) && (obj->type == XPATH_NODESET) &&
         (obj->nodesetval != NULL) && (obj->nodesetval->nodeNr >= 0)) {
-        struct qemud_vm_disk_def *prev = NULL;
         for (i = 0; i < obj->nodesetval->nodeNr; i++) {
             struct qemud_vm_disk_def *disk = calloc(1, sizeof(*disk));
             if (!disk) {
@@ -1752,13 +1833,20 @@
                 goto error;
             }
             def->ndisks++;
-            disk->next = NULL;
             if (i == 0) {
+                disk->next = NULL;
                 def->disks = disk;
             } else {
-                prev->next = disk;
+                struct qemud_vm_disk_def *ptr = def->disks;
+                while (ptr) {
+                    if (!ptr->next || qemudDiskCompare(ptr->next, disk) < 0) {
+                        disk->next = ptr->next;
+                        ptr->next = disk;
+                        break;
+                    }
+                    ptr = ptr->next;
+                }
             }
-            prev = disk;
         }
     }
     xmlXPathFreeObject(obj);
@@ -2207,30 +2295,32 @@
             goto no_memory;
     }
 
-    for (i = 0 ; i < vm->def->os.nBootDevs ; i++) {
-        switch (vm->def->os.bootDevs[i]) {
-        case QEMUD_BOOT_CDROM:
-            boot[i] = 'd';
-            break;
-        case QEMUD_BOOT_FLOPPY:
-            boot[i] = 'a';
-            break;
-        case QEMUD_BOOT_DISK:
-            boot[i] = 'c';
-            break;
-        case QEMUD_BOOT_NET:
-            boot[i] = 'n';
-            break;
-        default:
-            boot[i] = 'c';
-            break;
+    if (vm->def->virtType != QEMUD_VIRT_KVM) {
+        for (i = 0 ; i < vm->def->os.nBootDevs ; i++) {
+            switch (vm->def->os.bootDevs[i]) {
+            case QEMUD_BOOT_CDROM:
+                boot[i] = 'd';
+                break;
+            case QEMUD_BOOT_FLOPPY:
+                boot[i] = 'a';
+                break;
+            case QEMUD_BOOT_DISK:
+                boot[i] = 'c';
+                break;
+            case QEMUD_BOOT_NET:
+                boot[i] = 'n';
+                break;
+            default:
+                boot[i] = 'c';
+                break;
+            }
         }
+        boot[vm->def->os.nBootDevs] = '\0';
+        if (!((*argv)[++n] = strdup("-boot")))
+            goto no_memory;
+        if (!((*argv)[++n] = strdup(boot)))
+            goto no_memory;
     }
-    boot[vm->def->os.nBootDevs] = '\0';
-    if (!((*argv)[++n] = strdup("-boot")))
-        goto no_memory;
-    if (!((*argv)[++n] = strdup(boot)))
-        goto no_memory;
 
     if (vm->def->os.kernel[0]) {
         if (!((*argv)[++n] = strdup("-kernel")))
@@ -2251,28 +2341,74 @@
             goto no_memory;
     }
 
-    while (disk) {
-        char dev[NAME_MAX];
-        char file[PATH_MAX];
-        if (!strcmp(disk->dst, "hdc") &&
-            disk->device == QEMUD_DISK_CDROM) {
-            if (disk->src[0])
-                snprintf(dev, NAME_MAX, "-%s", "cdrom");
-            else {
-                /* Don't put anything on the cmdline for an empty cdrom*/
-                disk = disk->next;
-                continue;
-            }
-        } else
-            snprintf(dev, NAME_MAX, "-%s", disk->dst);
-        snprintf(file, PATH_MAX, "%s", disk->src);
-
-        if (!((*argv)[++n] = strdup(dev)))
-            goto no_memory;
-        if (!((*argv)[++n] = strdup(file)))
-            goto no_memory;
-
-        disk = disk->next;
+    if (vm->def->virtType == QEMUD_VIRT_KVM) {
+        char *handledDisks = NULL;
+        int j;
+
+        handledDisks = calloc(sizeof(*handledDisks), vm->def->ndisks);
+
+        if (!handledDisks)
+            goto no_memory;
+
+        /* When using -drive notation, we need to provide the devices in boot
+         * preference order. */
+        for (i = 0 ; i < vm->def->os.nBootDevs ; i++) {
+            if (!((*argv)[++n] = strdup("-drive")))
+                goto no_memory;
+
+            switch (vm->def->os.bootDevs[i]) {
+                case QEMUD_BOOT_CDROM:
+                    if (!((*argv)[++n] = qemudAddBootDrive(conn, vm->def, handledDisks, QEMUD_DISK_CDROM)))
+                        goto error;
+                    break;
+                 case QEMUD_BOOT_FLOPPY:
+                    if (!((*argv)[++n] = qemudAddBootDrive(conn, vm->def, handledDisks, QEMUD_DISK_FLOPPY)))
+                        goto error;
+                break;
+                case QEMUD_BOOT_DISK:
+                    if (!((*argv)[++n] = qemudAddBootDrive(conn, vm->def, handledDisks, QEMUD_DISK_DISK)))
+                        goto error;
+                break;
+            }
+        }
+
+        /* Pick up the rest of the devices */
+        j=0;
+        while (disk) {
+            if (!handledDisks[j]) {
+                handledDisks[j] = 1;
+                if (!((*argv)[++n] = strdup("-drive")))
+                    goto no_memory;
+                if (!((*argv)[++n] = qemudDriveOpt(disk, 0)))
+                    goto no_memory;
+            }
+            disk = disk->next;
+            j++;
+        }
+    } else {
+        while (disk) {
+            char dev[NAME_MAX];
+            char file[PATH_MAX];
+
+            if (!strcmp(disk->dst, "hdc") &&
+                disk->device == QEMUD_DISK_CDROM)
+				if (disk->src[0])
+	                snprintf(dev, NAME_MAX, "-%s", "cdrom");
+				else {
+					disk = disk->next;
+					continue;
+				}
+            else
+                snprintf(dev, NAME_MAX, "-%s", disk->dst);
+            snprintf(file, PATH_MAX, "%s", disk->src);
+
+            if (!((*argv)[++n] = strdup(dev)))
+                goto no_memory;
+            if (!((*argv)[++n] = strdup(file)))
+                goto no_memory;
+
+            disk = disk->next;
+        }
     }
 
     if (!net) {
@@ -3565,6 +3701,7 @@
             virBufferVSprintf(&buf, "      <source %s='%s'/>\n",
                               typeAttrs[disk->type], disk->src);
 
+        virBufferVSprintf(&buf, "      <target dev='%s' bus='%s'/>\n", disk->dst, qemudBusIdToName(disk->bus));
         virBufferVSprintf(&buf, "      <target dev='%s'/>\n", disk->dst);
 
         if (disk->readonly)

=== modified file 'src/qemu_conf.h'
--- src/qemu_conf.h	2008-04-25 20:46:13 +0000
+++ src/qemu_conf.h	2008-04-29 07:13:16 +0000
@@ -56,10 +56,17 @@
     QEMUD_DISK_FLOPPY,
 };
 
+enum qemud_vm_disk_bus {
+    QEMUD_DISK_BUS_IDE,
+    QEMUD_DISK_BUS_SCSI,
+    QEMUD_DISK_BUS_VIRTIO
+};
+
 /* Stores the virtual disk configuration */
 struct qemud_vm_disk_def {
     int type;
     int device;
+    int bus;
     char src[PATH_MAX];
     char dst[NAME_MAX];
     int readonly;

=== modified file 'src/util.c'
--- src/util.c	2008-04-25 14:53:05 +0000
+++ src/util.c	2008-04-29 06:59:49 +0000
@@ -771,3 +771,43 @@
 
     return -1;
 }
+
+/* Translates a device name of the form (regex) "[fhv]d[a-z]+" into
+ * the corresponding index (e.g. sda => 1, hdz => 26, vdaa => 27)
+ * @param name The name of the device
+ * @return name's index, or 0 on failure
+ */
+int virDiskNameToIndex(const char *name) {
+    const char *ptr = NULL;
+    int idx = 0;
+
+    if (strlen(name) < 3)
+        return 0;
+
+    switch (*name) {
+        case 'f':
+        case 'h':
+        case 'v':
+            break;
+        default:
+            return 0;
+    }
+
+    if (*(name + 1) != 'd')
+        return 0;
+
+    ptr = name+2;
+
+    while (*ptr) {
+        idx = idx * 26;
+
+        if ('a' > *ptr || 'z' < *ptr)
+            return 0;
+
+        idx += *ptr - 'a' + 1;
+        ptr++;
+    }
+
+    return idx;
+}
+

=== modified file 'src/util.h'
--- src/util.h	2008-04-25 14:53:05 +0000
+++ src/util.h	2008-04-29 06:59:57 +0000
@@ -92,4 +92,6 @@
 
 int virParseMacAddr(const char* str, unsigned char *addr);
 
+int virDiskNameToIndex(const char* str);
+
 #endif /* __VIR_UTIL_H__ */


-- 
Soren Hansen               | 
Virtualisation specialist  | Ubuntu Server Team
Canonical Ltd.             | http://www.ubuntu.com/
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20080429/82fb041a/attachment-0001.sig>


More information about the libvir-list mailing list