[libvirt] [PATCH v2 1/2] test: Implementing testDomainRename().

[Peter Krempa]

On Mon, Jan 08, 2018 at 01:02:59 -0200, Julio Faracco wrote:
> There is no method to rename inactive domains for test driver.
> After this patch, we can rename the domains using 'domrename'.
> 
>     virsh# domrename test anothertest
>     Domain successfully renamed
> 
> Signed-off-by: Julio Faracco <jcfaracco at gmail.com>
> ---
>  src/test/test_driver.c | 72 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 72 insertions(+)
> 
> diff --git a/src/test/test_driver.c b/src/test/test_driver.c
> index dc743b4..26bc8a2 100644
> --- a/src/test/test_driver.c
> +++ b/src/test/test_driver.c
> @@ -2618,6 +2618,77 @@ testDomainGetVcpuPinInfo(virDomainPtr dom,
>      return ret;
>  }
>  
> +static int
> +testDomainRenameCallback(virDomainObjPtr privdom,
> +                         const char *new_name,
> +                         unsigned int flags,
> +                         void *opaque)
> +{
> +    testDriverPtr driver = opaque;
> +    virObjectEventPtr event_new = NULL;
> +    virObjectEventPtr event_old = NULL;
> +    int ret = -1;
> +    char *new_dom_name = NULL;
> +    char *old_dom_name = NULL;
> +
> +    virCheckFlags(0, ret);
> +
> +    if (VIR_STRDUP(new_dom_name, new_name) < 0)
> +        goto cleanup;
> +
> +    event_old = virDomainEventLifecycleNewFromObj(privdom,
> +                                            VIR_DOMAIN_EVENT_UNDEFINED,
> +                                            VIR_DOMAIN_EVENT_UNDEFINED_RENAMED);
> +
> +    /* Switch name in domain definition. */
> +    old_dom_name = privdom->def->name;
> +    privdom->def->name = new_dom_name;
> +    new_dom_name = NULL;

Since the function below does not check that the VM is inactive at this
point, this code lacks rename of the 'newDef' object which might be
populated. Renaming an active VM which has newDef would then lead to
returing to old name once it turns off.

> +
> +    event_new = virDomainEventLifecycleNewFromObj(privdom,
> +                                              VIR_DOMAIN_EVENT_DEFINED,
> +                                              VIR_DOMAIN_EVENT_DEFINED_RENAMED);
> +    ret = 0;
> +
> + cleanup:
> +    VIR_FREE(old_dom_name);
> +    VIR_FREE(new_dom_name);
> +    testObjectEventQueue(driver, event_old);
> +    testObjectEventQueue(driver, event_new);
> +    return ret;
> +}
> +
> +static int testDomainRename(virDomainPtr dom,
> +                            const char *new_name,
> +                            unsigned int flags)

You are mixing two distinct coding styles of function headers in 1 hunk?

> +{
> +    testDriverPtr driver = dom->conn->privateData;
> +    virDomainObjPtr privdom = NULL;
> +    int ret = -1;
> +
> +    virCheckFlags(0, -1);
> +
> +    if (!(privdom = testDomObjFromDomain(dom)))
> +        goto cleanup;
> +
> +    if (!privdom->persistent) {
> +        virReportError(VIR_ERR_OPERATION_INVALID, "%s",
> +                       _("cannot rename a transient domain"));
> +        goto cleanup;
> +    }

If you are going to support renaming of running VMs, this check does not
make sense.

> +
> +    if (virDomainObjListRename(driver->domains, privdom, new_name, flags,
> +                               testDomainRenameCallback, driver) < 0)
> +        goto cleanup;
> +
> +    /* Success, domain has been renamed. */
> +    ret = 0;
> +
> + cleanup:
> +    virDomainObjEndAPI(&privdom);
> +    return ret;
> +}
> +
>  static char *testDomainGetXMLDesc(virDomainPtr domain, unsigned int flags)
>  {
>      testDriverPtr privconn = domain->conn->privateData;
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20180108/84637393/attachment-0001.sig>