9:24 a.m.
Hi,
first a short summary of what the situation is:
I'm running qemu 0.12.2, libvirt 0.7.6 on a E5520 based machine with kernel
2.6.33. I also tried 2.6.32.7 as I was using that before and still had the old
kernel around, but kernel version doesn't matter AFAICT. The host is a gentoo
AMD64 installation, the client is SuSE 11.0 (both 32 and 64 bit tested).
This system is our machine to run automated PCIe device tests on, so the idea
is to move PCIe devices from the host to different vms, run some testcases
there and then move them away again. This has worked before (I'm not
absolutely sure, but I would bet a bit of money that it was libvirt 0.7.5).
So, what I'm going to do is simply
virsh # attach-device suse11.0-AMD64 dev-pci-AD1-CL.xml
Device attached successfully
virsh # detach-device suse11.0-AMD64 dev-pci-AD1-CL.xml
Device detached successfully
This is just was I expect and what I now get with the two patches attached.
The original situation was somewhat different (i.e. broken):
-first I got complains in the log stating:
tried to create id "(null)" twice for "device"
Well, adding two devices with the same id is bad, and if that id is a NULL
pointer anyway IMHO it's better to not pass the parameter at all. That's what
the "null-pci-id" patch does.
-next, detaching failed. I found out that the command sent to qemu was "
pci_del pci_addr=fc67add0:7f41:fe38b65a" which is obviously bogus. After some
hours of digging through the command stack up and down I found that it was the
memcpy addressed in the "pci_memcpy" patch.
When the device is attached qemu just replies with an empty string. Afterwards
from the guestAddr the address of the PCI device in the client is copied back
to the info structure. Too bad that this structure was never initialized to
anything useful (at least not for me). When I kick out the memcpy() everything
works fine.
Configuration files of one device and one guest are attached, the others look
similar.
Note: when the client PCI id is passed to qemu anyway it would be cool if that
could be specified in the device XML. For USB devices, too.
Eike
Please keep me CC'ed as I'm not on the list.
6:54 a.m.
On Fri, Feb 26, 2010 at 04:24:11PM +0100, Rolf Eike Beer wrote:
Hi,
first a short summary of what the situation is:
I'm running qemu 0.12.2, libvirt 0.7.6 on a E5520 based machine with kernel
2.6.33. I also tried 2.6.32.7 as I was using that before and still had the old
kernel around, but kernel version doesn't matter AFAICT. The host is a gentoo
AMD64 installation, the client is SuSE 11.0 (both 32 and 64 bit tested).
This system is our machine to run automated PCIe device tests on, so the idea
is to move PCIe devices from the host to different vms, run some testcases
there and then move them away again. This has worked before (I'm not
absolutely sure, but I would bet a bit of money that it was libvirt 0.7.5).
So, what I'm going to do is simply
virsh # attach-device suse11.0-AMD64 dev-pci-AD1-CL.xml
Device attached successfully
virsh # detach-device suse11.0-AMD64 dev-pci-AD1-CL.xml
Device detached successfully
This is just was I expect and what I now get with the two patches attached.
The original situation was somewhat different (i.e. broken):
-first I got complains in the log stating:
tried to create id "(null)" twice for "device"
Well, adding two devices with the same id is bad, and if that id is a NULL
pointer anyway IMHO it's better to not pass the parameter at all. That's what
the "null-pci-id" patch does.
The goal of libvirt when using QEMU >= 0.12 is that 'id=' parameter is set
for all devices. This enables libvirt reliably identify devices when talking
to QEMU. Since you were seeing '(null)' there must be a place in the code
where we've forgotton to generate the device alias name for hotplug. We should
fix that, rather than hiding the problem.
-next, detaching failed. I found out that the command sent to qemu was "
pci_del pci_addr=fc67add0:7f41:fe38b65a" which is obviously bogus. After some
hours of digging through the command stack up and down I found that it was the
memcpy addressed in the "pci_memcpy" patch.
When the device is attached qemu just replies with an empty string. Afterwards
from the guestAddr the address of the PCI device in the client is copied back
to the info structure. Too bad that this structure was never initialized to
anything useful (at least not for me). When I kick out the memcpy() everything
works fine.
Ok, I see what's wrong here. The code has two ways of attaching devices
if (qemuCmdFlags & QEMUD_CMD_FLAG_DEVICE)
ret = qemuMonitorAddDevice(priv->mon, devstr);
else
ret = qemuMonitorAddPCIHostDevice(priv->mon,
&hostdev->source.subsys.u.pci,
&guestAddr);
With the qemuMonitorAddDevice() method, libvirt decides the PCi address ahead
of time. In the qemuMonitorAddPCIHostDevice() method, QEMU decides the PCI
addresses & tells us about it. Thus we only want todo the memcpy() bit in the
'else' case, never the 'if' case.
Note: when the client PCI id is passed to qemu anyway it would be
cool if that
could be specified in the device XML. For USB devices, too.
You already can supply the PCI address in the XML - if you run 'virsh dumpxml'
on your running guest you'll see the syntax for PCI addresses. For USB devices,
the device IDs on the bus are assigned by the guest OS when it activates the
USB devices, so I don't think there's any way to allow user specification of
that
Regards,
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 :|
12:47 p.m.
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
2:57 p.m.
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 :|
5380
days inactive
5383
days old
3 comments
2 participants
participants (2)
-
Daniel P. Berrange -
Rolf Eike Beer