This is not a properly formatted patch. Please use git send-email for submissions. Also read https://libvirt.org/hacking.html for coding style guidelines ... On Tue, Dec 12, 2017 at 16:53:48 +0800, zhangshengyu@fusionstack.cn wrote:

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 66e21c4bd..c83ce5718 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -4554,6 +4554,11 @@ virDomainDeviceDefPostParseCommon(virDomainDeviceDefPtr dev, disk->src->protocol == VIR_STORAGE_NET_PROTOCOL_ISCSI && virDomainPostParseCheckISCSIPath(&disk->src->path) < 0) return -1; + + if (disk->src->type == VIR_STORAGE_TYPE_NETWORK && + disk->src->protocol == VIR_STORAGE_NET_PROTOCOL_ISER && + virDomainPostParseCheckISCSIPath(&disk->src->path) < 0) + return -1;

... like how to format the code properly.

if (disk->bus != VIR_DOMAIN_DISK_BUS_VIRTIO && virDomainCheckVirtioOptions(disk->virtio) < 0) @@ -5160,7 +5165,8 @@ virDomainDiskDefValidate(const virDomainDiskDef *disk) if (!(disk->src->type == VIR_STORAGE_TYPE_BLOCK || disk->src->type == VIR_STORAGE_TYPE_VOLUME || (disk->src->type == VIR_STORAGE_TYPE_NETWORK && - disk->src->protocol == VIR_STORAGE_NET_PROTOCOL_ISCSI))) { + (disk->src->protocol == VIR_STORAGE_NET_PROTOCOL_ISCSI || + disk->src->protocol == VIR_STORAGE_NET_PROTOCOL_ISER)))) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("disk '%s' improperly configured for a " "device='lun'"), disk->dst); diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c index 585f0255e..a9543293d 100644 --- a/src/qemu/qemu_block.c +++ b/src/qemu/qemu_block.c @@ -831,7 +831,7 @@ qemuBlockStorageSourceGetISCSIProps(virStorageSourcePtr src) "s:portal", portal, "s:target", target, "u:lun", lun, - "s:transport", "tcp", + "s:transport", src->protocol == VIR_STORAGE_NET_PROTOCOL_ISER? "iser":"tcp",

too long line.

"S:user", username, "S:password-secret", objalias, NULL)); @@ -1030,6 +1030,7 @@ qemuBlockStorageSourceGetBackendProps(virStorageSourcePtr src) break;

case VIR_STORAGE_NET_PROTOCOL_ISCSI: + case VIR_STORAGE_NET_PROTOCOL_ISER:

Is it really a new protocol? Isn't it just a variation of ISCSI. Qemu treats it as a variation.

if (!(fileprops = qemuBlockStorageSourceGetISCSIProps(src))) return NULL; break;

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index d7150cae1..76b2a6cc9 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -8133,7 +8133,7 @@ int qemuDomainDefValidateDiskLunSource(const virStorageSource *src) { if (virStorageSourceGetActualType(src) == VIR_STORAGE_TYPE_NETWORK) { - if (src->protocol != VIR_STORAGE_NET_PROTOCOL_ISCSI) { + if (src->protocol != VIR_STORAGE_NET_PROTOCOL_ISCSI && src->protocol != VIR_STORAGE_NET_PROTOCOL_ISER) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("disk device='lun' is not supported " "for protocol='%s'"),

[...]

diff --git a/src/qemu/qemu_parse_command.c b/src/qemu/qemu_parse_command.c index 5fe3f97d0..72e137ce4 100644 --- a/src/qemu/qemu_parse_command.c +++ b/src/qemu/qemu_parse_command.c @@ -709,6 +709,12 @@ qemuParseCommandLineDisk(virDomainXMLOptionPtr xmlopt,

if (qemuParseISCSIString(def) < 0) goto error; + } else if (STRPREFIX(def->src->path, "iser:")) { + def->src->type = VIR_STORAGE_TYPE_NETWORK; + def->src->protocol = VIR_STORAGE_NET_PROTOCOL_ISER; + + if (qemuParseISCSIString(def) < 0) + goto error; } else if (STRPREFIX(def->src->path, "sheepdog:")) { char *p = def->src->path; char *port, *vdi; @@ -2157,6 +2163,7 @@ qemuParseCommandLine(virCapsPtr caps, goto error;

break; + case VIR_STORAGE_NET_PROTOCOL_ISER: case VIR_STORAGE_NET_PROTOCOL_ISCSI: if (qemuParseISCSIString(disk) < 0) goto error; diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c index 6594715e5..cf5c2904e 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c

[...]

@@ -3295,6 +3368,7 @@ static const struct virStorageSourceJSONDriverParser jsonParsers[] = { {"tftp", virStorageSourceParseBackingJSONUri, VIR_STORAGE_NET_PROTOCOL_TFTP}, {"gluster", virStorageSourceParseBackingJSONGluster, 0}, {"iscsi", virStorageSourceParseBackingJSONiSCSI, 0}, + {"iser", virStorageSourceParseBackingJSONiSER, 0},

So this is plain wrong. In qemu it's stil protocol called 'iscsi' with a transport of 'iser', not a completely new protocol. citation from qemu.git/qmp/block-core.json: ## # @IscsiTransport: # # An enumeration of libiscsi transport types # # Since: 2.9 ## { 'enum': 'IscsiTransport', 'data': [ 'tcp', 'iser' ] } ## # @IscsiHeaderDigest: # # An enumeration of header digests supported by libiscsi # # Since: 2.9 ## { 'enum': 'IscsiHeaderDigest', 'prefix': 'QAPI_ISCSI_HEADER_DIGEST', 'data': [ 'crc32c', 'none', 'crc32c-none', 'none-crc32c' ] } ## # @BlockdevOptionsIscsi: # # @transport: The iscsi transport type # # @portal: The address of the iscsi portal # # @target: The target iqn name # # @lun: LUN to connect to. Defaults to 0. # # @user: User name to log in with. If omitted, no CHAP # authentication is performed. # # @password-secret: The ID of a QCryptoSecret object providing # the password for the login. This option is required if # @user is specified. # # @initiator-name: The iqn name we want to identify to the target # as. If this option is not specified, an initiator name is # generated automatically. # # @header-digest: The desired header digest. Defaults to # none-crc32c. # # @timeout: Timeout in seconds after which a request will # timeout. 0 means no timeout and is the default. # # Driver specific block device options for iscsi # # Since: 2.9 ## { 'struct': 'BlockdevOptionsIscsi', 'data': { 'transport': 'IscsiTransport', 'portal': 'str', 'target': 'str', '*lun': 'int', '*user': 'str', '*password-secret': 'str', '*initiator-name': 'str', '*header-digest': 'IscsiHeaderDigest', '*timeout': 'int' } } Also it's missing a test case.

{"nbd", virStorageSourceParseBackingJSONNbd, 0}, {"sheepdog", virStorageSourceParseBackingJSONSheepdog, 0}, {"ssh", virStorageSourceParseBackingJSONSSH, 0},

I don't object to adding iser transport per-se, but this patch is a mess. It's missing proper commit message and the protocol is not a new protocol but rather a different transport for ISCSI. Please follow the contributor guidelines before submitting a new patch. Also please add a case to the qemuxml2argv test so that this addition is tested properly as I suspect that iser would work only if you enable authentication in current libvirt since you did not tweak qemuDiskSourceNeedsProps Peter