On Mon, Feb 20, 2017 at 15:14:21 -0500, John Ferlan wrote:
>
>
> On 02/20/2017 10:43 AM, Jiri Denemark wrote:
> > On Fri, Feb 17, 2017 at 14:39:21 -0500, John Ferlan wrote:
> >> Introduce API's to Prepare/Destroy a qemuDomainSecretInfoPtr to be
> >> used with a migrate or nbd TLS object
> >>
> >> Signed-off-by: John Ferlan <jferlan@xxxxxxxxxx>
> >> ---
> >> src/qemu/qemu_domain.c | 73 +++++++++++++++++++++++++++++++++++++++++
> >> src/qemu/qemu_domain.h | 88 +++++++++++++++++++++++++++++---------------------
> >> 2 files changed, 124 insertions(+), 37 deletions(-)
> >>
> >> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
> >> index be44843..dd3cfd5 100644
> >> --- a/src/qemu/qemu_domain.c
> >> +++ b/src/qemu/qemu_domain.c
> >> @@ -1370,6 +1370,77 @@ qemuDomainSecretChardevPrepare(virConnectPtr conn,
> >> }
> >>
> >>
> >> +/* qemuDomainSecretMigrateDestroy:
> >> + * @migSecinfo: Pointer to the secinfo from the incoming def
> >> + *
> >> + * Clear and destroy memory associated with the secret
> >> + */
> >> +void
> >> +qemuDomainSecretMigrateDestroy(qemuDomainSecretInfoPtr *migSecinfo)
> >> +{
> >> + if (!*migSecinfo)
> >> + return;
> >> +
> >> + qemuDomainSecretInfoFree(migSecinfo);
> >> +}
> >
> > This is a useless wrapper, please drop it.
> >
>
> Sure - it's like an automatic reaction - have allocation need free
> function... Will use qemuDomainSecretInfoFree instead...
It would make sense if migSecinfo was a new structure, but in this case
we would just end up with two functions usable for freeing a single
structure. Which is just asking for an inconsistent usage. And that's
what happened even in your series; sometimes qemuDomainSecretInfoFree is
used to free migSecinfo and sometime it's the new wrapper you call to do
the job.
> >> +int
> >> +qemuDomainSecretMigratePrepare(virConnectPtr conn,
> >> + qemuDomainObjPrivatePtr priv,
> >> + const char *srcAlias,
> >> + const char *secretUUID)
> >
> > Almost all lines in this functions were just copy-pasted from
> > qemuDomainSecretChardevPrepare. Could you merge the two? Ideally you can
> > just make it a function which lookups the secinfo and you can do the
> > rest in the caller.
>
> Sure. no problem... I'll create:
>
> static qemuDomainSecretInfoPtr
> qemuDomainSecretTLSObjectPrepare(virConnectPtr conn,
> qemuDomainObjPrivatePtr priv,
> const char *srcAlias,
> const char *secretUUID)
Great, but since the structure it creates is qemuDomainSecretInfo, can
you just call it qemuDomainSecretInfoNew or something similar to this?
Jirka
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
