Peter Krempa wrote:

> > +    /* Switch name in domain definition. */
> > +    old_dom_name = vm->def->name;
> > +    vm->def->name = new_dom_name;
> > +    new_dom_name = NULL;
> 
> Consider using
> 
> old_dom_name = g_steal_pointer(&vm->def->name);
> vm->def->name = g_steal_pointer(&new_dom_name);
> 
> > +

...

> > +    event_new = virDomainEventLifecycleNewFromObj(vm,
> > +                                              VIR_DOMAIN_EVENT_DEFINED,
> > +                                              
> > VIR_DOMAIN_EVENT_DEFINED_RENAMED);
> 
> Alignment is off.
> 
> 
> > +    virObjectEventStateQueue(privconn->domainEventState, event_old);

...

> > +    bhyveDomainNamePathsCleanup(ret < 0 ? new_dom_name : old_dom_name, 
> > true);
> 
> 
> This error path is convoluted. There's 3 distinct branches triggering on
> ret < 0.
> 
> I suggest you consolidate them. This will cause that
> the call to bhyveDomainNamePathsCleanup will be duplicated in both
> branches but it will be IMO much more clear  what is happening on succes
> sand what on error.

Thanks for the suggestions, applied and merged.

In the meantime, all of these three items are applicable to
qemuDomainRenameCallback(). Please let me know if you want me to address
them there too.

Roman

Reply via email to