[libvirt] [PATCH v2 1/5] Introduce virDomainRename API

[Michal Privoznik]

On 06.08.2015 12:21, Tomas Meszaros wrote:
> Also, among with this new API new ACL that restricts rename capability
> is invented too.
> 
> Signed-off-by: Tomas Meszaros <exo at tty.sk>
> ---
>  include/libvirt/libvirt-domain.h |  2 ++
>  src/access/viraccessperm.c       |  3 ++-
>  src/access/viraccessperm.h       |  6 ++++++
>  src/driver-hypervisor.h          |  5 +++++
>  src/libvirt-domain.c             | 31 +++++++++++++++++++++++++++++++
>  src/libvirt_public.syms          |  5 +++++
>  src/remote/remote_driver.c       |  1 +
>  src/remote/remote_protocol.x     | 17 ++++++++++++++++-
>  src/remote_protocol-structs      |  8 ++++++++
>  9 files changed, 76 insertions(+), 2 deletions(-)
> 
> diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h
> index e8202cf..2ddc47d 100644
> --- a/include/libvirt/libvirt-domain.h
> +++ b/include/libvirt/libvirt-domain.h
> @@ -3837,4 +3837,6 @@ int virDomainSetUserPassword(virDomainPtr dom,
>                               const char *password,
>                               unsigned int flags);
> 
> +int virDomainRename(virDomainPtr dom, const char *new_name);
> +
>  #endif /* __VIR_LIBVIRT_DOMAIN_H__ */
> diff --git a/src/access/viraccessperm.c b/src/access/viraccessperm.c
> index 0f58290..bdc7f60 100644
> --- a/src/access/viraccessperm.c
> +++ b/src/access/viraccessperm.c
> @@ -43,7 +43,8 @@ VIR_ENUM_IMPL(virAccessPermDomain,
>                "fs_trim", "fs_freeze",
>                "block_read", "block_write", "mem_read",
>                "open_graphics", "open_device", "screenshot",
> -              "open_namespace", "set_time", "set_password");
> +              "open_namespace", "set_time", "set_password",
> +              "rename");
> 

This ^^ ..

>  VIR_ENUM_IMPL(virAccessPermInterface,
>                VIR_ACCESS_PERM_INTERFACE_LAST,
> diff --git a/src/access/viraccessperm.h b/src/access/viraccessperm.h
> index 1817da7..6ae4ee7 100644
> --- a/src/access/viraccessperm.h
> +++ b/src/access/viraccessperm.h
> @@ -306,6 +306,12 @@ typedef enum {
>       */
>      VIR_ACCESS_PERM_DOMAIN_SET_PASSWORD,
> 
> +    /**
> +     * @desc: Rename domain
> +     * @message: Renaming the domain requires authorization
> +     */
> +    VIR_ACCESS_PERM_DOMAIN_RENAME,
> +

.. and this change ^^ are not required since the new perm is not used
anywhere.

>      VIR_ACCESS_PERM_DOMAIN_LAST,
>  } virAccessPermDomain;
> 
> diff --git a/src/driver-hypervisor.h b/src/driver-hypervisor.h
> index 3275343..e8c8c2a 100644
> --- a/src/driver-hypervisor.h
> +++ b/src/driver-hypervisor.h
> @@ -650,6 +650,10 @@ typedef int
>  (*virDrvDomainIsActive)(virDomainPtr dom);
> 
>  typedef int
> +(*virDrvDomainRename)(virDomainPtr dom,
> +                      const char *new_name);
> +
> +typedef int
>  (*virDrvDomainIsPersistent)(virDomainPtr dom);
> 
>  typedef int
> @@ -1347,6 +1351,7 @@ struct _virHypervisorDriver {
>      virDrvConnectIsEncrypted connectIsEncrypted;
>      virDrvConnectIsSecure connectIsSecure;
>      virDrvDomainIsActive domainIsActive;
> +    virDrvDomainRename domainRename;
>      virDrvDomainIsPersistent domainIsPersistent;
>      virDrvDomainIsUpdated domainIsUpdated;
>      virDrvConnectCompareCPU connectCompareCPU;
> diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
> index 837933f..c200965 100644
> --- a/src/libvirt-domain.c
> +++ b/src/libvirt-domain.c
> @@ -8774,6 +8774,37 @@ virDomainIsPersistent(virDomainPtr dom)
>      return -1;
>  }
> 
> +/**
> + * virDomainRename:
> + * @dom: pointer to the domain object
> + * @new_name: new domain name
> + *
> + * Rename an inactive domain. New domain name is specified in the second
> + * argument.

I wouldn't state here that only an inactive domain can be renamed. It's
just a limitation of current implementation which may change in the
future. If you want to be more verbose (and I guess it's desirable here)
you may say that depending on each driver implementation it may be
required that domain is in a specific state.

> + *
> + * Returns 0 if renamed, -1 on error
> + */
> +int
> +virDomainRename(virDomainPtr dom, const char *new_name)

We really, really need to have 'unsigned int flags' here. Even though it
is not currently needed, it may be in the future. Every new API should
have it.

> +{
> +    VIR_DEBUG("dom=%p, new_name=%s", dom, NULLSTR(new_name));
> +
> +    virResetLastError();
> +    virCheckDomainReturn(dom, -1);
> +    virCheckNonNullArgGoto(new_name, error);
> +
> +    if (dom->conn->driver->domainRename) {
> +        int ret = dom->conn->driver->domainRename(dom, new_name);
> +        if (ret < 0)
> +            goto error;
> +        return ret;
> +    }
> +
> +    virReportUnsupportedError();
> + error:
> +    virDispatchError(dom->conn);
> +    return -1;
> +}
> 

Michal