[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] Problems with attach-device/detach-device using libvirt 0.7.6



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 :|


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]