[libvirt] [PATCH V3] xen: check if device is assigned to guest before reattaching

[Osier Yang]

于 2011年04月27日 20:09, Yufang Zhang 写道:
> Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=664059
> 
> This is the version3 patch for BZ#664059. 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. The patch only works for Xen, for it just checks xenstore to get
> pci device information. This version fixes some formatting problem and moves
> lock-unlock out of loop so that the check is atomic.

We have same problem with qemu driver, though this patch looks fine
and fix the problem for xen driver, IMHO it's not the ideal solution,
ideally it will be nice if kernel could let us known if the device is
still in use. I filed bug against kernel, no feedback yet though.

> 
> Signed-off-by: Yufang Zhang<yuzhang at redhat.com>
> ---
>   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 2a07b7b..dd94fbc 100644
> --- a/src/xen/xen_driver.c
> +++ b/src/xen/xen_driver.c
> @@ -1927,11 +1927,70 @@ out:
>   }
> 
>   static int
> +xenUnifiedNodeDeviceAssignedDomainId (virNodeDevicePtr dev)
> +{
> +    int numdomains;
> +    int ret = -1, i;
> +    int *ids = NULL;
> +    char *bdf = NULL;
> +    char *xref = NULL;
> +    unsigned int domain, bus, slot, function;
> +    virConnectPtr conn = dev->conn;
> +    xenUnifiedPrivatePtr priv = conn->privateData;
> +
> +    /* Get active domains */
> +    numdomains = xenUnifiedNumOfDomains(conn);
> +    if (numdomains<  0) {
> +        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;
> +    }
> +
> +    xenUnifiedLock(priv);
> +    /* Check if bdf is assigned to one of active domains */
> +    for (i = 0; i<  numdomains; i++ ) {
> +        xref = xenStoreDomainGetPCIID(conn, ids[i], bdf);
> +        if (xref == NULL) {
> +            continue;
> +        } else {
> +            ret = ids[i];
> +            break;
> +        }
> +    }
> +    xenUnifiedUnlock(priv);
> +
> +    VIR_FREE(xref);
> +    VIR_FREE(bdf);
> +out:
> +    VIR_FREE(ids);
> +
> +    return ret;
> +}
> +
> +static int
>   xenUnifiedNodeDeviceReAttach (virNodeDevicePtr dev)
>   {
>       pciDevice *pci;
>       unsigned domain, bus, slot, function;
>       int ret = -1;
> +    int domid;
> 
>       if (xenUnifiedNodeDeviceGetPciInfo(dev,&domain,&bus,&slot,&function)<  0)
>           return -1;
> @@ -1940,6 +1999,14 @@ xenUnifiedNodeDeviceReAttach (virNodeDevicePtr dev)
>       if (!pci)
>           return -1;
> 
> +    /* Check if device is assigned to an active guest */
> +    if ((domid = xenUnifiedNodeDeviceAssignedDomainId(dev))>= 0) {
> +        xenUnifiedError(VIR_ERR_INTERNAL_ERROR,
> +                        _("Device %s has been assigned to guest %d"),
> +                        dev->name, domid);
> +        goto out;
> +    }
> +
>       if (pciReAttachDevice(pci, NULL)<  0)
>           goto out;
>