Re: [libvirt] [PATCH] iscsi: do not fail to stop a stopped pool

On 04/29/2015 10:40 AM, Ján Tomko wrote: > On Wed, Apr 29, 2015 at 10:10:11AM -0400, John Ferlan wrote: >> >> >> On 04/29/2015 09:08 AM, Ján Tomko wrote: >>> Just as we allow stopping filesystem pools when they were unmounted >>> externally, do not fail to stop an iscsi pool when someone else >>> closed the session externally. >>> >>> Resolves: >>> https://bugzilla.redhat.com/show_bug.cgi?id=1171984 >> >> For this I disagree - it doesn't resolve all the issues in 1171984. > > I can remove the 'Resolves:' line. > Fair enough >> It >> resolves a symptom of libvirt allowing more than one pool to use the >> same session. > > This resolves the error to stop a pool when there's no pool anymore, > whether that's because someone called StopPool earlier on a pool that > was duplicate but libvirt didn't catch it, or manually via messing with > iscsiadm. > I did some more testing of this and found one could start a domain using the pool in the funky state which would cause the session to be logged back in (I have authentication enabled too, so that could be a reason - I didn't chase deeper)....

This patch only addresses failure to shutdown the second of the duplicated pools, but trying to refresh the pool gets the same result - that is an error since the session was logged out. However, if someone calls refreshPool and there's a failure because of the logout, then the pool gets marked as inactive.

The checkPool will be "tricked" into believing the pool isn't started which shouldn't be a huge deal unless pool-autostart is set for the pool.

But I think someone would have expected (based on a recent series) that if a pool was started, that it could survive a 'service libvirtd restart'. The findPoolSources, uploadVol, downloadVol, and wipeVol don't require the session as they go direct to the block device >> >> While there is disagreement over the method I've taken : >> >> http://www.redhat.com/archives/libvir-list/2015-April/msg01197.html >> >> Simply "covering up" the original issue by just ignoring the error on >> stop doesn't seem to be the best solution to me. >> > > The proposed series aims to detect duplicate pools on the same hosts. > It does not deal with duplicate pools on different hosts. > Even if we change the duplicate checks to only deal with the target, > as I suggested here: > https://www.redhat.com/archives/libvir-list/2015-April/msg00959.html > (since libvirt's iscsi backend treats the same target on different hosts > as a duplicate pool, but the check above does not), this patch also > fixes stopping the pool after someone messes with iscsiadm manually, > > As noted in my response, duplicated IQN's on truly different host names (by name or address) is a different bug. Not that it doesn't deserve to be fixed, but let's take it one step at a time.

There's still another issue of how to resolve the "same host" when using different IP address families...