From 8002d3cb1b1c3d34ab89cf5c4fb7611ad64140e4 Mon Sep 17 00:00:00 2001 From: John Ferlan Date: Wed, 13 Sep 2017 10:25:25 -0400 Subject: [PATCH] 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 for the RBD and iSCSI prototcols. That way we can set up to allow the element to be formatted within the disk source. Since we've allowed the to be a child of , 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 or . For the argv2xml parsing, let's format under as a preference. Do not allow to be both a child of and . Modify the qemuxml2argvtest to add a parse failure when there is an as a child of *and* an as a child of . Add tests to validate that if the was found in , 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 as a child of . --- 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 + ...muargv2xml-disk-drive-network-rbd-auth.xml | 6 +- ...gv-disk-drive-network-source-auth-both.xml | 51 ++++++++++++++ ...l2argv-disk-drive-network-source-auth.args | 32 +++++++++ ...ml2argv-disk-drive-network-source-auth.xml | 45 +++++++++++++ tests/qemuxml2argvtest.c | 2 + ...2xmlout-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 58d86f9535..af1080683d 100644 --- a/docs/formatdomain.html.in +++ b/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' since 1.2.11 (QEMU only). +
auth
+
Since libvirt 3.9.0, the + auth element is supported for a disk + type "network" that is using a source + element with the protocol attributes "rbd" or "iscsi". + If present, the auth element provides the + authentication credentials needed to access the source. It + includes a mandatory attribute username, which + identifies the username to use during authentication, as well + as a sub-element secret with mandatory + attribute type, to tie back to + a libvirt secret object 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 uuid attribute + with the UUID of the secret object or a usage + attribute matching the key that was specified in the + secret object. +

@@ -3163,25 +3185,14 @@ are available, each defaulting to 0.

auth
-
The auth element is supported for a disk - type "network" that is using a source - element with the protocol attributes "rbd" or "iscsi". - If present, the auth element provides the - authentication credentials needed to access the source. It - includes a mandatory attribute username, which - identifies the username to use during authentication, as well - as a sub-element secret with mandatory - attribute type, to tie back to - a libvirt secret object 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 uuid attribute - with the UUID of the secret object or a usage - attribute matching the key that was specified in the - secret object. libvirt 0.9.7 +
Starting with libvirt 3.9.0 the + auth element is preferred to be a sub-element of + the source element. The element is still read and + managed as a disk sub-element. It is invalid to use + auth as both a sub-element of disk + and source. The auth element was + introduced as a disk sub-element in + libvirt 0.9.7.
geometry
The optional geometry element provides the diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 874af3ffa3..c99ee4f89b 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -1580,11 +1580,27 @@ + + + + + + + iscsi + + + + + + + + + @@ -1603,7 +1619,6 @@ sheepdog - iscsi ftp ftps tftp @@ -1663,6 +1678,7 @@ + diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index fe37b2bded..b207d33dae 100644 --- a/src/conf/domain_conf.c +++ b/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 as a child of and + * we find one as a child of , then force an error to + * avoid ambiguity */ + if (authdef && def->src->auth) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("an definition already found for " + "the 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 and found an child, + * then generate an error to avoid ambiguity */ + if (def->src->authInherited) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("an 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 + * for a volume source type. The 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 if defined there; otherwise, + * if defined as child of , then format later */ + if (def->src->auth && !def->src->authInherited) { if (virStorageAuthDefFormat(buf, def->src->auth) < 0) return -1; } diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c index d88183591d..3a2d2aa056 100644 --- a/src/util/virstoragefile.c +++ b/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 diff --git a/src/util/virstoragefile.h b/src/util/virstoragefile.h index 2f56aea1d4..3a6f9f2653 100644 --- a/src/util/virstoragefile.h +++ b/src/util/virstoragefile.h @@ -240,6 +240,7 @@ struct _virStorageSource { virStorageNetHostDefPtr hosts; virStorageSourcePoolDefPtr srcpool; virStorageAuthDefPtr auth; + bool authInherited; virStorageEncryptionPtr encryption; virObjectPtr privateData; 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 @@ - - - + + +
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 @@ + + QEMUGuest1 + c7a5fdbd-edaf-9455-926a-d65c16db1809 + 219136 + 219136 + 1 + + hvm + + + + destroy + restart + destroy + + /usr/bin/qemu-system-i686 + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + 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 @@ + + QEMUGuest1 + c7a5fdbd-edaf-9455-926a-d65c16db1809 + 219136 + 219136 + 1 + + hvm + + + + destroy + restart + destroy + + /usr/bin/qemu-system-i686 + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 01e7d6f6a9..ab5641484d 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 @@ + + QEMUGuest1 + c7a5fdbd-edaf-9455-926a-d65c16db1809 + 219136 + 219136 + 1 + + hvm + + + + destroy + restart + destroy + + /usr/bin/qemu-system-i686 + + + + + + + + + +
+ + + + + + + + + + + + +
+ + +
+ + + + + + + diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index 2185532a6c..4efaefe58a 100644 --- a/tests/qemuxml2xmltest.c +++ b/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); diff --git a/tests/virstoragetest.c b/tests/virstoragetest.c index 35e97ff263..e1d8751728 100644 --- a/tests/virstoragetest.c +++ b/tests/virstoragetest.c @@ -1362,6 +1362,9 @@ mymain(void) TEST_BACKING_PARSE("rbd:testshare:id=asdf:mon_host=example.com", "\n" " \n" + " \n" + " \n" + " \n" "\n"); TEST_BACKING_PARSE("nbd:example.org:6000:exportname=blah", "\n" @@ -1527,6 +1530,9 @@ mymain(void) "}", "\n" " \n" + " \n" + " \n" + " \n" "\n"); TEST_BACKING_PARSE("json:{\"file\":{\"driver\":\"rbd\"," "\"image\":\"test\","