2:56 a.m.
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/
8:27 a.m.
On Tue, Apr 29, 2008 at 09:56:13AM +0200, Soren Hansen wrote:
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.
@@ -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") &&
These two lines test the same thing !
+ strncmp((const char *)target, "vd", 2)) {
Its probably a better idea to just compress the explicit hda -> hdd
comparisons down into a single 'hd' prefix comparison, as you did
with 'vd'.
+ 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;
Can you use the STREQ macro here instead of strcmp.
+ else {
+ qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, "Invalid bus
type: %s", bus);
+ goto error;
+ }
This line needs to be marked for translation, with _(...). You can
run 'make syntax-check' for check for such issues.
+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;
+}
@@ -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) {
This is wrong - both QEMU and KVM can support the -drive parameter,
and not all KVM support it. You need to add a check in the method
qemudExtractVersionInfo() to probe for availability of the -drive
option, as we do with other opts - see QEMUD_CMD_FLAG_NO_REBOOT
for example.
Even if the -drive parameter is supported, it should still pass the
-boot a/c/d/n parameter in.
+ 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;
+ }
+ }
There is nothing in the -drive parameter handling, AFAICT, that requires
the boot drive to be listed first on the command line. So this first loop
is not needed, and this second loop is sufficient, simply turn on the
boot= flag on the first match drive type when iterating over the list.
+
+ /* 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++;
+ }
=== 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':
You need a case 's' there to allow for SCSI...
Regards,
Daniel
--
|: Red Hat, Engineering, Boston -o- http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
9:10 a.m.
On Tue, Apr 29, 2008 at 02:27:00PM +0100, Daniel P. Berrange wrote:
> + strcmp((const char *)target, "hdd")
&&
> + strcmp((const char *)target, "hdd") &&
These two lines test the same thing !
Quite so. My bad.
> + strncmp((const char *)target, "vd", 2)) {
Its probably a better idea to just compress the explicit hda -> hdd
comparisons down into a single 'hd' prefix comparison, as you did
with 'vd'.
Hm.. Yes, I suppose that if you're using -drive notation, "hd[e-z]"
starts making sense. Fixed.
> + 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;
Can you use the STREQ macro here instead of strcmp.
Erm... I *could*.. I'm curious, though, why e.g. the similar code right
above it doesn't use STREQ if that's the preferred way to do it?
IMO, it's better to change this sort of things all in one go, and keep
everything consistent until then.
> + else {
> + qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, "Invalid bus
type: %s", bus);
> + goto error;
> + }
This line needs to be marked for translation, with _(...).
Fixed.
You can run 'make syntax-check' for check for such issues.
Yes, in theory :) In the real world, however, "make syntax-check" fails
horribly here. I'll be fixing that up next.
> +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;
> +}
>
> @@ -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) {
This is wrong - both QEMU and KVM can support the -drive parameter,
Oh, I wasn't aware. Hmm..
and not all KVM support it. You need to add a check in the method
qemudExtractVersionInfo() to probe for availability of the -drive
option, as we do with other opts - see QEMUD_CMD_FLAG_NO_REBOOT for
example.
Interesting. I'll add that.
Even if the -drive parameter is supported, it should still pass the
-boot a/c/d/n parameter in.
Why? And how would you boot from a virtio device this way?
There is nothing in the -drive parameter handling, AFAICT, that
requires the boot drive to be listed first on the command line. So
this first loop is not needed, and this second loop is sufficient,
simply turn on the boot= flag on the first match drive type when
iterating over the list.
If you want to specify more than one boot device, the only way to
determine the priority is by specifying them ordered by priority. At
least, that's how it *ought* to be, but now I see that extboot only
actually supports one boot device. :/ I could have sworn I had seen it
work with both hard drive and cdrom.
> +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':
You need a case 's' there to allow for SCSI...
Good point. I suppose I do so in qemudParseDiskXml as well (when
checking the target).
--
Soren Hansen |
Virtualisation specialist | Ubuntu Server Team
Canonical Ltd. | http://www.ubuntu.com/
9:17 a.m.
On Tue, Apr 29, 2008 at 04:10:40PM +0200, Soren Hansen wrote:
On Tue, Apr 29, 2008 at 02:27:00PM +0100, Daniel P. Berrange wrote:
>> + 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;
> Can you use the STREQ macro here instead of strcmp.
Erm... I *could*.. I'm curious, though, why e.g. the similar code right
above it doesn't use STREQ if that's the preferred way to do it?
We've been slowly updating code to match these new standards when doing
patches.
> You can run 'make syntax-check' for check for such
issues.
Yes, in theory :) In the real world, however, "make syntax-check" fails
horribly here. I'll be fixing that up next.
If you send details to the list, Jim will no doubt be able to point you
in the right direction on this...
> Even if the -drive parameter is supported, it should still pass
the
> -boot a/c/d/n parameter in.
Why? And how would you boot from a virtio device this way?
It is needed for PXE boot at least, and IMHO, QEMU should treat 'boot c'
as if 'boot=on' were set for the first -drive parameter for back compat.
> There is nothing in the -drive parameter handling, AFAICT, that
> requires the boot drive to be listed first on the command line. So
> this first loop is not needed, and this second loop is sufficient,
> simply turn on the boot= flag on the first match drive type when
> iterating over the list.
If you want to specify more than one boot device, the only way to
determine the priority is by specifying them ordered by priority. At
least, that's how it *ought* to be, but now I see that extboot only
actually supports one boot device. :/ I could have sworn I had seen it
work with both hard drive and cdrom.
The QEMU code only allows a single boot device & will abort if > 1 has it
if (extboot_drive != -1) {
fprintf(stderr, "qemu: two bootable drives specified\n");
return -1;
}
Regards,
Dan.
--
|: Red Hat, Engineering, Boston -o- http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
9:22 a.m.
"Daniel P. Berrange" <berrange(a)redhat.com> wrote:
On Tue, Apr 29, 2008 at 04:10:40PM +0200, Soren Hansen wrote:
> On Tue, Apr 29, 2008 at 02:27:00PM +0100, Daniel P. Berrange wrote:
> >> + 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;
> > Can you use the STREQ macro here instead of strcmp.
>
> Erm... I *could*.. I'm curious, though, why e.g. the similar code right
> above it doesn't use STREQ if that's the preferred way to do it?
We've been slowly updating code to match these new standards when doing
patches.
FYI, there's already a test for this, but it's currently disabled.
If someone wants to turn it on (remove the sc_prohibit_strcmp
line from Makefile.cfg) and fix all the new violations exposed
by running "make syntax-check", it sounds like it would be welcome, now.
> > You can run 'make syntax-check' for check for such
issues.
>
> Yes, in theory :) In the real world, however, "make syntax-check" fails
> horribly here. I'll be fixing that up next.
If you send details to the list, Jim will no doubt be able to point you
in the right direction on this...
Yep. Though most of it is intended to be self explanatory.
If not, please let me know and I'll fix the rules in Makefile.maint.
9:33 a.m.
Jim Meyering <jim(a)meyering.net> wrote:
"Daniel P. Berrange" <berrange(a)redhat.com> wrote:
> On Tue, Apr 29, 2008 at 04:10:40PM +0200, Soren Hansen wrote:
>> On Tue, Apr 29, 2008 at 02:27:00PM +0100, Daniel P. Berrange wrote:
>> >> + 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;
>> > Can you use the STREQ macro here instead of strcmp.
>>
>> Erm... I *could*.. I'm curious, though, why e.g. the similar code right
>> above it doesn't use STREQ if that's the preferred way to do it?
>
> We've been slowly updating code to match these new standards when doing
> patches.
FYI, there's already a test for this, but it's currently disabled.
If someone wants to turn it on (remove the sc_prohibit_strcmp
line from Makefile.cfg) and fix all the new violations exposed
by running "make syntax-check", it sounds like it would be welcome, now.
In preparation for this, I've just relaxed the regexp in the check
to allow for libvirt's varying coding styles (space or not between
function name and opening parenthesis):
tests: recognize more uses of strcmp.
* Makefile.maint (sc_prohibit_strcmp): Relax regexp.
=============================================================================
Index: Makefile.maint
===================================================================
RCS file: /data/cvs/libvirt/Makefile.maint,v
retrieving revision 1.21
retrieving revision 1.22
diff -u -p -u -r1.21 -r1.22
--- Makefile.maint 21 Apr 2008 10:09:07 -0000 1.21
+++ Makefile.maint 29 Apr 2008 14:25:19 -0000 1.22
@@ -91,7 +91,7 @@ sc_prohibit_atoi_atof:
# Use STREQ rather than comparing strcmp == 0, or != 0.
sc_prohibit_strcmp:
- @grep -nE '! *str''cmp \(|\<str''cmp \([^)]+\) *==' \
+ @grep -nE '! *str''cmp *\(|\<str''cmp *\([^)]+\) *==' \
$$($(VC_LIST_EXCEPT)) && \
{ echo '$(ME): use STREQ in place of the above uses of str''cmp' \
1>&2; exit 1; } || :
9:42 a.m.
On Tue, Apr 29, 2008 at 03:17:14PM +0100, Daniel P. Berrange wrote:
>>> + 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;
>> Can you use the STREQ macro here instead of strcmp.
> Erm... I *could*.. I'm curious, though, why e.g. the similar code right
> above it doesn't use STREQ if that's the preferred way to do it?
We've been slowly updating code to match these new standards when doing
patches.
Well, if that's the way you do it, I'll follow suit.. However, I have
to say that I pity the person that reads the code and finds these two
sections of code that seem to do rather similar things, but use
different functions to do it, and then has to work out what on earth the
difference between the two might be.
>> You can run 'make syntax-check' for check for such
issues.
> Yes, in theory :) In the real world, however, "make syntax-check"
> fails horribly here. I'll be fixing that up next.
If you send details to the list, Jim will no doubt be able to point
you in the right direction on this...
I'll do that in a minute. Thanks.
>> Even if the -drive parameter is supported, it should still
pass the
>> -boot a/c/d/n parameter in.
> Why? And how would you boot from a virtio device this way?
It is needed for PXE boot at least, and IMHO,
Good point.
QEMU should treat 'boot c' as if 'boot=on' were set
for the first
-drive parameter for back compat.
Yes, indeed it should. Sadly, though, it doesn't.
kvm -drive file=root.qcow,if=virtio -boot c
fails miserably, while
kvm -drive file=root.qcow,if=virtio,boot=on
works beautifully.
This logic is going to look horrible :( Something like:
if boot == hd && (one of the disks is a virtio disk)
then
use new style -drive foo,boot=on notation
else
use old style -boot [acdn] notation
?
>> There is nothing in the -drive parameter handling, AFAICT,
that
>> requires the boot drive to be listed first on the command line. So
>> this first loop is not needed, and this second loop is sufficient,
>> simply turn on the boot= flag on the first match drive type when
>> iterating over the list.
> If you want to specify more than one boot device, the only way to
> determine the priority is by specifying them ordered by priority. At
> least, that's how it *ought* to be, but now I see that extboot only
> actually supports one boot device. :/ I could have sworn I had seen
> it work with both hard drive and cdrom.
The QEMU code only allows a single boot device & will abort if > 1 has
it
if (extboot_drive != -1) {
fprintf(stderr, "qemu: two bootable drives specified\n");
return -1;
}
Yes, that's what I noticed earlier today, although I geniunely hope this
is something that will be fixed at some point, and when that happy day
comes, I've either guessed correctly as to how it will derive boot
priorities and we won't have to fix anything, or I've guessed wrong, and
then we'll be no worse off than if we didn't adopt this approach right
now, AFAICS.
--
Soren Hansen |
Virtualisation specialist | Ubuntu Server Team
Canonical Ltd. | http://www.ubuntu.com/
10 a.m.
On Tue, Apr 29, 2008 at 04:42:54PM +0200, Soren Hansen wrote:
On Tue, Apr 29, 2008 at 03:17:14PM +0100, Daniel P. Berrange wrote:
>>>> + 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;
>>> Can you use the STREQ macro here instead of strcmp.
>> Erm... I *could*.. I'm curious, though, why e.g. the similar code right
>> above it doesn't use STREQ if that's the preferred way to do it?
> We've been slowly updating code to match these new standards when doing
> patches.
Well, if that's the way you do it, I'll follow suit.. However, I have
to say that I pity the person that reads the code and finds these two
sections of code that seem to do rather similar things, but use
different functions to do it, and then has to work out what on earth the
difference between the two might be.
Here is an update to the HACKING file intended to describe some of the
conventions in use...
Dan
Index: HACKING
===================================================================
RCS file: /data/cvs/libvirt/HACKING,v
retrieving revision 1.1
diff -r1.1 HACKING
45a46,160
Low level memory management
===========================
Use of the malloc/free/realloc/calloc APIs is deprecated in the libvirt
codebase, because they encourage a number of serious coding bugs and do
not enable compile time verification of checks for NULL. Instead of these
routines, use the macros from memory.h
- eg to allocate a single object:
virDomainPtr domain;
if (VIR_ALLOC(domain) < 0) {
__virRaiseError(VIR_ERROR_NO_MEMORY)
return NULL;
}
- eg to allocate an array of objects
virDomainPtr domains;
int ndomains = 10;
if (VIR_ALLOC_N(domains, ndomains) < 0) {
__virRaiseError(VIR_ERROR_NO_MEMORY)
return NULL;
}
- eg to allocate an array of object pointers
virDomainPtr *domains;
int ndomains = 10;
if (VIR_ALLOC_N(domains, ndomains) < 0) {
__virRaiseError(VIR_ERROR_NO_MEMORY)
return NULL;
}
- eg to re-allocate the array of domains to be longer
ndomains = 20
if (VIR_REALLOC_N(domains, ndomains) < 0) {
__virRaiseError(VIR_ERROR_NO_MEMORY)
return NULL;
}
- eg to free the domain
VIR_FREE(domain);
String comparisons
==================
Do not use the strcmp, strncmp, etc functions directly. Instead use
one of the following semantically named macros
- For strict equality:
STREQ(a,b)
STRNEQ(a,b)
- For case sensitive equality:
STRCASEEQ(a,b)
STRCASENEQ(a,b)
- For strict equality of a substring:
STREQLEN(a,b,n)
STRNEQLEN(a,b,n)
- For case sensitive equality of a substring:
STRCASEEQLEN(a,b,n)
STRCASENEQLEN(a,b,n)
- For strict equality of a prefix:
STRPREFIX(a,b)
Variable length string buffer
=============================
If there is a need for complex string concatenations, avoid using
the usual sequence of malloc/strcpy/strcat/snprintf functions and
make use of the virBuffer API described in buf.h
eg typical usage is as follows:
char *
somefunction(...) {
virBuffer buf = VIR_BUFFER_INITIALIZER;
...
virBufferAdd(&buf, "<domain>\n");
virBufferVSprint(&buf, " <memory>%d</memory>\n",
memory);
...
virBufferAdd(&buf, "</domain>\n");
....
if (virBufferError(&buf)) {
__virRaiseError(...);
return NULL;
}
return virBufferContentAndReset(&buf);
}
--
|: Red Hat, Engineering, Boston -o- http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
9:47 a.m.
On Tue, 2008-04-29 at 15:17 +0100, Daniel P. Berrange wrote:
> > Even if the -drive parameter is supported, it should still
pass the
> > -boot a/c/d/n parameter in.
>
> Why? And how would you boot from a virtio device this way?
It is needed for PXE boot at least, and IMHO, QEMU should treat 'boot c'
as if 'boot=on' were set for the first -drive parameter for back compat.
It was probably done this way because for normal IDE drivers, "-boot c"
is a different boot path than "boot=on" - i.e. the former goes through
the normal BIOS boot path, but "boot=on" uses the extboot PCI option ROM
hack.
Probably could just use extboot for "-boot c" when the drive in question
isn't an IDE drive, though ...
Cheers,
Mark.
1:11 p.m.
Hi,
sorry for the delay. Here's the newest version of the patch which should
address all the issues that have been raised so far.
=== modified file 'src/qemu_conf.c'
--- old/src/qemu_conf.c 2008-04-30 12:30:55 +0000
+++ new/src/qemu_conf.c 2008-05-06 17:58:44 +0000
@@ -491,6 +491,8 @@
*flags |= QEMUD_CMD_FLAG_KQEMU;
if (strstr(help, "-no-reboot"))
*flags |= QEMUD_CMD_FLAG_NO_REBOOT;
+ if (strstr(help, "\n-drive"))
+ *flags |= QEMUD_CMD_FLAG_DRIVE_OPT;
if (*version >= 9000)
*flags |= QEMUD_CMD_FLAG_VNC_COLON;
ret = 0;
@@ -568,6 +570,7 @@
xmlChar *source = NULL;
xmlChar *target = NULL;
xmlChar *type = NULL;
+ xmlChar *bus = NULL;
int typ = 0;
type = xmlGetProp(node, BAD_CAST "type");
@@ -598,6 +601,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;
}
@@ -643,10 +647,9 @@
disk->readonly = 1;
if ((!device || !strcmp((const char *)device, "disk")) &&
- strcmp((const char *)target, "hda") &&
- strcmp((const char *)target, "hdb") &&
- strcmp((const char *)target, "hdc") &&
- strcmp((const char *)target, "hdd")) {
+ strncmp((const char *)target, "hd", 2) &&
+ strncmp((const char *)target, "sd", 2) &&
+ strncmp((const char *)target, "vd", 2)) {
qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
_("Invalid harddisk device name: %s"), target);
goto error;
@@ -673,13 +676,28 @@
goto error;
}
+ if (!bus)
+ disk->bus = QEMUD_DISK_BUS_IDE;
+ else if (STREQ((const char *)bus, "ide"))
+ disk->bus = QEMUD_DISK_BUS_IDE;
+ else if (STREQ((const char *)bus, "scsi"))
+ disk->bus = QEMUD_DISK_BUS_SCSI;
+ else if (STREQ((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(device);
xmlFree(target);
xmlFree(source);
+ xmlFree(bus);
return 0;
error:
+ xmlFree(bus);
xmlFree(type);
xmlFree(target);
xmlFree(source);
@@ -1373,6 +1391,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
@@ -1762,7 +1842,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) {
@@ -1775,13 +1854,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);
@@ -2110,6 +2196,7 @@
struct qemud_vm_chr_def *parallel = vm->def->parallels;
struct utsname ut;
int disableKQEMU = 0;
+ int use_extboot = 0;
/* Make sure the binary we are about to try exec'ing exists.
* Technically we could catch the exec() failure, but that's
@@ -2230,30 +2317,47 @@
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;
- }
- }
- boot[vm->def->os.nBootDevs] = '\0';
- if (!((*argv)[++n] = strdup("-boot")))
- goto no_memory;
- if (!((*argv)[++n] = strdup(boot)))
- goto no_memory;
+ /* Use extboot if available and required */
+ if ((vm->qemuCmdFlags & QEMUD_CMD_FLAG_DRIVE_OPT) &&
+ (vm->def->os.nBootDevs == 1) &&
+ (vm->def->os.bootDevs[0] == QEMUD_BOOT_DISK)) {
+ struct qemud_vm_disk_def *tmp = disk;
+
+ while (tmp) {
+ if (tmp->bus == QEMUD_DISK_BUS_VIRTIO) {
+ use_extboot = 1;
+ break;
+ }
+ tmp = tmp->next;
+ }
+ }
+
+ if (!use_extboot) {
+ 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;
+ }
if (vm->def->os.kernel[0]) {
if (!((*argv)[++n] = strdup("-kernel")))
@@ -2274,28 +2378,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 (use_extboot) {
+ 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) {
@@ -3587,6 +3737,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'
--- old/src/qemu_conf.h 2008-04-30 12:30:55 +0000
+++ new/src/qemu_conf.h 2008-05-06 17:52:17 +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;
@@ -223,6 +230,7 @@
QEMUD_CMD_FLAG_KQEMU = 1,
QEMUD_CMD_FLAG_VNC_COLON = 2,
QEMUD_CMD_FLAG_NO_REBOOT = 4,
+ QEMUD_CMD_FLAG_DRIVE_OPT = 8,
};
=== modified file 'src/util.c'
--- old/src/util.c 2008-04-25 14:53:05 +0000
+++ new/src/util.c 2008-05-06 17:52:17 +0000
@@ -771,3 +771,44 @@
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':
+ case 's':
+ 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'
--- old/src/util.h 2008-04-25 14:53:05 +0000
+++ new/src/util.h 2008-05-06 17:52:18 +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/
4:02 p.m.
New subject: [libvirt] Re: [Libvir] [PATCH] Add bus attribute to disk target definition
On Tue, May 06, 2008 at 08:11:11PM +0200, Soren Hansen wrote:
sorry for the delay. Here's the newest version of the patch which
should
address all the issues that have been raised so far.
Yes & no. It didn't address the redundant re-ordering of -drive parameters
when using boot=on, nor re-add the -boot param to make PXE work again. One
further complication is that QEMU 0.9.1 has -drive support but not the
extboot support, so boot=on can't be used there. It rather annoyingly
complains
qemu: unknowm parameter 'boot' in
'file=/home/berrange/boot.iso,media=cdrom,boot=on'
=== modified file 'src/qemu_conf.c'
--- old/src/qemu_conf.c 2008-04-30 12:30:55 +0000
+++ new/src/qemu_conf.c 2008-05-06 17:58:44 +0000
@@ -491,6 +491,8 @@
*flags |= QEMUD_CMD_FLAG_KQEMU;
if (strstr(help, "-no-reboot"))
*flags |= QEMUD_CMD_FLAG_NO_REBOOT;
+ if (strstr(help, "\n-drive"))
+ *flags |= QEMUD_CMD_FLAG_DRIVE_OPT;
Turns out that this needed to also check for 'boot=on' availability.
@@ -673,13 +676,28 @@
goto error;
}
+ if (!bus)
+ disk->bus = QEMUD_DISK_BUS_IDE;
This was giving floppy disks a bus of 'ide' which isn't technically
correct - they're really their own bus type - I reckon we should call
it 'fdc'.
+ else if (STREQ((const char *)bus, "ide"))
+ disk->bus = QEMUD_DISK_BUS_IDE;
+ else if (STREQ((const char *)bus, "scsi"))
+ disk->bus = QEMUD_DISK_BUS_SCSI;
+ else if (STREQ((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(device);
xmlFree(target);
xmlFree(source);
+ xmlFree(bus);
return 0;
error:
+ xmlFree(bus);
xmlFree(type);
xmlFree(target);
xmlFree(source);
@@ -1373,6 +1391,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;
Typo - this should be comparing the 'bus' field rather than 'device' to
get IDE, SCSI and VirtIO disks ordered correctly wrt to each other.
+static const char *qemudBusIdToName(int busId) {
+ const char *busnames[] = { "ide",
+ "scsi",
+ "virtio" };
+
+ if (busId >= 0 && busId < 3)
+ return busnames[busId];
+ else
+ return 0;
+}
This can be changed to make use of the GNULIB 'verify_true' function
to get a compile time check fo the array cardinality vs the enum
length.
+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" : "");
This is missing the 'index=NNN' parameter so IDE disks are not getting
assigned to the correct bus/unit. eg if i define 'hda' and 'hdd' in the
XML the guest gets given 'hda' and 'hdb'.
@@ -1775,13 +1854,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) {
The parameters to the compare function are inverted so the ordering
is being reversed.
@@ -2110,6 +2196,7 @@
struct qemud_vm_chr_def *parallel = vm->def->parallels;
struct utsname ut;
int disableKQEMU = 0;
+ int use_extboot = 0;
/* Make sure the binary we are about to try exec'ing exists.
* Technically we could catch the exec() failure, but that's
@@ -2230,30 +2317,47 @@
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;
- }
- }
- boot[vm->def->os.nBootDevs] = '\0';
- if (!((*argv)[++n] = strdup("-boot")))
- goto no_memory;
- if (!((*argv)[++n] = strdup(boot)))
- goto no_memory;
This block shouldn't be removed, '-boot XXX' must be addeed
unconditionally whether using -drive's boot= flag or not.
+ if (use_extboot) {
+ 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++;
+ }
This double loop is redundant - there's no need to pull bootable drives to
the front of the list - this is why there's an explicit flag rather than
relying on ordering.
+ } 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);
This needed a check to verify that someone doesn't try to use virtio
disks with an old QEMU/KVM without -drive support.
@@ -3587,6 +3737,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);
The 2nd <target> tag ought to have been removed here...
I'm includng a updated version of your patch which addresses all these
notes & verifies them with the test suite we have. I believe this ought
to work no matter what version of QEMU / KVM is used - though obviously
older one's don't have option if using virtio.
I've also added a test case specifically for the new -drive / virtio
syntax
<disk type='block' device='disk'>
<source dev='/dev/HostVG/QEMUGuest1'/>
<target dev='hda' bus='ide'/>
</disk>
<disk type='block' device='cdrom'>
<source dev='/dev/HostVG/QEMUGuest2'/>
<target dev='hdc' bus='ide'/>
</disk>
<disk type='file' device='disk'>
<source file='/tmp/data.img'/>
<target dev='vda' bus='virtio'/>
</disk>
<disk type='file' device='disk'>
<source file='/tmp/logs.img'/>
<target dev='vdg' bus='virtio'/>
</disk>
Gets translated to
-drive file=/dev/HostVG/QEMUGuest1,if=ide,index=0,boot=on \
-drive file=/dev/HostVG/QEMUGuest2,if=ide,media=cdrom,index=2 \
-drive file=/tmp/data.img,if=virtio,index=0 \
-drive file=/tmp/logs.img,if=virtio,index=6
Yes, adding the ,index=XXX option is redundant for virtio since it will
ignore it, but it is harmless to define it since we have the data required.
Regards
Daniel.
Index: src/qemu_conf.c
===================================================================
RCS file: /data/cvs/libvirt/src/qemu_conf.c,v
retrieving revision 1.52
diff -u -p -u -p -r1.52 qemu_conf.c
--- src/qemu_conf.c 30 Apr 2008 12:30:55 -0000 1.52
+++ src/qemu_conf.c 6 May 2008 20:42:59 -0000
@@ -49,6 +49,7 @@
#include "buf.h"
#include "conf.h"
#include "util.h"
+#include "verify.h"
#define qemudLog(level, msg...) fprintf(stderr, msg)
@@ -491,6 +492,10 @@ static int qemudExtractVersionInfo(const
*flags |= QEMUD_CMD_FLAG_KQEMU;
if (strstr(help, "-no-reboot"))
*flags |= QEMUD_CMD_FLAG_NO_REBOOT;
+ if (strstr(help, "\n-drive"))
+ *flags |= QEMUD_CMD_FLAG_DRIVE_OPT;
+ if (strstr(help, "boot=on"))
+ *flags |= QEMUD_CMD_FLAG_DRIVE_BOOT_OPT;
if (*version >= 9000)
*flags |= QEMUD_CMD_FLAG_VNC_COLON;
ret = 0;
@@ -568,6 +573,7 @@ static int qemudParseDiskXML(virConnectP
xmlChar *source = NULL;
xmlChar *target = NULL;
xmlChar *type = NULL;
+ xmlChar *bus = NULL;
int typ = 0;
type = xmlGetProp(node, BAD_CAST "type");
@@ -598,6 +604,7 @@ static int qemudParseDiskXML(virConnectP
} 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;
}
@@ -643,10 +650,9 @@ static int qemudParseDiskXML(virConnectP
disk->readonly = 1;
if ((!device || !strcmp((const char *)device, "disk")) &&
- strcmp((const char *)target, "hda") &&
- strcmp((const char *)target, "hdb") &&
- strcmp((const char *)target, "hdc") &&
- strcmp((const char *)target, "hdd")) {
+ strncmp((const char *)target, "hd", 2) &&
+ strncmp((const char *)target, "sd", 2) &&
+ strncmp((const char *)target, "vd", 2)) {
qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
_("Invalid harddisk device name: %s"), target);
goto error;
@@ -673,13 +679,41 @@ static int qemudParseDiskXML(virConnectP
goto error;
}
+ if (!bus) {
+ if (disk->device == QEMUD_DISK_FLOPPY)
+ disk->bus = QEMUD_DISK_BUS_FDC;
+ else
+ disk->bus = QEMUD_DISK_BUS_IDE;
+ } else if (STREQ((const char *)bus, "ide"))
+ disk->bus = QEMUD_DISK_BUS_IDE;
+ else if (STREQ((const char *)bus, "fdc"))
+ disk->bus = QEMUD_DISK_BUS_FDC;
+ else if (STREQ((const char *)bus, "scsi"))
+ disk->bus = QEMUD_DISK_BUS_SCSI;
+ else if (STREQ((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;
+ }
+
+ if (disk->device == QEMUD_DISK_FLOPPY &&
+ disk->bus != QEMUD_DISK_BUS_FDC) {
+ qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
+ _("Invalid bus type '%s' for floppy disk"),
bus);
+ goto error;
+ }
+
xmlFree(device);
xmlFree(target);
xmlFree(source);
+ xmlFree(bus);
return 0;
error:
+ xmlFree(bus);
xmlFree(type);
xmlFree(target);
xmlFree(source);
@@ -1373,6 +1407,24 @@ static int qemudParseInputXML(virConnect
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->bus == b->bus)
+ return virDiskNameToIndex(a->dst) - virDiskNameToIndex(b->dst);
+ else
+ return a->bus - b->bus;
+}
+
+static const char *qemudBusIdToName(int busId, int qemuIF) {
+ const char *busnames[] = { "ide",
+ (qemuIF ? "floppy" : "fdc"),
+ "scsi",
+ "virtio" };
+ verify_true(ARRAY_CARDINALITY(busnames) == QEMUD_DISK_BUS_LAST);
+
+ return busnames[busId];
+}
/*
* Parses a libvirt XML definition of a guest, and populates the
@@ -1762,7 +1814,6 @@ static struct qemud_vm_def *qemudParseXM
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) {
@@ -1775,13 +1826,20 @@ static struct qemud_vm_def *qemudParseXM
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(disk, ptr->next) < 0) {
+ disk->next = ptr->next;
+ ptr->next = disk;
+ break;
+ }
+ ptr = ptr->next;
+ }
}
- prev = disk;
}
}
xmlXPathFreeObject(obj);
@@ -2176,7 +2234,7 @@ int qemudBuildCommandLine(virConnectPtr
snprintf(memory, sizeof(memory), "%lu", vm->def->memory/1024);
snprintf(vcpus, sizeof(vcpus), "%d", vm->def->vcpus);
- if (!(*argv = malloc(sizeof(**argv) * (len+1))))
+ if (!(*argv = calloc(len+1, sizeof(**argv))))
goto no_memory;
if (!((*argv)[++n] = strdup(vm->def->os.binary)))
goto no_memory;
@@ -2274,28 +2332,102 @@ int qemudBuildCommandLine(virConnectPtr
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;
+ /* If QEMU supports -drive param instead of old -hda, -hdb, -cdrom .. */
+ if (vm->qemuCmdFlags & QEMUD_CMD_FLAG_DRIVE_OPT) {
+ int bootCD = 0, bootFloppy = 0, bootDisk = 0;
+
+ /* If QEMU supports boot=on for -drive param... */
+ if (vm->qemuCmdFlags & QEMUD_CMD_FLAG_DRIVE_BOOT_OPT) {
+ for (i = 0 ; i < vm->def->os.nBootDevs ; i++) {
+ switch (vm->def->os.bootDevs[i]) {
+ case QEMUD_BOOT_CDROM:
+ bootCD = 1;
+ break;
+ case QEMUD_BOOT_FLOPPY:
+ bootFloppy = 1;
+ break;
+ case QEMUD_BOOT_DISK:
+ bootDisk = 1;
+ break;
+ }
}
- } 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;
+ while (disk) {
+ char opt[PATH_MAX];
+ const char *media = NULL;
+ int bootable = 0;
+ int idx = virDiskNameToIndex(disk->dst);
+ if (!((*argv)[++n] = strdup("-drive")))
+ goto no_memory;
- disk = disk->next;
+ if (idx < 0) {
+ qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
+ _("unsupported disk type '%s'"),
disk->dst);
+ goto error;
+ }
+
+ if (disk->device == QEMUD_DISK_CDROM)
+ media = "media=cdrom,";
+
+ switch (disk->device) {
+ case QEMUD_DISK_CDROM:
+ bootable = bootCD;
+ bootCD = 0;
+ break;
+ case QEMUD_DISK_FLOPPY:
+ bootable = bootFloppy;
+ bootFloppy = 0;
+ break;
+ case QEMUD_DISK_DISK:
+ bootable = bootDisk;
+ bootDisk = 0;
+ break;
+ }
+
+ snprintf(opt, PATH_MAX, "file=%s,if=%s,%sindex=%d%s",
+ disk->src, qemudBusIdToName(disk->bus, 1),
+ media ? media : "",
+ idx,
+ bootable ? ",boot=on" : "");
+
+ if (!((*argv)[++n] = strdup(opt)))
+ goto no_memory;
+ disk = disk->next;
+ }
+ } else {
+ while (disk) {
+ char dev[NAME_MAX];
+ char file[PATH_MAX];
+
+ if (STREQ(disk->dst, "hdc") &&
+ disk->device == QEMUD_DISK_CDROM) {
+ if (disk->src[0]) {
+ snprintf(dev, NAME_MAX, "-%s", "cdrom");
+ } else {
+ disk = disk->next;
+ continue;
+ }
+ } else {
+ if (STRPREFIX(disk->dst, "hd") ||
+ STRPREFIX(disk->dst, "fd")) {
+ snprintf(dev, NAME_MAX, "-%s", disk->dst);
+ } else {
+ qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
+ _("unsupported disk type '%s'"),
disk->dst);
+ goto error;
+ }
+ }
+
+ 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) {
@@ -2519,6 +2651,7 @@ int qemudBuildCommandLine(virConnectPtr
for (i = 0 ; i < n ; i++)
free((*argv)[i]);
free(*argv);
+ *argv = NULL;
}
return -1;
}
@@ -3373,10 +3506,7 @@ static int qemudGenerateXMLChar(virBuffe
"tcp",
"unix"
};
- /*verify(ARRAY_CARDINALITY(types) == QEMUD_CHR_SRC_TYPE_LAST);*/
-
- if (dev->srcType < 0 || dev->srcType >= QEMUD_CHR_SRC_TYPE_LAST)
- return -1;
+ verify_true(ARRAY_CARDINALITY(types) == QEMUD_CHR_SRC_TYPE_LAST);
/* Compat with legacy <console tty='/dev/pts/5'/> syntax */
if (STREQ(type, "console") &&
@@ -3587,7 +3717,8 @@ char *qemudGenerateXML(virConnectPtr con
virBufferVSprintf(&buf, " <source
%s='%s'/>\n",
typeAttrs[disk->type], disk->src);
- virBufferVSprintf(&buf, " <target dev='%s'/>\n",
disk->dst);
+ virBufferVSprintf(&buf, " <target dev='%s'
bus='%s'/>\n",
+ disk->dst, qemudBusIdToName(disk->bus, 0));
if (disk->readonly)
virBufferAddLit(&buf, " <readonly/>\n");
Index: src/qemu_conf.h
===================================================================
RCS file: /data/cvs/libvirt/src/qemu_conf.h,v
retrieving revision 1.24
diff -u -p -u -p -r1.24 qemu_conf.h
--- src/qemu_conf.h 30 Apr 2008 12:30:55 -0000 1.24
+++ src/qemu_conf.h 6 May 2008 20:42:59 -0000
@@ -56,10 +56,20 @@ enum qemud_vm_disk_device {
QEMUD_DISK_FLOPPY,
};
+enum qemud_vm_disk_bus {
+ QEMUD_DISK_BUS_IDE,
+ QEMUD_DISK_BUS_FDC,
+ QEMUD_DISK_BUS_SCSI,
+ QEMUD_DISK_BUS_VIRTIO,
+
+ QEMUD_DISK_BUS_LAST
+};
+
/* 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;
@@ -220,9 +230,11 @@ enum qemud_vm_graphics_type {
/* Internal flags to keep track of qemu command line capabilities */
enum qemud_cmd_flags {
- QEMUD_CMD_FLAG_KQEMU = 1,
- QEMUD_CMD_FLAG_VNC_COLON = 2,
- QEMUD_CMD_FLAG_NO_REBOOT = 4,
+ QEMUD_CMD_FLAG_KQEMU = (1 << 0),
+ QEMUD_CMD_FLAG_VNC_COLON = (1 << 1),
+ QEMUD_CMD_FLAG_NO_REBOOT = (1 << 2),
+ QEMUD_CMD_FLAG_DRIVE_OPT = (1 << 3),
+ QEMUD_CMD_FLAG_DRIVE_BOOT_OPT = (1 << 4),
};
Index: src/util.c
===================================================================
RCS file: /data/cvs/libvirt/src/util.c,v
retrieving revision 1.33
diff -u -p -u -p -r1.33 util.c
--- src/util.c 25 Apr 2008 14:53:05 -0000 1.33
+++ src/util.c 6 May 2008 20:42:59 -0000
@@ -771,3 +771,44 @@ virParseMacAddr(const char* str, unsigne
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 -1 on failure
+ */
+int virDiskNameToIndex(const char *name) {
+ const char *ptr = NULL;
+ int idx = 0;
+
+ if (strlen(name) < 3)
+ return -1;
+
+ switch (*name) {
+ case 'f':
+ case 'h':
+ case 'v':
+ case 's':
+ break;
+ default:
+ return 0;
+ }
+
+ if (*(name + 1) != 'd')
+ return -1;
+
+ ptr = name+2;
+
+ while (*ptr) {
+ idx = idx * 26;
+
+ if ('a' > *ptr || 'z' < *ptr)
+ return -1;
+
+ idx += *ptr - 'a';
+ ptr++;
+ }
+
+ return idx;
+}
+
Index: src/util.h
===================================================================
RCS file: /data/cvs/libvirt/src/util.h,v
retrieving revision 1.16
diff -u -p -u -p -r1.16 util.h
--- src/util.h 25 Apr 2008 14:53:05 -0000 1.16
+++ src/util.h 6 May 2008 20:42:59 -0000
@@ -92,4 +92,6 @@ int virParseNumber(const char **str);
int virParseMacAddr(const char* str, unsigned char *addr);
+int virDiskNameToIndex(const char* str);
+
#endif /* __VIR_UTIL_H__ */
Index: tests/qemuxml2argvtest.c
===================================================================
RCS file: /data/cvs/libvirt/tests/qemuxml2argvtest.c,v
retrieving revision 1.16
diff -u -p -u -p -r1.16 qemuxml2argvtest.c
--- tests/qemuxml2argvtest.c 30 Apr 2008 12:30:55 -0000 1.16
+++ tests/qemuxml2argvtest.c 6 May 2008 20:43:06 -0000
@@ -20,7 +20,7 @@ static struct qemud_driver driver;
#define MAX_FILE 4096
-static int testCompareXMLToArgvFiles(const char *xml, const char *cmd) {
+static int testCompareXMLToArgvFiles(const char *xml, const char *cmd, int driveFlag) {
char xmlData[MAX_FILE];
char argvData[MAX_FILE];
char *xmlPtr = &(xmlData[0]);
@@ -47,6 +47,10 @@ static int testCompareXMLToArgvFiles(con
vm.qemuVersion = 0 * 1000 * 100 + (8 * 1000) + 1;
vm.qemuCmdFlags = QEMUD_CMD_FLAG_VNC_COLON |
QEMUD_CMD_FLAG_NO_REBOOT;
+ if (driveFlag) {
+ vm.qemuCmdFlags |= QEMUD_CMD_FLAG_DRIVE_OPT;
+ vm.qemuCmdFlags |= QEMUD_CMD_FLAG_DRIVE_BOOT_OPT;
+ }
vm.migrateFrom[0] = '\0';
vmdef->vncActivePort = vmdef->vncPort;
@@ -94,14 +98,20 @@ static int testCompareXMLToArgvFiles(con
}
+struct testInfo {
+ const char *name;
+ int driveFlag;
+};
+
static int testCompareXMLToArgvHelper(const void *data) {
+ const struct testInfo *info = data;
char xml[PATH_MAX];
char args[PATH_MAX];
snprintf(xml, PATH_MAX, "%s/qemuxml2argvdata/qemuxml2argv-%s.xml",
- abs_srcdir, (const char*)data);
+ abs_srcdir, info->name);
snprintf(args, PATH_MAX, "%s/qemuxml2argvdata/qemuxml2argv-%s.args",
- abs_srcdir, (const char*)data);
- return testCompareXMLToArgvFiles(xml, args);
+ abs_srcdir, info->name);
+ return testCompareXMLToArgvFiles(xml, args, info->driveFlag);
}
@@ -125,40 +135,44 @@ main(int argc, char **argv)
driver.caps = qemudCapsInit();
-#define DO_TEST(name) \
- if (virtTestRun("QEMU XML-2-ARGV " name, \
- 1, testCompareXMLToArgvHelper, (name)) < 0) \
- ret = -1
-
- DO_TEST("minimal");
- DO_TEST("boot-cdrom");
- DO_TEST("boot-network");
- DO_TEST("boot-floppy");
- DO_TEST("clock-utc");
- DO_TEST("clock-localtime");
- DO_TEST("disk-cdrom");
- DO_TEST("disk-floppy");
- DO_TEST("disk-many");
- DO_TEST("graphics-vnc");
- DO_TEST("graphics-sdl");
- DO_TEST("input-usbmouse");
- DO_TEST("input-usbtablet");
- DO_TEST("misc-acpi");
- DO_TEST("misc-no-reboot");
- DO_TEST("net-user");
- DO_TEST("net-virtio");
-
- DO_TEST("serial-vc");
- DO_TEST("serial-pty");
- DO_TEST("serial-dev");
- DO_TEST("serial-file");
- DO_TEST("serial-unix");
- DO_TEST("serial-tcp");
- DO_TEST("serial-udp");
- DO_TEST("serial-tcp-telnet");
- DO_TEST("serial-many");
- DO_TEST("parallel-tcp");
- DO_TEST("console-compat");
+#define DO_TEST(name, driveFlag) \
+ do { \
+ struct testInfo info = { name, driveFlag }; \
+ if (virtTestRun("QEMU XML-2-ARGV " name, \
+ 1, testCompareXMLToArgvHelper, &info) < 0) \
+ ret = -1; \
+ } while (0)
+
+ DO_TEST("minimal", 0);
+ DO_TEST("boot-cdrom", 0);
+ DO_TEST("boot-network", 0);
+ DO_TEST("boot-floppy", 0);
+ DO_TEST("clock-utc", 0);
+ DO_TEST("clock-localtime", 0);
+ DO_TEST("disk-cdrom", 0);
+ DO_TEST("disk-floppy", 0);
+ DO_TEST("disk-many", 0);
+ DO_TEST("disk-virtio", 1);
+ DO_TEST("graphics-vnc", 0);
+ DO_TEST("graphics-sdl", 0);
+ DO_TEST("input-usbmouse", 0);
+ DO_TEST("input-usbtablet", 0);
+ DO_TEST("misc-acpi", 0);
+ DO_TEST("misc-no-reboot", 0);
+ DO_TEST("net-user", 0);
+ DO_TEST("net-virtio", 0);
+
+ DO_TEST("serial-vc", 0);
+ DO_TEST("serial-pty", 0);
+ DO_TEST("serial-dev", 0);
+ DO_TEST("serial-file", 0);
+ DO_TEST("serial-unix", 0);
+ DO_TEST("serial-tcp", 0);
+ DO_TEST("serial-udp", 0);
+ DO_TEST("serial-tcp-telnet", 0);
+ DO_TEST("serial-many", 0);
+ DO_TEST("parallel-tcp", 0);
+ DO_TEST("console-compat", 0);
virCapabilitiesFree(driver.caps);
Index: tests/qemuxml2argvdata/qemuxml2argv-boot-cdrom.xml
===================================================================
RCS file: /data/cvs/libvirt/tests/qemuxml2argvdata/qemuxml2argv-boot-cdrom.xml,v
retrieving revision 1.1
diff -u -p -u -p -r1.1 qemuxml2argv-boot-cdrom.xml
--- tests/qemuxml2argvdata/qemuxml2argv-boot-cdrom.xml 18 Jul 2007 21:34:22 -0000 1.1
+++ tests/qemuxml2argvdata/qemuxml2argv-boot-cdrom.xml 6 May 2008 20:43:06 -0000
@@ -16,7 +16,7 @@
<emulator>/usr/bin/qemu</emulator>
<disk type='block' device='cdrom'>
<source dev='/dev/cdrom'/>
- <target dev='hdc'/>
+ <target dev='hdc' bus='ide'/>
<readonly/>
</disk>
</devices>
Index: tests/qemuxml2argvdata/qemuxml2argv-boot-floppy.xml
===================================================================
RCS file: /data/cvs/libvirt/tests/qemuxml2argvdata/qemuxml2argv-boot-floppy.xml,v
retrieving revision 1.1
diff -u -p -u -p -r1.1 qemuxml2argv-boot-floppy.xml
--- tests/qemuxml2argvdata/qemuxml2argv-boot-floppy.xml 18 Jul 2007 21:34:22 -0000 1.1
+++ tests/qemuxml2argvdata/qemuxml2argv-boot-floppy.xml 6 May 2008 20:43:06 -0000
@@ -16,11 +16,11 @@
<emulator>/usr/bin/qemu</emulator>
<disk type='block' device='disk'>
<source dev='/dev/HostVG/QEMUGuest1'/>
- <target dev='hda'/>
+ <target dev='hda' bus='ide'/>
</disk>
<disk type='file' device='floppy'>
<source file='/tmp/firmware.img'/>
- <target dev='fda'/>
+ <target dev='fda' bus='fdc'/>
</disk>
</devices>
</domain>
Index: tests/qemuxml2argvdata/qemuxml2argv-boot-network.xml
===================================================================
RCS file: /data/cvs/libvirt/tests/qemuxml2argvdata/qemuxml2argv-boot-network.xml,v
retrieving revision 1.2
diff -u -p -u -p -r1.2 qemuxml2argv-boot-network.xml
--- tests/qemuxml2argvdata/qemuxml2argv-boot-network.xml 13 Sep 2007 22:06:54 -0000 1.2
+++ tests/qemuxml2argvdata/qemuxml2argv-boot-network.xml 6 May 2008 20:43:06 -0000
@@ -16,7 +16,7 @@
<emulator>/usr/bin/qemu</emulator>
<disk type='block' device='disk'>
<source dev='/dev/HostVG/QEMUGuest1'/>
- <target dev='hda'/>
+ <target dev='hda' bus='ide'/>
</disk>
</devices>
</domain>
Index: tests/qemuxml2argvdata/qemuxml2argv-clock-localtime.xml
===================================================================
RCS file: /data/cvs/libvirt/tests/qemuxml2argvdata/qemuxml2argv-clock-localtime.xml,v
retrieving revision 1.1
diff -u -p -u -p -r1.1 qemuxml2argv-clock-localtime.xml
--- tests/qemuxml2argvdata/qemuxml2argv-clock-localtime.xml 18 Jul 2007 21:34:22
-0000 1.1
+++ tests/qemuxml2argvdata/qemuxml2argv-clock-localtime.xml 6 May 2008 20:43:06 -0000
@@ -16,7 +16,7 @@
<emulator>/usr/bin/qemu</emulator>
<disk type='block' device='disk'>
<source dev='/dev/HostVG/QEMUGuest1'/>
- <target dev='hda'/>
+ <target dev='hda' bus='ide'/>
</disk>
</devices>
</domain>
Index: tests/qemuxml2argvdata/qemuxml2argv-clock-utc.xml
===================================================================
RCS file: /data/cvs/libvirt/tests/qemuxml2argvdata/qemuxml2argv-clock-utc.xml,v
retrieving revision 1.1
diff -u -p -u -p -r1.1 qemuxml2argv-clock-utc.xml
--- tests/qemuxml2argvdata/qemuxml2argv-clock-utc.xml 18 Jul 2007 21:34:22 -0000 1.1
+++ tests/qemuxml2argvdata/qemuxml2argv-clock-utc.xml 6 May 2008 20:43:06 -0000
@@ -16,7 +16,7 @@
<emulator>/usr/bin/qemu</emulator>
<disk type='block' device='disk'>
<source dev='/dev/HostVG/QEMUGuest1'/>
- <target dev='hda'/>
+ <target dev='hda' bus='ide'/>
</disk>
</devices>
</domain>
Index: tests/qemuxml2argvdata/qemuxml2argv-console-compat.xml
===================================================================
RCS file: /data/cvs/libvirt/tests/qemuxml2argvdata/qemuxml2argv-console-compat.xml,v
retrieving revision 1.1
diff -u -p -u -p -r1.1 qemuxml2argv-console-compat.xml
--- tests/qemuxml2argvdata/qemuxml2argv-console-compat.xml 25 Apr 2008 20:46:13 -0000 1.1
+++ tests/qemuxml2argvdata/qemuxml2argv-console-compat.xml 6 May 2008 20:43:06 -0000
@@ -16,7 +16,7 @@
<emulator>/usr/bin/qemu</emulator>
<disk type='block' device='disk'>
<source dev='/dev/HostVG/QEMUGuest1'/>
- <target dev='hda'/>
+ <target dev='hda' bus='ide'/>
</disk>
<serial type='pty'>
<target port='0'/>
Index: tests/qemuxml2argvdata/qemuxml2argv-disk-cdrom.xml
===================================================================
RCS file: /data/cvs/libvirt/tests/qemuxml2argvdata/qemuxml2argv-disk-cdrom.xml,v
retrieving revision 1.1
diff -u -p -u -p -r1.1 qemuxml2argv-disk-cdrom.xml
--- tests/qemuxml2argvdata/qemuxml2argv-disk-cdrom.xml 18 Jul 2007 21:34:22 -0000 1.1
+++ tests/qemuxml2argvdata/qemuxml2argv-disk-cdrom.xml 6 May 2008 20:43:06 -0000
@@ -16,11 +16,11 @@
<emulator>/usr/bin/qemu</emulator>
<disk type='block' device='disk'>
<source dev='/dev/HostVG/QEMUGuest1'/>
- <target dev='hda'/>
+ <target dev='hda' bus='ide'/>
</disk>
<disk type='file' device='cdrom'>
<source file='/root/boot.iso'/>
- <target dev='hdc'/>
+ <target dev='hdc' bus='ide'/>
<readonly/>
</disk>
</devices>
Index: tests/qemuxml2argvdata/qemuxml2argv-disk-floppy.xml
===================================================================
RCS file: /data/cvs/libvirt/tests/qemuxml2argvdata/qemuxml2argv-disk-floppy.xml,v
retrieving revision 1.1
diff -u -p -u -p -r1.1 qemuxml2argv-disk-floppy.xml
--- tests/qemuxml2argvdata/qemuxml2argv-disk-floppy.xml 18 Jul 2007 21:34:22 -0000 1.1
+++ tests/qemuxml2argvdata/qemuxml2argv-disk-floppy.xml 6 May 2008 20:43:06 -0000
@@ -16,15 +16,15 @@
<emulator>/usr/bin/qemu</emulator>
<disk type='block' device='disk'>
<source dev='/dev/HostVG/QEMUGuest1'/>
- <target dev='hda'/>
+ <target dev='hda' bus='ide'/>
</disk>
<disk type='block' device='floppy'>
<source dev='/dev/fd0'/>
- <target dev='fda'/>
+ <target dev='fda' bus='fdc'/>
</disk>
<disk type='file' device='floppy'>
<source file='/tmp/firmware.img'/>
- <target dev='fdb'/>
+ <target dev='fdb' bus='fdc'/>
</disk>
</devices>
</domain>
Index: tests/qemuxml2argvdata/qemuxml2argv-disk-many.xml
===================================================================
RCS file: /data/cvs/libvirt/tests/qemuxml2argvdata/qemuxml2argv-disk-many.xml,v
retrieving revision 1.1
diff -u -p -u -p -r1.1 qemuxml2argv-disk-many.xml
--- tests/qemuxml2argvdata/qemuxml2argv-disk-many.xml 18 Jul 2007 21:34:22 -0000 1.1
+++ tests/qemuxml2argvdata/qemuxml2argv-disk-many.xml 6 May 2008 20:43:06 -0000
@@ -16,19 +16,19 @@
<emulator>/usr/bin/qemu</emulator>
<disk type='block' device='disk'>
<source dev='/dev/HostVG/QEMUGuest1'/>
- <target dev='hda'/>
+ <target dev='hda' bus='ide'/>
</disk>
<disk type='block' device='disk'>
<source dev='/dev/HostVG/QEMUGuest2'/>
- <target dev='hdb'/>
+ <target dev='hdb' bus='ide'/>
</disk>
<disk type='file' device='disk'>
<source file='/tmp/data.img'/>
- <target dev='hdc'/>
+ <target dev='hdc' bus='ide'/>
</disk>
<disk type='file' device='disk'>
<source file='/tmp/logs.img'/>
- <target dev='hdd'/>
+ <target dev='hdd' bus='ide'/>
</disk>
</devices>
</domain>
Index: tests/qemuxml2argvdata/qemuxml2argv-disk-virtio.args
===================================================================
RCS file: tests/qemuxml2argvdata/qemuxml2argv-disk-virtio.args
diff -N tests/qemuxml2argvdata/qemuxml2argv-disk-virtio.args
--- /dev/null 1 Jan 1970 00:00:00 -0000
+++ tests/qemuxml2argvdata/qemuxml2argv-disk-virtio.args 6 May 2008 20:43:06 -0000
@@ -0,0 +1 @@
+/usr/bin/qemu -M pc -m 214 -smp 1 -nographic -monitor pty -no-acpi -boot c -drive
file=/dev/HostVG/QEMUGuest1,if=ide,index=0,boot=on -drive
file=/dev/HostVG/QEMUGuest2,if=ide,media=cdrom,index=2 -drive
file=/tmp/data.img,if=virtio,index=0 -drive file=/tmp/logs.img,if=virtio,index=6 -net none
-serial none -parallel none -usb
\ No newline at end of file
Index: tests/qemuxml2argvdata/qemuxml2argv-disk-virtio.xml
===================================================================
RCS file: tests/qemuxml2argvdata/qemuxml2argv-disk-virtio.xml
diff -N tests/qemuxml2argvdata/qemuxml2argv-disk-virtio.xml
--- /dev/null 1 Jan 1970 00:00:00 -0000
+++ tests/qemuxml2argvdata/qemuxml2argv-disk-virtio.xml 6 May 2008 20:43:06 -0000
@@ -0,0 +1,34 @@
+<domain type='qemu'>
+ <name>QEMUGuest1</name>
+ <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>
+ <memory>219200</memory>
+ <currentMemory>219200</currentMemory>
+ <vcpu>1</vcpu>
+ <os>
+ <type arch='i686' machine='pc'>hvm</type>
+ <boot dev='hd'/>
+ </os>
+ <clock offset='utc'/>
+ <on_poweroff>destroy</on_poweroff>
+ <on_reboot>restart</on_reboot>
+ <on_crash>destroy</on_crash>
+ <devices>
+ <emulator>/usr/bin/qemu</emulator>
+ <disk type='block' device='disk'>
+ <source dev='/dev/HostVG/QEMUGuest1'/>
+ <target dev='hda' bus='ide'/>
+ </disk>
+ <disk type='block' device='cdrom'>
+ <source dev='/dev/HostVG/QEMUGuest2'/>
+ <target dev='hdc' bus='ide'/>
+ </disk>
+ <disk type='file' device='disk'>
+ <source file='/tmp/data.img'/>
+ <target dev='vda' bus='virtio'/>
+ </disk>
+ <disk type='file' device='disk'>
+ <source file='/tmp/logs.img'/>
+ <target dev='vdg' bus='virtio'/>
+ </disk>
+ </devices>
+</domain>
Index: tests/qemuxml2argvdata/qemuxml2argv-graphics-sdl.xml
===================================================================
RCS file: /data/cvs/libvirt/tests/qemuxml2argvdata/qemuxml2argv-graphics-sdl.xml,v
retrieving revision 1.1
diff -u -p -u -p -r1.1 qemuxml2argv-graphics-sdl.xml
--- tests/qemuxml2argvdata/qemuxml2argv-graphics-sdl.xml 18 Jul 2007 21:34:22 -0000 1.1
+++ tests/qemuxml2argvdata/qemuxml2argv-graphics-sdl.xml 6 May 2008 20:43:06 -0000
@@ -16,7 +16,7 @@
<emulator>/usr/bin/qemu</emulator>
<disk type='block' device='disk'>
<source dev='/dev/HostVG/QEMUGuest1'/>
- <target dev='hda'/>
+ <target dev='hda' bus='ide'/>
</disk>
<input type='mouse' bus='ps2'/>
<graphics type='sdl'/>
Index: tests/qemuxml2argvdata/qemuxml2argv-graphics-vnc.xml
===================================================================
RCS file: /data/cvs/libvirt/tests/qemuxml2argvdata/qemuxml2argv-graphics-vnc.xml,v
retrieving revision 1.1
diff -u -p -u -p -r1.1 qemuxml2argv-graphics-vnc.xml
--- tests/qemuxml2argvdata/qemuxml2argv-graphics-vnc.xml 18 Jul 2007 21:34:22 -0000 1.1
+++ tests/qemuxml2argvdata/qemuxml2argv-graphics-vnc.xml 6 May 2008 20:43:06 -0000
@@ -16,7 +16,7 @@
<emulator>/usr/bin/qemu</emulator>
<disk type='block' device='disk'>
<source dev='/dev/HostVG/QEMUGuest1'/>
- <target dev='hda'/>
+ <target dev='hda' bus='ide'/>
</disk>
<input type='mouse' bus='ps2'/>
<graphics type='vnc' port='5903' listen='127.0.0.1'/>
Index: tests/qemuxml2argvdata/qemuxml2argv-input-usbmouse.xml
===================================================================
RCS file: /data/cvs/libvirt/tests/qemuxml2argvdata/qemuxml2argv-input-usbmouse.xml,v
retrieving revision 1.1
diff -u -p -u -p -r1.1 qemuxml2argv-input-usbmouse.xml
--- tests/qemuxml2argvdata/qemuxml2argv-input-usbmouse.xml 18 Jul 2007 21:34:22 -0000 1.1
+++ tests/qemuxml2argvdata/qemuxml2argv-input-usbmouse.xml 6 May 2008 20:43:06 -0000
@@ -16,7 +16,7 @@
<emulator>/usr/bin/qemu</emulator>
<disk type='block' device='disk'>
<source dev='/dev/HostVG/QEMUGuest1'/>
- <target dev='hda'/>
+ <target dev='hda' bus='ide'/>
</disk>
<input type='mouse' bus='usb'/>
</devices>
Index: tests/qemuxml2argvdata/qemuxml2argv-input-usbtablet.xml
===================================================================
RCS file: /data/cvs/libvirt/tests/qemuxml2argvdata/qemuxml2argv-input-usbtablet.xml,v
retrieving revision 1.1
diff -u -p -u -p -r1.1 qemuxml2argv-input-usbtablet.xml
--- tests/qemuxml2argvdata/qemuxml2argv-input-usbtablet.xml 18 Jul 2007 21:34:22
-0000 1.1
+++ tests/qemuxml2argvdata/qemuxml2argv-input-usbtablet.xml 6 May 2008 20:43:06 -0000
@@ -16,7 +16,7 @@
<emulator>/usr/bin/qemu</emulator>
<disk type='block' device='disk'>
<source dev='/dev/HostVG/QEMUGuest1'/>
- <target dev='hda'/>
+ <target dev='hda' bus='ide'/>
</disk>
<input type='tablet' bus='usb'/>
</devices>
Index: tests/qemuxml2argvdata/qemuxml2argv-minimal.xml
===================================================================
RCS file: /data/cvs/libvirt/tests/qemuxml2argvdata/qemuxml2argv-minimal.xml,v
retrieving revision 1.1
diff -u -p -u -p -r1.1 qemuxml2argv-minimal.xml
--- tests/qemuxml2argvdata/qemuxml2argv-minimal.xml 18 Jul 2007 21:34:22 -0000 1.1
+++ tests/qemuxml2argvdata/qemuxml2argv-minimal.xml 6 May 2008 20:43:06 -0000
@@ -16,7 +16,7 @@
<emulator>/usr/bin/qemu</emulator>
<disk type='block' device='disk'>
<source dev='/dev/HostVG/QEMUGuest1'/>
- <target dev='hda'/>
+ <target dev='hda' bus='ide'/>
</disk>
</devices>
</domain>
Index: tests/qemuxml2argvdata/qemuxml2argv-misc-acpi.xml
===================================================================
RCS file: /data/cvs/libvirt/tests/qemuxml2argvdata/qemuxml2argv-misc-acpi.xml,v
retrieving revision 1.1
diff -u -p -u -p -r1.1 qemuxml2argv-misc-acpi.xml
--- tests/qemuxml2argvdata/qemuxml2argv-misc-acpi.xml 18 Jul 2007 21:34:22 -0000 1.1
+++ tests/qemuxml2argvdata/qemuxml2argv-misc-acpi.xml 6 May 2008 20:43:06 -0000
@@ -19,7 +19,7 @@
<emulator>/usr/bin/qemu</emulator>
<disk type='block' device='disk'>
<source dev='/dev/HostVG/QEMUGuest1'/>
- <target dev='hda'/>
+ <target dev='hda' bus='ide'/>
</disk>
</devices>
</domain>
Index: tests/qemuxml2argvdata/qemuxml2argv-misc-no-reboot.xml
===================================================================
RCS file: /data/cvs/libvirt/tests/qemuxml2argvdata/qemuxml2argv-misc-no-reboot.xml,v
retrieving revision 1.1
diff -u -p -u -p -r1.1 qemuxml2argv-misc-no-reboot.xml
--- tests/qemuxml2argvdata/qemuxml2argv-misc-no-reboot.xml 18 Jul 2007 21:34:22 -0000 1.1
+++ tests/qemuxml2argvdata/qemuxml2argv-misc-no-reboot.xml 6 May 2008 20:43:06 -0000
@@ -16,7 +16,7 @@
<emulator>/usr/bin/qemu</emulator>
<disk type='block' device='disk'>
<source dev='/dev/HostVG/QEMUGuest1'/>
- <target dev='hda'/>
+ <target dev='hda' bus='ide'/>
</disk>
</devices>
</domain>
Index: tests/qemuxml2argvdata/qemuxml2argv-net-user.xml
===================================================================
RCS file: /data/cvs/libvirt/tests/qemuxml2argvdata/qemuxml2argv-net-user.xml,v
retrieving revision 1.1
diff -u -p -u -p -r1.1 qemuxml2argv-net-user.xml
--- tests/qemuxml2argvdata/qemuxml2argv-net-user.xml 18 Jul 2007 21:34:22 -0000 1.1
+++ tests/qemuxml2argvdata/qemuxml2argv-net-user.xml 6 May 2008 20:43:06 -0000
@@ -16,7 +16,7 @@
<emulator>/usr/bin/qemu</emulator>
<disk type='block' device='disk'>
<source dev='/dev/HostVG/QEMUGuest1'/>
- <target dev='hda'/>
+ <target dev='hda' bus='ide'/>
</disk>
<interface type='user'>
<mac address='00:11:22:33:44:55'/>
Index: tests/qemuxml2argvdata/qemuxml2argv-net-virtio.xml
===================================================================
RCS file: /data/cvs/libvirt/tests/qemuxml2argvdata/qemuxml2argv-net-virtio.xml,v
retrieving revision 1.1
diff -u -p -u -p -r1.1 qemuxml2argv-net-virtio.xml
--- tests/qemuxml2argvdata/qemuxml2argv-net-virtio.xml 30 Apr 2008 12:30:55 -0000 1.1
+++ tests/qemuxml2argvdata/qemuxml2argv-net-virtio.xml 6 May 2008 20:43:06 -0000
@@ -16,7 +16,7 @@
<emulator>/usr/bin/qemu</emulator>
<disk type='block' device='disk'>
<source dev='/dev/HostVG/QEMUGuest1'/>
- <target dev='hda'/>
+ <target dev='hda' bus='ide'/>
</disk>
<interface type='user'>
<mac address='00:11:22:33:44:55'/>
Index: tests/qemuxml2argvdata/qemuxml2argv-parallel-tcp.xml
===================================================================
RCS file: /data/cvs/libvirt/tests/qemuxml2argvdata/qemuxml2argv-parallel-tcp.xml,v
retrieving revision 1.1
diff -u -p -u -p -r1.1 qemuxml2argv-parallel-tcp.xml
--- tests/qemuxml2argvdata/qemuxml2argv-parallel-tcp.xml 25 Apr 2008 20:46:13 -0000 1.1
+++ tests/qemuxml2argvdata/qemuxml2argv-parallel-tcp.xml 6 May 2008 20:43:06 -0000
@@ -16,7 +16,7 @@
<emulator>/usr/bin/qemu</emulator>
<disk type='block' device='disk'>
<source dev='/dev/HostVG/QEMUGuest1'/>
- <target dev='hda'/>
+ <target dev='hda' bus='ide'/>
</disk>
<parallel type='tcp'>
<source mode='bind' host='127.0.0.1'
service='9999'/>
Index: tests/qemuxml2argvdata/qemuxml2argv-serial-dev.xml
===================================================================
RCS file: /data/cvs/libvirt/tests/qemuxml2argvdata/qemuxml2argv-serial-dev.xml,v
retrieving revision 1.1
diff -u -p -u -p -r1.1 qemuxml2argv-serial-dev.xml
--- tests/qemuxml2argvdata/qemuxml2argv-serial-dev.xml 25 Apr 2008 20:46:13 -0000 1.1
+++ tests/qemuxml2argvdata/qemuxml2argv-serial-dev.xml 6 May 2008 20:43:06 -0000
@@ -16,7 +16,7 @@
<emulator>/usr/bin/qemu</emulator>
<disk type='block' device='disk'>
<source dev='/dev/HostVG/QEMUGuest1'/>
- <target dev='hda'/>
+ <target dev='hda' bus='ide'/>
</disk>
<serial type='dev'>
<source path='/dev/ttyS2'/>
Index: tests/qemuxml2argvdata/qemuxml2argv-serial-file.xml
===================================================================
RCS file: /data/cvs/libvirt/tests/qemuxml2argvdata/qemuxml2argv-serial-file.xml,v
retrieving revision 1.1
diff -u -p -u -p -r1.1 qemuxml2argv-serial-file.xml
--- tests/qemuxml2argvdata/qemuxml2argv-serial-file.xml 25 Apr 2008 20:46:13 -0000 1.1
+++ tests/qemuxml2argvdata/qemuxml2argv-serial-file.xml 6 May 2008 20:43:06 -0000
@@ -16,7 +16,7 @@
<emulator>/usr/bin/qemu</emulator>
<disk type='block' device='disk'>
<source dev='/dev/HostVG/QEMUGuest1'/>
- <target dev='hda'/>
+ <target dev='hda' bus='ide'/>
</disk>
<serial type='file'>
<source path='/tmp/serial.log'/>
Index: tests/qemuxml2argvdata/qemuxml2argv-serial-many.xml
===================================================================
RCS file: /data/cvs/libvirt/tests/qemuxml2argvdata/qemuxml2argv-serial-many.xml,v
retrieving revision 1.1
diff -u -p -u -p -r1.1 qemuxml2argv-serial-many.xml
--- tests/qemuxml2argvdata/qemuxml2argv-serial-many.xml 25 Apr 2008 20:46:13 -0000 1.1
+++ tests/qemuxml2argvdata/qemuxml2argv-serial-many.xml 6 May 2008 20:43:06 -0000
@@ -16,7 +16,7 @@
<emulator>/usr/bin/qemu</emulator>
<disk type='block' device='disk'>
<source dev='/dev/HostVG/QEMUGuest1'/>
- <target dev='hda'/>
+ <target dev='hda' bus='ide'/>
</disk>
<serial type='pty'>
<target port='0'/>
Index: tests/qemuxml2argvdata/qemuxml2argv-serial-pty.xml
===================================================================
RCS file: /data/cvs/libvirt/tests/qemuxml2argvdata/qemuxml2argv-serial-pty.xml,v
retrieving revision 1.1
diff -u -p -u -p -r1.1 qemuxml2argv-serial-pty.xml
--- tests/qemuxml2argvdata/qemuxml2argv-serial-pty.xml 25 Apr 2008 20:46:13 -0000 1.1
+++ tests/qemuxml2argvdata/qemuxml2argv-serial-pty.xml 6 May 2008 20:43:06 -0000
@@ -16,7 +16,7 @@
<emulator>/usr/bin/qemu</emulator>
<disk type='block' device='disk'>
<source dev='/dev/HostVG/QEMUGuest1'/>
- <target dev='hda'/>
+ <target dev='hda' bus='ide'/>
</disk>
<serial type='pty'>
<target port='0'/>
Index: tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-telnet.xml
===================================================================
RCS file: /data/cvs/libvirt/tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-telnet.xml,v
retrieving revision 1.1
diff -u -p -u -p -r1.1 qemuxml2argv-serial-tcp-telnet.xml
--- tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-telnet.xml 25 Apr 2008 20:46:13
-0000 1.1
+++ tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-telnet.xml 6 May 2008 20:43:06 -0000
@@ -16,7 +16,7 @@
<emulator>/usr/bin/qemu</emulator>
<disk type='block' device='disk'>
<source dev='/dev/HostVG/QEMUGuest1'/>
- <target dev='hda'/>
+ <target dev='hda' bus='ide'/>
</disk>
<serial type='tcp'>
<source mode='bind' host='127.0.0.1'
service='9999'/>
Index: tests/qemuxml2argvdata/qemuxml2argv-serial-tcp.xml
===================================================================
RCS file: /data/cvs/libvirt/tests/qemuxml2argvdata/qemuxml2argv-serial-tcp.xml,v
retrieving revision 1.1
diff -u -p -u -p -r1.1 qemuxml2argv-serial-tcp.xml
--- tests/qemuxml2argvdata/qemuxml2argv-serial-tcp.xml 25 Apr 2008 20:46:13 -0000 1.1
+++ tests/qemuxml2argvdata/qemuxml2argv-serial-tcp.xml 6 May 2008 20:43:06 -0000
@@ -16,7 +16,7 @@
<emulator>/usr/bin/qemu</emulator>
<disk type='block' device='disk'>
<source dev='/dev/HostVG/QEMUGuest1'/>
- <target dev='hda'/>
+ <target dev='hda' bus='ide'/>
</disk>
<serial type='tcp'>
<source mode='connect' host='127.0.0.1'
service='9999'/>
Index: tests/qemuxml2argvdata/qemuxml2argv-serial-udp.xml
===================================================================
RCS file: /data/cvs/libvirt/tests/qemuxml2argvdata/qemuxml2argv-serial-udp.xml,v
retrieving revision 1.1
diff -u -p -u -p -r1.1 qemuxml2argv-serial-udp.xml
--- tests/qemuxml2argvdata/qemuxml2argv-serial-udp.xml 25 Apr 2008 20:46:13 -0000 1.1
+++ tests/qemuxml2argvdata/qemuxml2argv-serial-udp.xml 6 May 2008 20:43:06 -0000
@@ -16,7 +16,7 @@
<emulator>/usr/bin/qemu</emulator>
<disk type='block' device='disk'>
<source dev='/dev/HostVG/QEMUGuest1'/>
- <target dev='hda'/>
+ <target dev='hda' bus='ide'/>
</disk>
<serial type='udp'>
<source mode='bind' host='127.0.0.1'
service='9999'/>
Index: tests/qemuxml2argvdata/qemuxml2argv-serial-unix.xml
===================================================================
RCS file: /data/cvs/libvirt/tests/qemuxml2argvdata/qemuxml2argv-serial-unix.xml,v
retrieving revision 1.1
diff -u -p -u -p -r1.1 qemuxml2argv-serial-unix.xml
--- tests/qemuxml2argvdata/qemuxml2argv-serial-unix.xml 25 Apr 2008 20:46:13 -0000 1.1
+++ tests/qemuxml2argvdata/qemuxml2argv-serial-unix.xml 6 May 2008 20:43:06 -0000
@@ -16,7 +16,7 @@
<emulator>/usr/bin/qemu</emulator>
<disk type='block' device='disk'>
<source dev='/dev/HostVG/QEMUGuest1'/>
- <target dev='hda'/>
+ <target dev='hda' bus='ide'/>
</disk>
<serial type='unix'>
<source mode='connect' path='/tmp/serial.sock'/>
Index: tests/qemuxml2argvdata/qemuxml2argv-serial-vc.xml
===================================================================
RCS file: /data/cvs/libvirt/tests/qemuxml2argvdata/qemuxml2argv-serial-vc.xml,v
retrieving revision 1.1
diff -u -p -u -p -r1.1 qemuxml2argv-serial-vc.xml
--- tests/qemuxml2argvdata/qemuxml2argv-serial-vc.xml 25 Apr 2008 20:46:13 -0000 1.1
+++ tests/qemuxml2argvdata/qemuxml2argv-serial-vc.xml 6 May 2008 20:43:06 -0000
@@ -16,7 +16,7 @@
<emulator>/usr/bin/qemu</emulator>
<disk type='block' device='disk'>
<source dev='/dev/HostVG/QEMUGuest1'/>
- <target dev='hda'/>
+ <target dev='hda' bus='ide'/>
</disk>
<serial type='vc'>
<target port='0'/>
--
|: Red Hat, Engineering, Boston -o- http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
2:38 a.m.
New subject: [libvirt] Re: [Libvir] [PATCH] Add bus attribute to disk target definition
On Tue, May 06, 2008 at 10:02:21PM +0100, Daniel P. Berrange wrote:
> sorry for the delay. Here's the newest version of the patch
which
> should address all the issues that have been raised so far.
Yes & no. It didn't address the redundant re-ordering of -drive
parameters when using boot=on,
The reasoning here was (I mentioned this in a previous mail, too) that
when qemu/kvm some day grows the ability to have more than one boot
device specified with boot=on (using extboot or whatever), we're going
to have to change things *anyway*. Ordering the devices according to
boot priority seems like a reasonable guess as to what would be required
to do, so I figured I'd leave it as is.
nor re-add the -boot param to make PXE work again.
Yes and no. In the latest patch I provided I only set use_extboot if
there's only one boot device defined, and it's a virtio device, so PXE
booting would use the old-style "-boot n" syntax. I literallly woke up
this morning and instantly smacked my forehead due to another problem
this introduced, so I'm happy you changed it. :)
One further complication is that QEMU 0.9.1 has -drive support but
not
the extboot support, so boot=on can't be used there. It rather
annoyingly complains
qemu: unknowm parameter 'boot' in
'file=/home/berrange/boot.iso,media=cdrom,boot=on'
Ah, figures.
> + if (!bus)
> + disk->bus = QEMUD_DISK_BUS_IDE;
This was giving floppy disks a bus of 'ide' which isn't technically
correct - they're really their own bus type - I reckon we should call
it 'fdc'.
Ah. Yes, I must admit that floppy disks were completely off my radar.
This double loop is redundant - there's no need to pull bootable
drives to the front of the list - this is why there's an explicit flag
rather than relying on ordering.
I did this for two reasons:
a) I wanted to avoid the bootDisk, bootFloppy, bootCD variables approach
you used. It just didn't appeal to me. *shrug*
b) As I mentioned further up, this was also done in an attempt to match
what would be needed when it becomes possible to specify ",boot=on" for
more than one device, but we can revisit this when that day comes.
Your patch looks fine to me.
Oh, and thanks for doing all the test cases as well. I didn't want to
get started on those until we had agreed on the logic that should be
applied.
--
Soren Hansen |
Virtualisation specialist | Ubuntu Server Team
Canonical Ltd. | http://www.ubuntu.com/
6:45 a.m.
New subject: [libvirt] Re: [Libvir] [PATCH] Add bus attribute to disk target definition
On Wed, May 07, 2008 at 09:38:26AM +0200, Soren Hansen wrote:
Yes and no. In the latest patch I provided I only set use_extboot if
there's only one boot device defined, and it's a virtio device, so PXE
booting would use the old-style "-boot n" syntax. I literallly woke up
this morning and instantly smacked my forehead due to another problem
this introduced, so I'm happy you changed it. :)
The thing is that you can specify multiple boot devices in the XML to
set a priority, eg
<boot dev="network"/>
<boot dev="hd"/>
Gets maps to '--boot nc' so in this scenario you'd have a virtio device
being bootable, and also have PXE enabled.
Oh, and thanks for doing all the test cases as well. I didn't
want to
get started on those until we had agreed on the logic that should be
applied.
FWIW, I always start with the test cases first defining the XML and QEMU
args, and then write the impl until the test cases pass.
Dan.
--
|: Red Hat, Engineering, Boston -o- http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
12:53 a.m.
New subject: [libvirt] Libvir on Hardy: file a bug or not?
Hi,
I am using Ubuntu Hardy up to date.
When defining a domain with:
<domain type='kvm' id='1'>
<name>xp</name>
<uuid>dd4618e5-1c48-a481-0671-d75f38f59037</uuid>
<memory>524288</memory>
<currentMemory>524288</currentMemory>
<vcpu>1</vcpu>
<os>
<type arch='x86_64' machine='pc'>hvm</type>
<boot dev='hd'/>
</os>
<clock offset='localtime'/>
<on_poweroff>destroy</on_poweroff>
<on_reboot>destroy</on_reboot>
<on_crash>destroy</on_crash>
<devices>
<emulator>/usr/bin/qemu-system-x86_64</emulator>
<disk type='file' device='cdrom'>
<source file='/home/mihamina/xpjohnny.iso'/>
<target dev='hdc' bus='ide'/>
<readonly/>
</disk>
<disk type='file' device='disk'>
<source file='/home/mihamina/xp.img'/>
<target dev='hda' bus='ide'/>
</disk>
<interface type='bridge'>
<mac address='00:16:3e:1e:b2:22'/>
<source bridge='br0'/>
<target dev='vnet1'/>
</interface>
<input type='tablet' bus='usb'/>
<input type='mouse' bus='ps2'/>
<graphics type='vnc' port='5900' listen='127.0.0.1'/>
</devices>
</domain>
I get:
virsh # start xp
libvir: QEMU error : QEMU quit during console startup
qemu: unknowm parameter 'boot' in
'file=/home/mihamina/xp.img,if=ide,boot=on'
error: Failed to start domain xp
If I change the domain type with "qemu", I have no error (and the
virtual machine starts without any problem.
This is my installed things:
$ dpkg -l | awk '/(virt|qemu|kvm)/{print $2,$3}'
kvm 1:62+dfsg-0ubuntu8
libvirt-bin 0.4.0-2ubuntu8
libvirt0 0.4.0-2ubuntu8
python-libvirt 0.4.0-2ubuntu8
python-virtinst 0.300.2-0ubuntu6
python-virtkey 0.50ubuntu0.1
qemu 0.9.1-1ubuntu1
virt-manager 0.5.3-0ubuntu10
virt-viewer 0.0.2-1ubuntu1
Is it an upstream bug or a distribution bug?
Who is supposed to be noticed? The packager or the upstream (where please)?
PS: I wrote because I saw
http://www.mail-archive.com/libvir-list@redhat.com/msg06109.html
3:56 a.m.
New subject: [libvirt] Libvir on Hardy: file a bug or not?
On Tue, Jul 29, 2008 at 08:53:03AM +0300, Mihamina Rakotomandimby (R12y) wrote:
Hi,
I am using Ubuntu Hardy up to date.
I get:
virsh # start xp
libvir: QEMU error : QEMU quit during console startup
qemu: unknowm parameter 'boot' in
'file=/home/mihamina/xp.img,if=ide,boot=on'
error: Failed to start domain xp
If I change the domain type with "qemu", I have no error (and the
virtual machine starts without any problem.
The Ubuntu libvirt had a early version of the patch for supporting
the 'boot=on' parameter - I believe they always use it for any VM
with a domain type of 'kvm'. Since you are using regular QEMU it
is correct that you need to change the domain type to type=qemu
anyway.
This is my installed things:
$ dpkg -l | awk '/(virt|qemu|kvm)/{print $2,$3}'
kvm 1:62+dfsg-0ubuntu8
libvirt-bin 0.4.0-2ubuntu8
libvirt0 0.4.0-2ubuntu8
python-libvirt 0.4.0-2ubuntu8
python-virtinst 0.300.2-0ubuntu6
python-virtkey 0.50ubuntu0.1
qemu 0.9.1-1ubuntu1
virt-manager 0.5.3-0ubuntu10
virt-viewer 0.0.2-1ubuntu1
Is it an upstream bug or a distribution bug?
I believe it is a disto bug - the upstream code for this should be
auto-detecting whether 'boot=on' is supported automatically now.
Who is supposed to be noticed? The packager or the upstream (where
please)?
Generally speaking, if you are yusing the libvir packages built by your
OS distributor,then use their bug tracking system. They will forward
bugs to upstream libvirt as required, or fix OS specific bugs directly.
http://libvirt.org/bugs.html
Daniel
--
|: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
4:39 a.m.
New subject: [libvirt] Libvir on Hardy: file a bug or not?
Daniel P. Berrange wrote:
The Ubuntu libvirt had a early version of the patch for supporting
the 'boot=on' parameter - I believe they always use it for any VM
with a domain type of 'kvm'. Since you are using regular QEMU it
is correct that you need to change the domain type to type=qemu
anyway.
My problem if I use "qemu" as domain type is the guest is slow enough to
make me think it doesnt use the kvm.
In the beginning, and in Ubuntu documentation, the domain type is "qemu"
as default.
So, the question that might solve the problems:
How can assume it's realy using kvm (and not just qemu)?
Note: when I launch the guest via the "kvm" command [1], it's faster.
[1] e.g.:
$ kvm -m 512 -hda foo.img -cdrom bar.iso -boot d
4:43 a.m.
New subject: [libvirt] Libvir on Hardy: file a bug or not?
Quoting "Mihamina Rakotomandimby (R12y)"
<mihamina.rakotomandimby(a)etu.univ-orleans.fr>:
I cannot help you with your problem but in case you want to try the
latest and greatest libvirt, there are 0.4.4 packages available in my
PPA: http://launchpad.net/~pgquiles/+archive (UNOFFICIAL AND
UNSUPPORTED)
Hi,
I am using Ubuntu Hardy up to date.
When defining a domain with:
<domain type='kvm' id='1'>
<name>xp</name>
<uuid>dd4618e5-1c48-a481-0671-d75f38f59037</uuid>
<memory>524288</memory>
<currentMemory>524288</currentMemory>
<vcpu>1</vcpu>
<os>
<type arch='x86_64' machine='pc'>hvm</type>
<boot dev='hd'/>
</os>
<clock offset='localtime'/>
<on_poweroff>destroy</on_poweroff>
<on_reboot>destroy</on_reboot>
<on_crash>destroy</on_crash>
<devices>
<emulator>/usr/bin/qemu-system-x86_64</emulator>
<disk type='file' device='cdrom'>
<source file='/home/mihamina/xpjohnny.iso'/>
<target dev='hdc' bus='ide'/>
<readonly/>
</disk>
<disk type='file' device='disk'>
<source file='/home/mihamina/xp.img'/>
<target dev='hda' bus='ide'/>
</disk>
<interface type='bridge'>
<mac address='00:16:3e:1e:b2:22'/>
<source bridge='br0'/>
<target dev='vnet1'/>
</interface>
<input type='tablet' bus='usb'/>
<input type='mouse' bus='ps2'/>
<graphics type='vnc' port='5900' listen='127.0.0.1'/>
</devices>
</domain>
I get:
virsh # start xp
libvir: QEMU error : QEMU quit during console startup
qemu: unknowm parameter 'boot' in
'file=/home/mihamina/xp.img,if=ide,boot=on'
error: Failed to start domain xp
If I change the domain type with "qemu", I have no error (and the
virtual machine starts without any problem.
This is my installed things:
$ dpkg -l | awk '/(virt|qemu|kvm)/{print $2,$3}'
kvm 1:62+dfsg-0ubuntu8
libvirt-bin 0.4.0-2ubuntu8
libvirt0 0.4.0-2ubuntu8
python-libvirt 0.4.0-2ubuntu8
python-virtinst 0.300.2-0ubuntu6
python-virtkey 0.50ubuntu0.1
qemu 0.9.1-1ubuntu1
virt-manager 0.5.3-0ubuntu10
virt-viewer 0.0.2-1ubuntu1
Is it an upstream bug or a distribution bug?
Who is supposed to be noticed? The packager or the upstream (where please)?
PS: I wrote because I saw
http://www.mail-archive.com/libvir-list@redhat.com/msg06109.html
--
Libvir-list mailing list
Libvir-list(a)redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
--
Pau Garcia i Quiles
http://www.elpauer.org
(Due to my workload, I may need 10 days to answer)
7:44 a.m.
New subject: [libvirt] Libvir on Hardy: file a bug or not?
Pau Garcia i Quiles wrote:
I cannot help you with your problem but in case you want to try the
latest and greatest libvirt, there are 0.4.4 packages available in my
PPA: http://launchpad.net/~pgquiles/+archive (UNOFFICIAL AND UNSUPPORTED)
They doesnt solve the problem.
Thank you, anyway.
5959
days inactive
6050
days old
17 comments
6 participants
participants (6)
-
Daniel P. Berrange -
Jim Meyering -
Mark McLoughlin -
Mihamina Rakotomandimby (R12y) -
Pau Garcia i Quiles -
Soren Hansen