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 <bogorodskiy@gmail.com> --- 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 <pkrempa@redhat.com>