On Sat, Jul 04, 2026 at 11:18:25 +0200, Roman Bogorodskiy wrote:
> Implementation is fairly similar to the one found in the test and qemu
> drivers. Lack of checkpoint and snapshot support in the bhyve driver
> makes the implementation a bit simpler.
>
> Signed-off-by: Roman Bogorodskiy <[email protected]>
> ---
> src/bhyve/bhyve_domain.c | 27 +++++++++
> src/bhyve/bhyve_domain.h | 1 +
> src/bhyve/bhyve_driver.c | 122 +++++++++++++++++++++++++++++++++++++++
> 3 files changed, 150 insertions(+)
[...]
> +static int
> +bhyveDomainRenameCallback(virDomainObj *vm,
> + const char *new_name,
> + unsigned int flags,
> + void *opaque)
> +{
> + struct _bhyveConn *privconn = opaque;
> + virObjectEvent *event_new = NULL;
> + virObjectEvent *event_old = NULL;
> + int ret = -1;
> + virErrorPtr err = NULL;
> + g_autofree char *new_dom_name = NULL;
> + g_autofree char *old_dom_name = NULL;
> + g_autofree char *new_dom_cfg_file = NULL;
> + g_autofree char *new_dom_autostart_link = NULL;
> +
> + virCheckFlags(0, ret);
> +
> + if (strchr(new_name, '/')) {
> + virReportError(VIR_ERR_XML_ERROR,
> + _("name %1$s cannot contain '/'"), new_name);
> + return -1;
> + }
> +
> + new_dom_name = g_strdup(new_name);
> +
> + new_dom_cfg_file = virDomainConfigFile(BHYVE_CONFIG_DIR, new_dom_name);
> +
> + if (bhyveDomainNamePathsCleanup(new_name, false) < 0)
> + goto cleanup;
> +
> + if (vm->autostart) {
> + new_dom_autostart_link = virDomainConfigFile(BHYVE_AUTOSTART_DIR,
> new_dom_name);
> +
> + if (symlink(new_dom_cfg_file, new_dom_autostart_link) < 0) {
> + virReportSystemError(errno,
> + _("Failed to create symlink '%1$s' to
> '%2$s'"),
> + new_dom_autostart_link, new_dom_cfg_file);
> + return -1;
> + }
> + }
> +
> + /* 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);
> +
> + if (virDomainDefSave(vm->def, privconn->xmlopt, BHYVE_CONFIG_DIR) < 0)
> + goto cleanup;
> +
> + event_old = virDomainEventLifecycleNew(vm->def->id, old_dom_name,
> vm->def->uuid,
> + VIR_DOMAIN_EVENT_UNDEFINED,
> +
> VIR_DOMAIN_EVENT_UNDEFINED_RENAMED);
> + event_new = virDomainEventLifecycleNewFromObj(vm,
> + VIR_DOMAIN_EVENT_DEFINED,
> +
> VIR_DOMAIN_EVENT_DEFINED_RENAMED);
Alignment is off.
> + virObjectEventStateQueue(privconn->domainEventState, event_old);
> + virObjectEventStateQueue(privconn->domainEventState, event_new);
> + ret = 0;
> +
> + cleanup:
> + if (old_dom_name && ret < 0) {
> + new_dom_name = vm->def->name;
> + vm->def->name = old_dom_name;
> + old_dom_name = NULL;
> + }
> +
> + if (ret < 0)
> + virErrorPreserveLast(&err);
> + 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.
> + virErrorRestore(&err);
> + return ret;
> +}
Reviewed-by: Peter Krempa <[email protected]>
