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.
For now just allow the format in the RNG and read it in domain_conf.
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>.
The virschematest will read the new test files and validate from a
RNG viewpoint things are fine
Signed-off-by: John Ferlan <jferlan@redhat.com>
---
docs/schemas/domaincommon.rng | 20 +++++++-
src/conf/domain_conf.c | 53 ++++++++++++++++++++--
...v-disk-drive-network-iscsi-source-auth-both.xml | 36 +++++++++++++++
...l2argv-disk-drive-network-iscsi-source-auth.xml | 43 ++++++++++++++++++
...rgv-disk-drive-network-rbd-source-auth-both.xml | 45 ++++++++++++++++++
...xml2argv-disk-drive-network-rbd-source-auth.xml | 42 +++++++++++++++++
tests/qemuxml2argvtest.c | 2 +
7 files changed, 237 insertions(+), 4 deletions(-)
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-iscsi-source-auth-both.xml
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-iscsi-source-auth.xml
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-rbd-source-auth-both.xml
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-rbd-source-auth.xml
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index c9a4f7a9a..139f1eea2 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -1578,11 +1578,29 @@
<empty/>
</element>
</optional>
+ <optional>
+ <ref name="diskAuth"/>
+ </optional>
<empty/>
</interleave>
</element>
</define>
+ <define name="diskSourceNetworkProtocolISCSI">
+ <element name="source">
+ <attribute name="protocol">
+ <choice>
+ <value>iscsi</value>
+ </choice>
+ </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 +1619,6 @@
<attribute name="protocol">
<choice>
<value>sheepdog</value>
- <value>iscsi</value>
<value>ftp</value>
<value>ftps</value>
<value>tftp</value>
@@ -1644,6 +1661,7 @@
<ref name="diskSourceNetworkProtocolNBD"/>
<ref name="diskSourceNetworkProtocolGluster"/>
<ref name="diskSourceNetworkProtocolRBD"/>
+ <ref name="diskSourceNetworkProtocolISCSI"/>
<ref name="diskSourceNetworkProtocolHTTP"/>
<ref name="diskSourceNetworkProtocolSimple"/>
</choice>
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 8dca1357c..5c0218cdf 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -8107,6 +8107,29 @@ virDomainDiskSourcePoolDefParse(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,
@@ -8193,6 +8216,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 */
@@ -8770,6 +8796,7 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt,
char *serial = NULL;
char *startupPolicy = NULL;
virStorageAuthDefPtr authdef = NULL;
+ bool diskAuth = false;
char *tray = NULL;
char *removable = NULL;
char *logical_block_size = NULL;
@@ -8819,6 +8846,16 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt,
if (virDomainDiskSourceParse(cur, ctxt, def->src) < 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 (diskAuth && def->src->auth) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("an <auth> definition already found for "
+ "the <disk> definition"));
+ goto error;
+ }
+
source = true;
startupPolicy = virXMLPropString(cur, "startupPolicy");
@@ -8874,10 +8911,20 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt,
!(flags & VIR_DOMAIN_DEF_PARSE_INACTIVE)) {
if (virDomainDiskDefMirrorParse(def, cur, ctxt) < 0)
goto error;
- } else if (!authdef &&
+ } else if (!diskAuth &&
virXMLNodeNameEqual(cur, "auth")) {
+ diskAuth = true;
if (!(authdef = virStorageAuthDefParse(node->doc, cur)))
goto error;
+
+ /* If we've already parsed <source> and found an <auth> child,
+ * then generate an error to avoid ambiguity */
+ if (source && def->src->auth) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("an <auth> definition already found for "
+ "disk source"));
+ goto error;
+ }
} else if (virXMLNodeNameEqual(cur, "iotune")) {
if (virDomainDiskDefIotuneParse(def, ctxt) < 0)
goto error;
@@ -9111,8 +9158,8 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt,
def->dst = target;
target = NULL;
- def->src->auth = authdef;
- authdef = NULL;
+ if (diskAuth)
+ VIR_STEAL_PTR(def->src->auth, authdef);
def->src->encryption = encryption;
encryption = NULL;
def->domain_name = domain_name;
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-iscsi-source-auth-both.xml b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-iscsi-source-auth-both.xml
new file mode 100644
index 000000000..9f14f489f
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-iscsi-source-auth-both.xml
@@ -0,0 +1,36 @@
+<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>
+ <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-iscsi-source-auth.xml b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-iscsi-source-auth.xml
new file mode 100644
index 000000000..af2d51fe9
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-iscsi-source-auth.xml
@@ -0,0 +1,43 @@
+<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='iscsi' name='iqn.1992-01.com.example:storage/2'>
+ <host name='example.org' port='6000'/>
+ <auth username='myname'>
+ <secret type='iscsi' 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-rbd-source-auth-both.xml b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-rbd-source-auth-both.xml
new file mode 100644
index 000000000..62a781cd3
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-rbd-source-auth-both.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='block' device='disk'>
+ <driver name='qemu' type='raw'/>
+ <source dev='/dev/HostVG/QEMUGuest1'/>
+ <target dev='hda' bus='ide'/>
+ <address type='drive' controller='0' bus='0' target='0' unit='0'/>
+ </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='vda' bus='virtio'/>
+ </disk>
+ <controller type='usb' index='0'/>
+ <controller type='ide' 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-rbd-source-auth.xml b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-rbd-source-auth.xml
new file mode 100644
index 000000000..d25e4148b
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-rbd-source-auth.xml
@@ -0,0 +1,42 @@
+<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='block' device='disk'>
+ <driver name='qemu' type='raw'/>
+ <source dev='/dev/HostVG/QEMUGuest1'/>
+ <target dev='hda' bus='ide'/>
+ <address type='drive' controller='0' bus='0' target='0' unit='0'/>
+ </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='vda' bus='virtio'/>
+ </disk>
+ <controller type='usb' index='0'/>
+ <controller type='ide' 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 c8c479cbd..d16b3b7b8 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -919,6 +919,8 @@ 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-iscsi-source-auth-both", NONE);
+ DO_TEST_PARSE_ERROR("disk-drive-network-rbd-source-auth-both", NONE);
DO_TEST("disk-drive-network-iscsi-lun",
QEMU_CAPS_NODEFCONFIG, QEMU_CAPS_VIRTIO_SCSI,
QEMU_CAPS_SCSI_BLOCK);
--
2.13.5
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On Fri, Sep 15, 2017 at 20:30:06 -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.
>
> For now just allow the format in the RNG and read it in domain_conf.
>
> 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>.
>
> The virschematest will read the new test files and validate from a
> RNG viewpoint things are fine
>
> Signed-off-by: John Ferlan <jferlan@redhat.com>
> ---
> docs/schemas/domaincommon.rng | 20 +++++++-
> src/conf/domain_conf.c | 53 ++++++++++++++++++++--
> ...v-disk-drive-network-iscsi-source-auth-both.xml | 36 +++++++++++++++
> ...l2argv-disk-drive-network-iscsi-source-auth.xml | 43 ++++++++++++++++++
> ...rgv-disk-drive-network-rbd-source-auth-both.xml | 45 ++++++++++++++++++
> ...xml2argv-disk-drive-network-rbd-source-auth.xml | 42 +++++++++++++++++
> tests/qemuxml2argvtest.c | 2 +
> 7 files changed, 237 insertions(+), 4 deletions(-)
> create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-iscsi-source-auth-both.xml
> create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-iscsi-source-auth.xml
> create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-rbd-source-auth-both.xml
> create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-rbd-source-auth.xml
>
> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
> index c9a4f7a9a..139f1eea2 100644
> --- a/docs/schemas/domaincommon.rng
> +++ b/docs/schemas/domaincommon.rng
> @@ -1578,11 +1578,29 @@
> <empty/>
> </element>
> </optional>
> + <optional>
> + <ref name="diskAuth"/>
> + </optional>
> <empty/>
> </interleave>
> </element>
> </define>
>
> + <define name="diskSourceNetworkProtocolISCSI">
> + <element name="source">
> + <attribute name="protocol">
> + <choice>
> + <value>iscsi</value>
> + </choice>
AFAIK Michal removed the <choice> here a few days ago.
> + </attribute>
> + <attribute name="name"/>
> + <ref name="diskSourceNetworkHost"/>
> + <optional>
> + <ref name="diskAuth"/>
> + </optional>
> + </element>
> + </define>
> +
> <define name="diskSourceNetworkProtocolHTTP">
> <element name="source">
> <attribute name="protocol">
[...]
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 8dca1357c..5c0218cdf 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
[...]
> @@ -8770,6 +8796,7 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt,
> char *serial = NULL;
> char *startupPolicy = NULL;
> virStorageAuthDefPtr authdef = NULL;
> + bool diskAuth = false;
I don't think you need this bool, since the variable above is non-null
only in the correct cases and remains so until the end of the func when
you steal the value.
> char *tray = NULL;
> char *removable = NULL;
> char *logical_block_size = NULL;
[...]
I think you will need to remember that the <auth> stuff was part of
<disk> and not source, since we can't change that. More on that in
review of the next patch.
> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-iscsi-source-auth.xml b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-iscsi-source-auth.xml
This file is not used. See below for a comment I had on the same case
for RBD.
> new file mode 100644
> index 000000000..af2d51fe9
> --- /dev/null
> +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-iscsi-source-auth.xml
> @@ -0,0 +1,43 @@
> +<domain type='qemu'>
> + <name>QEMUGuest1</name>
> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>
[...]
> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-rbd-source-auth-both.xml b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-rbd-source-auth-both.xml
> new file mode 100644
> index 000000000..62a781cd3
> --- /dev/null
> +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-rbd-source-auth-both.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='block' device='disk'>
> + <driver name='qemu' type='raw'/>
> + <source dev='/dev/HostVG/QEMUGuest1'/>
> + <target dev='hda' bus='ide'/>
> + <address type='drive' controller='0' bus='0' target='0' unit='0'/>
This disk is not needed.
> + </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='vda' bus='virtio'/>
> + </disk>
> + <controller type='usb' index='0'/>
> + <controller type='ide' 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-rbd-source-auth.xml b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-rbd-source-auth.xml
This file is added but not used in any test. Also when you are going to
make it used later, can't you just add another disk to an existing test
case? We have a few of cases which already test RBD auth. There's no
need to add so much of the bloat just to test the new syntax.
> new file mode 100644
> index 000000000..d25e4148b
> --- /dev/null
> +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-rbd-source-auth.xml
> @@ -0,0 +1,42 @@
> +<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='block' device='disk'>
> + <driver name='qemu' type='raw'/>
> + <source dev='/dev/HostVG/QEMUGuest1'/>
> + <target dev='hda' bus='ide'/>
> + <address type='drive' controller='0' bus='0' target='0' unit='0'/>
> + </disk>
Lots of unnecessary stuff ...
> + <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='vda' bus='virtio'/>
> + </disk>
[...]
> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
> index c8c479cbd..d16b3b7b8 100644
> --- a/tests/qemuxml2argvtest.c
> +++ b/tests/qemuxml2argvtest.c
> @@ -919,6 +919,8 @@ 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-iscsi-source-auth-both", NONE);
> + DO_TEST_PARSE_ERROR("disk-drive-network-rbd-source-auth-both", NONE);
Do we really need both if the code paths in the parser are the same?
ACK with the test file stuff sorted out and the fix to the schema
change.
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On 09/21/2017 08:38 AM, Peter Krempa wrote:
> On Fri, Sep 15, 2017 at 20:30:06 -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.
>>
>> For now just allow the format in the RNG and read it in domain_conf.
>>
>> 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>.
>>
>> The virschematest will read the new test files and validate from a
>> RNG viewpoint things are fine
>>
>> Signed-off-by: John Ferlan <jferlan@redhat.com>
>> ---
>> docs/schemas/domaincommon.rng | 20 +++++++-
>> src/conf/domain_conf.c | 53 ++++++++++++++++++++--
>> ...v-disk-drive-network-iscsi-source-auth-both.xml | 36 +++++++++++++++
>> ...l2argv-disk-drive-network-iscsi-source-auth.xml | 43 ++++++++++++++++++
>> ...rgv-disk-drive-network-rbd-source-auth-both.xml | 45 ++++++++++++++++++
>> ...xml2argv-disk-drive-network-rbd-source-auth.xml | 42 +++++++++++++++++
>> tests/qemuxml2argvtest.c | 2 +
>> 7 files changed, 237 insertions(+), 4 deletions(-)
>> create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-iscsi-source-auth-both.xml
>> create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-iscsi-source-auth.xml
>> create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-rbd-source-auth-both.xml
>> create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-rbd-source-auth.xml
>>
>> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
>> index c9a4f7a9a..139f1eea2 100644
>> --- a/docs/schemas/domaincommon.rng
>> +++ b/docs/schemas/domaincommon.rng
>> @@ -1578,11 +1578,29 @@
>> <empty/>
>> </element>
>> </optional>
>> + <optional>
>> + <ref name="diskAuth"/>
>> + </optional>
>> <empty/>
>> </interleave>
>> </element>
>> </define>
>>
>> + <define name="diskSourceNetworkProtocolISCSI">
>> + <element name="source">
>> + <attribute name="protocol">
>> + <choice>
>> + <value>iscsi</value>
>> + </choice>
>
> AFAIK Michal removed the <choice> here a few days ago.
>
ah... probably after I had originally copied RBD... I will remove it.
>> + </attribute>
>> + <attribute name="name"/>
>> + <ref name="diskSourceNetworkHost"/>
>> + <optional>
>> + <ref name="diskAuth"/>
>> + </optional>
>> + </element>
>> + </define>
>> +
>> <define name="diskSourceNetworkProtocolHTTP">
>> <element name="source">
>> <attribute name="protocol">
>
>
> [...]
>
>> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
>> index 8dca1357c..5c0218cdf 100644
>> --- a/src/conf/domain_conf.c
>> +++ b/src/conf/domain_conf.c
>
> [...]
>
>> @@ -8770,6 +8796,7 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt,
>> char *serial = NULL;
>> char *startupPolicy = NULL;
>> virStorageAuthDefPtr authdef = NULL;
>> + bool diskAuth = false;
>
> I don't think you need this bool, since the variable above is non-null
> only in the correct cases and remains so until the end of the func when
> you steal the value.
>
>> char *tray = NULL;
>> char *removable = NULL;
>> char *logical_block_size = NULL;
>
> [...]
>
>
> I think you will need to remember that the <auth> stuff was part of
> <disk> and not source, since we can't change that. More on that in
> review of the next patch.
>
Right - I figured I'd give it a go with the absolute steal and replace
algorithm before going with the determine where <auth> was found and be
sure to output exactly as read in.
>> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-iscsi-source-auth.xml b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-iscsi-source-auth.xml
>
> This file is not used. See below for a comment I had on the same case
> for RBD.
>
Dang - thought I caught all those when I split things up.
>> new file mode 100644
>> index 000000000..af2d51fe9
>> --- /dev/null
>> +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-iscsi-source-auth.xml
>> @@ -0,0 +1,43 @@
>> +<domain type='qemu'>
>> + <name>QEMUGuest1</name>
>> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>
>
> [...]
[...]
>> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
>> index c8c479cbd..d16b3b7b8 100644
>> --- a/tests/qemuxml2argvtest.c
>> +++ b/tests/qemuxml2argvtest.c
>> @@ -919,6 +919,8 @@ 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-iscsi-source-auth-both", NONE);
>> + DO_TEST_PARSE_ERROR("disk-drive-network-rbd-source-auth-both", NONE);
>
> Do we really need both if the code paths in the parser are the same?
>
No - they've been separate "historically", but I combine things and
clean up the testing portion a bit between this and the subsequent
patch. Same for the encryption options.
It'll be a repost effort too...
thanks -
John
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
© 2016 - 2026 Red Hat, Inc.