On Mon, Mar 01, 2010 at 07:47:20PM +0100, Rolf Eike Beer wrote:
> I wrote:
>
> [Problems attaching and detaching PCI devices]
>
> Ok, today I was working on that again and USB attaching was found to be
> completely broken, too. Please drop my libvirt-0.7.6-null-pci-id.patch patch
> from the previous mail and use libvirt-0.7.6-null-device-id.patch instead: USB
> has the very same problem.
>
> The read problem was indeed that attaching USB devices used "pci-attach" as
> qemu command instead of "usb_add": libvirt-0.7.6-usb-attach.patch comes to
> help out. And I think it's rather confusing to get messages about errors in
> PCI device configuration when it's in fact an USB device. This had nothing to
> do with my problem, but I found that and libvirt-0.7.6-usb-messages.patch
> fixes it.
>
> Eike
> diff -aurp libvirt-0.7.6-orig/src/qemu/qemu_conf.c libvirt-0.7.6/src/qemu/qemu_conf.c
> --- libvirt-0.7.6-orig/src/qemu/qemu_conf.c 2010-02-03 17:56:38.000000000 +0100
> +++ libvirt-0.7.6/src/qemu/qemu_conf.c 2010-03-01 16:35:21.000000000 +0100
> @@ -2690,7 +2690,8 @@ qemuBuildPCIHostdevDevStr(virDomainHostd
> dev->source.subsys.u.pci.bus,
> dev->source.subsys.u.pci.slot,
> dev->source.subsys.u.pci.function);
> - virBufferVSprintf(&buf, ",id=%s", dev->info.alias);
> + if (dev->info.alias != NULL)
> + virBufferVSprintf(&buf, ",id=%s", dev->info.alias);
> if (qemuBuildDeviceAddressStr(&buf, &dev->info) < 0)
> goto error;
>
> @@ -2734,11 +2735,12 @@ qemuBuildUSBHostdevDevStr(virDomainHostd
> return NULL;
> }
>
> - if (virAsprintf(&ret, "usb-host,hostbus=%.3d,hostaddr=%.3d,id=%s",
> + if (virAsprintf(&ret, "usb-host,hostbus=%.3d,hostaddr=%.3d",
> dev->source.subsys.u.usb.bus,
> - dev->source.subsys.u.usb.device,
> - dev->info.alias) < 0)
> + dev->source.subsys.u.usb.device) < 0)
> virReportOOMError(NULL);
> + if (dev->info.alias != NULL)
> + virBufferVSprintf(&ret, ",id=%s", dev->info.alias);
>
> return ret;
> }
This patch isn't correct - all devices *must* have a alias assigned
before hotplug. The real issue here is the hotplug method is missing
the API call to assign the alias.
> diff -aurp libvirt-0.7.6-orig/src/qemu/qemu_driver.c libvirt-0.7.6/src/qemu/qemu_driver.c
> --- libvirt-0.7.6-orig/src/qemu/qemu_driver.c 2010-02-03 16:56:00.000000000 +0100
> +++ libvirt-0.7.6/src/qemu/qemu_driver.c 2010-03-01 16:32:26.000000000 +0100
> @@ -5831,7 +5831,7 @@ static int qemudDomainAttachHostUsbDevic
> char *devstr = NULL;
>
> if ((qemuCmdFlags & QEMUD_CMD_FLAG_DEVICE) &&
> - !(devstr = qemuBuildPCIHostdevDevStr(hostdev)))
> + !(devstr = qemuBuildUSBHostdevDevStr(hostdev)))
> goto error;
>
> if (VIR_REALLOC_N(vm->def->hostdevs, vm->def->nhostdevs+1) < 0) {
> diff -aurp libvirt-0.7.6-orig/src/qemu/qemu_conf.c libvirt-0.7.6/src/qemu/qemu_conf.c
> --- libvirt-0.7.6-orig/src/qemu/qemu_conf.c 2010-02-03 17:56:38.000000000 +0100
> +++ libvirt-0.7.6/src/qemu/qemu_conf.c 2010-03-01 16:35:21.000000000 +0100
> @@ -4776,7 +4776,7 @@ qemuParseCommandLineUSB(virConnectPtr co
>
> if (!STRPREFIX(val, "host:")) {
> qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
> - _("unknown PCI device syntax '%s'"), val);
> + _("unknown USB device syntax '%s'"), val);
> VIR_FREE(def);
> goto cleanup;
> }
> @@ -4792,21 +4792,21 @@ qemuParseCommandLineUSB(virConnectPtr co
> start = end + 1;
> if (virStrToLong_i(start, NULL, 16, &second) < 0) {
> qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
> - _("cannot extract PCI device product '%s'"), val);
> + _("cannot extract USB device product '%s'"), val);
> VIR_FREE(def);
> goto cleanup;
> }
> } else {
> if (virStrToLong_i(start, &end, 10, &first) < 0 || !end || *end != '.') {
> qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
> - _("cannot extract PCI device bus '%s'"), val);
> + _("cannot extract USB device bus '%s'"), val);
> VIR_FREE(def);
> goto cleanup;
> }
> start = end + 1;
> if (virStrToLong_i(start, NULL, 10, &second) < 0) {
> qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
> - _("cannot extract PCI device address '%s'"), val);
> + _("cannot extract USB device address '%s'"), val);
> VIR_FREE(def);
> goto cleanup;
> }
These two look good & i'll apply them now.
Daniel
--
|: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list