[libvirt] [PATCH v5 4/5] storage: Support "chap" authentication for iscsi pool

[Daniel P. Berrange]

On Wed, Jul 24, 2013 at 10:25:06AM +0200, Ján Tomko wrote:
> On 07/23/2013 04:56 PM, Daniel P. Berrange wrote:
> > On Tue, Jul 23, 2013 at 10:47:46AM -0400, John Ferlan wrote:
> >> On 07/23/2013 10:18 AM, Ján Tomko wrote:
> >>> On 07/22/2013 10:31 PM, John Ferlan wrote:
> >>>
> >>>> ---
> >>>>  src/storage/storage_backend_iscsi.c | 111 +++++++++++++++++++++++++++++++++++-
> >>>>  1 file changed, 110 insertions(+), 1 deletion(-)
> >>>>
> >>>
> >>> I can confirm this works, but it's a shame it doesn't work on autostart.
> >>>
> >>> ACK if you clarify the error.
> >>>
> >>> Jan
> >>>
> >>
> >> The autostart changes require getting a connection to the secret
> >> driver which I felt may take more time than I had to figure out
> >> how to get to work properly...
> >>
> >> In any case, I adjusted the message as follows (same in 5/5):
> >>
> >> diff --git a/src/storage/storage_backend_iscsi.c b/src/storage/storage_backend_i
> >> index 388d6ed..ee8dd2e 100644
> >> --- a/src/storage/storage_backend_iscsi.c
> >> +++ b/src/storage/storage_backend_iscsi.c
> >> @@ -714,7 +714,8 @@ virStorageBackendISCSISetAuth(const char *portal,
> >>  
> >>      if (!conn) {
> >>          virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> >> -                       _("iscsi 'chap' authentication requires connection"));
> >> +                       _("iscsi 'chap' authentication not supported "
> >> +                         "for autostarted pools"));
> >>          return -1;
> >>      }
> > 
> > I noticed that the nwfilter already unconditionally calls virConnectOpen("qemu://system"); so we're already in fact suffering from the problem with
> > autostart having a qemu dependency.
> > 
> > Given this, I'd support a patch which simply did
> > 
> >   conn = virConnectOpen(privilege ? "qemu:///system" : "qemu:///session");
> > 
> > in storageDriverAutostart, provided that we ignore any errors from
> > virConnectOpen, and fallback to use NULL for the connection in that
> > case.
> > 
> > Obviously this is something we'll still need to fix properly in a
> > future release, but at least it'll make autostart of storage pools
> > with auth work in the common case in the short term for this release.
> > 
> 
> Both secret and qemu drivers are registered after the storage driver on
> libvirtd startup, so autostarting these pools will only work on storage driver
> reload. On libvirtd startup it fails with:
> qemuConnectOpen:1033 : internal error qemu state driver is not active
> 
> (And it seems nwfilter only opens the qemu:// connection on reload)

Oh damn, yes, that pretty much dooms us.  We can't change the order of
the drivers either, because autostarting of QEMU guests, requires that
the storage pools be autostarted already.

To fix this would require that we split virStateInitialize into two
parts, virStateInitialize() and virStateAutoStart(). That's too big
a change todo for this release, but we could do it for next release
without too much trouble.


Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|