[PATCH 0/3] various cleanups
Cleanups of stuff noticed during recent review of backup NBD port allocation. Peter Krempa (3): qemu: backup: Move setup of VIR_STORAGE_NET_HOST_TRANS_FD to qemuBackupPrepare qemu: conf: Don't use VIR_ERR_INTERNAL_ERROR for config file parsing errors qemuTranslateSnapshotDiskSourcePool: Use proper error code src/qemu/qemu_backup.c | 52 +++++++++++++++++++++--------------------- src/qemu/qemu_conf.c | 22 +++++++++--------- 2 files changed, 37 insertions(+), 37 deletions(-) -- 2.53.0
From: Peter Krempa <pkrempa@redhat.com> Consolidate the code under qemuBackupPrepare. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_backup.c | 52 +++++++++++++++++++++--------------------- 1 file changed, 26 insertions(+), 26 deletions(-) diff --git a/src/qemu/qemu_backup.c b/src/qemu/qemu_backup.c index ddab4be34b..65a083ea74 100644 --- a/src/qemu/qemu_backup.c +++ b/src/qemu/qemu_backup.c @@ -60,7 +60,8 @@ qemuDomainGetBackup(virDomainObj *vm) static int qemuBackupPrepare(qemuDomainObjPrivate *priv, - virDomainBackupDef *def) + virDomainBackupDef *def, + qemuFDPassDirect **fdpass) { if (def->type == VIR_DOMAIN_BACKUP_TYPE_PULL) { @@ -86,7 +87,29 @@ qemuBackupPrepare(qemuDomainObjPrivate *priv, /* TODO: Do we need to mess with selinux? */ break; - case VIR_STORAGE_NET_HOST_TRANS_FD: + case VIR_STORAGE_NET_HOST_TRANS_FD: { + virDomainFDTuple *fdt = NULL; + VIR_AUTOCLOSE fdcopy = -1; + + if (!(fdt = virHashLookup(priv->fds, def->server->fdgroup))) { + virReportError(VIR_ERR_INVALID_ARG, + _("file descriptor group '%1$s' was not associated with the domain"), + def->server->fdgroup); + return -1; + } + + if (fdt->nfds != 1) { + virReportError(VIR_ERR_INVALID_ARG, + _("file descriptor group '%1$s' must contain only 1 file descriptor for NBD server"), + def->server->fdgroup); + return -1; + } + + def->server->qemu_fdname = g_strdup("libvirt-backup-nbd"); + fdcopy = dup(fdt->fds[0]); + *fdpass = qemuFDPassDirectNew(def->server->qemu_fdname, &fdcopy); + } + break; case VIR_STORAGE_NET_HOST_TRANS_RDMA: @@ -837,7 +860,7 @@ qemuBackupBegin(virDomainObj *vm, goto endjob; } - if (qemuBackupPrepare(priv, def) < 0) + if (qemuBackupPrepare(priv, def, &fdpass) < 0) goto endjob; if (qemuBackupBeginPrepareTLS(vm, cfg, def, &tlsProps, &tlsSecretProps) < 0) @@ -874,29 +897,6 @@ qemuBackupBegin(virDomainObj *vm, priv->backup = g_steal_pointer(&def); - if (pull && priv->backup->server->fdgroup) { - virDomainFDTuple *fdt = NULL; - VIR_AUTOCLOSE fdcopy = -1; - - if (!(fdt = virHashLookup(priv->fds, priv->backup->server->fdgroup))) { - virReportError(VIR_ERR_INVALID_ARG, - _("file descriptor group '%1$s' was not associated with the domain"), - priv->backup->server->fdgroup); - goto endjob; - } - - if (fdt->nfds != 1) { - virReportError(VIR_ERR_INVALID_ARG, - _("file descriptor group '%1$s' must contain only 1 file descriptor for NBD server"), - priv->backup->server->fdgroup); - goto endjob; - } - - priv->backup->server->qemu_fdname = g_strdup("libvirt-backup-nbd"); - fdcopy = dup(fdt->fds[0]); - fdpass = qemuFDPassDirectNew(priv->backup->server->qemu_fdname, &fdcopy); - } - if (qemuDomainObjEnterMonitorAsync(vm, VIR_ASYNC_JOB_BACKUP) < 0) goto endjob; -- 2.53.0
From: Peter Krempa <pkrempa@redhat.com> When parsing port ranges for the port allocator VIR_ERR_INTERNAL_ERROR is not the right error code for errors on the user-supplied numbers. Use VIR_ERR_CONF_SYNTAX instead. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_conf.c | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index d43e0c2b97..712422f995 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -626,7 +626,7 @@ virQEMUDriverConfigLoadRemoteDisplayEntry(virQEMUDriverConfig *cfg, /* if the port is too low, we can't get the display name * to tell to vnc (usually subtract 5700, e.g. localhost:1 * for port 5701) */ - virReportError(VIR_ERR_INTERNAL_ERROR, + virReportError(VIR_ERR_CONF_SYNTAX, _("%1$s: remote_websocket_port_min: port must be greater than or equal to %2$d"), filename, QEMU_WEBSOCKET_PORT_MIN); return -1; @@ -636,14 +636,14 @@ virQEMUDriverConfigLoadRemoteDisplayEntry(virQEMUDriverConfig *cfg, return -1; if (cfg->webSocketPortMax > QEMU_WEBSOCKET_PORT_MAX || cfg->webSocketPortMax < cfg->webSocketPortMin) { - virReportError(VIR_ERR_INTERNAL_ERROR, + virReportError(VIR_ERR_CONF_SYNTAX, _("%1$s: remote_websocket_port_max: port must be between the minimal port and %2$d"), filename, QEMU_WEBSOCKET_PORT_MAX); return -1; } if (cfg->webSocketPortMin > cfg->webSocketPortMax) { - virReportError(VIR_ERR_INTERNAL_ERROR, + virReportError(VIR_ERR_CONF_SYNTAX, _("%1$s: remote_websocket_port_min: min port must not be greater than max port"), filename); return -1; @@ -655,7 +655,7 @@ virQEMUDriverConfigLoadRemoteDisplayEntry(virQEMUDriverConfig *cfg, /* if the port is too low, we can't get the display name * to tell to vnc (usually subtract 5900, e.g. localhost:1 * for port 5901) */ - virReportError(VIR_ERR_INTERNAL_ERROR, + virReportError(VIR_ERR_CONF_SYNTAX, _("%1$s: remote_display_port_min: port must be greater than or equal to %2$d"), filename, QEMU_REMOTE_PORT_MIN); return -1; @@ -665,14 +665,14 @@ virQEMUDriverConfigLoadRemoteDisplayEntry(virQEMUDriverConfig *cfg, return -1; if (cfg->remotePortMax > QEMU_REMOTE_PORT_MAX || cfg->remotePortMax < cfg->remotePortMin) { - virReportError(VIR_ERR_INTERNAL_ERROR, + virReportError(VIR_ERR_CONF_SYNTAX, _("%1$s: remote_display_port_max: port must be between the minimal port and %2$d"), filename, QEMU_REMOTE_PORT_MAX); return -1; } if (cfg->remotePortMin > cfg->remotePortMax) { - virReportError(VIR_ERR_INTERNAL_ERROR, + virReportError(VIR_ERR_CONF_SYNTAX, _("%1$s: remote_display_port_min: min port must not be greater than max port"), filename); return -1; @@ -979,7 +979,7 @@ virQEMUDriverConfigLoadNetworkEntry(virQEMUDriverConfig *cfg, if (virConfGetValueUInt(conf, "migration_port_min", &cfg->migrationPortMin) < 0) return -1; if (cfg->migrationPortMin <= 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, + virReportError(VIR_ERR_CONF_SYNTAX, _("%1$s: migration_port_min: port must be greater than 0"), filename); return -1; @@ -989,7 +989,7 @@ virQEMUDriverConfigLoadNetworkEntry(virQEMUDriverConfig *cfg, return -1; if (cfg->migrationPortMax > 65535 || cfg->migrationPortMax < cfg->migrationPortMin) { - virReportError(VIR_ERR_INTERNAL_ERROR, + virReportError(VIR_ERR_CONF_SYNTAX, _("%1$s: migration_port_max: port must be between the minimal port %2$d and 65535"), filename, cfg->migrationPortMin); return -1; @@ -998,7 +998,7 @@ virQEMUDriverConfigLoadNetworkEntry(virQEMUDriverConfig *cfg, if (virConfGetValueUInt(conf, "backup_port_min", &cfg->backupPortMin) < 0) return -1; if (cfg->backupPortMin <= 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, + virReportError(VIR_ERR_CONF_SYNTAX, _("%1$s: backup_port_min: port must be greater than 0"), filename); return -1; @@ -1008,7 +1008,7 @@ virQEMUDriverConfigLoadNetworkEntry(virQEMUDriverConfig *cfg, return -1; if (cfg->backupPortMax > 65535 || cfg->backupPortMax < cfg->backupPortMin) { - virReportError(VIR_ERR_INTERNAL_ERROR, + virReportError(VIR_ERR_CONF_SYNTAX, _("%1$s: backup_port_max: port must be between the minimal port %2$d and 65535"), filename, cfg->backupPortMin); return -1; -- 2.53.0
From: Peter Krempa <pkrempa@redhat.com> The operation is not implemented so it's not really an internal error. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_conf.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index 712422f995..b2ced2baf0 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -1808,7 +1808,7 @@ qemuTranslateSnapshotDiskSourcePool(virDomainSnapshotDiskDef *def) if (def->src->type != VIR_STORAGE_TYPE_VOLUME) return 0; - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", _("Snapshots are not yet supported with 'pool' volumes")); return -1; } -- 2.53.0
On a Friday in 2026, Peter Krempa via Devel wrote:
Cleanups of stuff noticed during recent review of backup NBD port allocation.
Peter Krempa (3): qemu: backup: Move setup of VIR_STORAGE_NET_HOST_TRANS_FD to qemuBackupPrepare qemu: conf: Don't use VIR_ERR_INTERNAL_ERROR for config file parsing errors qemuTranslateSnapshotDiskSourcePool: Use proper error code
src/qemu/qemu_backup.c | 52 +++++++++++++++++++++--------------------- src/qemu/qemu_conf.c | 22 +++++++++--------- 2 files changed, 37 insertions(+), 37 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
participants (2)
-
Ján Tomko -
Peter Krempa