On Thu, 6 Jan 2022, Michal Prívozník wrote:
> 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@xxxxxxxxxxx>
> >>> Suggested-by: John Levon <john.levon@xxxxxxxxxxx>
> >>> Signed-off-by: Raphael Norwitz <raphael.norwitz@xxxxxxxxxxx>
> >>> ---
> >>> 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.
>
OK fine. I will post a patch soon.
