On Fri, Nov 14, 2014 at 01:24:18 +0800, Shanzhi Yu wrote:
For network type disk, host port is not checked when start a guest, so the error may be unclear 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 So make a check to make sure the port range from 0 to 65536
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1163553 Signed-off-by: Shanzhi Yu <shyu@redhat.com> --- src/qemu/qemu_command.c | 12 ++++++++++++ 1 file changed, 12 insertions(+)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index f674ba9..66082e1 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -3016,6 +3016,18 @@ qemuBuildNetworkDriveURI(int protocol, case VIR_STORAGE_NET_PROTOCOL_FTPS: case VIR_STORAGE_NET_PROTOCOL_TFTP: case VIR_STORAGE_NET_PROTOCOL_ISCSI: + if (VIR_ALLOC(uri) < 0) + goto cleanup;
What is the purpose of this allocation?
+ + if (qemuNetworkDriveGetPort(protocol, hosts->port) < 0 || + qemuNetworkDriveGetPort(protocol, hosts->port) > 65536) {
Either >= 65536 or > 65535.
+ virReportError(VIR_ERR_INTERNAL_ERROR, + _("port should be in range 0 to 65536 for '%s' host"), + virStorageNetProtocolTypeToString(protocol)); +
Extra empty line.
+ goto cleanup; + } +
The placement of this new code is just weired. I think we support port specification even for gluster. Not to mention that some network disks may have multiple hosts specified and we should check port numbers for all of them. In other words, this should be a general check not tight to any protocol.
case VIR_STORAGE_NET_PROTOCOL_GLUSTER: if (nhosts != 1) { virReportError(VIR_ERR_INTERNAL_ERROR,
Jirka