Re: [libvirt] [PATCH] qemu: add port check for iscsi host when disk type is network

On 11/13/14 11:17, Shanzhi Yu wrote: > On 11/13/2014 05:42 PM, Peter Krempa wrote: >> On 11/13/14 10:28, Shanzhi Yu wrote: >>> For network type disk, host port is not checked when source protocol is >>> iscsi, so the error is not sure when with invalid port. If pass -1 to >>> port >>> the error will be >>> error: Failed to start domain rh6-i >>> error: An error occurred, but the cause is unknown >>> >>> Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1163553 >>> Signed-off-by: Shanzhi Yu <shyu(a)redhat.com&gt; >>> --- >>> src/qemu/qemu_command.c | 6 ++++++ >>> 1 file changed, 6 insertions(+) >>> >>> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c >>> index f674ba9..f806225 100644 >>> --- a/src/qemu/qemu_command.c >>> +++ b/src/qemu/qemu_command.c >>> @@ -3016,6 +3016,12 @@ qemuBuildNetworkDriveURI(int protocol, >>> case VIR_STORAGE_NET_PROTOCOL_FTPS: >>> case VIR_STORAGE_NET_PROTOCOL_TFTP: >>> case VIR_STORAGE_NET_PROTOCOL_ISCSI: >>> + if (STRNEQ_NULLABLE(hosts->port,"3260")) { >>> + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", >>> + _("expected port for iscsi host >>> should be 3260")); >>> + goto cleanup; >>> + } >> Um? this would forbid to use any other port than 3260 for any of the >> protocols stated above. That doesn't make sense neither for iSCSI nor >> for the other ones. >> >> The code should make sure that the port is in range <0,65536> and don't >> force to use a certain port. Ohterwise we wouldn't need a port field. > How about make a check for every protocols above, eg FTP, HTTP. As > known, different protocol use certain port, if just make sure They use certain ports as a default. That doesn't mean the admin can't configure to run the service at a different port.

> the port is in range <0.65535>, error info may be like > > error: Failed to start domain rh6-i > error: internal error: process exited while connecting to monitor: > 2014-11-13T10:06:57.112170Z qemu-system-x86_64: -drive > file=ftp://10.66.6.111:100/mnt/nfs/rhel6.img,if=none,id=drive-virtio-disk0,format=raw,cache=none: > could not open disk image ftp://10.66.6.111:100/mnt/nfs/rhel6.img: curl > block device does not support writes You didn't understand the error message apparently. The message says that the FTP backend in qemu is supported only with read-only disks, not that port "100" is invalid.

Peter