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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxxxx>
> ---
>  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

Attachment: signature.asc
Description: OpenPGP digital signature

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]