[libvirt] [PATCH] conf: storage: Remove iSCSI <auth> parsing

Dave Allan dallan at redhat.com
Tue Feb 23 16:39:56 UTC 2010


On 02/23/2010 10:34 AM, Cole Robinson wrote:
> This was never wired up, and even generated broken XML until 0.7.2,
> so clearly no one was trying to use it. Dan recommended its removal,
> so lets drop it.

CHAP auth is a fundamental part of iSCSI, so I don't think we should 
remove support for it.  I'm happy to fix it as soon as I get a bit of 
time which will probably be in a couple of weeks.  It isn't a difficult 
thing to fix, I just have a bunch of stuff I need to get done before I 
can work on it.  That's being the case, I don't have a strong opinion on 
whether we remove and re-add it, or just wait for me to fix it.

Dave


> Signed-off-by: Cole Robinson<crobinso at redhat.com>
> ---
>   docs/schemas/storagepool.rng                    |   19 ---------
>   src/conf/storage_conf.c                         |   48 -----------------------
>   src/conf/storage_conf.h                         |   18 --------
>   tests/storagepoolxml2xmlin/pool-iscsi-auth.xml  |   17 --------
>   tests/storagepoolxml2xmlout/pool-iscsi-auth.xml |   20 ---------
>   tests/storagepoolxml2xmltest.c                  |    1 -
>   6 files changed, 0 insertions(+), 123 deletions(-)
>   delete mode 100644 tests/storagepoolxml2xmlin/pool-iscsi-auth.xml
>   delete mode 100644 tests/storagepoolxml2xmlout/pool-iscsi-auth.xml
>
> diff --git a/docs/schemas/storagepool.rng b/docs/schemas/storagepool.rng
> index 247664e..bcdca62 100644
> --- a/docs/schemas/storagepool.rng
> +++ b/docs/schemas/storagepool.rng
> @@ -234,22 +234,6 @@
>       </element>
>     </define>
>
> -<define name='sourceinfoauth'>
> -<element name='auth'>
> -<attribute name='type'>
> -<choice>
> -<value>chap</value>
> -</choice>
> -</attribute>
> -<attribute name='login'>
> -<text/>
> -</attribute>
> -<attribute name='passwd'>
> -<text/>
> -</attribute>
> -</element>
> -</define>
> -
>     <define name='sourcefmtfs'>
>       <optional>
>         <element name='format'>
> @@ -374,9 +358,6 @@
>         <optional>
>         <ref name='initiatorinfoiqn'/>
>         </optional>
> -<optional>
> -<ref name='sourceinfoauth'/>
> -</optional>
>       </element>
>     </define>
>
> diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c
> index 19a1db9..dd375b9 100644
> --- a/src/conf/storage_conf.c
> +++ b/src/conf/storage_conf.c
> @@ -283,11 +283,6 @@ virStoragePoolSourceFree(virStoragePoolSourcePtr source) {
>       VIR_FREE(source->name);
>       VIR_FREE(source->adapter);
>       VIR_FREE(source->initiator.iqn);
> -
> -    if (source->authType == VIR_STORAGE_POOL_AUTH_CHAP) {
> -        VIR_FREE(source->auth.chap.login);
> -        VIR_FREE(source->auth.chap.passwd);
> -    }
>   }
>
>   void
> @@ -363,26 +358,6 @@ virStoragePoolObjRemove(virStoragePoolObjListPtr pools,
>
>
>   static int
> -virStoragePoolDefParseAuthChap(xmlXPathContextPtr ctxt,
> -                               virStoragePoolAuthChapPtr auth) {
> -    auth->login = virXPathString("string(./auth/@login)", ctxt);
> -    if (auth->login == NULL) {
> -        virStorageReportError(VIR_ERR_XML_ERROR,
> -                              "%s", _("missing auth host attribute"));
> -        return -1;
> -    }
> -
> -    auth->passwd = virXPathString("string(./auth/@passwd)", ctxt);
> -    if (auth->passwd == NULL) {
> -        virStorageReportError(VIR_ERR_XML_ERROR,
> -                              "%s", _("missing auth passwd attribute"));
> -        return -1;
> -    }
> -
> -    return 0;
> -}
> -
> -static int
>   virStoragePoolDefParseSource(xmlXPathContextPtr ctxt,
>                                virStoragePoolSourcePtr source,
>                                int pool_type,
> @@ -445,25 +420,6 @@ virStoragePoolDefParseSource(xmlXPathContextPtr ctxt,
>       source->dir = virXPathString("string(./dir/@path)", ctxt);
>       source->adapter = virXPathString("string(./adapter/@name)", ctxt);
>
> -    authType = virXPathString("string(./auth/@type)", ctxt);
> -    if (authType == NULL) {
> -        source->authType = VIR_STORAGE_POOL_AUTH_NONE;
> -    } else {
> -        if (STREQ(authType, "chap")) {
> -            source->authType = VIR_STORAGE_POOL_AUTH_CHAP;
> -        } else {
> -            virStorageReportError(VIR_ERR_XML_ERROR,
> -                                  _("unknown auth type '%s'"),
> -                                  (const char *)authType);
> -            goto cleanup;
> -        }
> -    }
> -
> -    if (source->authType == VIR_STORAGE_POOL_AUTH_CHAP) {
> -        if (virStoragePoolDefParseAuthChap(ctxt,&source->auth.chap)<  0)
> -            goto cleanup;
> -    }
> -
>       ret = 0;
>   cleanup:
>       ctxt->node = relnode;
> @@ -867,10 +823,6 @@ virStoragePoolSourceFormat(virBufferPtr buf,
>       }
>
>
> -    if (src->authType == VIR_STORAGE_POOL_AUTH_CHAP)
> -        virBufferVSprintf(buf,"<auth type='chap' login='%s' passwd='%s'/>\n",
> -                          src->auth.chap.login,
> -                          src->auth.chap.passwd);
>       virBufferAddLit(buf,"</source>\n");
>
>       return 0;
> diff --git a/src/conf/storage_conf.h b/src/conf/storage_conf.h
> index c643984..1408128 100644
> --- a/src/conf/storage_conf.h
> +++ b/src/conf/storage_conf.h
> @@ -134,19 +134,6 @@ enum virStoragePoolDeviceType {
>   };
>
>
> -enum virStoragePoolAuthType {
> -    VIR_STORAGE_POOL_AUTH_NONE,
> -    VIR_STORAGE_POOL_AUTH_CHAP,
> -};
> -
> -typedef struct _virStoragePoolAuthChap virStoragePoolAuthChap;
> -typedef virStoragePoolAuthChap *virStoragePoolAuthChapPtr;
> -struct _virStoragePoolAuthChap {
> -    char *login;
> -    char *passwd;
> -};
> -
> -
>   /*
>    * For remote pools, info on how to reach the host
>    */
> @@ -232,11 +219,6 @@ struct _virStoragePoolSource {
>       /* Initiator IQN */
>       virStoragePoolSourceInitiatorAttr initiator;
>
> -    int authType;       /* virStoragePoolAuthType */
> -    union {
> -        virStoragePoolAuthChap chap;
> -    } auth;
> -
>       int format; /* Pool type specific format such as filesystem type, or lvm version, etc */
>   };
>
> diff --git a/tests/storagepoolxml2xmlin/pool-iscsi-auth.xml b/tests/storagepoolxml2xmlin/pool-iscsi-auth.xml
> deleted file mode 100644
> index f7d4d52..0000000
> --- a/tests/storagepoolxml2xmlin/pool-iscsi-auth.xml
> +++ /dev/null
> @@ -1,17 +0,0 @@
> -<pool type='iscsi'>
> -<name>virtimages</name>
> -<uuid>e9392370-2917-565e-692b-d057f46512d6</uuid>
> -<source>
> -<host name="iscsi.example.com"/>
> -<device path="demo-target"/>
> -<auth type='chap' login='foobar' passwd='frobbar'/>
> -</source>
> -<target>
> -<path>/dev/disk/by-path</path>
> -<permissions>
> -<mode>0700</mode>
> -<owner>0</owner>
> -<group>0</group>
> -</permissions>
> -</target>
> -</pool>
> diff --git a/tests/storagepoolxml2xmlout/pool-iscsi-auth.xml b/tests/storagepoolxml2xmlout/pool-iscsi-auth.xml
> deleted file mode 100644
> index 557295d..0000000
> --- a/tests/storagepoolxml2xmlout/pool-iscsi-auth.xml
> +++ /dev/null
> @@ -1,20 +0,0 @@
> -<pool type='iscsi'>
> -<name>virtimages</name>
> -<uuid>e9392370-2917-565e-692b-d057f46512d6</uuid>
> -<capacity>0</capacity>
> -<allocation>0</allocation>
> -<available>0</available>
> -<source>
> -<host name='iscsi.example.com'/>
> -<device path='demo-target'/>
> -<auth type='chap' login='foobar' passwd='frobbar'/>
> -</source>
> -<target>
> -<path>/dev/disk/by-path</path>
> -<permissions>
> -<mode>0700</mode>
> -<owner>0</owner>
> -<group>0</group>
> -</permissions>
> -</target>
> -</pool>
> diff --git a/tests/storagepoolxml2xmltest.c b/tests/storagepoolxml2xmltest.c
> index 4550407..1d7094b 100644
> --- a/tests/storagepoolxml2xmltest.c
> +++ b/tests/storagepoolxml2xmltest.c
> @@ -91,7 +91,6 @@ mymain(int argc, char **argv)
>       DO_TEST("pool-logical-create");
>       DO_TEST("pool-disk");
>       DO_TEST("pool-iscsi");
> -    DO_TEST("pool-iscsi-auth");
>       DO_TEST("pool-netfs");
>       DO_TEST("pool-scsi");
>       DO_TEST("pool-mpath");




More information about the libvir-list mailing list