Re: [RFC] qemu_migration: Fix virConnectOpenAuth error code

On 1/6/22 13:42, Ani Sinha wrote: > > > On Thu, 6 Jan 2022, Michal Prívozník wrote: > >> On 1/6/22 02:33, Raphael Norwitz wrote: >>> Today if virConnectOpenAuth fails, qemuMigrationSrcPerformPeer2Peer() >>> returns VIR_ERR_OPERATION_FAILED. This change switches that error code >>> to VIR_ERR_NO_CONNECT, which is more accurate. >>> >>> This should help libvirt consumers more intellegently retry migrations >>> on intermittent connection failures. >>> >>> CC: Bhuvnesh Jain <bhuvnesh.jain(a)nutanix.com&gt; >>> Suggested-by: John Levon <john.levon(a)nutanix.com&gt; >>> Signed-off-by: Raphael Norwitz <raphael.norwitz(a)nutanix.com&gt; >>> --- >>> src/qemu/qemu_migration.c | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c >>> index b9d7d582f5..f7ced209a4 100644 >>> --- a/src/qemu/qemu_migration.c >>> +++ b/src/qemu/qemu_migration.c >>> @@ -5145,7 +5145,7 @@ qemuMigrationSrcPerformPeer2Peer(virQEMUDriver *driver, >>> goto cleanup; >>> >>> if (dconn == NULL) { >>> - virReportError(VIR_ERR_OPERATION_FAILED, >>> + virReportError(VIR_ERR_NO_CONNECT, >>> _("Failed to connect to remote libvirt URI %s: %s"), >>> dconnuri, virGetLastErrorMessage()); >>> return -1; >> >> I'm not exactly sure why we need this virReportError() in the first >> place. Basically, we are just overwriting a more accurate error with >> this generic one. Doesn't removing this virReportError() fix the problem? > > Not all failure scenarios in virConnectOpenInternal() are > caught with a virReportError(). Hence, we do need this. Well, then the problem is in virConnectOpenInternal(). Either it should report error in all cases or none, because then the caller would have to check if error was reported or not.