[PATCH 0/2] bhyve: implement the virDomainRename API
Roman Bogorodskiy (2): bhyve: implement the virDomainRename API qemu: qemuDomainRenameCallback: remove unused variables src/bhyve/bhyve_domain.c | 27 +++++++++ src/bhyve/bhyve_domain.h | 1 + src/bhyve/bhyve_driver.c | 122 +++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_driver.c | 4 -- 4 files changed, 150 insertions(+), 4 deletions(-) -- 2.52.0
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(+) diff --git a/src/bhyve/bhyve_domain.c b/src/bhyve/bhyve_domain.c index 86e4de1fd8..d5bb22501c 100644 --- a/src/bhyve/bhyve_domain.c +++ b/src/bhyve/bhyve_domain.c @@ -703,3 +703,30 @@ virBhyveDomainObjStopWorker(virDomainObj *dom) g_object_unref(eventThread); virObjectLock(dom); } + +int +bhyveDomainNamePathsCleanup(const char *name, + bool bestEffort) +{ + g_autofree char *cfg_file = NULL; + g_autofree char *autostart_link = NULL; + + cfg_file = virDomainConfigFile(BHYVE_CONFIG_DIR, name); + autostart_link = virDomainConfigFile(BHYVE_AUTOSTART_DIR, name); + + if (virFileExists(cfg_file) && + unlink(cfg_file) < 0) { + virReportSystemError(errno, _("Failed to unlink '%1$s'"), cfg_file); + if (!bestEffort) + return -1; + } + + if (virFileIsLink(autostart_link) == 1 && + unlink(autostart_link) < 0) { + virReportSystemError(errno, _("Failed to unlink '%1$s'"), autostart_link); + if (!bestEffort) + return -1; + } + + return 0; +} diff --git a/src/bhyve/bhyve_domain.h b/src/bhyve/bhyve_domain.h index 888ef2f84b..fa7a685d59 100644 --- a/src/bhyve/bhyve_domain.h +++ b/src/bhyve/bhyve_domain.h @@ -54,3 +54,4 @@ extern virXMLNamespace virBhyveDriverDomainXMLNamespace; int virBhyveDomainObjStartWorker(virDomainObj *dom); void virBhyveDomainObjStopWorker(virDomainObj *dom); +int bhyveDomainNamePathsCleanup(const char *name, bool bestEffort); diff --git a/src/bhyve/bhyve_driver.c b/src/bhyve/bhyve_driver.c index 45e8aad5b5..37a7617346 100644 --- a/src/bhyve/bhyve_driver.c +++ b/src/bhyve/bhyve_driver.c @@ -2683,6 +2683,127 @@ bhyveDomainAuthorizedSSHKeysSet(virDomainPtr domain, return rv; } +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; + + 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); + 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); + virErrorRestore(&err); + return ret; +} + +static int +bhyveDomainRename(virDomainPtr domain, + const char *new_name, + unsigned int flags) +{ + struct _bhyveConn *privconn = domain->conn->privateData; + virDomainObj *vm = NULL; + int ret = -1; + + virCheckFlags(0, ret); + + if (!(vm = bhyveDomObjFromDomain(domain))) + goto cleanup; + + if (virDomainRenameEnsureACL(domain->conn, vm->def) < 0) + goto cleanup; + + if (virDomainObjBeginJob(vm, VIR_JOB_MODIFY) < 0) + goto cleanup; + + if (!vm->persistent) { + virReportError(VIR_ERR_OPERATION_INVALID, "%s", + _("cannot rename a transient domain")); + goto endjob; + } + + if (virDomainObjGetState(vm, NULL) != VIR_DOMAIN_SHUTOFF) { + virReportError(VIR_ERR_OPERATION_INVALID, + "%s", _("domain has to be shutoff before renaming")); + goto endjob; + } + + if (virDomainObjListRename(privconn->domains, vm, new_name, flags, + bhyveDomainRenameCallback, privconn) < 0) + goto endjob; + + /* Success, domain has been renamed. */ + ret = 0; + + endjob: + virDomainObjEndJob(vm); + + cleanup: + virDomainObjEndAPI(&vm); + return ret; +} + static virHypervisorDriver bhyveHypervisorDriver = { .name = "bhyve", .connectURIProbe = bhyveConnectURIProbe, @@ -2760,6 +2881,7 @@ static virHypervisorDriver bhyveHypervisorDriver = { .domainSetUserPassword = bhyveDomainSetUserPassword, /* 12.6.0 */ .domainAuthorizedSSHKeysGet = bhyveDomainAuthorizedSSHKeysGet, /* 12.6.0 */ .domainAuthorizedSSHKeysSet = bhyveDomainAuthorizedSSHKeysSet, /* 12.6.0 */ + .domainRename = bhyveDomainRename, /* 12.6.0 */ }; -- 2.52.0
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>
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
On Fri, Jul 10, 2026 at 19:06:31 +0200, Roman Bogorodskiy wrote:
Peter Krempa wrote:
[...]
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.
If you have the motivation to fix that one too go ahead. It definitely is not a requirement for your implementation.
The old_dom_cfg_file and old_dom_autostart_link variables are assigned to, but never really used, so remove them. Signed-off-by: Roman Bogorodskiy <bogorodskiy@gmail.com> --- src/qemu/qemu_driver.c | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 8ec9e1f9c4..3d57de389b 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -19029,9 +19029,7 @@ qemuDomainRenameCallback(virDomainObj *vm, 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 *old_dom_cfg_file = NULL; g_autofree char *new_dom_autostart_link = NULL; - g_autofree char *old_dom_autostart_link = NULL; struct qemuDomainMomentWriteMetadataData data = { .driver = driver, .vm = vm, @@ -19050,14 +19048,12 @@ qemuDomainRenameCallback(virDomainObj *vm, new_dom_name = g_strdup(new_name); new_dom_cfg_file = virDomainConfigFile(cfg->configDir, new_dom_name); - old_dom_cfg_file = virDomainConfigFile(cfg->configDir, vm->def->name); if (qemuDomainNamePathsCleanup(cfg, new_name, false) < 0) goto cleanup; if (vm->autostart) { new_dom_autostart_link = virDomainConfigFile(cfg->autostartDir, new_dom_name); - old_dom_autostart_link = virDomainConfigFile(cfg->autostartDir, vm->def->name); if (symlink(new_dom_cfg_file, new_dom_autostart_link) < 0) { virReportSystemError(errno, -- 2.52.0
On Sat, Jul 04, 2026 at 11:18:26 +0200, Roman Bogorodskiy wrote:
The old_dom_cfg_file and old_dom_autostart_link variables are assigned to, but never really used, so remove them.
Signed-off-by: Roman Bogorodskiy <bogorodskiy@gmail.com> --- src/qemu/qemu_driver.c | 4 ---- 1 file changed, 4 deletions(-)
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
participants (2)
-
Peter Krempa -
Roman Bogorodskiy