[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH] Fix QEMU driver custom domain status XML extensions



On Mon, Jan 18, 2010 at 05:16:39PM +0000, Daniel P. Berrange wrote:
> Invoking the virConnectGetCapabilities() method causes the QEMU
> driver to rebuild its internal capabilities object. Unfortunately
> it was forgetting to register the custom domain status XML hooks
> again.
> 
> To avoid this kind of error in the future, the code which builds
> capabilities is refactored into one single method, which can be
> called from all locations, ensuring reliable rebuilds.
> 
> * src/qemu/qemu_driver.c: Fix rebuilding of capabilities XML and
>   guarentee it is always consistent
> ---
>  src/qemu/qemu_driver.c |  110 +++++++++++++++++++++++-------------------------
>  1 files changed, 53 insertions(+), 57 deletions(-)
> 
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 2d80774..9a3ddfb 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -892,34 +892,6 @@ qemuReconnectDomains(struct qemud_driver *driver)
>  
>  
>  static int
> -qemudSecurityCapsInit(virSecurityDriverPtr secdrv,
> -                      virCapsPtr caps)
> -{
> -    const char *doi, *model;
> -
> -    doi = virSecurityDriverGetDOI(secdrv);
> -    model = virSecurityDriverGetModel(secdrv);
> -
> -    caps->host.secModel.model = strdup(model);
> -    if (!caps->host.secModel.model) {
> -        virReportOOMError(NULL);
> -        return -1;
> -    }
> -
> -    caps->host.secModel.doi = strdup(doi);
> -    if (!caps->host.secModel.doi) {
> -        virReportOOMError(NULL);
> -        return -1;
> -    }
> -
> -    VIR_DEBUG("Initialized caps for security driver \"%s\" with "
> -              "DOI \"%s\"", model, doi);
> -
> -    return 0;
> -}
> -
> -
> -static int
>  qemudSecurityInit(struct qemud_driver *qemud_drv)
>  {
>      int ret;
> @@ -940,15 +912,52 @@ qemudSecurityInit(struct qemud_driver *qemud_drv)
>      qemud_drv->securityDriver = security_drv;
>  
>      VIR_INFO("Initialized security driver %s", security_drv->name);
> -
> -    /*
> -     * Add security policy host caps now that the security driver is
> -     * initialized.
> -     */
> -    return qemudSecurityCapsInit(security_drv, qemud_drv->caps);
> +    return 0;
>  }
>  
>  
> +static virCapsPtr
> +qemuCreateCapabilities(virCapsPtr oldcaps,
> +                       virSecurityDriverPtr secDriver)
> +{
> +    virCapsPtr caps;
> +
> +    /* Basic host arch / guest machine capabilities */
> +    if (!(caps = qemudCapsInit(oldcaps))) {
> +        virReportOOMError(NULL);
> +        return NULL;
> +    }
> +
> +    /* Domain XML parser hooks */
> +    caps->privateDataAllocFunc = qemuDomainObjPrivateAlloc;
> +    caps->privateDataFreeFunc = qemuDomainObjPrivateFree;
> +    caps->privateDataXMLFormat = qemuDomainObjPrivateXMLFormat;
> +    caps->privateDataXMLParse = qemuDomainObjPrivateXMLParse;
> +
> +
> +    /* Security driver data */
> +    if (secDriver) {
> +        const char *doi, *model;
> +
> +        doi = virSecurityDriverGetDOI(secDriver);
> +        model = virSecurityDriverGetModel(secDriver);
> +
> +        if (!(caps->host.secModel.model = strdup(model)))
> +            goto no_memory;
> +        if (!(caps->host.secModel.doi = strdup(doi)))
> +            goto no_memory;
> +
> +        VIR_DEBUG("Initialized caps for security driver \"%s\" with "
> +                  "DOI \"%s\"", model, doi);
> +    }
> +
> +    return caps;
> +
> +no_memory:
> +    virReportOOMError(NULL);
> +    virCapabilitiesFree(caps);
> +    return NULL;
> +}
>  
>  /**
>   * qemudStartup:
> @@ -1074,13 +1083,12 @@ qemudStartup(int privileged) {
>                   virStrerror(-rc, buf, sizeof(buf)));
>      }
>  
> -    if ((qemu_driver->caps = qemudCapsInit(NULL)) == NULL)
> -        goto out_of_memory;
> +    if (qemudSecurityInit(qemu_driver) < 0)
> +        goto error;
>  
> -    qemu_driver->caps->privateDataAllocFunc = qemuDomainObjPrivateAlloc;
> -    qemu_driver->caps->privateDataFreeFunc = qemuDomainObjPrivateFree;
> -    qemu_driver->caps->privateDataXMLFormat = qemuDomainObjPrivateXMLFormat;
> -    qemu_driver->caps->privateDataXMLParse = qemuDomainObjPrivateXMLParse;
> +    if ((qemu_driver->caps = qemuCreateCapabilities(NULL,
> +                                                    qemu_driver->securityDriver)) == NULL)
> +        goto error;
>  
>      if ((qemu_driver->activePciHostdevs = pciDeviceListNew(NULL)) == NULL)
>          goto error;
> @@ -1104,10 +1112,6 @@ qemudStartup(int privileged) {
>          }
>      }
>  
> -    if (qemudSecurityInit(qemu_driver) < 0) {
> -        goto error;
> -    }
> -
>      /* If hugetlbfs is present, then we need to create a sub-directory within
>       * it, since we can't assume the root mount point has permissions that
>       * will let our spawned QEMU instances use it.
> @@ -3255,15 +3259,12 @@ static char *qemudGetCapabilities(virConnectPtr conn) {
>      char *xml = NULL;
>  
>      qemuDriverLock(driver);
> -    if ((caps = qemudCapsInit(qemu_driver->caps)) == NULL)
> -        goto no_memory;
>  
> -    caps->privateDataAllocFunc = qemuDomainObjPrivateAlloc;
> -    caps->privateDataFreeFunc = qemuDomainObjPrivateFree;
> -
> -    if (qemu_driver->securityDriver &&
> -        qemudSecurityCapsInit(qemu_driver->securityDriver, caps) < 0)
> -        goto no_memory;
> +    if ((caps = qemuCreateCapabilities(qemu_driver->caps,
> +                                       qemu_driver->securityDriver)) == NULL) {
> +        virCapabilitiesFree(caps);
> +        goto cleanup;
> +    }
>  
>      virCapabilitiesFree(qemu_driver->caps);
>      qemu_driver->caps = caps;
> @@ -3275,11 +3276,6 @@ cleanup:
>      qemuDriverUnlock(driver);
>  
>      return xml;
> -
> -no_memory:
> -    virCapabilitiesFree(caps);
> -    virReportOOMError(conn);
> -    goto cleanup;
>  }

  ACK, this looks cleaner 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/


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]