On Fri, Nov 19, 2010 at 11:09:31AM -0700, Eric Blake wrote:
> On 11/12/2010 09:22 AM, Daniel P. Berrange wrote:
> > The Linux iSCSI initiator toolchain has the dubious feature that
> > if you ever run the 'sendtargets' command to merely query what
> > targets are available from a server, the results will be recorded
> > in /var/lib/iscsi. Any time the '/etc/init.d/iscsi' script runs
> > in the future, it will then automatically login to all those
> > targets. /etc/init.d/iscsi is automatically run whenever a NIC
> > comes online.
>
> Is that worth reporting as a bug in the iscsi toolchain? At any rate,
> we need this patch whether or not that tool changes behavior to
> something more sane. However, I'm not sure this is ready for ack
> without answers to some questions first:
I've already reported it as a bug and it went nowhere productive
with the iSCSI maintainer claiming this insanity is a desirable
feature.
> > +static int
> > +virStorageBackendISCSIGetTargets(virStoragePoolObjPtr pool ATTRIBUTE_UNUSED,
> > + char **const groups,
> > + void *data)
> > +{
> > + struct virStorageBackendISCSITargetList *list = data;
> > + char *target;
> > +
> > + if (!(target = strdup(groups[1]))) {
> > + virReportOOMError();
> > + return -1;
> > + }
> > +
> > + if (VIR_REALLOC_N(list->targets, list->ntargets + 1) < 0) {
>
> Up to you if you want to rebase this to use VIR_RESIZE_N (or
> VIR_EXPAND_N), or to just leave that to me as a subsequent followup
> patch (I'm already searching through the code base for other instances
> to convert; one more won't hurt me).
I'll let you change this, there are many more instances of this
in the storage backends already.
>
> > static int
> > -virStorageBackendISCSIScanTargets(const char *portal)
> > +virStorageBackendISCSIScanTargets(const char *portal,
> > + const char *initiatoriqn,
> > + size_t *ntargetsret,
> > + char ***targetsret)
> > {
> > - const char *const sendtargets[] = {
> > - ISCSIADM, "--mode", "discovery", "--type", "sendtargets", "--portal", portal, NULL
> > + /**
> > + *
> > + * The output of sendtargets is very simple, just two columns,
> > + * portal then target name
> > + *
> > + * 192.168.122.185:3260,1 iqn.2004-04.com:fedora14:iscsi.demo0.bf6d84
> > + * 192.168.122.185:3260,1 iqn.2004-04.com:fedora14:iscsi.demo1.bf6d84
> > + * 192.168.122.185:3260,1 iqn.2004-04.com:fedora14:iscsi.demo2.bf6d84
> > + * 192.168.122.185:3260,1 iqn.2004-04.com:fedora14:iscsi.demo3.bf6d84
> > + */
> > + const char *regexes[] = {
> > + "^\\s*(\\S+)\\s+(\\S+)\\s*$"
>
> \s and \S are GNU-isms, and regcomp() on other platforms will reject it;
> is this regex only used on Linux, or do we need to be portable to iscsi
> implementations on other platforms?
The storage drivers are already full of this regex syntax so
this isn't making it any less portable really.
>
> > + };
> > + int vars[] = { 2 };
> > + const char *const cmdsendtarget[] = {
> > + ISCSIADM, "--mode", "discovery", "--type", "sendtargets",
> > + "--portal", portal, NULL
> > };
> > - if (virRun(sendtargets, NULL) < 0) {
> > + struct virStorageBackendISCSITargetList list;
> > + int i;
> > + int exitstatus;
> > +
> > + memset(&list, 0, sizeof(list));
> > +
> > + if (virStorageBackendRunProgRegex(NULL, /* No pool for callback */
> > + cmdsendtarget,
> > + 1,
> > + regexes,
> > + vars,
> > + virStorageBackendISCSIGetTargets,
> > + &list,
> > + &exitstatus) < 0) {
> > virStorageReportError(VIR_ERR_INTERNAL_ERROR,
> > - _("Failed to run %s to get target list"),
> > - sendtargets[0]);
> > + "%s", _("lvs command failed"));
>
> Should this message be about iscsiadm rather than lvs?
Yep.
Daniel
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list