conf: Add/Allow parsing the auth in the disk source

Since the virStorageAuthDefPtr auth; is a member of _virStorageSource
it really should be allowed to be a subelement of the disk <source>
for the RBD and iSCSI prototcols. That way we can set up to allow
the <auth> element to be formatted within the disk source.

Since we've allowed the <auth> to be a child of <disk>, we'll need
to keep track of how it was read so that when writing out we'll know
whether to format as child of <disk> or <source>. For the argv2xml
parsing, let's format under <source> as a preference. Do not allow
<auth> to be both a child of <disk> and <source>.

Modify the qemuxml2argvtest to add a parse failure when there is an
<auth> as a child of <disk> *and* an <auth> as a child of <source>.

Add tests to validate that if the <auth> was found in <source>, then
the resulting xml2xml and xml2arg works just fine.  The two new .args
file are exact copies of the non "-source" version of the file.

The virschematest will read the new test files and validate from a
RNG viewpoint things are fine

Update the virstoragefile, virstoragetest, and args2xml file to show
the "preference" to place <auth> as a child of <source>.

docs/formatdomain.html.in

@@ -2304,11 +2304,11 @@
       <host name="hostname" port="7000"/>
       <snapshot name="snapname"/>
       <config file="/path/to/file"/>
+      <auth username='myuser'>
+        <secret type='ceph' usage='mypassid'/>
+      </auth>
     </source>
     <target dev="hdc" bus="ide"/>
-    <auth username='myuser'>
-      <secret type='ceph' usage='mypassid'/>
-    </auth>
   </disk>
   <disk type='block' device='cdrom'>
     <driver name='qemu' type='raw'/>
@@ -2377,20 +2377,20 @@
     <driver name='qemu' type='raw'/>
     <source protocol='iscsi' name='iqn.2013-07.com.example:iscsi-nopool/2'>
       <host name='example.com' port='3260'/>
+      <auth username='myuser'>
+        <secret type='iscsi' usage='libvirtiscsi'/>
+      </auth>
     </source>
-    <auth username='myuser'>
-      <secret type='iscsi' usage='libvirtiscsi'/>
-    </auth>
     <target dev='vda' bus='virtio'/>
   </disk>
   <disk type='network' device='lun'>
     <driver name='qemu' type='raw'/>
     <source protocol='iscsi' name='iqn.2013-07.com.example:iscsi-nopool/1'>
       <host name='example.com' port='3260'/>
+      <auth username='myuser'>
+        <secret type='iscsi' usage='libvirtiscsi'/>
+      </auth>
     </source>
-    <auth username='myuser'>
-      <secret type='iscsi' usage='libvirtiscsi'/>
-    </auth>
     <target dev='sdb' bus='scsi'/>
   </disk>
   <disk type='volume' device='disk'>
@@ -2690,6 +2690,28 @@
             protocol. Supported for 'rbd' <span class="since">since 1.2.11
             (QEMU only).</span>
           </dd>
+          <dt><code>auth</code></dt>
+          <dd><span class="since">Since libvirt 3.9.0</span>, the
+            <code>auth</code> element is supported for a disk
+            <code>type</code> "network" that is using a <code>source</code>
+            element with the <code>protocol</code> attributes "rbd" or "iscsi".
+            If present, the <code>auth</code> element provides the
+            authentication credentials needed to access the source.  It
+            includes a mandatory attribute <code>username</code>, which
+            identifies the username to use during authentication, as well
+            as a sub-element <code>secret</code> with mandatory
+            attribute <code>type</code>, to tie back to
+            a <a href="formatsecret.html">libvirt secret object</a> that
+            holds the actual password or other credentials (the domain XML
+            intentionally does not expose the password, only the reference
+            to the object that does manage the password).
+            Known secret types are "ceph" for Ceph RBD network sources and
+            "iscsi" for CHAP authentication of iSCSI targets.
+            Both will require either a <code>uuid</code> attribute
+            with the UUID of the secret object or a <code>usage</code>
+            attribute matching the key that was specified in the
+            secret object.
+          </dd>
         </dl>
 
         <p>
@@ -3163,25 +3185,14 @@
         are available, each defaulting to 0.
       </dd>
       <dt><code>auth</code></dt>
-      <dd>The <code>auth</code> element is supported for a disk
-        <code>type</code> "network" that is using a <code>source</code>
-        element with the <code>protocol</code> attributes "rbd" or "iscsi".
-        If present, the <code>auth</code> element provides the
-        authentication credentials needed to access the source.  It
-        includes a mandatory attribute <code>username</code>, which
-        identifies the username to use during authentication, as well
-        as a sub-element <code>secret</code> with mandatory
-        attribute <code>type</code>, to tie back to
-        a <a href="formatsecret.html">libvirt secret object</a> that
-        holds the actual password or other credentials (the domain XML
-        intentionally does not expose the password, only the reference
-        to the object that does manage the password).
-        Known secret types are "ceph" for Ceph RBD network sources and
-        "iscsi" for CHAP authentication of iSCSI targets.
-        Both will require either a <code>uuid</code> attribute
-        with the UUID of the secret object or a <code>usage</code>
-        attribute matching the key that was specified in the
-        secret object.  <span class="since">libvirt 0.9.7</span>
+      <dd>Starting with <span class="since">libvirt 3.9.0</span> the
+        <code>auth</code> element is preferred to be a sub-element of
+        the <code>source</code> element. The element is still read and
+        managed as a <code>disk</code> sub-element. It is invalid to use
+        <code>auth</code> as both a sub-element of <code>disk</code>
+        and <code>source</code>. The <code>auth</code> element was
+        introduced as a <code>disk</code> sub-element in
+        <span class="since">libvirt 0.9.7.</span>
       </dd>
       <dt><code>geometry</code></dt>
       <dd>The optional <code>geometry</code> element provides the

docs/schemas/domaincommon.rng

@@ -1580,11 +1580,27 @@
             <empty/>
           </element>
         </optional>
+        <optional>
+          <ref name="diskAuth"/>
+        </optional>
         <empty/>
       </interleave>
     </element>
   </define>
 
+  <define name="diskSourceNetworkProtocolISCSI">
+    <element name="source">
+      <attribute name="protocol">
+        <value>iscsi</value>
+      </attribute>
+      <attribute name="name"/>
+      <ref name="diskSourceNetworkHost"/>
+      <optional>
+        <ref name="diskAuth"/>
+      </optional>
+    </element>
+  </define>
+
   <define name="diskSourceNetworkProtocolHTTP">
     <element name="source">
       <attribute name="protocol">
@@ -1603,7 +1619,6 @@
       <attribute name="protocol">
         <choice>
           <value>sheepdog</value>
-          <value>iscsi</value>
           <value>ftp</value>
           <value>ftps</value>
           <value>tftp</value>
@@ -1663,6 +1678,7 @@
       <ref name="diskSourceNetworkProtocolNBD"/>
       <ref name="diskSourceNetworkProtocolGluster"/>
       <ref name="diskSourceNetworkProtocolRBD"/>
+      <ref name="diskSourceNetworkProtocolISCSI"/>
       <ref name="diskSourceNetworkProtocolHTTP"/>
       <ref name="diskSourceNetworkProtocolSimple"/>
       <ref name="diskSourceNetworkProtocolVxHS"/>

src/conf/domain_conf.c

@@ -8278,6 +8278,29 @@ virDomainDiskSourceNetworkParse(xmlNodePtr node,
 }
 
 
+static int
+virDomainDiskSourceAuthParse(xmlNodePtr node,
+                             virStorageAuthDefPtr *authdefsrc)
+{
+    xmlNodePtr child;
+    virStorageAuthDefPtr authdef;
+
+    for (child = node->children; child; child = child->next) {
+        if (child->type == XML_ELEMENT_NODE &&
+            virXMLNodeNameEqual(child, "auth")) {
+
+            if (!(authdef = virStorageAuthDefParse(node->doc, child)))
+                return -1;
+
+            *authdefsrc = authdef;
+            return 0;
+        }
+    }
+
+    return 0;
+}
+
+
 int
 virDomainDiskSourceParse(xmlNodePtr node,
                          xmlXPathContextPtr ctxt,
@@ -8315,6 +8338,9 @@ virDomainDiskSourceParse(xmlNodePtr node,
         goto cleanup;
     }
 
+    if (virDomainDiskSourceAuthParse(node, &src->auth) < 0)
+        goto cleanup;
+
     /* People sometimes pass a bogus '' source path when they mean to omit the
      * source element completely (e.g. CDROM without media). This is just a
      * little compatibility check to help those broken apps */
@@ -8961,6 +8987,19 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt,
             if (virDomainDiskSourceParse(cur, ctxt, def->src, flags) < 0)
                 goto error;
 
+            /* If we've already found an <auth> as a child of <disk> and
+             * we find one as a child of <source>, then force an error to
+             * avoid ambiguity */
+            if (authdef && def->src->auth) {
+                virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+                               _("an <auth> definition already found for "
+                                 "the <disk> definition"));
+                goto error;
+            }
+
+            if (def->src->auth)
+                def->src->authInherited = true;
+
             source = true;
 
             startupPolicy = virXMLPropString(cur, "startupPolicy");
@@ -9018,6 +9057,15 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt,
                 goto error;
         } else if (!authdef &&
                    virXMLNodeNameEqual(cur, "auth")) {
+            /* If we've already parsed <source> and found an <auth> child,
+             * then generate an error to avoid ambiguity */
+            if (def->src->authInherited) {
+                virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+                               _("an <auth> definition already found for "
+                                 "disk source"));
+                goto error;
+            }
+
             if (!(authdef = virStorageAuthDefParse(node->doc, cur)))
                 goto error;
         } else if (virXMLNodeNameEqual(cur, "iotune")) {
@@ -9253,8 +9301,8 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt,
 
     def->dst = target;
     target = NULL;
-    def->src->auth = authdef;
-    authdef = NULL;
+    if (authdef)
+        VIR_STEAL_PTR(def->src->auth, authdef);
     def->src->encryption = encryption;
     encryption = NULL;
     def->domain_name = domain_name;
@@ -22028,6 +22076,17 @@ virDomainDiskSourceFormatInternal(virBufferPtr buf,
             goto error;
         }
 
+        /* Storage Source formatting will not carry through the blunder
+         * that disk source formatting had at one time to format the
+         * <auth> for a volume source type. The <auth> information is
+         * kept in the storage pool and would be overwritten anyway.
+         * So avoid formatting it for volumes. */
+        if (src->auth && src->authInherited &&
+            src->type != VIR_STORAGE_TYPE_VOLUME) {
+            if (virStorageAuthDefFormat(&childBuf, src->auth) < 0)
+                goto error;
+        }
+
         if (virXMLFormatElement(buf, "source", &attrBuf, &childBuf) < 0)
             goto error;
     }
@@ -22207,7 +22266,9 @@ virDomainDiskDefFormat(virBufferPtr buf,
         virBufferAddLit(buf, "/>\n");
     }
 
-    if (def->src->auth) {
+    /* Format as child of <disk> if defined there; otherwise,
+     * if defined as child of <source>, then format later */
+    if (def->src->auth && !def->src->authInherited) {
         if (virStorageAuthDefFormat(buf, def->src->auth) < 0)
             return -1;
     }

src/util/virstoragefile.c

@@ -2578,6 +2578,7 @@ virStorageSourceParseRBDColonString(const char *rbdstr,
                            virSecretUsageTypeToString(VIR_SECRET_USAGE_TYPE_CEPH)) < 0)
                 goto error;
             src->auth = authdef;
+            src->authInherited = true;
             authdef = NULL;
 
             /* Cannot formulate a secretType (eg, usage or uuid) given

src/util/virstoragefile.h

@@ -240,6 +240,7 @@ struct _virStorageSource {
     virStorageNetHostDefPtr hosts;
     virStorageSourcePoolDefPtr srcpool;
     virStorageAuthDefPtr auth;
+    bool authInherited;
     virStorageEncryptionPtr encryption;
 
     virObjectPtr privateData;

tests/qemuargv2xmldata/qemuargv2xml-disk-drive-network-rbd-auth.xml

@@ -22,13 +22,13 @@
     </disk>
     <disk type='network' device='disk'>
       <driver name='qemu' type='raw'/>
-      <auth username='myname'>
-        <secret type='ceph' usage='qemuargv2xml_usage'/>
-      </auth>
       <source protocol='rbd' name='pool/image'>
         <host name='mon1.example.org' port='6321'/>
         <host name='mon2.example.org' port='6322'/>
         <host name='mon3.example.org' port='6322'/>
+        <auth username='myname'>
+          <secret type='ceph' usage='qemuargv2xml_usage'/>
+        </auth>
       </source>
       <target dev='vda' bus='virtio'/>
       <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/>

tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-source-auth-both.xml

@@ -0,0 +1,51 @@
+<domain type='qemu'>
+  <name>QEMUGuest1</name>
+  <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>
+  <memory unit='KiB'>219136</memory>
+  <currentMemory unit='KiB'>219136</currentMemory>
+  <vcpu placement='static'>1</vcpu>
+  <os>
+    <type arch='i686' machine='pc'>hvm</type>
+    <boot dev='hd'/>
+  </os>
+  <clock offset='utc'/>
+  <on_poweroff>destroy</on_poweroff>
+  <on_reboot>restart</on_reboot>
+  <on_crash>destroy</on_crash>
+  <devices>
+    <emulator>/usr/bin/qemu-system-i686</emulator>
+    <disk type='network' device='disk'>
+      <driver name='qemu' type='raw'/>
+      <auth username='myname'>
+        <secret type='iscsi' usage='mycluster_myname'/>
+      </auth>
+      <source protocol='iscsi' name='iqn.1992-01.com.example:storage/1'>
+        <host name='example.org' port='6000'/>
+        <auth username='myname'>
+          <secret type='iscsi' usage='mycluster_myname'/>
+        </auth>
+      </source>
+      <target dev='vda' bus='virtio'/>
+    </disk>
+    <disk type='network' device='disk'>
+      <driver name='qemu' type='raw'/>
+      <auth username='myname'>
+        <secret type='ceph' usage='mycluster_myname'/>
+      </auth>
+      <source protocol='rbd' name='pool/image'>
+        <host name='mon1.example.org' port='6321'/>
+        <host name='mon2.example.org' port='6322'/>
+        <host name='mon3.example.org' port='6322'/>
+        <auth username='myname'>
+          <secret type='ceph' usage='mycluster_myname'/>
+        </auth>
+      </source>
+      <target dev='vdb' bus='virtio'/>
+    </disk>
+    <controller type='usb' index='0'/>
+    <controller type='pci' index='0' model='pci-root'/>
+    <input type='mouse' bus='ps2'/>
+    <input type='keyboard' bus='ps2'/>
+    <memballoon model='none'/>
+  </devices>
+</domain>

tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-source-auth.args

@@ -0,0 +1,32 @@
+LC_ALL=C \
+PATH=/bin \
+HOME=/home/test \
+USER=test \
+LOGNAME=test \
+QEMU_AUDIO_DRV=none \
+/usr/bin/qemu-system-i686 \
+-name QEMUGuest1 \
+-S \
+-M pc \
+-m 214 \
+-smp 1,sockets=1,cores=1,threads=1 \
+-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \
+-nographic \
+-nodefaults \
+-chardev socket,id=charmonitor,path=/tmp/lib/domain--1-QEMUGuest1/monitor.sock,\
+server,nowait \
+-mon chardev=charmonitor,id=monitor,mode=readline \
+-no-acpi \
+-boot c \
+-usb \
+-drive file=iscsi://myname:AQCVn5hO6HzFAhAAq0NCv8jtJcIcE+HOBlMQ1A@example.org:\
+6000/iqn.1992-01.com.example%3Astorage/1,format=raw,if=none,\
+id=drive-virtio-disk0 \
+-device virtio-blk-pci,bus=pci.0,addr=0x3,drive=drive-virtio-disk0,\
+id=virtio-disk0 \
+-drive 'file=rbd:pool/image:id=myname:\
+key=QVFDVm41aE82SHpGQWhBQXEwTkN2OGp0SmNJY0UrSE9CbE1RMUE=:\
+auth_supported=cephx\;none:mon_host=mon1.example.org\:6321\;mon2.example.org\:\
+6322\;mon3.example.org\:6322,format=raw,if=none,id=drive-virtio-disk1' \
+-device virtio-blk-pci,bus=pci.0,addr=0x4,drive=drive-virtio-disk1,\
+id=virtio-disk1

tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-source-auth.xml

@@ -0,0 +1,45 @@
+<domain type='qemu'>
+  <name>QEMUGuest1</name>
+  <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>
+  <memory unit='KiB'>219136</memory>
+  <currentMemory unit='KiB'>219136</currentMemory>
+  <vcpu placement='static'>1</vcpu>
+  <os>
+    <type arch='i686' machine='pc'>hvm</type>
+    <boot dev='hd'/>
+  </os>
+  <clock offset='utc'/>
+  <on_poweroff>destroy</on_poweroff>
+  <on_reboot>restart</on_reboot>
+  <on_crash>destroy</on_crash>
+  <devices>
+    <emulator>/usr/bin/qemu-system-i686</emulator>
+    <disk type='network' device='disk'>
+      <driver name='qemu' type='raw'/>
+      <source protocol='iscsi' name='iqn.1992-01.com.example:storage/1'>
+        <host name='example.org' port='6000'/>
+        <auth username='myname'>
+          <secret type='iscsi' usage='mycluster_myname'/>
+        </auth>
+      </source>
+      <target dev='vda' bus='virtio'/>
+    </disk>
+    <disk type='network' device='disk'>
+      <driver name='qemu' type='raw'/>
+      <source protocol='rbd' name='pool/image'>
+        <host name='mon1.example.org' port='6321'/>
+        <host name='mon2.example.org' port='6322'/>
+        <host name='mon3.example.org' port='6322'/>
+        <auth username='myname'>
+          <secret type='ceph' usage='mycluster_myname'/>
+        </auth>
+      </source>
+      <target dev='vdb' bus='virtio'/>
+    </disk>
+    <controller type='usb' index='0'/>
+    <controller type='pci' index='0' model='pci-root'/>
+    <input type='mouse' bus='ps2'/>
+    <input type='keyboard' bus='ps2'/>
+    <memballoon model='none'/>
+  </devices>
+</domain>

tests/qemuxml2argvtest.c

@@ -932,6 +932,7 @@ mymain(void)
     DO_TEST("disk-drive-network-iscsi-auth", NONE);
     DO_TEST_PARSE_ERROR("disk-drive-network-iscsi-auth-secrettype-invalid", NONE);
     DO_TEST_PARSE_ERROR("disk-drive-network-iscsi-auth-wrong-secrettype", NONE);
+    DO_TEST_PARSE_ERROR("disk-drive-network-source-auth-both", NONE);
     DO_TEST("disk-drive-network-iscsi-lun",
             QEMU_CAPS_NODEFCONFIG, QEMU_CAPS_VIRTIO_SCSI,
             QEMU_CAPS_SCSI_BLOCK);
@@ -940,6 +941,7 @@ mymain(void)
     DO_TEST("disk-drive-network-rbd", NONE);
     DO_TEST("disk-drive-network-sheepdog", NONE);
     DO_TEST("disk-drive-network-rbd-auth", NONE);
+    DO_TEST("disk-drive-network-source-auth", NONE);
 # ifdef HAVE_GNUTLS_CIPHER_ENCRYPT
     DO_TEST("disk-drive-network-rbd-auth-AES",
             QEMU_CAPS_OBJECT_SECRET, QEMU_CAPS_VIRTIO_SCSI);

tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-drive-network-source-auth.xml

@@ -0,0 +1,49 @@
+<domain type='qemu'>
+  <name>QEMUGuest1</name>
+  <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>
+  <memory unit='KiB'>219136</memory>
+  <currentMemory unit='KiB'>219136</currentMemory>
+  <vcpu placement='static'>1</vcpu>
+  <os>
+    <type arch='i686' machine='pc'>hvm</type>
+    <boot dev='hd'/>
+  </os>
+  <clock offset='utc'/>
+  <on_poweroff>destroy</on_poweroff>
+  <on_reboot>restart</on_reboot>
+  <on_crash>destroy</on_crash>
+  <devices>
+    <emulator>/usr/bin/qemu-system-i686</emulator>
+    <disk type='network' device='disk'>
+      <driver name='qemu' type='raw'/>
+      <source protocol='iscsi' name='iqn.1992-01.com.example:storage/1'>
+        <host name='example.org' port='6000'/>
+        <auth username='myname'>
+          <secret type='iscsi' usage='mycluster_myname'/>
+        </auth>
+      </source>
+      <target dev='vda' bus='virtio'/>
+      <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/>
+    </disk>
+    <disk type='network' device='disk'>
+      <driver name='qemu' type='raw'/>
+      <source protocol='rbd' name='pool/image'>
+        <host name='mon1.example.org' port='6321'/>
+        <host name='mon2.example.org' port='6322'/>
+        <host name='mon3.example.org' port='6322'/>
+        <auth username='myname'>
+          <secret type='ceph' usage='mycluster_myname'/>
+        </auth>
+      </source>
+      <target dev='vdb' bus='virtio'/>
+      <address type='pci' domain='0x0000' bus='0x00' slot='0x04' function='0x0'/>
+    </disk>
+    <controller type='usb' index='0'>
+      <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'/>
+    </controller>
+    <controller type='pci' index='0' model='pci-root'/>
+    <input type='mouse' bus='ps2'/>
+    <input type='keyboard' bus='ps2'/>
+    <memballoon model='none'/>
+  </devices>
+</domain>

tests/qemuxml2xmltest.c

@@ -520,6 +520,7 @@ mymain(void)
     DO_TEST("disk-drive-network-rbd-auth", NONE);
     DO_TEST("disk-drive-network-rbd-ipv6", NONE);
     DO_TEST("disk-drive-network-rbd-ceph-env", NONE);
+    DO_TEST("disk-drive-network-source-auth", NONE);
     DO_TEST("disk-drive-network-sheepdog", NONE);
     DO_TEST("disk-drive-network-vxhs", NONE);
     DO_TEST("disk-drive-network-tlsx509-vxhs", NONE);

tests/virstoragetest.c

@@ -1362,6 +1362,9 @@ mymain(void)
     TEST_BACKING_PARSE("rbd:testshare:id=asdf:mon_host=example.com",
                        "<source protocol='rbd' name='testshare'>\n"
                        "  <host name='example.com'/>\n"
+                       "  <auth username='asdf'>\n"
+                       "    <secret type='ceph'/>\n"
+                       "  </auth>\n"
                        "</source>\n");
     TEST_BACKING_PARSE("nbd:example.org:6000:exportname=blah",
                        "<source protocol='nbd' name='blah'>\n"
@@ -1527,6 +1530,9 @@ mymain(void)
                             "}",
                        "<source protocol='rbd' name='testshare'>\n"
                        "  <host name='example.com'/>\n"
+                       "  <auth username='asdf'>\n"
+                       "    <secret type='ceph'/>\n"
+                       "  </auth>\n"
                        "</source>\n");
     TEST_BACKING_PARSE("json:{\"file\":{\"driver\":\"rbd\","
                                        "\"image\":\"test\","