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 1602ed3e9..e773b2939 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -2293,11 +2293,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'/> @@ -2366,20 +2366,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'> @@ -2659,6 +2659,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.8.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> @@ -3128,25 +3150,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.8.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 36e2966f2..0f8c0ab8f 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> @@ -1656,6 +1671,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 44cfb52b4..fa20840c0 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -8106,6 +8106,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, @@ -8192,6 +8215,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 */ @@ -8818,6 +8844,19 @@ 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 (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"); @@ -8875,6 +8914,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")) { @@ -9110,8 +9158,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; @@ -21800,6 +21848,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; } @@ -21985,7 +22044,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 484a5c806..488798252 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -2551,6 +2551,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 f7e897f25..b3a786b42 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 3f30296c0..e1326b925 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 000000000..fed75ad70 --- /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 000000000..23b1490ee --- /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 000000000..bd84cc42f --- /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 70be0c32d..a240a21a6 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -927,6 +927,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); @@ -935,6 +936,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 000000000..9dc063dea --- /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 4b2fbd990..f733953e5 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -473,6 +473,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-scsi-device", diff --git a/tests/virstoragetest.c b/tests/virstoragetest.c index ffebd4dc1..fe1521d9c 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.5

Add helper to manage the virStorageSourcePtr allocation for disk->src, disk->mirror, and disk->src->backingStore. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/conf/domain_conf.c | 37 ++++++++++++++++++++++++++++++------- 1 file changed, 30 insertions(+), 7 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index fa20840c0..73fd07d68 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -1703,6 +1703,27 @@ virDomainDefGetVcpusTopology(const virDomainDef *def, } +static virStorageSourcePtr +virDomainDiskStorageSourceNew(virDomainXMLOptionPtr xmlopt) +{ + virStorageSourcePtr src; + + if (VIR_ALLOC(src) < 0) + return NULL; + + if (xmlopt && + xmlopt->privateData.diskSrcNew && + !(src->privateData = xmlopt->privateData.diskSrcNew())) + goto error; + + return src; + + error: + virStorageSourceFree(src); + return NULL; +} + + virDomainDiskDefPtr virDomainDiskDefNew(virDomainXMLOptionPtr xmlopt) { @@ -1711,7 +1732,7 @@ virDomainDiskDefNew(virDomainXMLOptionPtr xmlopt) if (VIR_ALLOC(ret) < 0) return NULL; - if (VIR_ALLOC(ret->src) < 0) + if (!(ret->src = virDomainDiskStorageSourceNew(xmlopt))) goto error; if (xmlopt && @@ -8234,7 +8255,8 @@ virDomainDiskSourceParse(xmlNodePtr node, static int -virDomainDiskBackingStoreParse(xmlXPathContextPtr ctxt, +virDomainDiskBackingStoreParse(virDomainXMLOptionPtr xmlopt, + xmlXPathContextPtr ctxt, virStorageSourcePtr src) { virStorageSourcePtr backingStore = NULL; @@ -8249,7 +8271,7 @@ virDomainDiskBackingStoreParse(xmlXPathContextPtr ctxt, goto cleanup; } - if (VIR_ALLOC(backingStore) < 0) + if (!(backingStore = virDomainDiskStorageSourceNew(xmlopt))) goto cleanup; if (!(type = virXMLPropString(ctxt->node, "type"))) { @@ -8285,7 +8307,7 @@ virDomainDiskBackingStoreParse(xmlXPathContextPtr ctxt, } if (virDomainDiskSourceParse(source, ctxt, backingStore) < 0 || - virDomainDiskBackingStoreParse(ctxt, backingStore) < 0) + virDomainDiskBackingStoreParse(xmlopt, ctxt, backingStore) < 0) goto cleanup; src->backingStore = backingStore; @@ -8386,6 +8408,7 @@ virDomainDiskDefIotuneParse(virDomainDiskDefPtr def, static int virDomainDiskDefMirrorParse(virDomainDiskDefPtr def, xmlNodePtr cur, + virDomainXMLOptionPtr xmlopt, xmlXPathContextPtr ctxt) { xmlNodePtr mirrorNode; @@ -8395,7 +8418,7 @@ virDomainDiskDefMirrorParse(virDomainDiskDefPtr def, char *blockJob = NULL; int ret = -1; - if (VIR_ALLOC(def->mirror) < 0) + if (!(def->mirror = virDomainDiskStorageSourceNew(xmlopt))) goto cleanup; if ((blockJob = virXMLPropString(cur, "job"))) { @@ -8910,7 +8933,7 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt, } else if (!def->mirror && virXMLNodeNameEqual(cur, "mirror") && !(flags & VIR_DOMAIN_DEF_PARSE_INACTIVE)) { - if (virDomainDiskDefMirrorParse(def, cur, ctxt) < 0) + if (virDomainDiskDefMirrorParse(def, cur, xmlopt, ctxt) < 0) goto error; } else if (!authdef && virXMLNodeNameEqual(cur, "auth")) { @@ -9174,7 +9197,7 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt, product = NULL; if (!(flags & VIR_DOMAIN_DEF_PARSE_DISK_SOURCE)) { - if (virDomainDiskBackingStoreParse(ctxt, def->src) < 0) + if (virDomainDiskBackingStoreParse(xmlopt, ctxt, def->src) < 0) goto error; } -- 2.13.5

Relocate into disk source private (qemuDomainDiskSrcPrivatePtr) Since the secret information is really _virStorageSource specific piece of data, let's manage the privateData from there instead of at the Disk level. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_command.c | 6 ++++-- src/qemu/qemu_domain.c | 9 +++++---- src/qemu/qemu_domain.h | 4 ---- src/qemu/qemu_hotplug.c | 11 ++++++++--- 4 files changed, 17 insertions(+), 13 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index abeb24846..a5033af6d 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -1349,7 +1349,8 @@ qemuBuildDriveSourceStr(virDomainDiskDefPtr disk, { int actualType = virStorageSourceGetActualType(disk->src); qemuDomainDiskPrivatePtr diskPriv = QEMU_DOMAIN_DISK_PRIVATE(disk); - qemuDomainSecretInfoPtr secinfo = diskPriv->secinfo; + qemuDomainDiskSrcPrivatePtr diskSrcPriv = QEMU_DOMAIN_DISK_SRC_PRIVATE(disk->src); + qemuDomainSecretInfoPtr secinfo = diskSrcPriv->secinfo; qemuDomainSecretInfoPtr encinfo = diskPriv->encinfo; virJSONValuePtr srcprops = NULL; char *source = NULL; @@ -2180,7 +2181,8 @@ qemuBuildDiskDriveCommandLine(virCommandPtr cmd, bool driveBoot = false; virDomainDiskDefPtr disk = def->disks[i]; qemuDomainDiskPrivatePtr diskPriv = QEMU_DOMAIN_DISK_PRIVATE(disk); - qemuDomainSecretInfoPtr secinfo = diskPriv->secinfo; + qemuDomainDiskSrcPrivatePtr diskSrcPriv = QEMU_DOMAIN_DISK_SRC_PRIVATE(disk->src); + qemuDomainSecretInfoPtr secinfo = diskSrcPriv->secinfo; qemuDomainSecretInfoPtr encinfo = diskPriv->encinfo; /* PowerPC pseries based VMs do not support floppy device */ diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 074fb3185..198fac408 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -921,7 +921,6 @@ qemuDomainDiskPrivateDispose(void *obj) { qemuDomainDiskPrivatePtr priv = obj; - qemuDomainSecretInfoFree(&priv->secinfo); qemuDomainSecretInfoFree(&priv->encinfo); } @@ -1337,9 +1336,10 @@ void qemuDomainSecretDiskDestroy(virDomainDiskDefPtr disk) { qemuDomainDiskPrivatePtr diskPriv = QEMU_DOMAIN_DISK_PRIVATE(disk); + qemuDomainDiskSrcPrivatePtr diskSrcPriv = QEMU_DOMAIN_DISK_SRC_PRIVATE(disk->src); - if (diskPriv && diskPriv->secinfo) - qemuDomainSecretInfoFree(&diskPriv->secinfo); + if (diskSrcPriv && diskSrcPriv->secinfo) + qemuDomainSecretInfoFree(&diskSrcPriv->secinfo); if (diskPriv && diskPriv->encinfo) qemuDomainSecretInfoFree(&diskPriv->encinfo); @@ -1388,6 +1388,7 @@ qemuDomainSecretDiskPrepare(virConnectPtr conn, { virStorageSourcePtr src = disk->src; qemuDomainDiskPrivatePtr diskPriv = QEMU_DOMAIN_DISK_PRIVATE(disk); + qemuDomainDiskSrcPrivatePtr diskSrcPriv = QEMU_DOMAIN_DISK_SRC_PRIVATE(disk->src); if (qemuDomainSecretDiskCapable(src)) { virSecretUsageType usageType = VIR_SECRET_USAGE_TYPE_ISCSI; @@ -1395,7 +1396,7 @@ qemuDomainSecretDiskPrepare(virConnectPtr conn, if (src->protocol == VIR_STORAGE_NET_PROTOCOL_RBD) usageType = VIR_SECRET_USAGE_TYPE_CEPH; - if (!(diskPriv->secinfo = + if (!(diskSrcPriv->secinfo = qemuDomainSecretInfoNew(conn, priv, disk->info.alias, usageType, src->auth->username, &src->auth->seclookupdef, false))) diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index f2c086d5d..9f0582ee2 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -345,10 +345,6 @@ struct _qemuDomainDiskPrivate { bool migrating; /* the disk is being migrated */ - /* for storage devices using auth/secret - * NB: *not* to be written to qemu domain object XML */ - qemuDomainSecretInfoPtr secinfo; - /* for storage devices using encryption/secret * Can have both <auth> and <encryption> for some disks * NB:*not* to be written to qemu domain object XML */ diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 7592049ea..ad13f93e8 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -218,6 +218,7 @@ qemuDomainChangeEjectableMedia(virQEMUDriverPtr driver, char *driveAlias = NULL; qemuDomainObjPrivatePtr priv = vm->privateData; qemuDomainDiskPrivatePtr diskPriv = QEMU_DOMAIN_DISK_PRIVATE(disk); + qemuDomainDiskSrcPrivatePtr diskSrcPriv = QEMU_DOMAIN_DISK_SRC_PRIVATE(disk->src); const char *format = NULL; char *sourcestr = NULL; @@ -259,7 +260,7 @@ qemuDomainChangeEjectableMedia(virQEMUDriverPtr driver, } if (!virStorageSourceIsEmpty(newsrc)) { - if (qemuGetDriveSourceString(newsrc, diskPriv->secinfo, &sourcestr) < 0) + if (qemuGetDriveSourceString(newsrc, diskSrcPriv->secinfo, &sourcestr) < 0) goto error; if (virStorageSourceGetActualType(newsrc) != VIR_STORAGE_TYPE_DIR) { @@ -329,6 +330,7 @@ qemuDomainAttachVirtioDiskDevice(virConnectPtr conn, virJSONValuePtr secobjProps = NULL; virJSONValuePtr encobjProps = NULL; qemuDomainDiskPrivatePtr diskPriv; + qemuDomainDiskSrcPrivatePtr diskSrcPriv; qemuDomainSecretInfoPtr secinfo; qemuDomainSecretInfoPtr encinfo; @@ -366,7 +368,8 @@ qemuDomainAttachVirtioDiskDevice(virConnectPtr conn, goto error; diskPriv = QEMU_DOMAIN_DISK_PRIVATE(disk); - secinfo = diskPriv->secinfo; + diskSrcPriv = QEMU_DOMAIN_DISK_SRC_PRIVATE(disk->src); + secinfo = diskSrcPriv->secinfo; if (secinfo && secinfo->type == VIR_DOMAIN_SECRET_INFO_TYPE_AES) { if (qemuBuildSecretInfoProps(secinfo, &secobjProps) < 0) goto error; @@ -621,6 +624,7 @@ qemuDomainAttachSCSIDisk(virConnectPtr conn, virJSONValuePtr encobjProps = NULL; virJSONValuePtr secobjProps = NULL; qemuDomainDiskPrivatePtr diskPriv; + qemuDomainDiskSrcPrivatePtr diskSrcPriv; qemuDomainSecretInfoPtr encinfo; qemuDomainSecretInfoPtr secinfo; @@ -654,7 +658,8 @@ qemuDomainAttachSCSIDisk(virConnectPtr conn, goto error; diskPriv = QEMU_DOMAIN_DISK_PRIVATE(disk); - secinfo = diskPriv->secinfo; + diskSrcPriv = QEMU_DOMAIN_DISK_SRC_PRIVATE(disk->src); + secinfo = diskSrcPriv->secinfo; if (secinfo && secinfo->type == VIR_DOMAIN_SECRET_INFO_TYPE_AES) { if (qemuBuildSecretInfoProps(secinfo, &secobjProps) < 0) goto error; -- 2.13.5

Since the encryption information can also be disk source specific move it from _qemuDomainDiskPrivate to _qemuDomainDiskSrcPrivate. Since the last allocated element from _qemuDomainDiskPrivate is removed, that means we no longer need qemuDomainDiskPrivateDispose. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_command.c | 6 ++---- src/qemu/qemu_domain.c | 21 +++++---------------- src/qemu/qemu_domain.h | 10 +++++----- src/qemu/qemu_hotplug.c | 8 ++------ 4 files changed, 14 insertions(+), 31 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index a5033af6d..9e1da10c9 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -1348,10 +1348,9 @@ qemuBuildDriveSourceStr(virDomainDiskDefPtr disk, virQEMUCapsPtr qemuCaps) { int actualType = virStorageSourceGetActualType(disk->src); - qemuDomainDiskPrivatePtr diskPriv = QEMU_DOMAIN_DISK_PRIVATE(disk); qemuDomainDiskSrcPrivatePtr diskSrcPriv = QEMU_DOMAIN_DISK_SRC_PRIVATE(disk->src); qemuDomainSecretInfoPtr secinfo = diskSrcPriv->secinfo; - qemuDomainSecretInfoPtr encinfo = diskPriv->encinfo; + qemuDomainSecretInfoPtr encinfo = diskSrcPriv->encinfo; virJSONValuePtr srcprops = NULL; char *source = NULL; int ret = -1; @@ -2180,10 +2179,9 @@ qemuBuildDiskDriveCommandLine(virCommandPtr cmd, unsigned int bootindex = 0; bool driveBoot = false; virDomainDiskDefPtr disk = def->disks[i]; - qemuDomainDiskPrivatePtr diskPriv = QEMU_DOMAIN_DISK_PRIVATE(disk); qemuDomainDiskSrcPrivatePtr diskSrcPriv = QEMU_DOMAIN_DISK_SRC_PRIVATE(disk->src); qemuDomainSecretInfoPtr secinfo = diskSrcPriv->secinfo; - qemuDomainSecretInfoPtr encinfo = diskPriv->encinfo; + qemuDomainSecretInfoPtr encinfo = diskSrcPriv->encinfo; /* PowerPC pseries based VMs do not support floppy device */ if (disk->device == VIR_DOMAIN_DISK_DEVICE_FLOPPY && diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 198fac408..1fd440284 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -884,7 +884,6 @@ qemuDomainSecretInfoFree(qemuDomainSecretInfoPtr *secinfo) static virClassPtr qemuDomainDiskPrivateClass; -static void qemuDomainDiskPrivateDispose(void *obj); static int qemuDomainDiskPrivateOnceInit(void) @@ -892,7 +891,7 @@ qemuDomainDiskPrivateOnceInit(void) qemuDomainDiskPrivateClass = virClassNew(virClassForObject(), "qemuDomainDiskPrivate", sizeof(qemuDomainDiskPrivate), - qemuDomainDiskPrivateDispose); + NULL); if (!qemuDomainDiskPrivateClass) return -1; else @@ -916,15 +915,6 @@ qemuDomainDiskPrivateNew(void) } -static void -qemuDomainDiskPrivateDispose(void *obj) -{ - qemuDomainDiskPrivatePtr priv = obj; - - qemuDomainSecretInfoFree(&priv->encinfo); -} - - static virClassPtr qemuDomainDiskSrcPrivateClass; static void qemuDomainDiskSrcPrivateDispose(void *obj); @@ -964,6 +954,7 @@ qemuDomainDiskSrcPrivateDispose(void *obj) qemuDomainDiskSrcPrivatePtr priv = obj; qemuDomainSecretInfoFree(&priv->secinfo); + qemuDomainSecretInfoFree(&priv->encinfo); } @@ -1335,14 +1326,13 @@ qemuDomainSecretInfoTLSNew(virConnectPtr conn, void qemuDomainSecretDiskDestroy(virDomainDiskDefPtr disk) { - qemuDomainDiskPrivatePtr diskPriv = QEMU_DOMAIN_DISK_PRIVATE(disk); qemuDomainDiskSrcPrivatePtr diskSrcPriv = QEMU_DOMAIN_DISK_SRC_PRIVATE(disk->src); if (diskSrcPriv && diskSrcPriv->secinfo) qemuDomainSecretInfoFree(&diskSrcPriv->secinfo); - if (diskPriv && diskPriv->encinfo) - qemuDomainSecretInfoFree(&diskPriv->encinfo); + if (diskSrcPriv && diskSrcPriv->encinfo) + qemuDomainSecretInfoFree(&diskSrcPriv->encinfo); } @@ -1387,7 +1377,6 @@ qemuDomainSecretDiskPrepare(virConnectPtr conn, virDomainDiskDefPtr disk) { virStorageSourcePtr src = disk->src; - qemuDomainDiskPrivatePtr diskPriv = QEMU_DOMAIN_DISK_PRIVATE(disk); qemuDomainDiskSrcPrivatePtr diskSrcPriv = QEMU_DOMAIN_DISK_SRC_PRIVATE(disk->src); if (qemuDomainSecretDiskCapable(src)) { @@ -1404,7 +1393,7 @@ qemuDomainSecretDiskPrepare(virConnectPtr conn, } if (qemuDomainDiskHasEncryptionSecret(src)) { - if (!(diskPriv->encinfo = + if (!(diskSrcPriv->encinfo = qemuDomainSecretInfoNew(conn, priv, disk->info.alias, VIR_SECRET_USAGE_TYPE_VOLUME, NULL, &src->encryption->secrets[0]->seclookupdef, diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index 9f0582ee2..81d30ab63 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -345,11 +345,6 @@ struct _qemuDomainDiskPrivate { bool migrating; /* the disk is being migrated */ - /* for storage devices using encryption/secret - * Can have both <auth> and <encryption> for some disks - * NB:*not* to be written to qemu domain object XML */ - qemuDomainSecretInfoPtr encinfo; - /* information about the device */ bool tray; /* device has tray */ bool removable; /* device media can be removed/changed */ @@ -366,6 +361,11 @@ struct _qemuDomainDiskSrcPrivate { /* for each storage source using auth/secret * NB: *not* to be written to qemu domain object XML */ qemuDomainSecretInfoPtr secinfo; + + /* for storage devices using encryption/secret + * Can have both <auth> and <encryption> for some disks + * NB:*not* to be written to qemu domain object XML */ + qemuDomainSecretInfoPtr encinfo; }; # define QEMU_DOMAIN_HOSTDEV_PRIVATE(hostdev) \ diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index ad13f93e8..3f1e6d986 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -329,7 +329,6 @@ qemuDomainAttachVirtioDiskDevice(virConnectPtr conn, const char *src = virDomainDiskGetSource(disk); virJSONValuePtr secobjProps = NULL; virJSONValuePtr encobjProps = NULL; - qemuDomainDiskPrivatePtr diskPriv; qemuDomainDiskSrcPrivatePtr diskSrcPriv; qemuDomainSecretInfoPtr secinfo; qemuDomainSecretInfoPtr encinfo; @@ -367,7 +366,6 @@ qemuDomainAttachVirtioDiskDevice(virConnectPtr conn, if (qemuDomainSecretDiskPrepare(conn, priv, disk) < 0) goto error; - diskPriv = QEMU_DOMAIN_DISK_PRIVATE(disk); diskSrcPriv = QEMU_DOMAIN_DISK_SRC_PRIVATE(disk->src); secinfo = diskSrcPriv->secinfo; if (secinfo && secinfo->type == VIR_DOMAIN_SECRET_INFO_TYPE_AES) { @@ -375,7 +373,7 @@ qemuDomainAttachVirtioDiskDevice(virConnectPtr conn, goto error; } - encinfo = diskPriv->encinfo; + encinfo = diskSrcPriv->encinfo; if (encinfo && qemuBuildSecretInfoProps(encinfo, &encobjProps) < 0) goto error; @@ -623,7 +621,6 @@ qemuDomainAttachSCSIDisk(virConnectPtr conn, virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); virJSONValuePtr encobjProps = NULL; virJSONValuePtr secobjProps = NULL; - qemuDomainDiskPrivatePtr diskPriv; qemuDomainDiskSrcPrivatePtr diskSrcPriv; qemuDomainSecretInfoPtr encinfo; qemuDomainSecretInfoPtr secinfo; @@ -657,7 +654,6 @@ qemuDomainAttachSCSIDisk(virConnectPtr conn, if (qemuDomainSecretDiskPrepare(conn, priv, disk) < 0) goto error; - diskPriv = QEMU_DOMAIN_DISK_PRIVATE(disk); diskSrcPriv = QEMU_DOMAIN_DISK_SRC_PRIVATE(disk->src); secinfo = diskSrcPriv->secinfo; if (secinfo && secinfo->type == VIR_DOMAIN_SECRET_INFO_TYPE_AES) { @@ -665,7 +661,7 @@ qemuDomainAttachSCSIDisk(virConnectPtr conn, goto error; } - encinfo = diskPriv->encinfo; + encinfo = diskSrcPriv->encinfo; if (encinfo && qemuBuildSecretInfoProps(encinfo, &encobjProps) < 0) goto error; -- 2.13.5

Rather than picking apart the two pieces we need/want (path, hosts, and auth)- let's just use the new virDomainDiskStorageSourceNew API in order to allocate and use a virStorageSourcePtr. The end result is that qemuBuildSCSIiSCSIHostdevDrvStr doesn't need to "fake" one for the qemuBuildNetworkDriveStr call. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/conf/domain_conf.c | 71 ++++++++++++++++++++++++++++--------------------- src/conf/domain_conf.h | 5 +--- src/qemu/qemu_command.c | 10 +------ src/qemu/qemu_domain.c | 8 +++--- src/qemu/qemu_hotplug.c | 2 +- 5 files changed, 47 insertions(+), 49 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index db52b060a..54e60aa72 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -2497,10 +2497,9 @@ virDomainHostdevSubsysSCSIiSCSIClear(virDomainHostdevSubsysSCSIiSCSIPtr iscsisrc { if (!iscsisrc) return; - VIR_FREE(iscsisrc->path); - virStorageNetHostDefFree(iscsisrc->nhosts, iscsisrc->hosts); - virStorageAuthDefFree(iscsisrc->auth); - iscsisrc->auth = NULL; + + virStorageSourceFree(iscsisrc->src); + iscsisrc->src = NULL; } @@ -4373,7 +4372,7 @@ virDomainHostdevDefPostParse(virDomainHostdevDefPtr dev, if (scsisrc->protocol == VIR_DOMAIN_HOSTDEV_SCSI_PROTOCOL_TYPE_ISCSI) { virDomainHostdevSubsysSCSIiSCSIPtr iscsisrc = &scsisrc->u.iscsi; - if (virDomainPostParseCheckISCSIPath(&iscsisrc->path) < 0) + if (virDomainPostParseCheckISCSIPath(&iscsisrc->src->path) < 0) return -1; } @@ -6908,7 +6907,8 @@ virDomainHostdevSubsysSCSIHostDefParseXML(xmlNodePtr sourcenode, } static int -virDomainHostdevSubsysSCSIiSCSIDefParseXML(xmlNodePtr sourcenode, +virDomainHostdevSubsysSCSIiSCSIDefParseXML(virDomainXMLOptionPtr xmlopt, + xmlNodePtr sourcenode, virDomainHostdevSubsysSCSIPtr def) { int ret = -1; @@ -6917,24 +6917,29 @@ virDomainHostdevSubsysSCSIiSCSIDefParseXML(xmlNodePtr sourcenode, virStorageAuthDefPtr authdef = NULL; virDomainHostdevSubsysSCSIiSCSIPtr iscsisrc = &def->u.iscsi; - /* Similar to virDomainDiskSourceParse for a VIR_STORAGE_TYPE_NETWORK */ + /* For the purposes of command line creation, this needs to look + * like a disk storage source */ + if (!(iscsisrc->src = virDomainDiskStorageSourceNew(xmlopt))) + return -1; + iscsisrc->src->type = VIR_STORAGE_TYPE_NETWORK; + iscsisrc->src->protocol = VIR_STORAGE_NET_PROTOCOL_ISCSI; - if (!(iscsisrc->path = virXMLPropString(sourcenode, "name"))) { + if (!(iscsisrc->src->path = virXMLPropString(sourcenode, "name"))) { virReportError(VIR_ERR_XML_ERROR, "%s", _("missing iSCSI hostdev source path name")); goto cleanup; } - if (virDomainStorageNetworkParseHosts(sourcenode, &iscsisrc->hosts, - &iscsisrc->nhosts) < 0) + if (virDomainStorageNetworkParseHosts(sourcenode, &iscsisrc->src->hosts, + &iscsisrc->src->nhosts) < 0) goto cleanup; - if (iscsisrc->nhosts < 1) { + if (iscsisrc->src->nhosts < 1) { virReportError(VIR_ERR_XML_ERROR, "%s", _("missing the host address for the iSCSI hostdev")); goto cleanup; } - if (iscsisrc->nhosts > 1) { + if (iscsisrc->src->nhosts > 1) { virReportError(VIR_ERR_XML_ERROR, "%s", _("only one source host address may be specified " "for the iSCSI hostdev")); @@ -6960,7 +6965,7 @@ virDomainHostdevSubsysSCSIiSCSIDefParseXML(xmlNodePtr sourcenode, authdef->secrettype); goto cleanup; } - iscsisrc->auth = authdef; + iscsisrc->src->auth = authdef; authdef = NULL; } cur = cur->next; @@ -6973,7 +6978,8 @@ virDomainHostdevSubsysSCSIiSCSIDefParseXML(xmlNodePtr sourcenode, } static int -virDomainHostdevSubsysSCSIDefParseXML(xmlNodePtr sourcenode, +virDomainHostdevSubsysSCSIDefParseXML(virDomainXMLOptionPtr xmlopt, + xmlNodePtr sourcenode, virDomainHostdevSubsysSCSIPtr scsisrc) { char *protocol = NULL; @@ -6991,7 +6997,8 @@ virDomainHostdevSubsysSCSIDefParseXML(xmlNodePtr sourcenode, } if (scsisrc->protocol == VIR_DOMAIN_HOSTDEV_SCSI_PROTOCOL_TYPE_ISCSI) - ret = virDomainHostdevSubsysSCSIiSCSIDefParseXML(sourcenode, scsisrc); + ret = virDomainHostdevSubsysSCSIiSCSIDefParseXML(xmlopt, sourcenode, + scsisrc); else ret = virDomainHostdevSubsysSCSIHostDefParseXML(sourcenode, scsisrc); @@ -7092,7 +7099,8 @@ virDomainHostdevSubsysMediatedDevDefParseXML(virDomainHostdevDefPtr def, } static int -virDomainHostdevDefParseXMLSubsys(xmlNodePtr node, +virDomainHostdevDefParseXMLSubsys(virDomainXMLOptionPtr xmlopt, + xmlNodePtr node, xmlXPathContextPtr ctxt, const char *type, virDomainHostdevDefPtr def, @@ -7236,7 +7244,7 @@ virDomainHostdevDefParseXMLSubsys(xmlNodePtr node, break; case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI: - if (virDomainHostdevSubsysSCSIDefParseXML(sourcenode, scsisrc) < 0) + if (virDomainHostdevSubsysSCSIDefParseXML(xmlopt, sourcenode, scsisrc) < 0) goto error; break; @@ -10101,7 +10109,8 @@ virDomainFSDefParseXML(xmlNodePtr node, } static int -virDomainActualNetDefParseXML(xmlNodePtr node, +virDomainActualNetDefParseXML(virDomainXMLOptionPtr xmlopt, + xmlNodePtr node, xmlXPathContextPtr ctxt, virDomainNetDefPtr parent, virDomainActualNetDefPtr *def, @@ -10211,7 +10220,7 @@ virDomainActualNetDefParseXML(xmlNodePtr node, VIR_STRDUP(addrtype, "usb") < 0) goto error; hostdev->mode = VIR_DOMAIN_HOSTDEV_MODE_SUBSYS; - if (virDomainHostdevDefParseXMLSubsys(node, ctxt, addrtype, + if (virDomainHostdevDefParseXMLSubsys(xmlopt, node, ctxt, addrtype, hostdev, flags) < 0) { goto error; } @@ -10546,7 +10555,7 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt, (flags & VIR_DOMAIN_DEF_PARSE_ACTUAL_NET) && def->type == VIR_DOMAIN_NET_TYPE_NETWORK && virXMLNodeNameEqual(cur, "actual")) { - if (virDomainActualNetDefParseXML(cur, ctxt, def, + if (virDomainActualNetDefParseXML(xmlopt, cur, ctxt, def, &actual, flags) < 0) { goto error; } @@ -10823,7 +10832,7 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt, VIR_STRDUP(addrtype, "usb") < 0) goto error; hostdev->mode = VIR_DOMAIN_HOSTDEV_MODE_SUBSYS; - if (virDomainHostdevDefParseXMLSubsys(node, ctxt, addrtype, + if (virDomainHostdevDefParseXMLSubsys(xmlopt, node, ctxt, addrtype, hostdev, flags) < 0) { goto error; } @@ -14422,7 +14431,7 @@ virDomainHostdevDefParseXML(virDomainXMLOptionPtr xmlopt, switch (def->mode) { case VIR_DOMAIN_HOSTDEV_MODE_SUBSYS: /* parse managed/mode/type, and the <source> element */ - if (virDomainHostdevDefParseXMLSubsys(node, ctxt, type, def, flags) < 0) + if (virDomainHostdevDefParseXMLSubsys(xmlopt, node, ctxt, type, def, flags) < 0) goto error; break; case VIR_DOMAIN_HOSTDEV_MODE_CAPABILITIES: @@ -15396,9 +15405,9 @@ virDomainHostdevMatchSubsysSCSIiSCSI(virDomainHostdevDefPtr first, virDomainHostdevSubsysSCSIiSCSIPtr second_iscsisrc = &second->source.subsys.u.scsi.u.iscsi; - if (STREQ(first_iscsisrc->hosts[0].name, second_iscsisrc->hosts[0].name) && - first_iscsisrc->hosts[0].port == second_iscsisrc->hosts[0].port && - STREQ(first_iscsisrc->path, second_iscsisrc->path)) + if (STREQ(first_iscsisrc->src->hosts[0].name, second_iscsisrc->src->hosts[0].name) && + first_iscsisrc->src->hosts[0].port == second_iscsisrc->src->hosts[0].port && + STREQ(first_iscsisrc->src->path, second_iscsisrc->src->path)) return 1; return 0; } @@ -22675,7 +22684,7 @@ virDomainHostdevDefFormatSubsys(virBufferPtr buf, virDomainHostdevSubsysSCSIProtocolTypeToString(scsisrc->protocol); virBufferAsprintf(buf, " protocol='%s' name='%s'", - protocol, iscsisrc->path); + protocol, iscsisrc->src->path); } if (def->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI_HOST) { @@ -22727,9 +22736,9 @@ virDomainHostdevDefFormatSubsys(virBufferPtr buf, case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI: if (scsisrc->protocol == VIR_DOMAIN_HOSTDEV_SCSI_PROTOCOL_TYPE_ISCSI) { virBufferAddLit(buf, "<host"); - virBufferEscapeString(buf, " name='%s'", iscsisrc->hosts[0].name); - if (iscsisrc->hosts[0].port) - virBufferAsprintf(buf, " port='%u'", iscsisrc->hosts[0].port); + virBufferEscapeString(buf, " name='%s'", iscsisrc->src->hosts[0].name); + if (iscsisrc->src->hosts[0].port) + virBufferAsprintf(buf, " port='%u'", iscsisrc->src->hosts[0].port); virBufferAddLit(buf, "/>\n"); } else { virBufferAsprintf(buf, "<adapter name='%s'/>\n", @@ -22756,8 +22765,8 @@ virDomainHostdevDefFormatSubsys(virBufferPtr buf, if (def->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI && scsisrc->protocol == VIR_DOMAIN_HOSTDEV_SCSI_PROTOCOL_TYPE_ISCSI && - iscsisrc->auth) { - if (virStorageAuthDefFormat(buf, iscsisrc->auth) < 0) + iscsisrc->src->auth) { + if (virStorageAuthDefFormat(buf, iscsisrc->src->auth) < 0) return -1; } diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index e727de423..a962a5b28 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -357,10 +357,7 @@ struct _virDomainHostdevSubsysSCSIHost { typedef struct _virDomainHostdevSubsysSCSIiSCSI virDomainHostdevSubsysSCSIiSCSI; typedef virDomainHostdevSubsysSCSIiSCSI *virDomainHostdevSubsysSCSIiSCSIPtr; struct _virDomainHostdevSubsysSCSIiSCSI { - char *path; - size_t nhosts; - virStorageNetHostDefPtr hosts; - virStorageAuthDefPtr auth; + virStorageSourcePtr src; }; typedef struct _virDomainHostdevSubsysSCSI virDomainHostdevSubsysSCSI; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 9e1da10c9..0dd5d08c1 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -4881,21 +4881,13 @@ static char * qemuBuildSCSIiSCSIHostdevDrvStr(virDomainHostdevDefPtr dev) { char *source = NULL; - virStorageSource src; qemuDomainHostdevPrivatePtr hostdevPriv = QEMU_DOMAIN_HOSTDEV_PRIVATE(dev); - memset(&src, 0, sizeof(src)); - virDomainHostdevSubsysSCSIPtr scsisrc = &dev->source.subsys.u.scsi; virDomainHostdevSubsysSCSIiSCSIPtr iscsisrc = &scsisrc->u.iscsi; - src.protocol = VIR_STORAGE_NET_PROTOCOL_ISCSI; - src.path = iscsisrc->path; - src.hosts = iscsisrc->hosts; - src.nhosts = iscsisrc->nhosts; - /* Rather than pull what we think we want - use the network disk code */ - source = qemuBuildNetworkDriveStr(&src, hostdevPriv->secinfo); + source = qemuBuildNetworkDriveStr(iscsisrc->src, hostdevPriv->secinfo); return source; } diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 1fd440284..1667e66df 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -1442,7 +1442,7 @@ qemuDomainSecretHostdevPrepare(virConnectPtr conn, virDomainHostdevSubsysSCSIiSCSIPtr iscsisrc = &scsisrc->u.iscsi; if (scsisrc->protocol == VIR_DOMAIN_HOSTDEV_SCSI_PROTOCOL_TYPE_ISCSI && - iscsisrc->auth) { + iscsisrc->src->auth) { qemuDomainHostdevPrivatePtr hostdevPriv = QEMU_DOMAIN_HOSTDEV_PRIVATE(hostdev); @@ -1450,8 +1450,8 @@ qemuDomainSecretHostdevPrepare(virConnectPtr conn, if (!(hostdevPriv->secinfo = qemuDomainSecretInfoNew(conn, priv, hostdev->info->alias, VIR_SECRET_USAGE_TYPE_ISCSI, - iscsisrc->auth->username, - &iscsisrc->auth->seclookupdef, + iscsisrc->src->auth->username, + &iscsisrc->src->auth->seclookupdef, false))) return -1; } @@ -7885,7 +7885,7 @@ qemuDomainGetHostdevPath(virDomainDefPtr def, /* Follow qemuSetupDiskCgroup() and qemuSetImageCgroupInternal() * which does nothing for non local storage */ - VIR_DEBUG("Not updating /dev for hostdev iSCSI path '%s'", iscsisrc->path); + VIR_DEBUG("Not updating /dev for hostdev iSCSI path '%s'", iscsisrc->src->path); } else { virDomainHostdevSubsysSCSIHostPtr scsihostsrc = &scsisrc->u.host; scsi = virSCSIDeviceNew(NULL, diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 3f1e6d986..c3def0d61 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -4899,7 +4899,7 @@ int qemuDomainDetachHostDevice(virQEMUDriverPtr driver, virDomainHostdevSubsysSCSIiSCSIPtr iscsisrc = &scsisrc->u.iscsi; virReportError(VIR_ERR_OPERATION_FAILED, _("host scsi iSCSI path %s not found"), - iscsisrc->path); + iscsisrc->src->path); } else { virDomainHostdevSubsysSCSIHostPtr scsihostsrc = &scsisrc->u.host; -- 2.13.5

Since it's not longer used to shuttle the @secinfo, let's remove the private hostdev completely. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/conf/domain_conf.c | 12 ++--------- src/conf/domain_conf.h | 4 +--- src/lxc/lxc_native.c | 2 +- src/qemu/qemu_domain.c | 50 +++---------------------------------------- src/qemu/qemu_domain.h | 14 ------------ src/qemu/qemu_parse_command.c | 4 ++-- src/vbox/vbox_common.c | 2 +- src/xenconfig/xen_common.c | 2 +- src/xenconfig/xen_sxpr.c | 2 +- src/xenconfig/xen_xl.c | 2 +- tests/virhostdevtest.c | 2 +- 11 files changed, 14 insertions(+), 82 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 54e60aa72..dfe22e511 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -2468,7 +2468,7 @@ void virDomainVideoDefFree(virDomainVideoDefPtr def) virDomainHostdevDefPtr -virDomainHostdevDefNew(virDomainXMLOptionPtr xmlopt) +virDomainHostdevDefNew(void) { virDomainHostdevDefPtr def; @@ -2478,11 +2478,6 @@ virDomainHostdevDefNew(virDomainXMLOptionPtr xmlopt) if (VIR_ALLOC(def->info) < 0) goto error; - if (xmlopt && - xmlopt->privateData.hostdevNew && - !(def->privateData = xmlopt->privateData.hostdevNew())) - goto error; - return def; error: @@ -2561,9 +2556,6 @@ void virDomainHostdevDefClear(virDomainHostdevDefPtr def) } break; } - - virObjectUnref(def->privateData); - def->privateData = NULL; } void virDomainTPMDefFree(virDomainTPMDefPtr def) @@ -14415,7 +14407,7 @@ virDomainHostdevDefParseXML(virDomainXMLOptionPtr xmlopt, ctxt->node = node; - if (!(def = virDomainHostdevDefNew(xmlopt))) + if (!(def = virDomainHostdevDefNew())) goto error; if (mode) { diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index a962a5b28..bcd141b9f 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -439,7 +439,6 @@ struct _virDomainHostdevCaps { /* basic device for direct passthrough */ struct _virDomainHostdevDef { virDomainDeviceDef parent; /* higher level Def containing this */ - virObjectPtr privateData; int mode; /* enum virDomainHostdevMode */ int startupPolicy; /* enum virDomainStartupPolicy */ @@ -2618,7 +2617,6 @@ struct _virDomainXMLPrivateDataCallbacks { * virDomainDefCopy and similar functions */ virDomainXMLPrivateDataNewFunc diskNew; virDomainXMLPrivateDataNewFunc diskSrcNew; - virDomainXMLPrivateDataNewFunc hostdevNew; virDomainXMLPrivateDataNewFunc vcpuNew; virDomainXMLPrivateDataNewFunc chrSourceNew; virDomainXMLPrivateDataFormatFunc format; @@ -2739,7 +2737,7 @@ void virDomainNVRAMDefFree(virDomainNVRAMDefPtr def); void virDomainWatchdogDefFree(virDomainWatchdogDefPtr def); virDomainVideoDefPtr virDomainVideoDefNew(void); void virDomainVideoDefFree(virDomainVideoDefPtr def); -virDomainHostdevDefPtr virDomainHostdevDefNew(virDomainXMLOptionPtr xmlopt); +virDomainHostdevDefPtr virDomainHostdevDefNew(void); void virDomainHostdevDefClear(virDomainHostdevDefPtr def); void virDomainHostdevDefFree(virDomainHostdevDefPtr def); void virDomainHubDefFree(virDomainHubDefPtr def); diff --git a/src/lxc/lxc_native.c b/src/lxc/lxc_native.c index 5fc6e7cda..033dd427c 100644 --- a/src/lxc/lxc_native.c +++ b/src/lxc/lxc_native.c @@ -394,7 +394,7 @@ lxcCreateNetDef(const char *type, static virDomainHostdevDefPtr lxcCreateHostdevDef(int mode, int type, const char *data) { - virDomainHostdevDefPtr hostdev = virDomainHostdevDefNew(NULL); + virDomainHostdevDefPtr hostdev = virDomainHostdevDefNew(); if (!hostdev) return NULL; diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index e0e04e2d8..eb2c972e9 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -958,49 +958,6 @@ qemuDomainDiskSrcPrivateDispose(void *obj) } -static virClassPtr qemuDomainHostdevPrivateClass; -static void qemuDomainHostdevPrivateDispose(void *obj); - -static int -qemuDomainHostdevPrivateOnceInit(void) -{ - qemuDomainHostdevPrivateClass = - virClassNew(virClassForObject(), - "qemuDomainHostdevPrivate", - sizeof(qemuDomainHostdevPrivate), - qemuDomainHostdevPrivateDispose); - if (!qemuDomainHostdevPrivateClass) - return -1; - else - return 0; -} - -VIR_ONCE_GLOBAL_INIT(qemuDomainHostdevPrivate) - -static virObjectPtr -qemuDomainHostdevPrivateNew(void) -{ - qemuDomainHostdevPrivatePtr priv; - - if (qemuDomainHostdevPrivateInitialize() < 0) - return NULL; - - if (!(priv = virObjectNew(qemuDomainHostdevPrivateClass))) - return NULL; - - return (virObjectPtr) priv; -} - - -static void -qemuDomainHostdevPrivateDispose(void *obj) -{ - qemuDomainHostdevPrivatePtr priv = obj; - - qemuDomainSecretInfoFree(&priv->secinfo); -} - - static virClassPtr qemuDomainVcpuPrivateClass; static void qemuDomainVcpuPrivateDispose(void *obj); @@ -1413,14 +1370,14 @@ qemuDomainSecretDiskPrepare(virConnectPtr conn, void qemuDomainSecretHostdevDestroy(virDomainHostdevDefPtr hostdev) { - qemuDomainDiskSrcPrivatePtr diskSrcPriv; - if (virHostdevIsSCSIDevice(hostdev)) { virDomainHostdevSubsysSCSIPtr scsisrc = &hostdev->source.subsys.u.scsi; virDomainHostdevSubsysSCSIiSCSIPtr iscsisrc = &scsisrc->u.iscsi; if (scsisrc->protocol == VIR_DOMAIN_HOSTDEV_SCSI_PROTOCOL_TYPE_ISCSI) { - diskSrcPriv = QEMU_DOMAIN_DISK_SRC_PRIVATE(iscsisrc->src); + qemuDomainDiskSrcPrivatePtr diskSrcPriv = + QEMU_DOMAIN_DISK_SRC_PRIVATE(iscsisrc->src); + if (diskSrcPriv && diskSrcPriv->secinfo) qemuDomainSecretInfoFree(&diskSrcPriv->secinfo); } @@ -2316,7 +2273,6 @@ virDomainXMLPrivateDataCallbacks virQEMUDriverPrivateDataCallbacks = { .diskNew = qemuDomainDiskPrivateNew, .diskSrcNew = qemuDomainDiskSrcPrivateNew, .vcpuNew = qemuDomainVcpuPrivateNew, - .hostdevNew = qemuDomainHostdevPrivateNew, .chrSourceNew = qemuDomainChrSourcePrivateNew, .parse = qemuDomainObjPrivateXMLParse, .format = qemuDomainObjPrivateXMLFormat, diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index 81d30ab63..46a8c39d5 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -368,10 +368,6 @@ struct _qemuDomainDiskSrcPrivate { qemuDomainSecretInfoPtr encinfo; }; -# define QEMU_DOMAIN_HOSTDEV_PRIVATE(hostdev) \ - ((qemuDomainHostdevPrivatePtr) (hostdev)->privateData) - - typedef struct _qemuDomainVcpuPrivate qemuDomainVcpuPrivate; typedef qemuDomainVcpuPrivate *qemuDomainVcpuPrivatePtr; struct _qemuDomainVcpuPrivate { @@ -406,16 +402,6 @@ struct qemuDomainDiskInfo { char *nodename; }; -typedef struct _qemuDomainHostdevPrivate qemuDomainHostdevPrivate; -typedef qemuDomainHostdevPrivate *qemuDomainHostdevPrivatePtr; -struct _qemuDomainHostdevPrivate { - virObject parent; - - /* for hostdev storage devices using auth/secret - * NB: *not* to be written to qemu domain object XML */ - qemuDomainSecretInfoPtr secinfo; -}; - # define QEMU_DOMAIN_CHR_SOURCE_PRIVATE(dev) \ ((qemuDomainChrSourcePrivatePtr) (dev)->privateData) diff --git a/src/qemu/qemu_parse_command.c b/src/qemu/qemu_parse_command.c index c9112dd90..9f28d1dd6 100644 --- a/src/qemu/qemu_parse_command.c +++ b/src/qemu/qemu_parse_command.c @@ -1166,7 +1166,7 @@ qemuParseCommandLinePCI(const char *val) int bus = 0, slot = 0, func = 0; const char *start; char *end; - virDomainHostdevDefPtr def = virDomainHostdevDefNew(NULL); + virDomainHostdevDefPtr def = virDomainHostdevDefNew(); if (!def) goto error; @@ -1216,7 +1216,7 @@ qemuParseCommandLinePCI(const char *val) static virDomainHostdevDefPtr qemuParseCommandLineUSB(const char *val) { - virDomainHostdevDefPtr def = virDomainHostdevDefNew(NULL); + virDomainHostdevDefPtr def = virDomainHostdevDefNew(); virDomainHostdevSubsysUSBPtr usbsrc; int first = 0, second = 0; const char *start; diff --git a/src/vbox/vbox_common.c b/src/vbox/vbox_common.c index 92ee37164..3ffaab857 100644 --- a/src/vbox/vbox_common.c +++ b/src/vbox/vbox_common.c @@ -2989,7 +2989,7 @@ vboxHostDeviceGetXMLDesc(vboxDriverPtr data, virDomainDefPtr def, IMachine *mach goto release_filters; for (i = 0; i < def->nhostdevs; i++) { - def->hostdevs[i] = virDomainHostdevDefNew(NULL); + def->hostdevs[i] = virDomainHostdevDefNew(); if (!def->hostdevs[i]) goto release_hostdevs; } diff --git a/src/xenconfig/xen_common.c b/src/xenconfig/xen_common.c index 6d7dc2cde..1ad993539 100644 --- a/src/xenconfig/xen_common.c +++ b/src/xenconfig/xen_common.c @@ -458,7 +458,7 @@ xenParsePCI(virConfPtr conf, virDomainDefPtr def) goto skippci; if (virStrToLong_i(func, NULL, 16, &funcID) < 0) goto skippci; - if (!(hostdev = virDomainHostdevDefNew(NULL))) + if (!(hostdev = virDomainHostdevDefNew())) return -1; hostdev->managed = false; diff --git a/src/xenconfig/xen_sxpr.c b/src/xenconfig/xen_sxpr.c index fefa61ac2..5e7a386b1 100644 --- a/src/xenconfig/xen_sxpr.c +++ b/src/xenconfig/xen_sxpr.c @@ -1110,7 +1110,7 @@ xenParseSxprPCI(virDomainDefPtr def, goto error; } - if (!(dev = virDomainHostdevDefNew(NULL))) + if (!(dev = virDomainHostdevDefNew())) goto error; dev->mode = VIR_DOMAIN_HOSTDEV_MODE_SUBSYS; diff --git a/src/xenconfig/xen_xl.c b/src/xenconfig/xen_xl.c index 8acbfe3f6..64011d9cb 100644 --- a/src/xenconfig/xen_xl.c +++ b/src/xenconfig/xen_xl.c @@ -733,7 +733,7 @@ xenParseXLUSB(virConfPtr conf, virDomainDefPtr def) goto skipusb; if (virStrToLong_i(device, NULL, 16, &devNum) < 0) goto skipusb; - if (!(hostdev = virDomainHostdevDefNew(NULL))) + if (!(hostdev = virDomainHostdevDefNew())) return -1; hostdev->managed = false; diff --git a/tests/virhostdevtest.c b/tests/virhostdevtest.c index 0ad58ddf3..66a0a20e3 100644 --- a/tests/virhostdevtest.c +++ b/tests/virhostdevtest.c @@ -88,7 +88,7 @@ myInit(void) for (i = 0; i < nhostdevs; i++) { virDomainHostdevSubsys subsys; - hostdevs[i] = virDomainHostdevDefNew(NULL); + hostdevs[i] = virDomainHostdevDefNew(); if (!hostdevs[i]) goto cleanup; hostdevs[i]->mode = VIR_DOMAIN_HOSTDEV_MODE_SUBSYS; -- 2.13.5

Add the capability to use the blockdev-add query-qmp-schema option to find the 'password-secret' parameter that will allow the iSCSI code to use the master secret object to encrypt the secret for an and only need to provide the object id of the secret on the command line thus obsfuscating the passphrase. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_capabilities.c | 2 ++ src/qemu/qemu_capabilities.h | 1 + tests/qemucapabilitiesdata/caps_2.10.0.s390x.xml | 1 + tests/qemucapabilitiesdata/caps_2.10.0.x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_2.9.0.ppc64le.xml | 1 + tests/qemucapabilitiesdata/caps_2.9.0.s390x.xml | 1 + tests/qemucapabilitiesdata/caps_2.9.0.x86_64.xml | 1 + 7 files changed, 8 insertions(+) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 085910dd4..3d9a8119d 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -442,6 +442,7 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST, /* 270 */ "vxhs", + "iscsi.password-secret", ); @@ -1802,6 +1803,7 @@ static struct virQEMUCapsStringFlags virQEMUCapsQMPSchemaQueries[] = { { "blockdev-add/arg-type/options/+gluster/debug-level", QEMU_CAPS_GLUSTER_DEBUG_LEVEL}, { "blockdev-add/arg-type/+gluster/debug", QEMU_CAPS_GLUSTER_DEBUG_LEVEL}, { "blockdev-add/arg-type/+vxhs", QEMU_CAPS_VXHS}, + { "blockdev-add/arg-type/+iscsi/password-secret", QEMU_CAPS_ISCSI_PASSWORD_SECRET }, }; struct virQEMUCapsObjectTypeProps { diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index 214734ff2..43f96e88f 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -428,6 +428,7 @@ typedef enum { /* 270 */ QEMU_CAPS_VXHS, /* -drive file.driver=vxhs via query-qmp-schema */ + QEMU_CAPS_ISCSI_PASSWORD_SECRET, /* -drive file.driver=iscsi,...,password-secret= */ QEMU_CAPS_LAST /* this must always be the last item */ } virQEMUCapsFlags; diff --git a/tests/qemucapabilitiesdata/caps_2.10.0.s390x.xml b/tests/qemucapabilitiesdata/caps_2.10.0.s390x.xml index 2806345b9..cf242f2df 100644 --- a/tests/qemucapabilitiesdata/caps_2.10.0.s390x.xml +++ b/tests/qemucapabilitiesdata/caps_2.10.0.s390x.xml @@ -140,6 +140,7 @@ <flag name='chardev-reconnect'/> <flag name='virtio-gpu.max_outputs'/> <flag name='vxhs'/> + <flag name='iscsi.password-secret'/> <version>2010000</version> <kvmVersion>0</kvmVersion> <package></package> diff --git a/tests/qemucapabilitiesdata/caps_2.10.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_2.10.0.x86_64.xml index 8a31431c0..0f02e231e 100644 --- a/tests/qemucapabilitiesdata/caps_2.10.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_2.10.0.x86_64.xml @@ -223,6 +223,7 @@ <flag name='chardev-reconnect'/> <flag name='virtio-gpu.max_outputs'/> <flag name='vxhs'/> + <flag name='iscsi.password-secret'/> <version>2010000</version> <kvmVersion>0</kvmVersion> <package> (v2.10.0)</package> diff --git a/tests/qemucapabilitiesdata/caps_2.9.0.ppc64le.xml b/tests/qemucapabilitiesdata/caps_2.9.0.ppc64le.xml index a373a6db6..c5eb3951f 100644 --- a/tests/qemucapabilitiesdata/caps_2.9.0.ppc64le.xml +++ b/tests/qemucapabilitiesdata/caps_2.9.0.ppc64le.xml @@ -172,6 +172,7 @@ <flag name='vnc-multi-servers'/> <flag name='chardev-reconnect'/> <flag name='virtio-gpu.max_outputs'/> + <flag name='iscsi.password-secret'/> <version>2009000</version> <kvmVersion>0</kvmVersion> <package> (v2.9.0)</package> diff --git a/tests/qemucapabilitiesdata/caps_2.9.0.s390x.xml b/tests/qemucapabilitiesdata/caps_2.9.0.s390x.xml index e80782cfb..99ad44ac5 100644 --- a/tests/qemucapabilitiesdata/caps_2.9.0.s390x.xml +++ b/tests/qemucapabilitiesdata/caps_2.9.0.s390x.xml @@ -137,6 +137,7 @@ <flag name='vnc-multi-servers'/> <flag name='chardev-reconnect'/> <flag name='virtio-gpu.max_outputs'/> + <flag name='iscsi.password-secret'/> <version>2009000</version> <kvmVersion>0</kvmVersion> <package></package> diff --git a/tests/qemucapabilitiesdata/caps_2.9.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_2.9.0.x86_64.xml index 3641d0332..bd446ff27 100644 --- a/tests/qemucapabilitiesdata/caps_2.9.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_2.9.0.x86_64.xml @@ -220,6 +220,7 @@ <flag name='vnc-multi-servers'/> <flag name='chardev-reconnect'/> <flag name='virtio-gpu.max_outputs'/> + <flag name='iscsi.password-secret'/> <version>2009000</version> <kvmVersion>0</kvmVersion> <package> (v2.9.0)</package> -- 2.13.5

Create a qemu* specific StorageSourceCopy helper because we need to be able to copy the PrivateData too if it exists without adding any knowledge to the virStorageSourceCopy function. Signed-off-by: John Ferlan <jferlan@redhat.com> --- Naturally I realized today after sending this yesterday that the virStorageSourceCopy of privateData needed to be addressed. This patch can either be squashed into patch 2 or it can be applied right after patch2. If really desired I can send a whole new series - although it really shouldn't be too difficult to apply after patch2 once the whole series is applied. There's also a patch to be squashed into Patch 7 that would perform the copy for the encinfo data. src/qemu/qemu_blockjob.c | 2 +- src/qemu/qemu_domain.c | 63 +++++++++++++++++++++++++++++++++++++++++++++++- src/qemu/qemu_domain.h | 4 +++ src/qemu/qemu_driver.c | 8 +++--- 4 files changed, 71 insertions(+), 6 deletions(-) diff --git a/src/qemu/qemu_blockjob.c b/src/qemu/qemu_blockjob.c index 415768ddc..c08d60a24 100644 --- a/src/qemu/qemu_blockjob.c +++ b/src/qemu/qemu_blockjob.c @@ -125,7 +125,7 @@ qemuBlockJobEventProcess(virQEMUDriverPtr driver, if ((persistDisk = virDomainDiskByName(vm->newDef, disk->dst, false))) { - copy = virStorageSourceCopy(disk->mirror, false); + copy = qemuDomainStorageSourceCopy(disk->mirror, false); if (!copy || virStorageSourceInitChainElement(copy, persistDisk->src, diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 9945778d9..290f337d4 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -818,7 +818,6 @@ qemuDomainMasterKeyRemove(qemuDomainObjPrivatePtr priv) * @vm: Pointer to the domain object * * As long as the underlying qemu has the secret capability, - * generate and store 'raw' in a file a random 32-byte key to * be used as a secret shared with qemu to share sensitive data. * * Returns: 0 on success, -1 w/ error message on failure @@ -883,6 +882,39 @@ qemuDomainSecretInfoFree(qemuDomainSecretInfoPtr *secinfo) } +static qemuDomainSecretInfoPtr +qemuDomainSecretInfoCopy(qemuDomainSecretInfoPtr src) +{ + qemuDomainSecretInfoPtr dst = NULL; + if (VIR_ALLOC(dst) < 0) + return NULL; + + dst->type = src->type; + if (src->type == VIR_DOMAIN_SECRET_INFO_TYPE_PLAIN) { + if (VIR_STRDUP(dst->s.plain.username, src->s.plain.username) < 0) + goto error; + + if (VIR_ALLOC_N(dst->s.plain.secret, src->s.plain.secretlen) < 0) + goto error; + + memcpy(dst->s.plain.secret, src->s.plain.secret, src->s.plain.secretlen); + dst->s.plain.secretlen = src->s.plain.secretlen; + } else { + if (VIR_STRDUP(dst->s.aes.username, src->s.aes.username) < 0 || + VIR_STRDUP(dst->s.aes.alias, src->s.aes.alias) < 0 || + VIR_STRDUP(dst->s.aes.iv, src->s.aes.alias) < 0 || + VIR_STRDUP(dst->s.aes.ciphertext, src->s.aes.ciphertext) < 0) + goto error; + } + + return dst; + + error: + qemuDomainSecretInfoFree(&dst); + return NULL; +} + + static virClassPtr qemuDomainDiskPrivateClass; static void qemuDomainDiskPrivateDispose(void *obj); @@ -959,6 +991,35 @@ qemuDomainDiskSrcPrivateNew(void) } +virStorageSourcePtr +qemuDomainStorageSourceCopy(const virStorageSource *src, + bool backingChain) +{ + qemuDomainDiskSrcPrivatePtr srcPriv = QEMU_DOMAIN_DISK_SRC_PRIVATE(src); + virStorageSourcePtr dst; + qemuDomainDiskSrcPrivatePtr dstPriv; + + if (!(dst = virStorageSourceCopy(src, backingChain))) + return NULL; + + if (!srcPriv->secinfo) + return dst; + + if (!(dst->privateData = qemuDomainDiskSrcPrivateNew())) + goto error; + + dstPriv = QEMU_DOMAIN_DISK_SRC_PRIVATE(dst); + if (!(dstPriv->secinfo = qemuDomainSecretInfoCopy(srcPriv->secinfo))) + goto error; + + return dst; + + error: + virStorageSourceFree(dst); + return NULL; +} + + static void qemuDomainDiskSrcPrivateDispose(void *obj) { diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index f2c086d5d..c31994c18 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -814,6 +814,10 @@ void qemuDomainMasterKeyRemove(qemuDomainObjPrivatePtr priv); void qemuDomainSecretInfoFree(qemuDomainSecretInfoPtr *secinfo) ATTRIBUTE_NONNULL(1); +virStorageSourcePtr +qemuDomainStorageSourceCopy(const virStorageSource *src, + bool backingChain); + void qemuDomainSecretDiskDestroy(virDomainDiskDefPtr disk) ATTRIBUTE_NONNULL(1); diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 4855c9047..d7ea9a32f 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -345,7 +345,7 @@ qemuSecurityChownCallback(const virStorageSource *src, if (chown(src->path, uid, gid) < 0) goto cleanup; } else { - if (!(cpy = virStorageSourceCopy(src, false))) + if (!(cpy = qemuDomainStorageSourceCopy(src, false))) goto cleanup; /* src file init reports errors, return -2 on failure */ @@ -14392,7 +14392,7 @@ qemuDomainSnapshotDiskDataCollect(virQEMUDriverPtr driver, dd->disk = vm->def->disks[i]; - if (!(dd->src = virStorageSourceCopy(snap->def->disks[i].src, false))) + if (!(dd->src = qemuDomainStorageSourceCopy(snap->def->disks[i].src, false))) goto error; if (virStorageSourceInitChainElement(dd->src, dd->disk->src, false) < 0) @@ -14421,7 +14421,7 @@ qemuDomainSnapshotDiskDataCollect(virQEMUDriverPtr driver, (dd->persistdisk = virDomainDiskByName(vm->newDef, dd->disk->dst, false))) { - if (!(dd->persistsrc = virStorageSourceCopy(dd->src, false))) + if (!(dd->persistsrc = qemuDomainStorageSourceCopy(dd->src, false))) goto error; if (virStorageSourceInitChainElement(dd->persistsrc, @@ -17438,7 +17438,7 @@ qemuDomainBlockCommit(virDomainPtr dom, /* For an active commit, clone enough of the base to act as the mirror */ if (topSource == disk->src) { - if (!(mirror = virStorageSourceCopy(baseSource, false))) + if (!(mirror = qemuDomainStorageSourceCopy(baseSource, false))) goto endjob; if (virStorageSourceInitChainElement(mirror, disk->src, -- 2.13.5