ä 2011å01æ05æ 21:54, Yufang Zhang åé:
Reattaching pci device back to host without destroying guest or detaching
device from guest would cause host to crash. This patch adds a check before
doing device reattach. If the device is being assigned to guest, libvirt
refuses to reattach device to host. Note that the patch only works for Xen,
for it just checks xenstore to get pci device information.
So the subject of the patch could be:
xen: Check if device is assigned to guest before reattaching
Signed-off-by: Yufang Zhang<yuzhang@xxxxxxxxxx>
---
src/xen/xen_driver.c | 67 ++++++++++++++++++++++++++++++++++++++++++++++++++
1 files changed, 67 insertions(+), 0 deletions(-)
diff --git a/src/xen/xen_driver.c b/src/xen/xen_driver.c
index 4c11b11..5773d3b 100644
--- a/src/xen/xen_driver.c
+++ b/src/xen/xen_driver.c
@@ -1891,11 +1891,70 @@ out:
}
static int
+xenUnifiedNodeDeviceIsAssigned (virNodeDevicePtr dev)
+{
+ int numdomains;
+ int ret = -1, i;
+ int *ids = NULL;
+ char *bdf = NULL;
+ char *xref = NULL;
+ unsigned domain, bus, slot, function;
unsigned int
+ virConnectPtr conn = dev->conn;
+ xenUnifiedPrivatePtr priv = conn->privateData;
+
+ /* Get active domains */
+ numdomains = xenUnifiedNumOfDomains(conn);
+ if (numdomains< 0) {
+ return(ret);
Generally we use "return ret;"
+ }
+ if (numdomains> 0){
+ if (VIR_ALLOC_N(ids, numdomains)< 0){
+ virReportOOMError();
+ goto out;
+ }
+ if ((numdomains = xenUnifiedListDomains(conn,&ids[0], numdomains))< 0){
+ goto out;
+ }
+ }
+
+ /* Get pci bdf */
+ if (xenUnifiedNodeDeviceGetPciInfo(dev,&domain,&bus,&slot,&function)< 0)
+ goto out;
+
+ if (virAsprintf(&bdf, "%04x:%02x:%02x.%0x",
+ domain, bus, slot, function)< 0) {
+ virReportOOMError();
+ goto out;
+ }
+
+ /* Check if bdf is assigned to one of active domains */
+ for (i = 0; i< numdomains; i++ ){
+ xenUnifiedLock(priv);
+ xref = xenStoreDomainGetPCIID(conn, ids[i], bdf);
+ xenUnifiedUnlock(priv);
+ if (xref == NULL)
+ continue;
+ else {
+ ret = ids[i];
+ break;
+ }
+ }
+
+ VIR_FREE(xref);
+ VIR_FREE(bdf);
+out:
+ VIR_FREE(ids);
+
+ return(ret);
+}
Generally, a function "*Is*" will return a bool value, TRUE/FALSE,
altough seems you want to get the domain ID to use in following
function to print. But does it make sense to find a better method?
or a better function name?
+
+static int
xenUnifiedNodeDeviceReAttach (virNodeDevicePtr dev)
{
pciDevice *pci;
unsigned domain, bus, slot, function;
int ret = -1;
+ int dom;
From the code, it should be "domain id" here. so probly you need
a better variable name here.
if (xenUnifiedNodeDeviceGetPciInfo(dev,&domain,&bus,&slot,&function)< 0)
return -1;
@@ -1904,6 +1963,14 @@ xenUnifiedNodeDeviceReAttach (virNodeDevicePtr dev)
if (!pci)
return -1;
+ /* Check if device is assigned to an active guest */
+ if ((dom = xenUnifiedNodeDeviceIsAssigned(dev))>= 0){
+ xenUnifiedError(VIR_ERR_INTERNAL_ERROR,
+ _("Device %s has been assigned to guest %d"),
+ dev->name, dom);
+ goto out;
+ }
+
if (pciReAttachDevice(pci, NULL)< 0)
goto out;
Anyway, I like the idea, we have bug (host will crash or reboot if
try to reattach a PCI device to host which is using by guest) of
qemu driver. Though we can't resolve the problem in the similar way,
as kvm doesn't record the PCI/domain pair like xenstore. It still
gives some inspiration.
Thanks
Osier
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list