RE: [libvirt] Re: [Patch v0.4] iSCSI Multi-IQN (Libvirt Support)

-----Original Message----- From: Daniel P. Berrange [mailto:berrange@redhat.com] Sent: Tuesday, November 17, 2009 11:21 PM To: Dave Allan Cc: Bellad, Sudhir; libvir-list(a)redhat.com; Iyer, Shyam; Domsch, Matt; KM, Paniraja Subject: Re: [libvirt] Re: [Patch v0.4] iSCSI Multi-IQN (Libvirt Support) On Mon, Nov 16, 2009 at 01:58:08PM -0500, Dave Allan wrote: > diff --git a/src/storage_backend_iscsi.c b/src/storage_backend_iscsi.c > index b516add..1fb21a5 100644 > --- a/src/storage_backend_iscsi.c > +++ b/src/storage_backend_iscsi.c > @@ -39,6 +39,10 @@ > #include "storage_backend_iscsi.h" > #include "util.h" > #include "memory.h" > +#include <sys/types.h> > +#include <sys/stat.h> > +#include <fcntl.h> > +#include <unistd.h> > > #define VIR_FROM_THIS VIR_FROM_STORAGE > > @@ -159,13 +163,54 @@ virStorageBackendISCSIConnection(virConnectPtr conn, > const char *portal, > const char *action) > { > - const char *const cmdargv[] = { > - ISCSIADM, "--mode", "node", "--portal", portal, > - "--targetname", pool->def->source.devices[0].path, action, NULL > - }; > - > - if (virRun(conn, cmdargv, NULL) < 0) > - return -1; > + DIR *dir; > + struct dirent *entry; > + > + > + if (pool->def->source.initiator.iqnname != NULL) { > > What's the point of this loop? At best, it's unneeded complexity,

> worst it will match multiple interface names which will create the > multiple sessions per pool scenario that I explicitly want to avoid. > > Secondly, if you want to do some sort of validation of the iqn, why are > you reading from a hardcoded directory? Can you use the output of > iscsiadm? That is likely to be a more stable interface than the > directory which I would expect is a compile time option to the iscsi > initiator. I'm really wondering much the same here - I don't see the purpose in iterating over this directory. The iqn given in the XML ought to be able to be passed straight to iscsadm's -I parameter