1:03 a.m.
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(-)
1:03 a.m.
New subject: [libvirt] [RFC PATCH 1/4] qemuGetPciHostDeviceList take hostdev list directly
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(a)redhat.com>
Cc: Don Dutile <ddutile(a)redhat.com>
Cc: Chris Lalancette <clalance(a)redhat.com>
Cc: Mark McLoughlin <markmc(a)redhat.com>
Signed-off-by: Chris Wright <chrisw(a)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
11:18 a.m.
New subject: [libvirt] [RFC PATCH 1/4] qemuGetPciHostDeviceList take hostdev list directly
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(a)redhat.com>
Cc: Don Dutile <ddutile(a)redhat.com>
Cc: Chris Lalancette <clalance(a)redhat.com>
Cc: Mark McLoughlin <markmc(a)redhat.com>
Signed-off-by: Chris Wright <chrisw(a)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(a)veillard.com | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library http://libvirt.org/
1:03 a.m.
New subject: [libvirt] [RFC PATCH 2/4] Add helpers qemuPrepareHostdevPCIDevices and qemuDomainReAttachHostdevDevices
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(a)redhat.com>
Cc: Don Dutile <ddutile(a)redhat.com>
Cc: Chris Lalancette <clalance(a)redhat.com>
Cc: Mark McLoughlin <markmc(a)redhat.com>
Signed-off-by: Chris Wright <chrisw(a)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
11:23 a.m.
New subject: [libvirt] [RFC PATCH 2/4] Add helpers qemuPrepareHostdevPCIDevices and qemuDomainReAttachHostdevDevices
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(a)veillard.com | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library http://libvirt.org/
11:29 a.m.
New subject: [libvirt] [RFC PATCH 2/4] Add helpers qemuPrepareHostdevPCIDevices and qemuDomainReAttachHostdevDevices
* Daniel Veillard (veillard(a)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;
1:03 a.m.
New subject: [libvirt] [RFC PATCH 3/4] qemudDomainAttachHostPciDevice refactor to use new helpers
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(a)redhat.com>
Cc: Don Dutile <ddutile(a)redhat.com>
Cc: Chris Lalancette <clalance(a)redhat.com>
Cc: Mark McLoughlin <markmc(a)redhat.com>
Signed-off-by: Chris Wright <chrisw(a)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
11:34 a.m.
New subject: [libvirt] [RFC PATCH 3/4] qemudDomainAttachHostPciDevice refactor to use new helpers
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(a)veillard.com | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library http://libvirt.org/
1:03 a.m.
New subject: [libvirt] [RFC PATCH 4/4] pciResetDevice: use inactive devices to determine safe reset
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(a)redhat.com>
Cc: Don Dutile <ddutile(a)redhat.com>
Cc: Chris Lalancette <clalance(a)redhat.com>
Cc: Mark McLoughlin <markmc(a)redhat.com>
Signed-off-by: Chris Wright <chrisw(a)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
11:49 a.m.
New subject: [libvirt] [RFC PATCH 4/4] pciResetDevice: use inactive devices to determine safe reset
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(a)veillard.com | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library http://libvirt.org/
11:53 a.m.
New subject: [libvirt] [RFC PATCH 4/4] pciResetDevice: use inactive devices to determine safe reset
* Daniel Veillard (veillard(a)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
2:17 p.m.
New subject: [libvirt] [RFC PATCH 4/4] pciResetDevice: use inactive devices to determine safe reset
On Mon, Jul 26, 2010 at 09:53:34AM -0700, Chris Wright wrote:
* Daniel Veillard (veillard(a)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(a)veillard.com | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library http://libvirt.org/
5233
days inactive
5235
days old
11 comments
2 participants
participants (2)
-
Chris Wright -
Daniel Veillard