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

[Daniel P. Berrange]

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.

Regards,
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 :|