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

Re: [libvirt] [PATCH v3 2/2] libxl: add tunnelled migration support



FYI: Couple of Coverity issues resulted from these patches.

>  int
> +libxlDomainMigrationPrepareTunnel3(virConnectPtr dconn,
> +                                   virStreamPtr st,
> +                                   virDomainDefPtr *def,
> +                                   const char *cookiein,
> +                                   int cookieinlen,
> +                                   unsigned int flags)
> +{
> +    libxlMigrationCookiePtr mig = NULL;
> +    libxlDriverPrivatePtr driver = dconn->privateData;
> +    virDomainObjPtr vm = NULL;
> +    libxlMigrationDstArgs *args = NULL;
> +    virThread thread;
> +    bool taint_hook = false;
> +    libxlDomainObjPrivatePtr priv = NULL;
> +    char *xmlout = NULL;
> +    int dataFD[2] = { -1, -1 };
> +    int ret = -1;
> +
> +    if (libxlDomainMigrationPrepareAny(dconn, def, cookiein, cookieinlen,
> +                                       &mig, &xmlout, &taint_hook) < 0)

Coverity notes that '@mig' will be leaked in this case when "if
(!cookiein || !cookieinlen) {" is true in libxlMigrationEatCookie and
failures from libxlDomainMigrationPrepareAny don't free it.


> +        goto error;
> +
> +    if (!(vm = virDomainObjListAdd(driver->domains, *def,
> +                                   driver->xmlopt,
> +                                   VIR_DOMAIN_OBJ_LIST_ADD_LIVE |
> +                                   VIR_DOMAIN_OBJ_LIST_ADD_CHECK_LIVE,
> +                                   NULL)))
> +        goto error;
> +
> +    *def = NULL;
> +    priv = vm->privateData;
> +
> +    if (taint_hook) {
> +        /* Domain XML has been altered by a hook script. */
> +        priv->hookRun = true;
> +    }
> +
> +    /*
> +     * The data flow of tunnel3 migration in the dest side:
> +     * stream -> pipe -> recvfd of libxlDomainStartRestore
> +     */
> +    if (pipe(dataFD) < 0)
> +        goto error;
> +
> +    /* Stream data will be written to pipeIn */
> +    if (virFDStreamOpen(st, dataFD[1]) < 0)
> +        goto error;
> +    dataFD[1] = -1; /* 'st' owns the FD now & will close it */
> +
> +    if (libxlMigrationDstArgsInitialize() < 0)
> +        goto error;
> +
> +    if (!(args = virObjectNew(libxlMigrationDstArgsClass)))
> +        goto error;
> +
> +    args->conn = dconn;
> +    args->vm = vm;
> +    args->flags = flags;
> +    args->migcookie = mig;
> +    /* Receive from pipeOut */
> +    args->recvfd = dataFD[0];
> +    args->nsocks = 0;
> +    if (virThreadCreate(&thread, false, libxlDoMigrateReceive, args) < 0) {
> +        virReportError(VIR_ERR_OPERATION_FAILED, "%s",
> +                       _("Failed to create thread for receiving migration data"));
> +        goto error;
> +    }
> +
> +    ret = 0;
> +    goto done;
> +
> + error:
> +    VIR_FORCE_CLOSE(dataFD[1]);
> +    VIR_FORCE_CLOSE(dataFD[0]);
> +    virObjectUnref(args);
> +    /* Remove virDomainObj from domain list */
> +    if (vm) {
> +        virDomainObjListRemove(driver->domains, vm);
> +        vm = NULL;
> +    }
> +
> + done:
> +    if (vm)
> +        virObjectUnlock(vm);
> +
> +    return ret;
> +}

[...]

> +
> +static int
> +libxlMigrationStartTunnel(libxlDriverPrivatePtr driver,
> +                          virDomainObjPtr vm,
> +                          unsigned long flags,
> +                          virStreamPtr st,
> +                          struct libxlTunnelControl *tc)
> +{
> +    libxlTunnelMigrationThread *arg = NULL;
> +    int ret = -1;
> +
> +    if (VIR_ALLOC(tc) < 0)
> +        goto out;
> +
> +    tc->dataFD[0] = -1;
> +    tc->dataFD[1] = -1;
> +    if (pipe(tc->dataFD) < 0) {
> +        virReportError(errno, "%s", _("Unable to make pipes"));

Coverity notes that failures would cause a leak for @tc (I assume here
and of course if virThreadCreate fails.  Perhaps the 'best' way to
handle that would be to set tc = NULL after ThreadCreate success and
just call libxlMigrationStopTunnel in "out:"...


John

> +        goto out;
> +    }
> +
> +    arg = &tc->tmThread;
> +    /* Read from pipe */
> +    arg->srcFD = tc->dataFD[0];
> +    /* Write to dest stream */
> +    arg->st = st;
> +    if (virThreadCreate(&tc->thread, true,
> +                        libxlTunnel3MigrationFunc, arg) < 0) {
> +        virReportError(errno, "%s",
> +                       _("Unable to create tunnel migration thread"));
> +        goto out;
> +    }
> +
> +    virObjectUnlock(vm);
> +    /* Send data to pipe */
> +    ret = libxlDoMigrateSend(driver, vm, flags, tc->dataFD[1]);
> +    virObjectLock(vm);
> +
> + out:
> +    /* libxlMigrationStopTunnel will be called in libxlDoMigrateP2P to free
> +     * all resources for us. */
> +    return ret;
> +}
> +

[...]


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