[libvirt] [RFC PATCH 0/4] fix secondary bus reset with active devices logic
The current logic can allow libvirt to issue a secondary bus reset despite the host having devices on the same bus as a device that's being assigned to a guest. The following patches refactor the code a little and then update the secondary bus reset logic to properly deal with active devices. This has been lightly tested, testing is still incomplete, hence the RFC to get any style comments out of the way once testing verifies this is working as expected. The following scenarios have been successfully tested: - Do not issue a secondary bus reset: - start guest w/ domain conf including hostdev that shares bus w/ devices owned by host - use attach-device to dynamically add hostdev to guest, hostdev shares bus w/ devices owned by host - attempt to node-reset a device assigned to guest - Issue a secondary bus reset: - start guest w/ domain conf including all devices on bus[1] - use attach-device to dynamically add hostdev to guest, hostdev does not share bus with other devices Not yet tested: - Issue a secondary bus reset: - when shutting down domain - when detaching device from domain [1] guest did not succesfully start w/ these devices, unclear if this patch series is implicated. Chris Wright (4): qemuGetPciHostDeviceList take hostdev list directly Add helpers qemuPrepareHostdevPCIDevices and qemuDomainReAttachHostdevDevices qemudDomainAttachHostPciDevice refactor to use new helpers pciResetDevice: use inactive devices to determine safe reset src/qemu/qemu_driver.c | 87 ++++++++++++++++++++---------------------------- src/util/pci.c | 20 ++++++----- src/util/pci.h | 3 +- src/xen/xen_driver.c | 2 +- 4 files changed, 50 insertions(+), 62 deletions(-)
Update qemuGetPciHostDeviceList to take a hostdev list and count directly, rather than getting this indirectly from domain definition. This will allow reuse for the attach-device case. Cc: Alex Williamson <alex.williamson@redhat.com> Cc: Don Dutile <ddutile@redhat.com> Cc: Chris Lalancette <clalance@redhat.com> Cc: Mark McLoughlin <markmc@redhat.com> Signed-off-by: Chris Wright <chrisw@redhat.com> --- src/qemu/qemu_driver.c | 12 ++++++------ 1 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index c96788b..6e18d41 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -2827,7 +2827,7 @@ cleanup: static pciDeviceList * -qemuGetPciHostDeviceList(virDomainDefPtr def) +qemuGetPciHostDeviceList(virDomainHostdevDefPtr *hostdevs, int nhostdevs) { pciDeviceList *list; int i; @@ -2835,8 +2835,8 @@ qemuGetPciHostDeviceList(virDomainDefPtr def) if (!(list = pciDeviceListNew())) return NULL; - for (i = 0 ; i < def->nhostdevs ; i++) { - virDomainHostdevDefPtr hostdev = def->hostdevs[i]; + for (i = 0 ; i < nhostdevs ; i++) { + virDomainHostdevDefPtr hostdev = hostdevs[i]; pciDevice *dev; if (hostdev->mode != VIR_DOMAIN_HOSTDEV_MODE_SUBSYS) @@ -2876,7 +2876,7 @@ qemuUpdateActivePciHostdevs(struct qemud_driver *driver, if (!def->nhostdevs) return 0; - if (!(pcidevs = qemuGetPciHostDeviceList(def))) + if (!(pcidevs = qemuGetPciHostDeviceList(def->hostdevs, def->nhostdevs))) return -1; for (i = 0; i < pciDeviceListCount(pcidevs); i++) { @@ -2904,7 +2904,7 @@ qemuPrepareHostPCIDevices(struct qemud_driver *driver, int i; int ret = -1; - if (!(pcidevs = qemuGetPciHostDeviceList(def))) + if (!(pcidevs = qemuGetPciHostDeviceList(def->hostdevs, def->nhostdevs))) return -1; /* We have to use 3 loops here. *All* devices must @@ -3056,7 +3056,7 @@ qemuDomainReAttachHostDevices(struct qemud_driver *driver, if (!def->nhostdevs) return; - if (!(pcidevs = qemuGetPciHostDeviceList(def))) { + if (!(pcidevs = qemuGetPciHostDeviceList(def->hostdevs, def->nhostdevs))) { virErrorPtr err = virGetLastError(); VIR_ERROR(_("Failed to allocate pciDeviceList: %s"), err ? err->message : _("unknown error")); -- 1.7.1.1
On Fri, Jul 23, 2010 at 11:03:44PM -0700, Chris Wright wrote:
Update qemuGetPciHostDeviceList to take a hostdev list and count directly, rather than getting this indirectly from domain definition. This will allow reuse for the attach-device case.
Cc: Alex Williamson <alex.williamson@redhat.com> Cc: Don Dutile <ddutile@redhat.com> Cc: Chris Lalancette <clalance@redhat.com> Cc: Mark McLoughlin <markmc@redhat.com> Signed-off-by: Chris Wright <chrisw@redhat.com> --- src/qemu/qemu_driver.c | 12 ++++++------ 1 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index c96788b..6e18d41 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -2827,7 +2827,7 @@ cleanup:
static pciDeviceList * -qemuGetPciHostDeviceList(virDomainDefPtr def) +qemuGetPciHostDeviceList(virDomainHostdevDefPtr *hostdevs, int nhostdevs) { pciDeviceList *list; int i; @@ -2835,8 +2835,8 @@ qemuGetPciHostDeviceList(virDomainDefPtr def) if (!(list = pciDeviceListNew())) return NULL;
- for (i = 0 ; i < def->nhostdevs ; i++) { - virDomainHostdevDefPtr hostdev = def->hostdevs[i]; + for (i = 0 ; i < nhostdevs ; i++) { + virDomainHostdevDefPtr hostdev = hostdevs[i]; pciDevice *dev;
if (hostdev->mode != VIR_DOMAIN_HOSTDEV_MODE_SUBSYS) @@ -2876,7 +2876,7 @@ qemuUpdateActivePciHostdevs(struct qemud_driver *driver, if (!def->nhostdevs) return 0;
- if (!(pcidevs = qemuGetPciHostDeviceList(def))) + if (!(pcidevs = qemuGetPciHostDeviceList(def->hostdevs, def->nhostdevs))) return -1;
for (i = 0; i < pciDeviceListCount(pcidevs); i++) { @@ -2904,7 +2904,7 @@ qemuPrepareHostPCIDevices(struct qemud_driver *driver, int i; int ret = -1;
- if (!(pcidevs = qemuGetPciHostDeviceList(def))) + if (!(pcidevs = qemuGetPciHostDeviceList(def->hostdevs, def->nhostdevs))) return -1;
/* We have to use 3 loops here. *All* devices must @@ -3056,7 +3056,7 @@ qemuDomainReAttachHostDevices(struct qemud_driver *driver, if (!def->nhostdevs) return;
- if (!(pcidevs = qemuGetPciHostDeviceList(def))) { + if (!(pcidevs = qemuGetPciHostDeviceList(def->hostdevs, def->nhostdevs))) { virErrorPtr err = virGetLastError(); VIR_ERROR(_("Failed to allocate pciDeviceList: %s"), err ? err->message : _("unknown error"));
ACK, looks fine, mechanical change Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/
These new helpers take hostdev list and count directly rather than getting them indirectly from domain definition. This will allow reuse for the attach-device case. Cc: Alex Williamson <alex.williamson@redhat.com> Cc: Don Dutile <ddutile@redhat.com> Cc: Chris Lalancette <clalance@redhat.com> Cc: Mark McLoughlin <markmc@redhat.com> Signed-off-by: Chris Wright <chrisw@redhat.com> --- src/qemu/qemu_driver.c | 34 +++++++++++++++++++++++++--------- 1 files changed, 25 insertions(+), 9 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 6e18d41..d8288da 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -2897,14 +2897,15 @@ cleanup: static int -qemuPrepareHostPCIDevices(struct qemud_driver *driver, - virDomainDefPtr def) +qemuPrepareHostdevPCIDevices(struct qemud_driver *driver, + virDomainHostdevDefPtr *hostdevs, + int nhostdevs) { pciDeviceList *pcidevs; int i; int ret = -1; - if (!(pcidevs = qemuGetPciHostDeviceList(def->hostdevs, def->nhostdevs))) + if (!(pcidevs = qemuGetPciHostDeviceList(hostdevs, nhostdevs))) return -1; /* We have to use 3 loops here. *All* devices must @@ -2954,6 +2955,13 @@ cleanup: return ret; } +static int +qemuPrepareHostPCIDevices(struct qemud_driver *driver, + virDomainDefPtr def) +{ + return qemuPrepareHostdevPCIDevices(driver, def->hostdevs, def->nhostdevs); +} + static int qemuPrepareHostUSBDevices(struct qemud_driver *driver ATTRIBUTE_UNUSED, @@ -3047,16 +3055,14 @@ qemudReattachManagedDevice(pciDevice *dev, struct qemud_driver *driver) } static void -qemuDomainReAttachHostDevices(struct qemud_driver *driver, - virDomainDefPtr def) +qemuDomainReAttachHostdevDevices(struct qemud_driver *driver, + virDomainHostdevDefPtr *hostdevs, + int nhostdevs) { pciDeviceList *pcidevs; int i; - if (!def->nhostdevs) - return; - - if (!(pcidevs = qemuGetPciHostDeviceList(def->hostdevs, def->nhostdevs))) { + if (!(pcidevs = qemuGetPciHostDeviceList(hostdevs, nhostdevs))) { virErrorPtr err = virGetLastError(); VIR_ERROR(_("Failed to allocate pciDeviceList: %s"), err ? err->message : _("unknown error")); @@ -3090,6 +3096,16 @@ qemuDomainReAttachHostDevices(struct qemud_driver *driver, pciDeviceListFree(pcidevs); } +static void +qemuDomainReAttachHostDevices(struct qemud_driver *driver, + virDomainDefPtr def) +{ + if (!def->nhostdevs) + return; + + qemuDomainReAttachHostdevDevices(driver, def->hostdevs, def->nhostdevs); +} + static const char *const defaultDeviceACL[] = { "/dev/null", "/dev/full", "/dev/zero", "/dev/random", "/dev/urandom", -- 1.7.1.1
On Fri, Jul 23, 2010 at 11:03:45PM -0700, Chris Wright wrote:
These new helpers take hostdev list and count directly rather than getting them indirectly from domain definition. This will allow reuse for the attach-device case.
+static int +qemuPrepareHostPCIDevices(struct qemud_driver *driver, + virDomainDefPtr def) +{ + return qemuPrepareHostdevPCIDevices(driver, def->hostdevs, def->nhostdevs);
Except for a tab here used for indenting, looks fine too Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/
* Daniel Veillard (veillard@redhat.com) wrote:
On Fri, Jul 23, 2010 at 11:03:45PM -0700, Chris Wright wrote:
These new helpers take hostdev list and count directly rather than getting them indirectly from domain definition. This will allow reuse for the attach-device case.
+static int +qemuPrepareHostPCIDevices(struct qemud_driver *driver, + virDomainDefPtr def) +{ + return qemuPrepareHostdevPCIDevices(driver, def->hostdevs, def->nhostdevs);
Except for a tab here used for indenting, looks fine too
Thanks, that and two other similar bits showed up in make syntax-check (thanks for the clue danpb). With changes below, it's make syntax-check clean. I can resend with those incorporated once testing is complete. thanks, -chris --- src/qemu/qemu_driver.c | 4 ++-- src/util/pci.c | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 28be97f..5ede991 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -3132,7 +3132,7 @@ static int qemuPrepareHostPCIDevices(struct qemud_driver *driver, virDomainDefPtr def) { - return qemuPrepareHostdevPCIDevices(driver, def->hostdevs, def->nhostdevs); + return qemuPrepareHostdevPCIDevices(driver, def->hostdevs, def->nhostdevs); } @@ -8017,7 +8017,7 @@ static int qemudDomainAttachHostPciDevice(struct qemud_driver *driver, } if (qemuPrepareHostdevPCIDevices(driver, &hostdev, 1)) - return -1; + return -1; if (qemuCmdFlags & QEMUD_CMD_FLAG_DEVICE) { if (qemuAssignDeviceHostdevAlias(vm->def, hostdev, -1) < 0) diff --git a/src/util/pci.c b/src/util/pci.c index 94bffae..1c10067 100644 --- a/src/util/pci.c +++ b/src/util/pci.c @@ -644,7 +644,7 @@ pciInitDevice(pciDevice *dev) int pciResetDevice(pciDevice *dev, pciDeviceList *activeDevs, - pciDeviceList *inactiveDevs) + pciDeviceList *inactiveDevs) { int ret = -1;
Eliminate code duplication by using the new helpers qemuPrepareHostdevPCIDevices and qemuDomainReAttachHostdevDevices. This reduces the number of open coded calls to pciResetDevice. Cc: Alex Williamson <alex.williamson@redhat.com> Cc: Don Dutile <ddutile@redhat.com> Cc: Chris Lalancette <clalance@redhat.com> Cc: Mark McLoughlin <markmc@redhat.com> Signed-off-by: Chris Wright <chrisw@redhat.com> --- src/qemu/qemu_driver.c | 37 +++---------------------------------- 1 files changed, 3 insertions(+), 34 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index d8288da..20946e6 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -7831,7 +7831,6 @@ static int qemudDomainAttachHostPciDevice(struct qemud_driver *driver, unsigned long long qemuCmdFlags) { qemuDomainObjPrivatePtr priv = vm->privateData; - pciDevice *pci; int ret; char *devstr = NULL; int configfd = -1; @@ -7842,25 +7841,8 @@ static int qemudDomainAttachHostPciDevice(struct qemud_driver *driver, return -1; } - pci = pciGetDevice(hostdev->source.subsys.u.pci.domain, - hostdev->source.subsys.u.pci.bus, - hostdev->source.subsys.u.pci.slot, - hostdev->source.subsys.u.pci.function); - if (!pci) - return -1; - - if (!pciDeviceIsAssignable(pci, !driver->relaxedACS) || - (hostdev->managed && pciDettachDevice(pci, driver->activePciHostdevs) < 0) || - pciResetDevice(pci, driver->activePciHostdevs) < 0) { - pciFreeDevice(pci); - return -1; - } - - if (pciDeviceListAdd(driver->activePciHostdevs, pci) < 0) { - pciFreeDevice(pci); - return -1; - } - pci = NULL; /* activePciHostdevs owns the 'pci' reference now */ + if (qemuPrepareHostdevPCIDevices(driver, &hostdev, 1)) + return -1; if (qemuCmdFlags & QEMUD_CMD_FLAG_DEVICE) { if (qemuAssignDeviceHostdevAlias(vm->def, hostdev, -1) < 0) @@ -7928,20 +7910,7 @@ error: qemuDomainPCIAddressReleaseAddr(priv->pciaddrs, &hostdev->info) < 0) VIR_WARN0("Unable to release PCI address on host device"); - pci = pciGetDevice(hostdev->source.subsys.u.pci.domain, - hostdev->source.subsys.u.pci.bus, - hostdev->source.subsys.u.pci.slot, - hostdev->source.subsys.u.pci.function); - - pciDeviceListDel(driver->activePciHostdevs, pci); - - if (pciResetDevice(pci, driver->activePciHostdevs) < 0) - VIR_WARN0("Unable to reset PCI device after assign failure"); - else if (hostdev->managed && - pciReAttachDevice(pci, driver->activePciHostdevs) < 0) - VIR_WARN0("Unable to re-attach PCI device after assign failure"); - pciFreeDevice(pci); - + qemuDomainReAttachHostdevDevices(driver, &hostdev, 1); VIR_FREE(devstr); VIR_FREE(configfd_name); -- 1.7.1.1
On Fri, Jul 23, 2010 at 11:03:46PM -0700, Chris Wright wrote:
Eliminate code duplication by using the new helpers qemuPrepareHostdevPCIDevices and qemuDomainReAttachHostdevDevices. This reduces the number of open coded calls to pciResetDevice.
ACK, nice cleanup, but I had to remove a tab again :-) Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/
When doing a PCI secondary bus reset, we must be sure that there are no active devices on the same bus segment. The active device tracking is designed to only track host devices that are active in use by guests. This ignores host devices that are actively in use by the host. So the current logic will reset host devices. Hilarity ensues. Switch this logic around and allow sbus reset when we are assigning all devices behind a bridge to the same guest at guest startup or as a result of a single attach-device command. Cc: Alex Williamson <alex.williamson@redhat.com> Cc: Don Dutile <ddutile@redhat.com> Cc: Chris Lalancette <clalance@redhat.com> Cc: Mark McLoughlin <markmc@redhat.com> Signed-off-by: Chris Wright <chrisw@redhat.com> --- src/qemu/qemu_driver.c | 8 ++++---- src/util/pci.c | 20 +++++++++++--------- src/util/pci.h | 3 ++- src/xen/xen_driver.c | 2 +- 4 files changed, 18 insertions(+), 15 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 20946e6..8f64247 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -2934,7 +2934,7 @@ qemuPrepareHostdevPCIDevices(struct qemud_driver *driver, * reset them */ for (i = 0; i < pciDeviceListCount(pcidevs); i++) { pciDevice *dev = pciDeviceListGet(pcidevs, i); - if (pciResetDevice(dev, driver->activePciHostdevs) < 0) + if (pciResetDevice(dev, driver->activePciHostdevs, pcidevs) < 0) goto cleanup; } @@ -3080,7 +3080,7 @@ qemuDomainReAttachHostdevDevices(struct qemud_driver *driver, for (i = 0; i < pciDeviceListCount(pcidevs); i++) { pciDevice *dev = pciDeviceListGet(pcidevs, i); - if (pciResetDevice(dev, driver->activePciHostdevs) < 0) { + if (pciResetDevice(dev, driver->activePciHostdevs, pcidevs) < 0) { virErrorPtr err = virGetLastError(); VIR_ERROR(_("Failed to reset PCI device: %s"), err ? err->message : _("unknown error")); @@ -8820,7 +8820,7 @@ static int qemudDomainDetachHostPciDevice(struct qemud_driver *driver, else { pciDeviceSetManaged(pci, detach->managed); pciDeviceListDel(driver->activePciHostdevs, pci); - if (pciResetDevice(pci, driver->activePciHostdevs) < 0) + if (pciResetDevice(pci, driver->activePciHostdevs, NULL) < 0) ret = -1; qemudReattachManagedDevice(pci, driver); pciFreeDevice(pci); @@ -11445,7 +11445,7 @@ qemudNodeDeviceReset (virNodeDevicePtr dev) qemuDriverLock(driver); - if (pciResetDevice(pci, driver->activePciHostdevs) < 0) + if (pciResetDevice(pci, driver->activePciHostdevs, NULL) < 0) goto out; ret = 0; diff --git a/src/util/pci.c b/src/util/pci.c index 6d0ca24..94bffae 100644 --- a/src/util/pci.c +++ b/src/util/pci.c @@ -440,11 +440,11 @@ pciDetectPowerManagementReset(pciDevice *dev) return 0; } -/* Any active devices other than the one supplied on the same domain/bus ? */ +/* Any active devices on the same domain/bus ? */ static int pciSharesBusWithActive(pciDevice *dev, pciDevice *check, void *data) { - pciDeviceList *activeDevs = data; + pciDeviceList *inactiveDevs = data; /* Different domain, different bus, or simply identical device */ if (dev->domain != check->domain || @@ -453,7 +453,8 @@ pciSharesBusWithActive(pciDevice *dev, pciDevice *check, void *data) dev->function == check->function)) return 0; - if (activeDevs && !pciDeviceListFind(activeDevs, check)) + /* same bus, but inactive, i.e. about to be assigned to guest */ + if (inactiveDevs && pciDeviceListFind(inactiveDevs, check)) return 0; return 1; @@ -461,11 +462,11 @@ pciSharesBusWithActive(pciDevice *dev, pciDevice *check, void *data) static pciDevice * pciBusContainsActiveDevices(pciDevice *dev, - pciDeviceList *activeDevs) + pciDeviceList *inactiveDevs) { pciDevice *active = NULL; if (pciIterDevices(pciSharesBusWithActive, - dev, &active, activeDevs) < 0) + dev, &active, inactiveDevs) < 0) return NULL; return active; } @@ -512,7 +513,7 @@ pciGetParentDevice(pciDevice *dev) */ static int pciTrySecondaryBusReset(pciDevice *dev, - pciDeviceList *activeDevs) + pciDeviceList *inactiveDevs) { pciDevice *parent, *conflict; uint8_t config_space[PCI_CONF_LEN]; @@ -524,7 +525,7 @@ pciTrySecondaryBusReset(pciDevice *dev, * In future, we could allow it so long as those devices * are not in use by the host or other guests. */ - if ((conflict = pciBusContainsActiveDevices(dev, activeDevs))) { + if ((conflict = pciBusContainsActiveDevices(dev, inactiveDevs))) { pciReportError(VIR_ERR_NO_SUPPORT, _("Active %s devices on bus with %s, not doing bus reset"), conflict->name, dev->name); @@ -642,7 +643,8 @@ pciInitDevice(pciDevice *dev) int pciResetDevice(pciDevice *dev, - pciDeviceList *activeDevs) + pciDeviceList *activeDevs, + pciDeviceList *inactiveDevs) { int ret = -1; @@ -670,7 +672,7 @@ pciResetDevice(pciDevice *dev, /* Bus reset is not an option with the root bus */ if (ret < 0 && dev->bus != 0) - ret = pciTrySecondaryBusReset(dev, activeDevs); + ret = pciTrySecondaryBusReset(dev, inactiveDevs); if (ret < 0) { virErrorPtr err = virGetLastError(); diff --git a/src/util/pci.h b/src/util/pci.h index 9aef81f..b767930 100644 --- a/src/util/pci.h +++ b/src/util/pci.h @@ -35,7 +35,8 @@ void pciFreeDevice (pciDevice *dev); int pciDettachDevice (pciDevice *dev, pciDeviceList *activeDevs); int pciReAttachDevice (pciDevice *dev, pciDeviceList *activeDevs); int pciResetDevice (pciDevice *dev, - pciDeviceList *activeDevs); + pciDeviceList *activeDevs, + pciDeviceList *inactiveDevs); void pciDeviceSetManaged(pciDevice *dev, unsigned managed); unsigned pciDeviceGetManaged(pciDevice *dev); diff --git a/src/xen/xen_driver.c b/src/xen/xen_driver.c index 3dd673b..5ebeb71 100644 --- a/src/xen/xen_driver.c +++ b/src/xen/xen_driver.c @@ -1892,7 +1892,7 @@ xenUnifiedNodeDeviceReset (virNodeDevicePtr dev) if (!pci) return -1; - if (pciResetDevice(pci, NULL) < 0) + if (pciResetDevice(pci, NULL, NULL) < 0) goto out; ret = 0; -- 1.7.1.1
On Fri, Jul 23, 2010 at 11:03:47PM -0700, Chris Wright wrote:
When doing a PCI secondary bus reset, we must be sure that there are no active devices on the same bus segment. The active device tracking is designed to only track host devices that are active in use by guests. This ignores host devices that are actively in use by the host. So the current logic will reset host devices. Hilarity ensues.
Switch this logic around and allow sbus reset when we are assigning all devices behind a bridge to the same guest at guest startup or as a result of a single attach-device command.
Okay that looks fine too, I pushed the 4 patches after the small changes for tabs. In the future "make syntax-check" helps avoid this :-) thanks ! Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/
* Daniel Veillard (veillard@redhat.com) wrote:
On Fri, Jul 23, 2010 at 11:03:47PM -0700, Chris Wright wrote:
When doing a PCI secondary bus reset, we must be sure that there are no active devices on the same bus segment. The active device tracking is designed to only track host devices that are active in use by guests. This ignores host devices that are actively in use by the host. So the current logic will reset host devices. Hilarity ensues.
Switch this logic around and allow sbus reset when we are assigning all devices behind a bridge to the same guest at guest startup or as a result of a single attach-device command.
Okay that looks fine too, I pushed the 4 patches after the small changes for tabs. In the future "make syntax-check" helps avoid this :-)
Great, thanks Daniel. I'm glad to learn about "make syntax-check"...my eye grep routine sucks at catching space/tab confusion ;-) thanks, -chris
On Mon, Jul 26, 2010 at 09:53:34AM -0700, Chris Wright wrote:
* Daniel Veillard (veillard@redhat.com) wrote:
On Fri, Jul 23, 2010 at 11:03:47PM -0700, Chris Wright wrote:
When doing a PCI secondary bus reset, we must be sure that there are no active devices on the same bus segment. The active device tracking is designed to only track host devices that are active in use by guests. This ignores host devices that are actively in use by the host. So the current logic will reset host devices. Hilarity ensues.
Switch this logic around and allow sbus reset when we are assigning all devices behind a bridge to the same guest at guest startup or as a result of a single attach-device command.
Okay that looks fine too, I pushed the 4 patches after the small changes for tabs. In the future "make syntax-check" helps avoid this :-)
Great, thanks Daniel. I'm glad to learn about "make syntax-check"...my eye grep routine sucks at catching space/tab confusion ;-)
Bahh just some bad kernel bias, one of Linus early mistake nobody fought enough to get rid of >:-> Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/
participants (2)
-
Chris Wright -
Daniel Veillard