Re: [PATCH 2/9] Add a <graphics> type for SPICE protocol

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

 



On 11/08/2010 12:27 PM, Daniel P. Berrange wrote:
> This adds an element
> 
>  <graphics type='spice' port='5903' tlsPort='5904' autoport='yes' listen='127.0.0.1'/>
> 
> This is the bare minimum that should be exposed in the guest
> config for SPICE. Other parameters are better handled as per
> host level configuration tunables
> 
> @@ -3214,6 +3221,50 @@ virDomainGraphicsDefParseXML(xmlNodePtr node, int flags) {
>              def->data.desktop.fullscreen = 0;
>  
>          def->data.desktop.display = virXMLPropString(node, "display");
> +    } else if (def->type == VIR_DOMAIN_GRAPHICS_TYPE_SPICE) {
> +        char *port = virXMLPropString(node, "port");
> +        char *tlsPort;
> +        char *autoport;
> +
> +        if (port) {
> +            if (virStrToLong_i(port, NULL, 10, &def->data.spice.port) < 0) {
> +                virDomainReportError(VIR_ERR_INTERNAL_ERROR,
> +                                     _("cannot parse spice port %s"), port);
> +                VIR_FREE(port);
> +                goto error;
> +            }

Still missing validation that data.spice.port got assigned something
less than 0x10000.  But I'm not sure it should hold up the series.

> @@ -544,6 +545,14 @@ struct _virDomainGraphicsDef {
>              char *display;
>              unsigned int fullscreen :1;
>          } desktop;
> +        struct {
> +            int port;
> +            int tlsPort;
> +            char *listenAddr;

No response to my question in 3/10 about whether we should use
virSocketAddr instead of a raw string?
https://www.redhat.com/archives/libvir-list/2010-November/msg00154.html

But still something that could be added as a later patch.  What you have
is not inherently wrong, so:

ACK.

-- 
Eric Blake   eblake@xxxxxxxxxx    +1-801-349-2682
Libvirt virtualization library http://libvirt.org

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]