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

[Jiri Denemark]

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 at 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