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

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



On 02/06/2013 08:52 AM, 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 d7d7163..6090623 100644
> --- a/src/qemu/qemu_domain.c
> +++ b/src/qemu/qemu_domain.c
> @@ -232,10 +232,15 @@ error:
>  
>  static void qemuDomainObjPrivateFree(void *data)
>  {
> +    size_t i;
>      qemuDomainObjPrivatePtr priv = data;
>  
>      virObjectUnref(priv->caps);
>  
> +    for (i = 0; i < priv->nnbdDisk; i++)
> +        VIR_FREE(priv->nbdDisk[i]);
> +    VIR_FREE(priv->nbdDisk);
> +
>      qemuDomainPCIAddressSetFree(priv->pciaddrs);
>      virDomainChrSourceDefFree(priv->monConfig);
>      qemuDomainObjFreeJob(priv);
> @@ -264,6 +269,7 @@ static int qemuDomainObjPrivateXMLFormat(virBufferPtr buf, void *data)
>      qemuDomainObjPrivatePtr priv = data;
>      const char *monitorpath;
>      enum qemuDomainJob job;
> +    size_t i;
>  
>      /* priv->monitor_chr is set only for qemu */
>      if (priv->monConfig) {
> @@ -286,7 +292,6 @@ static int qemuDomainObjPrivateXMLFormat(virBufferPtr buf, void *data)
>  
>  
>      if (priv->nvcpupids) {
> -        int i;
>          virBufferAddLit(buf, "  <vcpus>\n");
>          for (i = 0 ; i < priv->nvcpupids ; i++) {
>              virBufferAsprintf(buf, "    <vcpu pid='%d'/>\n", priv->vcpupids[i]);
> @@ -295,7 +300,6 @@ static int qemuDomainObjPrivateXMLFormat(virBufferPtr buf, void *data)
>      }
>  
>      if (priv->caps) {
> -        int i;
>          virBufferAddLit(buf, "  <qemuCaps>\n");
>          for (i = 0 ; i < QEMU_CAPS_LAST ; i++) {
>              if (qemuCapsGet(priv->caps, i)) {
> @@ -329,6 +333,13 @@ static int qemuDomainObjPrivateXMLFormat(virBufferPtr buf, void *data)
>      if (priv->fakeReboot)
>          virBufferAsprintf(buf, "  <fakereboot/>\n");
>  
> +    if (priv->nnbdDisk) {
> +        virBufferAddLit(buf, "  <nbdDisk>\n");
> +        for (i = 0; i < priv->nnbdDisk; i++)
> +            virBufferAsprintf(buf, "    <disk src='%s'/>\n", priv->nbdDisk[i]);
> +        virBufferAddLit(buf, "  </nbdDisk>\n");
> +    }
> +
>      return 0;
>  }
>  
> @@ -474,6 +485,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;

If we fail in here, then nbdDisk[] entries from i -> n will be empty.
Will this cause issues elsewhere (such as qemuProcessStop() below)
because other code assumes each nnbdDisk entry is populated with something?

Also I think the for() needs {} since we have multiple lines following.

> +    }
> +
>      return 0;
>  
>  error:
> diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
> index e8555d3..3548a51 100644
> --- a/src/qemu/qemu_domain.h
> +++ b/src/qemu/qemu_domain.h
> @@ -155,6 +155,8 @@ struct _qemuDomainObjPrivate {
>      unsigned long migMaxBandwidth;
>      char *origname;
>      int nbdPort; /* Port used for migration with NBD */
> +    size_t nnbdDisk;
> +    char **nbdDisk; /* src of disks we want to transfer via NBD */
>  
>      virChrdevsPtr devs;
>  
> diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
> index f3edd51..1379b23 100644
> --- a/src/qemu/qemu_migration.c
> +++ b/src/qemu/qemu_migration.c
> @@ -1561,6 +1561,7 @@ qemuMigrationPreCreateStorage(virQEMUDriverPtr driver,
>                                virDomainObjPtr vm,
>                                qemuMigrationCookiePtr mig)
>  {
> +    qemuDomainObjPrivatePtr priv = vm->privateData;
>      int ret = -1;
>      size_t i = 0;
>  
> @@ -1607,6 +1608,11 @@ qemuMigrationPreCreateStorage(virQEMUDriverPtr driver,
>              goto cleanup;
>          }
>  
> +        if ((VIR_REALLOC_N(priv->nbdDisk, priv->nnbdDisk + 1) < 0) ||
> +            !(priv->nbdDisk[priv->nnbdDisk++] = strdup(disk->src))) {
> +            virReportOOMError();
> +            goto cleanup;
> +        }
>      }
>  
>      ret = 0;
> @@ -4042,6 +4048,7 @@ qemuMigrationFinish(virQEMUDriverPtr driver,
>      int cookie_flags = 0;
>      qemuDomainObjPrivatePtr priv = vm->privateData;
>      virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
> +    size_t i;
>  
>      VIR_DEBUG("driver=%p, dconn=%p, vm=%p, cookiein=%s, cookieinlen=%d, "
>                "cookieout=%p, cookieoutlen=%p, flags=%lx, retcode=%d",
> @@ -4192,6 +4199,12 @@ qemuMigrationFinish(virQEMUDriverPtr driver,
>                                                   VIR_DOMAIN_EVENT_SUSPENDED,
>                                                   VIR_DOMAIN_EVENT_SUSPENDED_PAUSED);
>              }
> +
> +            /* Free disks transferred via NBD so they don't get deleted */
> +            for (i = 0; i < priv->nnbdDisk; i++)
> +                VIR_FREE(priv->nbdDisk[i]);
> +            VIR_FREE(priv->nbdDisk);
> +            priv->nnbdDisk = 0;
>          }
>  
>          if (virDomainObjIsActive(vm) &&
> diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
> index 2c57e0e..73420e3 100644
> --- a/src/qemu/qemu_process.c
> +++ b/src/qemu/qemu_process.c
> @@ -4245,6 +4245,14 @@ void qemuProcessStop(virQEMUDriverPtr driver,
>          priv->nbdPort = 0;
>      }
>  
> +    for (i = 0; i < priv->nnbdDisk; i++) {
> +        VIR_DEBUG("Unlinking %s due to unsuccessful NBD transfer",
> +                  priv->nbdDisk[i]);
> +        if (unlink(priv->nbdDisk[i]) < 0)
> +            virReportSystemError(errno, _("Unable to unlink %s"),
> +                                 priv->nbdDisk[i]);
> +    }
> +
>      if (priv->agent) {
>          qemuAgentClose(priv->agent);
>          priv->agent = NULL;
> 


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