[libvirt] [PATCH v5 01/16] conf: Add/Allow parsing the auth in the disk source

John Ferlan posted 16 patches 8 years, 4 months ago
There is a newer version of this series
[libvirt] [PATCH v5 01/16] conf: Add/Allow parsing the auth in the disk source
Posted by John Ferlan 8 years, 4 months ago
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>.

Signed-off-by: John Ferlan <jferlan@redhat.com>
---
 docs/formatdomain.html.in                          | 67 +++++++++++++---------
 docs/schemas/domaincommon.rng                      | 18 +++++-
 src/conf/domain_conf.c                             | 67 +++++++++++++++++++++-
 src/util/virstoragefile.c                          |  1 +
 src/util/virstoragefile.h                          |  1 +
 .../qemuargv2xml-disk-drive-network-rbd-auth.xml   |  6 +-
 ...ml2argv-disk-drive-network-source-auth-both.xml | 51 ++++++++++++++++
 ...emuxml2argv-disk-drive-network-source-auth.args | 32 +++++++++++
 ...qemuxml2argv-disk-drive-network-source-auth.xml | 45 +++++++++++++++
 tests/qemuxml2argvtest.c                           |  2 +
 ...muxml2xmlout-disk-drive-network-source-auth.xml | 49 ++++++++++++++++
 tests/qemuxml2xmltest.c                            |  1 +
 tests/virstoragetest.c                             |  6 ++
 13 files changed, 311 insertions(+), 35 deletions(-)
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-source-auth-both.xml
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-source-auth.args
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-source-auth.xml
 create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-drive-network-source-auth.xml

diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index c0e3c22213..74f2090d06 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -2297,11 +2297,11 @@
       &lt;host name="hostname" port="7000"/&gt;
       &lt;snapshot name="snapname"/&gt;
       &lt;config file="/path/to/file"/&gt;
+      &lt;auth username='myuser'&gt;
+        &lt;secret type='ceph' usage='mypassid'/&gt;
+      &lt;/auth&gt;
     &lt;/source&gt;
     &lt;target dev="hdc" bus="ide"/&gt;
-    &lt;auth username='myuser'&gt;
-      &lt;secret type='ceph' usage='mypassid'/&gt;
-    &lt;/auth&gt;
   &lt;/disk&gt;
   &lt;disk type='block' device='cdrom'&gt;
     &lt;driver name='qemu' type='raw'/&gt;
@@ -2370,20 +2370,20 @@
     &lt;driver name='qemu' type='raw'/&gt;
     &lt;source protocol='iscsi' name='iqn.2013-07.com.example:iscsi-nopool/2'&gt;
       &lt;host name='example.com' port='3260'/&gt;
+      &lt;auth username='myuser'&gt;
+        &lt;secret type='iscsi' usage='libvirtiscsi'/&gt;
+      &lt;/auth&gt;
     &lt;/source&gt;
-    &lt;auth username='myuser'&gt;
-      &lt;secret type='iscsi' usage='libvirtiscsi'/&gt;
-    &lt;/auth&gt;
     &lt;target dev='vda' bus='virtio'/&gt;
   &lt;/disk&gt;
   &lt;disk type='network' device='lun'&gt;
     &lt;driver name='qemu' type='raw'/&gt;
     &lt;source protocol='iscsi' name='iqn.2013-07.com.example:iscsi-nopool/1'&gt;
       &lt;host name='example.com' port='3260'/&gt;
+      &lt;auth username='myuser'&gt;
+        &lt;secret type='iscsi' usage='libvirtiscsi'/&gt;
+      &lt;/auth&gt;
     &lt;/source&gt;
-    &lt;auth username='myuser'&gt;
-      &lt;secret type='iscsi' usage='libvirtiscsi'/&gt;
-    &lt;/auth&gt;
     &lt;target dev='sdb' bus='scsi'/&gt;
   &lt;/disk&gt;
   &lt;disk type='volume' device='disk'&gt;
@@ -2683,6 +2683,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>
@@ -3156,25 +3178,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
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 4dbda6932d..895af55da1 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -1578,11 +1578,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">
@@ -1601,7 +1617,6 @@
       <attribute name="protocol">
         <choice>
           <value>sheepdog</value>
-          <value>iscsi</value>
           <value>ftp</value>
           <value>ftps</value>
           <value>tftp</value>
@@ -1661,6 +1676,7 @@
       <ref name="diskSourceNetworkProtocolNBD"/>
       <ref name="diskSourceNetworkProtocolGluster"/>
       <ref name="diskSourceNetworkProtocolRBD"/>
+      <ref name="diskSourceNetworkProtocolISCSI"/>
       <ref name="diskSourceNetworkProtocolHTTP"/>
       <ref name="diskSourceNetworkProtocolSimple"/>
       <ref name="diskSourceNetworkProtocolVxHS"/>
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 54be9028d7..91d554c3ee 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -8208,6 +8208,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,
@@ -8245,6 +8268,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 */
@@ -8881,6 +8907,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->authDefined = true;
+
             source = true;
 
             startupPolicy = virXMLPropString(cur, "startupPolicy");
@@ -8938,6 +8977,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->authDefined) {
+                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")) {
@@ -9173,8 +9221,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;
@@ -21873,6 +21921,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->authDefined &&
+            src->type != VIR_STORAGE_TYPE_VOLUME) {
+            if (virStorageAuthDefFormat(&childBuf, src->auth) < 0)
+                goto error;
+        }
+
         if (virXMLFormatElement(buf, "source", &attrBuf, &childBuf) < 0)
             goto error;
     }
@@ -22060,7 +22119,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->authDefined) {
         if (virStorageAuthDefFormat(buf, def->src->auth) < 0)
             return -1;
     }
diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c
index dd44949403..727ec52856 100644
--- a/src/util/virstoragefile.c
+++ b/src/util/virstoragefile.c
@@ -2558,6 +2558,7 @@ virStorageSourceParseRBDColonString(const char *rbdstr,
                            virSecretUsageTypeToString(VIR_SECRET_USAGE_TYPE_CEPH)) < 0)
                 goto error;
             src->auth = authdef;
+            src->authDefined = true;
             authdef = NULL;
 
             /* Cannot formulate a secretType (eg, usage or uuid) given
diff --git a/src/util/virstoragefile.h b/src/util/virstoragefile.h
index 74dee10f2f..c8bb1066fe 100644
--- a/src/util/virstoragefile.h
+++ b/src/util/virstoragefile.h
@@ -238,6 +238,7 @@ struct _virStorageSource {
     virStorageNetHostDefPtr hosts;
     virStorageSourcePoolDefPtr srcpool;
     virStorageAuthDefPtr auth;
+    bool authDefined;
     virStorageEncryptionPtr encryption;
 
     char *driverName;
diff --git a/tests/qemuargv2xmldata/qemuargv2xml-disk-drive-network-rbd-auth.xml b/tests/qemuargv2xmldata/qemuargv2xml-disk-drive-network-rbd-auth.xml
index 3f30296c0b..e1326b925c 100644
--- a/tests/qemuargv2xmldata/qemuargv2xml-disk-drive-network-rbd-auth.xml
+++ b/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'/>
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-source-auth-both.xml b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-source-auth-both.xml
new file mode 100644
index 0000000000..fed75ad70e
--- /dev/null
+++ b/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>
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-source-auth.args b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-source-auth.args
new file mode 100644
index 0000000000..23b1490eec
--- /dev/null
+++ b/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
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-source-auth.xml b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-source-auth.xml
new file mode 100644
index 0000000000..bd84cc42f6
--- /dev/null
+++ b/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>
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index a505864b87..a4ff7b80c1 100644
--- a/tests/qemuxml2argvtest.c
+++ b/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);
diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-drive-network-source-auth.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-drive-network-source-auth.xml
new file mode 100644
index 0000000000..9dc063dea9
--- /dev/null
+++ b/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>
diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c
index 7d7a5f1e4b..c484d8d17c 100644
--- a/tests/qemuxml2xmltest.c
+++ b/tests/qemuxml2xmltest.c
@@ -475,6 +475,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);
diff --git a/tests/virstoragetest.c b/tests/virstoragetest.c
index ffebd4dc1d..fe1521d9c0 100644
--- a/tests/virstoragetest.c
+++ b/tests/virstoragetest.c
@@ -1361,6 +1361,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"
@@ -1526,6 +1529,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\","
-- 
2.13.6

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v5 01/16] conf: Add/Allow parsing the auth in the disk source
Posted by Peter Krempa 8 years, 3 months ago
On Thu, Oct 05, 2017 at 09:22:08 -0400, John Ferlan wrote:
> 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>.
> 
> Signed-off-by: John Ferlan <jferlan@redhat.com>
> ---
>  docs/formatdomain.html.in                          | 67 +++++++++++++---------
>  docs/schemas/domaincommon.rng                      | 18 +++++-
>  src/conf/domain_conf.c                             | 67 +++++++++++++++++++++-
>  src/util/virstoragefile.c                          |  1 +
>  src/util/virstoragefile.h                          |  1 +
>  .../qemuargv2xml-disk-drive-network-rbd-auth.xml   |  6 +-
>  ...ml2argv-disk-drive-network-source-auth-both.xml | 51 ++++++++++++++++
>  ...emuxml2argv-disk-drive-network-source-auth.args | 32 +++++++++++
>  ...qemuxml2argv-disk-drive-network-source-auth.xml | 45 +++++++++++++++
>  tests/qemuxml2argvtest.c                           |  2 +
>  ...muxml2xmlout-disk-drive-network-source-auth.xml | 49 ++++++++++++++++
>  tests/qemuxml2xmltest.c                            |  1 +
>  tests/virstoragetest.c                             |  6 ++
>  13 files changed, 311 insertions(+), 35 deletions(-)
>  create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-source-auth-both.xml
>  create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-source-auth.args
>  create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-source-auth.xml
>  create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-drive-network-source-auth.xml

[...]

> diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
> index c0e3c22213..74f2090d06 100644
> --- a/docs/formatdomain.html.in
> +++ b/docs/formatdomain.html.in
> diff --git a/src/util/virstoragefile.h b/src/util/virstoragefile.h
> index 74dee10f2f..c8bb1066fe 100644
> --- a/src/util/virstoragefile.h
> +++ b/src/util/virstoragefile.h
> @@ -238,6 +238,7 @@ struct _virStorageSource {
>      virStorageNetHostDefPtr hosts;
>      virStorageSourcePoolDefPtr srcpool;
>      virStorageAuthDefPtr auth;
> +    bool authDefined;

I find this name slightly misleading. How about authInherited?

At any rate, this patch should be applicable without conflicts and it
will help in doing security related stuff for the backing chain.

ACK

You can push this right now. I'll pick out a few other things form this
series which will be helpful for the blockdev-add saga and pass back the
rest for the iSCSI work (mainly with pre-blockdev qemus).

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list