From 40a867daeb224d0d79551c4a91bd4a709d31aa40 Mon Sep 17 00:00:00 2001 From: "Daniel P. Berrange" Date: Wed, 5 Aug 2009 15:48:14 +0000 Subject: [PATCH] Misc important fixes from BZ - Fix crash when attaching/detaching non-existant PCI device (rhbz #510907) - Fix QEMU guest name/uuid uniqueness checks (rhbz #507405) - Fix to use correct pci_add/del syntax for QEMU (rhbz #499669) - Relabel disks before hotplugging them to guest (rhbz #496442) - Correctly handle 8-bit high bytes when escaping XML (rhbz #479517) --- libvirt-0.6.2-buf-locale-escape.patch | 24 ++++ libvirt-0.6.2-hotplug-labelling.patch | 37 +++++ libvirt-0.6.2-hotplug-monitor-syntax.patch | 125 ++++++++++++++++ libvirt-0.6.2-monitor-prompt-discard.patch | 52 +++++++ libvirt-0.6.2-pci-device-crash.patch | 22 +++ libvirt-0.6.2-qemu-name-uniqueness.patch | 160 +++++++++++++++++++++ libvirt.spec | 27 +++- 7 files changed, 446 insertions(+), 1 deletion(-) create mode 100644 libvirt-0.6.2-buf-locale-escape.patch create mode 100644 libvirt-0.6.2-hotplug-labelling.patch create mode 100644 libvirt-0.6.2-hotplug-monitor-syntax.patch create mode 100644 libvirt-0.6.2-monitor-prompt-discard.patch create mode 100644 libvirt-0.6.2-pci-device-crash.patch create mode 100644 libvirt-0.6.2-qemu-name-uniqueness.patch diff --git a/libvirt-0.6.2-buf-locale-escape.patch b/libvirt-0.6.2-buf-locale-escape.patch new file mode 100644 index 0000000..afd8931 --- /dev/null +++ b/libvirt-0.6.2-buf-locale-escape.patch @@ -0,0 +1,24 @@ +commit 8feb499ba2c3625632210c997b49f5df515c05d4 +Author: Daniel P. Berrange +Date: Tue Aug 4 18:13:09 2009 +0100 + + Fix escaping of 8-bit high characters + + Fix https://bugzilla.redhat.com/show_bug.cgi?id=479517 + + * src/buf.c: Cast to 'unsigned char' before doing compare to + avoid rejecting 8-bit high characters + +diff --git a/src/buf.c b/src/buf.c +index 259175d..c802aa2 100644 +--- a/src/buf.c ++++ b/src/buf.c +@@ -304,7 +304,7 @@ virBufferEscapeString(const virBufferPtr buf, const char *format, const char *st + *out++ = 'o'; + *out++ = 's'; + *out++ = ';'; +- } else if ((*cur >= 0x20) || (*cur == '\n') || (*cur == '\t') || ++ } else if (((unsigned char)*cur >= 0x20) || (*cur == '\n') || (*cur == '\t') || + (*cur == '\r')) { + /* + * default case, just copy ! diff --git a/libvirt-0.6.2-hotplug-labelling.patch b/libvirt-0.6.2-hotplug-labelling.patch new file mode 100644 index 0000000..8c23091 --- /dev/null +++ b/libvirt-0.6.2-hotplug-labelling.patch @@ -0,0 +1,37 @@ +commit 1795bfe4a177a5eff1b3b0a16d56df6f371c0f8e +Author: Daniel P. Berrange +Date: Mon Jul 6 16:01:55 2009 +0100 + + Fix SELinux denial during hotplug + + * src/qemu_driver.c: Relabel disk images *before* running QEMU + hotplug monitor commands + +diff --git a/src/qemu_driver.c b/src/qemu_driver.c +index 5a0ab12..342ba01 100644 +--- a/src/qemu_driver.c ++++ b/src/qemu_driver.c +@@ -4225,10 +4225,14 @@ static int qemudDomainAttachDevice(virDomainPtr dom, + switch (dev->data.disk->device) { + case VIR_DOMAIN_DISK_DEVICE_CDROM: + case VIR_DOMAIN_DISK_DEVICE_FLOPPY: ++ if (driver->securityDriver) ++ driver->securityDriver->domainSetSecurityImageLabel(dom->conn, vm, dev->data.disk); + ret = qemudDomainChangeEjectableMedia(dom->conn, vm, dev); + break; + + case VIR_DOMAIN_DISK_DEVICE_DISK: ++ if (driver->securityDriver) ++ driver->securityDriver->domainSetSecurityImageLabel(dom->conn, vm, dev->data.disk); + if (dev->data.disk->bus == VIR_DOMAIN_DISK_BUS_USB) { + ret = qemudDomainAttachUsbMassstorageDevice(dom->conn, vm, dev); + } else if (dev->data.disk->bus == VIR_DOMAIN_DISK_BUS_SCSI || +@@ -4240,8 +4244,6 @@ static int qemudDomainAttachDevice(virDomainPtr dom, + virDomainDiskBusTypeToString(dev->data.disk->bus)); + goto cleanup; + } +- if (driver->securityDriver) +- driver->securityDriver->domainSetSecurityImageLabel(dom->conn, vm, dev->data.disk); + break; + + default: diff --git a/libvirt-0.6.2-hotplug-monitor-syntax.patch b/libvirt-0.6.2-hotplug-monitor-syntax.patch new file mode 100644 index 0000000..46744a8 --- /dev/null +++ b/libvirt-0.6.2-hotplug-monitor-syntax.patch @@ -0,0 +1,125 @@ +commit 326ecb78145cfeb7706ef0dcd521b19d934950e7 +Author: Daniel P. Berrange +Date: Mon Jul 6 15:58:55 2009 +0100 + + Fix PCI device hotplug/unplug with newer QEMU + + * src/qemu_driver.c: Try new monitor syntax for hotplug first. If + that fails fallback to old KVM specific syntax + +diff --git a/src/qemu_driver.c b/src/qemu_driver.c +index 2e55045..5a0ab12 100644 +--- a/src/qemu_driver.c ++++ b/src/qemu_driver.c +@@ -4004,6 +4004,7 @@ static int qemudDomainAttachPciDiskDevice(virConnectPtr conn, + char *cmd, *reply, *s; + char *safe_path; + const char* type = virDomainDiskBusTypeToString(dev->data.disk->bus); ++ int tryOldSyntax = 0; + + for (i = 0 ; i < vm->def->ndisks ; i++) { + if (STREQ(vm->def->disks[i]->dst, dev->data.disk->dst)) { +@@ -4018,14 +4019,15 @@ static int qemudDomainAttachPciDiskDevice(virConnectPtr conn, + return -1; + } + ++try_command: + safe_path = qemudEscapeMonitorArg(dev->data.disk->src); + if (!safe_path) { + virReportOOMError(conn); + return -1; + } + +- ret = virAsprintf(&cmd, "pci_add 0 storage file=%s,if=%s", +- safe_path, type); ++ ret = virAsprintf(&cmd, "pci_add %s storage file=%s,if=%s", ++ (tryOldSyntax ? "0": "pci_addr=auto"), safe_path, type); + VIR_FREE(safe_path); + if (ret == -1) { + virReportOOMError(conn); +@@ -4041,17 +4043,27 @@ static int qemudDomainAttachPciDiskDevice(virConnectPtr conn, + + DEBUG ("%s: pci_add reply: %s", vm->def->name, reply); + /* If the command succeeds qemu prints: +- * OK bus 0... */ +-#define PCI_ATTACH_OK_MSG "OK bus 0, slot " +- if ((s=strstr(reply, PCI_ATTACH_OK_MSG))) { +- char* dummy = s; +- s += strlen(PCI_ATTACH_OK_MSG); ++ * OK bus 0, slot XXX... ++ * or ++ * OK domain 0, bus 0, slot XXX ++ */ ++ if ((s = strstr(reply, "OK ")) && ++ (s = strstr(s, "slot "))) { ++ char *dummy = s; ++ s += strlen("slot "); + + if (virStrToLong_i ((const char*)s, &dummy, 10, &dev->data.disk->slotnum) == -1) + VIR_WARN("%s", _("Unable to parse slot number\n")); ++ /* XXX not neccessarily always going to end up in domain 0 / bus 0 :-( */ ++ /* XXX this slotnum is not persistant across restarts :-( */ ++ } else if (!tryOldSyntax && strstr(reply, "invalid char in expression")) { ++ VIR_FREE(reply); ++ VIR_FREE(cmd); ++ tryOldSyntax = 1; ++ goto try_command; + } else { + qemudReportError (conn, dom, NULL, VIR_ERR_OPERATION_FAILED, +- _("adding %s disk failed"), type); ++ _("adding %s disk failed: %s"), type, reply); + VIR_FREE(reply); + VIR_FREE(cmd); + return -1; +@@ -4268,6 +4280,7 @@ static int qemudDomainDetachPciDiskDevice(virConnectPtr conn, + char *cmd = NULL; + char *reply = NULL; + virDomainDiskDefPtr detach = NULL; ++ int tryOldSyntax = 0; + + for (i = 0 ; i < vm->def->ndisks ; i++) { + if (STREQ(vm->def->disks[i]->dst, dev->data.disk->dst)) { +@@ -4289,9 +4302,17 @@ static int qemudDomainDetachPciDiskDevice(virConnectPtr conn, + goto cleanup; + } + +- if (virAsprintf(&cmd, "pci_del 0 %d", detach->slotnum) < 0) { +- virReportOOMError(conn); +- goto cleanup; ++try_command: ++ if (tryOldSyntax) { ++ if (virAsprintf(&cmd, "pci_del 0 %d", detach->slotnum) < 0) { ++ virReportOOMError(conn); ++ goto cleanup; ++ } ++ } else { ++ if (virAsprintf(&cmd, "pci_del pci_addr=0:0:%d", detach->slotnum) < 0) { ++ virReportOOMError(conn); ++ goto cleanup; ++ } + } + + if (qemudMonitorCommand(vm, cmd, &reply) < 0) { +@@ -4301,12 +4322,19 @@ static int qemudDomainDetachPciDiskDevice(virConnectPtr conn, + } + + DEBUG ("%s: pci_del reply: %s",vm->def->name, reply); ++ ++ if (!tryOldSyntax && ++ strstr(reply, "extraneous characters")) { ++ tryOldSyntax = 1; ++ goto try_command; ++ } + /* If the command fails due to a wrong slot qemu prints: invalid slot, + * nothing is printed on success */ +- if (strstr(reply, "invalid slot")) { ++ if (strstr(reply, "invalid slot") || ++ strstr(reply, "Invalid pci address")) { + qemudReportError (conn, NULL, NULL, VIR_ERR_OPERATION_FAILED, +- _("failed to detach disk %s: invalid slot %d"), +- detach->dst, detach->slotnum); ++ _("failed to detach disk %s: invalid slot %d: %s"), ++ detach->dst, detach->slotnum, reply); + goto cleanup; + } + diff --git a/libvirt-0.6.2-monitor-prompt-discard.patch b/libvirt-0.6.2-monitor-prompt-discard.patch new file mode 100644 index 0000000..694f80b --- /dev/null +++ b/libvirt-0.6.2-monitor-prompt-discard.patch @@ -0,0 +1,52 @@ +commit 2d1f2e706c8b13571e1227df1c69b2302da35d5a +Author: Daniel P. Berrange +Date: Mon Jul 6 15:45:04 2009 +0100 + + Fix problem with QEMU monitor welcome prompt confusing libvirt + after a libvirtd daemon restart with active guests + + * src/qemu_driver: Read and dicard pending monitor data + before issuing new monitor commands. + +diff --git a/src/qemu_driver.c b/src/qemu_driver.c +index e2b7acb..2e55045 100644 +--- a/src/qemu_driver.c ++++ b/src/qemu_driver.c +@@ -1744,6 +1744,28 @@ cleanup: + qemuDriverUnlock(driver); + } + ++ ++/* Throw away any data available on the monitor ++ * This is done before executing a command, in order ++ * to allow re-synchronization if something went badly ++ * wrong in the past. it also deals with problem of ++ * QEMU *sometimes* re-printing its initial greeting ++ * when we reconnect to the monitor after restarts. ++ */ ++static void ++qemuMonitorDiscardPendingData(virDomainObjPtr vm) { ++ char buf[1024]; ++ int ret = 0; ++ ++ /* Monitor is non-blocking, so just loop till we ++ * get -1 or 0. Don't bother with detecting ++ * errors, since we'll deal with that better later */ ++ do { ++ ret = read(vm->monitor, buf, sizeof (buf)-1); ++ } while (ret > 0); ++} ++ ++ + static int + qemudMonitorCommandExtra(const virDomainObjPtr vm, + const char *cmd, +@@ -1755,6 +1777,8 @@ qemudMonitorCommandExtra(const virDomainObjPtr vm, + size_t cmdlen = strlen(cmd); + size_t extralen = extra ? strlen(extra) : 0; + ++ qemuMonitorDiscardPendingData(vm); ++ + if (safewrite(vm->monitor, cmd, cmdlen) != cmdlen) + return -1; + if (safewrite(vm->monitor, "\r", 1) != 1) diff --git a/libvirt-0.6.2-pci-device-crash.patch b/libvirt-0.6.2-pci-device-crash.patch new file mode 100644 index 0000000..3d052e1 --- /dev/null +++ b/libvirt-0.6.2-pci-device-crash.patch @@ -0,0 +1,22 @@ +commit 4a7acedd3c59a6a750576cb8680bc3f08fe0b52c +Author: Daniel P. Berrange +Date: Thu Jul 16 13:23:32 2009 +0100 + + Fix free of unitialized data upon PCI open fail + +diff --git a/src/pci.c b/src/pci.c +index 3ffa0aa..4030a14 100644 +--- a/src/pci.c ++++ b/src/pci.c +@@ -834,10 +834,8 @@ pciReadDeviceID(pciDevice *dev, const char *id_name) + dev->name, id_name); + + /* ID string is '0xNNNN\n' ... i.e. 7 bytes */ +- if (virFileReadAll(path, 7, &id_str) < 7) { +- VIR_FREE(id_str); ++ if (virFileReadAll(path, 7, &id_str) < 0) + return NULL; +- } + + /* Check for 0x suffix */ + if (id_str[0] != '0' || id_str[1] != 'x') { diff --git a/libvirt-0.6.2-qemu-name-uniqueness.patch b/libvirt-0.6.2-qemu-name-uniqueness.patch new file mode 100644 index 0000000..0be473a --- /dev/null +++ b/libvirt-0.6.2-qemu-name-uniqueness.patch @@ -0,0 +1,160 @@ +diff -rupN libvirt-0.6.2/src/qemu_driver.c libvirt-0.6.2.new/src/qemu_driver.c +--- libvirt-0.6.2/src/qemu_driver.c 2009-08-05 16:25:22.000000000 +0100 ++++ libvirt-0.6.2.new/src/qemu_driver.c 2009-08-05 16:27:48.000000000 +0100 +@@ -2174,22 +2174,37 @@ static virDomainPtr qemudDomainCreate(vi + if (virSecurityDriverVerify(conn, def) < 0) + goto cleanup; + +- vm = virDomainFindByName(&driver->domains, def->name); +- if (vm) { +- qemudReportError(conn, NULL, NULL, VIR_ERR_OPERATION_FAILED, +- _("domain '%s' is already defined"), +- def->name); +- goto cleanup; +- } ++ /* See if a VM with matching UUID already exists */ + vm = virDomainFindByUUID(&driver->domains, def->uuid); + if (vm) { +- char uuidstr[VIR_UUID_STRING_BUFLEN]; ++ /* UUID matches, but if names don't match, refuse it */ ++ if (STRNEQ(vm->def->name, def->name)) { ++ char uuidstr[VIR_UUID_STRING_BUFLEN]; ++ virUUIDFormat(vm->def->uuid, uuidstr); ++ qemudReportError(conn, NULL, NULL, VIR_ERR_OPERATION_FAILED, ++ _("domain '%s' is already defined with uuid %s"), ++ vm->def->name, uuidstr); ++ goto cleanup; ++ } + +- virUUIDFormat(def->uuid, uuidstr); +- qemudReportError(conn, NULL, NULL, VIR_ERR_OPERATION_FAILED, +- _("domain with uuid '%s' is already defined"), +- uuidstr); +- goto cleanup; ++ /* UUID & name match, but if VM is already active, refuse it */ ++ if (virDomainIsActive(vm)) { ++ qemudReportError(conn, NULL, NULL, VIR_ERR_OPERATION_FAILED, ++ _("domain is already active as '%s'"), vm->def->name); ++ goto cleanup; ++ } ++ virDomainObjUnlock(vm); ++ } else { ++ /* UUID does not match, but if a name matches, refuse it */ ++ vm = virDomainFindByName(&driver->domains, def->name); ++ if (vm) { ++ char uuidstr[VIR_UUID_STRING_BUFLEN]; ++ virUUIDFormat(vm->def->uuid, uuidstr); ++ qemudReportError(conn, NULL, NULL, VIR_ERR_OPERATION_FAILED, ++ _("domain '%s' is already defined with uuid %s"), ++ def->name, uuidstr); ++ goto cleanup; ++ } + } + + if (!(vm = virDomainAssignDef(conn, +@@ -2368,6 +2383,11 @@ static int qemudDomainDestroy(virDomainP + _("no domain with matching id %d"), dom->id); + goto cleanup; + } ++ if (!virDomainIsActive(vm)) { ++ qemudReportError(dom->conn, dom, NULL, VIR_ERR_OPERATION_FAILED, ++ "%s", _("domain is not running")); ++ goto cleanup; ++ } + + qemudShutdownVMDaemon(dom->conn, driver, vm); + event = virDomainEventNewFromObj(vm, +@@ -3272,17 +3292,36 @@ static int qemudDomainRestore(virConnect + goto cleanup; + } + +- /* Ensure the name and UUID don't already exist in an active VM */ ++ /* See if a VM with matching UUID already exists */ + vm = virDomainFindByUUID(&driver->domains, def->uuid); +- if (!vm) +- vm = virDomainFindByName(&driver->domains, def->name); + if (vm) { ++ /* UUID matches, but if names don't match, refuse it */ ++ if (STRNEQ(vm->def->name, def->name)) { ++ char uuidstr[VIR_UUID_STRING_BUFLEN]; ++ virUUIDFormat(vm->def->uuid, uuidstr); ++ qemudReportError(conn, NULL, NULL, VIR_ERR_OPERATION_FAILED, ++ _("domain '%s' is already defined with uuid %s"), ++ vm->def->name, uuidstr); ++ goto cleanup; ++ } ++ ++ /* UUID & name match, but if VM is already active, refuse it */ + if (virDomainIsActive(vm)) { + qemudReportError(conn, NULL, NULL, VIR_ERR_OPERATION_FAILED, + _("domain is already active as '%s'"), vm->def->name); + goto cleanup; +- } else { +- virDomainObjUnlock(vm); ++ } ++ virDomainObjUnlock(vm); ++ } else { ++ /* UUID does not match, but if a name matches, refuse it */ ++ vm = virDomainFindByName(&driver->domains, def->name); ++ if (vm) { ++ char uuidstr[VIR_UUID_STRING_BUFLEN]; ++ virUUIDFormat(vm->def->uuid, uuidstr); ++ qemudReportError(conn, NULL, NULL, VIR_ERR_OPERATION_FAILED, ++ _("domain '%s' is already defined with uuid %s"), ++ def->name, uuidstr); ++ goto cleanup; + } + } + +@@ -3470,18 +3509,41 @@ static virDomainPtr qemudDomainDefine(vi + if (virSecurityDriverVerify(conn, def) < 0) + goto cleanup; + +- vm = virDomainFindByName(&driver->domains, def->name); ++ /* See if a VM with matching UUID already exists */ ++ vm = virDomainFindByUUID(&driver->domains, def->uuid); + if (vm) { ++ /* UUID matches, but if names don't match, refuse it */ ++ if (STRNEQ(vm->def->name, def->name)) { ++ char uuidstr[VIR_UUID_STRING_BUFLEN]; ++ virUUIDFormat(vm->def->uuid, uuidstr); ++ qemudReportError(conn, NULL, NULL, VIR_ERR_OPERATION_FAILED, ++ _("domain '%s' is already defined with uuid %s"), ++ vm->def->name, uuidstr); ++ goto cleanup; ++ } ++ ++ /* UUID & name match */ + virDomainObjUnlock(vm); + newVM = 0; ++ } else { ++ /* UUID does not match, but if a name matches, refuse it */ ++ vm = virDomainFindByName(&driver->domains, def->name); ++ if (vm) { ++ char uuidstr[VIR_UUID_STRING_BUFLEN]; ++ virUUIDFormat(vm->def->uuid, uuidstr); ++ qemudReportError(conn, NULL, NULL, VIR_ERR_OPERATION_FAILED, ++ _("domain '%s' is already defined with uuid %s"), ++ def->name, uuidstr); ++ goto cleanup; ++ } + } + + if (!(vm = virDomainAssignDef(conn, + &driver->domains, + def))) { +- virDomainDefFree(def); + goto cleanup; + } ++ def = NULL; + vm->persistent = 1; + + if (virDomainSaveConfig(conn, +@@ -3503,6 +3565,7 @@ static virDomainPtr qemudDomainDefine(vi + if (dom) dom->id = vm->def->id; + + cleanup: ++ virDomainDefFree(def); + if (vm) + virDomainObjUnlock(vm); + if (event) diff --git a/libvirt.spec b/libvirt.spec index d32224e..c8780c6 100644 --- a/libvirt.spec +++ b/libvirt.spec @@ -66,7 +66,7 @@ Summary: Library providing a simple API virtualization Name: libvirt Version: 0.6.2 -Release: 13%{?dist}%{?extra_release} +Release: 14%{?dist}%{?extra_release} License: LGPLv2+ Group: Development/Libraries Source: libvirt-%{version}.tar.gz @@ -104,6 +104,18 @@ Patch14: libvirt-0.6.2-avoid-broken-networking-with-newer-qemu.patch Patch15: libvirt-0.6.2-fix-libvirtd-crash-with-bad-capabilities-data.patch # Don't unnecessarily try to change a file context (bug #507555) Patch16: libvirt-0.6.2-do-not-unnecessarily-try-to-change-a-file-context.patch +# Misc useful fix +Patch17: libvirt-0.6.2-monitor-prompt-discard.patch +# rhbz #496442 +Patch18: libvirt-0.6.2-hotplug-labelling.patch +# rhbz 499669 +Patch19: libvirt-0.6.2-hotplug-monitor-syntax.patch +# rhbz #510907 +Patch20: libvirt-0.6.2-pci-device-crash.patch +# rhbz #507405 +Patch21: libvirt-0.6.2-qemu-name-uniqueness.patch +# rhbz #479517 +Patch22: libvirt-0.6.2-buf-locale-escape.patch # Not for upstream. Temporary hack till PulseAudio autostart # problems are sorted out when SELinux enforcing @@ -272,6 +284,12 @@ of recent versions of Linux (and other OSes). %patch14 -p1 %patch15 -p1 %patch16 -p1 +%patch17 -p1 +%patch18 -p1 +%patch19 -p1 +%patch20 -p1 +%patch21 -p1 +%patch22 -p1 %patch200 -p0 @@ -595,6 +613,13 @@ fi %endif %changelog +* Wed Aug 5 2009 Daniel P. Berrange - 0.6.2-14.fc11 +- Fix crash when attaching/detaching non-existant PCI device (rhbz #510907) +- Fix QEMU guest name/uuid uniqueness checks (rhbz #507405) +- Fix to use correct pci_add/del syntax for QEMU (rhbz #499669) +- Relabel disks before hotplugging them to guest (rhbz #496442) +- Correctly handle 8-bit high bytes when escaping XML (rhbz #479517) + * Fri Jul 3 2009 Mark McLoughlin - 0.6.2-13.fc11 - Fix libvirtd crash with bad capabilities data (bug #505635) - Don't unnecessarily try to change a file context (bug #507555)