Re: [libvirt] [PATCH v3 02/14] migration: refactor: reuse p2p url check

On 25.09.2015 17:12, Jiri Denemark wrote: > On Fri, Sep 18, 2015 at 18:05:40 +0300, Nikolay Shirokovskiy wrote: >> Refactor dconnuri local server URI check to common API. >> >> Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy(a)virtuozzo.com&gt; >> --- >> src/libvirt-domain.c | 44 ++++++++++++++++++++++++-------------------- >> 1 files changed, 24 insertions(+), 20 deletions(-) >> >> diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c >> index 697d58d..2e43062 100644 >> --- a/src/libvirt-domain.c >> +++ b/src/libvirt-domain.c >> @@ -3293,6 +3293,28 @@ virDomainMigrateVersion3Params(virDomainPtr domain, >> } >> >> >> +static int ATTRIBUTE_NONNULL(1) >> +virDomainMigrateCheckNotLocal(const char *dconnuri) >> +{ >> + virURIPtr tempuri = NULL; >> + int ret = -1; >> + >> + if (!(tempuri = virURIParse(dconnuri))) >> + goto cleanup; >> + if (!tempuri->server || STRPREFIX(tempuri->server, "localhost")) { >> + virReportInvalidArg(dconnuri, "%s", >> + _("Attempt to migrate guest to the same host %s")); > > Hmm, I don't think this new error message is better than the old one. > But anyway, we shouldn't need this code at all, checking dconnuri on a > client and then passing it to libvirtd which will have to check it again > does not make a lot of sense. > > In other words, I think we should just completely remove the dconuri > checks from virDomainMigratePeer2Peer*. > > Jirka > Sorry, for delay. I hoped you'll finish series review.

The meaning of this check is unclear, so i found it origins, see https://www.redhat.com/archives/libvir-list/2011-January/msg00437.html. This message states that without this check we can get a deadlock, thus I leave it. Also I investigate if this check had been moved to qemu code as reviewer suggested. Looks likes it is not.