On 07/21/2014 02:47 PM, John Ferlan wrote:
> Introduce a new structure to handle an iSCSI host device based on the
> existing virDomainHostdevSubsysSCSI by adding a "protocol='iscsi'" to
> the <source/> element. The hostdev structure mimics the existing
> <disk/> element for an iSCSI device (network) device. New XML is:
>
> <hostdev mode='subsystem' type='scsi' managed='yes'>
> <auth username='myname'>
> <secret type='iscsi' usage='mycluster_myname'/>
> </auth>
> <source protocol='iscsi' name='iqn.1992-01.com.example'>
> <host name='example.org' port='3260'/>
> </source>
> <address type='drive' controller='0' bus='0' target='2' unit='5'/>
> </hostdev>
>
> The controller element will mimic the existing scsi_host code insomuch
> as when 'lsi' and 'virtio-scsi' are used.
>
> Signed-off-by: John Ferlan <jferlan@xxxxxxxxxx>
> ---
> docs/formatdomain.html.in | 142 ++++++++++++-------
> docs/schemas/domaincommon.rng | 46 ++++++-
> src/conf/domain_conf.c | 152 ++++++++++++++++++---
> .../qemuxml2argv-hostdev-scsi-lsi-iscsi-auth.args | 14 ++
> .../qemuxml2argv-hostdev-scsi-lsi-iscsi-auth.xml | 46 +++++++
> .../qemuxml2argv-hostdev-scsi-lsi-iscsi.args | 14 ++
> .../qemuxml2argv-hostdev-scsi-lsi-iscsi.xml | 40 ++++++
> ...emuxml2argv-hostdev-scsi-virtio-iscsi-auth.args | 16 +++
> ...qemuxml2argv-hostdev-scsi-virtio-iscsi-auth.xml | 46 +++++++
> .../qemuxml2argv-hostdev-scsi-virtio-iscsi.args | 16 +++
> .../qemuxml2argv-hostdev-scsi-virtio-iscsi.xml | 40 ++++++
> tests/qemuxml2argvtest.c | 16 +++
> tests/qemuxml2xmltest.c | 5 +
> 13 files changed, 524 insertions(+), 69 deletions(-)
> create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-lsi-iscsi-auth.args
> create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-lsi-iscsi-auth.xml
> create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-lsi-iscsi.args
> create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-lsi-iscsi.xml
> create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-virtio-iscsi-auth.args
> create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-virtio-iscsi-auth.xml
> create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-virtio-iscsi.args
> create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-virtio-iscsi.xml
>
> + <dt>usb</dt>
> + <dd>For USB devices, the user is responsible to call
> + <code>virNodeDeviceDettach</code> (or
> + <code>virsh nodedev-dettach</code>) before starting the guest
The (preferred) virsh command is nodedev-detach (we can't correct the
API typo, but did correct the virsh typo).
> + <dt>scsi</dt>
> + <dd>SCSI devices are described by both the <code>adapter</code>
> + and <code>address</code> elements.
> + <p>
> + <span class="since">Since 1.2.7</span>, the <code>source</code>
1.2.8 now.
> +++ b/docs/schemas/domaincommon.rng
> @@ -3563,13 +3563,47 @@
> </choice>
> </attribute>
> </optional>
> + <optional>
> + <ref name='diskAuth'/>
> + </optional>
Does diskAuth work for all configurations, or only for the new iscsi
configuration? By putting it here, you are allowing it for all users,...
> <element name="source">
> - <interleave>
> - <ref name="sourceinfoadapter"/>
> - <element name="address">
> - <ref name="scsiaddress"/>
> - </element>
> - </interleave>
> + <optional>
> + <attribute name="protocol">
> + <choice>
> + <value>iscsi</value>
> + </choice>
...whereas using some <group> magic could make it valid only when
protocol is iscsi.
> + </attribute>
> + <attribute name="name">
> + <text/>
> + </attribute>
> + </optional>
> + <choice>
> + <group>
> + <interleave>
> + <ref name="sourceinfoadapter"/>
> + <element name="address">
> + <ref name="scsiaddress"/>
> + </element>
> + </interleave>
> + </group>
> + <group>
> + <interleave>
> + <oneOrMore>
> + <element name='host'>
> + <attribute name='name'>
> + <text/>
> + </attribute>
> + <optional>
> + <attribute name='port'>
> + <ref name="PortNumber"/>
> + </attribute>
> + </optional>
> + <empty/>
> + </element>
> + </oneOrMore>
> + </interleave>
This <interleave> adds nothing. It doesn't hurt to leave it in, but
when there is only one possible (repetition of a) child "host"
<element>, there is nothing to be interleaved.
> @@ -4205,10 +4210,93 @@ virDomainHostdevSubsysSCSIHostDefParseXML(xmlNodePtr sourcenode,
> }
>
> static int
> +virDomainHostdevSubsysSCSIiSCSIDefParseXML(xmlNodePtr sourcenode,
> + xmlNodePtr authnode,
> + virDomainHostdevSubsysSCSIPtr def)
> +{
> + int ret = -1;
> + int auth_secret_usage = -1;
> + virStorageAuthDefPtr authdef = NULL;
> + virDomainHostdevSubsysSCSIiSCSIPtr iscsisrc = &def->u.iscsi;
> +
> + /* Similar to virDomainDiskSourceParse for a VIR_STORAGE_TYPE_NETWORK */
Is there enough commonality to be worth a common shared function? I'm
okay if there isn't, but want to make sure we aren't doing unneeded
duplication.
> + }
> + if (iscsisrc->nhosts > 1) {
> + virReportError(VIR_ERR_XML_ERROR, "%s",
> + _("only one source host address may be specified "
> + "for the iSCSI hostdev"));
> + goto cleanup;
This doesn't match the .rng file which said <oneOrMore>.
> +static int
> virDomainHostdevSubsysSCSIDefParseXML(xmlNodePtr sourcenode,
> + xmlNodePtr authnode,
> virDomainHostdevSubsysSCSIPtr scsisrc)
> {
> - return virDomainHostdevSubsysSCSIHostDefParseXML(sourcenode, scsisrc);
> + char *protocol = NULL;
> + int ret = -1;
> +
> + if ((protocol = virXMLPropString(sourcenode, "protocol"))) {
> + scsisrc->protocol =
> + virDomainHostdevSubsysSCSIProtocolTypeFromString(protocol);
> + if (scsisrc->protocol < 0) {
> + virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> + _("Unknown SCSI subsystem protcol '%s'"),
s/protcol/protocol/
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list