[libvirt] [PATCH] qemu: add port check for iscsi host when disk type is network
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@redhat.com> --- 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; + } + case VIR_STORAGE_NET_PROTOCOL_GLUSTER: if (nhosts != 1) { virReportError(VIR_ERR_INTERNAL_ERROR, -- 1.9.3
On 11/13/2014 10:28 AM, 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@redhat.com> --- 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; + } +
NACK We need to allow other ports than 3260. (And your check would be done for FTP, HTTP and all the other protocols that fall through to the _ISCI case) Jan
case VIR_STORAGE_NET_PROTOCOL_GLUSTER: if (nhosts != 1) { virReportError(VIR_ERR_INTERNAL_ERROR,
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@redhat.com> --- 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.
+ case VIR_STORAGE_NET_PROTOCOL_GLUSTER: if (nhosts != 1) { virReportError(VIR_ERR_INTERNAL_ERROR,
NACK to this approach. Peter
On 11/13/2014 05:42 PM, Peter Krempa 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@redhat.com> --- 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
On 11/13/14 10:28, Shanzhi Yu wrote: 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 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
+ case VIR_STORAGE_NET_PROTOCOL_GLUSTER: if (nhosts != 1) { virReportError(VIR_ERR_INTERNAL_ERROR,
NACK to this approach.
Peter
-- Regards shyu
On 11/13/14 11:17, Shanzhi Yu wrote:
On 11/13/2014 05:42 PM, Peter Krempa 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@redhat.com> --- 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
On 11/13/14 10:28, Shanzhi Yu wrote: 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
On 11/13/2014 06:21 PM, Peter Krempa wrote:
On 11/13/14 11:17, Shanzhi Yu wrote:
On 11/13/2014 05:42 PM, Peter Krempa 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@redhat.com> --- 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
On 11/13/14 10:28, Shanzhi Yu wrote: 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.
Yes, I see.
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.
I misunderstand the error info. Thanks for explanation
Peter
-- Regards shyu
participants (3)
-
Ján Tomko -
Peter Krempa -
Shanzhi Yu