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@xxxxxxxxxxxxxx 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
Attachment:
signature.asc
Description: PGP signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
