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

Re: [libvirt] [PATCH v3 RESEND 12/12] qemu_migration: Unlink pre-created storage on error



On Mon, Feb 18, 2013 at 15:38:45 +0100, Michal Privoznik wrote:
> If migratioin fails because of whatever reason and we've
> pre-created any disks, we should remove them instead of letting
> them lying around. Moreover, we need to save the disks sources
> into domain status file in case libvirtd gets restarted.
> ---
>  src/qemu/qemu_domain.c    | 30 ++++++++++++++++++++++++++++--
>  src/qemu/qemu_domain.h    |  2 ++
>  src/qemu/qemu_migration.c | 13 +++++++++++++
>  src/qemu/qemu_process.c   |  8 ++++++++
>  4 files changed, 51 insertions(+), 2 deletions(-)
> 
> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
> index 8df2739..66e0d82 100644
> --- a/src/qemu/qemu_domain.c
> +++ b/src/qemu/qemu_domain.c
> @@ -231,10 +231,15 @@ error:
>  
>  static void qemuDomainObjPrivateFree(void *data)
>  {
> +    size_t i;

int or unsigned would also work :-)

>      qemuDomainObjPrivatePtr priv = data;
>  
>      virObjectUnref(priv->qemuCaps);
>  
> +    for (i = 0; i < priv->nnbdDisk; i++)
> +        VIR_FREE(priv->nbdDisk[i]);
> +    VIR_FREE(priv->nbdDisk);
> +
>      qemuDomainPCIAddressSetFree(priv->pciaddrs);
>      virDomainChrSourceDefFree(priv->monConfig);
>      qemuDomainObjFreeJob(priv);
> @@ -263,6 +268,7 @@ static int qemuDomainObjPrivateXMLFormat(virBufferPtr buf, void *data)
>      qemuDomainObjPrivatePtr priv = data;
>      const char *monitorpath;
>      enum qemuDomainJob job;
> +    size_t i;

int or unsigned would also work :-)

>  
>      /* priv->monitor_chr is set only for qemu */
>      if (priv->monConfig) {
...
> @@ -473,6 +484,21 @@ static int qemuDomainObjPrivateXMLParse(xmlXPathContextPtr ctxt, void *data)
>  
>      priv->fakeReboot = virXPathBoolean("boolean(./fakereboot)", ctxt) == 1;
>  
> +    n = virXPathNodeSet("./nbdDisk/disk", ctxt, &nodes);
> +    if (n < 0)
> +        goto error;
> +    if (n) {
> +        if (VIR_REALLOC_N(priv->nbdDisk, n) < 0) {
> +            virReportOOMError();
> +            goto error;
> +        }
> +        priv->nnbdDisk = n;
> +
> +        for (i = 0; i < n; i++)
> +            if (!(priv->nbdDisk[i] = virXMLPropString(nodes[i], "src")))
> +                goto error;

Ouch, for some reason I though virXMLPropString does not strdup the
attribute value. But looking at xmlGetProp (which virXMLPropString is
just a wrapper for) the caller is responsible for freeing the memory.
Thus your code is correct and we have a lot of memory leaks in our XML
parsers :-(

> +    }
> +
>      return 0;
>  
>  error:

ACK

Jirka


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