Re: [PATCH 1/8] Add new virDomainDefineXMLFlags public API

[Martin Kletzander <mkletzan@xxxxxxxxxx>]

On Tue, Nov 18, 2014 at 05:59:48PM +0000, Daniel P. Berrange wrote:
> The virDomainDefineXML method is one of the few that still lacks
> a 'unsigned int flags' parameter. This will be needed for adding

s/a/an/?

> XML validation to this API. virDomainCreateXML fortunately already
> has flags.
> ---
> include/libvirt/libvirt-domain.h |  4 ++++
> src/driver-hypervisor.h          |  5 +++++
> src/libvirt-domain.c             | 48 ++++++++++++++++++++++++++++++++++++++++
> src/libvirt_public.syms          |  5 +++++
> src/qemu/qemu_driver.c           | 10 ++++++++-
> src/remote/remote_driver.c       |  1 +
> src/remote/remote_protocol.x     | 19 +++++++++++++++-
> 7 files changed, 90 insertions(+), 2 deletions(-)
>
> diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h
> index 1fac2a3..864c16c 100644
> --- a/include/libvirt/libvirt-domain.h
> +++ b/include/libvirt/libvirt-domain.h
> @@ -1415,6 +1415,10 @@ int                     virDomainMemoryPeek (virDomainPtr dom,
>  */
> virDomainPtr            virDomainDefineXML      (virConnectPtr conn,
>                                                  const char *xml);
> +
> +virDomainPtr            virDomainDefineXMLFlags (virConnectPtr conn,
> +                                                 const char *xml,
> +                                                 unsigned int flags);
> int                     virDomainUndefine       (virDomainPtr domain);
>
> typedef enum {
> diff --git a/src/driver-hypervisor.h b/src/driver-hypervisor.h
> index ad66629..f73d40b 100644
> --- a/src/driver-hypervisor.h
> +++ b/src/driver-hypervisor.h
> @@ -314,6 +314,10 @@ typedef int
> typedef virDomainPtr
> (*virDrvDomainDefineXML)(virConnectPtr conn,
>                          const char *xml);
> +typedef virDomainPtr
> +(*virDrvDomainDefineXMLFlags)(virConnectPtr conn,
> +                              const char *xml,
> +                              unsigned int flags);
>
> typedef int
> (*virDrvDomainUndefine)(virDomainPtr dom);
> @@ -1260,6 +1264,7 @@ struct _virHypervisorDriver {
>     virDrvDomainCreateWithFlags domainCreateWithFlags;
>     virDrvDomainCreateWithFiles domainCreateWithFiles;
>     virDrvDomainDefineXML domainDefineXML;
> +    virDrvDomainDefineXMLFlags domainDefineXMLFlags;
>     virDrvDomainUndefine domainUndefine;
>     virDrvDomainUndefineFlags domainUndefineFlags;
>     virDrvDomainAttachDevice domainAttachDevice;
> diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
> index 2b0defc..e9d3608 100644
> --- a/src/libvirt-domain.c
> +++ b/src/libvirt-domain.c
> @@ -6473,6 +6473,54 @@ virDomainDefineXML(virConnectPtr conn, const char *xml)
>
>
> /**
> + * virDomainDefineXMLFlags:
> + * @conn: pointer to the hypervisor connection
> + * @xml: the XML description for the domain, preferably in UTF-8
> + * @flags: bitwise OR of virDomainDefineFlags
> + *
> + * Define a domain, but does not start it.

Either "defines" or "do not", I guess.

> + * This definition is persistent, until explicitly undefined with
> + * virDomainUndefine(). A previous definition for this domain would be
> + * overridden if it already exists.
> + *
> + * Some hypervisors may prevent this operation if there is a current
> + * block copy operation on a transient domain with the same id as the
> + * domain being defined; in that case, use virDomainBlockJobAbort() to
> + * stop the block copy first.
> + *
> + * virDomainFree should be used to free the resources after the
> + * domain object is no longer needed.
> + *
> + * Returns NULL in case of error, a pointer to the domain otherwise
> + */
> +virDomainPtr
> +virDomainDefineXMLFlags(virConnectPtr conn, const char *xml, unsigned int flags)
> +{
> +    VIR_DEBUG("conn=%p, xml=%s flags=%x", conn, xml, flags);
> +
> +    virResetLastError();
> +
> +    virCheckConnectReturn(conn, NULL);
> +    virCheckReadOnlyGoto(conn->flags, error);
> +    virCheckNonNullArgGoto(xml, error);
> +
> +    if (conn->driver->domainDefineXMLFlags) {
> +        virDomainPtr ret;
> +        ret = conn->driver->domainDefineXMLFlags(conn, xml, flags);
> +        if (!ret)
> +            goto error;
> +        return ret;
> +    }
> +
> +    virReportUnsupportedError();
> +
> + error:
> +    virDispatchError(conn);
> +    return NULL;
> +}
> +
> +
> +/**
>  * virDomainUndefine:
>  * @domain: pointer to a defined domain
>  *
> diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms
> index 5f95802..4d9973e 100644
> --- a/src/libvirt_public.syms
> +++ b/src/libvirt_public.syms
> @@ -684,4 +684,9 @@ LIBVIRT_1.2.9 {
>         virNodeAllocPages;
> } LIBVIRT_1.2.8;
>
> +LIBVIRT_1.2.11 {
> +    global:
> +        virDomainDefineXMLFlags;
> +} LIBVIRT_1.2.9;
> +
> # .... define new API here using predicted next version number ....
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index a84fd47..a877b75 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -6651,7 +6651,7 @@ qemuDomainCreate(virDomainPtr dom)
>     return qemuDomainCreateWithFlags(dom, 0);
> }
>
> -static virDomainPtr qemuDomainDefineXML(virConnectPtr conn, const char *xml)
> +static virDomainPtr qemuDomainDefineXMLFlags(virConnectPtr conn, const char *xml, unsigned int flags)

Pretty long line.

Also, this should be joined together with the second patch that
updates remote_protocol-structs.  Other than that it looks fine.

Martin

Attachment: signature.asc
Description: Digital signature

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list